feat(binary): verify SLSA build provenance via gh attestation#16
Merged
Conversation
Adds a third layer to the download verification chain: TLS -> SHA-256 sidecar -> SLSA build provenance (optional/required) SHA-256 alone cannot detect the case where a release-scope token is stolen and the attacker uploads both a malicious binary AND a matching malicious .sha256 sidecar to the same release. The SLSA attestation published by agent-analyzer's release workflow (actions/attest-build-provenance, Sigstore-signed, Rekor-logged) binds the artifact bytes to the GitHub Actions workflow run that produced them and closes that hole. Behavior: - Soft by default: if `gh` CLI is absent, log a warning and continue with just SHA-256. Most end users do not yet have `gh` installed. - AGENT_ANALYZER_REQUIRE_ATTESTATION=1 promotes missing-gh to a hard failure (recommended for CI / paranoid consumers). - A present `gh` that reports verification failure is ALWAYS a hard failure; we never silently ignore an explicit bad attestation. Implementation: - New `verifySlsaAttestation(filePath, opts)` with injectable `ghRunner` and `ghProbe` (matches existing test mock style). - Wired into `downloadBinary` after the SHA-256 gate, before extraction. The downloaded buffer is persisted to a scratch tmpfile just long enough for `gh attestation verify` to read it; the tmpfile is rmrf'd in a finally. - New `skipAttestation` option (LOCAL DEV ONLY, symmetric with `skipChecksum`), threaded through `ensureBinary` / `ensureBinarySync`. Tests (node:test): 10 new tests covering skipped/verified/failed states, env-var promotion, hard-fail on bad attestation, repo defaulting.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit de72b3a. Configure here.
Previously requireAttestation was dropped when sync callers crossed the child-process boundary, silently losing the hard-fail intent for SLSA verification when gh is missing. Tri-state forwarding: undefined lets the child fall back to AGENT_ANALYZER_REQUIRE_ATTESTATION env var (matching ensureBinary), boolean forwards the explicit setting.
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.

Summary
Closes the "stolen release token" gap in the binary downloader. Current chain is TLS + SHA-256 sidecar, which a leaked release-scope token can fully defeat (upload attacker binary + matching attacker .sha256). Adds SLSA build-provenance verification as a third gate via
gh attestation verify, which checks the Sigstore-signed attestation that agent-analyzer's release workflow publishes throughactions/attest-build-provenance.ghis not on PATH, warn and continue on SHA-256 alone. Not every end user hasghyet.AGENT_ANALYZER_REQUIRE_ATTESTATION=1promotes missing-ghto a hard failure. Recommended for CI.gh+ failed verify is always hard-fail regardless of mode.Verification chain
Implementation notes
verifySlsaAttestation(filePath, opts)with injectableghRunner+ghProbe(matches the repo's existing mock-via-injection test pattern, no monkey-patchingcp).downloadBinaryafter SHA-256, before extraction. The in-memory buffer is persisted to a scratch tmpfile just long enough for theghcall, thenrmrf'd in afinally.skipAttestationoption added (symmetric withskipChecksum, LOCAL DEV ONLY), threaded throughensureBinary/ensureBinarySync.Test plan
node --test lib/binary/index.test.js- 39 passing, 2 skipped (Windows-only cases), 0 failingisGhAvailableprobe semanticsDeferred design questions
gh attestation verifywill fail even for legitimate binaries that were previously verified. Current behavior = hard-fail. Alternative = cache a successful verification result locally and trust the cache for N days. Not implementing now - prefer correctness over availability for a security gate.gh attestation verifycaches Sigstore/Rekor lookups in the user'sghconfig dir; TTL is opaque and controlled bygh. We rely on it rather than managing our own.gh attestation verifyconsults Rekor by default. No action needed unless we want to tighten to "must have seen log witness" mode.Note
Medium Risk
Adds an additional security gate to the binary download/install path that can now hard-fail installs (or warn/skip) based on
gh attestation verifyresults and env/option flags. Risk is moderate due to new external CLI dependency behavior and failure-mode changes during installation.Overview
Adds SLSA build provenance verification to the binary downloader by running
gh attestation verifyon the downloaded release asset before extraction, failing the install when attestation verification fails.Introduces
verifySlsaAttestation/isGhAvailablehelpers (with injectable runners for tests), a soft-by-default skip whenghis missing, and an opt-in hard requirement viaAGENT_ANALYZER_REQUIRE_ATTESTATION=1(plusskipAttestationfor local dev), and threads the new options throughensureBinary/ensureBinarySyncwith expanded test coverage.Reviewed by Cursor Bugbot for commit de72b3a. Configure here.