feat(install): add auto-selecting curl|sh installer + streamline README - #387
feat(install): add auto-selecting curl|sh installer + streamline README#387mogul wants to merge 1 commit into
Conversation
9c86028 to
a160f6c
Compare
a160f6c to
48296c9
Compare
Add install.sh: a no-sudo, no-admin front door that auto-selects an install method (brew -> npm -> git clone) so non-technical users need not choose. The clone fallback is fully functional (shallow clone to ~/.local/share/acq, consent-gated PATH edit, idempotent); the npm path is wired via package.json bin/files; the brew path is stubbed pending a published homebrew tap. Support optional commit-SHA pinning (--sha / ACQ_INSTALL_SHA): the clone method checks out the pinned commit and verifies HEAD matches it, failing closed and cleaning up on mismatch. Git objects are content-addressed, so a matching SHA is itself an integrity check. Wire release-please (extra-files, $.version) to bump package.json's version on release so the npm package tracks the release manifest instead of a hand-maintained placeholder. Proactively trigger and explain the macOS Command Line Tools install (the git dependency is unavoidable at acq runtime), noting the pop-up may hide in the Dock. Fail closed when a requested --ref cannot be checked out, cleaning up any partial clone. Streamline the README quickstart to three steps for a non-technical audience and record the decision in docs/adr/0026-installation-and-distribution.md. Co-authored-by: OpenCode [claude-opus-4] <bret.mogilefsky@gsa.gov>
48296c9 to
41e79df
Compare
wz-gsa
left a comment
There was a problem hiding this comment.
Adversarial review — one blocking finding, two non-blocking
Blocking: the default install is unpinned, contradicting the script's and ADR's own security claims
install.sh:
# Pin to a release tag by default so `curl | sh` never runs a moving target.
# Override with --ref for testing a branch or specific tag.
REF="${ACQ_INSTALL_REF:-main}"The comment says "pin to a release tag by default." The code's default is main — a moving target, the opposite of what the comment claims.
This isn't a stale comment on a minor path — it's the headline, README-recommended install method this PR is built around:
curl -fsSL https://raw.githubusercontent.com/GSA-TTS/agentic-coding-quickstart/main/install.sh | sh
Note the URL itself already fetches the script from main; the script then clones the repo at main too (or, on the npm path, npm install -g github:GSA-TTS/agentic-coding-quickstart#main — REF feeds spec unconditionally whenever non-empty, so the same unpinned default reaches all three install methods, not just clone). Nothing in the README's quickstart flow tells a user to pass --ref <tag> or --sha <commit> — those exist, but they're opt-in, not the default.
The ADR states the opposite of what the code does:
"Our own
install.shand theacqclone are pinned; this carve-out applies only to the third-party msb bootstrap."
That's only true if a user overrides REF. As shipped, install.sh's own default is exactly as unpinned as the third-party msb installer it explicitly calls out as the exception — the Control Mapping cites SR-3 (Supply Chain Controls) and SR-11 (Component Authenticity) for a property the default path doesn't provide. release-please-config.json's change doesn't rewrite REF's default at release time either, so there's no mechanism that flips this to a real tag automatically.
Suggested fix: either (a) default REF to a real pinned tag (bump it as part of the release process, the way release-please-config.json's new extra-files entry already keeps package.json's version in sync — the same mechanism could point at this script or a generated constant), or (b) if main is genuinely intended as the default (rolling-release philosophy), fix the comment and the ADR to say so honestly rather than claim a pinning guarantee that isn't there. Either is fine; the current mismatch between claim and code is not.
Non-blocking: ensure_git's Command Line Tools wait has no non-interactive bypass
ensure_git unconditionally runs xcode-select --install and polls for up to 30 minutes when git isn't usable — with no ASSUME_YES/--yes check. --yes is documented as "Intended for non-interactive/CI use," but on a git-less macOS runner this launches a GUI dialog nothing can click and then blocks the job for up to 30 minutes before finally dying with a timeout message, rather than failing fast. Worth gating the wait (or skipping straight to the actionable die message) when ASSUME_YES=1 or stdin isn't a TTY.
Verified as sound (not flagged)
- The
--shaintegrity check is real: content-addressedgit rev-parse HEADcomparison,rm -rf+ fail-closed on mismatch, correctly restricted to the clone method (npm/brew reject--sha). confirm()fails closed (no way to ask → decline) and correctly never edits PATH without consent.git_is_usableprobes by running git, not justcommand -v— correctly catches the macOS shim-without-CLT-installed case.- No
sudo, no writes outside$HOME, npm PATH-detection viacommand -vafternpm install -gis not a race (POSIXshdoesn't hash between commands the way an interactive login shell does). - The Homebrew stub path is honestly labeled and safely falls through to the functional clone method rather than silently no-op'ing.
All 4 CI checks green; this is a review-blocker on the security-claim mismatch, not the implementation quality, which is otherwise careful.
AI-assisted (OpenCode).
Context
Onboarding surfaced two recurring pains for non-technical users (see the ADR's "Observed evidence" section):
./acq-from-the-wrong-folder trap — running./acqfrom a project/home dir yields "no such file", with no obvious recovery. Multiple users hit it.npm/brewdead-end on a bare Mac.This PR gives
acqa real install path that puts it onPATH.What's here
install.sh— one hardenedcurl | shfront door that auto-selects the best method already on the host: Homebrew → npm → managed git clone.package.jsonbin/files; installs placeacqon PATH and the launcher resolves itsacq.backends/tree through the npm symlink).~/.local/share/acq, symlink →~/.local/bin/acq, idempotent, nosudo. Fails closed if a requested--refcan't be checked out, cleaning up any partial clone.--sha/ACQ_INSTALL_SHA): checks out a full 40-char commit and verifiesHEADmatches it, failing closed (and cleaning up) on a missing/mismatched SHA — git objects are content-addressed, so a matching SHA is itself an integrity check.xcode-select --install(no admin), guides the user to the dialog (incl. "look in your Dock"), and waits until git is usable (5s poll, 30-min cap → actionable timeout).--method brew|npm|clone,--ref,--sha,--no-msb,--dry-run,--yes/-y.release-please-config.json— anextra-files($.version) entry so release-please bumpspackage.json'sversionon release;package.jsonnow carries the real manifest version (2.0.0) instead of a placeholder.<details>; new troubleshooting for the wrong-folder trap and CLT/Dock.docs/adr/0026-installation-and-distribution.md,status: accepted) — the design rationale, incl. why the hash-confirmed tarball was rejected (git/CLT is needed at runtime anyway; a full commit-SHA checkout is itself content-addressed integrity), and a carve-out noting the msb leg is an unpinned upstreamcurl | shoutside our control.Deferred (to be filed as issues after ADR sign-off)
GSA-TTS/homebrew-tap+acqformula → un-stub brew branch. (External repo — cannot be created from this repo.)SHA256SUMSand a canonical commit SHA per release; then swap README URLmain→ pinned tag, and default the installer to that SHA (the--shamechanism is already in place).Verification transcript
Run in the acq sandbox (Linux; macOS-specific CLT path exercised via stubs +
--dry-run):shellcheck --severity=warning install.shmarkdownlint-cli2(our files)scripts/test-acq-bats(bats-core, per ADR-0025)gitleaks protect --stagednpm pack --dry-runacq+acq.backends/(7) +package.json/README/LICENSE (11 files)npm install -g(local)acq, launcher resolves through symlink--method clone --yes(real, local repo)--ref(clone)--sha <matching>(real, local repo)--sha <well-formed but absent>--sha/--shawith--method npmrelease-pleaseversion syncnpm packreports manifest version2.0.0(no placeholder)--method bogusRollback
Revert this branch/PR.
install.shis additive and the README/ADR are docs; nothing inacq's runtime dispatch changed.Security impact
No auth/authz changes. Installer uses no
sudo, writes only under\$HOME, gates PATH edits and msb install on consent, fails closed on an unresolvable--ref, and (once releases publish it) will verify integrity via a pinned commit SHA. The msb leg delegates to msb's own upstream installer, which is unpinned and outside our control (documented in the ADR).AI-assisted (OpenCode). Human owner: @mogul.
Testing this PR before merge
The README one-liner points at
main, which won't haveinstall.shuntil this merges. To try it now, run it from a checkout of this branch. Safe (non-destructive) checks that never touch your real~/.localor PATH:For a real, non-throwaway install on macOS, just run
sh install.sh(auto-selects a method, asks before editing PATH or installing msb). On a bare Mac it will trigger the Command Line Tools dialog — accept it (look in your Dock if you don't see the window).