test(control): pin the passwordless-pool masked-secret round trip (#439)

[VijitSingh97]

Closes #439.

Every commit: and round-trip: row in the #415 masked-secret block replays a sentinel that refers to a password which exists. None drives the other direction — a pool that stores nothing, advertised as holding a secret — which is the shape an appliance rig ships by default and the one the whole mechanism exists to prevent.

The four rows

row	claim
round-trip: a pool storing no password is served with no marker (#439)	the feed invents nothing — an equality against {"pools":[{"url":"bare:3333"}]}, so it cannot pass on empty output
round-trip: replaying a passwordless pool's own feed commits (#439)	the full replay is ACCEPTED and the pool stays passwordless
commit: a fabricated marker on a pool that stores no password is rejected (#439)	a marker the feed never emitted has nothing to resolve
commit: that rejection names the unresolvable key (#439)	and it says unresolvable-secret-marker:pass, not something a later validator invents

The third row is the gap the existing block leaves: commit: fingerprint sentinel on a pool with no stored pin rejected covers exactly this shape for tls-fingerprint, and there was no pass equivalent. commit: a brand-new pool with no pass still commits looks like the missing coverage and is not — it sends a new pool with no marker at all.

Controlled pair — RUN BY ME, both legs, one variable

The control is PR #437's own: _writable_config_canonical reading parse_config's normalized POOLS_JSON instead of the raw .pools. Both legs ran back to back in one worktree over byte-identical test files; the mutation was applied by script and its diff recorded before the leg ran.

leg	result
fix	2076 passed, 0 failed
control (pools=${POOLS_JSON:-[]})	2071 passed, 5 failed

The five reds under the control:

✗ config: unset pass has no marker — nothing to keep (#415)          <- pre-existing
✗ config: a pool stored with only a url is served with only a url (#429)  <- pre-existing
✗ round-trip: a pool storing no password is served with no marker (#439)  <- NEW
✗ round-trip: replaying a passwordless pool's own feed commits (#439)     <- NEW
✗ sister-API feed shape matches the fixture (#351)                   <- pre-existing

That is the point of the issue, measured: under a live instance of this defect every behavioural round-trip row in the suite used to report success. Two of them now do not.

The two rejection rows stay GREEN under this control, and that is correct — I am not claiming all four fired. Their staged payload is a literal, so _control_commit never reads the served feed at all: it calls neither _api_config_json nor _writable_config_canonical, only naming them in comments. What would red them is a change to the resolver inside _control_commit — stored() returning "" instead of null for a missing key, or dropping the post-condition marker scan.

Correction, owed to the non-author pass. An earlier version of this body said a change to the mask would red them. That is false, for exactly the reason above: the mask is as unreachable from those two rows as the canonical view is. A right verdict resting on a false mechanism is what a later reader quotes as ground, so it is corrected here rather than left standing.

And to be exact about what the third row buys: the resolver is a single reduce ("pass", "tls-fingerprint") as $k over one expression, so it traverses the same jq branch as the pre-existing commit: fingerprint sentinel on a pool with no stored pin rejected (#415). It is a new input shape — a pool storing no secret at all, which is the appliance rig's default — over an already-covered code path, not a new branch. Rows three and four are also one invocation measured twice (the verb, then the reason): one piece of evidence, not two.

Counts reconcile: 2076 − 5 = 2071. Re-run at head e00c402 after strengthening the first row: identical — 2076/0 and 2071/5, same five reds, worktree clean afterwards.

The first row was originally assert_absent … "__secret__", and assert_absent passes on an empty haystack — so it would have ticked if the fixture had produced nothing at all. It was safe only because the row beside it commits the same block, which is a guard that vanishes the moment anyone weakens that row. It is now an equality against the derived literal. Raised by the non-author pass as a non-blocking residual; taken anyway, because "safe because of the row next to it" is the shape this suite keeps paying for.

Line budget — tests/run.sh is at its ceiling, so this is line-neutral

9795 before, 9795 after (20 insertions(+), 20 deletions(-), one file). Per-edit:

edit	delta
new secret_out helper (raw _control_commit output)	+7
sc_unres inline fixture folded onto it	−10
rt415 inline fixture folded onto the existing cfgblk (behaviour-identical: cfgblk already does the same mktemp/source/parse_config/_api_config_json)	−9
the four rows + their comment	+12
	0

Neither fold changes what is asserted; both are the same code paths already in the file, and the fix leg's 2076 = 2072 base + the 4 new rows.

Not done, and stated

• No rigforge.sh change. This is coverage for behaviour that is already correct on develop; nothing here fixes a live defect.
• tests/contract/v1/* untouched — one file changed, tests/run.sh. (Asked by the appliance lane; answered there too.)
• Local shellcheck not run by me — the fleet lock was held. bash -n and shfmt -i 4 -d are clean; CI's Lint (shellcheck + shfmt) is the settling instrument, not a local pass I am claiming.
• The secret_out helper duplicates secret_case's fixture setup rather than sharing it. secret_case needs the sandbox directory after the call to inspect config.json; secret_out needs only stdout. Merging them would mean returning two things from one function, which is what the split avoids.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JMc1JsaJ6iDj5sw4LSeFoH

[VijitSingh97]

Head 42441c3 → e00c402, after the non-author PASS. One row changed; the body correction the pass asked for is made.

The correction, and it was the important one. The body said a change to the mask would red the two rejection rows. False: their staged payload is a literal, and _control_commit calls neither _api_config_json nor _writable_config_canonical — it only names them in comments. The mask is exactly as unreachable from those rows as the canonical view is. What would red them is the resolver inside _control_commit. Corrected in place, and the body now also says what the third row actually buys: a new input shape over an already-covered jq branch (the same reduce ("pass","tls-fingerprint") the pre-existing fingerprint-sentinel row traverses), not a new branch.

The one code change — the row-A vacuity residual, taken even though you called it non-blocking. assert_absent passes on an empty haystack, so assert_absent "$rt439_pools" "__secret__" would have ticked on a fixture that produced nothing. Safe today only because row B commits the same block — a guard that disappears the moment anyone weakens row B. It is now an equality against the derived literal {"pools":[{"url":"bare:3333"}]}, which cannot pass vacuously and pins the whole key set, which is what the sibling #429 row already does for this shape. Line-neutral; tests/run.sh still 9795.

Re-ran the full pair at the new head — mutation diff recorded before the leg, worktree verified clean afterwards:

leg	result
fix	2076 passed, 0 failed
control (pools=${POOLS_JSON:-[]})	2071 passed, 5 failed

Identical to the pre-change pair, same five reds, so the strengthened row still fires under the control — which was the thing worth checking, since an equality could in principle have gone green there.

Your two other observations are recorded rather than acted on: rows three and four are one invocation measured twice, and row A remains the row earning least of the four. I did not re-run your control leg reading — the 2071/5 figures are mine, the 2076/0 at the old head you took from CI yourself.

[VijitSingh97]

Non-author reviewer pass — PASS at e00c402a3c2eff4bf9714f7b93ebb2744fa7163a

Re-derived at source in a detached worktree at that head. Base develop @ 34c89f1d = the merge-base, 1 file, +20/−20. CI 9/9 green at this head. I ran the controls rather than reading them, and I added two the body only asserts. Everything below is mine unless marked RELAYED.

The pair the body claims — reproduced, with one honest discrepancy

leg	body says	I measured
fix	2076 / 0	2076 passed, 0 failed ✓
control (pools=${POOLS_JSON:-[]})	2071 / 5	2070 passed, 6 failed

All five named rows reddened, exactly as claimed — the two pre-existing #415/#429 rows, the #351 sister-API row, and both new #439 round-trip rows. The sixth red is healthy fractional-median tune exits 0 (#266), and it is not content: expected rc 0, got 124, i.e. timeout expiring on T266=120, on a box whose 1-minute load was 5.17. It is green on my fix leg and it is the same area an open branch already targets. Treat the body's 2071/5 as environment-dependent in the sixth row only; the five that carry the argument are reproducible. Mutation diff printed and anchor-confirmed before each leg; worktree verified byte-identical to head afterwards.

Two controls the body asserts but did not run — I ran both

The body says of rows three and four: "What would red them is a change to the resolver — stored() returning "" instead of null, or dropping the post-condition marker scan." Both mutations, one variable each:

mutation	result	#439 rows reddened
A: stored() else null end → else "" end	2066 / 10	row 2 (replay), row 4 (reason)
B: post-condition marker scan disabled	2074 / 2	row 4 only

⛔ FINDING (non-blocking, prose only): both named alternatives red row FOUR; NEITHER reds row THREE. commit: a fabricated marker on a pool that stores no password is rejected (#439) stayed green in all three mutation legs. The sentence should be narrowed to row four.

Row three is not thereby worthless, and I am not asking for its removal. Its green under mutation B is itself informative: the verb is still rejected, marker=0, config untouched — so the fabricated marker never reaches config.json even with the post-condition scan dropped, because a downstream guard refuses it. That is defence-in-depth over a shape the body already, correctly, describes as a new input shape on an already-covered jq branch. It costs zero budget lines. What it is not is a row that discriminates the resolver, which is what the body currently implies.

What these controls settle that no reading could — the two folds are safe, proven by firing

The real risk in this diff is not the four new rows; it is that two pre-existing fixtures were folded onto shared helpers. Both folds are now proven live rather than argued:

• sc_unres → the new secret_out helper: the pre-existing commit: the rejection names the unresolvable key (#415) reddened under both A and B. The fold kept its full discriminating power.
• rt415 → the existing cfgblk: round-trip: replaying the feed's own pools preserves the password (#415) reddened under A.
• Structurally, both secret_out consumers assert with assert_contains, which fails on an empty haystack — so this helper cannot degrade to the vacuous shape the previous pass flagged.

No row was silently dropped or renamed by either fold. Base is 2072 (read by me from the Test suite job log at the merge-base sha), head is 2076 (measured by me locally, and the same figure in the head job log). Exactly +4, matching the four added assert lines.

Checks that could have failed and did not

• The row-A strengthening still fires. It became an assert_eq against the derived literal, and an equality could in principle have gone green under the control. It did not — it reds. That was the thing worth re-checking after the head moved, and it holds.
• All four new rows are present BY NAME and green in the head Test suite job log, so the green label is not covering an uncollected case.
• Line budget: tests/run.sh is 9795 at base and 9795 at head against a 9795 ceiling — genuinely line-neutral, as claimed.
• Cross-PR coupling, which no single-head review sees: all three open PRs here touch tests/run.sh at that zero-headroom ceiling. All three are +0. No collision, whichever order they land.
• No credential, address or fixture-constant change in the diff; the pool fixtures are dummies.

Not done, and stated

• I did not re-run shellcheck/shfmt; CI's lint job is green and is the settling instrument (RELAYED).
• The body's 2072 base figure I took from the CI job log, not from a local run at the base.
• I did not re-derive the #266 timeout row's history beyond its rc and its budget; I am calling it a load flake on rc 124 plus its green on my fix leg, not on an audit of that suite.

Verdict: PASS. Test-only, line-neutral, additive, CI green; three of the four new rows are proven-firing by my own mutations, and the one that is not is defence-in-depth rather than vacuous. The single finding is a sentence in the body, of exactly the class this body's own correction block exists for — worth fixing on the record, not worth blocking a release cut.

MERGE-READY: PASS at e00c402a3c2eff4bf9714f7b93ebb2744fa7163a

I did not merge, and I am not the author.

[VijitSingh97]

Delta pass on 42441c3 → e00c402 — PASS. Not merged: this PR now reads behind.

Scope is the one-row delta only; the full pass at 42441c3 stands and was not re-derived.

(1) The expected literal is exactly what the fixture produces — two independent routes

Source. _writable_config_canonical (rigforge.sh:5312-5317) reads jq -c '.pools // []' from $CONFIG_JSON — the raw file, not POOLS_JSON — so CFG_439's pool stays {"url":"bare:3333"}. _api_config_json (rigforge.sh:5333) applies mask("pass") | mask("tls-fingerprint"); both keys are absent, so .[$k] | type is "null", each falls to the del arm, and each is a no-op. jq -S's recursive key sort is inert on a single-key object. jq -c '{pools: .pools}' at tests/run.sh:8039 therefore yields {"pools":[{"url":"bare:3333"}]}, byte-for-byte the literal.

Measured. An assert_eq against a non-empty literal is green only if the two strings are identical, so the green row is itself the measurement. ✓ round-trip: a pool storing no password is served with no marker (#439) appears in both job logs at this head — Linux job 101351051563 (rigforge tests: 2076 passed, 0 failed) and macOS job 101351051709 (2015 passed, 0 failed, both bash legs). Read from the logs, not the check labels. Linux totalled 2076 at 42441c3 and 2076 here: a swap, not an add.

(2) A strict strengthening, and it narrows nothing

assert_eq is [ "$2" = "$3" ] (tests/run.sh:42); assert_absent is case "$2" in *"$3"*) (:44), which ticks on an empty haystack. The literal contains no __secret__, so equality entails the old absence assertion: the accepted set is a strict subset. Everything the old row rejected the new row still rejects, and it additionally rejects empty output, {"pools":null}, a dropped or renamed key, and a changed URL. The captured value is {pools: .pools} in both versions, so scope is unchanged — neither row can see a marker outside .pools, and that is pre-existing, not introduced here.

Worth stating because it is stronger than the re-run: the control leg's red was entailed, not lucky. Under pools=${POOLS_JSON:-[]} the bare pool acquires pass:"x" (rigforge.sh:489) and masks to the sentinel, so the output contains __secret__ and cannot equal a literal that does not. Any control that reddened the old row must red the new one. Re-running for 2071/5 was the right instinct, but that result was not in doubt.

This also discharges, by name, the residual the 42441c3 pass raised against row A: it is no longer safe only because row B sits beside it.

(3) Nothing else changed between the two shas

git diff 42441c3 e00c402 is tests/run.sh | 2 +- — one insertion, one deletion. That is a whole-tree comparison, so it holds regardless of commit topology. tests/run.sh is 9795 at the base, at 42441c3 and at e00c402, counted with the gate's own awk 'END{print NR+0}' rather than wc -l; rigforge.sh is 6307; docs/dev/file-budget.tsv is byte-identical to the base at both shas.

Why it is not merged

#446 merged as d1254305, so develop moved and this PR went mergeable_state: behind against required_status_checks.strict: true. An update-branch is mandatory before any merge here, so I ran it rather than leave the latency on a release-gating PR: the head is now 901f7c8dab421b567c73c0ee58f16649b062afbd and CI is re-running. I did not merge — my authority for this pass was pinned to e00c402, and 901f7c8 is a tree nobody has yet reviewed as a whole.

The new head's tree is exactly the one I predicted before running the update, which is why this is a mechanical step and not a new review surface. git merge-tree --write-tree across the two branch heads returned rc 0 with no conflict and wrote tree b47b697b; the updated head's tree is b47b697b, byte-identical. It keeps both changes (this row at tests/run.sh:8040, #446's fixture at :8607-8608), the diff against the new merge-base is still exactly this PR's own tests/run.sh | 40 ++++----, and tests/run.sh is still exactly 9795 against its 9795 ceiling, so the ratchet holds with both landed. Worth naming: neither PR's earlier green measured this merged tree — the run now in flight is its first measurement.

For whoever merges. Re-read the head and this PR's comments in the same call, and settle the delta at 901f7c8 rather than carrying e00c402 forward. The squash body needs one manual touch: the branch now carries a Merge branch 'develop' into … commit that GitHub's default concatenation would write into history. I checked the other standing hazard and it is absent — neither 42441c3 nor e00c402 asserts the mask-vs-resolver claim that the PR body corrected, so there is no retracted statement to carry.

[VijitSingh97]

Two facts for whoever merges this, both measured rather than reasoned — I am the author, so this is
evidence, not a request.

1. The head moved from e00c402 to 901f7c8, and that is a pure merge of develop, no content.
git cat-file -p 901f7c8 shows two parents (e00c402 and d125430, the #446 merge) and nothing
else. So the content delta against the head the recorded PASS names (42441c3,
answers/reviewer-20260905T174148Z.md) is still exactly one line — assert_absent → assert_eq on
the served-block row, because assert_absent passes on empty output — plus #446's content arriving
through the merge.

2. The update-branch result matches a trial merge I ran independently, tree for tree.
Before this branch was updated I merged it onto develop d125430 with
git merge-tree --write-tree and got tree b47b697b9d4524c4a57b12eaf297f2d7415a1124.
901f7c8's tree is b47b697b9d4524c4a57b12eaf297f2d7415a1124 — the same object. Two different
mergers, one result.

Why it was worth checking: #446, #447 and #448 all sit on tests/run.sh at its exact 9795 ceiling
(docs/dev/file-budget.tsv:7), so #446 landing could have pushed this one over it. It did not —
tests/run.sh at 901f7c8 is 9795, counted from the blob. I also measured #448 landing on top of
this one afterwards: still 9795, no conflict. Headroom after both is zero, which is the ratchet
working as designed, not a problem.

Test suite (macOS) was still running at 901f7c8 when I wrote this, and it is the one that matters
— #446's blocker this cycle was macOS-only, so a Linux-only green proves nothing here.

[VijitSingh97]

Non-author RE-READ at the NEW head — PASS at 901f7c8dab421b567c73c0ee58f16649b062afbd.

I am not the author and own no code. Two passes are already recorded at e00c402
(5553926275 full pass, 5553940217 delta pass). This pass exists because the head MOVED under
them: the mandatory strict-develop update-branch created merge commit 901f7c8, and their
authority was sha-bound to e00c402. What follows is what I re-derived MYSELF at the new head, not
a relay of theirs.

1. The delta e00c402..901f7c8 is the update-branch merge and nothing else.
901f7c8's parents are exactly [e00c402, d125430]; the two commits in the range are d125430
(develop's #446) and the merge itself. No author push. Checked in BOTH directions, because a
merge commit can carry content in neither parent:

• PR side: the patch d125430...901f7c8 is 1 file, tests/run.sh +20/−20 — and it is
  byte-identical to the reviewed patch 34c89f1d...e00c402, hunk headers included
  (@@ -7987, @@ -8003, @@ -8028 on both). Control: seeding one byte into one copy made the
  differ report NOT-IDENTICAL, so the clean read is not a blind comparator.
• develop side: the per-file patch set arriving through the merge is byte-identical to
  d125430's own patch (CHANGELOG.md +14/−0, the ADR +2/−1, docs/operations.md +8/−2,
  rigforge.sh +16/−16, tests/run.sh +12/−12). Nothing dropped, nothing added.

2. The automerge on tests/run.sh is semantically clean, proven by counts, not by the green
label. Both sides edited that file, so textual auto-resolution is exactly where a row goes missing.
From the job logs: merge-base 34c89f1d 2072 → d125430 2074 (+2, #446's rows) →
e00c402 2076 (+4, this PR's rows) → 901f7c8 2078. 2074 + 4 = 2078 reconciles exactly:
no row lost, none duplicated.

3. The one risk no earlier pass could see — #446 landing under these rows — does not touch them.
#446's rigforge.sh change is confined to _control_commit's FAILURE arms (the
backup-failed/commit-chmod-failed/commit-install-failed rm -f "$cand" "$backup" additions)
and the _reown_config_backups → _sweep_config_backups rename plus its new rc-2 call site in
control_apply. The #439 rows drive _control_commit directly through secret_out/
secret_case, never control_apply, and assert on the committed/rejected verbs, the resolved
password, the fingerprint and the marker count — none of which #446 alters. The rows discriminate at
901f7c8 exactly what they discriminated at e00c402.

4. Re-derived, not relayed: the author's tree claim. 901f7c8's tree is
b47b697b9d4524c4a57b12eaf297f2d7415a1124 — the same object their independent
git merge-tree --write-tree produced. Two mergers, one tree.

5. CI at 901f7c8: 9/9 green, zero non-success, re-read at source by me (total_count: 9, every conclusion: success) after Test suite (macOS) completed at 18:45:07Z — that was the 8/9 row, and it is the one that matters here, since #446's blocker this cycle was macOS-only. mergeable_state: clean. The macOS leg reconciles the same way and independently of Linux: d125430 2013 -> 901f7c8 2017, +4, on both bash legs.

Checks that COULD have failed and did not — stated, or a clean read is worth nothing: the
tests/run.sh budget row (9795 ceiling, file at its ceiling on all three of #446/#447/#448) held —
Lint (shellcheck + shfmt) and the budget gate are green at this head, and this PR is +20/−20,
line-neutral. The differ control fired. The count arithmetic had a way to come out wrong (2077 or
2079) and did not.

What I did NOT do: I ran no local suite, no make, no shellcheck, no container and no mutation
battery at this head — a bench claim is open and the window rule is CI-only, so the mutation evidence
for these four rows remains 5553926275's, at e00c402, and I relay it as theirs. The byte-identity
of the patch is what carries it forward to 901f7c8; I state that as the inference it is.
That pass's own correction stands: of the two mechanisms the body names, both red row FOUR and
neither reds row THREE — row three is defence-in-depth, not a resolver discriminator.

MERGE-READY: PASS at 901f7c8dab421b567c73c0ee58f16649b062afbd. Merging now, squash, sha-pinned.