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

push-guard: close the --json-path fail-open with required repo config #975

New Issue
Open
opened 2026-07-31 00:10:37 +00:00 by Mos · 0 comments
Mos commented 2026-07-31 00:10:37 +00:00
Copy Link
Copy Source

Follow-up to #974, which landed push-guard.sh with this limitation stated rather than fixed.

Authored by installer-7; filed by mos-claude, who holds the credential. Body is installer-7's, verbatim.

The fail-open

The staged-JSON check only runs when a caller passes --json-path. A repo that never passes it gets no JSON validation, forever, and nothing fails. Today's only mitigation is that the guard prints JSON check NOT REQUESTED on every run. That is visible rather than silent, which is categorically better — but it is still an absence, and an absence is not reviewable. Nobody reads a line that has been printed correctly ten thousand times.

Why not simply default the check on

Measured, not assumed: 17 of 119 tracked .json files in a real repository fail a strict parse, all legitimately — every tsconfig*.json is JSONC (comments are legal there) and the CA templates are Go templates carrying a .json suffix. On-by-default fires on ~14% of that repo's JSON. A guard that cries wolf gets routed around until the bypass is habitual, and then the bypass covers the true positives too. Broader is weaker here. That is why #974 made it opt-in.

Proposed fix

The guard reads repo-level config (.push-guard.json at repo root). If the config is absent, the guard refuses to run with a non-zero exit — it does not run with the check quietly disabled.

The config must state a decision explicitly, one of:

{ "json_paths": ["data/**/*.json"] }
{ "json_check": "none", "reason": "no generated JSON in this repo" }

json_check: none requires a non-empty reason. That is the whole point: it converts a silent absence into a written, diffable, reviewable decision. You can review a line in a config file. You cannot review the fact that nobody typed a flag.

A malformed config must refuse, never degrade to "treat as no config" — that path would rebuild the exact fail-open being closed.

Required needles (both polarities — a needle set without controls proves nothing)

# Kind Case Expected
1 NEEDLE no config present refuses, non-zero
2 NEEDLE config lists json_paths, broken JSON staged exit 3
3 NEEDLE json_check: none with no reason refuses
4 NEEDLE malformed/unparseable config refuses (must not fall back)
5 CONTROL config lists json_paths, JSON clean exit 0 and output names the paths/count checked
6 CONTROL json_check: none with reason exit 0 and output states the opt-out was recorded

Cases 5 and 6 must assert on output, not just exit 0. Exit 0 cannot distinguish "checked the files and they were fine" from "matched zero files and had nothing to check" — two controls in an earlier revision of this suite passed vacuously for exactly that reason, and the :(glob) mutation was caught only by output assertions.

Mutation requirements (the harness gets tested too)

  • Revert refuse-on-missing-config → needle 1 must fail
  • Make config parsing always succeed-empty → needle 4 must fail
  • Drop the reason requirement → needle 3 must fail

A mutant that no needle catches means the needle is decorative.

Rollout trap — named so nobody walks into it

The tempting migration is "warn for one release, then enforce." That warning phase IS the degrade-to-a-warning this tool exists to forbid, and it leaves the fail-open open for precisely as long as the warning is ignored — which is indefinitely. Recommend instead a hard cutover at a version bump: every consumer adds config or the guard refuses. It breaks loudly, once, which is the point. Consumers who genuinely have no JSON write four lines and json_check: none with a reason.

Acceptance

All 6 cases green, 3 mutation runs each failing exactly their intended needle, shellcheck clean, and the existing 19 cases still green.

Follow-up to #974, which landed `push-guard.sh` with this limitation **stated rather than fixed**. *Authored by installer-7; filed by mos-claude, who holds the credential. Body is installer-7's, verbatim.* ## The fail-open The staged-JSON check only runs when a caller passes `--json-path`. A repo that never passes it gets no JSON validation, forever, and nothing fails. Today's only mitigation is that the guard prints `JSON check NOT REQUESTED` on every run. That is visible rather than silent, which is categorically better — but it is still an absence, and **an absence is not reviewable**. Nobody reads a line that has been printed correctly ten thousand times. ## Why not simply default the check on Measured, not assumed: **17 of 119** tracked `.json` files in a real repository fail a strict parse, **all legitimately** — every `tsconfig*.json` is JSONC (comments are legal there) and the CA templates are Go templates carrying a `.json` suffix. On-by-default fires on ~14% of that repo's JSON. A guard that cries wolf gets routed around until the bypass is habitual, and then **the bypass covers the true positives too**. Broader is *weaker* here. That is why #974 made it opt-in. ## Proposed fix The guard reads repo-level config (`.push-guard.json` at repo root). If the config is **absent**, the guard **refuses to run** with a non-zero exit — it does not run with the check quietly disabled. The config must state a decision explicitly, one of: ```json { "json_paths": ["data/**/*.json"] } { "json_check": "none", "reason": "no generated JSON in this repo" } ``` `json_check: none` **requires** a non-empty `reason`. That is the whole point: it converts a silent absence into a **written, diffable, reviewable decision**. You can review a line in a config file. You cannot review the fact that nobody typed a flag. A **malformed** config must refuse, never degrade to "treat as no config" — that path would rebuild the exact fail-open being closed. ## Required needles (both polarities — a needle set without controls proves nothing) | # | Kind | Case | Expected | |---|------|------|----------| | 1 | NEEDLE | no config present | refuses, non-zero | | 2 | NEEDLE | config lists `json_paths`, broken JSON staged | exit 3 | | 3 | NEEDLE | `json_check: none` with **no** reason | refuses | | 4 | NEEDLE | malformed/unparseable config | refuses (must **not** fall back) | | 5 | CONTROL | config lists `json_paths`, JSON clean | exit 0 **and** output names the paths/count checked | | 6 | CONTROL | `json_check: none` **with** reason | exit 0 **and** output states the opt-out was recorded | Cases 5 and 6 **must assert on output**, not just exit 0. Exit 0 cannot distinguish "checked the files and they were fine" from "matched zero files and had nothing to check" — two controls in an earlier revision of this suite passed vacuously for exactly that reason, and the `:(glob)` mutation was caught **only** by output assertions. ## Mutation requirements (the harness gets tested too) - Revert refuse-on-missing-config → needle 1 must fail - Make config parsing always succeed-empty → needle 4 must fail - Drop the reason requirement → needle 3 must fail A mutant that no needle catches means the needle is decorative. ## Rollout trap — named so nobody walks into it The tempting migration is "warn for one release, then enforce." **That warning phase IS the degrade-to-a-warning this tool exists to forbid**, and it leaves the fail-open open for precisely as long as the warning is ignored — which is indefinitely. Recommend instead a **hard cutover at a version bump**: every consumer adds config or the guard refuses. It breaks loudly, once, which is the point. Consumers who genuinely have no JSON write four lines and `json_check: none` with a reason. ## Acceptance All 6 cases green, 3 mutation runs each failing exactly their intended needle, shellcheck clean, and the existing 19 cases still green.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jason.woltje
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#975