Skip to content

fix(install): make provenance check opt-in#1325

Merged
benjaminrigaud-gg merged 1 commit into
mainfrom
benjaminrigaud/end-609-ggshield-installation-failed-through-script-on-macos
Jun 25, 2026
Merged

fix(install): make provenance check opt-in#1325
benjaminrigaud-gg merged 1 commit into
mainfrom
benjaminrigaud/end-609-ggshield-installation-failed-through-script-on-macos

Conversation

@benjaminrigaud-gg

@benjaminrigaud-gg benjaminrigaud-gg commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Fixes END-609: the install aborted because gh attestation verify fails when gh is older than 2.56.0 — its bundled sigstore-go can't parse the Ed25519 (PKIX_ED25519) tlog key in Sigstore's trusted root. The installers ran gh automatically and failed closed, so an outdated (or unauthenticated) gh on $PATH blocked the whole install even though the sha256 checksum had already matched.

Following @clement-tourriere's review: gh is not a ggshield dependency, and a public attestation lookup still needs an authenticated gh, so running it automatically is fragile and usually can't run anyway. Build-provenance verification is now opt-in, in both install.sh and install.ps1:

  • Default: mandatory sha256 (fail-closed) only; gh is not invoked. The installer prints the manual gh attestation verify command.
  • GGSHIELD_REQUIRE_ATTESTATION=1 (also accepts true/yes/on): runs gh attestation verify and fails the install if gh is missing, older than 2.56.0, unauthenticated, or the provenance doesn't verify.

Also addressed from review:

  • Windows parityinstall.ps1 got the same opt-in design (it previously had the identical fail-closed bug).
  • No silent foot-gun — a non-1 truthy value (true/yes/on) now enables it too, and an unrecognized value warns instead of silently skipping.
  • Clearer errors — an explicit gh auth status precheck distinguishes "not logged in" from a real provenance mismatch.
  • Docsscripts/install/README.md now describes the opt-in behavior and documents GGSHIELD_REQUIRE_ATTESTATION.

Verified: 8/8 unit cases on the gate logic; earlier end-to-end runs with real gh binaries (default skips without invoking gh; strict dies clearly on missing/old/unauthenticated gh and verifies on 2.95.0); bash -n + shellcheck clean on install.sh. install.ps1 was not lintable locally (no pwsh).

Refs: END-609

@benjaminrigaud-gg benjaminrigaud-gg requested a review from a team as a code owner June 24, 2026 09:57
@linear

linear Bot commented Jun 24, 2026

Copy link
Copy Markdown

END-609

@benjaminrigaud-gg benjaminrigaud-gg force-pushed the benjaminrigaud/end-609-ggshield-installation-failed-through-script-on-macos branch 2 times, most recently from a330f80 to c5c224a Compare June 24, 2026 10:19
@clement-tourriere

Copy link
Copy Markdown
Member

I’m not convinced we should run gh automatically from the installer.

My understanding is that gh is not a ggshield install requirement: it is only used opportunistically for GitHub Artifact Attestation verification, on top of the mandatory SHA256 verification. But invoking an ambient CLI from $PATH makes the installer behavior depend on the user’s local gh version/auth/network state, which is fragile and seems to be the root cause of this bug.

I’d prefer one of these approaches instead:

  • keep SHA256 verification mandatory, but do not run gh by default;
  • document/print the optional manual provenance command, e.g. gh attestation verify <asset> --repo GitGuardian/ggshield;
  • or make attestation verification explicit opt-in/strict mode, e.g. GGSHIELD_REQUIRE_ATTESTATION=1, and then fail clearly if gh is missing, too old, unauthenticated, or cannot verify.

If we keep the current automatic gh call, I think this PR still has two issues to fix:

  1. Removing the gh auth status gate can make users with gh installed but not logged in fail installation. Current gh returns an auth-required error for public attestation lookup, and this output does not appear to be classified as a fall-open verifier problem.
  2. GGSHIELD_REQUIRE_ATTESTATION=1 is currently not strict when gh is missing or has no attestation subcommand, because verify_attestation() returns 0 before checking strict mode.

So my preference would be to remove the automatic dependency on gh from the default install path, and keep attestation as an explicit/manual verification step.

@benjaminrigaud-gg benjaminrigaud-gg self-assigned this Jun 24, 2026
@benjaminrigaud-gg benjaminrigaud-gg force-pushed the benjaminrigaud/end-609-ggshield-installation-failed-through-script-on-macos branch from c5c224a to bc20a9a Compare June 24, 2026 11:14
@benjaminrigaud-gg benjaminrigaud-gg changed the title fix(install): make provenance check best-effort fix(install): make provenance check opt-in Jun 24, 2026
@benjaminrigaud-gg

Copy link
Copy Markdown
Contributor Author

Both points confirmed and fixed, and I took the broader suggestion — provenance is now opt-in.

  • Default no longer invokes gh at all: the install relies on the mandatory sha256 and prints the manual gh attestation verify command. This removes the ambient-$PATH gh dependency behind END-609 (which, as you note, falls open for most users anyway since a public lookup still needs an authenticated gh).
  • GGSHIELD_REQUIRE_ATTESTATION=1 runs the check and fails closed if gh is missing, older than 2.56.0, unauthenticated, or the artifact doesn't verify.

On the two specifics:

  1. Confirmed — a logged-out gh prints To get started with GitHub CLI, please run: gh auth login, which the old classifier didn't match, so dropping the auth gate would have hard-failed those users. Moot now: the default path doesn't run gh, and strict mode fails with an actionable message.
  2. Confirmed — the early return 0 short-circuited strict mode. Strict now checks gh presence/version before that return and fails if either is missing.

Verified end-to-end: default skips without calling gh; strict mode dies with a clear message on missing/old/unauthenticated gh (2.55.0) and verifies on 2.95.0.

@benjaminrigaud-gg benjaminrigaud-gg force-pushed the benjaminrigaud/end-609-ggshield-installation-failed-through-script-on-macos branch 2 times, most recently from 58159d5 to 96e329d Compare June 24, 2026 12:05
gh attestation verify downloads Sigstore's trusted root, which since
Rekor v2 GA contains an Ed25519 (PKIX_ED25519) tlog key that gh builds
older than 2.56.0 cannot parse. The installers ran gh automatically and
failed closed, so an outdated (or unauthenticated) gh on PATH aborted
the whole install even though the sha256 checksum had already matched
(END-609).

gh is not a ggshield dependency: its version/auth/network state is
ambient, and even a public attestation lookup needs an authenticated gh,
so running it by default is fragile and usually cannot run anyway. Make
it opt-in in both install.sh and install.ps1. By default the install
relies on the mandatory sha256 check and prints the manual
`gh attestation verify` command. Set GGSHIELD_REQUIRE_ATTESTATION=1 (or
true/yes/on) to require provenance: it then fails closed if gh is
missing, older than 2.56.0, unauthenticated, or the artifact does not
verify.

Refs: END-609
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@benjaminrigaud-gg benjaminrigaud-gg force-pushed the benjaminrigaud/end-609-ggshield-installation-failed-through-script-on-macos branch from 96e329d to f82ddd4 Compare June 24, 2026 12:40
@benjaminrigaud-gg benjaminrigaud-gg merged commit 3e9ac16 into main Jun 25, 2026
40 checks passed
@benjaminrigaud-gg benjaminrigaud-gg deleted the benjaminrigaud/end-609-ggshield-installation-failed-through-script-on-macos branch June 25, 2026 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants