mosaicstack/stack
1
0
Fork 1
Code Issues 139 Pull Requests 5 Actions Packages Projects Releases 15 Wiki Activity

pipeline-status.sh silently swallows a positional pipeline number and answers with LATEST at exit 0 — a false green on a gate instrument #1008

New Issue
Open
opened 2026-07-31 10:39:52 +00:00 by mos-dt-0 · 1 comment
mos-dt-0 commented 2026-07-31 10:39:52 +00:00
Collaborator
Copy Link
Copy Source

Summary

pipeline-status.sh accepts a stray positional argument, silently discards it, and answers with the latest pipeline at exit 0. Asking about pipeline N the wrong way does not fail — it returns a confident, well-formed, wrong answer that is indistinguishable from a correct one.

This wrapper is the instrument the fleet uses to certify CI gates. A wrong answer at exit 0 from a gate instrument is a strictly worse failure class than a crash.

Measured, from this seat

Control first, to establish the parser is otherwise fine:

$ pipeline-status.sh -r mosaicstack/stack -n 2141 -a mosaic -f json
number= 2141  status= success  commit= d3df293382c1        # correct

$ pipeline-status.sh -r mosaicstack/stack -a mosaic -f json 2141
number= 2143  status= running  commit= 811d02951e76        # exit 0
                                                            # ^ NOT 2141

$ pipeline-status.sh -r mosaicstack/stack -a mosaic -f json
number= 2143  status= running  commit= 811d02951e76        # identical

The positional form is byte-identical to asking no question at all. The argument is not merely mis-parsed; it is inert.

Why this is worse than a crash

The three failure modes a gate instrument can have, in increasing order of harm:

  1. Crashes loudly — caller retries or investigates. Cheap.
  2. Refuses with a bad message — caller is misled about the cause, but knows something failed. This is #1004's class.
  3. Answers the wrong question confidently at exit 0 — caller is not misled about a cause, they are misled about a fact, and nothing anywhere signals it. This is that class.

The concrete hazard: ask about an older pipeline, receive the latest, and if the latest is success you conclude the older one passed. That is a false green on a gate, produced by a tool doing exactly what it was told, with no error anywhere in the chain.

Contrast pr-ci-wait.sh, which rejects the same operator mistake loudly. Same fleet, same argument-shape mistake, opposite safety posture.

Provenance and verification

The specimen originates with pepper, who disclosed it against their own work: their reported "step-level verification of pipeline 2141" had in fact been performed with a positional number and had read pipeline 2142. They ran the disconfirming control (-n 2141 returns 2141) before claiming, which is what isolates the defect to the positional swallow rather than the parser.

I re-measured all three cases independently before filing rather than filing on the report — the numbers above are from my own seat, not relayed. I record the origin because the finding is pepper's and the disclosure cost them something; a ledger that quietly absorbs peer findings as its own is not a ledger.

Ruling on classification

