fix(vfs): harden completion and path boundaries - #20
Merged
farhan-syah merged 2 commits intoJul 26, 2026
Merged
Conversation
…y backend Adds tests asserting canonical_native_path/resolve_native_path reject escaping and drive-letter components, and that the in-memory VFS treats equivalent path spellings as one lock domain and one file, matching the native backends' contract. Also switches two String clones in MemVfs away from unnecessary to_string() calls.
farhan-syah
force-pushed
the
codex/upstream/vfs-completion-validation
branch
from
July 26, 2026 23:06
431d6b1 to
937de16
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.
Harden VFS completion, range, and path boundaries
Summary
Deterministic request metadata is validated before a vectored operation mutates
any file or caller buffer, and backend completion values are validated before
PageDB uses them as counts, indices, offsets, or slice boundaries.
The contract is narrow and worth stating precisely: it is about request
validation, not rollback. A runtime device or filesystem failure part-way
through an operation is still reported as it happens. What can no longer happen
is a valid prefix of a batch being applied before a later deterministic error is
discovered.
No public API, on-disk format, feature flag, or dependency changes.
Why the backends need a shared rule
The VFS trait spans implementations with very different completion models:
dispatch_iorequires signedoff_toffsets and invokes block handlersasynchronously.
easy to misclassify.
io_uringreturns indexed completion queue entries that can be short,duplicated, missing, negative, or bound to the
-1offset sentinel.from browser APIs.
Each backend previously did some of its validation inside the same loop that
executed I/O, and some trusted completion counts before converting or slicing
with them. Individually these are small boundary bugs; together they mean the
pager cannot rely on one observable contract.
What changed
Shared validation layer
src/vfs/traits.rsgrows target-gated helpers for the boundaries shared by morethan one backend: canonical logical paths and safe root resolution, Win32
ReadFilelength validation, native read-completion bounds, indexed batchcompletion uniqueness,
io_uringpositioned-offset sentinel rejection, OPFSJavaScript safe-integer ranges and byte counts, signed native truncate lengths,
and IOCP immediate-success / pending / immediate-EOF classification.
These are production seams, not test-only ones: GCD, IOCP,
io_uring, and OPFScall the same functions the portable unit tests exercise.
The write side folds into the completion rules already on
main.checked_write_progressis the single write-side validator —write_all_atandevery backend that completes a short write go through it — so there is one
definition of what progress a backend may claim. Overreported counts surface as
the typed
PagedbError::VfsContractViolatedrather than free-text I/O errors,matching the read side.
Logical paths and lock identity
Root-backed implementations joined caller paths directly after trimming a
leading slash.
..could escape the configured root, and equivalent names suchas
/db.lock,db.lock, and separator variants resolved to the same physicalfile while occupying different in-process lock domains.
Paths are now canonicalized before use, and every component is judged in
isolation rather than by parsing the path as a whole. That distinction matters:
a platform prefix is only recognised in leading position, so a drive letter
deeper in the path (
a/C:/b) parses as an ordinary name — and pushing it wouldreplace the root outright instead of extending it. A colon is rejected on every
target, not just the one that gives it meaning, because a path that names a file
under the root on one platform and a different volume on another is precisely
the portability this store is supposed to guarantee.
The in-memory backend follows the same contract as the native ones. It is the
backend nearly every test runs on, so leaving it keyed by raw strings would mean
the advertised contract was the least exercised one.
Memory and Tokio
MemFile::write_atuses checked range arithmetic, and its vectored writercomputes and validates every range before resizing or copying — preventing both
a panic and a partially applied prefix.
TokioFilechecksoffset + lenbeforeseeking, and validates a full batch before the first seek or write. Temporary
directory helpers include an atomic sequence and use
create_dir, so acollision is observable rather than a silently shared fixture root.
Apple GCD
Reads and writes validate the complete non-empty byte range against
libc::off_t, not just the starting offset; vectored operations validate allranges before the first one runs; truncate rejects lengths the signed syscall
cannot represent.
The block-handler conversion is an explicit pointer cast rather than a
pointer-to-pointer
transmute.block2'sBlock<F: ?Sized>carries aPhantomData<F>, so the type isSizedand the cast is well-formed — thispreserves the documented ABI workaround while satisfying strict Clippy.
Windows IOCP
Vectored reads prevalidate every
ReadFilelength before touching callerbuffers, and read counts are checked before slicing or zero-filling.
Overlapped operations distinguish three start states: pending has a completion
packet to drain; immediate success also has one for an associated handle;
immediate
ERROR_HANDLE_EOFon a read is an empty read, not a storage failure.Every queued path drains and validates the matching
OVERLAPPEDbefore thestack allocation is released. Vectored writes retry positive short completions,
reject zero or overreported progress, and advance the positioned offset for the
unwritten suffix.
Linux and Android
io_uringPositioned reads reject the
u64::MAXvalue thatio_uringreads as the-1current-position sentinel for non-empty requests.
Batch completion uses one optional slot per request. Out-of-range keys remain
ignorable, but a duplicate in-range key is an error rather than being counted
twice and letting another request appear complete; every expected slot must be
present before results are returned.
Vectored writes validate all ranges before submission, skip already-complete
empty requests so a zero-byte completion is unambiguously impossible progress,
map completions back to the correct original request, and complete positive
short writes with positioned suffix retries. The ring mutex is released before
those retries so the ordinary single-write path can reacquire it.
Browser OPFS worker boundary
Rust validates offsets and lengths before converting them into JavaScript
numbers, and rejects non-finite, fractional, negative, unsafe, overreported, or
platform-unrepresentable values coming back before casting them. Read payload
length must match the validated count. Vectored requests are prevalidated before
the first dispatch, and positive short writes go through the common suffix-retry
helper. The worker rejects an invalid physical read count before slicing its
buffer.
This is static and target-compile proof, not a browser runtime claim.
Regression coverage
Portable unit tests in
src/vfs/traits.rscover the helpers directly:separator and leading-slash normalization, parent/current-directory rejection,
drive-letter rejection wherever it appears, root containment for resolved paths,
read and write completion bounds, indexed completion uniqueness, the
io_uringsentinel, OPFS safe-integer edges, signed truncate lengths, and every overlapped
start classification.
Backend integration suites (
tests/vfs_memory.rs,vfs_tokio.rs,vfs_gcd.rs,vfs_iocp.rs,vfs_iouring.rs) cover round-trip vectored I/O, EOF zero-fill,truncate and read-only behavior, root-escape rejection, equivalent lock
spellings, an unrepresentable starting offset, and a range whose first byte is
off_t::MAXbut whose last byte crosses the signed boundary.The implementation was developed against concrete failing checks: the in-memory
backend panicked on an overflowing vectored request while Tokio applied the
valid first write before rejecting the second; GCD filled an early read buffer
and applied an early write before discovering a later unrepresentable offset; a
GCD request starting at
off_t::MAXwith two bytes passed the old start-onlycheck; Tokio opened
../escaped-fileoutside its root;/db.lockanddb.lockoccupied different lock entries; and a drive-letter component mid-path resolved
outside the root entirely.
The Windows and Apple suites compile under their targets; hosted CI on those
platforms is the runtime authority.
Verification
On this branch, rebased onto
main:cargo nextest run --all-features --no-fail-fast: 583 passed, 5 skipped;PAGEDB_INVARIANT_CHECKS=1: 583 passed, 5 skipped;cargo fmt --all --check: pass;cargo clippy --all-targets --all-features -- -D warnings: pass;cargo check --no-default-features: pass;x86_64-pc-windows-gnu,aarch64-apple-darwin,aarch64-apple-ios,aarch64-linux-android,armv7-linux-androideabi,wasm32-wasip1, andwasm32-unknown-unknown --features opfs;cargo doc --no-deps --all-features: no warnings;node --checkon the OPFS worker: pass.Review focus
immediate EOF.
io_uringcompletion-to-request mapping and the lock release beforeshort-write suffix retries.
prefixes, without permitting root escape.
the
VfsFilewording.Merge notes
No on-disk format, snapshot format, catalog encoding, handle mode, or dependency
changes. The branch is rebased on current
main, so it builds on the positionalcompletion helpers already there rather than duplicating them.