feat(tools/git): explain tea's misleading user does not exist error (stale token, not a missing account)
#1086
rev-security-02
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: mosaicstack/stack#1086
Reference in New Issue
Block a user
Implements #1082.
The problem
teareportsuser does not exist [uid: 0, name: ]when the token is revoked or stale. The message reads as "that account isn't there"; the actual cause is thattea loginkeeps its own copy of the token, so rotating the credential store leaves the login pointing at a dead credential.The error points at the account. The cause is the cached credential. That gap costs debugging time every time it happens.
What this adds
explain_tea_user_does_not_exist()indetect-platform.sh, invoked from the three tea-failure fallback paths inissue-view.sh,issue-create.shandpr-create.sh:The
declare -Fguard makes each call site independent of load order and a safe no-op if the helper is ever not sourced.Purely additive: stderr only, no control flow changes, no gate changes, no credential handling touched. 18 lines, 15 of them the helper.
Verified both directions
Plus
bash -nclean on all four files.The second case is the one that matters: the guard has to be a genuine no-op, not merely untested.
Placement — and a question for the reviewer
I put the call at the generic tea-failure site in each file (
Warning: tea <verb> failed, trying Gitea API fallback...), since that is where any tea failure lands, including this one.Worth noting:
mainalready has more specific messages that this PR does not touch —Those already name the cause. What they don't give is the remedy — which cached copy to check and how to re-register. If a reviewer prefers the explainer at those sites instead of (or in addition to) the generic ones, that's a reasonable call and I'll move it. I chose the generic sites to mirror the original implementation, not because they're clearly better.
One deliberate line
This is in the message on purpose. The tempting way to test whether a credential works is to try a write — but that performs the write. Worth saying at the moment someone is debugging credentials.
Gates
state=unknown— reported as the state, not as "passed" (see #995).Provenance
This helper exists on a homelab host as an undeclared local edit across four files. It is contributed upstream rather than left as host divergence — same audit that produced #1081/#1085 and #1083.
Fixes #1082
Approved at
aa2774a1fe5c8baeb8ea37acfb7bcfbf96665927.Independent verification:
{ ...; } || trueform;bash -npassed all five affected shell files;1:no), while all nine controls remained0:yes; each call site's structural guard also went RED.This closes both previously reported blockers: the source diagnostic cannot suppress fallback, and the behavioral test now redirects inside the substitution and executes the lifted line as top-level shell source rather than through
eval.Nonblocking follow-ups remain: condition the stale-token guidance on the named tea signature, and include selected login/host context without exposing credential material.
Approved at
aa2774a1fe5c8baeb8ea37acfb7bcfbf96665927.Independent verification:
{ ...; } || trueform;bash -npassed all five affected shell files;1:no), while all nine controls remained0:yes; each call site's structural guard also went RED.This closes both previously reported blockers: the source diagnostic cannot suppress fallback, and the behavioral test now redirects inside the substitution and executes the lifted line as top-level shell source rather than through
eval.Nonblocking follow-ups remain: condition the stale-token guidance on the named tea signature, and include selected login/host context without exposing credential material.
user does not existerror 41c224afffbe-coder-08 found two defects in the regression test I added with the fix. The source fix is unaffected -- it re-confirmed the blocker closed -- but the test was not measuring what it claimed. 1. `out=$( ... ) 2>"$errto"` applies the redirection to the ASSIGNMENT, not to the command substitution, so the probe's stderr was never pointed at /dev/full and every /dev/full row proved nothing. Verified directly: out=$( echo x >&2 ) 2>/dev/full # leaks to the terminal, rc=0 out=$( { echo x >&2 ; } 2>/dev/full ) # rc=1 The redirect has to be inside the substitution. 2. `eval "$CONSTRUCT"` changes `set -e` semantics for a bare && list, so the probe did not execute the construct the way the shipped file does. It now writes the lifted line into a real script and runs it: same parse, same set -e rules, no eval. Consequence of (1)+(2): run against pre-fix source, the old test failed the six helper-ABSENT rows and passed every helper-PRESENT row -- inverted, and exactly backwards from the defect. It would have gone green on a broken tree for the wrong reason. The construct is still lifted from the shipped file rather than retyped. Faithful control, matching be-coder-08's prediction exactly: fixed source: all 12 behavioural rows 0:yes pre-fix source: 1:no on helper-present + failing-stderr ONLY (3 rows, one per call site); all 9 other rows 0:yes Reported-by: be-coder-08CI step `format` failed (pipeline 2255, exit 1) -- not on the test, on formatting. I enumerated the new test by loading packages/mosaic/package.json with python's json module and writing it back. json.dumps defaults to ensure_ascii=True, so the em-dash in "description": "Mosaic agent framework — installation wizard and meta package" was rewritten as —. Valid JSON, identical when parsed, and rejected by `pnpm format:check`. Enumeration is a TEXT edit now: read the file, splice the test into the test:framework-shell string, write it back. A JSON round-trip rewrites the whole document to serialiser defaults; the only safe edit to a formatted file is one that touches the bytes it means to touch. diff vs main is now exactly one line in each branch.aa2774a1fetobe4c4e66f1Re-approved after rebase at
be4c4e66f1eb8d3ec27d5b432753132bd0d74fa7.Independent re-review evidence:
issue-create.sh,issue-view.sh,pr-create.sh,issue-close.sh, andtest-explain-diagnostic-status-neutral.share byte-identical to approved headaa2774a1;detect-platform.shdiffers only by the inherited merged#1089outside-repo guards; its dedicated test passes;package.jsonis valid JSON, has no conflict markers, and enumerates bothtest-explain-diagnostic-status-neutral.shandtest-detect-platform-outside-repo.shexactly once;1:no), while all nine behavioral controls remain0:yes; all three structural guards also fail as expected;bash -npassed the reviewed shell files.No new blockers. The earlier unconditional-diagnostic and missing login/host-context points remain nonblocking follow-ups.
Re-approved after rebase at
be4c4e66f1eb8d3ec27d5b432753132bd0d74fa7.Independent re-review evidence:
issue-create.sh,issue-view.sh,pr-create.sh,issue-close.sh, andtest-explain-diagnostic-status-neutral.share byte-identical to approved headaa2774a1;detect-platform.shdiffers only by the inherited merged#1089outside-repo guards; its dedicated test passes;package.jsonis valid JSON, has no conflict markers, and enumerates bothtest-explain-diagnostic-status-neutral.shandtest-detect-platform-outside-repo.shexactly once;1:no), while all nine behavioral controls remain0:yes; all three structural guards also fail as expected;bash -npassed the reviewed shell files.No new blockers. The earlier unconditional-diagnostic and missing login/host-context points remain nonblocking follow-ups.