Automate the headless keyring-probe composition test

.github/workflows/headless-probe-composition.yml

@@ -0,0 +1,324 @@
+name: Headless Keyring Probe
+
+# Composition gate for the #568 incident class: on headless macOS, a keyring
+# probe that blocks must be cut off at the deadline so the command completes
+# through the file fallback instead of hanging forever.
+#
+# Unit tests cover each piece — the bounded probe kills and reaps its child
+# (credstore probe_darwin_test.go:41), the store is lazy until the first
+# credential op, and headless implies a 10s bound. What none of them cover is
+# the composition: the real binary, on real macOS, in a real headless session,
+# against a probe that really blocks.
+#
+# Two environmental facts shape how that is reached.
+#
+# 1. A macos-latest job step is NOT headless. macos-26-arm64 reports
+#    `launchctl managername: Aqua`, so sessionIsHeadless() is false there and
+#    the bounded path never engages. Over `ssh localhost` the session manager
+#    is Background with no TTY, which is not a workaround but the actual #568
+#    scenario: a piped installer over ssh that cannot answer an unlock prompt.
+#    Remote Login is already On in the image; only an authorized_keys entry is
+#    needed.
+#
+# 2. A locked keychain on this image does not block. Measured 2026-08-02:
+#    `security` returns errSecInteractionNotAllowed in under 200ms, so the
+#    cancellation path never ran and the test proved nothing. Rather than hunt
+#    for a real keychain that blocks, substitute a command that reliably does.
+#
+# credstore's securityPath is a package-level var initialized to a constant, so
+# the linker can repoint it at `/usr/bin/caffeinate`, which hangs until killed.
+# No production change and no runtime override — the executable-path trust
+# boundary is unchanged in shipped builds. The keychain is never touched.
+
+on:
+  workflow_call:
+    # The point of the design: release.yml can invoke this against the exact
+    # tag SHA. workflow_dispatch cannot gate a release — a dispatch runs
+    # against the tip of the selected ref, not an arbitrary commit.
+  pull_request:
+    branches: [main]
+    # The whole surface the acceptance path traverses: the store and its
+    # headless detection, the command that reaches it, the app/CLI lifecycle
+    # that builds the store, the entrypoint, the credstore pin (which owns the
+    # securityPath symbol this job injects into), and both workflow files —
+    # self-proving, the same trick installer-bash32 uses at test.yml:249.
+    paths:
+      - 'internal/auth/**'
+      - 'internal/appctx/**'
+      - 'internal/cli/**'
+      - 'internal/commands/auth.go'
+      - 'cmd/basecamp/**'
+      - 'go.mod'
+      - 'go.sum'
+      - '.github/workflows/headless-probe-composition.yml'
+      - '.github/workflows/release.yml'
+  workflow_dispatch:
+
+permissions: {}
+
+env:
+  # Overriding this symbol is what makes the probe block deterministically.
+  # `caffeinate -i` runs until killed, matching how probeBounded invokes
+  # securityPath (`<path> -i`, fed a command on stdin it never reads).
+  SECURITY_PATH_SYMBOL: github.com/basecamp/cli/credstore.securityPath
+  HANGING_COMMAND: /usr/bin/caffeinate
+
+jobs:
+  keychain:
+    name: Bounded probe under a blocking child
+    runs-on: macos-latest
+    timeout-minutes: 15
+    permissions:
+      contents: read
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          persist-credentials: false
+
+      - name: Set up Go
+        uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
+        with:
+          go-version-file: 'go.mod'
+
+      - name: Build the test-instrumented binary
+        run: |
+          # Deliberately NOT called release-shaped: the linker override makes
+          # this a composition gate on the source revision, not a test of the
+          # exact shipped artifact. Everything else about the build matches
+          # goreleaser (-trimpath, no dev tag), so the code path is the
+          # shipped one.
+          CGO_ENABLED=0 go build -trimpath \
+            -ldflags "-X=${SECURITY_PATH_SYMBOL}=${HANGING_COMMAND}" \
+            -o ./bin/basecamp-hanging-probe ./cmd/basecamp
+          ./bin/basecamp-hanging-probe --version
+
+      - name: Assert the linker injection took
+        run: |
+          # The timing floor already catches a failed injection — the real
+          # `security` fast-fails well under 8s — but `-X` against a renamed or
+          # removed symbol is silently ignored by the linker, so assert it
+          # directly to make that failure diagnosable at a glance rather than
+          # arriving as an unexplained fast run.
+          if ! LC_ALL=C grep -qa "$HANGING_COMMAND" ./bin/basecamp-hanging-probe; then
+            echo "::error::${HANGING_COMMAND} is not embedded in the binary — the -X override against ${SECURITY_PATH_SYMBOL} did not apply. The symbol was probably renamed or removed by a credstore bump."
+            exit 1
+          fi
+          echo "Injected ${SECURITY_PATH_SYMBOL} -> ${HANGING_COMMAND}"
+
+      - name: Open a headless session to localhost
+        run: |
+          # Remote Login is already On in the hosted image, so this only needs
+          # an authorized_keys entry. The key is generated per run and never
+          # leaves the runner.
+          mkdir -p ~/.ssh
+          ssh-keygen -t ed25519 -N '' -f ~/.ssh/id_keychain_acceptance -q
+          cat ~/.ssh/id_keychain_acceptance.pub >> ~/.ssh/authorized_keys
+          chmod 700 ~/.ssh
+          chmod 600 ~/.ssh/authorized_keys
+          ssh -i ~/.ssh/id_keychain_acceptance -o BatchMode=yes \
+              -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
+              -o LogLevel=ERROR -o ConnectTimeout=15 localhost true
+
+      - name: Assert the ssh session is headless
+        run: |
+          # sessionIsHeadless() (internal/auth/keyring.go) sets the 10s bound
+          # only when no stream is a terminal AND launchctl reports no Aqua
+          # session. This is the real detection under test — not stubbed — so
+          # assert both in the session that will actually run the command.
+          manager=$(ssh -i ~/.ssh/id_keychain_acceptance -o BatchMode=yes \
+              -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
+              -o LogLevel=ERROR localhost /bin/launchctl managername)
+          echo "ssh session managername: $manager"
+          if [ "$manager" = "Aqua" ]; then
+            echo "::error::ssh session reports an Aqua session — sessionIsHeadless() would be false, the probe would run unbounded through go-keyring, and this test would prove nothing."
+            exit 1
+          fi
+
+          # `tty` exits non-zero when stdin is not a terminal, which is the
+          # answer we want, so it must not trip `set -e`.
+          tty_state=$(ssh -i ~/.ssh/id_keychain_acceptance -o BatchMode=yes \
+              -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
+              -o LogLevel=ERROR localhost tty < /dev/null || true)
+          echo "ssh session tty: $tty_state"
+          if [ "$tty_state" != "not a tty" ]; then
+            echo "::error::ssh session has a TTY ($tty_state) — an interactive session keeps the unbounded probe by design."
+            exit 1
+          fi
+
+      - name: Assert the void-the-test variables are disarmed
+        run: |
+          # Either of these skips the code under test entirely, and would leave
+          # a green check that proved nothing:
+          #   BASECAMP_NO_KEYRING non-empty -> credstore.NewStore returns a
+          #     file store without probing (credstore/store.go:68).
+          #   BASECAMP_TOKEN non-empty -> auth status returns before
+          #     IsAuthenticated(), so the store is never constructed
+          #     (internal/commands/auth.go:61).
+          # Both are checked for non-emptiness because that is exactly what the
+          # production code tests: os.Getenv(...) != "".
+          #
+          # Checked in the ssh session, not the job: the command runs there, and
+          # a login shell profile could set either independently of the job env.
+          state=$(ssh -i ~/.ssh/id_keychain_acceptance -o BatchMode=yes \
+              -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
+              -o LogLevel=ERROR localhost \
+              'echo "no_keyring=[${BASECAMP_NO_KEYRING:-}] token=[${BASECAMP_TOKEN:-}]"')
+          echo "ssh session: $state"
+          if [ "$state" != 'no_keyring=[] token=[]' ]; then
+            echo "::error::BASECAMP_NO_KEYRING or BASECAMP_TOKEN is non-empty in the ssh session — the credential store would be skipped and this test would prove nothing."
+            exit 1
+          fi
+
+      - name: Run a credential-touching command against a blocking probe
+        id: probe
+        run: |
+          SSH="ssh -i $HOME/.ssh/id_keychain_acceptance -o BatchMode=yes"
+          SSH="$SSH -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null"
+          SSH="$SSH -o LogLevel=ERROR localhost"
+
+          # `auth status` reaches IsAuthenticated() -> Store.ensure() ->
+          # credstore.NewStore -> probe -> probeBounded, which execs the
+          # injected hanging command. ssh without -t allocates no TTY and both
+          # streams are redirected, so sessionIsHeadless() holds and the 10s
+          # bound engages.
+          #
+          # The watchdog runs on the remote side so it kills the binary itself
+          # rather than just the ssh client, which would leave the process
+          # behind. perl: alarm() survives exec and SIGALRM's default
+          # disposition is restored across it, so the timer rides on the
+          # binary's own PID. `timeout` is homebrew/coreutils-only and may be
+          # absent; a shell background-kill would race PID recycling.
+          start=$(perl -MTime::HiRes=time -e 'printf "%.3f\n", time')
+          status=0
+          $SSH "perl -e 'alarm(60); exec @ARGV or die \"exec: \$!\"' \
+                  '$GITHUB_WORKSPACE/bin/basecamp-hanging-probe' auth status --json" \
+            > /tmp/keychain-stdout.txt 2> /tmp/keychain-stderr.txt < /dev/null \
+            || status=$?
+          end=$(perl -MTime::HiRes=time -e 'printf "%.3f\n", time')
+          elapsed=$(perl -e 'printf "%.3f\n", $ARGV[1] - $ARGV[0]' "$start" "$end")
+
+          echo "elapsed=${elapsed}s exit=${status}"
+          {
+            echo "elapsed=$elapsed"
+            echo "status=$status"
+          } >> "$GITHUB_OUTPUT"
+
+      - name: Assert the timeout path ran and the bound held
+        env:
+          ELAPSED: ${{ steps.probe.outputs.elapsed }}
+          STATUS: ${{ steps.probe.outputs.status }}
+        run: |
+          # The floor is what makes this a test rather than a ritual, and it is
+          # self-proving across both seams. If headless detection breaks, the
+          # store takes the unbounded probeDirect path through go-keyring and
+          # the real `security` returns in ~0.2s. If the -X injection breaks,
+          # probeBounded execs the real `security`, which also returns in
+          # ~0.2s. Either way the run lands under the floor and goes red — a
+          # broken harness cannot masquerade as a passing test.
+          #
+          #   >= 8s   the probe really blocked and the bound cut it off
+          #   ~0.2s   harness broken (see above) — proves nothing
+          #   <= 25s  the bound held (10s probe + startup + ssh round-trip)
+          #   60s     watchdog fired — #568 regression
+          echo "elapsed=${ELAPSED}s exit=${STATUS}"
+
+          # ssh reports a signalled remote command as exit 255, so accept
+          # either that or the direct 128+SIGALRM.
+          if [ "$STATUS" = "142" ] || [ "$STATUS" = "255" ]; then
+            echo "::error::Watchdog fired at 60s (SIGALRM) — the command never returned. The probe was not cut off at its deadline. This is the #568 hang."
+            exit 1
+          fi
+
+          under_floor=$(perl -e 'print(($ARGV[0] < 8.0) ? 1 : 0)' "$ELAPSED")
+          if [ "$under_floor" = "1" ]; then
+            echo "::error::Completed in ${ELAPSED}s, under the 8s floor — the probe never blocked, so the cancellation path never ran and this check proves nothing. Check the headless assertion and the linker injection. Do not weaken the floor to make it green."
+            exit 1
+          fi
+
+          over_ceiling=$(perl -e 'print(($ARGV[0] > 25.0) ? 1 : 0)' "$ELAPSED")
+          if [ "$over_ceiling" = "1" ]; then
+            echo "::error::Took ${ELAPSED}s, past the 25s ceiling — the headless probe bound did not hold."
+            exit 1
+          fi
+
+          if [ "$STATUS" != "0" ]; then
+            echo "::error::auth status exited $STATUS — it did not complete through the file fallback."
+            exit 1
+          fi
+
+      - name: Assert no blocking probe process survived
+        run: |
+          # A deadline that leaves the blocking child running still leaks a
+          # process per invocation, so assert none outlives the CLI.
+          #
+          # This proves NO PERSISTENT CHILD SURVIVED. It does NOT prove the CLI
+          # parent reaped its own child, and must not be read that way: had the
+          # parent killed without waiting, the child would be a zombie,
+          # reparented to PID 1 when the CLI exits, and likely reaped by init
+          # before this pgrep runs — so this check would still pass. The
+          # authoritative parent-side reaping proof is credstore's darwin unit
+          # test, which pins ESRCH (probe_darwin_test.go:41). A composition
+          # test could only make the stronger claim by holding the CLI parent
+          # alive while inspecting child state, or by exposing a test-only wait
+          # result; neither is warranted for a release gate.
+          #
+          # This is a gate, not a diagnostic. It deliberately does not live in
+          # the `if: always()` diagnostics step below, where `|| echo` would
+          # print a survivor and let the release gate stay green.
+          #
+          # Matched on the exact argv the probe spawns (`<path> -i`, anchored)
+          # rather than a bare substring, so an unrelated caffeinate elsewhere
+          # on the runner cannot fail this, and a real survivor cannot hide
+          # behind a loose match. CommandContext kills synchronously inside
+          # cmd.Run() before the binary exits, so there is no race to sleep
+          # around.
+          if survivors=$(pgrep -fl "^${HANGING_COMMAND} -i$"); then
+            echo "::error::A blocking probe process outlived the CLI — the child was not killed at the deadline. This is the #568 process-leak class."
+            printf '%s\n' "$survivors"
+            exit 1
+          fi
+          echo "No ${HANGING_COMMAND} process survived the CLI. (Parent-side reaping is pinned separately by credstore's darwin unit test.)"
+
+      - name: Assert the command completed through the fallback
+        run: |
+          # Exit 0 alone would not prove the command ran to completion. The
+          # envelope does. No credentials are seeded: once the keyring is
+          # unavailable, reporting not-authenticated *is* the success path.
+          cat /tmp/keychain-stdout.txt
+          if ! grep -Eq '"ok"[[:space:]]*:[[:space:]]*true' /tmp/keychain-stdout.txt; then
+            echo "::error::JSON envelope is not ok:true"
+            exit 1
+          fi
+          if ! grep -Eq '"authenticated"[[:space:]]*:[[:space:]]*false' /tmp/keychain-stdout.txt; then
+            echo "::error::Expected authenticated:false in the envelope"
+            exit 1
+          fi
+
+      - name: Diagnostics
+        if: always()
+        env:
+          ELAPSED: ${{ steps.probe.outputs.elapsed }}
+          STATUS: ${{ steps.probe.outputs.status }}
+        run: |
+          echo "elapsed: ${ELAPSED:-<not measured>}"
+          echo "exit code: ${STATUS:-<not measured>}"
+          echo "--- launchctl managername (job step) ---"
+          /bin/launchctl managername 2>&1 || true
+          echo "--- launchctl managername (ssh session) ---"
+          ssh -i ~/.ssh/id_keychain_acceptance -o BatchMode=yes \
+              -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \
+              -o LogLevel=ERROR localhost /bin/launchctl managername 2>&1 || true
+          echo "--- injected securityPath ---"
+          LC_ALL=C grep -oa "$HANGING_COMMAND" ./bin/basecamp-hanging-probe | head -1 || echo "<not embedded>"
+          # Reporting only — NOT the gate. The binding assertion is the
+          # "Assert no blocking probe process survived" step above, which exits
+          # non-zero on a match. This duplicate exists because diagnostics run
+          # `if: always()`, so it still reports child state when the job failed
+          # earlier and never reached that assertion. Do not add `exit 1` here
+          # expecting it to close a hole; the hole is already closed.
+          echo "--- stray hanging children (reporting only; gated above) ---"
+          pgrep -fl "^${HANGING_COMMAND} -i$" || echo "<none>"
+          echo "--- stdout ---"
+          cat /tmp/keychain-stdout.txt 2>/dev/null || echo "<none>"
+          echo "--- stderr ---"
+          cat /tmp/keychain-stderr.txt 2>/dev/null || echo "<none>"

.github/workflows/release.yml

@@ -20,6 +20,21 @@ jobs:
       security-events: write
       pull-requests: read
 
+  # Invoked here rather than merely run beforehand: scripts/release.sh commits
+  # and pushes a release-prep commit to main *before* tagging, so anything run
+  # ahead of time covers the wrong SHA. As a called workflow this runs against
+  # the exact tag SHA.
+  #
+  # contents: read is required and cannot be inherited — this workflow sets
+  # permissions: {} above, and a called workflow can only maintain or reduce
+  # the caller's token permissions, never elevate them. Without it,
+  # actions/checkout inside the reusable workflow has no repository access.
+  headless-probe:
+    name: Headless keyring probe
+    uses: ./.github/workflows/headless-probe-composition.yml
+    permissions:
+      contents: read
+
   test:
     name: Test before release
     runs-on: ubuntu-latest
@@ -103,7 +118,7 @@ jobs:
 
   release:
     name: Release
-    needs: [test, security]
+    needs: [test, security, headless-probe]
     runs-on: ubuntu-latest
     timeout-minutes: 45
     environment: release