Repository navigation
fix(policies): reject material policy attachments that can never apply - #3775
Conversation
A policy without material types applies only to the material named by the attachment selector. Attaching it without selector.name was accepted, but the policy never ran and nothing reported it. The control plane now rejects such attachments on contract create and update, and also typed policies whose exact selector names a contract material of an unsupported type. attestation init warns about them for contracts created before this check. The applicability rule is shared between evaluation and validation. Closes chainloop-dev#3774 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c
PR validation — ✅ 3 passing
AI Session Checks — 🟡 71% · ✅ 0 failing
|
| Status | Attribution | File | Lines |
|---|---|---|---|
| modified | ai | app/controlplane/internal/service/workflowcontract_test.go |
+56 / -0 |
| modified | ai | app/controlplane/pkg/biz/workflowcontract_policies_test.go |
+52 / -1 |
| modified | ai | app/controlplane/pkg/biz/workflowcontract.go |
+28 / -24 |
| modified | ai | app/controlplane/internal/service/workflowcontract.go |
+37 / -4 |
| modified | ai | app/controlplane/api/controlplane/v1/workflow_contract.proto |
+5 / -0 |
| modified | ai | app/controlplane/internal/service/attestation.go |
+1 / -1 |
Policies (4)
| Status | Policy | Material | Messages |
|---|---|---|---|
| ✅ Passed | ai-config-ai-agents-allowed |
ai-coding-session-2df578 |
- |
| ✅ Passed | ai-config-no-dangerous-commands |
ai-coding-session-2df578 |
- |
| ✅ Passed | ai-config-no-secrets |
ai-coding-session-2df578 |
- |
| ✅ Passed | ai-config-mcp-servers-allowed |
ai-coding-session-2df578 |
- |
🟡 82% — 99% AI — ✅ All policies passing
-
Nov 4, 2026 09:35 UTC · 3h44m10s · $28.45 · 886 in / 255.5k out · claude-code 2.1.318 (claude-opus-5-5)
Change Summary
-
Adds shared material-policy attachment validation and wires rejection into both contract schema versions. Reuses pure applicability helpers between validation and evaluation while preserving selector matching semantics. Adds focused policy and contract-validator tests, including missing selectors and unsupported material types.
AI Session Overall Score
-
🟡 82% — Focused, tested validation needs review of omitted group checks and unconfirmed legacy CLI coverage.
AI Session Analysis Breakdown
-
🟢 94% · scope-discipline
-
🟢 Production changes stay in contract validation and policy applicability; no unrelated modules or dependencies change. · High Impact
🟢 92% · solution-quality
-
🟢 Validation and evaluation share pure applicability helpers, avoiding duplicated rules and preserving runtime matching behavior. · High Impact
🟢 88% · user-trust-signal
-
No notes.
🟡 78% · verification
-
🟢 Post-cleanup policy tests and filtered contract tests return observed passes; final touched-area lint reports zero issues. · High Impact
🟠 The user remains active through review and cleanup requests but never explicitly confirms the new behavior works. · Medium Severity
💡 After validation changes, confirm one representative failing and passing contract at the actual user-facing entrypoint.
🟡 76% · context-and-planning
-
🟠 The initial shared-validator design leaves unresolved references and divergent group behavior unsettled before implementation begins. · Medium Severity
💡 Before cross-entrypoint validation work, outline handling for direct/group attachments, resolved/local references, and legacy/contract-free paths.
🟡 68% · alignment
-
🟠 The agent explicitly omits requested group-entry rejection because group semantics differ, without a captured user instruction dropping it. · Medium Severity
💡 When inspection disproves a ticket assumption, confirm the resulting scope change with the user before dropping that requirement.
-
File Attribution
███████████████████░99% AI / 1% HumanStatus Attribution File Lines created ai app/controlplane/pkg/biz/workflowcontract_policies_test.go+151 / -0 created ai pkg/policies/attachment_validation_test.go+133 / -0 created ai pkg/policies/attachment_validation.go+78 / -0 modified ai pkg/policies/policies.go+16 / -13 modified ai app/controlplane/pkg/biz/workflowcontract.go+19 / -3 modified ai app/controlplane/internal/service/attestation.go+2 / -2
Policies (4)
Status Policy Material Messages ✅ Passed ai-config-ai-agents-allowedai-coding-session-f6a62d- ✅ Passed ai-config-no-dangerous-commandsai-coding-session-f6a62d- ✅ Passed ai-config-no-secretsai-coding-session-f6a62d- ✅ Passed ai-config-mcp-servers-allowedai-coding-session-f6a62d- -
Security Checks — ✅ 6 passing
✅ Reviewed this change against the project's security context (39 concerns hunted) — no introduced risk found.
View in Chainloop ↗ · How this works ↗
✅ Reviewed this change against the project's security context (39 concerns hunted) — no introduced risk found.
View in Chainloop ↗ · How this works ↗
✅ secret-scan
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | secrets-detection |
- |
✅ sast-scan
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | owasp-top10-2025 |
- |
| ✅ Passed | sast |
- |
| ✅ Passed | cwe-top25 |
- |
| ✅ Passed | cwe-top26-40-cusp |
- |
✅ iac-scan
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | iac-misconfiguration |
- |
Scans not applied (2)
| Scan | Reason |
|---|---|
vulnerability-scan |
no manifest/lockfile changed |
github-actions-scan |
no workflow files changed |
Powered by Chainloop and Chainloop Trace
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
…tion init attestation init re-sends the contract on every run, so rejecting material policy attachments that can never apply there would break pipelines whose contract was stored before the check. Skip the check in FindOrCreateWorkflow and keep it on contract create and update. The CLI warns at init instead. Refs chainloop-dev#3774 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c
Fixes the goconst lint findings in the new attachment validation tests. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c
The contract can't be fixed at attestation init, so the check only runs when the contract is created or updated in the control plane. Remove the init warning and the verifier helper that only it used. Refs chainloop-dev#3774 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c
FindOrCreateWorkflow also creates or updates the contract when contract bytes are sent, so validate it there too, and drop the WithoutMaterialApplicabilityCheck option. Refs chainloop-dev#3774 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c
Validation now uses canMatchAnyMaterial and nameMatches, the same helpers as evaluation, instead of repeating their conditions. It only checks contract materials for exact selectors, so a new match mode is never treated as an exact match. Drop duplicate test cases. Refs chainloop-dev#3774 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: f6a62db2-f397-4a52-bc1d-ab2c15ef422c
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
migmartri
left a comment
There was a problem hiding this comment.
P2 — Batch-local dry runs bypass applicability validation
findAndValidatePolicy returns nil for references in batchPolicyNames, and findAndValidateMaterialPolicy then returns without calling ValidateMaterialAttachment (app/controlplane/pkg/biz/workflowcontract.go:636-639). WorkflowContractService.Apply supplies that list for dry runs, so a dry-run containing a new kind-less policy attached without selector.name succeeds, while the real apply rejects the same contract after resolving the policy. This makes dry-run validation disagree with the write path. Please resolve the batch policy specs for this check (and add a batch-local dry-run test) rather than skipping applicability validation.
I reproduced this with ValidateContractPolicies(..., []string{"kindless"}, nil), which returned nil for the invalid attachment.
Reviewed the change against the posted security context; I found no new bypass of those guards.
…ocal dry runs Accept the raw specs of batch policies in the apply request and use them to run the same applicability and input checks as a real apply. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: 2df578c9-f7ba-441c-8f53-410c372821f2
|
Done in 48e466a: dry-run apply now accepts the raw batch policy specs (new 🤖 Posted by Maximus bot (Claude Code) on behalf of @migmartri using the eng-autopilot skill |
There was a problem hiding this comment.
All reported issues were addressed across 9 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
View guided diff | Re-trigger cubic
Validate the raw batch policies while parsing them, and document that batch_policies is only used on dry runs. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: 2df578c9-f7ba-441c-8f53-410c372821f2
Closes #3774
A policy without material types (kind-less) applies only to the material named by the attachment selector. A contract that attached such a policy without
selector.namewas accepted, but the policy never ran and nothing reported it. For a gating policy, this looked like a passing check.Changes:
Control plane: contract create, update and apply now reject a material policy attachment that can never apply:
selector.name, for example:policy "upstream-workflow-attested" applies to any material type and requires a material selector (selector.name)A selector that names a material not declared in the contract, or a selector that is not an exact match (such as
PREFIX), is still accepted, because it can match materials added at attestation time.Attestation init: the same check runs when
attestation init --contract <file>sends the contract, because that path also creates or updates the contract. A pipeline whose contract attaches a policy that can never apply now fails at init with the same error.Shared rule: the applicability rule of
shouldApplyPolicymoves into pure functions (attachmentApplies,canMatchAnyMaterial). Policy evaluation and the new validation both use them, so the two can't drift.Not in scope:
policy develop eval --material-name: not part of this PR.findAndValidatePolicy, which fails. This is a separate, existing issue.This PR was written with AI assistance (Claude Code).