From be146c3acd087ec814d35ba53f1456a042257332 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Thu, 27 Aug 2026 00:54:28 +0200 Subject: [PATCH] fix(cli): evaluate policies against the material on disk in policy devel eval `policy devel eval` fed policies the redacted copy of a material instead of the file on disk. A policy hunting for leaked credentials in a CHAINLOOP_AI_CODING_SESSION saw sanitized input and reported no violations, silently and without an error, so the tool used to author policies disagreed with what production does. The crafter marks a redacted material with `chainloop.material.redacted`, which is what makes `GetEvaluableContent` read the untouched local file rather than the stored content. `devel eval` replaced the crafter's annotation map with the one built from `--annotation` flags, dropping that marker; with no flags the map was emptied outright. User annotations are now layered on top of the crafter's instead of replacing them. The `chainloop.` namespace is crafter-owned and not overridable, so an `--annotation` flag cannot put the old behaviour back, matching how `Crafter.stageMaterial` protects contract-provided annotations on `attestation add`. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 84e77448-d9b9-48d5-8599-9337da722d02 --- app/cli/internal/policydevel/eval.go | 37 +++++- app/cli/internal/policydevel/eval_test.go | 116 ++++++++++++++++++ .../ai-coding-session-no-secrets-policy.yaml | 19 +++ .../ai-coding-session-with-secret.json | 26 ++++ 4 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 app/cli/internal/policydevel/testdata/ai-coding-session-no-secrets-policy.yaml create mode 100644 app/cli/internal/policydevel/testdata/ai-coding-session-with-secret.json diff --git a/app/cli/internal/policydevel/eval.go b/app/cli/internal/policydevel/eval.go index 859c1393d..b5a8faee9 100644 --- a/app/cli/internal/policydevel/eval.go +++ b/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 diff --git a/app/cli/internal/policydevel/eval_test.go b/app/cli/internal/policydevel/eval_test.go index bbc758774..a98a97a8a 100644 --- a/app/cli/internal/policydevel/eval_test.go +++ b/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()) + }) + } +} diff --git a/app/cli/internal/policydevel/testdata/ai-coding-session-no-secrets-policy.yaml b/app/cli/internal/policydevel/testdata/ai-coding-session-no-secrets-policy.yaml new file mode 100644 index 000000000..8d38945f2 --- /dev/null +++ b/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" + } diff --git a/app/cli/internal/policydevel/testdata/ai-coding-session-with-secret.json b/app/cli/internal/policydevel/testdata/ai-coding-session-with-secret.json new file mode 100644 index 000000000..7bf435489 --- /dev/null +++ b/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__" + } + } + ] + } + } +}