core: O_NOFOLLOW the download target open

DAEMON's safepath-adversarial.md accepts a TOCTOU residual between its
canonicalise-and-check and the download starting, on the stated grounds
that CORE's O_NOFOLLOW open of the final file closes it. That flag was
never actually set: SparseFile::open used O_WRONLY|O_CREAT|O_CLOEXEC, so
a symlink swapped in as the final path component after DAEMON's check
would be followed and redirect our pwrites outside the allowed roots.

Add O_NOFOLLOW. A symlinked leaf now fails the open with ELOOP, which
errno_to_error already maps to Error::path_rejected. Regular files and
the O_CREAT of a fresh part file are unaffected; resume (existing regular
part file) is unaffected. Test that a symlinked destination is rejected
rather than silently followed, and that the link target is never touched.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HPPSGhiArbvQgwC2DNiURS

core/src/io/sparse_file.cpp

@@ -74,7 +74,12 @@ Result<void> SparseFile::open(std::string_view path, const OpenOptions &opts) {
         return ErrorInfo(Error::internal, "SparseFile already open");
 
     std::string p(path);
-    int flags = O_WRONLY | O_CREAT | O_CLOEXEC;
+    // O_NOFOLLOW: the final component of a download target must never be a symlink, on
+    // create or on resume. DAEMON canonicalises the path and checks it against the allowed
+    // roots before start(), but a symlink swapped in afterwards would redirect our writes
+    // outside those roots (daemon/docs/safepath-adversarial.md leans on this open closing
+    // that TOCTOU window). A symlinked leaf fails here with ELOOP -> Error::path_rejected.
+    int flags = O_WRONLY | O_CREAT | O_CLOEXEC | O_NOFOLLOW;
     if (opts.truncate_existing)
         flags |= O_TRUNC;
 

core/tests/io/sparse_file_test.cpp

@@ -117,6 +117,23 @@ VT_TEST(sparse_open_bad_path_is_path_rejected) {
     VT_CHECK(!f.is_open());
 }
 
+VT_TEST(sparse_symlinked_target_is_rejected) {
+    // A symlink swapped in as the final path component after DAEMON's canonicalise-and-check
+    // must not be followed: the open is O_NOFOLLOW, so it fails with ELOOP -> path_rejected
+    // rather than redirecting our writes through the link.
+    TempPath link;    // the download target the caller hands us
+    TempPath target;  // where the symlink points (would-be victim, outside allowed roots)
+    VT_REQUIRE(::symlink(target.path.c_str(), link.path.c_str()) == 0);
+
+    SparseFile f;
+    auto r = f.open(link.path, {.total_size = 4096});
+    VT_REQUIRE(!r.has_value());
+    VT_CHECK_EQ(r.error().code, Error::path_rejected);
+    VT_CHECK(!f.is_open());
+    // the link target was never created/written through
+    VT_CHECK_EQ(::access(target.path.c_str(), F_OK), -1);
+}
+
 VT_TEST(sparse_ops_on_closed_file_error) {
     SparseFile f;
     VT_CHECK_EQ(f.write_at(0, bytes("x")).error().code, Error::internal);

docs/04-engine-design.md

@@ -61,8 +61,14 @@ Rules:
 
 ## 4. Disk I/O — the buffer setting you asked for
 
-One file, opened once, `O_WRONLY`. Each segment `pwrite()`s at its own absolute offset, so
+One file, opened once, `O_WRONLY | O_NOFOLLOW`. Each segment `pwrite()`s at its own absolute
-**there is no reassembly pass and no second write of the whole file.**
+offset, so **there is no reassembly pass and no second write of the whole file.**
+
+`O_NOFOLLOW` on the part-file open: DAEMON canonicalises the save path and checks it against
+the allowed roots before `start()`, but the final component could be swapped for a symlink
+in the window between that check and our open. A symlinked leaf is rejected here (`ELOOP` →
+`Error::path_rejected`), not followed — it closes the TOCTOU residual that
+`daemon/docs/safepath-adversarial.md` accepts on those grounds.
 
 - `posix_fallocate()` the full size up front → contiguous extents, no ENOSPC surprise at
   99 %, no fragmentation.