feat(install): Codex opt-out and credential-free attestation verification - #1197
Conversation
…tion Implements both asks from #1178 (reported and designed by @astradevkin): - Per-agent installer opt-outs: --skip-codex / --skip-gemini / --skip-kiro flags, PLANNOTATOR_SKIP_{CODEX,GEMINI,KIRO}_INSTALL env vars, and config.json skipInstall.{codex,gemini,kiro} keys, with flag > env > config precedence mirroring verifyAttestation. Detected-but-skipped is reported as its own honest state, never conflated with not-detected, and a skipped agent's home is neither written nor cleaned up. - Credential-free provenance verification: when --verify-attestation is active, the Sigstore bundle is fetched from GitHub's public attestations endpoint (single unauthenticated attempt, no retry) and verified via gh attestation verify --bundle with the same --repo/--source-ref/--signer-workflow constraints; gh's authenticated fetch remains the fallback. TUF trust-root failures are reported as connectivity, distinct from real provenance failures; every path stays fail-closed. Zero behavior change for users who do not opt in: the default install path is unchanged (verified by sandbox-HOME parity runs against main).
Review (at
|
- H1: a failed gh --bundle invocation now retries once through the exact authenticated path before any classification, so an older gh (unknown flag) or a corrupt bundle never reports as a provenance failure. Pinned by a functional stub-gh test; a real failure still fails again on the retry and aborts. - M2: the sh config layer extracts the skipInstall object (awk, character indexed) before matching per-agent keys and honors explicit false as a veto; cmd now parses the real JSON via PowerShell like ps1. Functional tests cover the foreign-key collision and explicit-true cases. - M3: sh names the real cause when the bundle path cannot run (no JSON extractor vs fetch vs extraction failure) and gains python3 and jq fallback extractors; docs state the dependency. - M4: ps1/cmd gate the existing-integration note on plannotator content in hooks.json and word the skip state as what those platforms actually do (manual instructions suppressed). - M5: sh bundle lives inside a private mktemp -d, one rm -rf on every exit, and a mktemp failure degrades to the fallback instead of aborting. - M6: README, verifying-your-install, environment-variables, and installation docs updated for the credential-free path and skip flags. - M7: ps1/cmd extraction replaced with a byte-exact string scanner (no ConvertFrom/ConvertTo round trip, immune to DateTime coercion), with PowerShell-driven unit tests over the captured real attestations response plus synthetic DateTime and brace-in-string controls. - Lows: fail-closed abort pinned by a functional test; TUF beats auth in cmd classification (matches sh/ps1); skipped-state output mentions the shared ~/.agents/skills; Gemini summary is skip-aware and gains an honest not-detected state; --skip-opencode do-not-write switch added (flag, env var, config key) across all three scripts.
Delta re-review (at
|
…oof fail-closed test, lows) - R1: install.cmd's attestation fetcher no longer touches disk. The %RANDOM%-named %TEMP% .ps1 (predictable-path code execution, the M5 class escalated) is replaced by powershell -NoProfile -EncodedCommand with a base64(UTF-16LE) payload defined next to its full REM PS: plaintext; a test decodes the blob and asserts byte equality with the documented lines plus the security-relevant shape (env-var inputs, ordinal scan, no JSON round trip, distinct exit codes). Inputs still travel via env vars. Verified end to end under pwsh: the decoded blob fetched the real attestations response, wrote 2 bundles, gh verified the real v0.25.1 binary credential-free (exit 0) and rejected a wrong binary (exit 1). - R2: the fail-closed test now asserts the output ENDS with 'Refusing to install.' - mutation-verified: with the verify-failure exit 1 deleted the mutant still exits 1 via an incidental mv failure, but the trailing mv error breaks the endsWith and the test fails; restored, it passes. - Lows: CI guard test fails loudly when process.env.CI is set and no pwsh/powershell is on PATH (scanner coverage cannot silently vanish); scanner IndexOf calls are ordinal in ps1 and the encoded cmd variant; the awk skipInstall extraction requires optional-whitespace-then-colon- then-brace after the key (string values can no longer anchor it, with non-token occurrences skipped, unit-checked against escaped-embedded payloads); install.cmd comments warn that the fallback-reason literals inside parenthesized blocks must stay parenthesis-free.
Final delta (at
|
TLDR: Closes #1178. (Updated through review round three: the cmd fetcher now runs via
-EncodedCommandwith no temp file on disk, and the fail-closed test is mutation-proof.) The installers gain explicit opt-outs for the Codex, Gemini, Kiro, and OpenCode integrations (flag, env var, config key) with honest three-state reporting, and provenance verification now fetches the attestation bundle from GitHub's public attestations endpoint and verifies withgh attestation verify --bundle, so it works with zero credentials; gh's authenticated fetch stays as fallback. Users who do not opt in see zero change in files or actions. All findings from the adversarial review round are addressed in the second commit.Both designs are @astradevkin's, straight from the issue: the skip-var naming and detection-as-consent critique for the Codex opt-out, and the discovery that the public attestations endpoint serves the exact bundle the authenticated path fetches, including the 60/hr per-IP rate-limit note (single fetch attempt, never retried) and the observation that the Sigstore TUF trust root is fetched on every run and deserves its own failure message. Thank you for the unusually rigorous report, including the five negative controls.
Feature 1: per-agent install opt-outs
New in all three scripts (
install.sh,install.ps1,install.cmd):--skip-codex/-SkipCodex,PLANNOTATOR_SKIP_CODEX_INSTALL=1, and{ "skipInstall": { "codex": true } }in~/.plannotator/config.json. Precedence: flag over env var over config. The env var also accepts0/false/noto override a config-enabled skip for one run.Codex: detected, skipped (--skip-codex)(the parenthetical names whichever source triggered the skip),Codex was not detected., and the existing installed output. When a previous install did wire Codex and this run skips, one line notes the existing integration is unaffected.~/.agents/skillsserve multiple agents and are still installed.install.shextracts theskipInstallobject itself (character-indexed, works for single-line JSON) before matching per-agent keys and honors an explicitfalseas a veto;install.cmdparses the real JSON via PowerShell exactly likeinstall.ps1. A"codex": trueunder any other config key can never opt anyone out.hooks.jsonactually references plannotator, never on mere file existence (review M4).Mirrored opt-outs: Gemini (
--skip-gemini), Kiro (--skip-kiro), and, per the issue's original ask, OpenCode (--skip-opencode). OpenCode has no detection leg, so its opt-out is a plain do-not-write switch (command stubs, plugin cache clear, stale-stub sweep) with a skip note rather than detected/not-detected reporting. The Gemini summary section is now also honest when Gemini is not detected.PlannotatorConfiggains theskipInstallfield (installer-read only), and AGENTS.md, the README, and the marketing installation, environment-variables, and verifying-your-install pages document everything (review M6).Feature 2: credential-free attestation verification
Active only under the existing opt-in (
--verify-attestation/PLANNOTATOR_VERIFY_ATTESTATION/ configverifyAttestation), which stays off by default:https://api.github.com/repos/backnotprop/plannotator/attestations/sha256:<digest>unauthenticated. Single fetch attempt, no retry loop (the anonymous API allows 60 requests/hour per IP)..attestations[].bundleinto a JSONL file, the formatgh attestation downloadwrites. Dependency note: on macOS/Linux the extraction needs one JSON tool on PATH: node, then python3, then jq, whichever is present (review M3); with none of the three the installer says so by name and falls back, it never blames a fetch that did not run. On Windows, PowerShell extracts each bundle as a byte-exact substring of the raw response via a string-literal-aware brace scanner, deliberately not aConvertFrom-Json/ConvertTo-Jsonround trip, whose DateTime coercion re-serializes date-shaped strings differently across PowerShell 5.1 and 7 and could corrupt a bundle field (review M7 guard).gh attestation verify <file> --bundle <bundle.jsonl>with the unchanged--repo+--source-ref+--signer-workflowconstraints. The sh bundle file lives inside a privatemktemp -ddirectory, removed on every exit path; a mktemp failure degrades to the fallback instead of aborting (review M5).--bundle, or a corrupted bundle, therefore ends in a working install or an accurate environment error, never a false "no valid signed provenance". A real provenance failure fails again on the retry and aborts.Failure messages are distinct and aligned across all three scripts (TUF beats auth in classification precedence everywhere), all fail-closed: real provenance failure (loud, aborts), Sigstore trust root unreachable (worded as connectivity; the TUF root is fetched per-run), and bundle-path-incomplete-plus-no-login (environment error naming the recovery options). The gh CLI is still required for the verify itself; only the login requirement is dropped.
Verification transcripts
macOS end-to-end (
GH_CONFIG_DIRat an empty dir,GH_TOKEN/GITHUB_TOKENcleared; gh reports "not logged into any GitHub hosts"):plannotator-darwin-arm64from v0.25.132d28fac...GET .../attestations/sha256:<digest>unauthenticatedgh attestation verify --bundlewith full constraints, zero credentialsinstall.sh --verify-attestationrun in a sandbox HOME, gh unauthenticatedverified build provenance (SLSA, credential-free via the public attestations API)Windows status: no Windows environment was reachable during this round, stated plainly. As the substitute: (a) the exact scanner region of
install.ps1, and the scanner decoded frominstall.cmd's-EncodedCommandpayload, are exercised by new PowerShell-driven unit tests against a captured real attestations response committed atscripts/fixtures/attestations-response.json(bundle_url values sanitized, every other byte as served), asserting each emitted bundle is a byte-exact substring of the response and parses to the exact bundle object, plus synthetic DateTime-string and braces-inside-strings controls; and (b) theinstall.ps1bundle path (its literal fetch shape plus its literal scanner region) was run end-to-end under PowerShell 7.5.4 on macOS with zero credentials against the real endpoint and real v0.25.1 binary: fetch 200, 2 bundles extracted, verify exit 0, wrong-signer-workflow and wrong-binary negative controls exit 1. Windows PowerShell 5.1 remains untested end-to-end; the scanner uses only .NET string APIs available on 5.1, and any 5.1-specific failure degrades through the H1 retry to the authenticated path rather than a false provenance alarm.Zero-default-change guarantee
Users who pass no new flag and set no new env var or config key get the behavior of main, verified two ways: statically (skips default off; all attestation changes live inside the off-by-default verify branch) and dynamically (main's
install.shand this branch's run in fresh sandbox HOMEs with identical inputs produce identical file trees, identical Codexconfig.tomlhashes, and identical output) with one deliberate, review-requested exception: on machines without~/.gemini, the final Gemini section now honestly prints "Gemini was not detected" instead of generic usage instructions. Files written and actions taken are identical everywhere; machines with~/.geminisee unchanged output. Sandbox runs also re-confirmed the skip matrix: flag/env/config each honored and named as the source, env0overriding configtrue, flag beating env, and a pre-seededhooks.jsonsurviving a skip run hash-identical.Tests
bun test scripts/install.test.ts: 110 pass. New since the review: four functional tests runninginstall.shagainst a sandbox HOME with stubbed curl/gh/git (H1 fallback engages and the provenance message does not print; a real verify failure exits 1 and installs nothing, pinning the fail-closed abort the reviewer proved untested; the M2 foreign-key collision installs instead of skipping;skipInstall.codex: trueskips and names the config), and six PowerShell-driven scanner tests (two scanner sources times three cases) that skip cleanly when no PowerShell is on the host.bun test: 2868 pass, 0 fail.bash -npasses;install.ps1parses clean under the PowerShell 7.5.4 AST parser. shellcheck remains unavailable on the dev machine.Review round three (at cf595e9)
install.cmdno longer writes any fetch helper to disk: the previous%TEMP%\plannotator-attfetch-%RANDOM%.ps1(a predictable-path code-execution vector, the M5 class escalated) is replaced bypowershell -NoProfile -EncodedCommandwith the payload defined next to its fullREM PS:plaintext. A test decodes the blob and asserts byte equality with those documented lines plus the security-relevant shape, so the blob cannot drift from what a reviewer reads. Inputs still travel via env vars. The decoded payload was run end to end under pwsh against the real endpoint: 2 bundles written, gh verified the real v0.25.1 binary with zero credentials (exit 0), wrong-binary control failed (exit 1).exit 1deleted, the mutant still exits nonzero through an incidentalmvfailure, but the trailingmverror breaks the assertion and the test fails; restored, it passes.CIis set and no PowerShell is on PATH (the scanner coverage cannot silently skip away); the scannerIndexOfcalls are ordinal in both variants; the awkskipInstallextraction requires optional-whitespace-then-colon-then-brace after the key so string values cannot anchor it (non-token occurrences are skipped; unit-checked against escaped-embedded payloads);install.cmdcomments warn that the fallback-reason literals inside parenthesized blocks must stay parenthesis-free.bun test scripts/install.test.ts113 pass / 0 fail with PowerShell on PATH, 107 pass / 6 skip / 0 fail without; fullbun test2871 pass / 0 fail;bash -nand the PS 7.5.4 AST parse clean.Release note
Merging re-opens the S3 install-script sync at release time so the hosted
plannotator.ai/install.sh|ps1|cmdcopies pick up these changes.