fix(cli): evaluate policies against the material on disk in policy devel eval (#3379)

Author: migmartri

app/cli/internal/policydevel/eval.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strings"
 
 	controlplanev1 "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1"
 	v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1"
@@ -79,7 +80,7 @@ func Evaluate(opts *EvalOptions, logger zerolog.Logger) (*EvalSummary, error) {
 	if err != nil {
 		return nil, err
 	}
-	material.Annotations = opts.Annotations
+	mergeAnnotations(material, opts.Annotations, &logger)
 
 	// 3. Verify material against policy
 	summary, err := verifyMaterial(policies, material, opts.MaterialPath, opts.Debug, opts.AllowedHostnames, opts.AttestationClient, opts.ControlPlaneConn, opts.ProjectName, opts.ProjectVersionName, &logger)
@@ -90,6 +91,40 @@ func Evaluate(opts *EvalOptions, logger zerolog.Logger) (*EvalSummary, error) {
 	return summary, nil
 }
 
+// mergeAnnotations layers the user's --annotation flags on top of the ones the
+// crafter produced, rather than replacing them.
+//
+// The crafter's annotations carry more than metadata: chainloop.material.redacted
+// is what tells the policy engine to evaluate the untouched file on disk instead
+// of the sanitized copy staged for upload. Dropping it would silently feed
+// policies redacted input, which is exactly what a secret-hunting policy must
+// not see.
+//
+// The chainloop.* namespace is therefore crafter-owned and not overridable, so
+// that a --annotation flag cannot put back the behaviour this guards against.
+// Crafter.stageMaterial protects the equivalent invariant on `attestation add`
+// by refusing to override annotations that come from the contract.
+func mergeAnnotations(material *v12.Attestation_Material, annotations map[string]string, logger *zerolog.Logger) {
+	if len(annotations) == 0 {
+		return
+	}
+
+	// Crafters that do not go through uploadAndCraft (container image, string)
+	// leave the map nil.
+	if material.Annotations == nil {
+		material.Annotations = make(map[string]string, len(annotations))
+	}
+
+	for k, v := range annotations {
+		if strings.HasPrefix(k, v12.AnnotationPrefix) {
+			logger.Info().Str("annotation", k).Msg("reserved annotation namespace, it is set by the crafter and can not be overridden, skipping")
+			continue
+		}
+
+		material.Annotations[k] = v
+	}
+}
+
 func createPolicies(policyPath string, inputs map[string]string) (*v1.Policies, error) {
 	// Check if the policy path already has a scheme (chainloop://, http://, https://, file://)
 	ref := policyPath

app/cli/internal/policydevel/eval_test.go

@@ -15,11 +15,13 @@
 package policydevel
 
 import (
+	"bytes"
 	"encoding/json"
 	"os"
 	"path/filepath"
 	"testing"
 
+	v12 "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1"
 	"github.com/rs/zerolog"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -230,3 +232,117 @@ func TestEvaluateSimplifiedPolicies(t *testing.T) {
 		assert.Contains(t, string(result.Result.Violations[0]), "too few components")
 	})
 }
+
+// fixtureGitHubPAT is assembled from fragments so that this file does not itself
+// carry a credential-shaped literal for secret scanners to flag.
+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 {
+	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))
+
+	path := filepath.Join(t.TempDir(), "ai-coding-session.json")
+	require.NoError(t, os.WriteFile(path, content, 0600))
+
+	return path
+}
+
+// Policies must be evaluated against the material as it sits on disk. The crafter
+// redacts the copy it stages for upload, and a dry run always stages inline, so
+// evaluating the staged bytes would hide from a policy the very secret it exists
+// to catch.
+func TestEvaluateReadsUnredactedMaterialFromDisk(t *testing.T) {
+	testCases := []struct {
+		name        string
+		annotations map[string]string
+	}{
+		// The bug this pins: with no --annotation flags the crafter's annotation
+		// map was replaced by an empty one, so the marker that selects the file on
+		// disk was lost and policies evaluated the sanitized copy.
+		{name: "without user annotations", annotations: nil},
+		{name: "with user annotations", annotations: map[string]string{"custom": "value"}},
+		{
+			name:        "with an attempt to override the redaction marker",
+			annotations: map[string]string{v12.AnnotationMaterialRedacted: "false"},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			opts := &EvalOptions{
+				PolicyPath:   "testdata/ai-coding-session-no-secrets-policy.yaml",
+				MaterialKind: "CHAINLOOP_AI_CODING_SESSION",
+				MaterialPath: writeSessionFixture(t),
+				Annotations:  tc.annotations,
+			}
+
+			result, err := Evaluate(opts, zerolog.New(os.Stderr))
+			require.NoError(t, err)
+			require.NotNil(t, result)
+
+			assert.False(t, result.Result.Skipped)
+			require.Len(t, result.Result.Violations, 1)
+			assert.Contains(t, result.Result.Violations[0], "GitHub token found")
+		})
+	}
+}
+
+func TestMergeAnnotations(t *testing.T) {
+	testCases := []struct {
+		name     string
+		existing map[string]string
+		user     map[string]string
+		want     map[string]string
+	}{
+		{
+			name:     "crafter annotations survive when the user supplies none",
+			existing: map[string]string{v12.AnnotationMaterialRedacted: v12.AnnotationValueTrue},
+			user:     nil,
+			want:     map[string]string{v12.AnnotationMaterialRedacted: v12.AnnotationValueTrue},
+		},
+		{
+			name:     "user annotations are added alongside the crafter's",
+			existing: map[string]string{v12.AnnotationMaterialRedacted: v12.AnnotationValueTrue},
+			user:     map[string]string{"custom": "value"},
+			want:     map[string]string{v12.AnnotationMaterialRedacted: v12.AnnotationValueTrue, "custom": "value"},
+		},
+		{
+			name:     "user annotations win on conflict outside the reserved namespace",
+			existing: map[string]string{"custom": "crafted"},
+			user:     map[string]string{"custom": "user"},
+			want:     map[string]string{"custom": "user"},
+		},
+		{
+			name:     "the reserved chainloop namespace can not be overridden",
+			existing: map[string]string{v12.AnnotationMaterialRedacted: v12.AnnotationValueTrue},
+			user:     map[string]string{v12.AnnotationMaterialRedacted: "false", "custom": "value"},
+			want:     map[string]string{v12.AnnotationMaterialRedacted: v12.AnnotationValueTrue, "custom": "value"},
+		},
+		{
+			name:     "a material with no annotations gets the user's",
+			existing: nil,
+			user:     map[string]string{"custom": "value"},
+			want:     map[string]string{"custom": "value"},
+		},
+		{
+			name:     "nothing to merge leaves the material untouched",
+			existing: nil,
+			user:     nil,
+			want:     nil,
+		},
+	}
+
+	logger := zerolog.New(os.Stderr)
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			material := &v12.Attestation_Material{Annotations: tc.existing}
+			mergeAnnotations(material, tc.user, &logger)
+			assert.Equal(t, tc.want, material.GetAnnotations())
+		})
+	}
+}

