Repository navigation
fix(intoto): tie the default GitHub signer for attest in-toto to the current run - #1377
Conversation
github-actions
Bot
added
documentation
|
Claude finished @tooky's task in 2m 17s —— View job Follow-up review of new commit (5777dfe..1caa311)
Only one new commit since the last review: Result: mbevc1's "New (low): duplicate check drops the whole download URL query" finding is resolved — the dedup logic now only matches identities against earlier listings, keeping same-listing entries that share an identity, with new test coverage for both the single-listing case and the filter-ignoring-server case. No new bugs or regressions found in the changed lines. The other open threads/findings were either already resolved before this commit or are not code issues (confirmed in the summary comment below). Full details posted as a summary comment. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Follow-up review of new commits (415a221..45d17c0)Previous finding — resolved. The only prior inline finding (comment, on New commits reviewed:
No new inline comments. |
|
All six points from the #1372 review are fixed. Two minor findings, neither blocking: 1. Duplicate attestations when a server ignores 2. SPDX filter only works if GitHub prefix-matches Nit: |
A workflow file signs as its repository whatever ref it runs on, so the fixtures gain the same provenance statement signed from a branch and from a pull request, each in a run of its own and logged after the build's. They stand for what anyone who can push a branch can attest about an artifact. Regenerating makes new keys, so every signed fixture and the trust root change with them.
With --source github in GitHub Actions and no signer flag, the signer defaulted to any workflow file in the repository, on any ref. Run from another workflow, such as a deployment, the newest attestation that verified was recorded. Anyone able to push a branch could run actions/attest with a predicate of their own for a production digest, and Kosli recorded it as verified. The default signer now also requires the certificate to have been issued to the run the command is part of, in any of its attempts. The restriction lives in the policy, so the verifier enforces it and an attestation another run made is never reported as verified by default. When the run made none, the command fails, names each attestation with its run and signer, and names the flags that pin a signer. An attestation recorded in the run that made it is chosen as before, and one an earlier run made about the same artifact is passed over without a warning. A pinned signer is unchanged: it still records an attestation from another run. The help text, the example for reading from another run and the sample workflow now pin the workflow file with its ref rather than every workflow in the repository. Addresses point 1 of the review on #1372.
The repository to read and the run the command is part of were each worked out in three places: when choosing the source, when pinning the default signer and when building the source to read. validate now settles both once and keeps them, so the default signer is pinned to the repository and run that are actually read. Addresses point 6 of the review on #1372.
--repository owner/name lost its owner: the name after the slash was read under --github-org, so --repository platform/app in a run that defaulted --github-org to acme read acme/app, and pinned the default signer to it. The owner now comes from --repository when it names one, and --github-org is no longer needed with it. A --github-org the caller gave, rather than one GitHub Actions defaulted, must name the same owner; a mismatch is refused, since reading either repository would ignore one of the two flags. A --repository that is neither a name nor owner/name is refused too, rather than read as its last path segment. Addresses point 2 of the review on #1372.
attest in-toto took its --repository default from whichever CI it ran in, so on GitLab CI, Bitbucket Pipelines, Azure DevOps or CircleCI it was that CI's project name. A caller there who gave --github-org but not --repository had a repository of that name read on GitHub, and a 404 blaming the token, rather than being told --repository is required. Both flags now default only in GitHub Actions, where the run's repository is a GitHub repository. Addresses point 4 of the review on #1372.
The listing of an artifact's attestations was not filtered, so recording provenance also downloaded every SBOM bundle, and each download went through a new retrying client. The listing is now asked for each predicate type of the kind wanted, with GitHub's predicate_type filter, and every bundle is downloaded through one client. A source is told the kind it is read for, so one that can narrow what it returns does. GitHub's documentation says little about the filter. On github.com in November 2026 it matched an attestation whose predicate type starts with it, so https://spdx.dev/Document finds .../v2.3, and it answered a filter matching nothing with 404, as it does a token that cannot read the repository. When no filter finds anything the whole listing is read as before. That tells the two apart, keeps the error that names the predicate types GitHub holds instead, and does not depend on how the filter matches. The fake GitHub servers in the tests filter the same way. Addresses point 3 of the review on #1372.
An attestation read from GitHub that has no public transparency log entry, as in a private repository, is linked to GitHub's API listing of the artifact's attestations. That link was recorded with the SHA-256 of the one attestation, which the listing can never match: it holds every attestation of the artifact. The listing is still linked, since it is where the attestation is held, but without a fingerprint. The help text says so, and that reading it needs a token. A transparency log entry is linked with the fingerprint as before. Addresses point 5 of the review on #1372.
In GitHub Actions, --github-org given alone for another owner is refused because --repository defaults to the run's own repository. The error named a --repository the caller never passed. It now says the repository was defaulted from GITHUB_REPOSITORY, and that passing --repository as well reads another one. The refusal itself stays: reading <github-org>/<name> would be a guess. When the caller gave both flags the error is as before.
When no predicate type filter finds anything, the whole listing is read and every bundle in it downloaded, which is the cost the filter exists to avoid. The comment now says that is deliberate: it is paid only on that path, to name what GitHub holds instead.
…ilter The listings for a kind's predicate types were appended as they came. A server that ignores predicate_type, as an older GitHub Enterprise Server may, answers each with everything it holds, so every attestation was listed twice: each bundle was downloaded twice and each warning printed twice, though the attestation chosen was still the right one. An attestation already listed is now skipped. It is told apart by where its bundle is downloaded from, without the query that signs the URL afresh for each listing, or by the bundle itself when it is given inline.
45d17c0 to
5777dfe
Compare
|
@mbevc1 thanks.
The nit: agreed, intended and harmless. |
|
Could you also please check the following: New (low): duplicate check drops the whole download URL query |
The duplicate check told attestations apart by their download URL without its query, and applied inside a single listing too. Two different attestations whose download URLs differ only in the query would have been read as one. An entry is now skipped only when an earlier listing gave it. Within one listing every entry is kept, so the check still stops a server that ignores the predicate type filter from having each attestation read twice, and can no longer drop one.
Follow-up review of new commit (5777dfe..1caa311)Only one new commit since the last review: Previous findings in this area:
No new bugs, security issues, or regressions found in the changed lines. |
|
@mbevc1 done in 1caa311. An entry is now skipped only when an earlier listing gave it, so two attestations in the same listing are both read even when their download URLs differ only in the query. There is a test for that case. Across listings the comparison still ignores the query, because each listing signs its download URLs afresh. |
Summary
kosli attest in-toto --source githubcould record an attestation made by anyone able to push a branch. With no signer flag in GitHub Actions, the signer defaulted to any workflow file in the repository on any ref, sobuild.yml@refs/heads/feature-xandbuild.yml@refs/pull/12/mergeboth passed. Run from a deployment job, the command took the newest attestation that verified, so a made-up SLSA predicate for a production digest could be recorded asverified: true.The default signer now also requires that the certificate was issued to the run the command is part of, in any of its attempts.
default signer (no signer flag, GitHub Actions on github.com) issuer https://token.actions.githubusercontent.com identity ^https://github.com/<owner>/<repository>/.github/workflows/ + run the certificate was issued to this run, in any attemptThis is point 1 of the review of #1372, which merged before the review's six points were dealt with; points 2 to 6 are below.
--source githubis not in a release yet (v2.56.0 does not include it), so no released behaviour changes.What changes for a workflow using the default signer
In the run that attested the artifact (a later step or job, or a re-run): nothing. The same attestation is recorded with the same output.
In another run, such as a deployment workflow: the command fails until a signer is pinned. The error names each attestation with its run and signer, and the flags to pass:
A pinned signer behaves as before and still records an attestation another run made. The default is tied to the run instead of to protected refs.
Attestations that earlier runs made about the same artifact are passed over without a warning when this run's own is recorded. An image rebuilt from cache can keep its digest, and a warning per earlier run would grow with every build.
The run is part of the policy (
Policy.RunInvocationURI), so the verifier enforces it: under the default, an attestation from another run is never reported as verified.The help text, the example for reading from another run and the sample workflow now pin the workflow file with its ref. The old example pinned every workflow in the repository by regexp, which had the same weakness. Docs are generated from the help text.
2.
--repository owner/namekeeps its ownerThe owner was always taken from
--github-org, so--repository platform/appread<github-org>/app. The owner now comes from--repositorywhen it names one, and a--github-orgthe caller gave that disagrees is refused.--github-org--repositoryacmewidgetacme/widgetplatform/appplatform/appacme, defaulted by GitHub Actionsplatform/appplatform/app(wasacme/app)acme, given by the callerplatform/appplatform, given by the calleracme/widget, defaulted by GitHub Actionsplatform/widget); the error says the repository was defaulted fromGITHUB_REPOSITORY, and to pass--repositoryas wellhttps://github.com/acme/widgetwidget)3. Attestations of other kinds are no longer downloaded
Recording provenance used to download every SBOM bundle too, each through a new retrying client. The listing is now filtered by predicate type, and every bundle downloads through one client.
--predicate-typepredicate_typefilters sentprovenancehttps://slsa.dev/provenance/v1,https://slsa.dev/provenance/v0.2sbomhttps://spdx.dev/Document,https://cyclonedx.org/bomGitHub does not document how the filter matches. On github.com it matches by prefix, and answers a filter that matches nothing with 404, the same answer a token that cannot read the repository gets (see Evidence). So when no filter finds anything, the command reads the whole listing as it did before. That keeps the error that names the predicate types GitHub holds instead, and tells "none of this kind" apart from "cannot read the repository".
GitHub also accepts the shorthand
sbom. The command sends full predicate types because the shorthand could not be checked against a CycloneDX attestation.A server that ignores the filter, as an older GitHub Enterprise Server may, answers each listing with everything it holds. An attestation that an earlier listing already gave is skipped, so each is still downloaded and reported once. Entries within one listing are never compared with each other.
One dependency remains. If GitHub ever matched the filter exactly, a versioned SPDX attestation (
https://spdx.dev/Document/v2.3) would be missed whenever a CycloneDX one for the same digest matched.4.
--github-organd--repositorydefault only in GitHub ActionsOn GitLab CI, Bitbucket Pipelines, Azure DevOps and CircleCI,
--repositorydefaulted to that CI's project name, so--github-orgalone led to a 404 that blamed the token. There is no default there now, and the caller is told both flags are required.5. The fallback
attestationlink is recorded without a fingerprintAn attestation with no public transparency log entry, as in a private repository, links GitHub's API listing, since that is where it is held. The listing holds every attestation of the artifact, so no fingerprint could match it, and the link is now recorded without one. The help text says the link needs a token. A transparency log entry is linked with the fingerprint as before.
6. The repository and the run are worked out once
They were worked out in three places.
validatenow does it once, and the default signer is built from the same repository that is read.Commits
Each builds and passes the in-toto tests on its own.
--repository owner/name(point 2)--repositorythat--github-orgdisagrees with was defaulted (from the review of this PR)Evidence
All tests are offline: a fake GitHub server and a throwaway Sigstore trust root.
Point 1.
TestGitHubSourceDefaultSignerholds three attestations of one artifact, all signed by the repository's ownbuild.yml: onmainin the build, then onrefs/heads/feature-xand onrefs/pull/12/mergein later runs.mainattestation. With--certificate-identity ...build.yml@refs/heads/mainthe deployment run records it too, and warns that the other two failed verification.Point 3. How github.com answered
predicate_typeon 13 November 2026 forghcr.io/kosli-dev/cli:v2.56.0, whose digest holds one SPDX and one SLSA v1 attestation:predicate_typehttps://slsa.dev/provenance/v1https://slsa.dev/provenancehttps://spdx.dev/Documenthttps://spdx.dev/Dochttps://cyclonedx.org/bom,https://slsa.dev/provenance/v0.2sbomThe fake GitHub servers in the tests filter the same way, and
TestGitHubSourceReadsOnlyThePredicateTypeAskedForrecords an SBOM while the provenance beside it cannot be downloaded.Points 2, 4 and 5 are covered by
TestGitHubOwnerAndName,TestAttestInTotoGitHubOrgInARunOfAnotherOwnersRepository,TestAttestInTotoGitHubRepositoryDefaultsandTestAttestationLink.Merge Danger
Door: two-way. The change is confined to the CLI, the command is beta, and
--source githubhas not been released.Blast Radius:
kosli attest in-totowith the default signer from a run other than the one that attested fails until it pins a signer. The error says what to pass.--github-orggiven alone in GitHub Actions for another owner, and a--repositorythat is neither a name norowner/name, are now refused.attestationlink.