Repository navigation
Conversation
AI Session Checks —
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
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. |
2ee9aae to
f907159
Compare
javirln
changed the title
a26107c to
7257a6e
Compare
There was a problem hiding this comment.
All reported issues were addressed across 17 reviewed files. 35 files intentionally excluded from review.
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
dc48f8f to
efb870b
Compare
| UseLatestVersion: opts.UseLatestVersion, | ||
| RequireExistingVersion: opts.RequireExistingVersion, | ||
| MarkAsLatest: opts.MarkAsLatest, | ||
| SandboxIdentity: sandboxIdentityToken, |
There was a problem hiding this comment.
I understood from the spec that the sandbox identity token was going to be put into the attestation, and verified from the attestation contents. This way, we could verify past attestations completely, not only on att init.
There was a problem hiding this comment.
The implementation follows the merged spec here. The spec keeps the attestation format as it is. The token goes in the attestation init request, and the control plane records the verified identity on the run (R-006, and "Other runners and the attestation format" in the Proposal). At push, the control plane compares the attestation with that record and rejects an authenticated sandbox runner without a verified identity (D-010).
The spec does not keep the raw token after init, so a later check relies on the record of the control plane. If you want to verify past attestations from their content alone, the CLI would record the token in the predicate. That changes the attestation format, so I would add it as a new decision in the spec, in this PR or as a follow-up. Which do you prefer?
| @@ -0,0 +1,2 @@ | |||
| -- Modify "workflow_runs" table | |||
| ALTER TABLE "workflow_runs" ADD COLUMN "sandbox_identity" jsonb NULL; | |||
There was a problem hiding this comment.
why do we need to store info?
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
…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
The platform gives a signed identity token to each Chainloop sandbox that it spawns. The new sandboxidentity package defines the token and how to sign and verify it. The token is a JWT signed with a new key for each sandbox. The key has a short-lived certificate from the control plane CA, and the certificate chain is in the x5c header. The CA marks the certificate only for a caller with the dedicated permission (R-002). The token names the organization, the project, the workflow, the sandbox run and the Chainloop token of the sandbox, and it holds the commit verification with the full commit hash (R-003). It has no expiry of its own (D-013). Verify checks the chain against the trusted root of the control plane, the mark, the organization, the project and the workflow, that the time of signing is inside the certificate lifetime (D-007), and the signature. The Issuer helper lets the platform issue a token in one call. The signing request gets the sandbox_identity_issuer flag, and the attestation init request gets the sandbox_identity token. The verifier exports ValidateCertificate, so the identity and the attestation use the same certificate check. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Chainloop-Trace-Sessions: 4503bdcc-d428-4e05-ade6-966360bf5543
The control plane now issues the certificates that sign a sandbox identity, and verifies and enforces the identity. - A new policy, PolicySandboxIdentityIssue, is the dedicated permission. No role holds it, and only a system API token can hold it (R-002, D-004). The signing service marks the certificate only for a caller that holds it. The CA writes the mark in the organizational unit of the subject (D-005). A CA that does not write the subject itself gives no marked certificate, and a mark that was not requested gives no certificate. - At attestation init, the control plane verifies the identity of a CHAINLOOP_SANDBOX run against its own CA, for the attested workflow, and only from the Chainloop token that the identity names (R-003, D-012). It records the identity on the workflow run, and tells the CLI the result. A token that does not verify is the same as no token (D-011). - When it stores an attestation of the sandbox runner, the control plane rejects an authenticated runner without a verified identity, and a commit verification that the identity does not hold (R-006, D-010). Other runners keep their behavior. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Chainloop-Trace-Sessions: 4503bdcc-d428-4e05-ade6-966360bf5543
…tity The CHAINLOOP_SANDBOX runner reads the identity token from CHAINLOOP_SANDBOX_IDENTITY. At attestation init, the CLI sends the token to the control plane. When the control plane verified it, the CLI also verifies it against the trusted root of the control plane, in the same way as attestation verify. Only then is the runner authenticated (R-004), and only then does it return the commit verification of the identity, for the full hash of the attested commit (R-005). A token that does not verify, or a control plane that does not know the identity, gives a runner that is not authenticated and has no platform data. The CLI logs the reason (D-011). This replaces the unsigned hand-off in CHAINLOOP_SANDBOX_COMMIT_VERIFICATION and its "unavailable" status for an unreadable value. No released CLI reads that value. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Chainloop-Trace-Sessions: 4503bdcc-d428-4e05-ade6-966360bf5543
Init loads the state that it wrote, and the load makes a new runner. A new CHAINLOOP_SANDBOX runner has no verified identity, so the runner info that init resolves next recorded authenticated=false, while git.head had the commit verification of the identity (R-004). Init now keeps its own runner when the runner can carry a verified identity. The other runners do not change. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Chainloop-Trace-Sessions: 4503bdcc-d428-4e05-ade6-966360bf5543
…t verification The git.head subject also records the signature algorithm, and the renderer takes it from the commit verification first. When the identity holds a signature algorithm, the control plane now also requires the attestation to record the same one (R-006). When the identity has no algorithm, the attestation takes it from the signature of the commit, so the control plane does not compare it. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Chainloop-Trace-Sessions: 4503bdcc-d428-4e05-ade6-966360bf5543
The request log of the control plane printed the arguments of AttestationService/Init, with the sandbox identity token. Redact replaces the token in the logged request, and keeps the other fields. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Chainloop-Trace-Sessions: 4503bdcc-d428-4e05-ade6-966360bf5543
…suance Verify rejects a certificate for another organization than the claims. The issuer now checks it too, so the error shows up when the platform issues the identity, and not later in the CLI. Refs PFM-7678 Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Assisted-by: Claude Code Chainloop-Trace-Sessions: 4503bdcc-d428-4e05-ade6-966360bf5543
…eation Create refuses the permission to issue a sandbox identity for a token that is not a system token, and accepts it for a system token. Refs PFM-7678 Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Assisted-by: Claude Code Chainloop-Trace-Sessions: 4503bdcc-d428-4e05-ade6-966360bf5543
The loop over dependent attestations declared a new run variable that shadowed the run of the attestation. Give it its own name, so it is clear that the later checks use the run of the attestation. Refs PFM-7678 Signed-off-by: Javier Rodriguez <javier@chainloop.dev> Assisted-by: Claude Code Chainloop-Trace-Sessions: 4503bdcc-d428-4e05-ade6-966360bf5543
efb870b to
c8f24ff
Compare
Context: PFM-7678. Design: Spec PFM-7678: Verified identity for Chainloop sandboxes, merged in chainloop-dev/platform#6875. It replaces D-009 of the commit verification spec (chainloop-dev/platform#6821). The platform side is chainloop-dev/platform#6861, which waits for a release with this change.
Today any CLI run can claim to be a Chainloop sandbox. So the platform cannot give a sandbox data that the attestation can trust. With this change, the platform gives each sandbox a signed identity token, and the CLI and the control plane verify it.
sandboxidentitypackage defines it. It is a JWT signed with a new key for each sandbox. Thex5cheader holds a short-lived certificate from the control plane CA. The token names the organization, the project, the workflow, the sandbox run and the Chainloop token of the sandbox. It holds the commit verification of the attested commit. It has no expiry of its own: the control plane accepts it only with the Chainloop token that it names, and that token expires.sandbox_identity:create. A caller with this permission gets a certificate with a mark in its subject. No role holds the permission, and only a system API token can hold it. A CA that cannot write the mark gives no certificate.CHAINLOOP_SANDBOXrunner reads the token from theCHAINLOOP_SANDBOX_IDENTITYenvironment value. The CLI verifies the token against the trusted root of the control plane, in the same way as theattestation verifycommand. Only then is the runner authenticated, and only then doesgit.headrecord the commit verification. A token that does not verify gives a runner that is not authenticated and has no platform data. The CLI logs the reason.The attestation format does not change, and the other runners do not change.
Notes on the implementation:
SandboxIdentityResult, which the spec does not name. The CLI authenticates the runner only when the control plane verified the identity and the verification of the CLI also passes. So the CLI and the control plane cannot disagree about the identity.signature.algorithmofgit.head.[REDACTED].This replaces the unsigned
CHAINLOOP_SANDBOX_COMMIT_VERIFICATIONvalue of the first version of this PR. No released CLI reads that value.CommitVerificationkeeps its JSON tags and status names, because the token carries the commit verification in that format.AI disclosure: AI assisted with this change.
Assisted-by: Claude Code