From 779c37d59b4a2095383cf3e37c33cdd611c78b07 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Fri, 28 Aug 2026 14:37:33 +0200 Subject: [PATCH 1/5] fix(policies): evaluate redacted materials against the content that was stored Secret redaction only rewrote the copy of an AI coding session that leaves the machine; policies were still evaluated against the untouched original on disk. Since a policy is user-authored Rego that can send what it reads to an allowed hostname, that made the policy engine a path for the very credentials redaction exists to remove. It also does not survive server-side evaluation, where the redacted copy is the only content available. Crafters that do not store the artifact verbatim now report the bytes they stored through materials.Craft, and the crafter passes them to both policy verifiers. Content resolution fails closed: a material annotated as redacted refuses to resolve at all without its sanitized copy, uniformly across CAS backends, rather than falling back to the file on disk. This also covers skip-upload materials, whose sanitized bytes were previously retained nowhere. Nothing is held in memory for materials that were not transformed: those keep resolving their content from the inline copy or the file, as before. Assisted-by: Claude Code Signed-off-by: Jose I. Paris --- app/cli/internal/policydevel/eval.go | 38 ++-- app/cli/internal/policydevel/eval_test.go | 33 +++- .../ai-coding-session-no-secrets-policy.yaml | 21 ++- .../api/attestation/v1/crafting_state.go | 63 +++++-- .../api/attestation/v1/crafting_state_test.go | 171 +++++++++++++----- pkg/attestation/crafter/crafter.go | 14 +- pkg/attestation/crafter/crafter_test.go | 82 +++++++++ .../materials/chainloop_ai_coding_session.go | 58 ++++-- ...inloop_ai_coding_session_redaction_test.go | 48 ++++- .../crafter/materials/materials.go | 44 ++++- .../crafter/materials/materials_test.go | 7 +- .../contracts/with_ai_session_policy.yaml | 4 + .../policies/ai_session_no_secrets.yaml | 25 +++ pkg/policies/policies.go | 32 +++- pkg/policies/policies_test.go | 106 +++++++++++ pkg/policies/policy_groups.go | 9 +- pkg/policies/policy_groups_test.go | 67 +++++++ 17 files changed, 686 insertions(+), 136 deletions(-) create mode 100644 pkg/attestation/crafter/testdata/contracts/with_ai_session_policy.yaml create mode 100644 pkg/attestation/crafter/testdata/policies/ai_session_no_secrets.yaml diff --git a/app/cli/internal/policydevel/eval.go b/app/cli/internal/policydevel/eval.go index b5a8faee9..689ff8ad9 100644 --- a/app/cli/internal/policydevel/eval.go +++ b/app/cli/internal/policydevel/eval.go @@ -76,14 +76,19 @@ func Evaluate(opts *EvalOptions, logger zerolog.Logger) (*EvalSummary, error) { } // 2. Craft material with annotations - material, err := craftMaterial(opts.MaterialPath, opts.MaterialKind, &logger) + crafted, err := craftMaterial(opts.MaterialPath, opts.MaterialKind, &logger) if err != nil { return nil, err } + material := crafted.Material 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) + // 3. Verify material against policy. A crafter that transformed the artifact + // before storing it hands back what it stored, and that is what the policy + // must be evaluated against — `policy devel eval` has to reproduce what + // `attestation add` does, or a policy would be developed against input the + // real run never sees. + summary, err := verifyMaterial(policies, material, opts.MaterialPath, crafted.EvaluableContent, opts.Debug, opts.AllowedHostnames, opts.AttestationClient, opts.ControlPlaneConn, opts.ProjectName, opts.ProjectVersionName, &logger) if err != nil { return nil, err } @@ -95,15 +100,15 @@ 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 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. +// is how a policy learns that secrets were found and 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. // // 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. +// that a --annotation flag cannot clear the marker. 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 @@ -145,7 +150,7 @@ func createPolicies(policyPath string, inputs map[string]string) (*v1.Policies, }, nil } -func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materialPath string, debug bool, allowedHostnames []string, attestationClient controlplanev1.AttestationServiceClient, grpcConn *grpc.ClientConn, projectName, projectVersion string, logger *zerolog.Logger) (*EvalSummary, error) { +func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materialPath string, evaluableContent []byte, debug bool, allowedHostnames []string, attestationClient controlplanev1.AttestationServiceClient, grpcConn *grpc.ClientConn, projectName, projectVersion string, logger *zerolog.Logger) (*EvalSummary, error) { var opts []policies.PolicyVerifierOption if len(allowedHostnames) > 0 { opts = append(opts, policies.WithAllowedHostnames(allowedHostnames...)) @@ -159,7 +164,8 @@ func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materi } v := policies.NewPolicyVerifier(pol, attestationClient, logger, opts...) - policyEvs, err := v.VerifyMaterial(context.Background(), material, materialPath) + policyEvs, err := v.VerifyMaterial(context.Background(), material, materialPath, + policies.WithMaterialContent(evaluableContent)) if err != nil { return nil, err } @@ -225,7 +231,7 @@ func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materi return summary, nil } -func craftMaterial(materialPath, materialKind string, logger *zerolog.Logger) (*v12.Attestation_Material, error) { +func craftMaterial(materialPath, materialKind string, logger *zerolog.Logger) (*materials.CraftResult, error) { backend := &casclient.CASBackend{ Name: "backend", MaxSize: 0, @@ -252,15 +258,15 @@ func craftMaterial(materialPath, materialKind string, logger *zerolog.Logger) (* return nil, fmt.Errorf("could not auto-detect material kind for: %s", materialPath) } -func craft(materialPath string, kind v1.CraftingSchema_Material_MaterialType, name string, backend *casclient.CASBackend, logger *zerolog.Logger) (*v12.Attestation_Material, error) { +func craft(materialPath string, kind v1.CraftingSchema_Material_MaterialType, name string, backend *casclient.CASBackend, logger *zerolog.Logger) (*materials.CraftResult, error) { materialSchema := &v1.CraftingSchema_Material{ Type: kind, Name: name, } - m, err := materials.Craft(context.Background(), materialSchema, materialPath, backend, nil, logger, nil) + res, err := materials.Craft(context.Background(), materialSchema, materialPath, backend, nil, logger, nil) if err != nil { return nil, fmt.Errorf("failed to craft material (kind=%s): %w", kind.String(), err) } - return m, nil + return res, nil } diff --git a/app/cli/internal/policydevel/eval_test.go b/app/cli/internal/policydevel/eval_test.go index a98a97a8a..7c8e97cec 100644 --- a/app/cli/internal/policydevel/eval_test.go +++ b/app/cli/internal/policydevel/eval_test.go @@ -252,21 +252,23 @@ func writeSessionFixture(t *testing.T) string { 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) { +// Policies must be evaluated against the copy the crafter stored, never the file +// on disk. A policy is arbitrary user-authored Rego that may ship what it reads to +// an allowed hostname, so handing it the credential that redaction just stripped +// out would turn the policy engine into an exfiltration path. +// +// `policy devel eval` has to agree with `attestation add` on this, or a policy +// would be developed against input the real run never produces. +func TestEvaluateReadsRedactedMaterial(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"}}, { + // The chainloop.* namespace is crafter-owned: clearing the marker + // from the command line must not be able to re-open the disk path. name: "with an attempt to override the redaction marker", annotations: map[string]string{v12.AnnotationMaterialRedacted: "false"}, }, @@ -279,6 +281,9 @@ func TestEvaluateReadsUnredactedMaterialFromDisk(t *testing.T) { MaterialKind: "CHAINLOOP_AI_CODING_SESSION", MaterialPath: writeSessionFixture(t), Annotations: tc.annotations, + // Debug surfaces the exact bytes handed to the engine, which is + // what the strongest assertion below inspects. + Debug: true, } result, err := Evaluate(opts, zerolog.New(os.Stderr)) @@ -287,7 +292,17 @@ func TestEvaluateReadsUnredactedMaterialFromDisk(t *testing.T) { assert.False(t, result.Result.Skipped) require.Len(t, result.Result.Violations, 1) - assert.Contains(t, result.Result.Violations[0], "GitHub token found") + assert.Contains(t, result.Result.Violations[0], "secrets were found and redacted") + + // The point of the whole change: the credential is nowhere in the + // policy input, and the placeholder that replaced it is. + require.NotNil(t, result.DebugInfo) + require.NotEmpty(t, result.DebugInfo.Inputs) + for _, input := range result.DebugInfo.Inputs { + assert.NotContains(t, string(input), fixtureGitHubPAT, + "the policy engine must never receive the un-redacted credential") + assert.Contains(t, string(input), "[REDACTED:") + } }) } } 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 index 8d38945f2..95b807518 100644 --- 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 @@ -2,7 +2,7 @@ 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 + description: Policy that fails when the session carried a credential spec: policies: - kind: CHAINLOOP_AI_CODING_SESSION @@ -11,9 +11,20 @@ spec: 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. + # Secrets are stripped out of the session before it is stored, and + # policies evaluate the stored copy, so a leaked credential is not here + # to be pattern-matched any more: it shows up as the placeholder that + # replaced it. Unanchored counterpart of + # internal/redaction.IsDefaultPlaceholder. violations contains msg if { - regex.match(`ghp_[A-Za-z0-9]{36}`, json.marshal(input)) - msg := "GitHub token found in the coding session" + regex.match(`\[REDACTED(:[^\]\s]*)?\]`, json.marshal(input)) + msg := "secrets were found and redacted in the coding session" + } + + # The opt-out is visible too, so a bypass cannot pass silently: nothing + # was redacted, which is exactly why nothing can be concluded from the + # absence of placeholders above. + violations contains msg if { + input.chainloop_metadata.annotations["chainloop.material.redaction.skipped"] == "true" + msg := "the coding session was stored without secret redaction" } diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state.go index cbcded7f4..73e01edc2 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state.go @@ -62,8 +62,9 @@ var ( // 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 read the untouched - // local file instead of the stored content (see GetEvaluableContent). + // 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 GetEvaluableContentFrom, which fails closed without it). AnnotationMaterialRedacted = CreateAnnotation("material.redacted") // AnnotationMaterialRedactionCount is how many secrets were replaced. AnnotationMaterialRedactionCount = CreateAnnotation("material.redaction.count") @@ -110,30 +111,62 @@ func (m *Attestation_Material) NormalizedOutput() (*NormalizedMaterialOutput, er return nil, fmt.Errorf("unknown material: %s", m.MaterialType) } -// GetEvaluableContent returns the content to be sent to policy evaluations +// ErrRedactedContentRequired is returned when policy evaluation is asked to +// resolve a material whose stored copy was sanitized, without being given that +// copy. There is no safe source left: the file on disk is the un-redacted +// original, and for a CAS-backed material the sanitized bytes were streamed away +// and are not on the material at all. +var ErrRedactedContentRequired = errors.New( + "sanitized content is required to evaluate a redacted material: " + + "policies must never be handed the un-redacted original") + +// GetEvaluableContent returns the content to be sent to policy evaluations, +// resolved from the material's stored copy or the file on disk. func (m *Attestation_Material) GetEvaluableContent(value string) ([]byte, error) { + return m.GetEvaluableContentFrom(value, nil) +} + +// GetEvaluableContentFrom is GetEvaluableContent with an explicit content source. +// +// content, when non-empty, is what policies are evaluated against, overriding +// both the inline bytes and the file on disk. Crafters that transform an artifact +// before it leaves the machine — redacting secrets out of an AI coding session — +// hand back the bytes they stored so that the policy engine sees exactly those, +// whatever CAS backend is in use. +// +// A material marked redacted with no content supplied fails closed. One +// consequence is worth knowing: such a material's policy input cannot be +// reconstructed from persisted crafting state alone, so any future push-time or +// server-side material evaluation has to plumb the bytes through as well. +func (m *Attestation_Material) GetEvaluableContentFrom(value string, content []byte) ([]byte, error) { var rawMaterial []byte var err error + // Fail closed before any source is chosen: a material whose stored copy was + // sanitized has exactly one valid policy input, and it is not the file on + // disk. Checked here rather than inside the artifact branch below so that it + // also covers material kinds that carry no artifact. + if len(content) == 0 && m.GetAnnotations()[AnnotationMaterialRedacted] == AnnotationValueTrue { + return nil, ErrRedactedContentRequired + } + artifact := m.GetArtifact() if artifact == nil && m.GetSbomArtifact() != nil { artifact = m.GetSbomArtifact().GetArtifact() } if artifact != nil { - // Policies must evaluate the artifact as it was produced. For a redacted - // material the inline bytes are the sanitized copy, so the untouched file - // on disk wins; without a local path there is nothing better to read. - // This keeps the policy input identical whatever CAS backend is in use, - // which matters most in dry runs, where the backend is always inline and - // is exactly where people test their policies. - useInlineContent := m.InlineCas - if m.GetAnnotations()[AnnotationMaterialRedacted] == AnnotationValueTrue && value != "" { - useInlineContent = false - } - switch { - case useInlineContent: + case len(content) > 0: + // NOTE: ingestMaterialToJSON re-reads the artifact from `value` for + // the kinds it projects from a path (JUNIT_XML, HELM_CHART), so + // supplied content would be silently ignored for those. Unreachable + // today, since only CHAINLOOP_AI_CODING_SESSION supplies content and + // it is JSON-native. A crafter that starts transforming what it + // stores for a path-projected kind must make the projection + // content-based first, or it will fail open. + rawMaterial = content + case m.InlineCas: rawMaterial = artifact.GetContent() case value == "": return nil, errors.New("artifact path required") diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state_test.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state_test.go index b95f9e9dd..7d5edcbac 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state_test.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state_test.go @@ -491,73 +491,111 @@ func TestTruffleHogCleanScanIsEvaluable(t *testing.T) { assert.Empty(t, elements, "a clean scan has zero findings") } -// TestGetEvaluableContentRedactedPrefersDisk covers the interaction between an -// inline CAS backend and a crafter that redacted the content before storing it. -// Inline materials normally carry their own content, but a redacted material's -// inline bytes are the sanitized copy, so policies have to read the untouched -// file from disk instead. Without that, the policy input would silently differ -// between an inline backend and every other one. -func TestGetEvaluableContentRedactedPrefersDisk(t *testing.T) { +// TestGetEvaluableContentRedactedNeverReadsDisk pins the invariant that policies +// are handed exactly the bytes that were stored. For a redacted material the +// original on disk is the one thing they must never see, whatever CAS backend is +// in use: it still holds the secrets that redaction deliberately kept out of the +// evidence store, and a policy is user-authored code that can ship what it reads. +// +// The sanitized copy therefore has to be supplied explicitly. Without it there is +// no safe source, so resolution fails closed rather than falling back to the file. +func TestGetEvaluableContentRedactedNeverReadsDisk(t *testing.T) { const ( - // Any distinguishable value works here; this test is about which source + // Any distinguishable values work here; this test is about which source // the content is read from, not about detection. onDiskSecret = "the-unredacted-original" inlineSecret = "[REDACTED:aws-access-token]" + suppliedTag = "[REDACTED:supplied]" - onDisk = `{"secret":"` + onDiskSecret + `"}` - inline = `{"secret":"` + inlineSecret + `"}` + onDisk = `{"secret":"` + onDiskSecret + `"}` + inline = `{"secret":"` + inlineSecret + `"}` + supplied = `{"secret":"` + suppliedTag + `"}` ) diskPath := filepath.Join(t.TempDir(), "session.json") require.NoError(t, os.WriteFile(diskPath, []byte(onDisk), 0o600)) testCases := []struct { - name string - inlineCas bool - redacted bool - path string - wantSecret string - wantErr bool + name string + inlineCas bool + annotations map[string]string + path string + content []byte + wantSecret string + wantErr error }{ { - name: "inline without the marker keeps reading the inline content", - inlineCas: true, - path: diskPath, - wantSecret: inlineSecret, + name: "inline and redacted without the sanitized copy fails closed", + inlineCas: true, + // The inline bytes ARE the sanitized copy here, so reading them would + // happen to be correct. It still fails: the caller has not said which + // content is authoritative, and guessing is what this test forbids. + annotations: map[string]string{AnnotationMaterialRedacted: AnnotationValueTrue}, + path: diskPath, + wantErr: ErrRedactedContentRequired, }, { - name: "inline and redacted reads the original from disk", - inlineCas: true, - redacted: true, - path: diskPath, - wantSecret: onDiskSecret, + name: "non-inline and redacted without the sanitized copy fails closed", + annotations: map[string]string{AnnotationMaterialRedacted: AnnotationValueTrue}, + path: diskPath, + wantErr: ErrRedactedContentRequired, }, { - name: "inline and redacted with no path falls back to the inline content", - inlineCas: true, - redacted: true, - wantSecret: inlineSecret, + name: "redacted with neither a path nor the sanitized copy fails closed", + inlineCas: true, + annotations: map[string]string{AnnotationMaterialRedacted: AnnotationValueTrue}, + wantErr: ErrRedactedContentRequired, }, { - name: "inline and redacted with an unreadable path fails loudly", - inlineCas: true, - redacted: true, - path: filepath.Join(t.TempDir(), "missing.json"), - // Silently falling back to the inline content here would point - // policies at redacted material without saying so. - wantErr: true, + name: "redacted with an empty but non-nil sanitized copy fails closed", + // Guards the len() check: a zero-length slice must not slip past into + // the empty-content fallback and yield an empty policy input. + annotations: map[string]string{AnnotationMaterialRedacted: AnnotationValueTrue}, + path: diskPath, + content: []byte{}, + wantErr: ErrRedactedContentRequired, + }, + { + name: "redacted with the sanitized copy evaluates it, not the disk file", + inlineCas: true, + annotations: map[string]string{AnnotationMaterialRedacted: AnnotationValueTrue}, + path: diskPath, + content: []byte(supplied), + wantSecret: suppliedTag, + }, + { + name: "redacted with the sanitized copy needs no path at all", + annotations: map[string]string{AnnotationMaterialRedacted: AnnotationValueTrue}, + content: []byte(supplied), + wantSecret: suppliedTag, }, { - name: "non-inline is unaffected by the marker", - redacted: true, + name: "redaction skipped on purpose still reads from disk", + // --skip-secret-redaction stores the session exactly as captured, so + // the disk file and the stored copy are the same bytes. The opt-out is + // recorded in the attestation and is deliberately not fail-closed. + annotations: map[string]string{AnnotationMaterialRedactionSkipped: AnnotationValueTrue}, + path: diskPath, + wantSecret: onDiskSecret, + }, + { + name: "unredacted inline reads the inline content as before", + inlineCas: true, path: diskPath, - wantSecret: onDiskSecret, + wantSecret: inlineSecret, }, { - name: "non-inline without the marker reads from disk as before", + name: "unredacted non-inline reads from disk as before", path: diskPath, wantSecret: onDiskSecret, }, + { + name: "supplied content wins over the inline copy even unredacted", + inlineCas: true, + path: diskPath, + content: []byte(supplied), + wantSecret: suppliedTag, + }, } for _, tc := range testCases { @@ -565,6 +603,7 @@ func TestGetEvaluableContentRedactedPrefersDisk(t *testing.T) { m := &Attestation_Material{ MaterialType: schemaapi.CraftingSchema_Material_CHAINLOOP_AI_CODING_SESSION, InlineCas: tc.inlineCas, + Annotations: tc.annotations, M: &Attestation_Material_Artifact_{ Artifact: &Attestation_Material_Artifact{ Name: "session.json", @@ -573,30 +612,66 @@ func TestGetEvaluableContentRedactedPrefersDisk(t *testing.T) { }, }, } - if tc.redacted { - m.Annotations = map[string]string{AnnotationMaterialRedacted: "true"} - } - content, err := m.GetEvaluableContent(tc.path) - if tc.wantErr { - require.Error(t, err) + content, err := m.GetEvaluableContentFrom(tc.path, tc.content) + if tc.wantErr != nil { + require.ErrorIs(t, err, tc.wantErr) return } require.NoError(t, err) // GetEvaluableContent also injects a chainloop_metadata block, so - // assert on the field that distinguishes the two sources. + // assert on the field that distinguishes the sources. var decoded map[string]any require.NoError(t, json.Unmarshal(content, &decoded)) assert.Equal(t, tc.wantSecret, decoded["secret"]) + + if tc.annotations[AnnotationMaterialRedacted] == AnnotationValueTrue { + assert.NotEqual(t, onDiskSecret, decoded["secret"], + "the un-redacted original must never reach the policy engine") + } }) } } +// TestGetEvaluableContentFromInjectsMetadata pins that supplying the content does +// not bypass the projection the policy engine relies on: the chainloop_metadata +// descriptor is still injected, so a policy can read the redaction annotations +// alongside the sanitized body. +func TestGetEvaluableContentFromInjectsMetadata(t *testing.T) { + m := &Attestation_Material{ + MaterialType: schemaapi.CraftingSchema_Material_CHAINLOOP_AI_CODING_SESSION, + Annotations: map[string]string{ + AnnotationMaterialRedacted: AnnotationValueTrue, + AnnotationMaterialRedactionCount: "2", + AnnotationMaterialRedactionRules: "jwt", + }, + M: &Attestation_Material_Artifact_{ + Artifact: &Attestation_Material_Artifact{Name: "session.json", Digest: "sha256:deadbeef"}, + }, + } + + content, err := m.GetEvaluableContentFrom("", []byte(`{"secret":"[REDACTED:jwt]"}`)) + require.NoError(t, err) + + var decoded struct { + Secret string `json:"secret"` + Metadata struct { + Annotations map[string]string `json:"annotations"` + } `json:"chainloop_metadata"` + } + require.NoError(t, json.Unmarshal(content, &decoded)) + + assert.Equal(t, "[REDACTED:jwt]", decoded.Secret) + assert.Equal(t, AnnotationValueTrue, decoded.Metadata.Annotations[AnnotationMaterialRedacted]) + assert.Equal(t, "2", decoded.Metadata.Annotations[AnnotationMaterialRedactionCount]) + assert.Equal(t, "jwt", decoded.Metadata.Annotations[AnnotationMaterialRedactionRules]) +} + // TestTruffleHogCleanScanIsEvaluableInline is the inline counterpart of // TestTruffleHogCleanScanIsEvaluable. It pins down that the canonical empty // content substituted for a zero-byte report projects the same either way, so -// preferring the disk path for redacted materials cannot regress it. +// that resolving a redacted material's content differently cannot regress it. func TestTruffleHogCleanScanIsEvaluableInline(t *testing.T) { m := &Attestation_Material{ MaterialType: schemaapi.CraftingSchema_Material_TRUFFLEHOG_JSON, diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index 5945ecd7a..7e17d1bee 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -761,13 +761,21 @@ func (c *Crafter) stageMaterial(ctx context.Context, m *schemaapi.CraftingSchema } // 3- Craft resulting material - mt, err := materials.Craft(context.Background(), m, value, casBackend, c.ociRegistryAuth, c.Logger, &materials.CraftingOpts{ + crafted, err := materials.Craft(context.Background(), m, value, casBackend, c.ociRegistryAuth, c.Logger, &materials.CraftingOpts{ NoStrictValidation: c.noStrictValidation, SkipSecretRedaction: c.skipSecretRedaction, }) if err != nil { return nil, err } + mt := crafted.Material + + // Crafters that transformed the artifact before storing it (redacting secrets + // out of an AI coding session) hand back what they stored, and that is what + // the policies below must see. Reading the file on disk instead would feed + // user-authored Rego the very secrets redaction removed. nil for every other + // material, which resolves its content the usual way. + withEvaluableContent := policies.WithMaterialContent(crafted.EvaluableContent) // 4 - Populate annotations from the ones provided at runtime // a) we do not allow overriding values that come from the contract @@ -820,7 +828,7 @@ func (c *Crafter) stageMaterial(ctx context.Context, m *schemaapi.CraftingSchema policies.WithDefaultGate(c.CraftingState.Attestation.GetBlockOnPolicyViolation()), policies.WithProjectContext(projectName, projectVersion), ) - policyGroupResults, err := pgv.VerifyMaterial(ctx, mt, value) + policyGroupResults, err := pgv.VerifyMaterial(ctx, mt, value, withEvaluableContent) if err != nil { return nil, fmt.Errorf("error applying policy groups to material: %w", err) } @@ -841,7 +849,7 @@ func (c *Crafter) stageMaterial(ctx context.Context, m *schemaapi.CraftingSchema policies.WithProjectContext(projectName, projectVersion), policies.WithRuntimeInputs(addOptions.runtimeInputs), ) - policyResults, err := pv.VerifyMaterial(ctx, mt, value) + policyResults, err := pv.VerifyMaterial(ctx, mt, value, withEvaluableContent) if err != nil { return nil, fmt.Errorf("error applying policies to material: %w", err) } diff --git a/pkg/attestation/crafter/crafter_test.go b/pkg/attestation/crafter/crafter_test.go index 3a8946f03..cede069ca 100644 --- a/pkg/attestation/crafter/crafter_test.go +++ b/pkg/attestation/crafter/crafter_test.go @@ -757,6 +757,88 @@ func (s *crafterSuite) TestAddMaterialsAutomaticInvalidNameSurfacesValidationErr assert.NotContains(s.T(), err.Error(), "failed to auto-discover material kind") } +// TestAddMaterialRedactedSessionIsWhatPoliciesSee is the end-to-end guarantee, +// and the case that was impossible before: with a real (non-inline) CAS backend +// the sanitized copy is streamed away and dropped, so the only content left on +// the machine is the un-redacted file. Policies used to be evaluated against +// exactly that. +// +// The attached policy looks for the raw credential rather than the placeholder, +// so it can only stay quiet if the engine genuinely never receives it. +func (s *crafterSuite) TestAddMaterialRedactedSessionIsWhatPoliciesSee() { + // Assembled from fragments so that nothing credential-shaped is committed, + // matching the fixture placeholders in the aicodingsession package. + const githubPAT = "ghp_erOZlZv0B1e3amrQ" + "ugdwZ8Ro2W4kDql9WPTf" + + sessionPath := materializeSessionFixture(s.T(), + "./materials/aicodingsession/testdata/session-with-secrets.json") + + uploader := mUploader.NewUploader(s.T()) + uploader.On("Upload", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(&casclient.UpDownStatus{Digest: "deadbeef", Filename: "session-with-secrets.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) + + // The material is not inline, so the sanitized bytes live nowhere the + // evaluation could have read them back from. + assert.False(s.T(), m.InlineCas) + assert.Equal(s.T(), v1.AnnotationValueTrue, m.Annotations[v1.AnnotationMaterialRedacted]) + + evaluations := c.CraftingState.Attestation.PolicyEvaluations + require.Len(s.T(), evaluations, 1) + + messages := make([]string, 0, len(evaluations[0].Violations)) + for _, v := range evaluations[0].Violations { + messages = append(messages, v.GetMessage()) + } + + assert.Contains(s.T(), messages, "secrets were found and redacted in the coding session") + assert.NotContains(s.T(), messages, "GitHub token reached the policy engine", + "the policy engine was handed the un-redacted session") + + // The source file is untouched, which is precisely why reading it would have + // leaked: the material no longer describes it. + onDisk, err := os.ReadFile(sessionPath) + require.NoError(s.T(), err) + assert.Contains(s.T(), string(onDisk), githubPAT) +} + +// 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. +func materializeSessionFixture(t *testing.T, src string) string { + t.Helper() + + content, err := os.ReadFile(src) + require.NoError(t, err) + + // Keep in sync with fixtureSecrets in the aicodingsession package. + secrets := map[string]string{ + "__AWS_ACCESS_KEY_ID__": "AKIA" + "4G7TI63VCBIRS4GW", + "__AWS_SECRET_ACCESS_KEY__": "kQ7zXn2VbW9pLm4RtY6" + "uHs3JdF8gA1cE5oPzQwXn", + "__GITHUB_PAT__": "ghp_erOZlZv0B1e3amrQ" + "ugdwZ8Ro2W4kDql9WPTf", + "__ANTHROPIC_API_KEY__": "sk-ant-api03-sT5wsx9DwmaHZDL0dUWKNhAhULxa35sUzyLFK9" + "5QBTZMDJTYn8p0J7ZQbwpYGYCQeW5eXAAGtVSmhp7UO9vxHJtSBC0xpAA", + "__GIT_REPOSITORY_WITH_CREDENTIALS__": "https://oauth2:" + + "ghp_erOZlZv0B1e3amrQ" + "ugdwZ8Ro2W4kDql9WPTf" + "@github.com/example/repo.git", + } + resolved := string(content) + for placeholder, secret := range secrets { + resolved = strings.ReplaceAll(resolved, placeholder, secret) + } + + path := filepath.Join(t.TempDir(), filepath.Base(src)) + require.NoError(t, os.WriteFile(path, []byte(resolved), 0o600)) + return path +} + func (s *crafterSuite) TestAddMaterialsFromArchiveAtomic() { // Build the fixture in-process so no binary blob is checked in. zipFixture := filepath.Join(s.T().TempDir(), "two-files.zip") diff --git a/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go b/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go index 2305b6d21..8f4bb2259 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go @@ -69,14 +69,28 @@ func NewChainloopAICodingSessionCrafter(schema *schemaapi.CraftingSchema_Materia // secrets found in it, calculates the digest, uploads it and returns the // material definition. // -// Only the stored copy is redacted. The file on disk is left untouched, and it -// is what policies are evaluated against once this material has been staged, so -// a policy that looks for leaked credentials still sees what the agent actually -// captured. +// The file on disk is left untouched; only the stored copy is redacted, which +// means the sanitized bytes are dropped here. Callers that go on to evaluate +// policies must craft through materials.Craft and pass on the EvaluableContent it +// returns, or evaluation of a redacted session will fail rather than silently read +// the original from disk. func (c *ChainloopAICodingSessionCrafter) Craft(ctx context.Context, artifactPath string) (*api.Attestation_Material, error) { + material, _, err := c.transformCraft(ctx, artifactPath) + return material, err +} + +// transformCraft is Craft, additionally returning the bytes it stored in place of +// the artifact: the sanitized copy when redaction replaced something, and nil +// otherwise, meaning the file on disk is what was stored and can be read from +// there as usual. +// +// Redaction only rewrites the copy that leaves the machine, so a caller that +// reads the artifact back off disk gets the credentials the session captured. +// That is what these bytes exist to prevent. +func (c *ChainloopAICodingSessionCrafter) transformCraft(ctx context.Context, artifactPath string) (*api.Attestation_Material, []byte, error) { f, err := os.ReadFile(artifactPath) if err != nil { - return nil, fmt.Errorf("can't open the file: %w", err) + return nil, nil, fmt.Errorf("can't open the file: %w", err) } // Unmarshal envelope, keeping data as raw JSON for schema validation @@ -85,35 +99,42 @@ func (c *ChainloopAICodingSessionCrafter) Craft(ctx context.Context, artifactPat } if err := json.Unmarshal(f, &envelope); err != nil { c.logger.Debug().Err(err).Msg("error decoding file") - return nil, fmt.Errorf("invalid JSON format: %w", err) + return nil, nil, fmt.Errorf("invalid JSON format: %w", err) } // Unmarshal data into typed struct for annotation extraction var data aicodingsession.Data if err := json.Unmarshal(envelope.Data, &data); err != nil { c.logger.Debug().Err(err).Msg("error decoding data field") - return nil, fmt.Errorf("failed to unmarshal data: %w", err) + return nil, nil, fmt.Errorf("failed to unmarshal data: %w", err) } // Validate using raw JSON to preserve unknown fields for strict schema validation var rawData any if err := json.Unmarshal(envelope.Data, &rawData); err != nil { - return nil, fmt.Errorf("failed to unmarshal data for validation: %w", err) + return nil, nil, fmt.Errorf("failed to unmarshal data for validation: %w", err) } if err := schemavalidators.ValidateAICodingSession(rawData, schemavalidators.AICodingSessionVersion0_1); err != nil { c.logger.Debug().Err(err).Msg("schema validation failed") - return nil, fmt.Errorf("AI coding session validation failed: %w", err) + return nil, nil, fmt.Errorf("AI coding session validation failed: %w", err) } - craftOpts, report, err := c.redact(ctx, f) + redacted, report, err := c.redact(ctx, f) if err != nil { - return nil, err + return nil, 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)) } material, err := uploadAndCraft(ctx, c.input, c.backend, artifactPath, c.logger, craftOpts...) if err != nil { - return nil, err + return nil, nil, err } c.annotateRedaction(material, report) @@ -128,17 +149,18 @@ func (c *ChainloopAICodingSessionCrafter) Craft(ctx context.Context, artifactPat material.Annotations[annotationAICodingModel] = data.Model.Primary } - return material, nil + return material, redacted, nil } -// redact strips secrets out of the session content, returning the options that -// make uploadAndCraft store the sanitized copy instead of the file on disk. +// 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. // // 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 // uploaded unscanned. The operator's escape hatch is --skip-secret-redaction, // whose use is recorded in the attestation. -func (c *ChainloopAICodingSessionCrafter) redact(ctx context.Context, content []byte) ([]uploadAndCraftOption, *redaction.Report, error) { +func (c *ChainloopAICodingSessionCrafter) redact(ctx context.Context, content []byte) ([]byte, *redaction.Report, error) { if c.skipRedaction { c.logger.Warn().Msg("secret redaction is DISABLED: the AI coding session will be stored exactly as captured") return nil, nil, nil @@ -150,12 +172,10 @@ func (c *ChainloopAICodingSessionCrafter) redact(ctx context.Context, content [] } if !report.Changed() { - // Nothing was replaced, so upload the file as it is on disk and keep the - // digest reproducible from the source file. return nil, report, nil } - return []uploadAndCraftOption{withContentOverride(redacted)}, report, nil + return redacted, report, nil } // annotateRedaction records what redaction did, so that it is visible in the diff --git a/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go b/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go index ca6697b68..5aa1e5037 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go @@ -180,8 +180,9 @@ func materializeFixture(t *testing.T, src string) string { } // TestChainloopAICodingSessionCrafterRedaction is the end-to-end guarantee: the -// bytes that leave the machine carry no secrets, while the file on disk — what -// policies are evaluated against afterwards — is left untouched. +// bytes that leave the machine carry no secrets, the file on disk is left +// untouched, and the content handed to policy evaluation is byte-for-byte the +// content that was stored — never the original. func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { const ( withSecrets = "./aicodingsession/testdata/session-with-secrets.json" @@ -193,6 +194,7 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { filePath string skipRedaction bool inlineBackend bool + skipUpload bool wantRedacted bool wantCount string wantRules string @@ -214,6 +216,16 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { wantCount: "7", wantRules: "anthropic-api-key,aws-access-token,aws-secret-access-key,github-pat", }, + { + // 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: "the opt-out is recorded in the attestation", filePath: withSecrets, @@ -229,8 +241,9 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { t.Run(tc.name, func(t *testing.T) { logger := zerolog.Nop() schema := &schemaapi.CraftingSchema_Material{ - Name: "test", - Type: schemaapi.CraftingSchema_Material_CHAINLOOP_AI_CODING_SESSION, + Name: "test", + Type: schemaapi.CraftingSchema_Material_CHAINLOOP_AI_CODING_SESSION, + SkipUpload: tc.skipUpload, } path := materializeFixture(t, tc.filePath) @@ -240,7 +253,7 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { // A nil Uploader is what the CLI builds for an inline CAS backend. backend := &casclient.CASBackend{Name: "not-set"} var stored []byte - if !tc.inlineBackend { + if !tc.inlineBackend && !tc.skipUpload { uploader := mUploader.NewUploader(t) uploader.On("Upload", mock.Anything, mock.Anything, mock.Anything, mock.Anything). Run(func(args mock.Arguments) { @@ -256,7 +269,7 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { WithAICodingSessionSkipRedaction(tc.skipRedaction)) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), path) + got, evaluable, err := crafter.transformCraft(context.TODO(), path) require.NoError(t, err) if tc.inlineBackend { @@ -270,22 +283,39 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { switch { case tc.wantRedacted: assert.Equal(t, "true", got.Annotations[api.AnnotationMaterialRedacted]) - assert.NotContains(t, string(stored), awsKey) - assert.Contains(t, string(stored), "[REDACTED:aws-access-token]") // 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, evaluable, "a redacted session must hand back its sanitized copy") + assert.Equal(t, sha256Digest(string(evaluable)), got.GetArtifact().Digest) + assert.NotContains(t, string(evaluable), awsKey) + assert.Contains(t, string(evaluable), "[REDACTED:aws-access-token]") + + if stored != nil { + assert.Equal(t, string(stored), string(evaluable)) + assert.NotContains(t, string(stored), awsKey) + assert.Contains(t, string(stored), "[REDACTED:aws-access-token]") + } case tc.skipRedaction: assert.Equal(t, "true", got.Annotations[api.AnnotationMaterialRedactionSkipped]) 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, evaluable) default: assert.NotContains(t, 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, evaluable) } - // Redaction must never touch the file the policies will read. + // Redaction must never touch the source file. stillOnDisk, err := os.ReadFile(path) require.NoError(t, err) assert.Equal(t, string(original), string(stillOnDisk)) diff --git a/pkg/attestation/crafter/materials/materials.go b/pkg/attestation/crafter/materials/materials.go index 73574bd3b..6433f8745 100644 --- a/pkg/attestation/crafter/materials/materials.go +++ b/pkg/attestation/crafter/materials/materials.go @@ -172,8 +172,10 @@ type uploadAndCraftOpts struct { // leaves the machine, such as redacting secrets out of an AI coding session. // The recorded digest, size and uploaded (or inline) body then all describe // the transformed content, which is the only content that ever reaches the - // CAS. The file on disk is left untouched, and remains what policies are - // evaluated against. The original filename is preserved. + // CAS. The file on disk is left untouched, but it is no longer what the + // material describes: such a crafter must also report the transformed bytes + // (see transformCrafter), since reading the artifact back off disk would no + // longer yield the stored content. The original filename is preserved. contentOverride []byte } @@ -350,6 +352,32 @@ type Craftable interface { Craft(ctx context.Context, value string) (*api.Attestation_Material, error) } +// transformCrafter is implemented by crafters that do not store the artifact +// verbatim, and returns the bytes they stored in its place. For such a material +// the file on disk is no longer the stored content, so anything that needs to +// read the artifact back has to be given these bytes instead. What it is then +// used for is the caller's business: today it feeds policy evaluation, which must +// not see the data the transformation removed. +// +// Opt-in on purpose, and deliberately not on crafterCommon: a content field +// shared by every crafter would let any of them report content that diverges +// from its own recorded digest. +type transformCrafter interface { + transformCraft(ctx context.Context, value string) (*api.Attestation_Material, []byte, error) +} + +// CraftResult is a crafted material together with the content policies must be +// evaluated against. +type CraftResult struct { + Material *api.Attestation_Material + // EvaluableContent is the sanitized copy of the artifact, set only by + // crafters that transformed it before storing it — today, an AI coding + // session with secrets redacted out of it. nil means the stored bytes are the + // file on disk, so policy evaluation resolves the content the usual way and + // nothing extra is held in memory. + EvaluableContent []byte +} + // CraftingOpts contains options for crafting materials type CraftingOpts struct { NoStrictValidation bool @@ -359,7 +387,7 @@ type CraftingOpts struct { } //nolint:gocyclo -func Craft(ctx context.Context, materialSchema *schemaapi.CraftingSchema_Material, value string, casBackend *casclient.CASBackend, ociAuth authn.Keychain, logger *zerolog.Logger, opts *CraftingOpts) (*api.Attestation_Material, error) { +func Craft(ctx context.Context, materialSchema *schemaapi.CraftingSchema_Material, value string, casBackend *casclient.CASBackend, ociAuth authn.Keychain, logger *zerolog.Logger, opts *CraftingOpts) (*CraftResult, error) { var crafter Craftable var err error @@ -470,7 +498,13 @@ func Craft(ctx context.Context, materialSchema *schemaapi.CraftingSchema_Materia return nil, err } - m, err := crafter.Craft(ctx, value) + var m *api.Attestation_Material + var transformed []byte + if tc, ok := crafter.(transformCrafter); ok { + m, transformed, err = tc.transformCraft(ctx, value) + } else { + m, err = crafter.Craft(ctx, value) + } if err != nil { return nil, fmt.Errorf("crafting material: %w", err) } @@ -489,5 +523,5 @@ func Craft(ctx context.Context, materialSchema *schemaapi.CraftingSchema_Materia m.Output = materialSchema.Output m.Required = !materialSchema.Optional - return m, nil + return &CraftResult{Material: m, EvaluableContent: transformed}, nil } diff --git a/pkg/attestation/crafter/materials/materials_test.go b/pkg/attestation/crafter/materials/materials_test.go index ef9a88be6..f5a9a586b 100644 --- a/pkg/attestation/crafter/materials/materials_test.go +++ b/pkg/attestation/crafter/materials/materials_test.go @@ -62,8 +62,13 @@ func TestCraft(t *testing.T) { }, } - got, err := materials.Craft(context.TODO(), schema, "test-value", nil, nil, nil, nil) + res, err := materials.Craft(context.TODO(), schema, "test-value", nil, nil, nil, nil) require.NoError(t, err) + // A crafter that does not transform the artifact holds nothing back for the + // policy engine; the content is resolved from the material or the file. + assert.Nil(res.EvaluableContent) + + got := res.Material assert.Equal(contractAPI.CraftingSchema_Material_STRING, got.MaterialType) assert.False(got.UploadedToCas) assert.Equal(got.GetString_(), &attestationApi.Attestation_Material_KeyVal{ diff --git a/pkg/attestation/crafter/testdata/contracts/with_ai_session_policy.yaml b/pkg/attestation/crafter/testdata/contracts/with_ai_session_policy.yaml new file mode 100644 index 000000000..abb1212e6 --- /dev/null +++ b/pkg/attestation/crafter/testdata/contracts/with_ai_session_policy.yaml @@ -0,0 +1,4 @@ +schemaVersion: "v1" +policies: + materials: + - ref: file://testdata/policies/ai_session_no_secrets.yaml diff --git a/pkg/attestation/crafter/testdata/policies/ai_session_no_secrets.yaml b/pkg/attestation/crafter/testdata/policies/ai_session_no_secrets.yaml new file mode 100644 index 000000000..349fefc42 --- /dev/null +++ b/pkg/attestation/crafter/testdata/policies/ai_session_no_secrets.yaml @@ -0,0 +1,25 @@ +apiVersion: chainloop.dev/v1 +kind: Policy +metadata: + name: ai-session-no-secrets + description: Reports the credentials it can see in an AI coding session +spec: + policies: + - kind: CHAINLOOP_AI_CODING_SESSION + embedded: | + package main + + import rego.v1 + + # Deliberately looks for the raw credential, not the placeholder: this + # policy exists to prove what the engine is handed. If redaction has done + # its job upstream, this rule can never fire. + violations contains msg if { + regex.match(`ghp_[A-Za-z0-9]{36}`, json.marshal(input)) + msg := "GitHub token reached the policy engine" + } + + violations contains msg if { + regex.match(`\[REDACTED(:[^\]\s]*)?\]`, json.marshal(input)) + msg := "secrets were found and redacted in the coding session" + } diff --git a/pkg/policies/policies.go b/pkg/policies/policies.go index 781f4309b..d786dbce7 100644 --- a/pkg/policies/policies.go +++ b/pkg/policies/policies.go @@ -67,10 +67,35 @@ func (e *PolicyError) Unwrap() error { } type Verifier interface { - VerifyMaterial(ctx context.Context, m *v12.Attestation_Material, path string) ([]*v12.PolicyEvaluation, error) + VerifyMaterial(ctx context.Context, m *v12.Attestation_Material, path string, opts ...VerifyMaterialOption) ([]*v12.PolicyEvaluation, error) VerifyStatement(ctx context.Context, statement *intoto.Statement) ([]*v12.PolicyEvaluation, error) } +// verifyMaterialOpts tunes a single material verification. +type verifyMaterialOpts struct { + content []byte +} + +// VerifyMaterialOption tunes a single call to VerifyMaterial. +type VerifyMaterialOption func(*verifyMaterialOpts) + +// WithMaterialContent supplies the bytes to evaluate instead of resolving them +// from the material's stored copy or the file on disk. Crafters that transform an +// artifact before storing it must pass it, so that the policy engine sees the +// content that was stored rather than the untouched original: a redacted material +// fails to resolve at all without it. nil is a no-op. +func WithMaterialContent(content []byte) VerifyMaterialOption { + return func(o *verifyMaterialOpts) { o.content = content } +} + +func newVerifyMaterialOpts(opts ...VerifyMaterialOption) *verifyMaterialOpts { + o := &verifyMaterialOpts{} + for _, opt := range opts { + opt(o) + } + return o +} + // EvalPhase represents the phase of the attestation lifecycle where evaluation is happening. type EvalPhase int @@ -262,8 +287,9 @@ func NewPolicyVerifier(policies *v1.Policies, client v13.AttestationServiceClien } // VerifyMaterial applies all required policies to a material -func (pv *PolicyVerifier) VerifyMaterial(ctx context.Context, material *v12.Attestation_Material, artifactPath string) ([]*v12.PolicyEvaluation, error) { +func (pv *PolicyVerifier) VerifyMaterial(ctx context.Context, material *v12.Attestation_Material, artifactPath string, opts ...VerifyMaterialOption) ([]*v12.PolicyEvaluation, error) { result := make([]*v12.PolicyEvaluation, 0) + o := newVerifyMaterialOpts(opts...) attachments, err := pv.requiredPoliciesForMaterial(ctx, material) if err != nil { @@ -275,7 +301,7 @@ func (pv *PolicyVerifier) VerifyMaterial(ctx context.Context, material *v12.Atte } // Load material content - subject, err := material.GetEvaluableContent(artifactPath) + subject, err := material.GetEvaluableContentFrom(artifactPath, o.content) if err != nil { return nil, NewPolicyError(err) } diff --git a/pkg/policies/policies_test.go b/pkg/policies/policies_test.go index cb2399a3f..ddba5014b 100644 --- a/pkg/policies/policies_test.go +++ b/pkg/policies/policies_test.go @@ -20,6 +20,7 @@ import ( "fmt" "io/fs" "os" + "path/filepath" "testing" "time" @@ -701,6 +702,111 @@ func (s *testSuite) TestInvalidInlineMaterial() { s.Equal("Not made with syft", res[0].Violations[0].Message) } +// TestVerifyMaterialSuppliedContent covers WithMaterialContent, the channel a +// crafter uses to say which bytes it stored. Supplied content must win over every +// other source, and a material whose stored copy was sanitized must refuse to be +// evaluated without it rather than quietly reading the original from disk. +func (s *testSuite) TestVerifyMaterialSuppliedContent() { + onDisk, err := os.ReadFile("testdata/sbom-spdx.json") + s.Require().NoError(err) + + diskPath := filepath.Join(s.T().TempDir(), "sbom.json") + s.Require().NoError(os.WriteFile(diskPath, onDisk, 0o600)) + + notAnSBOM := []byte(`{"this": { "is": "not", "a": "sbom"}}`) + + pol := &v12.Policies{ + Materials: []*v12.PolicyAttachment{ + {Policy: &v12.PolicyAttachment_Ref{Ref: "file://testdata/sbom_syft.yaml"}}, + }, + } + + testCases := []struct { + name string + redacted bool + inlineCas bool + inlineContent []byte + path string + content []byte + wantErr error + wantViolations int + }{ + { + // The disk file is a valid syft SBOM, the supplied bytes are not, so + // the violation count says which one the engine actually saw. + name: "supplied content is evaluated instead of the file on disk", + path: diskPath, + content: notAnSBOM, + wantViolations: 1, + }, + { + name: "supplied content wins over the inline copy", + inlineCas: true, + inlineContent: onDisk, + content: notAnSBOM, + wantViolations: 1, + }, + { + name: "without supplied content the file on disk is still read", + path: diskPath, + wantViolations: 0, + }, + { + name: "a redacted material refuses to be evaluated from disk", + redacted: true, + path: diskPath, + wantErr: v1.ErrRedactedContentRequired, + }, + { + name: "a redacted inline material refuses just the same", + redacted: true, + inlineCas: true, + inlineContent: onDisk, + path: diskPath, + wantErr: v1.ErrRedactedContentRequired, + }, + { + name: "a redacted material with its sanitized copy evaluates it", + redacted: true, + path: diskPath, + content: notAnSBOM, + wantViolations: 1, + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + material := &v1.Attestation_Material{ + M: &v1.Attestation_Material_Artifact_{Artifact: &v1.Attestation_Material_Artifact{ + Content: tc.inlineContent, + }}, + MaterialType: v12.CraftingSchema_Material_SBOM_SPDX_JSON, + InlineCas: tc.inlineCas, + } + if tc.redacted { + material.Annotations = map[string]string{ + v1.AnnotationMaterialRedacted: v1.AnnotationValueTrue, + } + } + + verifier := NewPolicyVerifier(pol, nil, &s.logger) + + res, err := verifier.VerifyMaterial(context.TODO(), material, tc.path, + WithMaterialContent(tc.content)) + if tc.wantErr != nil { + s.Require().ErrorIs(err, tc.wantErr) + // Wrapped so callers keep treating it as a policy failure. + var policyErr *PolicyError + s.Require().ErrorAs(err, &policyErr) + return + } + s.Require().NoError(err) + s.Require().Len(res, 1) + s.Len(res[0].Violations, tc.wantViolations) + }) + } +} + // TestVerifyMaterialScopedRuntimeInputs reproduces the trusted-binaries scenario // from PFM-6530: two policies both declare `ignored_paths`, and a runtime input // scoped to one of them must land only on that policy, not the other. diff --git a/pkg/policies/policy_groups.go b/pkg/policies/policy_groups.go index e42a6e893..159c3772e 100644 --- a/pkg/policies/policy_groups.go +++ b/pkg/policies/policy_groups.go @@ -49,8 +49,9 @@ func NewPolicyGroupVerifier(policyGroups []*v1.PolicyGroupAttachment, policies * } // VerifyMaterial evaluates a material against groups of policies defined in the schema -func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *api.Attestation_Material, path string) ([]*api.PolicyEvaluation, error) { +func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *api.Attestation_Material, path string, opts ...VerifyMaterialOption) ([]*api.PolicyEvaluation, error) { result := make([]*api.PolicyEvaluation, 0) + o := newVerifyMaterialOpts(opts...) groupAtts := pgv.policyGroups @@ -82,8 +83,10 @@ func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *ap continue } - // Load material content once for all policies in this group - subject, err := material.GetEvaluableContent(path) + // Load material content once for all policies in this group. Kept below + // the skip above so that a material with no applicable policies never + // resolves its content at all. + subject, err := material.GetEvaluableContentFrom(path, o.content) if err != nil { return nil, NewPolicyError(err) } diff --git a/pkg/policies/policy_groups_test.go b/pkg/policies/policy_groups_test.go index 89831b269..040a80a9a 100644 --- a/pkg/policies/policy_groups_test.go +++ b/pkg/policies/policy_groups_test.go @@ -321,6 +321,73 @@ func (s *groupsTestSuite) TestVerifyStatement() { } } +// TestVerifyMaterialSuppliedContent mirrors the PolicyVerifier test for groups, +// which resolve content per group rather than once up front. The case worth +// pinning is the last one: resolution stays behind the "no policies apply" skip, +// so a redacted material that no policy in the group selects must pass through +// untouched instead of failing closed on content it never needed. +func (s *groupsTestSuite) TestVerifyMaterialSuppliedContent() { + const ( + matching = `{"specVersion": "1.4"}` + notMatching = `{"specVersion": "1.0"}` + ) + + testCases := []struct { + name string + materialType v1.CraftingSchema_Material_MaterialType + content []byte + wantErr error + wantEvals int + }{ + { + name: "supplied content is evaluated", + materialType: v1.CraftingSchema_Material_OPENVEX, + content: []byte(matching), + wantEvals: 1, + }, + { + name: "a redacted material without its sanitized copy fails closed", + materialType: v1.CraftingSchema_Material_OPENVEX, + wantErr: api.ErrRedactedContentRequired, + }, + { + name: "a redacted material no policy selects is never resolved at all", + // The group only covers SBOM_CYCLONEDX_JSON and OPENVEX, so nothing + // in it selects a coding session: the content is never loaded and + // there is nothing to fail closed over. + materialType: v1.CraftingSchema_Material_CHAINLOOP_AI_CODING_SESSION, + wantEvals: 0, + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + material := &api.Attestation_Material{ + M: &api.Attestation_Material_Artifact_{Artifact: &api.Attestation_Material_Artifact{ + Content: []byte(notMatching), + }}, + MaterialType: tc.materialType, + InlineCas: true, + Annotations: map[string]string{ + api.AnnotationMaterialRedacted: api.AnnotationValueTrue, + }, + } + + groups := []*v1.PolicyGroupAttachment{{Ref: "file://testdata/policy_group_multikind.yaml"}} + verifier := NewPolicyGroupVerifier(groups, nil, nil, &s.logger) + + res, err := verifier.VerifyMaterial(context.TODO(), material, "", + WithMaterialContent(tc.content)) + if tc.wantErr != nil { + s.Require().ErrorIs(err, tc.wantErr) + return + } + s.Require().NoError(err) + s.Len(res, tc.wantEvals) + }) + } +} + func (s *groupsTestSuite) TestVerifyMaterialMultiKind() { cases := []struct { name string From 74479847bf899b9e5351d511c10248762eb0922c Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Fri, 28 Aug 2026 19:21:10 +0200 Subject: [PATCH 2/5] refactor(materials): return CraftResult from Craftable.Craft Reporting the bytes a crafter stored in place of the artifact was bolted on as a second, optional interface method alongside Craft. Fold it into Craft itself: Craftable returns a CraftResult carrying the material and, when the artifact was not stored verbatim, the bytes that replaced it. One method instead of two, so a crafter cannot report content through a path the generic craft flow does not take, and the capability is visible in the interface rather than discovered by type assertion. Every crafter but the AI coding session stores the artifact as it found it and leaves Transformed nil. Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: 06ec9990-4ad5-4b3c-ad86-2421b24dba57 --- app/cli/internal/policydevel/eval.go | 6 +- pkg/attestation/crafter/crafter.go | 6 +- .../crafter/materials/accesschk.go | 4 +- .../crafter/materials/accesschk_test.go | 2 +- pkg/attestation/crafter/materials/artifact.go | 5 +- .../crafter/materials/artifact_test.go | 6 +- pkg/attestation/crafter/materials/asyncapi.go | 4 +- .../crafter/materials/asyncapi_test.go | 4 +- .../crafter/materials/attestation.go | 5 +- .../crafter/materials/attestation_test.go | 4 +- .../crafter/materials/blackduck.go | 5 +- .../crafter/materials/blackduck_test.go | 2 +- .../materials/chainloop_ai_agent_config.go | 4 +- .../chainloop_ai_agent_config_test.go | 2 +- .../materials/chainloop_ai_coding_session.go | 40 +++++-------- ...inloop_ai_coding_session_redaction_test.go | 17 +++--- .../chainloop_ai_coding_session_test.go | 2 +- .../chainloop_ai_security_context.go | 4 +- .../chainloop_ai_security_context_test.go | 10 ++-- .../crafter/materials/chainloop_pr_info.go | 5 +- .../crafter/materials/checkmarx.go | 4 +- .../crafter/materials/checkmarx_test.go | 2 +- .../crafter/materials/cobertura.go | 5 +- .../crafter/materials/cobertura_test.go | 4 +- .../materials/craft_result_external_test.go | 36 ++++++++++++ .../crafter/materials/craft_result_test.go | 35 ++++++++++++ pkg/attestation/crafter/materials/csaf.go | 4 +- .../crafter/materials/csaf_test.go | 2 +- .../crafter/materials/cyclonedxjson.go | 4 +- .../crafter/materials/cyclonedxjson_test.go | 6 +- .../crafter/materials/detect_secrets.go | 4 +- .../crafter/materials/detect_secrets_test.go | 2 +- pkg/attestation/crafter/materials/dranzer.go | 4 +- .../crafter/materials/dranzer_test.go | 4 +- pkg/attestation/crafter/materials/evidence.go | 4 +- .../crafter/materials/evidence_test.go | 8 +-- .../crafter/materials/ghas_code_scan.go | 5 +- .../crafter/materials/ghas_dependency_scan.go | 5 +- .../crafter/materials/ghas_secret_scan.go | 5 +- .../crafter/materials/ghas_test.go | 6 +- pkg/attestation/crafter/materials/gitlab.go | 4 +- .../crafter/materials/gitlab_test.go | 2 +- pkg/attestation/crafter/materials/gitleaks.go | 4 +- .../crafter/materials/gitleaks_test.go | 2 +- pkg/attestation/crafter/materials/graphql.go | 4 +- .../crafter/materials/graphql_test.go | 2 +- .../crafter/materials/helmchart.go | 4 +- .../crafter/materials/helmchart_test.go | 2 +- pkg/attestation/crafter/materials/jacoco.go | 5 +- .../crafter/materials/jacoco_test.go | 2 +- .../crafter/materials/junit_xml.go | 5 +- .../crafter/materials/junit_xml_test.go | 2 +- .../crafter/materials/materials.go | 56 +++++++++---------- .../crafter/materials/materials_test.go | 6 +- .../crafter/materials/oci_image.go | 8 +-- .../crafter/materials/oci_image_test.go | 8 +-- pkg/attestation/crafter/materials/openapi.go | 4 +- .../crafter/materials/openapi_test.go | 6 +- pkg/attestation/crafter/materials/openvex.go | 5 +- .../crafter/materials/openvex_test.go | 2 +- .../crafter/materials/oversecured.go | 4 +- .../crafter/materials/oversecured_test.go | 2 +- pkg/attestation/crafter/materials/pitest.go | 5 +- .../crafter/materials/pitest_test.go | 2 +- pkg/attestation/crafter/materials/radamsa.go | 9 ++- .../crafter/materials/radamsa_test.go | 4 +- .../crafter/materials/runnercontext.go | 5 +- .../crafter/materials/runnercontext_test.go | 2 +- pkg/attestation/crafter/materials/sarif.go | 4 +- .../crafter/materials/sarif_test.go | 6 +- .../crafter/materials/scorecard.go | 4 +- .../crafter/materials/scorecard_test.go | 4 +- pkg/attestation/crafter/materials/sigcheck.go | 4 +- .../crafter/materials/sigcheck_test.go | 2 +- .../crafter/materials/slsaprovenance.go | 5 +- .../crafter/materials/slsaprovenance_test.go | 2 +- pkg/attestation/crafter/materials/spdxjson.go | 4 +- .../crafter/materials/spdxjson_test.go | 2 +- pkg/attestation/crafter/materials/string.go | 6 +- .../crafter/materials/string_test.go | 2 +- .../crafter/materials/trufflehog.go | 5 +- .../crafter/materials/trufflehog_test.go | 4 +- .../crafter/materials/twistcli_scan.go | 5 +- .../crafter/materials/twistcli_scan_test.go | 2 +- pkg/attestation/crafter/materials/zap.go | 5 +- pkg/attestation/crafter/materials/zap_test.go | 2 +- 86 files changed, 278 insertions(+), 242 deletions(-) create mode 100644 pkg/attestation/crafter/materials/craft_result_external_test.go create mode 100644 pkg/attestation/crafter/materials/craft_result_test.go diff --git a/app/cli/internal/policydevel/eval.go b/app/cli/internal/policydevel/eval.go index 689ff8ad9..bb4f0185c 100644 --- a/app/cli/internal/policydevel/eval.go +++ b/app/cli/internal/policydevel/eval.go @@ -88,7 +88,7 @@ func Evaluate(opts *EvalOptions, logger zerolog.Logger) (*EvalSummary, error) { // must be evaluated against — `policy devel eval` has to reproduce what // `attestation add` does, or a policy would be developed against input the // real run never sees. - summary, err := verifyMaterial(policies, material, opts.MaterialPath, crafted.EvaluableContent, opts.Debug, opts.AllowedHostnames, opts.AttestationClient, opts.ControlPlaneConn, opts.ProjectName, opts.ProjectVersionName, &logger) + summary, err := verifyMaterial(policies, material, opts.MaterialPath, crafted.Transformed, opts.Debug, opts.AllowedHostnames, opts.AttestationClient, opts.ControlPlaneConn, opts.ProjectName, opts.ProjectVersionName, &logger) if err != nil { return nil, err } @@ -150,7 +150,7 @@ func createPolicies(policyPath string, inputs map[string]string) (*v1.Policies, }, nil } -func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materialPath string, evaluableContent []byte, debug bool, allowedHostnames []string, attestationClient controlplanev1.AttestationServiceClient, grpcConn *grpc.ClientConn, projectName, projectVersion string, logger *zerolog.Logger) (*EvalSummary, error) { +func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materialPath string, transformed []byte, debug bool, allowedHostnames []string, attestationClient controlplanev1.AttestationServiceClient, grpcConn *grpc.ClientConn, projectName, projectVersion string, logger *zerolog.Logger) (*EvalSummary, error) { var opts []policies.PolicyVerifierOption if len(allowedHostnames) > 0 { opts = append(opts, policies.WithAllowedHostnames(allowedHostnames...)) @@ -165,7 +165,7 @@ func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materi v := policies.NewPolicyVerifier(pol, attestationClient, logger, opts...) policyEvs, err := v.VerifyMaterial(context.Background(), material, materialPath, - policies.WithMaterialContent(evaluableContent)) + policies.WithMaterialContent(transformed)) if err != nil { return nil, err } diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index 7e17d1bee..c9b7499bd 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -775,7 +775,7 @@ func (c *Crafter) stageMaterial(ctx context.Context, m *schemaapi.CraftingSchema // the policies below must see. Reading the file on disk instead would feed // user-authored Rego the very secrets redaction removed. nil for every other // material, which resolves its content the usual way. - withEvaluableContent := policies.WithMaterialContent(crafted.EvaluableContent) + withStoredContent := policies.WithMaterialContent(crafted.Transformed) // 4 - Populate annotations from the ones provided at runtime // a) we do not allow overriding values that come from the contract @@ -828,7 +828,7 @@ func (c *Crafter) stageMaterial(ctx context.Context, m *schemaapi.CraftingSchema policies.WithDefaultGate(c.CraftingState.Attestation.GetBlockOnPolicyViolation()), policies.WithProjectContext(projectName, projectVersion), ) - policyGroupResults, err := pgv.VerifyMaterial(ctx, mt, value, withEvaluableContent) + policyGroupResults, err := pgv.VerifyMaterial(ctx, mt, value, withStoredContent) if err != nil { return nil, fmt.Errorf("error applying policy groups to material: %w", err) } @@ -849,7 +849,7 @@ func (c *Crafter) stageMaterial(ctx context.Context, m *schemaapi.CraftingSchema policies.WithProjectContext(projectName, projectVersion), policies.WithRuntimeInputs(addOptions.runtimeInputs), ) - policyResults, err := pv.VerifyMaterial(ctx, mt, value, withEvaluableContent) + policyResults, err := pv.VerifyMaterial(ctx, mt, value, withStoredContent) if err != nil { return nil, fmt.Errorf("error applying policies to material: %w", err) } diff --git a/pkg/attestation/crafter/materials/accesschk.go b/pkg/attestation/crafter/materials/accesschk.go index e2b6890b3..a60dec66b 100644 --- a/pkg/attestation/crafter/materials/accesschk.go +++ b/pkg/attestation/crafter/materials/accesschk.go @@ -43,7 +43,7 @@ func NewAccessChkCrafter(schema *schemaapi.CraftingSchema_Material, backend *cas return &AccessChkCrafter{backend: backend, crafterCommon: craftCommon}, nil } -func (i *AccessChkCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *AccessChkCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { f, err := os.Open(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -71,7 +71,7 @@ func (i *AccessChkCrafter) Craft(ctx context.Context, filePath string) (*api.Att i.injectAnnotations(m, report) - return m, nil + return craftResult(m, nil) } func (i *AccessChkCrafter) injectAnnotations(m *api.Attestation_Material, report *accesschk.Report) { diff --git a/pkg/attestation/crafter/materials/accesschk_test.go b/pkg/attestation/crafter/materials/accesschk_test.go index a46827798..034042c35 100644 --- a/pkg/attestation/crafter/materials/accesschk_test.go +++ b/pkg/attestation/crafter/materials/accesschk_test.go @@ -120,7 +120,7 @@ func TestAccessChkCrafter_Craft(t *testing.T) { crafter, err := materials.NewAccessChkCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/artifact.go b/pkg/attestation/crafter/materials/artifact.go index 2364eab7d..c0cf774be 100644 --- a/pkg/attestation/crafter/materials/artifact.go +++ b/pkg/attestation/crafter/materials/artifact.go @@ -20,7 +20,6 @@ import ( "fmt" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/rs/zerolog" ) @@ -40,6 +39,6 @@ func NewArtifactCrafter(schema *schemaapi.CraftingSchema_Material, backend *casc } // Craft will calculate the digest of the artifact, simulate an upload and return the material definition -func (i *ArtifactCrafter) Craft(ctx context.Context, artifactPath string) (*api.Attestation_Material, error) { - return uploadAndCraft(ctx, i.input, i.backend, artifactPath, i.logger) +func (i *ArtifactCrafter) Craft(ctx context.Context, artifactPath string) (*CraftResult, error) { + return craftResult(uploadAndCraft(ctx, i.input, i.backend, artifactPath, i.logger)) } diff --git a/pkg/attestation/crafter/materials/artifact_test.go b/pkg/attestation/crafter/materials/artifact_test.go index e42509a19..e9b0f80c1 100644 --- a/pkg/attestation/crafter/materials/artifact_test.go +++ b/pkg/attestation/crafter/materials/artifact_test.go @@ -88,7 +88,7 @@ func TestArtifactCraft(t *testing.T) { crafter, err := materials.NewArtifactCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/simple.txt") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/simple.txt")) assert.NoError(err) assert.Equal(contractAPI.CraftingSchema_Material_ARTIFACT.String(), got.MaterialType.String()) assert.True(got.UploadedToCas) @@ -116,7 +116,7 @@ func TestArtifactCraftInline(t *testing.T) { crafter, err := materials.NewArtifactCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/simple.txt") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/simple.txt")) assert.NoError(err) assertMaterial(t, got) }) @@ -129,7 +129,7 @@ func TestArtifactCraftInline(t *testing.T) { crafter, err := materials.NewArtifactCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/simple.txt") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/simple.txt")) assert.NoError(err) assertMaterial(t, got) }) diff --git a/pkg/attestation/crafter/materials/asyncapi.go b/pkg/attestation/crafter/materials/asyncapi.go index f4a5912cd..f524dcbf6 100644 --- a/pkg/attestation/crafter/materials/asyncapi.go +++ b/pkg/attestation/crafter/materials/asyncapi.go @@ -61,7 +61,7 @@ func NewAsyncAPICrafter(materialSchema *schemaapi.CraftingSchema_Material, backe return c, nil } -func (i *AsyncAPICrafter) Craft(ctx context.Context, filepath string) (*api.Attestation_Material, error) { +func (i *AsyncAPICrafter) Craft(ctx context.Context, filepath string) (*CraftResult, error) { i.logger.Debug().Str("path", filepath).Msg("decoding AsyncAPI spec file") f, err := os.ReadFile(filepath) @@ -103,7 +103,7 @@ func (i *AsyncAPICrafter) Craft(ctx context.Context, filepath string) (*api.Atte i.injectAnnotations(m, doc) - return m, nil + return craftResult(m, nil) } func (i *AsyncAPICrafter) injectAnnotations(m *api.Attestation_Material, doc map[string]interface{}) { diff --git a/pkg/attestation/crafter/materials/asyncapi_test.go b/pkg/attestation/crafter/materials/asyncapi_test.go index c5b1daa95..640ba77ef 100644 --- a/pkg/attestation/crafter/materials/asyncapi_test.go +++ b/pkg/attestation/crafter/materials/asyncapi_test.go @@ -155,7 +155,7 @@ func TestAsyncAPICraft(t *testing.T) { crafter, err := materials.NewAsyncAPICrafter(tc.schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return @@ -194,7 +194,7 @@ func TestAsyncAPICraftNoStrictValidation(t *testing.T) { crafter, err := materials.NewAsyncAPICrafter(schema, backend, &l, materials.WithAsyncAPINoStrictValidation(true)) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/asyncapi-invalid.json") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/asyncapi-invalid.json")) require.NoError(t, err) assert.NotNil(t, got) assert.Equal(t, schema.Type.String(), got.MaterialType.String()) diff --git a/pkg/attestation/crafter/materials/attestation.go b/pkg/attestation/crafter/materials/attestation.go index 6c69b8942..cbb1cb399 100644 --- a/pkg/attestation/crafter/materials/attestation.go +++ b/pkg/attestation/crafter/materials/attestation.go @@ -21,7 +21,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" attestation2 "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/attestation" "github.com/chainloop-dev/chainloop/pkg/attestation/renderer/chainloop" "github.com/chainloop-dev/chainloop/pkg/casclient" @@ -49,7 +48,7 @@ func NewAttestationCrafter(schema *schemaapi.CraftingSchema_Material, backend *c } // Craft will calculate the digest of the artifact, simulate an upload and return the material definition -func (i *AttestationCrafter) Craft(ctx context.Context, artifactPath string) (*api.Attestation_Material, error) { +func (i *AttestationCrafter) Craft(ctx context.Context, artifactPath string) (*CraftResult, error) { data, err := os.ReadFile(artifactPath) if err != nil { return nil, fmt.Errorf("artifact file cannot be read: %w", err) @@ -81,5 +80,5 @@ func (i *AttestationCrafter) Craft(ctx context.Context, artifactPath string) (*a return nil, fmt.Errorf("the provided predicate is not a valid chainloop attestation: found=%q", intotoStatement.PredicateType) } - return uploadAndCraft(ctx, i.input, i.backend, artifactPath, i.logger) + return craftResult(uploadAndCraft(ctx, i.input, i.backend, artifactPath, i.logger)) } diff --git a/pkg/attestation/crafter/materials/attestation_test.go b/pkg/attestation/crafter/materials/attestation_test.go index c70497788..42e6c3d81 100644 --- a/pkg/attestation/crafter/materials/attestation_test.go +++ b/pkg/attestation/crafter/materials/attestation_test.go @@ -148,7 +148,7 @@ func TestAttestationCraft(t *testing.T) { crafter, err := materials.NewAttestationCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.Background(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.Background(), tc.filePath)) if tc.expectErr { assert.Error(err) return @@ -180,7 +180,7 @@ func TestAttestationCraftInline(t *testing.T) { crafter, err := materials.NewAttestationCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/attestation-dsse.json") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/attestation-dsse.json")) assert.NoError(err) assert.NotNil(got) diff --git a/pkg/attestation/crafter/materials/blackduck.go b/pkg/attestation/crafter/materials/blackduck.go index 153745feb..f60a8c9f8 100644 --- a/pkg/attestation/crafter/materials/blackduck.go +++ b/pkg/attestation/crafter/materials/blackduck.go @@ -22,7 +22,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/rs/zerolog" ) @@ -49,7 +48,7 @@ type blackduckRequiredFields struct { DetailedCodeLocations any `json:"detailedCodeLocations"` } -func (i *BlackduckSCAJSONCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *BlackduckSCAJSONCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { data, err := os.ReadFile(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -65,5 +64,5 @@ func (i *BlackduckSCAJSONCrafter) Craft(ctx context.Context, filePath string) (* return nil, fmt.Errorf("invalid Blackduck SCA scan: %w", ErrInvalidMaterialType) } - return uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger) + return craftResult(uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger)) } diff --git a/pkg/attestation/crafter/materials/blackduck_test.go b/pkg/attestation/crafter/materials/blackduck_test.go index 07e3731fa..c390668c0 100644 --- a/pkg/attestation/crafter/materials/blackduck_test.go +++ b/pkg/attestation/crafter/materials/blackduck_test.go @@ -86,7 +86,7 @@ func TestBlackduckJSONCraft(t *testing.T) { crafter, err := materials.NewBlackduckSCAJSONCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/chainloop_ai_agent_config.go b/pkg/attestation/crafter/materials/chainloop_ai_agent_config.go index a89e1e03b..1ce0952b0 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_agent_config.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_agent_config.go @@ -50,7 +50,7 @@ func NewChainloopAIAgentConfigCrafter(schema *schemaapi.CraftingSchema_Material, // Craft validates the AI agent config against the JSON schema, calculates the digest, // uploads it and returns the material definition. -func (c *ChainloopAIAgentConfigCrafter) Craft(ctx context.Context, artifactPath string) (*api.Attestation_Material, error) { +func (c *ChainloopAIAgentConfigCrafter) Craft(ctx context.Context, artifactPath string) (*CraftResult, error) { f, err := os.ReadFile(artifactPath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -93,5 +93,5 @@ func (c *ChainloopAIAgentConfigCrafter) Craft(ctx context.Context, artifactPath material.Annotations[annotationAIAgentName] = data.Agent.Name } - return material, nil + return craftResult(material, nil) } diff --git a/pkg/attestation/crafter/materials/chainloop_ai_agent_config_test.go b/pkg/attestation/crafter/materials/chainloop_ai_agent_config_test.go index cf425f6d3..46145e186 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_agent_config_test.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_agent_config_test.go @@ -300,7 +300,7 @@ func TestChainloopAIAgentConfigCrafter_AgentNameAnnotation(t *testing.T) { crafter, err := NewChainloopAIAgentConfigCrafter(schema, backend, &logger) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) require.NoError(t, err) assert.Equal(t, tc.expectedAgentName, got.Annotations[annotationAIAgentName]) diff --git a/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go b/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go index 8f4bb2259..b208b6b76 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go @@ -69,28 +69,14 @@ func NewChainloopAICodingSessionCrafter(schema *schemaapi.CraftingSchema_Materia // secrets found in it, calculates the digest, uploads it and returns the // material definition. // -// The file on disk is left untouched; only the stored copy is redacted, which -// means the sanitized bytes are dropped here. Callers that go on to evaluate -// policies must craft through materials.Craft and pass on the EvaluableContent it -// returns, or evaluation of a redacted session will fail rather than silently read -// the original from disk. -func (c *ChainloopAICodingSessionCrafter) Craft(ctx context.Context, artifactPath string) (*api.Attestation_Material, error) { - material, _, err := c.transformCraft(ctx, artifactPath) - return material, err -} - -// transformCraft is Craft, additionally returning the bytes it stored in place of -// the artifact: the sanitized copy when redaction replaced something, and nil -// otherwise, meaning the file on disk is what was stored and can be read from -// there as usual. -// -// Redaction only rewrites the copy that leaves the machine, so a caller that -// reads the artifact back off disk gets the credentials the session captured. -// That is what these bytes exist to prevent. -func (c *ChainloopAICodingSessionCrafter) transformCraft(ctx context.Context, artifactPath string) (*api.Attestation_Material, []byte, error) { +// 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.Transformed +// 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) { f, err := os.ReadFile(artifactPath) if err != nil { - return nil, nil, fmt.Errorf("can't open the file: %w", err) + return nil, fmt.Errorf("can't open the file: %w", err) } // Unmarshal envelope, keeping data as raw JSON for schema validation @@ -99,30 +85,30 @@ func (c *ChainloopAICodingSessionCrafter) transformCraft(ctx context.Context, ar } if err := json.Unmarshal(f, &envelope); err != nil { c.logger.Debug().Err(err).Msg("error decoding file") - return nil, nil, fmt.Errorf("invalid JSON format: %w", err) + return nil, fmt.Errorf("invalid JSON format: %w", err) } // Unmarshal data into typed struct for annotation extraction var data aicodingsession.Data if err := json.Unmarshal(envelope.Data, &data); err != nil { c.logger.Debug().Err(err).Msg("error decoding data field") - return nil, nil, fmt.Errorf("failed to unmarshal data: %w", err) + return nil, fmt.Errorf("failed to unmarshal data: %w", err) } // Validate using raw JSON to preserve unknown fields for strict schema validation var rawData any if err := json.Unmarshal(envelope.Data, &rawData); err != nil { - return nil, nil, fmt.Errorf("failed to unmarshal data for validation: %w", err) + return nil, fmt.Errorf("failed to unmarshal data for validation: %w", err) } if err := schemavalidators.ValidateAICodingSession(rawData, schemavalidators.AICodingSessionVersion0_1); err != nil { c.logger.Debug().Err(err).Msg("schema validation failed") - return nil, nil, fmt.Errorf("AI coding session validation failed: %w", err) + return nil, fmt.Errorf("AI coding session validation failed: %w", err) } redacted, report, err := c.redact(ctx, f) if err != nil { - return nil, nil, err + return nil, err } // Substituting the stored content only when something was actually replaced @@ -134,7 +120,7 @@ func (c *ChainloopAICodingSessionCrafter) transformCraft(ctx context.Context, ar material, err := uploadAndCraft(ctx, c.input, c.backend, artifactPath, c.logger, craftOpts...) if err != nil { - return nil, nil, err + return nil, err } c.annotateRedaction(material, report) @@ -149,7 +135,7 @@ func (c *ChainloopAICodingSessionCrafter) transformCraft(ctx context.Context, ar material.Annotations[annotationAICodingModel] = data.Model.Primary } - return material, redacted, nil + return &CraftResult{Material: material, Transformed: redacted}, nil } // redact strips secrets out of the session content, returning the sanitized copy diff --git a/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go b/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go index 5aa1e5037..db6f41525 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go @@ -269,8 +269,9 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { WithAICodingSessionSkipRedaction(tc.skipRedaction)) require.NoError(t, err) - got, evaluable, err := crafter.transformCraft(context.TODO(), path) + res, err := crafter.Craft(context.TODO(), path) require.NoError(t, err) + got, transformed := res.Material, res.Transformed if tc.inlineBackend { require.True(t, got.InlineCas) @@ -290,13 +291,13 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { // 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, evaluable, "a redacted session must hand back its sanitized copy") - assert.Equal(t, sha256Digest(string(evaluable)), got.GetArtifact().Digest) - assert.NotContains(t, string(evaluable), awsKey) - assert.Contains(t, string(evaluable), "[REDACTED:aws-access-token]") + require.NotNil(t, transformed, "a redacted session must hand back its sanitized copy") + assert.Equal(t, sha256Digest(string(transformed)), got.GetArtifact().Digest) + assert.NotContains(t, string(transformed), awsKey) + assert.Contains(t, string(transformed), "[REDACTED:aws-access-token]") if stored != nil { - assert.Equal(t, string(stored), string(evaluable)) + assert.Equal(t, string(stored), string(transformed)) assert.NotContains(t, string(stored), awsKey) assert.Contains(t, string(stored), "[REDACTED:aws-access-token]") } @@ -306,13 +307,13 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { 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, evaluable) + assert.Nil(t, transformed) default: assert.NotContains(t, 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, evaluable) + assert.Nil(t, transformed) } // Redaction must never touch the source file. diff --git a/pkg/attestation/crafter/materials/chainloop_ai_coding_session_test.go b/pkg/attestation/crafter/materials/chainloop_ai_coding_session_test.go index 82ce7fd9c..11144f23d 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_coding_session_test.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_coding_session_test.go @@ -339,7 +339,7 @@ func TestChainloopAICodingSessionCrafter_Annotations(t *testing.T) { crafter, err := NewChainloopAICodingSessionCrafter(schema, backend, &logger) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) require.NoError(t, err) assert.Equal(t, tc.expectedAgentName, got.Annotations[annotationAIAgentName]) diff --git a/pkg/attestation/crafter/materials/chainloop_ai_security_context.go b/pkg/attestation/crafter/materials/chainloop_ai_security_context.go index 1d4ee9504..8ddc0aa3b 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_security_context.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_security_context.go @@ -55,7 +55,7 @@ func NewChainloopAISecurityContextCrafter(schema *schemaapi.CraftingSchema_Mater // Craft validates the AI security context against the JSON schema, calculates the // digest, uploads it and returns the material definition. -func (c *ChainloopAISecurityContextCrafter) Craft(ctx context.Context, artifactPath string) (*api.Attestation_Material, error) { +func (c *ChainloopAISecurityContextCrafter) Craft(ctx context.Context, artifactPath string) (*CraftResult, error) { f, err := os.ReadFile(artifactPath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -106,7 +106,7 @@ func (c *ChainloopAISecurityContextCrafter) Craft(ctx context.Context, artifactP c.annotate(material, &data) - return material, nil + return craftResult(material, nil) } // validateSecurityContextEnvelope checks the two constant envelope fields and diff --git a/pkg/attestation/crafter/materials/chainloop_ai_security_context_test.go b/pkg/attestation/crafter/materials/chainloop_ai_security_context_test.go index 860a0092b..245701618 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_security_context_test.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_security_context_test.go @@ -115,7 +115,7 @@ func TestChainloopAISecurityContextCrafter_Craft(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - got, err := newSecurityContextCrafter(t).Craft(context.TODO(), tc.path) + got, err := craftedMaterial(newSecurityContextCrafter(t).Craft(context.TODO(), tc.path)) if tc.wantErr != "" { require.Error(t, err) @@ -232,7 +232,7 @@ func TestChainloopAISecurityContextCrafter_Annotations(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - got, err := newSecurityContextCrafter(t).Craft(context.TODO(), tc.path) + got, err := craftedMaterial(newSecurityContextCrafter(t).Craft(context.TODO(), tc.path)) require.NoError(t, err) assert.Equal(t, tc.headSHA, got.Annotations[annotationSecurityContextHeadSHA]) @@ -272,7 +272,7 @@ func TestChainloopAISecurityContextCrafter_ToolAnnotations(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - got, err := newSecurityContextCrafter(t).Craft(context.TODO(), tc.path) + got, err := craftedMaterial(newSecurityContextCrafter(t).Craft(context.TODO(), tc.path)) require.NoError(t, err) assert.Equal(t, tc.wantTools, got.Annotations[AnnotationToolsKey]) @@ -321,7 +321,7 @@ func TestChainloopAISecurityContextCrafter_ToolAnnotationsIncomplete(t *testing. t.Run(tc.name, func(t *testing.T) { path := securityContextWithProvenance(t, tc.tool, tc.toolVersion) - got, err := newSecurityContextCrafter(t).Craft(context.TODO(), path) + got, err := craftedMaterial(newSecurityContextCrafter(t).Craft(context.TODO(), path)) require.NoError(t, err) assert.Equal(t, tc.wantTools, got.Annotations[AnnotationToolsKey]) @@ -381,7 +381,7 @@ func TestChainloopAISecurityContextCrafter_ReconcilesIsAlwaysAnnotated(t *testin path := filepath.Join(t.TempDir(), "security-context.json") require.NoError(t, os.WriteFile(path, mustMarshal(t, doc), 0o600)) - got, err := newSecurityContextCrafter(t).Craft(context.TODO(), path) + got, err := craftedMaterial(newSecurityContextCrafter(t).Craft(context.TODO(), path)) require.NoError(t, err) assert.Equal(t, "false", got.Annotations[annotationSecurityContextReconciles]) } diff --git a/pkg/attestation/crafter/materials/chainloop_pr_info.go b/pkg/attestation/crafter/materials/chainloop_pr_info.go index 275052d39..46694c4a4 100644 --- a/pkg/attestation/crafter/materials/chainloop_pr_info.go +++ b/pkg/attestation/crafter/materials/chainloop_pr_info.go @@ -22,7 +22,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/chainloop-dev/chainloop/pkg/prinfo" @@ -48,7 +47,7 @@ func NewChainloopPRInfoCrafter(schema *schemaapi.CraftingSchema_Material, backen // Craft will validate the PR info against the JSON schema, calculate the digest of the artifact, // upload it and return the material definition. -func (i *ChainloopPRInfoCrafter) Craft(ctx context.Context, artifactPath string) (*api.Attestation_Material, error) { +func (i *ChainloopPRInfoCrafter) Craft(ctx context.Context, artifactPath string) (*CraftResult, error) { // Read the file f, err := os.ReadFile(artifactPath) if err != nil { @@ -85,5 +84,5 @@ func (i *ChainloopPRInfoCrafter) Craft(ctx context.Context, artifactPath string) return nil, err } - return material, nil + return craftResult(material, nil) } diff --git a/pkg/attestation/crafter/materials/checkmarx.go b/pkg/attestation/crafter/materials/checkmarx.go index fef3a5caa..fc0e6cc52 100644 --- a/pkg/attestation/crafter/materials/checkmarx.go +++ b/pkg/attestation/crafter/materials/checkmarx.go @@ -84,7 +84,7 @@ func NewCheckmarxCrafter(schema *schemaapi.CraftingSchema_Material, backend *cas return &CheckmarxCrafter{backend: backend, crafterCommon: craftCommon}, nil } -func (i *CheckmarxCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *CheckmarxCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { data, err := os.ReadFile(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -138,7 +138,7 @@ func (i *CheckmarxCrafter) Craft(ctx context.Context, filePath string) (*api.Att i.injectAnnotations(m, typeSet) - return m, nil + return craftResult(m, nil) } func (i *CheckmarxCrafter) injectAnnotations(m *api.Attestation_Material, typeSet map[string]struct{}) { diff --git a/pkg/attestation/crafter/materials/checkmarx_test.go b/pkg/attestation/crafter/materials/checkmarx_test.go index b80fdd1d8..39eeba06e 100644 --- a/pkg/attestation/crafter/materials/checkmarx_test.go +++ b/pkg/attestation/crafter/materials/checkmarx_test.go @@ -154,7 +154,7 @@ func TestCheckmarxCrafter_Craft(t *testing.T) { crafter, err := materials.NewCheckmarxCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/cobertura.go b/pkg/attestation/crafter/materials/cobertura.go index 54738a724..8f3a1eb5a 100644 --- a/pkg/attestation/crafter/materials/cobertura.go +++ b/pkg/attestation/crafter/materials/cobertura.go @@ -23,7 +23,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/cobertura" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/rs/zerolog" @@ -41,7 +40,7 @@ func NewCoberturaCrafter(schema *schemaapi.CraftingSchema_Material, backend *cas } } -func (c *CoberturaCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (c *CoberturaCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { f, err := os.Open(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -73,5 +72,5 @@ func (c *CoberturaCrafter) Craft(ctx context.Context, filePath string) (*api.Att return nil, fmt.Errorf("invalid Cobertura report file, missing coverage data: %w", ErrInvalidMaterialType) } - return uploadAndCraft(ctx, c.input, c.backend, filePath, c.logger) + return craftResult(uploadAndCraft(ctx, c.input, c.backend, filePath, c.logger)) } diff --git a/pkg/attestation/crafter/materials/cobertura_test.go b/pkg/attestation/crafter/materials/cobertura_test.go index 4431a4884..b2437ac0e 100644 --- a/pkg/attestation/crafter/materials/cobertura_test.go +++ b/pkg/attestation/crafter/materials/cobertura_test.go @@ -88,7 +88,7 @@ func TestCoberturaCraft(t *testing.T) { backend := &casclient.CASBackend{Uploader: uploader} crafter := materials.NewCoberturaCrafter(schema, backend, &l) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return @@ -122,7 +122,7 @@ func TestCoberturaCraftEmptyReport(t *testing.T) { backend := &casclient.CASBackend{Uploader: uploader} crafter := materials.NewCoberturaCrafter(schema, backend, &l) - got, err := crafter.Craft(context.TODO(), "./testdata/cobertura-empty.xml") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/cobertura-empty.xml")) require.NoError(t, err, "an empty-but-valid cobertura report must be accepted") require.NotNil(t, got) assert.Equal(t, contractAPI.CraftingSchema_Material_COBERTURA_XML.String(), got.MaterialType.String()) diff --git a/pkg/attestation/crafter/materials/craft_result_external_test.go b/pkg/attestation/crafter/materials/craft_result_external_test.go new file mode 100644 index 000000000..cfb6377fe --- /dev/null +++ b/pkg/attestation/crafter/materials/craft_result_external_test.go @@ -0,0 +1,36 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package materials_test + +import ( + attestationApi "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" + "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials" +) + +// craftedMaterial unwraps a Craft call for the tests that assert only on the +// material. It keeps the (value, error) shape so wrapping a call changes nothing +// about how the error is handled: +// +// got, err := craftedMaterial(crafter.Craft(ctx, path)) +// +// Tests that care about what the crafter stored in place of the artifact use +// CraftResult.Transformed directly instead. +func craftedMaterial(res *materials.CraftResult, err error) (*attestationApi.Attestation_Material, error) { + if res == nil { + return nil, err + } + return res.Material, err +} diff --git a/pkg/attestation/crafter/materials/craft_result_test.go b/pkg/attestation/crafter/materials/craft_result_test.go new file mode 100644 index 000000000..436ad68b6 --- /dev/null +++ b/pkg/attestation/crafter/materials/craft_result_test.go @@ -0,0 +1,35 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package materials + +import ( + api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" +) + +// craftedMaterial unwraps a Craft call for the tests that assert only on the +// material. It keeps the (value, error) shape so wrapping a call changes nothing +// about how the error is handled: +// +// got, err := craftedMaterial(crafter.Craft(ctx, path)) +// +// Tests that care about what the crafter stored in place of the artifact use +// CraftResult.Transformed directly instead. +func craftedMaterial(res *CraftResult, err error) (*api.Attestation_Material, error) { + if res == nil { + return nil, err + } + return res.Material, err +} diff --git a/pkg/attestation/crafter/materials/csaf.go b/pkg/attestation/crafter/materials/csaf.go index 212eca421..d888dcc7d 100644 --- a/pkg/attestation/crafter/materials/csaf.go +++ b/pkg/attestation/crafter/materials/csaf.go @@ -88,7 +88,7 @@ func baseCSAFCrafter(materialSchema *schemaapi.CraftingSchema_Material, backend }, nil } -func (i *CSAFCrafter) Craft(ctx context.Context, filepath string) (*api.Attestation_Material, error) { +func (i *CSAFCrafter) Craft(ctx context.Context, filepath string) (*CraftResult, error) { i.logger.Debug().Str("path", filepath).Msg("decoding CSAF file") f, err := os.ReadFile(filepath) if err != nil { @@ -136,7 +136,7 @@ func (i *CSAFCrafter) Craft(ctx context.Context, filepath string) (*api.Attestat i.injectAnnotations(m, documentMap) - return m, nil + return craftResult(m, nil) } func (i *CSAFCrafter) injectAnnotations(m *api.Attestation_Material, documentMap map[string]any) { diff --git a/pkg/attestation/crafter/materials/csaf_test.go b/pkg/attestation/crafter/materials/csaf_test.go index 46a9b6e3b..574b015d3 100644 --- a/pkg/attestation/crafter/materials/csaf_test.go +++ b/pkg/attestation/crafter/materials/csaf_test.go @@ -245,7 +245,7 @@ func TestCSAFCraft(t *testing.T) { require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/cyclonedxjson.go b/pkg/attestation/crafter/materials/cyclonedxjson.go index a0364ffd7..5a6cf426e 100644 --- a/pkg/attestation/crafter/materials/cyclonedxjson.go +++ b/pkg/attestation/crafter/materials/cyclonedxjson.go @@ -113,7 +113,7 @@ func NewCyclonedxJSONCrafter(materialSchema *schemaapi.CraftingSchema_Material, return c, nil } -func (i *CyclonedxJSONCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *CyclonedxJSONCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { f, err := os.ReadFile(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -181,7 +181,7 @@ func (i *CyclonedxJSONCrafter) Craft(ctx context.Context, filePath string) (*api i.injectAnnotations(m, &doc) - return res, nil + return craftResult(res, nil) } func (i *CyclonedxJSONCrafter) injectAnnotations(m *api.Attestation_Material, doc *cyclonedxDoc) { diff --git a/pkg/attestation/crafter/materials/cyclonedxjson_test.go b/pkg/attestation/crafter/materials/cyclonedxjson_test.go index 437f6268a..93c43057b 100644 --- a/pkg/attestation/crafter/materials/cyclonedxjson_test.go +++ b/pkg/attestation/crafter/materials/cyclonedxjson_test.go @@ -221,7 +221,7 @@ func TestCyclonedxJSONCraft(t *testing.T) { crafter, err := materials.NewCyclonedxJSONCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { ast.ErrorContains(err, tc.wantErr) return @@ -314,7 +314,7 @@ func TestCycloneDXJSONCraftNoStrictValidation(t *testing.T) { materials.WithCycloneDXNoStrictValidation(tc.noStrictValidation)) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { ast.ErrorContains(err, tc.wantErr) return @@ -370,7 +370,7 @@ func TestCycloneDXJSONCraft_SkipUpload(t *testing.T) { crafter, err := materials.NewCyclonedxJSONCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), filePath)) require.NoError(t, err) ast.Equal(contractAPI.CraftingSchema_Material_SBOM_CYCLONEDX_JSON.String(), got.MaterialType.String()) diff --git a/pkg/attestation/crafter/materials/detect_secrets.go b/pkg/attestation/crafter/materials/detect_secrets.go index 9fa3e958b..4dcc81774 100644 --- a/pkg/attestation/crafter/materials/detect_secrets.go +++ b/pkg/attestation/crafter/materials/detect_secrets.go @@ -49,7 +49,7 @@ func NewDetectSecretsCrafter(schema *schemaapi.CraftingSchema_Material, backend return &DetectSecretsCrafter{backend: backend, crafterCommon: craftCommon}, nil } -func (i *DetectSecretsCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *DetectSecretsCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { data, err := os.ReadFile(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -80,7 +80,7 @@ func (i *DetectSecretsCrafter) Craft(ctx context.Context, filePath string) (*api i.injectAnnotations(m, &baseline) - return m, nil + return craftResult(m, nil) } func (i *DetectSecretsCrafter) injectAnnotations(m *api.Attestation_Material, baseline *detectSecretsBaseline) { diff --git a/pkg/attestation/crafter/materials/detect_secrets_test.go b/pkg/attestation/crafter/materials/detect_secrets_test.go index 39668f8cf..c87f4b422 100644 --- a/pkg/attestation/crafter/materials/detect_secrets_test.go +++ b/pkg/attestation/crafter/materials/detect_secrets_test.go @@ -122,7 +122,7 @@ func TestDetectSecretsCrafter_Craft(t *testing.T) { crafter, err := materials.NewDetectSecretsCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/dranzer.go b/pkg/attestation/crafter/materials/dranzer.go index f42ff16df..1a0409401 100644 --- a/pkg/attestation/crafter/materials/dranzer.go +++ b/pkg/attestation/crafter/materials/dranzer.go @@ -55,7 +55,7 @@ func NewDranzerCrafter(schema *schemaapi.CraftingSchema_Material, backend *cascl return &DranzerCrafter{backend: backend, crafterCommon: craftCommon}, nil } -func (i *DranzerCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *DranzerCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { // dranzer emits free-form text, so the fingerprint is soft: the input only has // to resemble dranzer output (a test-engine version banner, a parsed object or // finding, or the run-summary line). Inspect accepts a single report or an @@ -84,7 +84,7 @@ func (i *DranzerCrafter) Craft(ctx context.Context, filePath string) (*api.Attes i.injectAnnotations(m, inspection) - return m, nil + return craftResult(m, nil) } func (i *DranzerCrafter) injectAnnotations(m *api.Attestation_Material, inspection dranzer.Inspection) { diff --git a/pkg/attestation/crafter/materials/dranzer_test.go b/pkg/attestation/crafter/materials/dranzer_test.go index af7c71bef..bff522bf7 100644 --- a/pkg/attestation/crafter/materials/dranzer_test.go +++ b/pkg/attestation/crafter/materials/dranzer_test.go @@ -147,7 +147,7 @@ func TestDranzerCrafter_Craft(t *testing.T) { crafter, err := materials.NewDranzerCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return @@ -183,7 +183,7 @@ func TestDranzerCrafter_CraftArchive(t *testing.T) { } crafter, err := materials.NewDranzerCrafter(schema, &casclient.CASBackend{Uploader: uploader}, &l) require.NoError(t, err) - return crafter.Craft(context.TODO(), path) + return craftedMaterial(crafter.Craft(context.TODO(), path)) } t.Run("zip of the four mode reports is accepted", func(t *testing.T) { diff --git a/pkg/attestation/crafter/materials/evidence.go b/pkg/attestation/crafter/materials/evidence.go index a3b71943f..028d6c128 100644 --- a/pkg/attestation/crafter/materials/evidence.go +++ b/pkg/attestation/crafter/materials/evidence.go @@ -66,7 +66,7 @@ func NewEvidenceCrafter(schema *schemaapi.CraftingSchema_Material, backend *casc // Craft will calculate the digest of the artifact, simulate an upload and return the material definition // If the evidence is in JSON format with id, data (and optionally schema) fields, // it will extract those as annotations -func (i *EvidenceCrafter) Craft(ctx context.Context, artifactPath string) (*api.Attestation_Material, error) { +func (i *EvidenceCrafter) Craft(ctx context.Context, artifactPath string) (*CraftResult, error) { material, err := uploadAndCraft(ctx, i.input, i.backend, artifactPath, i.logger) if err != nil { return nil, err @@ -75,7 +75,7 @@ func (i *EvidenceCrafter) Craft(ctx context.Context, artifactPath string) (*api. // Try to parse as JSON and extract annotations i.tryExtractAnnotations(material, artifactPath) - return material, nil + return craftResult(material, nil) } // tryExtractAnnotations attempts to parse the evidence as JSON and extract id/schema fields as annotations diff --git a/pkg/attestation/crafter/materials/evidence_test.go b/pkg/attestation/crafter/materials/evidence_test.go index b73e9d517..5214b6353 100644 --- a/pkg/attestation/crafter/materials/evidence_test.go +++ b/pkg/attestation/crafter/materials/evidence_test.go @@ -88,7 +88,7 @@ func TestEvidenceCraft(t *testing.T) { crafter, err := materials.NewEvidenceCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/simple.txt") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/simple.txt")) assert.NoError(err) assert.Equal(contractAPI.CraftingSchema_Material_EVIDENCE.String(), got.MaterialType.String()) assert.True(got.UploadedToCas) @@ -113,7 +113,7 @@ func TestEvidenceCraftInline(t *testing.T) { crafter, err := materials.NewEvidenceCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/simple.txt") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/simple.txt")) assert.NoError(err) assertEvidenceMaterial(t, got) }) @@ -126,7 +126,7 @@ func TestEvidenceCraftInline(t *testing.T) { crafter, err := materials.NewEvidenceCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/simple.txt") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/simple.txt")) assert.NoError(err) assertEvidenceMaterial(t, got) }) @@ -225,7 +225,7 @@ func TestEvidenceCraftWithJSONAnnotations(t *testing.T) { crafter, err := materials.NewEvidenceCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) assert.NoError(err) assert.Equal(contractAPI.CraftingSchema_Material_EVIDENCE.String(), got.MaterialType.String()) diff --git a/pkg/attestation/crafter/materials/ghas_code_scan.go b/pkg/attestation/crafter/materials/ghas_code_scan.go index d565ec56e..320e22b2c 100644 --- a/pkg/attestation/crafter/materials/ghas_code_scan.go +++ b/pkg/attestation/crafter/materials/ghas_code_scan.go @@ -22,7 +22,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/google/go-github/v66/github" "github.com/rs/zerolog" @@ -45,7 +44,7 @@ func NewGHASCodeScanCrafter(materialSchema *schemaapi.CraftingSchema_Material, b } // Craft will validate the CodeScan alerts report and craft the material -func (i *GHASCodeScanCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *GHASCodeScanCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { var alerts []*github.Alert report, err := os.ReadFile(filePath) @@ -70,5 +69,5 @@ func (i *GHASCodeScanCrafter) Craft(ctx context.Context, filePath string) (*api. } // Call uploadAndCraft with the path of the JSON report file - return uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger) + return craftResult(uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger)) } diff --git a/pkg/attestation/crafter/materials/ghas_dependency_scan.go b/pkg/attestation/crafter/materials/ghas_dependency_scan.go index 892de6ef4..998570478 100644 --- a/pkg/attestation/crafter/materials/ghas_dependency_scan.go +++ b/pkg/attestation/crafter/materials/ghas_dependency_scan.go @@ -22,7 +22,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/google/go-github/v66/github" "github.com/rs/zerolog" @@ -45,7 +44,7 @@ func NewGHASDependencyScanCrafter(materialSchema *schemaapi.CraftingSchema_Mater } // Craft will validate the CodeScan alerts report and craft the material -func (i *GHASDependencyScanCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *GHASDependencyScanCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { var alerts []*github.DependabotAlert report, err := os.ReadFile(filePath) @@ -70,5 +69,5 @@ func (i *GHASDependencyScanCrafter) Craft(ctx context.Context, filePath string) } // Call uploadAndCraft with the path of the JSON report file - return uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger) + return craftResult(uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger)) } diff --git a/pkg/attestation/crafter/materials/ghas_secret_scan.go b/pkg/attestation/crafter/materials/ghas_secret_scan.go index dcf094c93..262eb9083 100644 --- a/pkg/attestation/crafter/materials/ghas_secret_scan.go +++ b/pkg/attestation/crafter/materials/ghas_secret_scan.go @@ -22,7 +22,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/google/go-github/v66/github" "github.com/rs/zerolog" @@ -45,7 +44,7 @@ func NewGHASSecretScanCrafter(materialSchema *schemaapi.CraftingSchema_Material, } // Craft will validate the CodeScan alerts report and craft the material -func (i *GHASSecretScanCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *GHASSecretScanCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { var alerts []*github.SecretScanningAlert report, err := os.ReadFile(filePath) @@ -71,5 +70,5 @@ func (i *GHASSecretScanCrafter) Craft(ctx context.Context, filePath string) (*ap } // Call uploadAndCraft with the path of the JSON report file - return uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger) + return craftResult(uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger)) } diff --git a/pkg/attestation/crafter/materials/ghas_test.go b/pkg/attestation/crafter/materials/ghas_test.go index 2a99d3a63..103affcad 100644 --- a/pkg/attestation/crafter/materials/ghas_test.go +++ b/pkg/attestation/crafter/materials/ghas_test.go @@ -117,7 +117,7 @@ func TestGHASCodeScanCraft(t *testing.T) { crafter, err := materials.NewGHASCodeScanCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return @@ -189,7 +189,7 @@ func TestGHASSecretScanCraft(t *testing.T) { crafter, err := materials.NewGHASSecretScanCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return @@ -261,7 +261,7 @@ func TestGHASDependencyScanCraft(t *testing.T) { crafter, err := materials.NewGHASDependencyScanCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/gitlab.go b/pkg/attestation/crafter/materials/gitlab.go index e97b2f519..9f51e619d 100644 --- a/pkg/attestation/crafter/materials/gitlab.go +++ b/pkg/attestation/crafter/materials/gitlab.go @@ -44,7 +44,7 @@ func NewGitlabCrafter(schema *schemaapi.CraftingSchema_Material, backend *cascli return &GitlabCrafter{backend: backend, crafterCommon: craftCommon}, nil } -func (i *GitlabCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *GitlabCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { data, err := os.ReadFile(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -66,7 +66,7 @@ func (i *GitlabCrafter) Craft(ctx context.Context, filePath string) (*api.Attest i.injectAnnotations(m, &glReport) - return m, nil + return craftResult(m, nil) } func (i *GitlabCrafter) injectAnnotations(m *api.Attestation_Material, glReport *report.Report) { diff --git a/pkg/attestation/crafter/materials/gitlab_test.go b/pkg/attestation/crafter/materials/gitlab_test.go index 301eee73c..673d20a1a 100644 --- a/pkg/attestation/crafter/materials/gitlab_test.go +++ b/pkg/attestation/crafter/materials/gitlab_test.go @@ -138,7 +138,7 @@ func TestGitlabCrafter_Craft(t *testing.T) { crafter, err := materials.NewGitlabCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/gitleaks.go b/pkg/attestation/crafter/materials/gitleaks.go index a6cbff237..054aefcb9 100644 --- a/pkg/attestation/crafter/materials/gitleaks.go +++ b/pkg/attestation/crafter/materials/gitleaks.go @@ -41,7 +41,7 @@ func NewGitleaksReportCrafter(schema *schemaapi.CraftingSchema_Material, backend return &GitleaksReportCrafter{backend: backend, crafterCommon: craftCommon}, nil } -func (i *GitleaksReportCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *GitleaksReportCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { var findings []report.Finding data, err := os.ReadFile(filePath) @@ -73,7 +73,7 @@ func (i *GitleaksReportCrafter) Craft(ctx context.Context, filePath string) (*ap i.injectAnnotations(m) - return m, nil + return craftResult(m, nil) } func (i *GitleaksReportCrafter) injectAnnotations(m *api.Attestation_Material) { diff --git a/pkg/attestation/crafter/materials/gitleaks_test.go b/pkg/attestation/crafter/materials/gitleaks_test.go index e28d241e3..f75b4e8b4 100644 --- a/pkg/attestation/crafter/materials/gitleaks_test.go +++ b/pkg/attestation/crafter/materials/gitleaks_test.go @@ -120,7 +120,7 @@ func TestGitleaksReportCrafter_Craft(t *testing.T) { crafter, err := materials.NewGitleaksReportCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/graphql.go b/pkg/attestation/crafter/materials/graphql.go index 81b6ed9d5..d3673d292 100644 --- a/pkg/attestation/crafter/materials/graphql.go +++ b/pkg/attestation/crafter/materials/graphql.go @@ -47,7 +47,7 @@ func NewGraphQLCrafter(materialSchema *schemaapi.CraftingSchema_Material, backen }, nil } -func (i *GraphQLCrafter) Craft(ctx context.Context, filepath string) (*api.Attestation_Material, error) { +func (i *GraphQLCrafter) Craft(ctx context.Context, filepath string) (*CraftResult, error) { i.logger.Debug().Str("path", filepath).Msg("decoding GraphQL SDL file") content, err := os.ReadFile(filepath) @@ -69,7 +69,7 @@ func (i *GraphQLCrafter) Craft(ctx context.Context, filepath string) (*api.Attes i.injectAnnotations(m, doc) - return m, nil + return craftResult(m, nil) } func (i *GraphQLCrafter) injectAnnotations(m *api.Attestation_Material, doc *ast.SchemaDocument) { diff --git a/pkg/attestation/crafter/materials/graphql_test.go b/pkg/attestation/crafter/materials/graphql_test.go index c2e1ec3fc..3322eccec 100644 --- a/pkg/attestation/crafter/materials/graphql_test.go +++ b/pkg/attestation/crafter/materials/graphql_test.go @@ -130,7 +130,7 @@ func TestGraphQLCraft(t *testing.T) { crafter, err := materials.NewGraphQLCrafter(tc.schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/helmchart.go b/pkg/attestation/crafter/materials/helmchart.go index 1dfdc3b0f..2c1b90d9b 100644 --- a/pkg/attestation/crafter/materials/helmchart.go +++ b/pkg/attestation/crafter/materials/helmchart.go @@ -67,7 +67,7 @@ func NewHelmChartCrafter(materialSchema *schemaapi.CraftingSchema_Material, back }, nil } -func (c *HelmChartCrafter) Craft(ctx context.Context, helmChartRef string) (*api.Attestation_Material, error) { +func (c *HelmChartCrafter) Craft(ctx context.Context, helmChartRef string) (*CraftResult, error) { const ociProtocol = "oci://" // if it starts with oci://, it's an OCI image @@ -79,7 +79,7 @@ func (c *HelmChartCrafter) Craft(ctx context.Context, helmChartRef string) (*api c.logger.Debug().Str("name", helmChartRef).Msg("loading from local path") // otherwise, it's a local file - return c.craftLocalHelmChart(ctx, helmChartRef) + return craftResult(c.craftLocalHelmChart(ctx, helmChartRef)) } func (c *HelmChartCrafter) craftLocalHelmChart(ctx context.Context, filepath string) (*api.Attestation_Material, error) { diff --git a/pkg/attestation/crafter/materials/helmchart_test.go b/pkg/attestation/crafter/materials/helmchart_test.go index 1fc4bceba..de885c220 100644 --- a/pkg/attestation/crafter/materials/helmchart_test.go +++ b/pkg/attestation/crafter/materials/helmchart_test.go @@ -130,7 +130,7 @@ func TestHelmChartCraft(t *testing.T) { crafter, err := materials.NewHelmChartCrafter(schema, backend, nil, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/jacoco.go b/pkg/attestation/crafter/materials/jacoco.go index 076246ef6..09078cbb0 100644 --- a/pkg/attestation/crafter/materials/jacoco.go +++ b/pkg/attestation/crafter/materials/jacoco.go @@ -24,7 +24,6 @@ import ( "slices" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/jacoco" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/rs/zerolog" @@ -42,7 +41,7 @@ func NewJacocoCrafter(schema *schemaapi.CraftingSchema_Material, backend *cascli } } -func (c *JacocoCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (c *JacocoCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { f, err := os.Open(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -70,5 +69,5 @@ func (c *JacocoCrafter) Craft(ctx context.Context, filePath string) (*api.Attest }) { return nil, fmt.Errorf("invalid Jacoco report file: %w", ErrInvalidMaterialType) } - return uploadAndCraft(ctx, c.input, c.backend, filePath, c.logger) + return craftResult(uploadAndCraft(ctx, c.input, c.backend, filePath, c.logger)) } diff --git a/pkg/attestation/crafter/materials/jacoco_test.go b/pkg/attestation/crafter/materials/jacoco_test.go index 7688b8108..f42346bb2 100644 --- a/pkg/attestation/crafter/materials/jacoco_test.go +++ b/pkg/attestation/crafter/materials/jacoco_test.go @@ -83,7 +83,7 @@ func TestJacocoCraft(t *testing.T) { backend := &casclient.CASBackend{Uploader: uploader} crafter := materials.NewJacocoCrafter(schema, backend, &l) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/junit_xml.go b/pkg/attestation/crafter/materials/junit_xml.go index c7c72c3f4..2d83237cf 100644 --- a/pkg/attestation/crafter/materials/junit_xml.go +++ b/pkg/attestation/crafter/materials/junit_xml.go @@ -20,7 +20,6 @@ import ( "fmt" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" materialsjunit "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/junit" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/rs/zerolog" @@ -39,12 +38,12 @@ func NewJUnitXMLCrafter(schema *schemaapi.CraftingSchema_Material, backend *casc return &JUnitXMLCrafter{backend: backend, crafterCommon: craftCommon}, nil } -func (i *JUnitXMLCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *JUnitXMLCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { if err := i.validate(filePath); err != nil { return nil, err } - return uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger) + return craftResult(uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger)) } func (i *JUnitXMLCrafter) validate(filePath string) error { diff --git a/pkg/attestation/crafter/materials/junit_xml_test.go b/pkg/attestation/crafter/materials/junit_xml_test.go index e10cb840b..a5248c3ba 100644 --- a/pkg/attestation/crafter/materials/junit_xml_test.go +++ b/pkg/attestation/crafter/materials/junit_xml_test.go @@ -123,7 +123,7 @@ func TestJUnitXMLCraft(t *testing.T) { crafter, err := materials.NewJUnitXMLCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/materials.go b/pkg/attestation/crafter/materials/materials.go index 6433f8745..25266d6d5 100644 --- a/pkg/attestation/crafter/materials/materials.go +++ b/pkg/attestation/crafter/materials/materials.go @@ -349,33 +349,34 @@ func fileStatsFromBytes(filename string, content []byte) (*fileInfo, error) { } type Craftable interface { - Craft(ctx context.Context, value string) (*api.Attestation_Material, error) + Craft(ctx context.Context, value string) (*CraftResult, error) } -// transformCrafter is implemented by crafters that do not store the artifact -// verbatim, and returns the bytes they stored in its place. For such a material -// the file on disk is no longer the stored content, so anything that needs to -// read the artifact back has to be given these bytes instead. What it is then -// used for is the caller's business: today it feeds policy evaluation, which must -// not see the data the transformation removed. -// -// Opt-in on purpose, and deliberately not on crafterCommon: a content field -// shared by every crafter would let any of them report content that diverges -// from its own recorded digest. -type transformCrafter interface { - transformCraft(ctx context.Context, value string) (*api.Attestation_Material, []byte, error) -} - -// CraftResult is a crafted material together with the content policies must be -// evaluated against. +// CraftResult is what crafting an artifact yields. type CraftResult struct { Material *api.Attestation_Material - // EvaluableContent is the sanitized copy of the artifact, set only by - // crafters that transformed it before storing it — today, an AI coding - // session with secrets redacted out of it. nil means the stored bytes are the - // file on disk, so policy evaluation resolves the content the usual way and + // Transformed is what the crafter stored in place of the artifact, when it + // did not store it verbatim — today, an AI coding session with secrets + // redacted out of it. For such a material the file on disk is no longer the + // stored content, so anything that needs to read the artifact back has to be + // given these bytes instead; what they are then used for is the caller's + // business, and today they feed policy evaluation, which must not see the + // data the transformation removed. + // + // nil for every crafter that stores the artifact as it found it, which is all + // but one: their content resolves from the material or the file as usual and // nothing extra is held in memory. - EvaluableContent []byte + Transformed []byte +} + +// craftResult wraps a crafted material, propagating an error unchanged. It keeps +// the crafters that store the artifact verbatim — all but the AI coding session — +// to a single-line return. +func craftResult(m *api.Attestation_Material, err error) (*CraftResult, error) { + if err != nil { + return nil, err + } + return &CraftResult{Material: m}, nil } // CraftingOpts contains options for crafting materials @@ -498,16 +499,11 @@ func Craft(ctx context.Context, materialSchema *schemaapi.CraftingSchema_Materia return nil, err } - var m *api.Attestation_Material - var transformed []byte - if tc, ok := crafter.(transformCrafter); ok { - m, transformed, err = tc.transformCraft(ctx, value) - } else { - m, err = crafter.Craft(ctx, value) - } + res, err := crafter.Craft(ctx, value) if err != nil { return nil, fmt.Errorf("crafting material: %w", err) } + m := res.Material m.AddedAt = timestamppb.New(time.Now()) if m.Annotations == nil { @@ -523,5 +519,5 @@ func Craft(ctx context.Context, materialSchema *schemaapi.CraftingSchema_Materia m.Output = materialSchema.Output m.Required = !materialSchema.Optional - return &CraftResult{Material: m, EvaluableContent: transformed}, nil + return res, nil } diff --git a/pkg/attestation/crafter/materials/materials_test.go b/pkg/attestation/crafter/materials/materials_test.go index f5a9a586b..cb68a4e57 100644 --- a/pkg/attestation/crafter/materials/materials_test.go +++ b/pkg/attestation/crafter/materials/materials_test.go @@ -64,9 +64,9 @@ func TestCraft(t *testing.T) { res, err := materials.Craft(context.TODO(), schema, "test-value", nil, nil, nil, nil) require.NoError(t, err) - // A crafter that does not transform the artifact holds nothing back for the - // policy engine; the content is resolved from the material or the file. - assert.Nil(res.EvaluableContent) + // A crafter that stores the artifact as it found it reports nothing extra, so + // the content resolves from the material or the file as usual. + assert.Nil(res.Transformed) got := res.Material assert.Equal(contractAPI.CraftingSchema_Material_STRING, got.MaterialType) diff --git a/pkg/attestation/crafter/materials/oci_image.go b/pkg/attestation/crafter/materials/oci_image.go index 7548800df..818e10a72 100644 --- a/pkg/attestation/crafter/materials/oci_image.go +++ b/pkg/attestation/crafter/materials/oci_image.go @@ -89,12 +89,12 @@ func NewOCIImageCrafter(schema *schemaapi.CraftingSchema_Material, ociAuth authn return c, nil } -func (i *OCIImageCrafter) Craft(ctx context.Context, imageRef string) (*api.Attestation_Material, error) { +func (i *OCIImageCrafter) Craft(ctx context.Context, imageRef string) (*CraftResult, error) { // Check if imageRef is a path to an OCI layout directory layoutPath, digestSelector := parseLayoutReference(imageRef) if i.isOCILayoutPath(layoutPath) { i.logger.Debug().Str("path", layoutPath).Str("digest", digestSelector).Msg("detected OCI layout directory") - return i.craftFromLayout(ctx, layoutPath, digestSelector) + return craftResult(i.craftFromLayout(ctx, layoutPath, digestSelector)) } // Otherwise, treat as remote registry reference @@ -152,11 +152,11 @@ func (i *OCIImageCrafter) Craft(ctx context.Context, imageRef string) (*api.Atte containerImage.SignatureProvider = string(signatureInfo.provider) } - return &api.Attestation_Material{ + return craftResult(&api.Attestation_Material{ MaterialType: i.input.Type, M: &api.Attestation_Material_ContainerImage_{ ContainerImage: containerImage}, - }, nil + }, nil) } // checkForSignature checks for a signature for the given image reference. diff --git a/pkg/attestation/crafter/materials/oci_image_test.go b/pkg/attestation/crafter/materials/oci_image_test.go index bc0660eae..dda38dd69 100644 --- a/pkg/attestation/crafter/materials/oci_image_test.go +++ b/pkg/attestation/crafter/materials/oci_image_test.go @@ -114,7 +114,7 @@ func TestOCIImageCraft_Layout(t *testing.T) { crafter, err := materials.NewOCIImageCrafter(schema, nil, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.layoutPath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.layoutPath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return @@ -202,7 +202,7 @@ func TestOCIImageCraft_LayoutWithDigestSelector(t *testing.T) { crafter, err := materials.NewOCIImageCrafter(schema, nil, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), imageRef) + got, err := craftedMaterial(crafter.Craft(context.TODO(), imageRef)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return @@ -279,7 +279,7 @@ func TestOCIImageCraft_RemoteTag(t *testing.T) { crafter, err := materials.NewOCIImageCrafter(schema, authn.DefaultKeychain, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.imageRef) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.imageRef)) require.NoError(t, err) containerImage := got.GetContainerImage() @@ -325,7 +325,7 @@ func TestOCIImageCraft_LayoutTagExtraction(t *testing.T) { crafter, err := materials.NewOCIImageCrafter(schema, nil, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "testdata/oci-layouts/containerd@"+tc.digestSelector) + got, err := craftedMaterial(crafter.Craft(context.TODO(), "testdata/oci-layouts/containerd@"+tc.digestSelector)) require.NoError(t, err) containerImage := got.GetContainerImage() diff --git a/pkg/attestation/crafter/materials/openapi.go b/pkg/attestation/crafter/materials/openapi.go index 85e2d7245..df7db1fcf 100644 --- a/pkg/attestation/crafter/materials/openapi.go +++ b/pkg/attestation/crafter/materials/openapi.go @@ -60,7 +60,7 @@ func NewOpenAPICrafter(materialSchema *schemaapi.CraftingSchema_Material, backen return crafter, nil } -func (i *OpenAPICrafter) Craft(ctx context.Context, filepath string) (*api.Attestation_Material, error) { +func (i *OpenAPICrafter) Craft(ctx context.Context, filepath string) (*CraftResult, error) { i.logger.Debug().Str("path", filepath).Msg("decoding OpenAPI spec file") data, err := os.ReadFile(filepath) @@ -107,7 +107,7 @@ func (i *OpenAPICrafter) Craft(ctx context.Context, filepath string) (*api.Attes i.injectAnnotations(m, doc) - return m, nil + return craftResult(m, nil) } func (i *OpenAPICrafter) injectAnnotations(m *api.Attestation_Material, doc map[string]interface{}) { diff --git a/pkg/attestation/crafter/materials/openapi_test.go b/pkg/attestation/crafter/materials/openapi_test.go index 742c1c3fc..37b4c0296 100644 --- a/pkg/attestation/crafter/materials/openapi_test.go +++ b/pkg/attestation/crafter/materials/openapi_test.go @@ -189,7 +189,7 @@ func TestOpenAPICraft(t *testing.T) { crafter, err := materials.NewOpenAPICrafter(tc.schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return @@ -228,7 +228,7 @@ func TestOpenAPICraftNoStrictValidationSwagger2(t *testing.T) { crafter, err := materials.NewOpenAPICrafter(schema, backend, &l, materials.WithOpenAPINoStrictValidation(true)) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/swagger-2.0-invalid.json") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/swagger-2.0-invalid.json")) require.NoError(t, err) assert.NotNil(t, got) assert.Equal(t, schema.Type.String(), got.MaterialType.String()) @@ -252,7 +252,7 @@ func TestOpenAPICraftNoStrictValidation(t *testing.T) { crafter, err := materials.NewOpenAPICrafter(schema, backend, &l, materials.WithOpenAPINoStrictValidation(true)) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/openapi-invalid.json") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/openapi-invalid.json")) require.NoError(t, err) assert.NotNil(t, got) assert.Equal(t, schema.Type.String(), got.MaterialType.String()) diff --git a/pkg/attestation/crafter/materials/openvex.go b/pkg/attestation/crafter/materials/openvex.go index 25b47778a..8de94e0ea 100644 --- a/pkg/attestation/crafter/materials/openvex.go +++ b/pkg/attestation/crafter/materials/openvex.go @@ -21,7 +21,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/openvex/go-vex/pkg/vex" "github.com/rs/zerolog" @@ -43,7 +42,7 @@ func NewOpenVEXCrafter(materialSchema *schemaapi.CraftingSchema_Material, backen }, nil } -func (i *OpenVEXCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *OpenVEXCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { data, err := os.ReadFile(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -60,5 +59,5 @@ func (i *OpenVEXCrafter) Craft(ctx context.Context, filePath string) (*api.Attes return nil, fmt.Errorf("invalid OpenVEX file: %w", ErrInvalidMaterialType) } - return uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger) + return craftResult(uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger)) } diff --git a/pkg/attestation/crafter/materials/openvex_test.go b/pkg/attestation/crafter/materials/openvex_test.go index 162155501..011b3d6fc 100644 --- a/pkg/attestation/crafter/materials/openvex_test.go +++ b/pkg/attestation/crafter/materials/openvex_test.go @@ -114,7 +114,7 @@ func TestOpenVEXCraft(t *testing.T) { crafter, err := materials.NewOpenVEXCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/oversecured.go b/pkg/attestation/crafter/materials/oversecured.go index ac9cf0e5a..6b51d1836 100644 --- a/pkg/attestation/crafter/materials/oversecured.go +++ b/pkg/attestation/crafter/materials/oversecured.go @@ -81,7 +81,7 @@ func NewOversecuredCrafter(schema *schemaapi.CraftingSchema_Material, backend *c return &OversecuredCrafter{backend: backend, crafterCommon: craftCommon}, nil } -func (i *OversecuredCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *OversecuredCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { data, err := os.ReadFile(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -137,7 +137,7 @@ func (i *OversecuredCrafter) Craft(ctx context.Context, filePath string) (*api.A i.injectAnnotations(m) - return m, nil + return craftResult(m, nil) } func (i *OversecuredCrafter) injectAnnotations(m *api.Attestation_Material) { diff --git a/pkg/attestation/crafter/materials/oversecured_test.go b/pkg/attestation/crafter/materials/oversecured_test.go index 8dce9c46e..aa5bd9b4e 100644 --- a/pkg/attestation/crafter/materials/oversecured_test.go +++ b/pkg/attestation/crafter/materials/oversecured_test.go @@ -182,7 +182,7 @@ func TestOversecuredCrafter_Craft(t *testing.T) { crafter, err := materials.NewOversecuredCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/pitest.go b/pkg/attestation/crafter/materials/pitest.go index 5c54f88c3..a5f958224 100644 --- a/pkg/attestation/crafter/materials/pitest.go +++ b/pkg/attestation/crafter/materials/pitest.go @@ -23,7 +23,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/pitest" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/rs/zerolog" @@ -41,7 +40,7 @@ func NewPitestCrafter(schema *schemaapi.CraftingSchema_Material, backend *cascli } } -func (c *PitestCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (c *PitestCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { f, err := os.Open(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -68,5 +67,5 @@ func (c *PitestCrafter) Craft(ctx context.Context, filePath string) (*api.Attest return nil, fmt.Errorf("invalid PIT report file, no mutations found: %w", ErrInvalidMaterialType) } - return uploadAndCraft(ctx, c.input, c.backend, filePath, c.logger) + return craftResult(uploadAndCraft(ctx, c.input, c.backend, filePath, c.logger)) } diff --git a/pkg/attestation/crafter/materials/pitest_test.go b/pkg/attestation/crafter/materials/pitest_test.go index f5cb77fab..3bed7eb17 100644 --- a/pkg/attestation/crafter/materials/pitest_test.go +++ b/pkg/attestation/crafter/materials/pitest_test.go @@ -96,7 +96,7 @@ func TestPitestCraft(t *testing.T) { backend := &casclient.CASBackend{Uploader: uploader} crafter := materials.NewPitestCrafter(schema, backend, &l) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/radamsa.go b/pkg/attestation/crafter/materials/radamsa.go index 2fc8d6627..ba04bd2ed 100644 --- a/pkg/attestation/crafter/materials/radamsa.go +++ b/pkg/attestation/crafter/materials/radamsa.go @@ -28,7 +28,6 @@ import ( "strconv" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/radamsa" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/rs/zerolog" @@ -63,7 +62,7 @@ func NewRadamsaReportCrafter(schema *schemaapi.CraftingSchema_Material, backend }, nil } -func (c *RadamsaReportCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (c *RadamsaReportCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { // InspectReport applies the same content detection and per-entry parse the // policy projection (radamsa.ParseReportBytes) applies at eval time, so a // material accepted here is guaranteed to be evaluable. The archive itself is @@ -88,7 +87,7 @@ func (c *RadamsaReportCrafter) Craft(ctx context.Context, filePath string) (*api } m.Annotations[AnnotationToolNameKey] = radamsaToolName m.Annotations[AnnotationRadamsaReportRecordsCount] = strconv.Itoa(records) - return m, nil + return craftResult(m, nil) } // RadamsaCrashesCrafter crafts a RADAMSA_CRASHES material out of either a single @@ -109,7 +108,7 @@ func NewRadamsaCrashesCrafter(schema *schemaapi.CraftingSchema_Material, backend }, nil } -func (c *RadamsaCrashesCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (c *RadamsaCrashesCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { info, err := os.Stat(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -138,7 +137,7 @@ func (c *RadamsaCrashesCrafter) Craft(ctx context.Context, filePath string) (*ap } m.Annotations[AnnotationToolNameKey] = radamsaToolName m.Annotations[AnnotationRadamsaCrashesCount] = strconv.Itoa(count) - return m, nil + return craftResult(m, nil) } // inspectCrashesArchive reports whether path is a readable zip or tar.gz and, if diff --git a/pkg/attestation/crafter/materials/radamsa_test.go b/pkg/attestation/crafter/materials/radamsa_test.go index e27f02805..0564bf5eb 100644 --- a/pkg/attestation/crafter/materials/radamsa_test.go +++ b/pkg/attestation/crafter/materials/radamsa_test.go @@ -100,7 +100,7 @@ func TestRadamsaReportCrafter_Craft(t *testing.T) { crafter, err := materials.NewRadamsaReportCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return @@ -163,7 +163,7 @@ func TestRadamsaCrashesCrafter_Craft(t *testing.T) { crafter, err := materials.NewRadamsaCrashesCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/runnercontext.go b/pkg/attestation/crafter/materials/runnercontext.go index 9d03a4c2f..37d964569 100644 --- a/pkg/attestation/crafter/materials/runnercontext.go +++ b/pkg/attestation/crafter/materials/runnercontext.go @@ -23,7 +23,6 @@ import ( schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/internal/schemavalidators" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/rs/zerolog" ) @@ -44,7 +43,7 @@ func NewRunnerContextCrafter(materialSchema *schemaapi.CraftingSchema_Material, }, nil } -func (r *RunnerContextCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (r *RunnerContextCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { f, err := os.ReadFile(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -63,5 +62,5 @@ func (r *RunnerContextCrafter) Craft(ctx context.Context, filePath string) (*api return nil, fmt.Errorf("invalid Chainloop runner context file: %w", ErrInvalidMaterialType) } - return uploadAndCraft(ctx, r.input, r.backend, filePath, r.logger) + return craftResult(uploadAndCraft(ctx, r.input, r.backend, filePath, r.logger)) } diff --git a/pkg/attestation/crafter/materials/runnercontext_test.go b/pkg/attestation/crafter/materials/runnercontext_test.go index e90503a62..ce1b3c1c0 100644 --- a/pkg/attestation/crafter/materials/runnercontext_test.go +++ b/pkg/attestation/crafter/materials/runnercontext_test.go @@ -117,7 +117,7 @@ func TestChainloopRunnerContextCraft(t *testing.T) { crafter, err := materials.NewRunnerContextCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/sarif.go b/pkg/attestation/crafter/materials/sarif.go index 5dce15ab6..b3017258e 100644 --- a/pkg/attestation/crafter/materials/sarif.go +++ b/pkg/attestation/crafter/materials/sarif.go @@ -51,7 +51,7 @@ func NewSARIFCrafter(materialSchema *schemaapi.CraftingSchema_Material, backend }, nil } -func (i *SARIFCrafter) Craft(ctx context.Context, filepath string) (*api.Attestation_Material, error) { +func (i *SARIFCrafter) Craft(ctx context.Context, filepath string) (*CraftResult, error) { i.logger.Debug().Str("path", filepath).Msg("decoding SARIF file") // sarif.Open will take care of checkif if the file exists or not and unmarshal it, we just need to check if the schema is present to validate that it's a valid SARIF file @@ -71,7 +71,7 @@ func (i *SARIFCrafter) Craft(ctx context.Context, filepath string) (*api.Attesta i.injectAnnotations(m, doc) - return m, nil + return craftResult(m, nil) } func (i *SARIFCrafter) injectAnnotations(m *api.Attestation_Material, doc *sarif.Report) { diff --git a/pkg/attestation/crafter/materials/sarif_test.go b/pkg/attestation/crafter/materials/sarif_test.go index bf84131da..b80210d9f 100644 --- a/pkg/attestation/crafter/materials/sarif_test.go +++ b/pkg/attestation/crafter/materials/sarif_test.go @@ -112,7 +112,7 @@ func TestSARIFCraft(t *testing.T) { crafter, err := materials.NewSARIFCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return @@ -214,7 +214,7 @@ func TestSARIFCraft_ScanTypes(t *testing.T) { crafter, err := materials.NewSARIFCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) require.NoError(t, err) for k, v := range tc.annotations { @@ -272,7 +272,7 @@ func TestSARIFCraft_SkipUpload(t *testing.T) { crafter, err := materials.NewSARIFCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), filePath)) require.NoError(t, err) assert.Equal(contractAPI.CraftingSchema_Material_SARIF.String(), got.MaterialType.String()) diff --git a/pkg/attestation/crafter/materials/scorecard.go b/pkg/attestation/crafter/materials/scorecard.go index d784f4f38..cfcc00514 100644 --- a/pkg/attestation/crafter/materials/scorecard.go +++ b/pkg/attestation/crafter/materials/scorecard.go @@ -83,7 +83,7 @@ func NewOSSFScorecardCrafter(materialSchema *schemaapi.CraftingSchema_Material, return c, nil } -func (i *OSSFScorecardCrafter) Craft(ctx context.Context, filepath string) (*api.Attestation_Material, error) { +func (i *OSSFScorecardCrafter) Craft(ctx context.Context, filepath string) (*CraftResult, error) { i.logger.Debug().Str("path", filepath).Msg("decoding OpenSSF Scorecard report") data, err := os.ReadFile(filepath) @@ -130,7 +130,7 @@ func (i *OSSFScorecardCrafter) Craft(ctx context.Context, filepath string) (*api i.injectAnnotations(m, &report) - return m, nil + return craftResult(m, nil) } func (i *OSSFScorecardCrafter) injectAnnotations(m *api.Attestation_Material, report *scorecardReport) { diff --git a/pkg/attestation/crafter/materials/scorecard_test.go b/pkg/attestation/crafter/materials/scorecard_test.go index c161027d1..be73f8b8c 100644 --- a/pkg/attestation/crafter/materials/scorecard_test.go +++ b/pkg/attestation/crafter/materials/scorecard_test.go @@ -129,7 +129,7 @@ func TestOSSFScorecardCrafter_Craft(t *testing.T) { crafter, err := materials.NewOSSFScorecardCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return @@ -191,7 +191,7 @@ func TestOSSFScorecardCrafter_Craft_NoStrictValidation(t *testing.T) { crafter, err := materials.NewOSSFScorecardCrafter(schema, backend, &l, materials.WithOSSFScorecardNoStrictValidation(true)) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/sigcheck.go b/pkg/attestation/crafter/materials/sigcheck.go index 739256e17..b6b5808e6 100644 --- a/pkg/attestation/crafter/materials/sigcheck.go +++ b/pkg/attestation/crafter/materials/sigcheck.go @@ -40,7 +40,7 @@ func NewSigcheckCrafter(schema *schemaapi.CraftingSchema_Material, backend *casc return &SigcheckCrafter{backend: backend, crafterCommon: craftCommon}, nil } -func (i *SigcheckCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *SigcheckCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { data, err := os.ReadFile(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -69,7 +69,7 @@ func (i *SigcheckCrafter) Craft(ctx context.Context, filePath string) (*api.Atte i.injectAnnotations(m) - return m, nil + return craftResult(m, nil) } func (i *SigcheckCrafter) injectAnnotations(m *api.Attestation_Material) { diff --git a/pkg/attestation/crafter/materials/sigcheck_test.go b/pkg/attestation/crafter/materials/sigcheck_test.go index cfbf821fb..16d35cb79 100644 --- a/pkg/attestation/crafter/materials/sigcheck_test.go +++ b/pkg/attestation/crafter/materials/sigcheck_test.go @@ -116,7 +116,7 @@ func TestSigcheckCrafter_Craft(t *testing.T) { crafter, err := materials.NewSigcheckCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/slsaprovenance.go b/pkg/attestation/crafter/materials/slsaprovenance.go index 63b124813..56e31bf1f 100644 --- a/pkg/attestation/crafter/materials/slsaprovenance.go +++ b/pkg/attestation/crafter/materials/slsaprovenance.go @@ -21,7 +21,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" intoto "github.com/in-toto/attestation/go/v1" intotoatt "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/v1" @@ -48,7 +47,7 @@ func NewSLSAProvenanceCrafter(schema *schemaapi.CraftingSchema_Material, backend } // Craft will calculate the digest of the artifact, simulate an upload and return the material definition -func (i *SLSAProvenanceCrafter) Craft(ctx context.Context, artifactPath string) (*api.Attestation_Material, error) { +func (i *SLSAProvenanceCrafter) Craft(ctx context.Context, artifactPath string) (*CraftResult, error) { data, err := os.ReadFile(artifactPath) if err != nil { return nil, fmt.Errorf("artifact file cannot be read: %w", err) @@ -68,5 +67,5 @@ func (i *SLSAProvenanceCrafter) Craft(ctx context.Context, artifactPath string) return nil, fmt.Errorf("the provided predicate is not a valid SLSA Provenance: found=%q", p) } - return uploadAndCraft(ctx, i.input, i.backend, artifactPath, i.logger) + return craftResult(uploadAndCraft(ctx, i.input, i.backend, artifactPath, i.logger)) } diff --git a/pkg/attestation/crafter/materials/slsaprovenance_test.go b/pkg/attestation/crafter/materials/slsaprovenance_test.go index ca8db590c..820a30aff 100644 --- a/pkg/attestation/crafter/materials/slsaprovenance_test.go +++ b/pkg/attestation/crafter/materials/slsaprovenance_test.go @@ -85,7 +85,7 @@ func TestSLSAProvenanceCraft(t *testing.T) { crafter, err := materials.NewSLSAProvenanceCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/slsa_provenance.sigstore.json") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/slsa_provenance.sigstore.json")) assert.NoError(err) assert.Equal(contractAPI.CraftingSchema_Material_SLSA_PROVENANCE.String(), got.MaterialType.String()) assert.True(got.UploadedToCas) diff --git a/pkg/attestation/crafter/materials/spdxjson.go b/pkg/attestation/crafter/materials/spdxjson.go index 4533ad009..ad185a75b 100644 --- a/pkg/attestation/crafter/materials/spdxjson.go +++ b/pkg/attestation/crafter/materials/spdxjson.go @@ -47,7 +47,7 @@ func NewSPDXJSONCrafter(materialSchema *schemaapi.CraftingSchema_Material, backe }, nil } -func (i *SPDXJSONCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *SPDXJSONCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { f, err := os.Open(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -80,7 +80,7 @@ func (i *SPDXJSONCrafter) Craft(ctx context.Context, filePath string) (*api.Atte i.injectAnnotations(m, doc) - return res, nil + return craftResult(res, nil) } // extractMainComponent inspects the SPDX document and extracts the main component if any. diff --git a/pkg/attestation/crafter/materials/spdxjson_test.go b/pkg/attestation/crafter/materials/spdxjson_test.go index 35f4491b3..85c7524f2 100644 --- a/pkg/attestation/crafter/materials/spdxjson_test.go +++ b/pkg/attestation/crafter/materials/spdxjson_test.go @@ -203,7 +203,7 @@ func TestSPDXJSONCraft(t *testing.T) { crafter, err := materials.NewSPDXJSONCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/string.go b/pkg/attestation/crafter/materials/string.go index 17eefd11e..dfb72d4f0 100644 --- a/pkg/attestation/crafter/materials/string.go +++ b/pkg/attestation/crafter/materials/string.go @@ -40,12 +40,12 @@ func NewStringCrafter(materialSchema *schemaapi.CraftingSchema_Material) (*Strin }, nil } -func (i *StringCrafter) Craft(_ context.Context, value string) (*api.Attestation_Material, error) { +func (i *StringCrafter) Craft(_ context.Context, value string) (*CraftResult, error) { hash, _, err := cr_v1.SHA256(strings.NewReader(value)) if err != nil { return nil, fmt.Errorf("generating digest: %w", err) } - return &api.Attestation_Material{ + return craftResult(&api.Attestation_Material{ MaterialType: i.input.Type, M: &api.Attestation_Material_String_{ String_: &api.Attestation_Material_KeyVal{ @@ -57,5 +57,5 @@ func (i *StringCrafter) Craft(_ context.Context, value string) (*api.Attestation Annotations: map[string]string{ AnnotationMaterialSize: strconv.Itoa(len(value)), }, - }, nil + }, nil) } diff --git a/pkg/attestation/crafter/materials/string_test.go b/pkg/attestation/crafter/materials/string_test.go index 07d696e90..024db8ce5 100644 --- a/pkg/attestation/crafter/materials/string_test.go +++ b/pkg/attestation/crafter/materials/string_test.go @@ -72,7 +72,7 @@ func TestStringCraft(t *testing.T) { crafter, err := materials.NewStringCrafter(schema) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "value") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "value")) assert.NoError(err) assert.Equal(contractAPI.CraftingSchema_Material_STRING, got.MaterialType) assert.False(got.UploadedToCas) diff --git a/pkg/attestation/crafter/materials/trufflehog.go b/pkg/attestation/crafter/materials/trufflehog.go index 0fa647a71..61f23e4a0 100644 --- a/pkg/attestation/crafter/materials/trufflehog.go +++ b/pkg/attestation/crafter/materials/trufflehog.go @@ -21,7 +21,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials/trufflehog" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/rs/zerolog" @@ -46,7 +45,7 @@ func NewTrufflehogCrafter(schema *schemaapi.CraftingSchema_Material, backend *ca }, nil } -func (i *TrufflehogCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *TrufflehogCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { f, err := os.Open(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -87,5 +86,5 @@ func (i *TrufflehogCrafter) Craft(ctx context.Context, filePath string) (*api.At } m.Annotations[AnnotationToolNameKey] = trufflehogToolName - return m, nil + return craftResult(m, nil) } diff --git a/pkg/attestation/crafter/materials/trufflehog_test.go b/pkg/attestation/crafter/materials/trufflehog_test.go index a628b6be1..909e76089 100644 --- a/pkg/attestation/crafter/materials/trufflehog_test.go +++ b/pkg/attestation/crafter/materials/trufflehog_test.go @@ -124,7 +124,7 @@ func TestTrufflehogCrafter_Craft(t *testing.T) { crafter, err := materials.NewTrufflehogCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(t, err, tc.wantErr) return @@ -163,7 +163,7 @@ func TestTrufflehogCrafter_CleanScanEmptyFile(t *testing.T) { crafter, err := materials.NewTrufflehogCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), "./testdata/trufflehog-clean-scan.jsonl") + got, err := craftedMaterial(crafter.Craft(context.TODO(), "./testdata/trufflehog-clean-scan.jsonl")) require.NoError(t, err, "a clean scan (zero-byte output) must be accepted") assert.True(t, got.UploadedToCas) assert.Equal(t, emptyReportDigest, got.GetArtifact().Digest, "clean scan must be stored as canonical []") diff --git a/pkg/attestation/crafter/materials/twistcli_scan.go b/pkg/attestation/crafter/materials/twistcli_scan.go index f6cf95d6e..b63c827f0 100644 --- a/pkg/attestation/crafter/materials/twistcli_scan.go +++ b/pkg/attestation/crafter/materials/twistcli_scan.go @@ -22,7 +22,6 @@ import ( "os" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/rs/zerolog" @@ -49,7 +48,7 @@ func NewTwistCLIScanCrafter(materialSchema *schemaapi.CraftingSchema_Material, b }, nil } -func (i *TwistCLIScanCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *TwistCLIScanCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { f, err := os.ReadFile(filePath) if err != nil { return nil, fmt.Errorf("can't open the file: %w", err) @@ -66,5 +65,5 @@ func (i *TwistCLIScanCrafter) Craft(ctx context.Context, filePath string) (*api. return nil, fmt.Errorf("invalid twistcli scan file: %w", ErrInvalidMaterialType) } - return uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger) + return craftResult(uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger)) } diff --git a/pkg/attestation/crafter/materials/twistcli_scan_test.go b/pkg/attestation/crafter/materials/twistcli_scan_test.go index ee272cfda..3146a6ff1 100644 --- a/pkg/attestation/crafter/materials/twistcli_scan_test.go +++ b/pkg/attestation/crafter/materials/twistcli_scan_test.go @@ -112,7 +112,7 @@ func TestTwistCLIScanCraft(t *testing.T) { crafter, err := materials.NewTwistCLIScanCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return diff --git a/pkg/attestation/crafter/materials/zap.go b/pkg/attestation/crafter/materials/zap.go index 8c9aab9ab..c4c3a1683 100644 --- a/pkg/attestation/crafter/materials/zap.go +++ b/pkg/attestation/crafter/materials/zap.go @@ -23,7 +23,6 @@ import ( "io" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" - api "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/casclient" "github.com/rs/zerolog" @@ -58,7 +57,7 @@ func NewZAPCrafter(materialSchema *schemaapi.CraftingSchema_Material, backend *c } // Craft will extract the ZAP JSON report from the zip file and upload it to the CAS -func (i *ZAPCrafter) Craft(ctx context.Context, filePath string) (*api.Attestation_Material, error) { +func (i *ZAPCrafter) Craft(ctx context.Context, filePath string) (*CraftResult, error) { archive, err := zip.OpenReader(filePath) if err != nil { return nil, fmt.Errorf("can't open the zip file: %w", err) @@ -105,5 +104,5 @@ func (i *ZAPCrafter) Craft(ctx context.Context, filePath string) (*api.Attestati } // Call uploadAndCraft with the path of the JSON report file - return uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger) + return craftResult(uploadAndCraft(ctx, i.input, i.backend, filePath, i.logger)) } diff --git a/pkg/attestation/crafter/materials/zap_test.go b/pkg/attestation/crafter/materials/zap_test.go index eb89c289e..ee5bc1fa9 100644 --- a/pkg/attestation/crafter/materials/zap_test.go +++ b/pkg/attestation/crafter/materials/zap_test.go @@ -117,7 +117,7 @@ func TestNewZAPCraft(t *testing.T) { crafter, err := materials.NewZAPCrafter(schema, backend, &l) require.NoError(t, err) - got, err := crafter.Craft(context.TODO(), tc.filePath) + got, err := craftedMaterial(crafter.Craft(context.TODO(), tc.filePath)) if tc.wantErr != "" { assert.ErrorContains(err, tc.wantErr) return From 9e2936e1a6faf964e671f09874de3e3c74ede490 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Fri, 28 Aug 2026 19:51:32 +0200 Subject: [PATCH 3/5] refactor(attestation): fold GetEvaluableContentFrom into GetEvaluableContent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The single-argument GetEvaluableContent had no callers left outside tests once both policy verifiers moved to the variant taking an explicit content source, so keeping it as a delegating wrapper only offered a second way in — one that resolves a redacted material's content from the file on disk and fails. Give GetEvaluableContent the content parameter and drop the wrapper. Callers that have nothing to supply pass nil, which is the behaviour the old signature had. Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: 06ec9990-4ad5-4b3c-ad86-2421b24dba57 --- .../api/attestation/v1/crafting_state.go | 19 ++++++--------- .../api/attestation/v1/crafting_state_test.go | 24 +++++++++---------- .../engine/rego/radamsa_report_test.go | 2 +- pkg/policies/policies.go | 2 +- pkg/policies/policy_groups.go | 2 +- 5 files changed, 22 insertions(+), 27 deletions(-) diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state.go index 73e01edc2..051a7f10a 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state.go @@ -64,7 +64,7 @@ var ( // 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 GetEvaluableContentFrom, which fails closed without it). + // (see GetEvaluableContent, which fails closed without it). AnnotationMaterialRedacted = CreateAnnotation("material.redacted") // AnnotationMaterialRedactionCount is how many secrets were replaced. AnnotationMaterialRedactionCount = CreateAnnotation("material.redaction.count") @@ -120,25 +120,20 @@ var ErrRedactedContentRequired = errors.New( "sanitized content is required to evaluate a redacted material: " + "policies must never be handed the un-redacted original") -// GetEvaluableContent returns the content to be sent to policy evaluations, -// resolved from the material's stored copy or the file on disk. -func (m *Attestation_Material) GetEvaluableContent(value string) ([]byte, error) { - return m.GetEvaluableContentFrom(value, nil) -} - -// GetEvaluableContentFrom is GetEvaluableContent with an explicit content source. +// GetEvaluableContent returns the content to be sent to policy evaluations. // // content, when non-empty, is what policies are evaluated against, overriding -// both the inline bytes and the file on disk. Crafters that transform an artifact +// both the inline bytes and the file at value. Crafters that transform an artifact // before it leaves the machine — redacting secrets out of an AI coding session — -// hand back the bytes they stored so that the policy engine sees exactly those, -// whatever CAS backend is in use. +// report the bytes they stored so that the policy engine sees exactly those, +// whatever CAS backend is in use. Pass nil to resolve the content from the +// material or the file, which is what every other material does. // // A material marked redacted with no content supplied fails closed. One // consequence is worth knowing: such a material's policy input cannot be // reconstructed from persisted crafting state alone, so any future push-time or // server-side material evaluation has to plumb the bytes through as well. -func (m *Attestation_Material) GetEvaluableContentFrom(value string, content []byte) ([]byte, error) { +func (m *Attestation_Material) GetEvaluableContent(value string, content []byte) ([]byte, error) { var rawMaterial []byte var err error diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state_test.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state_test.go index 7d5edcbac..8d22c650b 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state_test.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state_test.go @@ -319,7 +319,7 @@ func TestGetEvaluableContentWithMetadata(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - content, err := tc.material.GetEvaluableContent(tc.filename) + content, err := tc.material.GetEvaluableContent(tc.filename, nil) assert.NoError(t, err) decoder := json.NewDecoder(bytes.NewReader(content)) @@ -349,7 +349,7 @@ func TestDranzerBundleIsEvaluable(t *testing.T) { }, } - content, err := m.GetEvaluableContent("testdata/dranzer-bundle.zip") + content, err := m.GetEvaluableContent("testdata/dranzer-bundle.zip", nil) require.NoError(t, err) var decoded map[string]any @@ -402,7 +402,7 @@ func TestRadamsaReportArchiveIsEvaluable(t *testing.T) { }, } - content, err := m.GetEvaluableContent(tc.path) + content, err := m.GetEvaluableContent(tc.path, nil) require.NoError(t, err) var decoded map[string]any @@ -460,7 +460,7 @@ func TestCoberturaEmptyReportIsEvaluable(t *testing.T) { }, } - content, err := m.GetEvaluableContent("testdata/cobertura-empty.xml") + content, err := m.GetEvaluableContent("testdata/cobertura-empty.xml", nil) require.NoError(t, err, "empty report must be evaluable, not error on NaN") var decoded map[string]any @@ -481,7 +481,7 @@ func TestTruffleHogCleanScanIsEvaluable(t *testing.T) { }, } - content, err := m.GetEvaluableContent("testdata/trufflehog-clean-scan.jsonl") + content, err := m.GetEvaluableContent("testdata/trufflehog-clean-scan.jsonl", nil) require.NoError(t, err, "clean scan must be evaluable") var decoded map[string]any @@ -613,7 +613,7 @@ func TestGetEvaluableContentRedactedNeverReadsDisk(t *testing.T) { }, } - content, err := m.GetEvaluableContentFrom(tc.path, tc.content) + content, err := m.GetEvaluableContent(tc.path, tc.content) if tc.wantErr != nil { require.ErrorIs(t, err, tc.wantErr) return @@ -634,11 +634,11 @@ func TestGetEvaluableContentRedactedNeverReadsDisk(t *testing.T) { } } -// TestGetEvaluableContentFromInjectsMetadata pins that supplying the content does +// TestGetEvaluableContentInjectsMetadata pins that supplying the content does // not bypass the projection the policy engine relies on: the chainloop_metadata // descriptor is still injected, so a policy can read the redaction annotations // alongside the sanitized body. -func TestGetEvaluableContentFromInjectsMetadata(t *testing.T) { +func TestGetEvaluableContentInjectsMetadata(t *testing.T) { m := &Attestation_Material{ MaterialType: schemaapi.CraftingSchema_Material_CHAINLOOP_AI_CODING_SESSION, Annotations: map[string]string{ @@ -651,7 +651,7 @@ func TestGetEvaluableContentFromInjectsMetadata(t *testing.T) { }, } - content, err := m.GetEvaluableContentFrom("", []byte(`{"secret":"[REDACTED:jwt]"}`)) + content, err := m.GetEvaluableContent("", []byte(`{"secret":"[REDACTED:jwt]"}`)) require.NoError(t, err) var decoded struct { @@ -685,7 +685,7 @@ func TestTruffleHogCleanScanIsEvaluableInline(t *testing.T) { }, } - content, err := m.GetEvaluableContent("testdata/trufflehog-clean-scan.jsonl") + content, err := m.GetEvaluableContent("testdata/trufflehog-clean-scan.jsonl", nil) require.NoError(t, err) var decoded map[string]any @@ -711,7 +711,7 @@ func pitestMaterial() *Attestation_Material { func pitestMutations(t *testing.T, path string) (map[string]any, []any) { t.Helper() - content, err := pitestMaterial().GetEvaluableContent(path) + content, err := pitestMaterial().GetEvaluableContent(path, nil) require.NoError(t, err) var decoded map[string]any @@ -803,6 +803,6 @@ func TestPitestFullMutationMatrixIsEvaluable(t *testing.T) { // TestPitestInvalidReportIsNotEvaluable guards that a non-PIT XML report // fails the projection instead of producing an empty policy input. func TestPitestInvalidReportIsNotEvaluable(t *testing.T) { - _, err := pitestMaterial().GetEvaluableContent("testdata/cobertura.xml") + _, err := pitestMaterial().GetEvaluableContent("testdata/cobertura.xml", nil) require.ErrorContains(t, err, "invalid PIT report file") } diff --git a/pkg/policies/engine/rego/radamsa_report_test.go b/pkg/policies/engine/rego/radamsa_report_test.go index 08018b0ec..b8e6d9d61 100644 --- a/pkg/policies/engine/rego/radamsa_report_test.go +++ b/pkg/policies/engine/rego/radamsa_report_test.go @@ -121,7 +121,7 @@ func TestRadamsaMinIterationsAgainstArchiveMaterial(t *testing.T) { Artifact: &v1.Attestation_Material_Artifact{Name: "fuzz-report", Digest: "sha256:deadbeef"}, }, } - input, err := m.GetEvaluableContent(tc.path) + input, err := m.GetEvaluableContent(tc.path, nil) if tc.wantIngestErr { require.Error(t, err, "malformed evidence must be rejected, not silently dropped") return diff --git a/pkg/policies/policies.go b/pkg/policies/policies.go index d786dbce7..6ac6fa216 100644 --- a/pkg/policies/policies.go +++ b/pkg/policies/policies.go @@ -301,7 +301,7 @@ func (pv *PolicyVerifier) VerifyMaterial(ctx context.Context, material *v12.Atte } // Load material content - subject, err := material.GetEvaluableContentFrom(artifactPath, o.content) + subject, err := material.GetEvaluableContent(artifactPath, o.content) if err != nil { return nil, NewPolicyError(err) } diff --git a/pkg/policies/policy_groups.go b/pkg/policies/policy_groups.go index 159c3772e..f3779eae9 100644 --- a/pkg/policies/policy_groups.go +++ b/pkg/policies/policy_groups.go @@ -86,7 +86,7 @@ func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *ap // Load material content once for all policies in this group. Kept below // the skip above so that a material with no applicable policies never // resolves its content at all. - subject, err := material.GetEvaluableContentFrom(path, o.content) + subject, err := material.GetEvaluableContent(path, o.content) if err != nil { return nil, NewPolicyError(err) } From 22e549c1ba4f3d5185abe697a819003e6de0ceef Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Fri, 28 Aug 2026 19:59:27 +0200 Subject: [PATCH 4/5] refactor(materials): rename CraftResult.Transformed to Content The field holds the content the crafter stored, which is what every consumer wants from it. Naming it after the transformation described how it came to exist rather than what it is. Locals that carried the field's value are renamed to match; the prose that describes crafters transforming an artifact is unchanged. Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: 06ec9990-4ad5-4b3c-ad86-2421b24dba57 --- app/cli/internal/policydevel/eval.go | 6 +++--- pkg/attestation/crafter/crafter.go | 2 +- .../materials/chainloop_ai_coding_session.go | 4 ++-- ...chainloop_ai_coding_session_redaction_test.go | 16 ++++++++-------- .../materials/craft_result_external_test.go | 2 +- .../crafter/materials/craft_result_test.go | 2 +- pkg/attestation/crafter/materials/materials.go | 16 ++++++++-------- .../crafter/materials/materials_test.go | 2 +- 8 files changed, 25 insertions(+), 25 deletions(-) diff --git a/app/cli/internal/policydevel/eval.go b/app/cli/internal/policydevel/eval.go index bb4f0185c..03614cdde 100644 --- a/app/cli/internal/policydevel/eval.go +++ b/app/cli/internal/policydevel/eval.go @@ -88,7 +88,7 @@ func Evaluate(opts *EvalOptions, logger zerolog.Logger) (*EvalSummary, error) { // must be evaluated against — `policy devel eval` has to reproduce what // `attestation add` does, or a policy would be developed against input the // real run never sees. - summary, err := verifyMaterial(policies, material, opts.MaterialPath, crafted.Transformed, opts.Debug, opts.AllowedHostnames, opts.AttestationClient, opts.ControlPlaneConn, opts.ProjectName, opts.ProjectVersionName, &logger) + summary, err := verifyMaterial(policies, material, opts.MaterialPath, crafted.Content, opts.Debug, opts.AllowedHostnames, opts.AttestationClient, opts.ControlPlaneConn, opts.ProjectName, opts.ProjectVersionName, &logger) if err != nil { return nil, err } @@ -150,7 +150,7 @@ func createPolicies(policyPath string, inputs map[string]string) (*v1.Policies, }, nil } -func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materialPath string, transformed []byte, debug bool, allowedHostnames []string, attestationClient controlplanev1.AttestationServiceClient, grpcConn *grpc.ClientConn, projectName, projectVersion string, logger *zerolog.Logger) (*EvalSummary, error) { +func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materialPath string, content []byte, debug bool, allowedHostnames []string, attestationClient controlplanev1.AttestationServiceClient, grpcConn *grpc.ClientConn, projectName, projectVersion string, logger *zerolog.Logger) (*EvalSummary, error) { var opts []policies.PolicyVerifierOption if len(allowedHostnames) > 0 { opts = append(opts, policies.WithAllowedHostnames(allowedHostnames...)) @@ -165,7 +165,7 @@ func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materi v := policies.NewPolicyVerifier(pol, attestationClient, logger, opts...) policyEvs, err := v.VerifyMaterial(context.Background(), material, materialPath, - policies.WithMaterialContent(transformed)) + policies.WithMaterialContent(content)) if err != nil { return nil, err } diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index c9b7499bd..d4fa8ec16 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -775,7 +775,7 @@ func (c *Crafter) stageMaterial(ctx context.Context, m *schemaapi.CraftingSchema // the policies below must see. Reading the file on disk instead would feed // user-authored Rego the very secrets redaction removed. nil for every other // material, which resolves its content the usual way. - withStoredContent := policies.WithMaterialContent(crafted.Transformed) + withStoredContent := policies.WithMaterialContent(crafted.Content) // 4 - Populate annotations from the ones provided at runtime // a) we do not allow overriding values that come from the contract diff --git a/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go b/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go index b208b6b76..86eff53ec 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_coding_session.go @@ -70,7 +70,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.Transformed +// anything was redacted. The sanitized 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) { @@ -135,7 +135,7 @@ func (c *ChainloopAICodingSessionCrafter) Craft(ctx context.Context, artifactPat material.Annotations[annotationAICodingModel] = data.Model.Primary } - return &CraftResult{Material: material, Transformed: redacted}, nil + return &CraftResult{Material: material, Content: redacted}, nil } // redact strips secrets out of the session content, returning the sanitized copy diff --git a/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go b/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go index db6f41525..1e5191648 100644 --- a/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go +++ b/pkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go @@ -271,7 +271,7 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { res, err := crafter.Craft(context.TODO(), path) require.NoError(t, err) - got, transformed := res.Material, res.Transformed + got, content := res.Material, res.Content if tc.inlineBackend { require.True(t, got.InlineCas) @@ -291,13 +291,13 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { // 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, transformed, "a redacted session must hand back its sanitized copy") - assert.Equal(t, sha256Digest(string(transformed)), got.GetArtifact().Digest) - assert.NotContains(t, string(transformed), awsKey) - assert.Contains(t, string(transformed), "[REDACTED:aws-access-token]") + 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), "[REDACTED:aws-access-token]") if stored != nil { - assert.Equal(t, string(stored), string(transformed)) + assert.Equal(t, string(stored), string(content)) assert.NotContains(t, string(stored), awsKey) assert.Contains(t, string(stored), "[REDACTED:aws-access-token]") } @@ -307,13 +307,13 @@ func TestChainloopAICodingSessionCrafterRedaction(t *testing.T) { 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, transformed) + assert.Nil(t, content) default: assert.NotContains(t, 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, transformed) + assert.Nil(t, content) } // Redaction must never touch the source file. diff --git a/pkg/attestation/crafter/materials/craft_result_external_test.go b/pkg/attestation/crafter/materials/craft_result_external_test.go index cfb6377fe..5545d2e4b 100644 --- a/pkg/attestation/crafter/materials/craft_result_external_test.go +++ b/pkg/attestation/crafter/materials/craft_result_external_test.go @@ -27,7 +27,7 @@ import ( // got, err := craftedMaterial(crafter.Craft(ctx, path)) // // Tests that care about what the crafter stored in place of the artifact use -// CraftResult.Transformed directly instead. +// CraftResult.Content directly instead. func craftedMaterial(res *materials.CraftResult, err error) (*attestationApi.Attestation_Material, error) { if res == nil { return nil, err diff --git a/pkg/attestation/crafter/materials/craft_result_test.go b/pkg/attestation/crafter/materials/craft_result_test.go index 436ad68b6..480a61491 100644 --- a/pkg/attestation/crafter/materials/craft_result_test.go +++ b/pkg/attestation/crafter/materials/craft_result_test.go @@ -26,7 +26,7 @@ import ( // got, err := craftedMaterial(crafter.Craft(ctx, path)) // // Tests that care about what the crafter stored in place of the artifact use -// CraftResult.Transformed directly instead. +// CraftResult.Content directly instead. func craftedMaterial(res *CraftResult, err error) (*api.Attestation_Material, error) { if res == nil { return nil, err diff --git a/pkg/attestation/crafter/materials/materials.go b/pkg/attestation/crafter/materials/materials.go index 25266d6d5..d6039ebf1 100644 --- a/pkg/attestation/crafter/materials/materials.go +++ b/pkg/attestation/crafter/materials/materials.go @@ -355,18 +355,18 @@ type Craftable interface { // CraftResult is what crafting an artifact yields. type CraftResult struct { Material *api.Attestation_Material - // Transformed is what the crafter stored in place of the artifact, when it - // did not store it verbatim — today, an AI coding session with secrets - // redacted out of it. For such a material the file on disk is no longer the - // stored content, so anything that needs to read the artifact back has to be - // given these bytes instead; what they are then used for is the caller's - // business, and today they feed policy evaluation, which must not see the - // data the transformation removed. + // Content is what the crafter stored in place of the artifact, when it did + // not store it verbatim — today, an AI coding session with secrets redacted + // out of it. For such a material the file on disk is no longer the stored + // content, so anything that needs to read the artifact back has to be given + // these bytes instead; what they are then used for is the caller's business, + // and today they feed policy evaluation, which must not see the data the + // transformation removed. // // nil for every crafter that stores the artifact as it found it, which is all // but one: their content resolves from the material or the file as usual and // nothing extra is held in memory. - Transformed []byte + Content []byte } // craftResult wraps a crafted material, propagating an error unchanged. It keeps diff --git a/pkg/attestation/crafter/materials/materials_test.go b/pkg/attestation/crafter/materials/materials_test.go index cb68a4e57..6b1c83323 100644 --- a/pkg/attestation/crafter/materials/materials_test.go +++ b/pkg/attestation/crafter/materials/materials_test.go @@ -66,7 +66,7 @@ func TestCraft(t *testing.T) { require.NoError(t, err) // A crafter that stores the artifact as it found it reports nothing extra, so // the content resolves from the material or the file as usual. - assert.Nil(res.Transformed) + assert.Nil(res.Content) got := res.Material assert.Equal(contractAPI.CraftingSchema_Material_STRING, got.MaterialType) From 7458830ea96aca5d21037913df27825b16027930 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Fri, 28 Aug 2026 20:05:06 +0200 Subject: [PATCH 5/5] refactor(policies): take the material content as a parameter, not an option The functional-option machinery existed to carry a single []byte, which is more ceremony than the value deserves: a type, a constructor, an accumulator and an option struct so that one call site could pass one slice. VerifyMaterial takes the content as a plain parameter instead. Callers that have nothing to supply pass nil, which reads no worse than omitting an option and makes the alternative visible in the signature. Assisted-by: Claude Code Signed-off-by: Jose I. Paris Chainloop-Trace-Sessions: 06ec9990-4ad5-4b3c-ad86-2421b24dba57 --- app/cli/internal/policydevel/eval.go | 3 +-- pkg/attestation/crafter/crafter.go | 16 +++++------- pkg/policies/concurrency_test.go | 2 +- pkg/policies/policies.go | 38 +++++++--------------------- pkg/policies/policies_test.go | 19 +++++++------- pkg/policies/policy_groups.go | 5 ++-- pkg/policies/policy_groups_test.go | 13 +++++----- 7 files changed, 35 insertions(+), 61 deletions(-) diff --git a/app/cli/internal/policydevel/eval.go b/app/cli/internal/policydevel/eval.go index 03614cdde..d18b5d7c1 100644 --- a/app/cli/internal/policydevel/eval.go +++ b/app/cli/internal/policydevel/eval.go @@ -164,8 +164,7 @@ func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materi } v := policies.NewPolicyVerifier(pol, attestationClient, logger, opts...) - policyEvs, err := v.VerifyMaterial(context.Background(), material, materialPath, - policies.WithMaterialContent(content)) + policyEvs, err := v.VerifyMaterial(context.Background(), material, materialPath, content) if err != nil { return nil, err } diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index d4fa8ec16..893ddf899 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -770,13 +770,6 @@ func (c *Crafter) stageMaterial(ctx context.Context, m *schemaapi.CraftingSchema } mt := crafted.Material - // Crafters that transformed the artifact before storing it (redacting secrets - // out of an AI coding session) hand back what they stored, and that is what - // the policies below must see. Reading the file on disk instead would feed - // user-authored Rego the very secrets redaction removed. nil for every other - // material, which resolves its content the usual way. - withStoredContent := policies.WithMaterialContent(crafted.Content) - // 4 - Populate annotations from the ones provided at runtime // a) we do not allow overriding values that come from the contract // b) we allow adding annotations that are not defined in the contract @@ -828,7 +821,12 @@ func (c *Crafter) stageMaterial(ctx context.Context, m *schemaapi.CraftingSchema policies.WithDefaultGate(c.CraftingState.Attestation.GetBlockOnPolicyViolation()), policies.WithProjectContext(projectName, projectVersion), ) - policyGroupResults, err := pgv.VerifyMaterial(ctx, mt, value, withStoredContent) + // 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 + // 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) if err != nil { return nil, fmt.Errorf("error applying policy groups to material: %w", err) } @@ -849,7 +847,7 @@ func (c *Crafter) stageMaterial(ctx context.Context, m *schemaapi.CraftingSchema policies.WithProjectContext(projectName, projectVersion), policies.WithRuntimeInputs(addOptions.runtimeInputs), ) - policyResults, err := pv.VerifyMaterial(ctx, mt, value, withStoredContent) + policyResults, err := pv.VerifyMaterial(ctx, mt, value, crafted.Content) if err != nil { return nil, fmt.Errorf("error applying policies to material: %w", err) } diff --git a/pkg/policies/concurrency_test.go b/pkg/policies/concurrency_test.go index 0269d0f80..15266bc00 100644 --- a/pkg/policies/concurrency_test.go +++ b/pkg/policies/concurrency_test.go @@ -112,7 +112,7 @@ func TestConcurrentVerifyMaterial(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - res, err := pv.VerifyMaterial(context.Background(), material, "testdata/sbom-spdx.json") + res, err := pv.VerifyMaterial(context.Background(), material, "testdata/sbom-spdx.json", nil) errs[i] = err results[i] = res }() diff --git a/pkg/policies/policies.go b/pkg/policies/policies.go index 6ac6fa216..403a973a5 100644 --- a/pkg/policies/policies.go +++ b/pkg/policies/policies.go @@ -67,35 +67,16 @@ func (e *PolicyError) Unwrap() error { } type Verifier interface { - VerifyMaterial(ctx context.Context, m *v12.Attestation_Material, path string, opts ...VerifyMaterialOption) ([]*v12.PolicyEvaluation, error) + // VerifyMaterial evaluates a material. content, when non-nil, is the bytes to + // evaluate instead of resolving them from the material's stored copy or the + // file at path: a crafter that did not store the artifact verbatim reports + // what it stored, and that is what the engine must see rather than the + // untouched original. A redacted material does not resolve at all without it. + // Pass nil to resolve the content the usual way. + VerifyMaterial(ctx context.Context, m *v12.Attestation_Material, path string, content []byte) ([]*v12.PolicyEvaluation, error) VerifyStatement(ctx context.Context, statement *intoto.Statement) ([]*v12.PolicyEvaluation, error) } -// verifyMaterialOpts tunes a single material verification. -type verifyMaterialOpts struct { - content []byte -} - -// VerifyMaterialOption tunes a single call to VerifyMaterial. -type VerifyMaterialOption func(*verifyMaterialOpts) - -// WithMaterialContent supplies the bytes to evaluate instead of resolving them -// from the material's stored copy or the file on disk. Crafters that transform an -// artifact before storing it must pass it, so that the policy engine sees the -// content that was stored rather than the untouched original: a redacted material -// fails to resolve at all without it. nil is a no-op. -func WithMaterialContent(content []byte) VerifyMaterialOption { - return func(o *verifyMaterialOpts) { o.content = content } -} - -func newVerifyMaterialOpts(opts ...VerifyMaterialOption) *verifyMaterialOpts { - o := &verifyMaterialOpts{} - for _, opt := range opts { - opt(o) - } - return o -} - // EvalPhase represents the phase of the attestation lifecycle where evaluation is happening. type EvalPhase int @@ -287,9 +268,8 @@ func NewPolicyVerifier(policies *v1.Policies, client v13.AttestationServiceClien } // VerifyMaterial applies all required policies to a material -func (pv *PolicyVerifier) VerifyMaterial(ctx context.Context, material *v12.Attestation_Material, artifactPath string, opts ...VerifyMaterialOption) ([]*v12.PolicyEvaluation, error) { +func (pv *PolicyVerifier) VerifyMaterial(ctx context.Context, material *v12.Attestation_Material, artifactPath string, content []byte) ([]*v12.PolicyEvaluation, error) { result := make([]*v12.PolicyEvaluation, 0) - o := newVerifyMaterialOpts(opts...) attachments, err := pv.requiredPoliciesForMaterial(ctx, material) if err != nil { @@ -301,7 +281,7 @@ func (pv *PolicyVerifier) VerifyMaterial(ctx context.Context, material *v12.Atte } // Load material content - subject, err := material.GetEvaluableContent(artifactPath, o.content) + subject, err := material.GetEvaluableContent(artifactPath, content) if err != nil { return nil, NewPolicyError(err) } diff --git a/pkg/policies/policies_test.go b/pkg/policies/policies_test.go index ddba5014b..a9e825a2b 100644 --- a/pkg/policies/policies_test.go +++ b/pkg/policies/policies_test.go @@ -658,7 +658,7 @@ func (s *testSuite) TestValidInlineMaterial() { verifier := NewPolicyVerifier(schema.Policies, nil, &s.logger) - res, err := verifier.VerifyMaterial(context.TODO(), material, "") + res, err := verifier.VerifyMaterial(context.TODO(), material, "", nil) s.Require().NoError(err) s.Len(res, 1) s.Equal("made-with-syft", res[0].Name) @@ -693,7 +693,7 @@ func (s *testSuite) TestInvalidInlineMaterial() { verifier := NewPolicyVerifier(schema.Policies, nil, &s.logger) - res, err := verifier.VerifyMaterial(context.TODO(), material, "") + res, err := verifier.VerifyMaterial(context.TODO(), material, "", nil) s.Require().NoError(err) s.Len(res, 1) s.Equal("made-with-syft", res[0].Name) @@ -702,7 +702,7 @@ func (s *testSuite) TestInvalidInlineMaterial() { s.Equal("Not made with syft", res[0].Violations[0].Message) } -// TestVerifyMaterialSuppliedContent covers WithMaterialContent, the channel a +// TestVerifyMaterialSuppliedContent covers the content parameter, the channel a // crafter uses to say which bytes it stored. Supplied content must win over every // other source, and a material whose stored copy was sanitized must refuse to be // evaluated without it rather than quietly reading the original from disk. @@ -791,8 +791,7 @@ func (s *testSuite) TestVerifyMaterialSuppliedContent() { verifier := NewPolicyVerifier(pol, nil, &s.logger) - res, err := verifier.VerifyMaterial(context.TODO(), material, tc.path, - WithMaterialContent(tc.content)) + res, err := verifier.VerifyMaterial(context.TODO(), material, tc.path, tc.content) if tc.wantErr != nil { s.Require().ErrorIs(err, tc.wantErr) // Wrapped so callers keep treating it as a policy failure. @@ -835,7 +834,7 @@ func (s *testSuite) TestVerifyMaterialScopedRuntimeInputs() { verifier := NewPolicyVerifier(schema.Policies, nil, &s.logger, WithRuntimeInputs(runtimeInputs)) - res, err := verifier.VerifyMaterial(context.TODO(), material, "") + res, err := verifier.VerifyMaterial(context.TODO(), material, "", nil) s.Require().NoError(err) s.Require().Len(res, 2) @@ -1361,7 +1360,7 @@ func (s *testSuite) TestNewResultFormat() { } verifier := NewPolicyVerifier(schema.Policies, nil, &s.logger) - res, err := verifier.VerifyMaterial(context.TODO(), material, "") + res, err := verifier.VerifyMaterial(context.TODO(), material, "", nil) if tc.expectErr { s.Error(err) @@ -1432,7 +1431,7 @@ func (s *testSuite) TestContainerMaterial() { } verifier := NewPolicyVerifier(schema.Policies, nil, &s.logger) - res, err := verifier.VerifyMaterial(context.TODO(), material, "") + res, err := verifier.VerifyMaterial(context.TODO(), material, "", nil) if tc.expectErr { s.Error(err) @@ -1519,7 +1518,7 @@ func (s *testSuite) TestMultiKindAWithIgnore() { } verifier := NewPolicyVerifier(schema.Policies, nil, &s.logger) - res, err := verifier.VerifyMaterial(context.TODO(), material, "") + res, err := verifier.VerifyMaterial(context.TODO(), material, "", nil) if tc.expectErr { s.Error(err) @@ -1675,7 +1674,7 @@ func (s *testSuite) TestUndefinedBuiltinGracefulDegradation() { verifier := NewPolicyVerifier(schema.Policies, nil, &s.logger) - res, err := verifier.VerifyMaterial(context.TODO(), material, "") + res, err := verifier.VerifyMaterial(context.TODO(), material, "", nil) s.Require().NoError(err, "undefined chainloop builtin should not cause a hard error") s.Require().Len(res, 1) s.True(res[0].Skipped, "policy should be marked as skipped") diff --git a/pkg/policies/policy_groups.go b/pkg/policies/policy_groups.go index f3779eae9..8e74984b9 100644 --- a/pkg/policies/policy_groups.go +++ b/pkg/policies/policy_groups.go @@ -49,9 +49,8 @@ func NewPolicyGroupVerifier(policyGroups []*v1.PolicyGroupAttachment, policies * } // VerifyMaterial evaluates a material against groups of policies defined in the schema -func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *api.Attestation_Material, path string, opts ...VerifyMaterialOption) ([]*api.PolicyEvaluation, error) { +func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *api.Attestation_Material, path string, content []byte) ([]*api.PolicyEvaluation, error) { result := make([]*api.PolicyEvaluation, 0) - o := newVerifyMaterialOpts(opts...) groupAtts := pgv.policyGroups @@ -86,7 +85,7 @@ func (pgv *PolicyGroupVerifier) VerifyMaterial(ctx context.Context, material *ap // Load material content once for all policies in this group. Kept below // the skip above so that a material with no applicable policies never // resolves its content at all. - subject, err := material.GetEvaluableContent(path, o.content) + subject, err := material.GetEvaluableContent(path, content) if err != nil { return nil, NewPolicyError(err) } diff --git a/pkg/policies/policy_groups_test.go b/pkg/policies/policy_groups_test.go index 040a80a9a..50b95571d 100644 --- a/pkg/policies/policy_groups_test.go +++ b/pkg/policies/policy_groups_test.go @@ -376,8 +376,7 @@ func (s *groupsTestSuite) TestVerifyMaterialSuppliedContent() { groups := []*v1.PolicyGroupAttachment{{Ref: "file://testdata/policy_group_multikind.yaml"}} verifier := NewPolicyGroupVerifier(groups, nil, nil, &s.logger) - res, err := verifier.VerifyMaterial(context.TODO(), material, "", - WithMaterialContent(tc.content)) + res, err := verifier.VerifyMaterial(context.TODO(), material, "", tc.content) if tc.wantErr != nil { s.Require().ErrorIs(err, tc.wantErr) return @@ -444,7 +443,7 @@ func (s *groupsTestSuite) TestVerifyMaterialMultiKind() { } verifier := NewPolicyGroupVerifier(schema.PolicyGroups, nil, nil, &s.logger) - res, err := verifier.VerifyMaterial(context.TODO(), material, "") + res, err := verifier.VerifyMaterial(context.TODO(), material, "", nil) if tc.expectErr { s.Error(err) @@ -531,7 +530,7 @@ func (s *groupsTestSuite) TestGroupInputs() { } s.Run(tc.name, func() { v := NewPolicyGroupVerifier(schema.PolicyGroups, nil, nil, &s.logger) - evs, err := v.VerifyMaterial(context.TODO(), material, "") + evs, err := v.VerifyMaterial(context.TODO(), material, "", nil) if tc.wantErr { s.Error(err) s.Contains(err.Error(), tc.errMsg) @@ -618,7 +617,7 @@ func (s *groupsTestSuite) TestSkipPolicies() { } verifier := NewPolicyGroupVerifier(schema.GetPolicyGroups(), nil, nil, &s.logger) - evs, err := verifier.VerifyMaterial(context.Background(), material, "") + evs, err := verifier.VerifyMaterial(context.Background(), material, "", nil) if tc.expectErr { s.Error(err) @@ -715,7 +714,7 @@ func (s *groupsTestSuite) TestSkipBothMaterialAndAttestationPolicies() { } verifier := NewPolicyGroupVerifier(schema.GetPolicyGroups(), nil, nil, &s.logger) - materialEvs, err := verifier.VerifyMaterial(context.Background(), material, "") + materialEvs, err := verifier.VerifyMaterial(context.Background(), material, "", nil) s.Require().NoError(err) s.Len(materialEvs, 0, "material policy should be skipped") @@ -842,7 +841,7 @@ func (s *groupsTestSuite) TestVerifyMaterialInheritsGroupGate() { } verifier := NewPolicyGroupVerifier(schema.GetPolicyGroups(), nil, nil, &s.logger, WithDefaultGate(false)) - evs, err := verifier.VerifyMaterial(context.Background(), material, "") + evs, err := verifier.VerifyMaterial(context.Background(), material, "", nil) s.Require().NoError(err) s.Len(evs, 1)