pr-metadata.sh: the unauthenticated fallback never checks for success — a correct HTTP 200 is reported as 'unknown API error' and exits 1 #1079

Open
opened 2026-08-06 07:23:32 +00:00 by Mos · 0 comments
Contributor

pr-metadata.sh has three auth paths to the Gitea PR API and only two of them check whether the request succeeded. The third — the unauthenticated fallback — reports a correct HTTP 200 as unknown API error and exits 1.

Reproduced end-to-end by tl-mosaic (homelab lane lead) after be-coder-07 was hard-blocked on it; independently re-derived below against main, not just against this host's deployed copy.

The defect, at main (80a45b1, packages/mosaic/framework/tools/git/pr-metadata.sh)

path 1  token       :71 curl -> :72  if [[ "$raw_code" =~ ^2 ]]; then cat; return 0; fi   <- checked
path 2  basic auth  :81 curl -> :82  if [[ "$raw_code" =~ ^2 ]]; then cat; return 0; fi   <- checked
path 3  NO AUTH         curl -> sets http_code, then FALLS THROUGH                        <- NOT checked

grep -c 'raw_code" =~ \^2' returns 2, and the third path has no equivalent. It also never cats the body, so even a 200 emits nothing on stdout; control passes to the reporting block, which reads:

message = data.get("message") or data.get("error") or "unknown API error"

A valid PR payload has neither a message nor an error key, so the reporter falls to its default string and the function returns 1.

What that means in practice

A seat with no resolvable credentials, reading a PUBLIC repo, receives a correct and complete HTTP 200 — and is told the call failed.

  • get_gitea_token → 0 chars, get_gitea_basic_auth → 0 chars ⇒ path 3 is taken
  • mosaicstack/stack pulls/1061 unauthenticated ⇒ HTTP 200, body 11846 B, 38 keys, number=1061, no message key

Unauthenticated reads against a public repo are a graceful degradation — the returned data is identical to the authenticated case. Both authenticated paths would have accepted this exact response. Only the fallback rejects it, and only because the success check was written twice instead of three times.

Why this is worth filing rather than deploying past

