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..0c9a4ca 100755 --- a/npm/bootstrap-names.sh +++ b/npm/bootstrap-names.sh @@ -87,15 +87,45 @@ 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. +# +# 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" + 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 + TRUST_STATE="$(trust_state "$pkg")" + fi +} + 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 +271,12 @@ JSON fi # 2. Attach the trusted publisher, so npm-publish.yml's OIDC can publish it. - case "$(trust_state "$pkg")" in + # 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) echo " trust: already grants publish for $WORKFLOW" ;; diff --git a/npm/test-bootstrap-names.sh b/npm/test-bootstrap-names.sh index c080528..4f79efa 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,87 @@ 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. +# +# 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 retried + CALL_FILE="$(mktemp)"; echo 0 > "$CALL_FILE" + 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" "$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" "$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 + +# 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