fix(spawn): compare the no-mistakes delivery target by identity, not string equality #8

Merged

john merged 2 commits from fm/fm-guard-compares-wrong-field into main 2026-08-10 20:27:51 +00:00

Conversation 0 Commits 2 Files changed 5 +468 -2

john commented 2026-08-10 19:53:29 +00:00

Owner

Copy link

The defect

The delivery-target guard added in #7 refuses every Forgejo project in the fleet, including this repository, on a mismatch that is not real.

no-mistakes init stores the upstream URL with its userinfo masked — the literal string redacted in place of the SSH username:

stored: ssh://redacted@git.pyr0.dev:2222/pyr0/johnvenice-dev.git
origin: ssh://git@git.pyr0.dev:2222/pyr0/johnvenice-dev.git

The guard compared those with string equality, so it failed on a redaction artifact.

Why this is a tool defect, not stale data

• Re-registering reproduces it. Verified on v1.41.2, then again on v1.46.0 after upgrading — a fresh eject + init still wrote redacted. Upgrading does not fix it.
• It affects every Forgejo project plus this repository. codelight escapes only because its GitHub URL is SCP-style with no ssh:// userinfo component.
• The real push destination is correct everywhere. Every project's no-mistakes bare mirror has an origin pointing at the correct Forgejo URL.
• Deliveries succeed with the masked value present. wishlist-galore ran the full pipeline four times on 2026-08-09 after its masked value was written; PRs 112, 113 and 114 all landed on Forgejo correctly.

So the stored column is a masked display value, not the target the tool pushes to.

The fix

Compare by identity — host, port and repository path — and ignore userinfo. Both ssh://user@host:port/path and SCP-style git@host:path forms are normalised; an unparseable URL refuses rather than passes, preserving the guard's fail-closed design.

The literal mask string is deliberately not special-cased: matching it would break the moment the tool changes its mask, and would silently accept a genuinely wrong username.

What still refuses

The protection #7 was written for is intact — a GitHub host where Forgejo is expected, a wrong repository path, an unrecorded target, an unreadable database, and an unparseable URL all still refuse the spawn.

Tests

This function shipped with no coverage, which is how it went out broken.

Correction to an earlier claim in this description: only one test was genuinely red before the fix — test_no_mistakes_masked_userinfo_accepts_spawn. The other spawn-level cases already passed pre-fix, because plain string equality is over-strict rather than under-strict: every refusal case it should reject, it already rejected. Those tests are regression guards for the new normalisation, not reproductions of the bug. tests/fm-nm-target-lib.test.sh covers new code that had no pre-fix equivalent to be red against.

That distinction matters: a test that passes against broken code proves nothing about the bug, and describing all of them as red would have overstated the evidence.

## The defect The delivery-target guard added in #7 refuses **every Forgejo project in the fleet**, including this repository, on a mismatch that is not real. `no-mistakes init` stores the upstream URL with its userinfo masked — the literal string `redacted` in place of the SSH username: ``` stored: ssh://redacted@git.pyr0.dev:2222/pyr0/johnvenice-dev.git origin: ssh://git@git.pyr0.dev:2222/pyr0/johnvenice-dev.git ``` The guard compared those with string equality, so it failed on a redaction artifact. ## Why this is a tool defect, not stale data - Re-registering reproduces it. Verified on v1.41.2, then again on **v1.46.0** after upgrading — a fresh `eject` + `init` still wrote `redacted`. Upgrading does not fix it. - It affects every Forgejo project plus this repository. `codelight` escapes only because its GitHub URL is SCP-style with no `ssh://` userinfo component. - **The real push destination is correct everywhere.** Every project's no-mistakes bare mirror has an `origin` pointing at the correct Forgejo URL. - **Deliveries succeed with the masked value present.** wishlist-galore ran the full pipeline four times on 2026-08-09 after its masked value was written; PRs 112, 113 and 114 all landed on Forgejo correctly. So the stored column is a masked display value, not the target the tool pushes to. ## The fix Compare by identity — host, port and repository path — and ignore userinfo. Both `ssh://user@host:port/path` and SCP-style `git@host:path` forms are normalised; an unparseable URL **refuses** rather than passes, preserving the guard's fail-closed design. The literal mask string is deliberately **not** special-cased: matching it would break the moment the tool changes its mask, and would silently accept a genuinely wrong username. ## What still refuses The protection #7 was written for is intact — a GitHub host where Forgejo is expected, a wrong repository path, an unrecorded target, an unreadable database, and an unparseable URL all still refuse the spawn. ## Tests This function shipped with no coverage, which is how it went out broken. **Correction to an earlier claim in this description:** only **one** test was genuinely red before the fix — `test_no_mistakes_masked_userinfo_accepts_spawn`. The other spawn-level cases already passed pre-fix, because plain string equality is *over*-strict rather than under-strict: every refusal case it should reject, it already rejected. Those tests are regression guards for the new normalisation, not reproductions of the bug. `tests/fm-nm-target-lib.test.sh` covers new code that had no pre-fix equivalent to be red against. That distinction matters: a test that passes against broken code proves nothing about the bug, and describing all of them as red would have overstated the evidence.

john added 2 commits 2026-08-10 19:53:29 +00:00

fix(spawn): compare no-mistakes delivery target by identity, not string equality ... 9ed2d52f33

no-mistakes' own state.sqlite masks the recorded upstream_url's SSH
userinfo as the literal string "redacted" (confirmed across versions
by re-registering a clone - it is a tool behavior, not stale data).
validate_no_mistakes_delivery_target() compared that recorded value
against the live git origin with plain string equality, so every
project reached over ssh://user@host/... failed the comparison on the
masked username alone and was refused a no-mistakes spawn, even though
the real push destination (the bare mirror's own origin remote) was
correct. Only the codelight project escaped it, because its
git@host:path form carries no ssh:// userinfo to mask.

bin/fm-nm-target-lib.sh normalizes both URLs to host/port/path,
ignoring userinfo, and compares that identity instead. It handles both
fleet URL shapes (ssh://[user@]host[:port]/path and scp-style
[user@]host:path) and refuses on anything it cannot parse, so an
unrecognized shape still fails closed rather than silently matching.
The literal mask string is never special-cased, so this survives the
mask changing and still catches a genuinely wrong username's host or
path drifting.

Tests prove the masked-userinfo and scp-style acceptance cases were
red against the prior plain-equality comparison (only fixed by this
change), plus regression coverage for different host, different path,
different port, an unrecorded project, an unreadable database, and an
unparseable recorded value - all of which must keep refusing.

no-mistakes(document): list fm-nm-target-lib.sh in the bin/ toolbelt inventory 0b320a095a

john merged commit a0763eb254 into main 2026-08-10 20:27:51 +00:00

john referenced this pull request from a commit 2026-08-10 20:27:52 +00:00

Merge pull request 'fix(spawn): compare the no-mistakes delivery target by identity, not string equality' (#8) from fm/fm-guard-compares-wrong-field into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

john/firstmate!8

No description provided.