fix(agent-skills): verify gh release attestations - #678
Conversation
Code Coverage OverviewLanguages: Go, C# Go / code-coverage/testThe overall coverage remains at 50%, unchanged from the C# / code-coverage/dotnetThe overall coverage remains at 100%, unchanged from the Updated |
✅MegaLinter analysis: Success
Notices📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining See detailed reports in MegaLinter artifacts Your project could benefit from a custom flavor, which would allow you to run only the linters you need, and thus improve runtime performances. (Skip this info by defining
|
|
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit dc0a837. Configure here.
devantler
left a comment
There was a problem hiding this comment.
🤖 Generated by the Agentic Engineer
Static review
Reviewed commit: dc0a837d6426caf42c399bcddb871ab82c49de5b
This branch was reviewed statically only; no branch code was checked out or executed.
| # Linux path downloads to a file rather than streaming `curl | tar` so the network | ||
| # pull is a single retryable command. | ||
| retry curl -fsSL -o "$tmp/$asset" "$url" | ||
| if ! gh attestation verify "$tmp/$asset" --repo cli/cli; then |
There was a problem hiding this comment.
[P1] Bind verification to the requested release tag
--repo cli/cli is only the minimum provenance identity: this succeeds for an attestation from that repository without proving the archive belongs to refs/tags/v${REQUIRED}. A different-ref attested artifact substituted under the downloaded filename can therefore pass this gate and be installed. Bind the check to the expected tag/source ref (and expected signer workflow), or use release-asset verification for v${REQUIRED}, and add a negative cross-ref test.

Motivation
ghrelease archive, installed it into the runner PATH, and later executed it withGH_TOKENwithout any archive integrity or provenance verification.Description
.scripts/ensure-gh-skill.shthat requires a trusted runner-providedghwithgh attestationsupport and aborts if not present.gh attestation verify "$tmp/$asset" --repo cli/clibefore extraction or installation, failing the step if verification does not pass.GH_TOKENinto theEnsure gh >= required versionstep in bothsetup-agent-skills/action.yamlandupdate-agent-skills/action.yamlso the verifier runs with the existing runner token.setup-agent-skills/README.mdandupdate-agent-skills/README.md..github/workflows/ci.yamlwith a regression scenario that simulates a substituted release asset and asserts it is rejected before extraction/installation.Testing
bash -n .scripts/ensure-gh-skill.shto verify script parses cleanly (passed).action.yamland workflow files (passed).ghandcurl) that simulates a compromised release and asserted the script rejects it before extraction (passed).git diff --checkto validate no whitespace issues (passed).shellcheck,yamllint, andactionlintchecks were attempted but could not be fully run in this environment due to missing tools or external network/proxy restrictions (installation/network failures reported).Codex Task