Skip to content

fix(policies): evaluate redacted materials against the content that was stored - #3383

Draft
jiparis wants to merge 5 commits into
chainloop-dev:mainfrom
jiparis:jiparis/pfm-7073-redaction-send-redacted-content-to-policies
Draft

fix(policies): evaluate redacted materials against the content that was stored#3383
jiparis wants to merge 5 commits into
chainloop-dev:mainfrom
jiparis:jiparis/pfm-7073-redaction-send-redacted-content-to-policies

Conversation

@jiparis

@jiparisjiparis commented Aug 28, 2026

Copy link
Copy Markdown
Member

Secret redaction (#3368) only rewrites the copy of a CHAINLOOP_AI_CODING_SESSION that leaves the machine. Policies were still evaluated against the untouched original on disk, having access to any potential secret that could have leaked into the AI session.

The invariant now implemented is that the policy engine receives exactly the bytes that were stored.

Changes

  • Crafters that do not store the artifact verbatim implement a transformCrafter seam and report the bytes they stored. materials.Craft surfaces them as CraftResult.EvaluableContent, and the crafter passes them to both policy verifiers via the new policies.WithMaterialContent per-call option.
  • Content resolution fails closed: a material annotated chainloop.material.redacted=true returns ErrRedactedContentRequired rather than falling back to the file on disk. This is uniform across CAS backends, inline included.
  • chainloop policy devel eval threads the same content through, so a policy is developed against the input the real run produces.
  • The bundled testdata policy now detects [REDACTED:<rule>] placeholders and the redaction annotations, since the raw pattern it looked for is no longer present in its input.

Materials that were not transformed are unaffected and hold nothing extra in memory: their content still resolves from the inline copy or the file on disk. This also closes a gap for skipUpload materials, whose sanitized bytes were previously retained nowhere at all.

Notable consequences

A redacted material's policy input can no longer be reconstructed from persisted crafting state alone, so a future push-time or server-side material evaluation must plumb the bytes through as well. materials.Craft now returns *CraftResult and policies.Verifier.VerifyMaterial takes variadic options — both breaking for external importers of these packages.

Not covered here

Two gaps remain that are properties of redaction rather than this plumbing: --skip-secret-redaction stores the session as captured, and secrets counted as report.Unlocated (landing in a protected path, or spanning two JSON leaves) are not redacted in the stored copy either. Detecting placeholders in the upstream ai-coding-session-no-secrets policy is the second part of the issue and lives in another repository; that policy should keep its pattern matching alongside the placeholder check, since pattern matching remains the only thing that catches the secrets redaction could not reach.

AI disclosure

Produced with assistance from Claude Code; every commit carries an Assisted-by: Claude Code trailer.

Review in cubic

…as 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 <jiparis@chainloop.dev>
@chainloop-platform

chainloop-platformBot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

AI Session Checks — 🟢 92% · ⚠️ 1 failing

Avg scoreSessionsFailing policiesAttributionFilesLinesTotal Duration
🟢 92%1⚠️ 1100% AI / 0% Human99+1046 / -4916h54m9s

🟢 92% — 100% AI — ⚠️ 1 policies failing

Aug 28, 2026 11:10 UTC · 6h54m9s · $112.27 · 1.3k in / 360.2k out · claude-code 2.1.250 (claude-opus-5)

View session details ↗

Change Summary

  • Plumbs sanitized AI-session content through crafting and policy evaluation so policies read stored bytes, not the original file.
  • Updates policy evaluation tests and bundled no-secrets policy inputs around redacted placeholders and annotations.
  • Refactors the materials API around CraftResult, folds GetEvaluableContentFrom into GetEvaluableContent, and renames CraftResult.Transformed to Content.

AI Session Overall Score

🟢 92% — Well-planned, aligned session with strong verification and calm user follow-up.

AI Session Analysis Breakdown

🟢 94% · user-trust-signal

No notes.

🟢 93% · alignment

🟢 Follow-up refactors tracked specific user requests rather than drifting into unrelated work. · High Impact

🟢 92% · scope-discipline

No notes.

🟢 92% · verification

🟢 AI ran focused tests, full suites, vet, and lint before reporting completion. · High Impact

🟢 91% · solution-quality

No notes.

🟢 90% · context-and-planning

🟢 AI wrote and revised a substantive plan before major edits began. · High Impact


File Attribution

████████████████████ 100% AI / 0% Human

StatusAttributionFileLines
modifiedaipkg/attestation/crafter/api/attestation/v1/crafting_state_test.go+135 / -60
modifiedaipkg/policies/policies_test.go+115 / -10
modifiedaipkg/attestation/crafter/materials/materials.go+73 / -43
modifiedaipkg/attestation/crafter/materials/chainloop_ai_coding_session.go+54 / -48
modifiedaipkg/attestation/crafter/api/attestation/v1/crafting_state.go+55 / -27
modifiedaipkg/attestation/crafter/crafter_test.go+82 / -0
modifiedaipkg/attestation/crafter/materials/chainloop_ai_coding_session_redaction_test.go+56 / -25
modifiedaipkg/policies/policy_groups_test.go+73 / -7
modifiedaipkg/policies/policies.go+39 / -33
modifiedaiapp/cli/internal/policydevel/eval.go+29 / -24
modifiedaipkg/attestation/crafter/crafter.go+22 / -16
modifiedaipkg/attestation/crafter/materials/craft_result_external_test.go+37 / -1
modifiedaipkg/attestation/crafter/materials/craft_result_test.go+36 / -1
modifiedaiapp/cli/internal/policydevel/eval_test.go+24 / -9
createdaipkg/attestation/crafter/testdata/policies/ai_session_no_secrets.yaml+25 / -0
modifiedaiapp/cli/internal/policydevel/testdata/ai-coding-session-no-secrets-policy.yaml+16 / -5
modifiedaipkg/policies/policy_groups.go+9 / -7
modifiedaipkg/attestation/crafter/materials/materials_test.go+10 / -5
modifiedaipkg/attestation/crafter/materials/chainloop_ai_security_context_test.go+5 / -5
modifiedaipkg/attestation/crafter/materials/radamsa.go+4 / -5
modifiedaipkg/attestation/crafter/materials/evidence_test.go+4 / -4
modifiedaipkg/attestation/crafter/materials/oci_image.go+4 / -4
modifiedaipkg/attestation/crafter/materials/oci_image_test.go+4 / -4
modifiedaipkg/attestation/crafter/materials/artifact_test.go+3 / -3
modifiedaipkg/attestation/crafter/materials/cyclonedxjson_test.go+3 / -3

…and 74 more file(s).


Policies (4, 1 failing)

StatusPolicyMaterialMessages
✅ Passedai-config-ai-agents-allowedai-coding-session-06ec99-
✅ Passedai-config-no-dangerous-commandsai-coding-session-06ec99-
⚠️ Failedai-config-no-secretsai-coding-session-06ec99
  • Potential secret (Quoted API key/password) found in session content [turn=436, source=tool_result, line=10, value=Secret ...tY6"]
  • Potential secret (Quoted API key/password) found in session content [turn=58, source=tool_result, line=28, value=Secret ...tY6"]
✅ Passedai-config-mcp-servers-allowedai-coding-session-06ec99-

Powered by Chainloop and Chainloop Trace

@cubic-dev-aicubic-dev-aiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 17 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/attestation/crafter/api/attestation/v1/crafting_state.go">
<violation number="1" location="pkg/attestation/crafter/api/attestation/v1/crafting_state.go:160">
P2: When callers supply material content for a material without an `Artifact`, this branch is never reached, so policies receive `{}` rather than the supplied bytes. Select the explicit content before artifact-dependent resolution so `WithMaterialContent` honors its override contract for every material kind.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic


switch {
case useInlineContent:
case len(content) > 0:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When callers supply material content for a material without an Artifact, this branch is never reached, so policies receive {} rather than the supplied bytes. Select the explicit content before artifact-dependent resolution so WithMaterialContent honors its override contract for every material kind.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/attestation/crafter/api/attestation/v1/crafting_state.go, line 160:
<comment>When callers supply material content for a material without an `Artifact`, this branch is never reached, so policies receive `{}` rather than the supplied bytes. Select the explicit content before artifact-dependent resolution so `WithMaterialContent` honors its override contract for every material kind.</comment>
<file context>
@@ -110,30 +111,62 @@ func (m *Attestation_Material) NormalizedOutput() (*NormalizedMaterialOutput, er
-
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
</file context>

@jiparis
jiparis marked this pull request as draft August 28, 2026 17:12
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 <jiparis@chainloop.dev>
Chainloop-Trace-Sessions: 06ec9990-4ad5-4b3c-ad86-2421b24dba57
…Content
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 <jiparis@chainloop.dev>
Chainloop-Trace-Sessions: 06ec9990-4ad5-4b3c-ad86-2421b24dba57
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 <jiparis@chainloop.dev>
Chainloop-Trace-Sessions: 06ec9990-4ad5-4b3c-ad86-2421b24dba57
…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 <jiparis@chainloop.dev>
Chainloop-Trace-Sessions: 06ec9990-4ad5-4b3c-ad86-2421b24dba57
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@jiparis