app/cli/internal/policydevel/testdata/ai-coding-session-no-secrets-policy.yaml

@@ -0,0 +1,19 @@
+apiVersion: chainloop.dev/v1
+kind: Policy
+metadata:
+  name: ai-coding-session-no-secrets
+  description: Policy that fails when a GitHub token is present in the session
+spec:
+  policies:
+    - kind: CHAINLOOP_AI_CODING_SESSION
+      embedded: |
+        package main
+
+        import rego.v1
+
+        # The token is matched by shape rather than by value so that this file
+        # does not itself carry a credential-looking literal.
+        violations contains msg if {
+            regex.match(`ghp_[A-Za-z0-9]{36}`, json.marshal(input))
+            msg := "GitHub token found in the coding session"
+        }

app/cli/internal/policydevel/testdata/ai-coding-session-with-secret.json

@@ -0,0 +1,26 @@
+{
+  "chainloop.material.evidence.id": "CHAINLOOP_AI_CODING_SESSION",
+  "schema": "https://schemas.chainloop.dev/aicodingsession/0.1/ai-coding-session.schema.json",
+  "data": {
+    "schema_version": "v1",
+    "agent": {
+      "name": "cursor"
+    },
+    "session": {
+      "id": "abc-123",
+      "started_at": "2026-03-25T15:10:49.161Z",
+      "duration_seconds": 100
+    },
+    "raw_session": {
+      "main": [
+        {
+          "type": "user",
+          "message": {
+            "role": "user",
+            "content": "push the branch with GITHUB_TOKEN=__GITHUB_PAT__"
+          }
+        }
+      ]
+    }
+  }
+}