Skip to content

fix(vfs): harden completion and path boundaries - #20

Merged
farhan-syah merged 2 commits into
NodeDB-Lab:mainfrom
presempathy-awb:codex/upstream/vfs-completion-validation
Jul 26, 2026
Merged

fix(vfs): harden completion and path boundaries#20
farhan-syah merged 2 commits into
NodeDB-Lab:mainfrom
presempathy-awb:codex/upstream/vfs-completion-validation

Conversation

@presempathy-awb

@presempathy-awb presempathy-awb commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

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:

  • Tokio and the in-memory backend expose ordinary Rust offsets and lengths.
  • Apple dispatch_io requires signed off_t offsets and invokes block handlers
    asynchronously.
  • Windows IOCP uses overlapped operations whose immediate-success behavior is
    easy to misclassify.
  • Linux io_uring returns indexed completion queue entries that can be short,
    duplicated, missing, negative, or bound to the -1 offset sentinel.
  • OPFS crosses a Rust-to-JavaScript number boundary and receives byte counts
    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.rs grows target-gated helpers for the boundaries shared by more
than one backend: canonical logical paths and safe root resolution, Win32
ReadFile length validation, native read-completion bounds, indexed batch
completion uniqueness, io_uring positioned-offset sentinel rejection, OPFS
JavaScript 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 OPFS
call the same functions the portable unit tests exercise.

The write side folds into the completion rules already on main.
checked_write_progress is the single write-side validator — write_all_at and
every 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::VfsContractViolated rather 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 such
as /db.lock, db.lock, and separator variants resolved to the same physical
file 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 would
replace 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_at uses checked range arithmetic, and its vectored writer
computes and validates every range before resizing or copying — preventing both
a panic and a partially applied prefix. TokioFile checks offset + len before
seeking, and validates a full batch before the first seek or write. Temporary
directory helpers include an atomic sequence and use create_dir, so a
collision 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 all
ranges 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's Block<F: ?Sized> carries a
PhantomData<F>, so the type is Sized and the cast is well-formed — this
preserves the documented ABI workaround while satisfying strict Clippy.

Windows IOCP

Vectored reads prevalidate every ReadFile length before touching caller
buffers, 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_EOF on a read is an empty read, not a storage failure.
Every queued path drains and validates the matching OVERLAPPED before the
stack 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_uring

Positioned reads reject the u64::MAX value that io_uring reads as the -1
current-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.rs cover 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_uring
sentinel, 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::MAX but 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::MAX with two bytes passed the old start-only
check; Tokio opened ../escaped-file outside its root; /db.lock and db.lock
occupied 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;
  • the same suite under 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;
  • library checks pass for x86_64-pc-windows-gnu, aarch64-apple-darwin,
    aarch64-apple-ios, aarch64-linux-android, armv7-linux-androideabi,
    wasm32-wasip1, and wasm32-unknown-unknown --features opfs;
  • cargo doc --no-deps --all-features: no warnings;
  • node --check on the OPFS worker: pass.

Review focus

  1. IOCP completion packet ownership, especially immediate success versus
    immediate EOF.
  2. io_uring completion-to-request mapping and the lock release before
    short-write suffix retries.
  3. OPFS raw JavaScript count serialization and safe-integer validation.
  4. Per-component path canonicalization across slash styles and platform
    prefixes, without permitting root escape.
  5. The distinction between deterministic prevalidation and runtime I/O errors in
    the VfsFile wording.

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 positional
completion helpers already there rather than duplicating them.

presempathy-awb and others added 2 commits July 27, 2026 06:58
…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
farhan-syah force-pushed the codex/upstream/vfs-completion-validation branch from 431d6b1 to 937de16 Compare July 26, 2026 23:06
@farhan-syah
farhan-syah merged commit 9441530 into NodeDB-Lab:main Jul 26, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants