alkblobs/tasks/s6-local-symlink-doctrine.md

id, name, status, depends_on, scope, risk, impact, level, tags

id	name	status	depends_on	scope	risk	impact	level	tags
s6-local-symlink-doctrine	S-6 — the local tier's stat/get paths stop following symlinks: one doctrine with walk_committed's loud rejection	completed		narrow	low	component	implementation	wave-10 review-001 security local

Description

Review-001 S-6. The local tier follows symlinks on the stat/get paths while walk_committed rejects them loudly — one behavior, two doctrines. A symlink planted at a committed path makes get/size serve the target (and pre-F-3 made put/commit silently dedup; post-F-3 the no-replace hardlink publish fails on an existing destination — verify that arm holds for a symlink destination).

The current follow points (post-F-3 locations):

• src/local.rs:730 — size: fs::metadata(&committed) follows symlinks.
• src/local.rs:623-625 — get: File::open(&committed) + file.metadata() follow symlinks.
• src/local.rs:491 — the staging discard's stage.exists() follows (staging area, lower stakes — judge whether the doctrine extends).
• The publish path (fsync_publish, src/local.rs:390-415): fs::hard_link(stage, committed) fails when the destination exists — including a symlink destination — so put/commit should already reject loudly. Verify by test (the review's silent-dedup premise predates F-3's no-replace publish).

The fix: symlink_metadata-based checks (portable — the engine has a non-unix read_exact path, so O_NOFOLLOW alone is not the shape) on the stat/get paths: a symlink at a committed path is rejected loudly — the same doctrine as walk_committed's fatal-malformed rejection, not silently served and not silently Missing. Design points, record in Notes:

• The rejection's error shape: Error::Io with a doctrine-naming message (no new variant — the family is ADR-pinned; the F-4 refusal's precedent).
• get's TOCTOU window: the symlink_metadata pre-check before File::open narrows but does not close the race; on unix, OpenOptions custom flags (O_NOFOLLOW) close it. Judge: portable symlink_metadata check as the base, the unix O_NOFOLLOW close as an optional riding fix (the F-3 posture — unix/non-unix agreement — is the precedent for judging both platforms' shapes together).
• Whether has (which rides size, src/local.rs:614-616) inherits the doctrine automatically (it does, if size rejects) — confirm and record.

