From 6fe8fda3c8abe9d6ffde72951ad544de64edd0a8 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Wed, 16 Sep 2026 19:23:24 +0500 Subject: [PATCH 1/2] fix(npm): let npm run its 2FA prompt when reading trust state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `trust_state` captures npm's output with `$(...)`, but npm's trust commands need an INTERACTIVE 2FA challenge: they print "Authenticate your account at: " and wait on the terminal. Captured stdout swallows that, npm fails with EOTP, and the script correctly reports `unknown` and stops. It did exactly that against the real registry on a freshly logged-in session, twice; the same command uncaptured succeeded immediately. The classification was right and the plumbing was wrong. On `unknown` the state is now re-read once through an UNCAPTURED call, so the handshake happens with stdio attached. A warm session prompts for nothing. Extracted as `trust_state_interactive` so the harness can drive it. Inline in the caller it was untestable, and that mattered: a mutation deleting the retry passed all 28 fixtures, because they stub npm out entirely and the interaction between command substitution and npm's prompt is invisible to them. Two cases now cover it — a fake npm that fails its captured reads and answers afterwards, and a warm one that answers immediately — asserting both the classification AND whether the retry ran, since "granted either way" cannot tell a working retry from one that always prompts. Both mutants are caught. The call counter lives in a file, because `$(...)` is a subshell and the first version of the test saw the retry happen and still read `unknown`. Also pins the first fixture captured from a REAL registry rather than written to match the parser: verbatim `npm trust list` output for @bugsee/cli-win32-arm64. Every other case proves the parser self-consistent; this one proves it agrees with npm. And renames a local `status`, which is read-only in zsh — harmless under the bash shebang, free to avoid. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- npm/README.md | 7 ++++ npm/bootstrap-names.sh | 38 ++++++++++++++++--- npm/test-bootstrap-names.sh | 74 +++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 6 deletions(-) diff --git a/npm/README.md b/npm/README.md index 725c79b..a886c71 100644 --- a/npm/README.md +++ b/npm/README.md @@ -136,6 +136,13 @@ What it does per name, and why: expired session must never be read as "unconfigured", or the script ends up advising you to revoke a perfectly good entry. + On `unknown` the script retries once with an UNCAPTURED `npm trust list`. + npm's trust commands need an interactive 2FA challenge — it prints + "Authenticate your account at: …" and waits on the terminal — and a captured + stdout swallows that prompt, so npm fails with `EOTP`. Follow the prompt when + it appears; a session already warm from an earlier command prompts for + nothing. + `--allow-publish` requires npm >= 11.15. An OLDER npm accepts the command without it and creates an entry carrying no publish permission — which looks configured and still 404s on release day. The script detects that and routes diff --git a/npm/bootstrap-names.sh b/npm/bootstrap-names.sh index 43a3c99..6c1d3b1 100755 --- a/npm/bootstrap-names.sh +++ b/npm/bootstrap-names.sh @@ -87,15 +87,39 @@ fi # only ever be tested against fixtures invented to match it — that proves the # parser self-consistent and nothing about npm. The text path remains as a # fallback for an npm whose --json is missing or empty. +# `trust_state` with one uncaptured retry, which is what makes it work at all on a +# cold session. +# +# `trust_state` captures npm's output, and npm's trust commands need an +# INTERACTIVE 2FA handshake: npm wants to print "Authenticate your account at: +# " and wait on the TERMINAL. A captured stdout swallows that, so npm fails +# with EOTP and `trust_state` correctly reports `unknown`. One UNCAPTURED call +# lets the handshake happen with stdio attached; the session then covers the +# captured reads. Only done when needed, so a warm session prompts for nothing. +# +# A separate function ONLY so the test harness can drive it: the fixtures stub npm +# out, so nothing about this retry was observable while it sat inline in the +# caller — and a mutation removing it passed all 28 of them. +trust_state_interactive() { + local pkg="$1" state + state="$(trust_state "$pkg")" + if [ "$state" = unknown ]; then + echo " auth: npm needs an interactive 2FA challenge — follow its prompt" + $TRUST_NPM trust list "$pkg" || true + state="$(trust_state "$pkg")" + fi + printf '%s' "$state" +} + trust_state() { - local pkg="$1" out status + local pkg="$1" out rc out="$($TRUST_NPM trust list "$pkg" --json 2>/dev/null)" - status=$? - if [ "$status" -ne 0 ] || [ -z "$out" ]; then + rc=$? + if [ "$rc" -ne 0 ] || [ -z "$out" ]; then out="$($TRUST_NPM trust list "$pkg" 2>/dev/null)" - status=$? - [ "$status" -ne 0 ] && { echo unknown; return 0; } + rc=$? + [ "$rc" -ne 0 ] && { echo unknown; return 0; } printf '%s' "$out" | awk -v WORKFLOW="$WORKFLOW" -v REPO="$REPO" ' # LINE-oriented, with an entry boundary at a REPEATED key. Do not assume # npm separates entries with a blank line: if it does not, a paragraph @@ -241,7 +265,9 @@ JSON fi # 2. Attach the trusted publisher, so npm-publish.yml's OIDC can publish it. - case "$(trust_state "$pkg")" in + state="$(trust_state_interactive "$pkg")" + + case "$state" in granted) echo " trust: already grants publish for $WORKFLOW" ;; diff --git a/npm/test-bootstrap-names.sh b/npm/test-bootstrap-names.sh index c080528..df3cf7b 100755 --- a/npm/test-bootstrap-names.sh +++ b/npm/test-bootstrap-names.sh @@ -45,6 +45,8 @@ TRUST_NPM="fake_npm" # shellcheck disable=SC1090 eval "$(awk '/^trust_state\(\) \{/,/^\}/' "$SCRIPT_UNDER_TEST")" +# shellcheck disable=SC1090 +eval "$(awk '/^trust_state_interactive\(\) \{/,/^\}/' "$SCRIPT_UNDER_TEST")" fails=0 t() { @@ -59,6 +61,20 @@ t() { fi } +# ---- CAPTURED FROM A REAL REGISTRY ----------------------------------------- +# Verbatim `npx npm@latest trust list @bugsee/cli-win32-arm64` output, npm +# 12.x, 2026-09-16. Every other fixture here was written to match the parser, +# which proves the parser self-consistent and nothing about npm; this one is +# the other way round. Do not "tidy" it — the leading blank line and the exact +# labels are the point. +t "real npm output is classified granted" granted text 0 ' +type: github +id: 6827c0f2-26cd-49fc-9cd3-374f7bb1d3b2 +file: npm-publish.yml +repository: bugsee/bugsee-cli +permissions: publish, stage publish +' + # ---- the state that must never be guessed -------------------------------- # npm errored. Previously this returned "not configured", and the script then # advised revoking a possibly-correct entry. @@ -185,6 +201,64 @@ t "JSON: unrelated metadata does not disarm the guard" unknown both 0 '{"meta":{ t "JSON: npm supporting --json with no entries is 'absent'" absent both 0 '[]' t "JSON: a non-zero --json call falls back, not 'absent'" unknown json 1 '[]' +# ---- the uncaptured retry --------------------------------------------------- +# This is the half the 28 fixtures above CANNOT see: they stub npm out, so the +# interaction between command substitution and npm's 2FA prompt is invisible to +# them. The retry lived inline in the caller, and a mutation deleting it passed +# every one of them. +# +# A fake npm that fails its first two (CAPTURED) reads and answers afterwards +# stands in for the real failure mode: captured stdout -> EOTP -> `unknown`, then +# an uncaptured call that authenticates, then a re-read that succeeds. It also has +# no `--json` support, like the npm that produced the text fixture above. +REAL_ENTRY=' +type: github +id: 6827c0f2-26cd-49fc-9cd3-374f7bb1d3b2 +file: npm-publish.yml +repository: bugsee/bugsee-cli +permissions: publish, stage publish +' +# The call count lives in a FILE, not a variable: `trust_state` is invoked through +# `$(...)`, and a subshell's increments die with it — which is why a first attempt +# at this test saw the retry happen and still read `unknown`. +# shellcheck disable=SC2329,SC2317 +flaky_npm() { + local json=0 n + for a in "$@"; do [ "$a" = "--json" ] && json=1; done + n=$(( $(cat "$CALL_FILE") + 1 )) + echo "$n" > "$CALL_FILE" + [ "$json" = 1 ] && return 1 + [ "$n" -le "$FAIL_FIRST" ] && return 1 + printf '%s' "$REAL_ENTRY" +} + +# Asserts the classification AND whether the interactive retry was used, because +# "granted either way" cannot tell a working retry from one that always runs. +ti() { + local name="$1" expected="$2" want_retry="$3" out got retried + CALL_FILE="$(mktemp)"; echo 0 > "$CALL_FILE" + out="$(TRUST_NPM=flaky_npm trust_state_interactive "@bugsee/x" 2>/dev/null)" + got="$(printf '%s' "$out" | tail -1)" + case "$out" in *"needs an interactive 2FA challenge"*) retried=yes ;; *) retried=no ;; esac + rm -f "$CALL_FILE" + if [ "$got" = "$expected" ] && [ "$retried" = "$want_retry" ]; then + printf ' ok %s\n' "$name" + else + printf ' FAIL %s (expected %s/retry=%s, got %s/retry=%s)\n' \ + "$name" "$expected" "$want_retry" "$got" "$retried" + fails=1 + fi +} + +# A cold session: the captured reads fail, so the retry is what rescues it. +FAIL_FIRST=2 +ti "an EOTP-failing captured read is retried uncaptured, then classified" granted yes + +# A warm session answers immediately, and nothing is retried — so a session +# already authenticated is never prompted. +FAIL_FIRST=0 +ti "a session that already answers is classified without a retry" granted no + if [ "$fails" -eq 0 ]; then echo "all trust_state cases pass" else From 506e414a1940c463f4889957979e33ea3d1bf1c1 Mon Sep 17 00:00:00 2001 From: Alexey Karimov Date: Sat, 19 Sep 2026 15:49:57 +0500 Subject: [PATCH 2/2] fix(npm): the extracted retry was captured again at the call site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #53 caught this, and it was a regression I introduced in the same PR. Extracting `trust_state_interactive` so the harness could drive it left the caller doing state="$(trust_state_interactive "$pkg")" which recaptures the exact stdout the fix exists to leave attached to the terminal. On the cold session this PR is about, npm's prompt would be swallowed again AND `$state` would arrive as several lines matching no arm of the `case` — so it would exit "could not classify (unexpected output)" instead of just failing to authenticate. The inline version it replaced was correct. The state now comes back in `TRUST_STATE` rather than on stdout, which is the only shape that keeps an uncaptured call uncaptured. My test hid it: `ti()` took the function's stdout and stripped the last line with `tail -1`, which quietly accommodated the state arriving with npm's output glued to it — a real caller has no `tail`. It now reads `TRUST_STATE` and compares exactly, and sends stdout to a file the way the real call site sends it to the terminal. And because no amount of driving the function can check its CALL SITE, that is pinned structurally: the suite greps the script (comments stripped — both files explain the hazard using the shape being searched for) and fails if the interactive read is wrapped in a command substitution again. Three mutants caught: the reviewer's finding reintroduced verbatim, the retry deleted, and the retry always taken. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- npm/bootstrap-names.sh | 21 +++++++++++++++------ npm/test-bootstrap-names.sh | 35 +++++++++++++++++++++++++++++------ 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/npm/bootstrap-names.sh b/npm/bootstrap-names.sh index 6c1d3b1..0c9a4ca 100755 --- a/npm/bootstrap-names.sh +++ b/npm/bootstrap-names.sh @@ -100,15 +100,21 @@ fi # A separate function ONLY so the test harness can drive it: the fixtures stub npm # out, so nothing about this retry was observable while it sat inline in the # caller — and a mutation removing it passed all 28 of them. +# +# The answer comes back in TRUST_STATE, NOT on stdout, and that is the whole +# point. A caller writing `state="$(trust_state_interactive …)"` would recapture +# the stdout this function exists to leave attached to the terminal: npm's prompt +# would be swallowed again, and `$state` would arrive as several lines that match +# no arm of the caller's `case`. The first version of this function did exactly +# that. The test asserts the call site is not wrapped. trust_state_interactive() { - local pkg="$1" state - state="$(trust_state "$pkg")" - if [ "$state" = unknown ]; then + local pkg="$1" + TRUST_STATE="$(trust_state "$pkg")" + if [ "$TRUST_STATE" = unknown ]; then echo " auth: npm needs an interactive 2FA challenge — follow its prompt" $TRUST_NPM trust list "$pkg" || true - state="$(trust_state "$pkg")" + TRUST_STATE="$(trust_state "$pkg")" fi - printf '%s' "$state" } trust_state() { @@ -265,7 +271,10 @@ JSON fi # 2. Attach the trusted publisher, so npm-publish.yml's OIDC can publish it. - state="$(trust_state_interactive "$pkg")" + # NOT `state="$(trust_state_interactive …)"`: that would recapture the stdout + # npm's 2FA prompt needs. The state arrives in TRUST_STATE instead. + trust_state_interactive "$pkg" + state="$TRUST_STATE" case "$state" in granted) diff --git a/npm/test-bootstrap-names.sh b/npm/test-bootstrap-names.sh index df3cf7b..4f79efa 100755 --- a/npm/test-bootstrap-names.sh +++ b/npm/test-bootstrap-names.sh @@ -234,22 +234,45 @@ flaky_npm() { # Asserts the classification AND whether the interactive retry was used, because # "granted either way" cannot tell a working retry from one that always runs. +# +# The state is read from TRUST_STATE and compared EXACTLY — no `tail -1`. An +# earlier version of this test took the function's stdout and stripped the last +# line off it, which quietly accommodated the bug the reviewer found: the state +# was arriving with npm's output glued to it, and a real caller has no `tail`. +# Stdout goes to a FILE here for the same reason the real call site leaves it +# attached to the terminal — capturing it with `$(...)` would put the function in +# a subshell and lose TRUST_STATE with it. ti() { - local name="$1" expected="$2" want_retry="$3" out got retried + local name="$1" expected="$2" want_retry="$3" out retried CALL_FILE="$(mktemp)"; echo 0 > "$CALL_FILE" - out="$(TRUST_NPM=flaky_npm trust_state_interactive "@bugsee/x" 2>/dev/null)" - got="$(printf '%s' "$out" | tail -1)" + local out_file; out_file="$(mktemp)" + TRUST_STATE="" + TRUST_NPM=flaky_npm trust_state_interactive "@bugsee/x" > "$out_file" 2>/dev/null + out="$(cat "$out_file")" case "$out" in *"needs an interactive 2FA challenge"*) retried=yes ;; *) retried=no ;; esac - rm -f "$CALL_FILE" - if [ "$got" = "$expected" ] && [ "$retried" = "$want_retry" ]; then + rm -f "$CALL_FILE" "$out_file" + if [ "$TRUST_STATE" = "$expected" ] && [ "$retried" = "$want_retry" ]; then printf ' ok %s\n' "$name" else printf ' FAIL %s (expected %s/retry=%s, got %s/retry=%s)\n' \ - "$name" "$expected" "$want_retry" "$got" "$retried" + "$name" "$expected" "$want_retry" "$TRUST_STATE" "$retried" fails=1 fi } +# The call site itself, which no amount of driving the function can check: wrapping +# it in a command substitution recaptures the stdout npm's prompt needs, and the +# multi-line value then matches no arm of the caller's `case`. That is precisely +# the regression the reviewer caught, so it is pinned structurally. +# Comments stripped first: this file's own explanation of the hazard, and the +# script's, both contain the very shape being searched for. +if grep -vE '^[[:space:]]*#' "$SCRIPT_UNDER_TEST" | grep -qE '[$][(][[:space:]]*trust_state_interactive'; then + printf ' FAIL trust_state_interactive is called in a command substitution — recaptures the 2FA prompt\n' + fails=1 +else + printf ' ok the interactive read is not wrapped in a command substitution\n' +fi + # A cold session: the captured reads fail, so the retry is what rescues it. FAIL_FIRST=2 ti "an EOTP-failing captured read is retried uncaptured, then classified" granted yes