Two guards that looked like they held (v0.3.2) - #7
Merged
Conversation
Found by independent review, both the same family: a check whose condition can never fail for the inputs it exists to catch. The canonicalization guard read `unless string.valid_encoding?` — always true on an ASCII-8BIT string, and BINARY is exactly what Rack and CDN headers deliver, so ten request-evidence values passed unchecked. Invalid bytes emitted canonical JSON that was not itself valid UTF-8, which RFC 8785 forbids and a verifier in another language may reject or normalize into a different digest: the "still verifiable years later" promise failing silently. Measured before touching it — bytes that ARE valid UTF-8 canonicalize byte-identically tagged either way, so no digest anyone has written changes, and a test pins that. Genuinely invalid bytes are now refused at write time, and a stored value that can no longer be canonicalized reports a binding mismatch rather than raising out of the integrity check. `record_ip_geolocation(country: nil)` silently enabled country, region, and city: with plain nil keyword defaults, an explicit nil is indistinguishable from an omitted keyword, so the coarse-trio default caught it. The realistic caller is `country: settings[:geo]` with an empty setting — enabling a category of personal data as a side effect, the one thing the frictionless pass never relaxed. A sentinel tells the two apart. (The proposed one-line fix would not have worked: with every value nil, a true/false test is still false.) Also: a scaffolding `legal_basis_reference:` is refused like a scaffolding `because:` — it lands in the compiled revision permanently, where a TODO reads as a determination. And the installer's prompts stop promising refusals the 0.3.x relaxation removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013A6ZmfmFuQ2z3GyvQCuECA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Independent review found two no-op guards. Canonicalization:
valid_encoding?is always true on ASCII-8BIT — the encoding Rack delivers — so invalid bytes produced canonical JSON that was not valid UTF-8 (RFC 8785 violation; cross-language verifiers may derive a different digest). Measured first: valid bytes canonicalize byte-identically either way, so no existing digest changes, and that is now pinned by a test; invalid bytes are refused, and an uncanonicalizable stored value reports a mismatch instead of crashing the integrity check. Geolocation:country: nilsilently enabled the coarse trio (explicit nil indistinguishable from omitted under plain nil defaults — the suggested one-liner would not have fixed it); a sentinel now refuses it. Plus a scaffoldinglegal_basis_reference:guard and installer prompts that match 0.3.x behavior. 836 runs, 92.47% line / 73.68% branch, rubocop clean.🤖 Generated with Claude Code
https://claude.ai/code/session_013A6ZmfmFuQ2z3GyvQCuECA