Tests: a symlink planted at a committed path — get/size/has reject loudly (the doctrine message); put/commit against the planted path fail (the hardlink destination-exists arm — the publish's loud rejection verified); walk_committed's existing loud rejection stays green (the doctrine's other half, unmodified).

Acceptance Criteria

• size/get (and has through them) reject a symlink at a committed path loudly (symlink_metadata-based; the error shape recorded in Notes)
• The unix O_NOFOLLOW TOCTOU close judged (landed or consciously deferred, recorded)
• The publish path's loud rejection against a planted symlink verified by test (the post-F-3 no-replace arm)
• Tests: the planted-symlink round (get/size/has reject; put/ commit fail; walk_committed unmodified green)
• Gates green: cargo test (default), cargo test --all-features server-less, cargo test --all-features + live harness, clippy --all-targets -- -D warnings across the feature variants, cargo fmt --check

References

• docs/reviews/001-storage-surface-general-review.md (S-6 — pre-split coordinates; the F-3 stamp — the publish path's post-fix shape)
• docs/architecture/decisions/015-local-engine-durability-posture.md (the no-replace publish the verification rides)
• docs/plans/implementation.md (§Wave 10; §Review gates — the security gate)
• src/local.rs (size, get, has, fsync_publish, walk_committed — the doctrine's loud-rejection contrast)
• src/local.rs tests (the walk_committed doctrine tests — the planted-symlink round's home)

Notes

Filled by implementation agent (2026-10-15):

• The rejection's error shape (the task's design point): the existing family's honest member — Error::Io (String) with a doctrine-naming message, no new variant (the family is ADR-pinned; the F-4 refusal's precedent). One builder, malformed_committed_entry(path, found) over three helpers in src/local.rs: the message form is "malformed committed entry: {path:?} holds a {kind}, not a regular file — rejected loudly, never served or dedup-mapped (the committed-tree doctrine)" — "malformed" vocabulary matching walk_committed's messages, so the pool's fatal-malformed shape reads with one voice across list, stat/get, and publish.
• The doctrine generalizes to non-regular, not just symlinks (recorded judgment): the doctrine statement is "committed paths hold regular files", so the pre-check rejects any non-regular entry (symlink, directory, other) with the same message family — a directory at a committed path was previously served silently by size/get (a dir's stat answers Ok, File::open succeeds on unix) while the walk rejected it loudly at list time; the same one-behavior-two-doctrine shape the review named for symlinks. The new portable test pins the directory arm too.
• The unix O_NOFOLLOW TOCTOU close: landed (judged per the F-3 posture — both platforms' shapes judged together). The portable symlink_metadata pre-check is the base on both platforms; on unix the open additionally carries O_NOFOLLOW, so a symlink slipped in after the pre-check fails the open (final-component loop → errno ELOOP), mapped to the same doctrine message. Tooling note: ErrorKind::FilesystemLoop is unstable on the toolchain (rustc 1.99), so the arm matches raw_os_error() == Some(ELOOP) via libc — the dep is target-gated (cfg(unix)) and already in the lock transitively (tokio), so the lean-compile posture holds. The opened handle's own metadata re-checks the type again (the non-unix shape of the race) — on non-unix the pre-check narrows the window but does not close it; that residual is the recorded posture (std offers no reparse-point open flag), same-message rejection on both platforms.
• has inherits the doctrine automatically — confirmed: it rides size (src/local.rs has → size), so a planted symlink/entry is an Err on has too; pinned by the test round.
• The publish's dedup arm refined, verified by test: the AlreadyExists arm re-stats the destination with symlink_metadata — a regular file is the dedup form (Ok, stage discarded, the CAS first-writer-wins contract unchanged, the existing no-replace race test and its SyncEvent sequence stay green); a symlink or any non-regular entry is rejected loudly (put and staged commit both reject, zero stage residue, nothing written through the link's target — the failed link never touches the destination, ADR-015's own statement); a destination vanished between the failed link and the re-stat (a sweep reaped the first writer in the window) maps to the CAS racing-publisher form Ok — the same shape the plain dedup arm shares when a sweep reaps between a loser's link and its return, recorded rather than re-litigated.
• The staging discard's stage.exists() (line-491 arm): the doctrine does not extend (judged): staging paths are engine-internal in-flight files, never committed paths; a planted symlink at a stage path is removed by remove_file, which unlinks the link itself and never the target — no serving, no write-through, no replacement. The doctrine holds at the committed paths.
• Windows residual (recorded, non-goal): std's DirEntry::file_type follows symlinks on Windows, so walk_committed's blob-level loud rejection of a planted symlink is unix-true (this task's planted-symlink tests are cfg(unix)); on Windows the doctrine is carried by the symlink_metadata pre-check on the stat/get paths. The portable (non-symlink-API) directory-arm test runs on both platforms.
• get's return type needed Debug for the tests' unwrap_err() shape (LocalCursor now derives it — File in a cursor is already Debug; no behavior change), and the dedup-SyncEvent sequence is untouched by the destination re-stat (no new events between Link and StageUnlinked).

Summary

Landed the committed-tree doctrine in src/local.rs (review-001 S-6): one behavior with walk_committed's fatal-malformed rejection, on every path that touches a committed path. The new open_committed helper (behind get/size/has) pre-checks with portable symlink_metadata — never following the link — and rejects any non-regular entry at the path loudly with a doctrine-naming Error::Io; on unix the open rides O_NOFOLLOW (the TOCTOU close, errno-ELOOP mapped to the same message; target-gated libc dep, already transitively in the lock), and the opened handle's metadata re-checks the type; NotFound stays the absent-entry None. has inherits through size (confirmed, pinned). The publish's losing-link arm (fsync_publish's AlreadyExists path) re-stats the destination: a regular file dedup-maps exactly as before (the CAS contract and the no-replace race test's SyncEvent sequence unchanged); a symlink — or any non-regular entry — rejects loudly on both the whole-value put and the staged commit (zero stage residue; the failed hardlink never touches the destination, so nothing is written through the planted link); a destination vanished in the window keeps the CAS racing-publisher Ok shape. The staging discard's stage.exists() arm is judged out of the doctrine's scope (remove_file on a symlink unlinks the link, never the target) — recorded in Notes. The module doc carries the doctrine statement. Tests: the planted-symlink round (get/size/has reject loudly — both the replaced-a-real-entry and never-published shapes; the walk's blob-level rejection pinned unmodified; decoy untouched), the portable directory arm (stat/get/has plus both publish forms reject), and the planted-symlink publish round (put/commit reject, decoy stands, link stands, zero stage residue).

Verification: cargo test green (149 lib + 54 conformance + 4 git-interop), cargo test --all-features green server-less (324 + 125 + 4) and against the live pglo-poc harness, cargo clippy --all-targets -- -D warnings and cargo clippy --all-features --all-targets -- -D warnings green, cargo fmt --check green, cargo doc --no-deps clean.