hub F0.3 follow-up: the prefix-twin test passes on the vulnerable code — a fixture that cannot reach the state it names

[dp-web4]

Follow-up from the #708 review (merged at 5513af9). The shipped control is correct and is guarded by two tests that provably fail on the reverted vulnerable shape. This is about a third test that cannot fail, and a fixture that misdescribes itself.

Measurement

I reverted the fix in a throwaway worktree — dropped the sha.len() != 40 hex check from RatifiedManifest::validate() and restored the old comparison:

fn sha_matches(a: &str, b: &str) -> bool {
    let (a, b) = (a.trim(), b.trim());
    let n = a.len().min(b.len());
    if n < 7 { return false; }
    a[..n].eq_ignore_ascii_case(&b[..n])
}

Then re-ran cargo test -p hub-lib ratified:::

test	on vulnerable code
an_abbreviated_sha_is_refused_at_admission	FAILED ✅ real guard
a_non_ascii_sha_yields_a_verdict_not_a_panic	FAILED (panic) ✅ real guard
two_commits_sharing_a_prefix_are_never_interchangeable	PASSED ❌ proves nothing

Why the third one cannot fail

The prefix defect only manifests when the manifest sha is short. The test pins the manifest at SHA_A, a full 40. With a 40-char left operand, n = min(40, len(build)) and the old code compares every character it has — returning Stale exactly as the fixed code does. Both implementations satisfy the asserted property, so the assertion separates nothing. It is a clearing arm where both options behave the same: confirmation, not verification.

This matters mainly because the PR record claims otherwise — the test's own doc comment says "This cannot pass by accident under a byte-compare-only fix, which is why it is the discriminating one," and the closure note repeats it. That claim belongs to an_abbreviated_sha_is_refused_at_admission.

The fixture defect that points at the same gap

// Full 40-hex commit ids. SHA_A and SHA_PREFIX_TWIN deliberately share the
// first 7 characters ...
pub(super) const SHA_PREFIX_TWIN: &str = "abcdef19999999999999999999999999999999f";  // 39 chars

39, not 40. A 39-char string can be neither an admitted manifest sha (validate() refuses it) nor a build stamp (40-hex by construction), so the test exercises a state that cannot occur in production — while the equal-length, distinct, prefix-colliding pair, which is the case a human actually creates by typing an abbreviation, is never exercised at all.

The test guards its fixture with assert_eq!(&SHA_A[..7], &SHA_PREFIX_TWIN[..7]) and assert_ne!, but nothing asserts the length. The same file gets this right elsewhere: a_non_ascii_sha_yields_a_verdict_not_a_panic asserts payload.len() == 41 and !payload.is_char_boundary(40) — proving the fixture can reach the state it names, before testing anything about it.

Suggested fix

1. Make SHA_PREFIX_TWIN a full 40 hex sharing 7 with SHA_A, and add assert_eq!(SHA_PREFIX_TWIN.len(), 40) to the fixture guard (all three constants, cheaply: a loop over [SHA_A, SHA_B, SHA_PREFIX_TWIN]).
2. Rewrite the test so the abbreviation is on the manifest side, which is where the defect lived — an abbreviated manifest sha must be refused at admission and therefore never reach a comparison with its twin. That version fails on the vulnerable code.
3. Correct the doc comments on the test and the constants so the record no longer names the wrong test as the discriminating one.

Low urgency — no behaviour is wrong. The risk is that someone trimming tests later keeps the one that proves nothing and drops the one that carries the regression.