Repository navigation
Conversation
The CHAINLOOP_SANDBOX runner gets no SCM token, so it cannot ask the provider whether the attested commit is signed. The platform can verify the commit before it starts the sandbox. The runner now reads that result from CHAINLOOP_SANDBOX_COMMIT_VERIFICATION and returns it only when its commit hash is the attested commit. A malformed value records the unavailable status. CommitVerification gets JSON tags, and its status encodes as a name, so the platform and the CLI share one format. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Chainloop-Trace-Sessions: 0ebed99f-c817-4242-9a55-3c64202ac7f3
PR validation —
|
| Status | Policy | Material | Messages |
|---|---|---|---|
pr-min-approvals |
pr-info |
|
|
| ✅ Passed | pr-description-required |
pr-info |
- |
| ✅ Passed | pr-user-story-linked |
pr-info |
- |
AI Session Checks — 🟢 89% · ⚠️ 1 failing
| Avg score | Sessions | Failing policies | Attribution | Files | Lines | Total Duration |
|---|---|---|---|---|---|---|
| 🟢 89% | 2 | 100% AI / 0% Human | 6 | +464 / -27 | 30h14m31s |
🟢 91% — 100% AI — ⚠️ 1 policies failing
-
Oct 9, 2026 06:24 UTC · 2h2m38s · $93.51 · 2.0k in / 602.2k out · claude-code 2.1.295 (claude-opus-5-5)
Change Summary
-
- Adds
docs/specs/PFM-7678-pr-attestation-commit-verification.mdas a spec-only change. - Documents why commit verification was missed across earlier hand-offs.
- Defines requirements, decisions, non-goals, risks, and the merge-commit open question.
- Adds
AI Session Overall Score
-
🟢 91% — Spec-only PR stayed aligned and tightly scoped; runtime lanes were not applicable.
AI Session Analysis Breakdown
-
🟢 96% · scope-discipline
-
🟢 AI isolated the PR to one spec file on a dedicated worktree. · High Impact
🟢 92% · alignment
-
🟢 AI followed the requested path from analysis to ready-for-review spec PR. · Medium Impact
🟢 91% · user-trust-signal
-
No notes.
🟢 84% · context-and-planning
-
🟢 AI asked clarifying eng-spec questions before drafting. · High Impact
abstained · solution-quality
-
No notes.
abstained · verification
-
🟡 Runtime verification and solution quality were not assessed because this PR is spec-only. · Low Severity
-
File Attribution
████████████████████100% AI / 0% HumanStatus Attribution File Lines modified ai pkg/attestation/crafter/runners/chainloopsandbox_test.go+123 / -0 created ai pkg/attestation/crafter/runners/commitverification/commitverification_test.go+79 / -0 modified ai pkg/attestation/crafter/runners/commitverification/commitverification.go+50 / -6 modified ai pkg/attestation/crafter/runners/chainloopsandbox.go+41 / -2
Policies (4, 1 failing)
Status Policy Material Messages ✅ Passed ai-config-ai-agents-allowedai-coding-session-0ebed9- ✅ Passed ai-config-no-dangerous-commandsai-coding-session-0ebed9- ⚠️ Failedai-config-no-secretsai-coding-session-0ebed9- Secret (generic-password) detected in session content [turn=30, source=tool_result, line=148]: 148 - Username:
[CHAINLOOP_TRACE_REDACTED:generic-password] - Secret (generic-password) detected in session content [turn=30, source=tool_result, line=149]: 149 - Password:
[CHAINLOOP_TRACE_REDACTED:generic-password] - Secret (generic-password) detected in session content [turn=30, source=tool_result, line=195]: 195 | rustfs (optional) | 9002 (API)
9003 (console) | S3-compatible storage backend |[CHAINLOOP_TRACE_REDACTED:generic-password]/[CHAINLOOP_TRACE_REDACTED:generic-password]| - Secret (generic-password) detected in session content [turn=407, source=assistant-tool_use:Bash, line=1]: L=/home/javi/orca/workspaces/platform/pfm-7678-record-commit-signature-verification-in/deployment/lab/config/control-plane/config.devel.yaml; grep -nE "^[a-z_]+:|policy_providers|url:|default:|name:|a...
- Secret (generic-password) detected in session content [turn=412, source=assistant-tool_use:Bash, line=1]: L=/home/javi/orca/workspaces/platform/pfm-7678-record-commit-signature-verification-in/deployment/lab/config/control-plane/config.devel.yaml; sed -n 1,20p $L; sed -n 46,58p $L; sed -n 69,110p $L | sed...
- Secret (generic-password) detected in session content [turn=414, source=tool_result, line=39]: client_secret: [CHAINLOOP_TRACE_REDACTED:generic-password]
- Secret (generic-password) detected in session content [turn=414, source=tool_result, line=42]: generated_jws_hmac_secret: [CHAINLOOP_TRACE_REDACTED:generic-password]
- Secret (generic-password) detected in session content [turn=414, source=tool_result, line=45]: cas_robot_account_private_key_path: [CHAINLOOP_TRACE_REDACTED:generic-password]
- Secret (generic-password) detected in session content [turn=425, source=tool_result, line=19]: 19 password: '[CHAINLOOP_TRACE_REDACTED:generic-password]',
- Secret (generic-password) detected in session content [turn=47, source=assistant-tool_use:Bash, line=1]: R=/home/javi/orca/workspaces/platform/pfm-7678-record-commit-signature-verification-in/deployment/lab/config/backend/config.devel.yaml; cat -n $R | sed -E 's/((secret|token|password|key|hmac)[a-z_]*: ...
- Secret (generic-password) detected in session content [turn=49, source=tool_result, line=101]: 101 # password: [CHAINLOOP_TRACE_REDACTED:generic-password]
- Secret (generic-password) detected in session content [turn=49, source=tool_result, line=21]: 21 ui_server_token: [CHAINLOOP_TRACE_REDACTED:generic-password]
- Secret (generic-password) detected in session content [turn=49, source=tool_result, line=44]: 44 chainloop_jws_hmac_secret: [CHAINLOOP_TRACE_REDACTED:generic-password]
- Secret (generic-password) detected in session content [turn=49, source=tool_result, line=72]: 72 webhook_secret: [CHAINLOOP_TRACE_REDACTED:generic-password]
- Secret (generic-password) detected in session content [turn=49, source=tool_result, line=74]: 74 secrets_backend:[CHAINLOOP_TRACE_REDACTED:generic-password]
- Secret (generic-password) detected in session content [turn=49, source=tool_result, line=77]: 77 token: [CHAINLOOP_TRACE_REDACTED:generic-password]
- Secret (generic-password) detected in session content [turn=49, source=tool_result, line=78]: 78 secret_prefix: [CHAINLOOP_TRACE_REDACTED:generic-password]
- Secret (generic-password) detected in session content [turn=599, source=assistant-tool_use:Bash, line=1]: B=/home/javi/orca/workspaces/platform/pfm-7678-record-commit-signature-verification-in/backend; grep -n "sandbox:" -A 40 $B/configs/samples/config.yaml | sed -E 's/((secret|token|password|key|api_key)...
- Secret (generic-password) detected in session content [turn=601, source=tool_result, line=11]: 73- token: [CHAINLOOP_TRACE_REDACTED:generic-password]
- Secret (generic-password) detected in session content [turn=601, source=tool_result, line=3]: 65- api_key: [CHAINLOOP_TRACE_REDACTED:generic-password]
- … and 1 more — view all ↗
✅ Passed ai-config-mcp-servers-allowedai-coding-session-0ebed9- -
🟢 87% — 100% AI — ✅ All policies passing
-
Oct 13, 2026 12:02 UTC · 28h11m53s · $48.55 · 1.3k in / 559.9k out · claude-code 2.1.296 (claude-opus-5-5)
Change Summary
-
- Implements commit-verification recording across the OSS runner and platform PR-attestation paths.
- Runs local package tests, lint checks, e2e data-path checks, and PR-check follow-up while opening and updating the PRs.
- Re-scopes the design into a sandbox-identity spec, revises it through review, and pushes the spec PR out of draft.
AI Session Overall Score
-
🟢 87% — Strong execution; only planning lagged the task's breadth.
AI Session Analysis Breakdown
-
🟢 92% · alignment
-
🟢 The user re-scoped the work explicitly before the spec rewrite and later PR steps. · High Impact
🟢 92% · user-trust-signal
-
No notes.
🟢 90% · scope-discipline
-
No notes.
🟢 90% · solution-quality
-
🟢 When lint blocked, the AI reran it serially instead of bypassing checks. · High Impact
🟢 86% · verification
-
🟢 Multiple
go testruns passed across the OSS and platform packages. · High Impact
🟡 68% · context-and-planning
-
🟠 Planning stayed implicit across a multi-repo implementation, review, and spec rewrite. · Medium Severity
💡 For work this broad, write a short shared plan before editing so later pivots and handoffs keep a stable frame.
-
File Attribution
████████████████████100% AI / 0% HumanStatus Attribution File Lines modified ai docs/specs/PFM-7678-sandbox-identity.md+170 / -19 modified ai docs/specs/PFM-7678-pr-attestation-commit-verification.md+1 / -0
Policies (4)
Status Policy Material Messages ✅ Passed ai-config-ai-agents-allowedai-coding-session-4503bd- ✅ Passed ai-config-no-dangerous-commandsai-coding-session-4503bd- ✅ Passed ai-config-no-secretsai-coding-session-4503bd- ✅ Passed ai-config-mcp-servers-allowedai-coding-session-4503bd- -
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 |
- |
Scans not applied (3)
| Scan | Reason |
|---|---|
vulnerability-scan |
no manifest/lockfile changed |
github-actions-scan |
no workflow files changed |
iac-scan |
no IaC files changed |
Security context
✅ Nothing this change touches has a recorded security-fix history.
View in Chainloop ↗ · How this works ↗
Powered by Chainloop and Chainloop Trace
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/attestation/crafter/runners/chainloopsandbox.go">
<violation number="1" location="pkg/attestation/crafter/runners/chainloopsandbox.go:90">
P2: An explicitly empty value is treated as though the variable were unset, so malformed input does not produce the promised `unavailable` status. Use `os.LookupEnv` to distinguish an unset variable and let an empty value reach the decode-error path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
| // ChainloopSandboxCommitVerificationEnvVar. It returns nil when no verification was | ||
| // given, or when the verification is for a different commit than commitHash. | ||
| func (r *ChainloopSandbox) VerifyCommitSignature(_ context.Context, commitHash string) *commitverification.CommitVerification { | ||
| raw := os.Getenv(ChainloopSandboxCommitVerificationEnvVar) |
There was a problem hiding this comment.
P2: An explicitly empty value is treated as though the variable were unset, so malformed input does not produce the promised unavailable status. Use os.LookupEnv to distinguish an unset variable and let an empty value reach the decode-error path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At pkg/attestation/crafter/runners/chainloopsandbox.go, line 90:
<comment>An explicitly empty value is treated as though the variable were unset, so malformed input does not produce the promised `unavailable` status. Use `os.LookupEnv` to distinguish an unset variable and let an empty value reach the decode-error path.</comment>
<file context>
@@ -67,8 +83,31 @@ func (r *ChainloopSandbox) Environment() RunnerEnvironment {
+// ChainloopSandboxCommitVerificationEnvVar. It returns nil when no verification was
+// given, or when the verification is for a different commit than commitHash.
+func (r *ChainloopSandbox) VerifyCommitSignature(_ context.Context, commitHash string) *commitverification.CommitVerification {
+ raw := os.Getenv(ChainloopSandboxCommitVerificationEnvVar)
+ if raw == "" || commitHash == "" {
+ return nil
</file context>
There was a problem hiding this comment.
Not changed, by decision. An empty variable is the usual way to unset a variable, so the runner treats it as not set and records no verification. The platform never sends an empty value: it omits the variable when it has no result. Neither outcome records a verified commit.
…tatus A given verification with no status, or with the unspecified status, carries no result. The runner now records the unavailable status for it, as it does for a value without a verification. Before, the attestation recorded the unspecified status. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Chainloop-Trace-Sessions: 4503bdcc-d428-4e05-ade6-966360bf5543
migmartri
left a comment
migmartri
left a comment
There was a problem hiding this comment.
I am confused about this PR, do you have a task for it?
|
@migmartri Yes, it is PFM-7678: https://linear.app/chainloop/issue/PFM-7678 The built-in PR attestation runs the CLI in the platform sandbox with the The approved spec (chainloop-dev/platform#6821) makes the platform verify the commit with the clone credential before it starts the sandbox. Only the result goes into the sandbox, not the token. This PR is the CLI half: the sandbox runner reads that result and records it only for the attested commit. The platform half is chainloop-dev/platform#6861, which waits for a release with this change. I added these links to the description. |
| // ChainloopSandboxCommitVerificationEnvVar holds a SandboxCommitVerification as JSON. | ||
| // The sandbox gets no SCM token, so the platform verifies the commit before it starts | ||
| // the sandbox and gives the result to the runner in this variable. | ||
| const ChainloopSandboxCommitVerificationEnvVar = "CHAINLOOP_SANDBOX_COMMIT_VERIFICATION" |
There was a problem hiding this comment.
can we trust a json struct to decide wether something is verified or not? My understanding is that to show verified, we do rely on a signed JWT that we check?
There was a problem hiding this comment.
In CI, the commit verification does not come from a signed JWT either. The CLI calls the GitHub or GitLab commit API with GITHUB_TOKEN or GITLAB_TOKEN and records the answer (VerifyGitHubCommit, VerifyGitLabCommit). The signed JWT that the CLI checks is the runner OIDC token. It sets runner.authenticated and the runner environment, not the commit verification.
Here the platform calls the same functions and gives the result to the CLI. The value is not signed, by decision D-009 in the spec (chainloop-dev/platform#6821). Any run with CHAINLOOP_SANDBOX can set it, and the sandbox runner records authenticated: false. The attestation content comes from the client: a token holder with a changed CLI can already write any git.head values. The spec lists a signed result as a later hardening: the platform signs the result and the CLI checks the signature.
If you want the signed result in this change, I will update the spec and do it in both PRs.
There was a problem hiding this comment.
not sure, I just want to make sure that whatever security contraints we have today to define that a commit is verified it's kept, so I am not 100% sure
There was a problem hiding this comment.
It does not keep it. Today, to record a forged "verified", you must change the CLI or build the attestation by hand. With this PR, two environment variables on the released CLI are enough.
To keep today's constraint, the CLI must accept only a result that it can authenticate. I will change the design: the platform signs the result, and the CLI verifies the signature with a key that it trusts. I will update the spec first (D-009 changes), then this PR and chainloop-dev/platform#6861. I am putting this PR back to draft until then.
There was a problem hiding this comment.
The new design is in the spec PR chainloop-dev/platform#6875, and it is wider than this result. The platform gives each sandbox run a signed identity, with a short-lived certificate from the control plane. The CLI and the control plane verify it. A verified run records an authenticated sandbox runner, and the commit verification travels in the identity. The control plane rejects an attestation that claims an authenticated sandbox runner, or a commit verification, that the identity does not support. This PR stays in draft until the spec merges.
2 participants
Context: PFM-7678. Design: the spec in chainloop-dev/platform#6821. The platform side is chainloop-dev/platform#6861, which waits for a release with this change.
The
CHAINLOOP_SANDBOXrunner gets no SCM token. So it cannot ask GitHub or GitLab whether the attested commit is signed, andgit.headrecords no verification. Asource-commitpolicy withcheck_author_verifiedfails on every attestation that the sandbox makes.With this change, the platform verifies the commit before it starts the sandbox. It gives the result to the CLI in
CHAINLOOP_SANDBOX_COMMIT_VERIFICATION, together with the full hash of the verified commit. No SCM token goes into the sandbox. The runner returns the result fromVerifyCommitSignature:unavailablestatus.CommitVerificationgets JSON tags, and its status encodes as a name (verified,unverified,unavailable,not_applicable). The platform and the CLI use one format. A change in the order of the status constants does not change the meaning of a value.AI disclosure: this change was made with AI assistance.
Assisted-by: Claude Code