Bound the keyring probe on headless sessions

Merged
JE
3mo ago
+134-5

Completes the #568 keychain-hang lineage: after lazy store construction (#578) fixed credential-free commands, credential-touching commands on a locked headless keychain still blocked unbounded. basecamp/cli#56 added the upstream prerequisite (StoreOptions.ProbeTimeout — a bounded probe whose darwin implementation actually kills the hung security child and falls back to file storage). This PR bumps to that credstore (pseudo-version of the merge commit; no release tag exists yet) and wires it.

Interactivity-aware bound

  • Headless (stderr not a terminal): probe bounded at 10s. A headless session can never answer a keychain unlock prompt, so the only prior outcomes were an infinite hang or the manual BASECAMP_NO_KEYRING=1 escape hatch. Now: bounded probe → file fallback with the existing plaintext warning.
  • Interactive: unbounded probe, exactly as today. A locked keychain in an interactive session raises an unlock prompt the user may legitimately take longer than any reasonable timeout to answer — cutting it off would silently degrade them to plaintext file storage. Not a regression: interactive sessions were never the incident class.
  • Stderr, not stdout, as the signal: interactive use routinely pipes stdout (| jq) while prompts still reach the terminal.

ForceFile is deliberately not wired — credstore's internal timeout-to-file fallback covers the CLI's need, and BASECAMP_NO_KEYRING already provides forced file mode.

Tests

TestEnsureBoundsProbeOnlyWhenHeadless pins both branches through the existing newCredStore seam plus a new stderrIsTerminal seam (precedent: stdoutIsTerminal in update_notice.go). Existing laziness test unchanged.

Verification

  • bin/ci green; go test -race ./internal/auth/... green.
  • Behavioral smoke on macOS: piped (headless-path) auth status probes within the bound against a healthy keychain and loads keyring credentials (expired: false; the file fallback holds a stale token, distinguishing the paths).
  • Upstream credstore at the pinned commit: eleven-round review convergence on basecamp/cli#56, macOS -race -count=2 proof at merge head.

Refs #568


Summary by cubic

Bound the keyring probe to 10s only in fully headless sessions (no TTYs and no GUI) and fall back to file storage to prevent hangs on locked keychains. Interactive or GUI-launched sessions keep the unbounded probe. Addresses #568.

  • Bug Fixes

    • Treat sessions as headless only when stdin, stdout, and stderr are all non-terminals and no GUI is available (macOS: launchctl managername != Aqua; Linux/*BSD: no DISPLAY/WAYLAND_DISPLAY; Windows: GUI check is always false since Credential Manager never prompts); set ProbeTimeout to 10s so credstore falls back to file storage.
    • Keep the probe unbounded when any stdio is a terminal or a GUI is present to allow unlock prompts.
  • Dependencies

    • Bump github.com/basecamp/cli to v0.2.2-0.20260728023309-04e401b12c6c to use credstore.StoreOptions.ProbeTimeout.