Skip to content

feat(controlplane): enforce workflow contracts when attestations are stored - #3465

Merged
jiparis merged 2 commits into
chainloop-dev:mainfrom
jiparis:feat/enforce-contract-server-side
Sep 24, 2026
Merged

jiparis merged 2 commits into
chainloop-dev:mainfrom
jiparis:feat/enforce-contract-server-side

Conversation

@jiparis

@jiparis jiparis commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Summary

The workflow contract was enforced only by the CLI. crafter.ValidateAttestation had no caller in the control plane, so AttestationService.Store accepted any well-formed, signed attestation regardless of whether it satisfied the contract revision pinned on the run. Because the bundle is signed by the same client that decides whether to run that check, a valid signature says nothing about contract compliance — a modified or custom crafter could push an attestation that satisfies no contract.

The control plane is now the authority. WorkflowRunUseCase.SaveAttestation loads the contract revision pinned on the workflow run at init time and rejects an attestation whose required materials are missing, before anything is written to the database or CAS. The rejection surfaces as a client error naming the missing materials and the revision they were measured against.

This applies to all new attestations. Already-stored attestations are not revalidated.

Behaviour

  • Every non-optional material declared in the contract must be present.
  • Choke groups keep their "at least one of" semantics.
  • Materials the contract does not declare remain allowed, so runtime-added evidence and exploded archives are unaffected.
  • Covers both v1 and v2 contract formats.

The CLI keeps its own check, which still gives fast local feedback before a push.

The material presence check is extracted out of CraftingState.ValidateComplete into a validator shared by the CLI and the control plane, so the two cannot drift apart.

Out of scope

Contract-declared annotations, whether contract-declared policies were actually evaluated, and the BlockOnPolicyViolation strategy all remain client-enforced.

AI disclosure

This contribution was produced with AI assistance (Claude Code). Each commit carries an Assisted-by: Claude Code trailer.

Review in cubic

…stored

The workflow contract was only enforced by the CLI. crafter.ValidateAttestation
had no caller in the control plane, so Store accepted any well-formed, signed
attestation regardless of whether it satisfied the contract revision pinned on
the run. The bundle is signed by the same client that decides whether to run
that check, so a valid signature says nothing about contract compliance.

SaveAttestation now loads the contract revision pinned on the workflow run at
init time and rejects an attestation whose required materials are missing,
before anything is persisted. Choke groups are honoured, and materials the
contract does not declare remain allowed.

The material presence check is extracted into a validator shared by the CLI and
the control plane so both enforce identical semantics.

Assisted-by: Claude Code
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>

Chainloop-Trace-Sessions: 513023ae-e4c2-4c52-ba7b-5ef838eda385
@chainloop-platform

chainloop-platform Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

AI Session Checks — 🟡 84% · ✅ 0 failing

Avg score Sessions Failing policies Attribution Files Lines Total Duration
🟡 84% 1 ✅ 0 100% AI / 0% Human 7 +668 / -8 7h57m2s

🟡 84% — 100% AI — ✅ All policies passing

Sep 22, 2026 09:40 UTC · 7h57m2s · $60.04 · 804 in / 268.4k out · claude-code 2.1.278 (claude-opus-5)

View session details ↗

Change Summary

  • Adds shared ValidateMaterialsPresence logic and enforces contract materials during attestation save.
  • Adds integration, round-trip, and error-mapping tests for contract enforcement behavior.
  • Adds a side-effect-free preflight so skipDB rejects invalid bundles before CAS upload.

AI Session Overall Score

🟡 84% — Good fix and verification, but one user-corrected detour kept the session from green.

AI Session Analysis Breakdown

🟢 92% · context-and-planning

🟢 A detailed written plan landed before substantive edits and was updated after clarification. · High Impact

🟢 90% · solution-quality

🟢 The review finding became a side-effect-free preflight, not a narrow CAS workaround. · High Impact

🟢 89% · scope-discipline

No notes.

🟢 88% · verification

🟢 Focused Go tests were added and rerun after the review fix. · High Impact

🟡 The session relied on automated tests only; the engaged user never confirmed behavior directly. · Low Severity

🟡 74% · user-trust-signal

No notes.

🟡 72% · alignment

🟠 AI introduced an empty-string-material behavior change the user had to reject and remove. · Medium Severity

💡 When a risk fix changes accepted behavior, ask before folding it into the main patch.


File Attribution

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

Status Attribution File Lines
modified ai app/controlplane/pkg/biz/workflowrun_contract_integration_test.go +275 / -0
created ai pkg/attestation/renderer/chainloop/materials_roundtrip_test.go +147 / -0
modified ai pkg/attestation/crafter/api/attestation/v1/crafting_state_validations_test.go +111 / -0
modified ai app/controlplane/pkg/biz/workflowrun.go +92 / -5
modified ai pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go +25 / -3
modified ai app/controlplane/internal/service/service_test.go +10 / -0
modified ai app/controlplane/internal/service/attestation.go +8 / -0

Policies (4)

Status Policy Material Messages
✅ Passed ai-config-ai-agents-allowed ai-coding-session-513023 -
✅ Passed ai-config-no-dangerous-commands ai-coding-session-513023 -
✅ Passed ai-config-no-secrets ai-coding-session-513023 -
✅ Passed ai-config-mcp-servers-allowed ai-coding-session-513023 -

Security Checks — ✅ 5 passing

✅ 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 -

⏭️ 3 scans not applied

Scan Reason
vulnerability-scan no manifest/lockfile changed
github-actions-scan no workflow files changed
iac-scan no IaC files changed

View attestation ↗


PR validation — ⚠️ 1 failing

Status Policy Material Messages
✅ Passed pr-min-approvals pr-info -
✅ Passed pr-description-required pr-info -
⚠️ Failed pr-user-story-linked pr-info PR/MR #3465 does not reference a user story or issue in title, description, or branch 'feat(controlplane): enforce workflow contracts when attestations are stored'. Expected patterns: ["(?i)[A-Z]+-[0-9]+", "#[0-9]+", "(?i)[A-Z]{2", "}-[0-9]+", "(?i)gh-[0-9]+", "(?i)\[[A-Z]+-[0-9]+\]"]

View attestation ↗


Powered by Chainloop and Chainloop Trace

@jiparis
jiparis requested a review from a team September 22, 2026 15:27

@cubic-dev-ai cubic-dev-ai Bot 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.

All reported issues were addressed across 8 files

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

Re-trigger cubic

Comment thread app/controlplane/pkg/biz/workflowrun.go
When skip_db_storage is enabled the bundle is uploaded to the CAS backend
before SaveAttestation runs, so an attestation rejected for violating its
contract would already have left an orphaned blob behind.

Add ValidateAttestationContract, a side-effect-free check the synchronous CAS
path runs before uploading. SaveAttestation keeps its own check, which remains
the authoritative one since it sits on the path every attestation takes.

Assisted-by: Claude Code
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>

Chainloop-Trace-Sessions: 513023ae-e4c2-4c52-ba7b-5ef838eda385
@jiparis
jiparis merged commit 9aa7066 into chainloop-dev:main Sep 24, 2026
16 of 17 checks passed
@jiparis
jiparis deleted the feat/enforce-contract-server-side branch September 24, 2026 14:34
Sign up for free to 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.

3 participants