Repository navigation
Conversation
Contributor
|
Claude finished @tooky's task in 2m 8s —— View job Follow-up review of commit 8cda42f
Status of previous findings
New findingsNone. The refactor in |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
tooky
marked this pull request as draft
November 11, 2026 12:13
tooky
changed the title
tooky
changed the title
tooky
force-pushed
the
fm/intoto-bundle
branch
from
November 11, 2026 15:29
1f9397b to
ade2c37
Compare
|
|
||
| result, entity, err := v.verifySignature(a, digest) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("%s %w: %w", a.Describe(), ErrFailedVerification, err) |
Contributor
There was a problem hiding this comment.
Nit / worth a conscious choice: every error from verifySignature gets wrapped in ErrFailedVerification, so it now fails the whole file. That includes cases that aren't a bad signature:
it is signed with a key, not a certificate. Verify it with --key(line ~179)a DSSE envelope carries no certificate to check --certificate-identity against(line ~172)trust.forCertificate/forKeyerrors, such as an unrecognised issuer org or a failed TUF fetchloadKeyerrors, such as an unreadable--keyfile
So a mixed .intoto.jsonl holding a key-signed DSSE line and a certificate-signed bundle, run with --certificate-identity, now fails hard instead of recording the bundle and warning about the DSSE line. You can argue "can't be checked against the pinned signer" is close enough to "wrong signer", and the help text ("because another signer made it or it was changed after signing") reads either way. If the narrower meaning is the one you want, return the signing-method mismatch errors before the wrap, as !PinsSigner() already does. Either way, a test pinning the behaviour would help.
A cosmetic point too: with more than one failure, find.go builds "N provenance attestations failed verification: attestation 1 failed verification: …; attestation 2 failed verification: …", so the phrase repeats.
Contributor
Author
There was a problem hiding this comment.
Split in 8cda42f. Setting up the check and making it are now separate steps. Anything that stops the check being made is ErrCannotVerify, reported as "could not be verified":
- a trust root that can't be fetched;
- a certificate issuer with no trust root;
- an unreadable
--key; - signer flags that don't fit how the attestation was signed, which now includes
--keyon a certificate.
Only what sigstore-go's check itself rejects (signature, identity, log entry, timestamps) is ErrFailedVerification. Both still record nothing. So the mixed file you describe, a key-signed DSSE line plus a certificate bundle checked with --certificate-identity, still fails, but now it says the DSSE line could not be verified rather than that it failed verification. Tests cover both kinds, including a simulated TUF fetch failure, at the Verify, Find and command level.
With several such candidates, the message is now "N provenance attestations could not be accepted: attestation 1 failed verification: …; attestation 2 could not be verified: …", so the phrase no longer repeats.
Contributor
Author
|
A note on the red The Snyk integration still had a project configured for The Snyk scans that run in GitHub Actions pass on this PR ( |
tooky
marked this pull request as ready for review
November 11, 2026 16:40
Contributor
|
#1368 fixes Snyk tests |
attest in-toto sends commit info through gitview.BasicCommitInfo, as attest artifact, begin trail and every attestation already do. The OpenAPI contract test now covers it against the GitCommit component, so a field the CLI sends that the schema drops, or a newly required one, fails here. The published schema marks only sha1 as required, which suggests a commit known only by its SHA could leave the other fields out. The server does not accept that on POST /artifacts: it validates message, author, timestamp and branch as required, and refuses an omitted branch where it accepts an empty one. So BasicCommitInfo keeps sending every field, and attest in-toto sends no commit info at all, rather than a partial one, when it cannot find the details.
Add internal/intoto, the package behind attest in-toto, starting with reading and selecting attestations. Nothing is verified yet. Read accepts the three forms build systems write: a Sigstore bundle (actions/attest, cosign), a DSSE envelope, and a bare statement (buildx's default unsigned output), either alone in a file or one per line as in a .intoto.jsonl file. Statements are parsed with the in-toto attestation Go types. The predicate is also kept as the raw bytes the payload carried, because the typed Statement holds it as a protobuf Struct, which turns every number into a float64, and an SBOM handed on from it would no longer be the document that was signed. Raw holds the bytes each attestation came from, the whole file when it holds one, so the checksum recorded as evidence later is one a reader can reproduce from the file. Matching keeps the attestations of one predicate kind, provenance (SLSA v1 and v0.2) or sbom (SPDX and CycloneDX, versioned or not), that list the artifact's digest as a subject. When none does it says what it found instead, the predicate types or the subjects, since a wrong file or a wrong artifact is the likely mistake and the message is where the user finds out which.
A signed attestation is only worth recording if Kosli checks who signed it, so verification always pins a signer: a certificate identity (exact or by regular expression) with its OIDC issuer, or a public key. There is deliberately no way to accept any valid certificate. The flag names in the messages mirror cosign verify-attestation and gh attestation verify, so existing policies carry across. Trust roots come from TUF. A Fulcio certificate issued by Sigstore's public-good instance is checked against its trust root and must be logged in Rekor; one issued by GitHub, which signs attestations for private repositories on its own instance, is checked against GitHub's trust root and must carry an RFC 3161 timestamp instead. GitHub's TUF root is embedded, as the GitHub CLI embeds it. Roots are fetched only when a signature needs one, so unsigned input and key signatures with no timestamps never touch the network. Verification also checks the statement's subject against the artifact, both before signing is considered (so an unsigned statement about another artifact is refused too) and through sigstore-go's artifact policy. Unsigned attestations are refused unless the policy allows them, and are then marked unverified. A bare DSSE envelope has no certificate, so it can be verified only with a key. The evidence returned carries the signer, the log index, the verified integrated time and signed timestamp, a link to the log entry, and the SHA-256 of the bytes the attestation was read from. Find ties this together for a Source, the interface the GitHub and OCI sources will implement later; BundleFile is the first. When several attestations qualify it prefers a verified one, then the newest by its trusted time. The signed fixtures are generated offline by an opt-in test against a throwaway Sigstore whose trust root is committed alongside them; the README in testdata explains how to regenerate them.
ProvenanceFrom reads a SLSA provenance statement into what Kosli records an artifact by: the build URL, the commit and branch, the repository and its provider, the commit URL, and the builder, build type and workflow that annotate it. It reads the statement alone and never the environment, because attest in-toto may run in a different job or workflow from the build and must record the build's commit and run, not its own. Build types are mapped where their shape is known. GitHub Actions (actions/attest and the SLSA GitHub generator, v1 and v0.2) names the workflow, ref, repository and repository ID. BuildKit names the commit and repository in its VCS metadata; its invocation ID is a build reference rather than a URL, so the run's URL comes from the builder ID that docker/build-push-action sets. BuildKit also spells invocationID and buildInvocationID against the SLSA spec, which the predicates are read leniently enough to accept. For any other build type the commit and repository come from the git dependency, and Missing says what could not be found so the caller can warn or fail. The commit URL is derived from the repository's host: GitHub, GitLab and Bitbucket Cloud are recognised, and a self-hosted host can be named through the provider the caller passes, which attest in-toto takes from --repo-provider. The mapping lives here rather than in the attest artifact payload so a future provenance attestation type on the server can reuse it as is. SBOMDocument hands the SPDX or CycloneDX predicate on as the bytes the payload carried, for internal/sbom to summarise exactly as attest sbom does.
A provenance statement names the commit an artifact was built from but not its message, author or time, and attest in-toto may run where no checkout of that commit exists. So the details are looked up in the repository the statement names. types.CommitFinder is the provider-neutral shape of that lookup, a commit by SHA in a repository named by its path on the host, returning types.Commit like the PR evidence code does. GitLab, Bitbucket and Azure DevOps can implement it later and be chosen by the repository's host. The GitHub implementation uses the REST commits endpoint through the existing client, so --github-base-url reaches GitHub Enterprise Server and --debug dumps the exchange. The author is formatted as git and the PR evidence format it, "Name <email>", and the message is trimmed as gitview trims it.
attest in-toto records an SBOM found in an attestation the same way attest sbom records an SBOM file, so the trail shows the same record either way. This pulls the two pieces it needs out of loadSbom without changing what attest sbom sends. sbomAttestationData takes the attached bytes and the SBOM document separately. For attest sbom they are the same file. When the SBOM arrives inside a bundle, the bundle is what is attached, and the recorded checksum has to stay the checksum of the attachment, which is the promise attest sbom makes: a reader can verify it by hand. withAnnotations replaces the annotate method's merge, keeping its reason for building a new map rather than writing into the --annotate map.
Build systems already produce in-toto attestations as they build, and recording the same evidence in Kosli meant regenerating it with other attest commands, losing the signatures that make the original trustworthy. attest in-toto records the attestation itself, verified against a pinned signer, and maps its predicate onto the record Kosli already keeps for it. --predicate-type selects one kind per invocation and --name names the one resulting record, exactly as on attest artifact and attest sbom, KOSLI_NAME included: - provenance reports the artifact through the attest artifact payload, with --name as its template name (so it is held to the API's letters, digits, '-' and '_'). The commit, branch, repository and build URL come from the verified statement, never the CI environment, so the command can run in another job or workflow and still record the build's commit and run. A build or commit URL the statement lacks can be supplied with --build-url and --commit-url, and the command fails naming them when neither is available. The commit's message, author and time are looked up in the repository the statement names, on GitHub with --github-token, then read from a local checkout. Failing both, only the SHA is sent, as git_commit, with a warning that the commit's details and branch were not recorded: POST /artifacts refuses commit info missing any of them, whatever the schema says. - sbom reports an SBOM attestation built exactly as attest sbom builds one, with the bundle as its single attachment, so the recorded checksum stays the checksum of what was attached. Both carry the evidence of verification: a verified annotation, the signer, the log index and integrated time or signed timestamp, and the transparency log entry as an external URL pinned by the SHA-256 of the attestation's bytes. Provenance adds builder_id, build_type and workflow. These keys are reserved from --annotate. --source accepts only bundle for now; github and oci follow as further intoto.Source implementations. The command stays thin: reading, verification and mapping live in internal/intoto. The tests verify against the committed fixture trust root, swapped in for the TUF-backed one, and serve the commit lookup from a fake GitHub API, so none reaches the network. One records both kinds against a flow template expecting app and app.sbom and checks the trail is compliant.
actions/attest-sbom and cosign attest hand back the SBOM signed inside an in-toto attestation rather than as a plain file. attest sbom now takes either: when --sbom-file is a Sigstore bundle or DSSE envelope it reads the SPDX or CycloneDX document from the attestation, refusing any other predicate, and records it exactly as it records a plain file. The bundle or envelope is what is uploaded, so the recorded checksum is still the checksum of the file supplied, as the help has always promised, and the signed SBOM can be recovered from the attachment. Given --certificate-identity (or its regexp form) with --certificate-oidc-issuer, or --key, the attestation is verified first, the same flags and checks as attest in-toto, including that an attestation about an artifact lists that artifact as its subject. An SBOM from an attestation is annotated with verified and, when verified, with the signer, log entry and times; those keys are reserved only in that case, so plain SBOM files can still be annotated with them. Asking to verify a plain SBOM or an unsigned envelope is refused, since there is no signature to check. The help text points to attest in-toto for picking an SBOM out of a file holding several attestations, and for provenance. internal/intoto gains ReadEnvelope, which tells a bundle or envelope from anything else so a plain SBOM is never mistaken for a malformed attestation, and verification without an artifact digest for SBOMs recorded on a trail.
…t from infrastructure. CI exports KOSLI_GITHUB_TOKEN, and viper binds it to --github-token. Two cases in TestAttestInTotoCommandTestSuite/TestProvenanceWhenTheStatementLacksDetails ("records only the commit's SHA when its details cannot be found" and "has the server accept an artifact known only by its commit's SHA") expect that no commit lookup applies. With the ambient token set, the command called api.github.com for acme/widget, got a 404, and printed an extra "could not look up commit" warning. That warning broke the anchored regex and the golden. Invariant: tests of the "no lookup applies" path must not depend on whether the environment has a GitHub token. Both affected cases are in that one test function. The function's other cases pass --github-token explicitly, which overrides the env var. Fix: at the top of TestProvenanceWhenTheStatementLacksDetails, call suite.T().Setenv("KOSLI_GITHUB_TOKEN", ""). Viper's AllowEmptyEnv(false) treats an empty value as unset. Production code is unchanged. Verification: ran the suite method with KOSLI_GITHUB_TOKEN=bogus against the local server. All cases passed, including the two that failed in CI. I had to remove the "reads the commit's details from a local checkout" case temporarily, because go-git can't open this worktree's repo locally (worktreeconfig extension). That's an environment quirk that doesn't apply to CI's normal clone, and the file was restored afterwards. go vet and gofmt are clean
…. The case TestAttestInTotoCommandTestSuite/TestProvenanceWhenTheStatementLacksDetails/"reads the commit's details from a local checkout that holds it" reads HEAD's commit message and author with the regex `"[^"]+"`. The commit from the previous round (b066005) has a message containing escaped quotes (\"...\"). The JSON payload therefore held `\"`, the pattern couldn't match, and the case failed. The command's output was correct; the test broke on whatever text the latest commit happened to contain. Invariant: a regex over a JSON string the test doesn't control (the local checkout's commit message and author) must accept any valid JSON string, escapes included. The other `[^"]+` patterns in the file (lines 159 and 184) only match values from fixtures, so they're unaffected. Fix (test-only, cmd/kosli/attestInToto_test.go): added `const jsonString = "(?:[^"\\]|\\.)+"` and used it for the message and author fields in that case's goldenRegex. Production code is unchanged. Verification: - A throwaway Go test fed both patterns output with escaped quotes and a <-escaped author. The new pattern matched; the old one didn't, which reproduces the CI failure. - Ran TestAttestInTotoCommandTestSuite locally against the local server with KOSLI_GITHUB_TOKEN=bogus. All cases passed except this one, which can't run here: go-git can't open this worktree's repository ("does not support extension: worktreeconfig"). So the exact failing case hasn't been run end to end; that happens in CI's normal clone. - go vet and gofmt are clean. The change is not committed
attest sbom returned before verification when no signer was pinned, and verification was the only place the subject was checked. An unsigned envelope about a different artifact was therefore recorded against --fingerprint. The statement says which artifact it describes whether or not it is signed, so the subject is now checked first, with the same error a signed attestation gets. A signed bundle given without a signer is still recorded as unverified, as before; the help text now says its signature is not checked.
An .intoto.jsonl file with several lines failed the single-envelope probe, was taken for a plain SBOM, and was reported as "not valid JSON" by the SBOM parser. ReadEnvelope now says the content holds several documents, and attest sbom names the command that picks the SBOM from such a file.
With --repo-provider github for a GitHub Enterprise host, the lookup still went to github.com unless --github-base-url was also given, and fell back to a warning. The help for --repo-provider, --github-base-url and the command now says so. The API URL is deliberately not derived from the statement's host, so the token only goes to a server the user chose.
When a file held a provenance from the wrong signer and an unsigned one, and --allow-unsigned was set, the unsigned one was recorded and the bad signature only produced a warning. A signature that does not verify is a stronger signal than a missing one, so attest in-toto now fails and records nothing when any candidate fails verification, by another signer or after being changed, even if another candidate would qualify. This matches "a wrong signer identity or tampered payload fails verification and records nothing". Candidates that are merely unsigned are still passed over with a warning, and the rest of the choice is unchanged: verified first, then the newest.
tooky
force-pushed
the
fm/intoto-bundle
branch
from
November 11, 2026 17:12
ade2c37 to
5aa2ad0
Compare
Every error on the way to checking a signature was reported as "failed verification", so a trust root that could not be fetched, a certificate issuer with no trust root, a key file that could not be read, or signer flags that do not fit how the attestation was signed (identity flags on a key-signed bundle or DSSE envelope, --key on a certificate) read as a forged attestation. Setting up the check and making it are now separate steps. Anything that stops the check being made is ErrCannotVerify, reported as "could not be verified"; only a signature, identity, log entry or timestamp that the check rejects is ErrFailedVerification. attest in-toto still records nothing when any candidate is either, and the help text says which error means what. With several such candidates the message no longer repeats "failed verification".
| } | ||
| result, err := verifier.Verify(entity, policy) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("%s %w: %w", a.Describe(), ErrFailedVerification, err) |
Contributor
There was a problem hiding this comment.
Resolved in 8cda42f: signatureCheck/keyCheck now do all the setup work (loading the key, finding the trust root, building the identity policy, checking the signing method matches the policy) and return a new ErrCannotVerify on failure, separate from ErrFailedVerification, which now only wraps the error from the actual verifier.Verify(entity, policy) call. Find still fails the whole file on either (a deliberate choice, explained in the commit message and help text), but the error text now correctly says "could not be verified" instead of "failed verification" for setup problems (unreadable --key, untrusted issuer, wrong signing method, unfetchable trust root), and the multi-failure message no longer repeats "failed verification" at both levels ("N provenance attestations could not be accepted: attestation 1 failed verification: ..."). Covered by new cases in TestVerifySignedBundle, TestVerifyKeySignedEnvelope, the new TestVerifyWhenTheSignatureCannotBeChecked, and TestFind.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Slice 1 of #1360: a new
kosli attest in-totocommand that records an in-toto attestation a build already produced (SLSA provenance, or an SPDX/CycloneDX SBOM), verifying its signature first. It reads only from a file (--source bundle). Fetching from GitHub's attestations API (slice 2) and from OCI registries (slice 3) come later.attest sbomalso learns to read an SBOM wrapped in a Sigstore bundle or DSSE envelope.What the command does
--bundletakes a Sigstore bundle, a DSSE envelope, a bare in-toto statement, or an.intoto.jsonlfile of these.--predicate-type provenance|sbompicks one kind; others are ignored. When several qualify, a verified one wins, then the newest.--certificate-identity[-regexp]+--certificate-oidc-issuer, or--key); there's no "any certificate" mode. Unsigned statements need--allow-unsignedand are recorded withverified=false. If any candidate in the file fails its signature check (another signer, or changed after signing), the command fails and records nothing. So does a candidate whose signature can't be checked at all, such as when a trust root can't be fetched, the certificate's issuer has no trust root, or the signer flags don't fit how it was signed (identity flags on a key-signed bundle or DSSE envelope,--keyon a certificate). That error says "could not be verified" rather than "failed verification", so a network or configuration problem isn't reported as a forged attestation.--name, filled from the statement rather than the CI environment, so running it in a later job still records the build's commit and run.--build-url/--commit-urlfill gaps. Commit message, author and time come from GitHub (--github-token; GHES needs--repo-provider githuband--github-base-url, so the token only goes to a host you name), or from the checkout at--repo-root.attest sbompayload, so the trail shows the same record, with the attestation attached.verified,signer_identity,signer_issuer,log_index,integrated_time,signed_timestamp, plusbuilder_id/build_type/workflow(provenance) orsbom_format/sbom_sha256(SBOM); these keys are reserved from--annotate. A Rekor v1 entry is linked as theattestationexternal URL, pinned by the SHA-256 of the attestation's bytes.attest sbom --sbom-filenow accepts a bundle or DSSE envelope (JSON carrying a SigstoremediaTypeor a DSSEpayloadType). It checks the predicate is an SBOM and the subject matches. It verifies the signature when the new--certificate-identity[-regexp]/--certificate-oidc-issuer/--keyflags are given. Plain SBOM files behave as before. A signed bundle with no signer pinned is still accepted asverified=false, so existing pipelines don't break; the help text says so. A multi-document.intoto.jsonlgets an error pointing atattest in-toto.Where this differs from #1360
No
omitemptyongitview.BasicCommitInfo. The issue proposed sending a commit known only by its SHA and branch, because the publishedGitCommitschema requires onlysha1.POST /api/v2/artifactsvalidates more than that:git_commit_infosent'message' is a required property'branch' is a required property""null, withgit_commitsetOmitting an empty branch would have broken
attest artifacton a detached HEAD. SoBasicCommitInfois unchanged, andattest artifact,begin trailand attestations send exactly what they did. Whenattest in-totocan't find a commit's details, it sendsgit_commitwithgit_commit_info: nulland warns. The OpenAPI contract test gains a struct-levelGitCommitcase only. Server bug: kosli-dev/server#7988.External URL only for Rekor v1 entries. Bundles GitHub signs for private repos (timestamped, not logged) and Rekor v2 entries get no
attestationURL. The GitHub attestation URL arrives with--source github.Other choices:
--sourcedefaults tobundle.--build-url/--commit-urlaren't defaulted from CI, which would give the current job's run. The issue's "anything else → custom attestation" mapping isn't reachable while--predicate-typetakes onlyprovenanceandsbom.Reading the commits
75b643edcontract test case forBasicCommitInfoagainstGitCommit; the message explains theomitemptydecision.36e4f08einternal/intoto: read bundles, envelopes, statements and.intoto.jsonl, and match by predicate kind and subject.a213c4e8verification with sigstore-go. Trust roots come over TUF: Sigstore public-good, plus GitHub's instance from an embedded initial root (github-tuf-root.json, asghembeds it). Most of this commit is offline fixtures signed by a throwaway Sigstore;internal/intoto/testdata/README.mdsays how to regenerate them.6b72452bmap provenance and SBOM predicates onto Kosli fields.7424c319GitHub commit lookup behind a provider-neutraltypes.CommitFinder.93a63929refactorattest sbompayload building soattest in-totocan share it; no change to whatattest sbomsends.59ddd133theattest in-totocommand, its tests, help text and empty-flag-audit entries.4fe746c1attest sbomreads bundles and DSSE envelopes.2a220e0areview tidy-up.88b6ac5a,2103ac75test-only fixes (an ambientKOSLI_GITHUB_TOKENin CI, and a regex over a commit message with escaped quotes). Their subjects are whole paragraphs; the change is a few lines.8dce16f7,7719a9ea,290b885b,51952383review follow-ups: check the SBOM subject even with no signer pinned; point multi-document files atattest in-toto; document GHES lookups; record nothing when any candidate fails verification.5aa2ad0brestore a doc comment.8cda42fatell a signature that can't be checked (trust root fetch, unknown issuer, method mismatch, unreadable key) apart from a bad one; both still record nothing.Things to know
make buildbuilds it).~/.sigstore), so it needs network access. Unsigned and key-signed attestations without timestamps fetch nothing.Testing
CI is green. Locally:
Dry run of unsigned provenance (
KOSLI_API_TOKEN=DRY_RUN … --allow-unsigned --bundle internal/intoto/testdata/statements/github-provenance.json) prints the artifact payload withgit_commit_info: null,template_reference_name: appandverified: "false".