test(adapters): collapse the sanitize_path suite, and fix the bypass it found - #13
Merged
Conversation
…it found
Two hand-written assertion lists (`sanitize_strips_traversal` and
`sanitize_path_neutralizes_traversal_and_drive_letters`) had grown to 88
assert_eq! lines covering one 25-line pure function. 69 of those arrived one
per commit — 12% of the repository's history is single-assert commits against
sanitize_path, which is unreviewable and still only ever tests inputs someone
thought of.
Replaced with:
- `sanitize_path_table`: all 87 unique cases preserved verbatim as data,
grouped by what they demonstrate. Extracted mechanically (with a parser
that handles Rust raw strings ending in a backslash, e.g. r"src\app\")
and cross-checked against the assert_eq! count in each block, so no case
was dropped in translation. Adding a case is now a row, not a commit.
- `sanitize_path_invariants`: six properties that must hold for ALL input —
no `..` segment survives, no empty/`.` segments, never absolute, no
control or zero-width characters, no drive prefix, and idempotence.
The property test immediately found a real bypass in sanitize_path itself:
sanitize_path("A\x7f:") -> "A:" (expected "")
The drive-letter check ran against the RAW segment, before control and
zero-width characters were filtered. Putting any such character between the
letter and the colon made the segment longer than 2 bytes, so the check missed
it; the character was then stripped and the drive prefix survived into the
output. "C\u{200b}:/Users/dev/x.rs" leaked through the same way.
Fixed by filtering first and testing the filtered segment. Windows drive
prefix neutralization is a stated boundary-hardening control, so this closes a
real hole in it rather than a theoretical one. Four regression cases added to
the table alongside the property.
proptest is a dev-dependency only, with default-features off (no fork/timeout
machinery) to keep the dependency graph small for the egress audit.
Verified: fmt, clippy, cargo test --locked --all-targets (346 pass / 4
ignored), no_network_in_archive, egress-check.sh, toolchain-pin-check.sh.
The property test was run 5 times over fresh case sets to confirm stability.
hsusul
force-pushed
the
test/sanitize-path-property
branch
from
August 22, 2026 22:10
0776864 to
4afd3fd
Compare
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
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.
Problem
Two hand-written assertion lists covered one 25-line pure function with 88
assert_eq!lines. 69 of them arrived one per commit — 12% of this repository's entire history is single-assert commits againstsanitize_path. That history is unreviewable, and enumerating slash/dot permutations by hand only ever tests inputs someone thought of.Change
sanitize_path_table— all 87 unique cases preserved verbatim as data, grouped by what each demonstrates. I extracted them mechanically rather than by hand, with a parser that correctly handles Rust raw strings ending in a backslash (r"src\app\"), and cross-checked the extracted count against theassert_eq!count in each block so nothing was silently dropped. Adding a case is now a row, not a commit.sanitize_path_invariants— six properties that must hold for all input: no..segment survives, no empty or.segments, never absolute, no control or zero-width characters, no drive prefix, and idempotence.The property test found a real bug
Not a test problem — a genuine bypass in
sanitize_path:The drive-letter check tested the raw segment, before control and zero-width characters were filtered out. Slipping any such character between the letter and the colon made the segment longer than 2 bytes, so the length check missed it — then the character was stripped a moment later and the drive prefix survived into the output.
Windows drive-prefix neutralization is a stated boundary-hardening control, so this was a real hole in it, not a theoretical one. Fixed by filtering first and testing the filtered segment. Four regression cases are in the table alongside the property that caught it.
This is the argument for the change in one example: 88 hand-written cases never found it; the property found it on the first run.
Dependency
proptestas a dev-dependency only,default-features = false— drops the fork/timeout machinery and itsrusty-fork+wait-timeoutdeps, keeping the graph small for the egress audit.egress-check.shstill passes.Verification
cargo fmt --all -- --checkcargo clippy … -D warningscargo test --locked -p lore-core -p lore-ipc --all-targetscargo test -p lore-core --test no_network_in_archive./scripts/egress-check.sh./scripts/toolchain-pin-check.shsanitize_path_invariants× 5 runs