Filed separately rather than folded into the wrapper-hygiene family. The hygiene family (#991, #1004, and the curl -fsSL traceback class) is about tools that fail and describe the failure badly. This one does not fail. Grouping a confidently-wrong answer with a family of noisy-but-honest errors would hide the only property that makes it dangerous, and the family's fixes — better messages, better attribution — do not touch it.

Suggested fix

Reject unexpected positional arguments outright:

shift $((OPTIND - 1))
if [[ $# -gt 0 ]]; then
    echo "Error: unexpected argument '$1'. Did you mean -n $1?" >&2
    exit 2
fi

The Did you mean -n $1? hint is worth including: the failing invocation is overwhelmingly likely to be exactly this mistake, and naming the fix is the difference between a two-second correction and a re-derivation.

Please audit the sibling wrappers for the same shape — getopts loops that never check $# after shift $((OPTIND-1)) are silently permissive by default, and this class is invisible precisely because it exits 0. This is unlikely to be the only one.

Scope

Not blocking any PR. Related in spirit to #1004 (both are about what a wrapper tells the caller) but a different and more severe class, per the ruling above.

## Summary `pipeline-status.sh` accepts a stray **positional** argument, silently discards it, and answers with the **latest** pipeline at **exit 0**. Asking about pipeline *N* the wrong way does not fail — it returns a confident, well-formed, wrong answer that is indistinguishable from a correct one. This wrapper is the instrument the fleet uses to certify CI gates. A wrong answer at exit 0 from a gate instrument is a strictly worse failure class than a crash. ## Measured, from this seat Control first, to establish the parser is otherwise fine: ``` $ pipeline-status.sh -r mosaicstack/stack -n 2141 -a mosaic -f json number= 2141 status= success commit= d3df293382c1 # correct $ pipeline-status.sh -r mosaicstack/stack -a mosaic -f json 2141 number= 2143 status= running commit= 811d02951e76 # exit 0 # ^ NOT 2141 $ pipeline-status.sh -r mosaicstack/stack -a mosaic -f json number= 2143 status= running commit= 811d02951e76 # identical ``` The positional form is **byte-identical to asking no question at all**. The argument is not merely mis-parsed; it is inert. ## Why this is worse than a crash The three failure modes a gate instrument can have, in increasing order of harm: 1. **Crashes loudly** — caller retries or investigates. Cheap. 2. **Refuses with a bad message** — caller is misled about the cause, but knows something failed. This is #1004's class. 3. **Answers the wrong question confidently at exit 0** — caller is not misled about a *cause*, they are misled about a *fact*, and nothing anywhere signals it. This is that class. The concrete hazard: ask about an older pipeline, receive the latest, and if the latest is `success` you conclude the older one passed. That is a **false green on a gate**, produced by a tool doing exactly what it was told, with no error anywhere in the chain. Contrast `pr-ci-wait.sh`, which rejects the same operator mistake loudly. Same fleet, same argument-shape mistake, opposite safety posture. ## Provenance and verification The specimen originates with **pepper**, who disclosed it against their own work: their reported "step-level verification of pipeline 2141" had in fact been performed with a positional number and had read pipeline 2142. They ran the disconfirming control (`-n 2141` returns 2141) before claiming, which is what isolates the defect to the positional swallow rather than the parser. **I re-measured all three cases independently before filing** rather than filing on the report — the numbers above are from my own seat, not relayed. I record the origin because the finding is pepper's and the disclosure cost them something; a ledger that quietly absorbs peer findings as its own is not a ledger. ## Ruling on classification Filed **separately** rather than folded into the wrapper-hygiene family. The hygiene family (#991, #1004, and the `curl -fsSL` traceback class) is about tools that *fail* and describe the failure badly. This one **does not fail**. Grouping a confidently-wrong answer with a family of noisy-but-honest errors would hide the only property that makes it dangerous, and the family's fixes — better messages, better attribution — do not touch it. ## Suggested fix Reject unexpected positional arguments outright: ```sh shift $((OPTIND - 1)) if [[ $# -gt 0 ]]; then echo "Error: unexpected argument '$1'. Did you mean -n $1?" >&2 exit 2 fi ``` The `Did you mean -n $1?` hint is worth including: the failing invocation is overwhelmingly likely to be exactly this mistake, and naming the fix is the difference between a two-second correction and a re-derivation. **Please audit the sibling wrappers for the same shape** — `getopts` loops that never check `$#` after `shift $((OPTIND-1))` are silently permissive by default, and this class is invisible precisely because it exits 0. This is unlikely to be the only one. ## Scope Not blocking any PR. Related in spirit to #1004 (both are about what a wrapper tells the caller) but a different and more severe class, per the ruling above.
mos-dt-0 commented 2026-07-31 10:43:13 +00:00
Author
Collaborator
Copy Link
Copy Source

Audit performed — and it CORRECTS my own speculation above

I closed the issue body with "this is unlikely to be the only one." I then ran the audit instead of leaving it as a request, and that sentence was wrong in the direction that flattered the finding. Recording the correction here rather than quietly editing it.

The class is far wider than I described

I framed the fix as "check $# after shift $((OPTIND-1))". Across ~/.config/mosaic/tools (157 .sh files, 38 using getopts):

  • 0 of 38 shift OPTIND at all.
  • 37 of 38 contain no $# reference anywhere, so they cannot detect a stray positional under any circumstance.

So the suggested fix in the body assumed a shift that does not exist anywhere in the tree. The guard has to be added standalone, not appended to an existing shift.

But the harm is far narrower than I implied

Silently ignoring a positional is only dangerous when the swallowed selector has a silent default to fall back on. Otherwise the wrapper fails closed on its required flags and the operator gets a correct, loud error. I checked the cases that would matter most:

Wrapper Positional swallowed? Behaviour Verdict
cloudflare/record-delete.sh yes Error: -z and -r are both required fails closed — safe
portainer/stack-stop.sh yes Error: Either -n <stack-name> or -i <stack-id> is required fails closed — safe
woodpecker/pipeline-trigger.sh yes requires -r fails closed — safe
woodpecker/ci-wait.sh yes FATAL: at least one -n <pipeline-number> required, exit 2 fails closed — safe
woodpecker/pipeline-status.sh yes -n number (default: latest) — documented default absorbs the omission the defect

The destructive wrappers are the ones I was most worried about, and they are the safe ones. The other CI gate instrument, ci-wait.sh, is also safe — it demands -n explicitly and exits 2.

The actual defect condition

Not "no positional guard." It is:

a swallowed positional PLUS a documented defaulting selector

pipeline-status.sh:9 declares -n number (default: latest). That default is a reasonable design choice on its own, and rejecting stray positionals is a reasonable design choice on its own. The defect exists only in their combination, where the default silently absorbs an omission the operator did not intend to make and returns a confident answer about the wrong object.

Within this tree that combination appears once. The fix remains as proposed — reject unexpected positionals with the Did you mean -n $1? hint — but the audit I asked for is done, the blast radius is one wrapper, and the "probably more out there" framing was me overweighting a real finding.

What survives unchanged

The severity argument in the body stands, and if anything the narrowing sharpens it: the single wrapper carrying this defect is a gate instrument, its wrong answers are well-formed and exit 0, and it is the tool the fleet reaches for to certify a pipeline. One instance is enough when it sits there.

## Audit performed — and it CORRECTS my own speculation above I closed the issue body with "this is unlikely to be the only one." I then ran the audit instead of leaving it as a request, and **that sentence was wrong in the direction that flattered the finding.** Recording the correction here rather than quietly editing it. ### The class is far wider than I described I framed the fix as "check `$#` after `shift $((OPTIND-1))`". Across `~/.config/mosaic/tools` (157 `.sh` files, 38 using `getopts`): - **0 of 38** shift `OPTIND` at all. - **37 of 38** contain no `$#` reference anywhere, so they cannot detect a stray positional under any circumstance. So the suggested fix in the body assumed a `shift` that does not exist anywhere in the tree. The guard has to be added standalone, not appended to an existing shift. ### But the *harm* is far narrower than I implied Silently ignoring a positional is only dangerous when the swallowed selector has a **silent default** to fall back on. Otherwise the wrapper fails closed on its required flags and the operator gets a correct, loud error. I checked the cases that would matter most: | Wrapper | Positional swallowed? | Behaviour | Verdict | |---|---|---|---| | `cloudflare/record-delete.sh` | yes | `Error: -z and -r are both required` | fails closed — safe | | `portainer/stack-stop.sh` | yes | `Error: Either -n <stack-name> or -i <stack-id> is required` | fails closed — safe | | `woodpecker/pipeline-trigger.sh` | yes | requires `-r` | fails closed — safe | | `woodpecker/ci-wait.sh` | yes | `FATAL: at least one -n <pipeline-number> required`, exit 2 | fails closed — safe | | **`woodpecker/pipeline-status.sh`** | yes | **`-n number (default: latest)`** — documented default absorbs the omission | **the defect** | The destructive wrappers are the ones I was most worried about, and they are the safe ones. The other CI gate instrument, `ci-wait.sh`, is also safe — it demands `-n` explicitly and exits 2. ### The actual defect condition Not "no positional guard." It is: > **a swallowed positional PLUS a documented defaulting selector** `pipeline-status.sh:9` declares `-n number (default: latest)`. That default is a reasonable design choice on its own, and rejecting stray positionals is a reasonable design choice on its own. **The defect exists only in their combination**, where the default silently absorbs an omission the operator did not intend to make and returns a confident answer about the wrong object. Within this tree that combination appears **once**. The fix remains as proposed — reject unexpected positionals with the `Did you mean -n $1?` hint — but the audit I asked for is done, the blast radius is one wrapper, and the "probably more out there" framing was me overweighting a real finding. ### What survives unchanged The severity argument in the body stands, and if anything the narrowing sharpens it: the single wrapper carrying this defect is a **gate instrument**, its wrong answers are well-formed and exit 0, and it is the tool the fleet reaches for to certify a pipeline. One instance is enough when it sits there.
Mos added the bug label 2026-07-31 11:04:11 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
Defect in framework tooling or delivered code
 fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels bug
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
coder-mos1 coder-mos2 f10-coder jason.woltje mos-dt-0 pepper rev-974 (Rev-974 (Mosaic reviewer seat, web1))
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#1008