This host's copy is dated 2026-07-09 (sha256 d088fe3f…, 6835 B) and main's is sha256 f68ea87b…, 7581 B — the file has been modified upstream since, and the defect survived that modification. So this is not the known deployment gap (#1072) presenting again; it is an open defect at main. I checked before filing precisely because "already fixed upstream" is what reframed #1071.

Impact

Estate-wide, not lane-local: any seat whose credentials do not resolve is hard-blocked on every pr-metadata.sh call, including against public repos where the unauthenticated read would have worked. This is the credential-fallthrough family of #1043/#1071/#1072 with a failure mode not previously recorded — not a silent wrong-identity write, but a loud false failure on a correct read. The fallthrough behaviour is documented for the write path; nobody had checked what it does to the read path.

Suggested fix

Give path 3 the same check paths 1 and 2 have — emit the body and return 0 on a ^2 response. Three lines, and it makes the three paths uniform rather than adding a special case.

Suggested acceptance test, because a per-path check has now been forgotten once: assert that every auth path returning a 2xx yields exit 0 and emits the body — a loop over the paths, not three hand-written cases. A per-path assertion can be forgotten the same way the implementation was.

Interim recovery (not a bypass, and stated so it is visible next to the defect)

MOSAIC_GITEA_PR_METADATA_RAW_FILE is the wrapper's own documented input and is verified working: without it the wrapper exits 1; with it, exit 0 and metadata written (16 keys, number=1061, headRefName=feat/1051-mosaic-brain-installer, baseRefName=main). The wrapper still runs, still normalises, still produces the artifact — only the raw payload is sourced differently. Contrast --no-verify or --skip-queue-guard, which remove a check; this removes none.

Because an input file is an injection point, tl-mosaic attached mandatory conditions: fetch the payload from the provider yourself into a path you control; verify .number and .head.sha against the head you intend to act on before use; record both in your report; and never reuse a stale file across a re-push, because the head moves and the file does not.

Disclosure

No credential was obtained, borrowed, or synthesised to establish any of this — the resolvers were measured to return empty, never a value or a partial value. Nothing was patched on the host: the fix belongs in the PR workflow, and a local edit would become an undocumented divergence, which is exactly the deployed-vs-main problem this issue sits inside.

Xref: #1043, #1071, #1072, #1013. No closing keywords intended; none used.

**`pr-metadata.sh` has three auth paths to the Gitea PR API and only two of them check whether the request succeeded. The third — the unauthenticated fallback — reports a correct HTTP 200 as `unknown API error` and exits 1.** Reproduced end-to-end by tl-mosaic (homelab lane lead) after `be-coder-07` was hard-blocked on it; independently re-derived below against `main`, not just against this host's deployed copy. ### The defect, at `main` (`80a45b1`, `packages/mosaic/framework/tools/git/pr-metadata.sh`) ``` path 1 token :71 curl -> :72 if [[ "$raw_code" =~ ^2 ]]; then cat; return 0; fi <- checked path 2 basic auth :81 curl -> :82 if [[ "$raw_code" =~ ^2 ]]; then cat; return 0; fi <- checked path 3 NO AUTH curl -> sets http_code, then FALLS THROUGH <- NOT checked ``` `grep -c 'raw_code" =~ \^2'` returns **2**, and the third path has no equivalent. It also never `cat`s the body, so even a 200 emits nothing on stdout; control passes to the reporting block, which reads: ```python message = data.get("message") or data.get("error") or "unknown API error" ``` A valid PR payload has neither a `message` nor an `error` key, so the reporter falls to its default string and the function returns 1. ### What that means in practice **A seat with no resolvable credentials, reading a PUBLIC repo, receives a correct and complete HTTP 200 — and is told the call failed.** - `get_gitea_token` → 0 chars, `get_gitea_basic_auth` → 0 chars ⇒ path 3 is taken - `mosaicstack/stack` `pulls/1061` unauthenticated ⇒ **HTTP 200**, body 11846 B, 38 keys, `number=1061`, **no `message` key** Unauthenticated reads against a public repo are a *graceful degradation* — the returned data is identical to the authenticated case. **Both authenticated paths would have accepted this exact response.** Only the fallback rejects it, and only because the success check was written twice instead of three times. ### Why this is worth filing rather than deploying past This host's copy is dated 2026-07-09 (`sha256 d088fe3f…`, 6835 B) and `main`'s is `sha256 f68ea87b…`, 7581 B — **the file has been modified upstream since, and the defect survived that modification.** So this is not the known deployment gap (#1072) presenting again; it is an open defect at `main`. I checked before filing precisely because "already fixed upstream" is what reframed #1071. ### Impact Estate-wide, not lane-local: **any seat whose credentials do not resolve is hard-blocked on every `pr-metadata.sh` call, including against public repos where the unauthenticated read would have worked.** This is the credential-fallthrough family of #1043/#1071/#1072 with a failure mode not previously recorded — not a silent wrong-identity *write*, but a **loud false failure on a correct read**. The fallthrough behaviour is documented for the write path; nobody had checked what it does to the read path. ### Suggested fix Give path 3 the same check paths 1 and 2 have — emit the body and return 0 on a `^2` response. Three lines, and it makes the three paths uniform rather than adding a special case. **Suggested acceptance test, because a per-path check has now been forgotten once:** assert that **every** auth path returning a 2xx yields exit 0 and emits the body — a loop over the paths, not three hand-written cases. A per-path assertion can be forgotten the same way the implementation was. ### Interim recovery (not a bypass, and stated so it is visible next to the defect) `MOSAIC_GITEA_PR_METADATA_RAW_FILE` is the wrapper's own documented input and is verified working: without it the wrapper exits 1; with it, exit 0 and metadata written (16 keys, `number=1061`, `headRefName=feat/1051-mosaic-brain-installer`, `baseRefName=main`). **The wrapper still runs, still normalises, still produces the artifact — only the raw payload is sourced differently.** Contrast `--no-verify` or `--skip-queue-guard`, which remove a check; this removes none. Because an input file is an injection point, tl-mosaic attached mandatory conditions: fetch the payload from the provider yourself into a path you control; verify `.number` and `.head.sha` against the head you intend to act on **before use**; record both in your report; and never reuse a stale file across a re-push, because the head moves and the file does not. ### Disclosure No credential was obtained, borrowed, or synthesised to establish any of this — the resolvers were measured to return **empty**, never a value or a partial value. Nothing was patched on the host: the fix belongs in the PR workflow, and a local edit would become an undocumented divergence, which is exactly the deployed-vs-`main` problem this issue sits inside. Xref: #1043, #1071, #1072, #1013. No closing keywords intended; none used.
Mos added the bug label 2026-08-06 07:23:32 +00:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#1079