fix(crafter): mark AI coding sessions redacted whenever the secret scan runs

app/cli/internal/policydevel/eval.go

@@ -100,7 +100,8 @@ func Evaluate(opts *EvalOptions, logger zerolog.Logger) (*EvalSummary, error) {
 // crafter produced, rather than replacing them.
 //
 // The crafter's annotations carry more than metadata: chainloop.material.redacted
-// is how a policy learns that secrets were found and stripped out, and it is what
+// is how a policy learns that the content was scanned for secrets and any found
+// were stripped out, and it is what
 // makes content resolution fail closed rather than fall back to the un-redacted
 // file on disk. Dropping it would hide the redaction from the policy and re-open
 // the path this exists to close.

app/cli/internal/policydevel/eval_test.go

@@ -238,13 +238,14 @@ func TestEvaluateSimplifiedPolicies(t *testing.T) {
 const fixtureGitHubPAT = "ghp_erOZlZv0B1e3amrQ" + "ugdwZ8Ro2W4kDql9WPTf"
 
 // writeSessionFixture materialises an AI coding session fixture with its
-// credential placeholder resolved, so that the crafter sees a real secret on disk.
-func writeSessionFixture(t *testing.T) string {
+// credential placeholder resolved to token: fixtureGitHubPAT for the crafter to
+// see a real secret on disk, anything else for a clean session.
+func writeSessionFixture(t *testing.T, token string) string {
 	t.Helper()
 
 	content, err := os.ReadFile("testdata/ai-coding-session-with-secret.json")
 	require.NoError(t, err)
-	content = bytes.ReplaceAll(content, []byte("__GITHUB_PAT__"), []byte(fixtureGitHubPAT))
+	content = bytes.ReplaceAll(content, []byte("__GITHUB_PAT__"), []byte(token))
 
 	path := filepath.Join(t.TempDir(), "ai-coding-session.json")
 	require.NoError(t, os.WriteFile(path, content, 0600))
@@ -279,7 +280,7 @@ func TestEvaluateReadsRedactedMaterial(t *testing.T) {
 			opts := &EvalOptions{
 				PolicyPath:   "testdata/ai-coding-session-no-secrets-policy.yaml",
 				MaterialKind: "CHAINLOOP_AI_CODING_SESSION",
-				MaterialPath: writeSessionFixture(t),
+				MaterialPath: writeSessionFixture(t, fixtureGitHubPAT),
 				Annotations:  tc.annotations,
 				// Debug surfaces the exact bytes handed to the engine, which is
 				// what the strongest assertion below inspects.
@@ -307,6 +308,35 @@ func TestEvaluateReadsRedactedMaterial(t *testing.T) {
 	}
 }
 
+// A session the scan found nothing in is still marked redacted, as `attestation
+// add` marks it, and the marker must not make evaluation fail closed: the policy
+// is evaluated against the scanned content.
+func TestEvaluateCleanScannedMaterial(t *testing.T) {
+	const token = "not-a-credential"
+
+	opts := &EvalOptions{
+		PolicyPath:   "testdata/ai-coding-session-no-secrets-policy.yaml",
+		MaterialKind: "CHAINLOOP_AI_CODING_SESSION",
+		MaterialPath: writeSessionFixture(t, token),
+		Debug:        true,
+	}
+
+	result, err := Evaluate(opts, zerolog.New(os.Stderr))
+	require.NoError(t, err)
+	require.NotNil(t, result)
+
+	assert.False(t, result.Result.Skipped)
+	assert.Empty(t, result.Result.Violations)
+
+	require.NotNil(t, result.DebugInfo)
+	require.NotEmpty(t, result.DebugInfo.Inputs)
+	for _, input := range result.DebugInfo.Inputs {
+		assert.Contains(t, string(input), token)
+		assert.Contains(t, string(input), `"chainloop.material.redacted":"true"`)
+		assert.Contains(t, string(input), `"chainloop.material.redaction.count":"0"`)
+	}
+}
+
 func TestMergeAnnotations(t *testing.T) {
 	testCases := []struct {
 		name     string

pkg/attestation/crafter/api/attestation/v1/crafting_state.go

@@ -59,14 +59,16 @@ var (
 	AnnotationsSBOMMainComponentType    = CreateAnnotation("material.sbom.main_component.type")
 	AnnotationsSBOMMainComponentVersion = CreateAnnotation("material.sbom.main_component.version")
 
-	// AnnotationMaterialRedacted marks a material whose stored content was
-	// transformed by its crafter before upload, to strip secrets out of it. Two
-	// things follow from it: the recorded digest describes the redacted artifact
-	// rather than the file on disk, and policy evaluation must be handed that
-	// sanitized copy explicitly, because the file on disk still holds the secrets
-	// (see GetEvaluableContent, which fails closed without it).
+	// AnnotationMaterialRedacted marks a material whose content went through
+	// secret redaction before upload, whether or not anything was found. When
+	// something was replaced (AnnotationMaterialRedactionCount above zero) the
+	// recorded digest describes the redacted artifact rather than the file on
+	// disk, which still holds the secrets. Either way policy evaluation must be
+	// handed the scanned copy explicitly (see GetEvaluableContent, which fails
+	// closed without it).
 	AnnotationMaterialRedacted = CreateAnnotation("material.redacted")
-	// AnnotationMaterialRedactionCount is how many secrets were replaced.
+	// AnnotationMaterialRedactionCount is how many secrets were replaced, zero
+	// for a scan that found nothing.
 	AnnotationMaterialRedactionCount = CreateAnnotation("material.redaction.count")
 	// AnnotationMaterialRedactionRules lists the detection rules that matched,
 	// so a policy can act on the kind of credential that was present.

pkg/attestation/crafter/crafter.go

@@ -821,9 +821,9 @@ func (c *Crafter) stageMaterial(ctx context.Context, m *schemaapi.CraftingSchema
 		policies.WithDefaultGate(c.CraftingState.Attestation.GetBlockOnPolicyViolation()),
 		policies.WithProjectContext(projectName, projectVersion),
 	)
-	// crafted.Content is what a crafter that did not store the artifact verbatim
-	// stored in its place (an AI coding session with secrets redacted out of it),
-	// and it is what the policies must see. Reading the file at value instead would
+	// crafted.Content is what a crafter that transformed the artifact scanned and
+	// stored (an AI coding session, with any secrets redacted out of it), and it
+	// is what the policies must see. Reading the file at value instead would
 	// feed user-authored Rego the very secrets redaction removed. nil for every
 	// other material, which resolves its content the usual way.
 	policyGroupResults, err := pgv.VerifyMaterial(ctx, mt, value, crafted.Content)

pkg/attestation/crafter/crafter_test.go

@@ -20,6 +20,8 @@ import (
 	"archive/zip"
 	"compress/gzip"
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"fmt"
 	"maps"
 	"os"
@@ -811,6 +813,42 @@ func (s *crafterSuite) TestAddMaterialRedactedSessionIsWhatPoliciesSee() {
 	assert.Contains(s.T(), string(onDisk), githubPAT)
 }
 
+// TestAddMaterialCleanSessionIsMarkedScanned covers a session the scan found
+// nothing in. It is marked redacted, so that policies can tell it apart from a
+// session that was never scanned, and that marker must not make evaluation fail
+// closed: the crafter hands the scanned bytes to the policy engine.
+func (s *crafterSuite) TestAddMaterialCleanSessionIsMarkedScanned() {
+	sessionPath := materializeSessionFixture(s.T(), "./materials/testdata/ai-coding-session.json")
+	onDisk, err := os.ReadFile(sessionPath)
+	require.NoError(s.T(), err)
+
+	uploader := mUploader.NewUploader(s.T())
+	uploader.On("Upload", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+		Return(&casclient.UpDownStatus{Digest: "deadbeef", Filename: "ai-coding-session.json"}, nil)
+	backend := &casclient.CASBackend{Uploader: uploader}
+
+	c, err := newInitializedCrafter(s.T(), "testdata/contracts/with_ai_session_policy.yaml",
+		&v1.WorkflowMetadata{}, false, "", runners.NewGeneric())
+	require.NoError(s.T(), err)
+
+	m, err := c.AddMaterialContractFree(context.Background(), "",
+		schemaapi.CraftingSchema_Material_CHAINLOOP_AI_CODING_SESSION.String(),
+		"ai-session", sessionPath, backend, nil)
+	require.NoError(s.T(), err)
+
+	assert.Equal(s.T(), v1.AnnotationValueTrue, m.Annotations[v1.AnnotationMaterialRedacted])
+	assert.Equal(s.T(), "0", m.Annotations[v1.AnnotationMaterialRedactionCount])
+	assert.NotContains(s.T(), m.Annotations, v1.AnnotationMaterialRedactionSkipped)
+
+	// Nothing was replaced, so the digest is still that of the source file.
+	sum := sha256.Sum256(onDisk)
+	assert.Equal(s.T(), "sha256:"+hex.EncodeToString(sum[:]), m.GetArtifact().GetDigest())
+
+	evaluations := c.CraftingState.Attestation.PolicyEvaluations
+	require.Len(s.T(), evaluations, 1)
+	assert.Empty(s.T(), evaluations[0].Violations)
+}
+
 // materializeSessionFixture writes a copy of an AI coding session fixture with
 // its credential placeholders resolved, and returns its path. The crafter reads
 // the artifact from disk, so the substitution has to land in a real file.

pkg/attestation/crafter/materials/chainloop_ai_coding_session.go

@@ -79,7 +79,7 @@ func NewChainloopAICodingSessionCrafter(schema *schemaapi.CraftingSchema_Materia
 // material definition.
 //
 // The file on disk is left untouched, so it is no longer the stored content once
-// anything was redacted. The sanitized copy is returned as CraftResult.Content
+// anything was redacted. The scanned copy is returned as CraftResult.Content
 // for whoever needs to read the artifact back — today policy evaluation, which
 // must not be handed the credentials the session captured.
 func (c *ChainloopAICodingSessionCrafter) Craft(ctx context.Context, artifactPath string) (*CraftResult, error) {
@@ -115,16 +115,16 @@ func (c *ChainloopAICodingSessionCrafter) Craft(ctx context.Context, artifactPat
 		return nil, fmt.Errorf("AI coding session validation failed: %w", err)
 	}
 
-	redacted, report, err := c.redact(ctx, f)
+	scanned, report, err := c.redact(ctx, f)
 	if err != nil {
 		return nil, err
 	}
 
 	// Substituting the stored content only when something was actually replaced
 	// keeps a clean session's digest reproducible from its source file.
 	var craftOpts []uploadAndCraftOption
-	if redacted != nil {
-		craftOpts = append(craftOpts, withContentOverride(redacted))
+	if report.Changed() {
+		craftOpts = append(craftOpts, withContentOverride(scanned))
 	}
 
 	material, err := uploadAndCraft(ctx, c.input, c.backend, artifactPath, c.logger, craftOpts...)
@@ -147,12 +147,12 @@ func (c *ChainloopAICodingSessionCrafter) Craft(ctx context.Context, artifactPat
 	// Surface how the session was run
 	material.Annotations[annotationAICodingSessionMode] = aicodingsession.ResolveMode(data.Session.Mode)
 
-	return &CraftResult{Material: material, Content: redacted}, nil
+	return &CraftResult{Material: material, Content: scanned}, nil
 }
 
-// redact strips secrets out of the session content, returning the sanitized copy
-// to store in place of the file on disk, or nil when nothing was replaced and the
-// file itself is what gets stored.
+// redact strips secrets out of the session content and returns the scanned
+// copy: the sanitized bytes when something was replaced, the content itself when
+// the scan found nothing, nil when redaction is skipped.
 //
 // Redaction fails closed: if the content cannot be scanned or the result no
 // longer matches the schema, the material is not crafted at all rather than
@@ -170,14 +170,16 @@ func (c *ChainloopAICodingSessionCrafter) redact(ctx context.Context, content []
 	}
 
 	if !report.Changed() {
-		return nil, report, nil
+		return content, report, nil
 	}
 
 	return redacted, report, nil
 }
 
 // annotateRedaction records what redaction did, so that it is visible in the
 // attestation and actionable by policies rather than an invisible rewrite.
+// The material is marked redacted whenever the scan ran, even when it found
+// nothing (see api.AnnotationMaterialRedacted).
 func (c *ChainloopAICodingSessionCrafter) annotateRedaction(material *api.Attestation_Material, report *redaction.Report) {
 	if c.skipRedaction {
 		material.Annotations[api.AnnotationMaterialRedactionSkipped] = api.AnnotationValueTrue
@@ -196,13 +198,14 @@ func (c *ChainloopAICodingSessionCrafter) annotateRedaction(material *api.Attest
 			Msg("some detected secrets could not be redacted")
 	}
 
+	material.Annotations[api.AnnotationMaterialRedacted] = api.AnnotationValueTrue
+	material.Annotations[api.AnnotationMaterialRedactionCount] = strconv.Itoa(report.Replacements)
+
 	if !report.Changed() {
 		return
 	}
 
 	rules := report.RuleIDs()
-	material.Annotations[api.AnnotationMaterialRedacted] = api.AnnotationValueTrue
-	material.Annotations[api.AnnotationMaterialRedactionCount] = strconv.Itoa(report.Replacements)
 	material.Annotations[api.AnnotationMaterialRedactionRules] = strings.Join(rules, ",")
 
 	c.logger.Info().Int("count", report.Replacements).Strs("rules", rules).

pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go

@@ -187,6 +187,8 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) {
 	const (
 		withSecrets = "./aicodingsession/testdata/session-with-secrets.json"
 		clean       = "./testdata/ai-coding-session.json"
+		// The rules that match the secrets in withSecrets.
+		withSecretsRules = "anthropic-api-key,aws-access-token,aws-secret-access-key,github-pat"
 	)
 
 	testCases := []struct {
@@ -195,45 +197,44 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) {
 		skipRedaction bool
 		inlineBackend bool
 		skipUpload    bool
-		wantRedacted  bool
 		wantCount     string
 		wantRules     string
 	}{
 		{
-			name:         "secrets are stripped before upload",
-			filePath:     withSecrets,
-			wantRedacted: true,
-			wantCount:    "7",
-			wantRules:    "anthropic-api-key,aws-access-token,aws-secret-access-key,github-pat",
+			name:      "secrets are stripped before upload",
+			filePath:  withSecrets,
+			wantCount: "7",
+			wantRules: withSecretsRules,
 		},
 		{
 			// An inline backend embeds the content into the attestation itself,
 			// so it matters most there that the redacted copy is the stored one.
 			name:          "an inline backend embeds the redacted copy",
 			filePath:      withSecrets,
 			inlineBackend: true,
-			wantRedacted:  true,
 			wantCount:     "7",
-			wantRules:     "anthropic-api-key,aws-access-token,aws-secret-access-key,github-pat",
+			wantRules:     withSecretsRules,
 		},
 		{
 			// Neither uploaded nor stored inline, so the sanitized copy exists
 			// nowhere but in what the crafter hands back.
-			name:         "skipping the upload still yields the redacted copy",
-			filePath:     withSecrets,
-			skipUpload:   true,
-			wantRedacted: true,
-			wantCount:    "7",
-			wantRules:    "anthropic-api-key,aws-access-token,aws-secret-access-key,github-pat",
+			name:       "skipping the upload still yields the redacted copy",
+			filePath:   withSecrets,
+			skipUpload: true,
+			wantCount:  "7",
+			wantRules:  withSecretsRules,
 		},
 		{
 			name:          "the opt-out is recorded in the attestation",
 			filePath:      withSecrets,
 			skipRedaction: true,
 		},
 		{
-			name:     "a clean session is not marked as redacted",
-			filePath: clean,
+			// The scan ran and found nothing: the material says so, so that a
+			// clean verdict is distinguishable from a session never scanned.
+			name:      "a clean session is marked as scanned with nothing replaced",
+			filePath:  clean,
+			wantCount: "0",
 		},
 	}
 
@@ -281,39 +282,38 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) {
 			assert.Equal(t, tc.wantCount, got.Annotations[api.AnnotationMaterialRedactionCount])
 			assert.Equal(t, tc.wantRules, got.Annotations[api.AnnotationMaterialRedactionRules])
 
-			switch {
-			case tc.wantRedacted:
-				assert.Equal(t, "true", got.Annotations[api.AnnotationMaterialRedacted])
-				// The digest describes the redacted artifact, not the source file.
-				assert.NotEqual(t, sha256Digest(string(original)), got.GetArtifact().Digest)
-
-				// The sanitized copy is what policies must be handed. Comparing
-				// it against the recorded digest is the strongest available form
-				// of "policies see exactly what was stored": it holds even for
-				// skip-upload, where the stored bytes are kept nowhere else.
-				require.NotNil(t, content, "a redacted session must hand back its sanitized copy")
-				assert.Equal(t, sha256Digest(string(content)), got.GetArtifact().Digest)
-				assert.NotContains(t, string(content), awsKey)
-				assert.Contains(t, string(content), "[CHAINLOOP_TRACE_REDACTED:aws-access-token]")
-
-				if stored != nil {
-					assert.Equal(t, string(stored), string(content))
-					assert.NotContains(t, string(stored), awsKey)
-					assert.Contains(t, string(stored), "[CHAINLOOP_TRACE_REDACTED:aws-access-token]")
-				}
-			case tc.skipRedaction:
+			if tc.skipRedaction {
 				assert.Equal(t, "true", got.Annotations[api.AnnotationMaterialRedactionSkipped])
+				assert.NotContains(t, got.Annotations, api.AnnotationMaterialRedacted)
 				assert.Contains(t, string(stored), awsKey)
 				assert.Equal(t, sha256Digest(string(original)), got.GetArtifact().Digest)
 				// Nothing was transformed, so nothing is held in memory for the
 				// policy engine: it reads the file, which is what was stored.
 				assert.Nil(t, content)
-			default:
-				assert.NotContains(t, got.Annotations, api.AnnotationMaterialRedacted)
+			} else {
+				// Marked redacted whenever the scan ran, even when it found nothing.
+				assert.Equal(t, "true", got.Annotations[api.AnnotationMaterialRedacted])
 				assert.NotContains(t, got.Annotations, api.AnnotationMaterialRedactionSkipped)
-				// Nothing to redact, so the digest stays reproducible from the file.
-				assert.Equal(t, sha256Digest(string(original)), got.GetArtifact().Digest)
-				assert.Nil(t, content)
+
+				// The scanned copy is what policies must be handed, since a
+				// material marked redacted fails closed without it. Comparing it
+				// against the recorded digest is the strongest available form of
+				// "policies see exactly what was stored": it holds even for
+				// skip-upload, where the stored bytes are kept nowhere else.
+				require.NotNil(t, content, "a scanned session must hand back the scanned copy")
+				assert.Equal(t, sha256Digest(string(content)), got.GetArtifact().Digest)
+				if stored != nil {
+					assert.Equal(t, string(stored), string(content))
+				}
+
+				if tc.wantCount == "0" {
+					// Nothing was replaced, so the stored artifact is the file
+					// itself and the digest stays reproducible from it.
+					assert.Equal(t, string(original), string(content))
+				} else {
+					assert.NotContains(t, string(content), awsKey)
+					assert.Contains(t, string(content), "[CHAINLOOP_TRACE_REDACTED:aws-access-token]")
+				}
 			}
 
 			// Redaction must never touch the source file.