Filesystem: remove pre-journal temporary files on cancellation and I/O failure #802

Open
opened 2026-10-02 13:12:26 +00:00 by kayg · 5 comments
Owner

Source finding from the read-only sec-fs audit requested on #663. Reliability follow-up; no Instance outage or exploit is claimed.

Context and evidence

Source c4a61e8cf0 (origin/dev). These write and recovery functions are unchanged in round 7a.

  • crates/calternal-fs/src/root.rs:1075–1110: temp returns a File plus a name. Even the O_TMPFILE path is linked to that name before return. The tuple has no Drop cleanup guard.
  • crates/calternal-fs/src/write.rs:202–210, 402–412: write_inner awaits input after allocating the named temporary. It unlinks on an error only after the inner future returns. Dropping the outer future before that return skips the cleanup.
  • crates/calternal-fs/src/file_ops.rs:138–204: copy has early error returns after allocating a named temporary, without a general cleanup guard.
  • crates/calternal-fs/src/uploads.rs:57–98: a chunk write/fsync error can likewise leave a named temporary; upload expiry will eventually remove that upload directory.
  • crates/calternal-fs/src/journal.rs:159–193: restart recovery removes abandoned temporary files only inside the journal directory. It does not remove a pre-journal temporary in a Home.
  • Reserved temporary names stay outside User listings, but quota.rs:34–61 counts their bytes in Home usage. The User cannot remove them through the ordinary Files path.

Reasoned impact and classification

Cancellation or an I/O failure before publication can leave hidden bytes and charge them to the User's quota indefinitely. The target content remains intact. This report does not claim a user-controlled way to cancel the server request, a quota bypass or cross-User exhaustion. Treat it as a reliability follow-up until reachability and scale justify a stronger classification.

Concrete fix

Use one held-parent temporary guard with Drop cleanup until a durable journal takes ownership. Cover async cancellation and every pre-journal return. Transfer cleanup responsibility explicitly at journal publication so recovery inputs cannot be removed accidentally. Add bounded startup cleanup for pre-journal leftovers with ownership rules that distinguish active writes from abandoned files.

Defensive tests

Use a small controlled AsyncRead and abort a local unit-test future before publication. Assert that target bytes and logical quota are unchanged and that the private name is absent. Use existing write/copy/chunk I/O fault hooks, then reopen Root and verify cleanup. Keep fixtures small. Do not change existing test expectations.

Duplicate search: all-state quoted temporary-file, cancellation and orphan searches. #96 concerns upload lease expiry; #259 concerns Index lag; #2 covers the original filesystem layer. No specific cancellation-safe temporary guard issue was found.

Source finding from the read-only sec-fs audit requested on #663. Reliability follow-up; no Instance outage or exploit is claimed. ## Context and evidence Source c4a61e8cf090170f35b1bed3350d9de20c83ecd5 (`origin/dev`). These write and recovery functions are unchanged in round 7a. - `crates/calternal-fs/src/root.rs:1075–1110`: `temp` returns a File plus a name. Even the O_TMPFILE path is linked to that name before return. The tuple has no Drop cleanup guard. - `crates/calternal-fs/src/write.rs:202–210, 402–412`: `write_inner` awaits input after allocating the named temporary. It unlinks on an error only after the inner future returns. Dropping the outer future before that return skips the cleanup. - `crates/calternal-fs/src/file_ops.rs:138–204`: copy has early error returns after allocating a named temporary, without a general cleanup guard. - `crates/calternal-fs/src/uploads.rs:57–98`: a chunk write/fsync error can likewise leave a named temporary; upload expiry will eventually remove that upload directory. - `crates/calternal-fs/src/journal.rs:159–193`: restart recovery removes abandoned temporary files only inside the journal directory. It does not remove a pre-journal temporary in a Home. - Reserved temporary names stay outside User listings, but `quota.rs:34–61` counts their bytes in Home usage. The User cannot remove them through the ordinary Files path. ## Reasoned impact and classification Cancellation or an I/O failure before publication can leave hidden bytes and charge them to the User's quota indefinitely. The target content remains intact. This report does not claim a user-controlled way to cancel the server request, a quota bypass or cross-User exhaustion. Treat it as a reliability follow-up until reachability and scale justify a stronger classification. ## Concrete fix Use one held-parent temporary guard with Drop cleanup until a durable journal takes ownership. Cover async cancellation and every pre-journal return. Transfer cleanup responsibility explicitly at journal publication so recovery inputs cannot be removed accidentally. Add bounded startup cleanup for pre-journal leftovers with ownership rules that distinguish active writes from abandoned files. ## Defensive tests Use a small controlled AsyncRead and abort a local unit-test future before publication. Assert that target bytes and logical quota are unchanged and that the private name is absent. Use existing write/copy/chunk I/O fault hooks, then reopen Root and verify cleanup. Keep fixtures small. Do not change existing test expectations. Duplicate search: all-state quoted temporary-file, cancellation and orphan searches. #96 concerns upload lease expiry; #259 concerns Index lag; #2 covers the original filesystem layer. No specific cancellation-safe temporary guard issue was found.
Author
Owner

#802 follow-up verified against already built libraries using an inert driver with 16,390 empty temporary files. The old library left 11 names; the streamed recovery library left zero. The driver also confirms the corrected fixture creates its .system parent first. The old/new driver exits were 1 and 0. This supplements the first filesystem run (68 tests passed; one new fixture setup error, corrected) while the final full crate test waits for the server build.

The follow-up also marks held cleanup descriptors close-on-exec. No Home directory descriptor should reach a media child. These changes are a separate cleanup commit; final crate gates remain pending.

#802 follow-up verified against already built libraries using an inert driver with 16,390 empty temporary files. The old library left 11 names; the streamed recovery library left zero. The driver also confirms the corrected fixture creates its .system parent first. The old/new driver exits were 1 and 0. This supplements the first filesystem run (68 tests passed; one new fixture setup error, corrected) while the final full crate test waits for the server build. The follow-up also marks held cleanup descriptors close-on-exec. No Home directory descriptor should reach a media child. These changes are a separate cleanup commit; final crate gates remain pending.
Author
Owner

Temporary ownership and full streamed recovery are committed. Final FS clippy passed; all 69 unit and 42 integration tests passed. The old/new recovery driver left 11/0 names. Head: bd6e97e09b. Full handoff and verbatim output are on #779. No push or deploy.

Temporary ownership and full streamed recovery are committed. Final FS clippy passed; all 69 unit and 42 integration tests passed. The old/new recovery driver left 11/0 names. Head: bd6e97e09b56c8df6ae770f5bb440aaf2d9d8f36. Full handoff and verbatim output are on #779. No push or deploy.
Author
Owner

Evidence: crates/calternal-fs/src/journal.rs:276 builds the full child path.
Line 279 opens that full path. Line 282 returns every error except NotFound.
The new traversal has no depth or path bound. Each level also retains a
Dir descriptor (:287). crates/calternal-server/src/wire.rs:1100 propagates
recovery failure before server startup.

A directory tree can exist with a descendant path longer than the kernel's
single-call path limit. Moving a directory changes its descendants' full
paths without opening them. crates/calternal-fs/src/file_ops.rs:29 validates
the source and destination, but does not validate every descendant path.
The former cleanup skipped excessive depth; the new full scan attempts each
full path. An overlong descendant path therefore makes recovery fail and
prevents the Instance from starting. A low inherited descriptor limit also
makes the retained directory stack fail. No startup failure was reproduced.

Rule: owner availability rule; DESIGN §2; #802 cleanup must use bounded work
and safe directory handles.

Fix: open each child relative to the held parent with the existing resolve
flags. Set an explicit descriptor budget. Resume traversal without retaining
one open descriptor per unbounded level. Do not fail all startup because an
optional abandoned-file sweep encounters a tree it cannot scan.

Test idea: use a small inert directory tree with a long total path, created
through directory handles, and a separate test process with a small descriptor
limit. Recovery must return successfully, preserve ordinary files, and clean
abandoned names in reachable directories. Do not alter existing expectations.

Duplicate search: recovery/depth and ENAMETOOLONG. #802 owns this shared
cleanup fix; add this evidence there.

F2 — P2: thumbnail temporary files bypass active cleanup ownership

Evidence: crates/calternal-fs/src/thumbnails.rs:133 creates a
.calternal-tmp- file directly. ThumbnailTemp (:90) stores no
TemporaryGuard. The new recovery sweep at journal.rs:265 deletes such
regular files unless the temporary registry contains their parent and name.
The creation does not register them. publish_thumbnail_for (:175) can
also return on a sync or rename error without removing the temporary.

The documented concurrent-recovery invariant therefore excludes an active
thumbnail. Recovery can remove its name before publication. Publication can
then fail despite intact rendered bytes. An I/O failure can also leave the
name until the next recovery. This affects Derived data, not content files.
No concurrent failure was reproduced.

Rule: #802; DESIGN §39; reuse gate.

Fix: create thumbnail temporaries through Root::temp and retain its guard
inside ThumbnailTemp until publication or discard. Keep the existing WebP
validation and cache identity. Use the same registry lock as other writers.

Test idea: create a thumbnail temporary, run recovery through a second Root

Evidence: `crates/calternal-fs/src/journal.rs:276` builds the full child path. Line 279 opens that full path. Line 282 returns every error except NotFound. The new traversal has no depth or path bound. Each level also retains a `Dir` descriptor (`:287`). `crates/calternal-server/src/wire.rs:1100` propagates recovery failure before server startup. A directory tree can exist with a descendant path longer than the kernel's single-call path limit. Moving a directory changes its descendants' full paths without opening them. `crates/calternal-fs/src/file_ops.rs:29` validates the source and destination, but does not validate every descendant path. The former cleanup skipped excessive depth; the new full scan attempts each full path. An overlong descendant path therefore makes recovery fail and prevents the Instance from starting. A low inherited descriptor limit also makes the retained directory stack fail. No startup failure was reproduced. Rule: owner availability rule; DESIGN §2; #802 cleanup must use bounded work and safe directory handles. Fix: open each child relative to the held parent with the existing resolve flags. Set an explicit descriptor budget. Resume traversal without retaining one open descriptor per unbounded level. Do not fail all startup because an optional abandoned-file sweep encounters a tree it cannot scan. Test idea: use a small inert directory tree with a long total path, created through directory handles, and a separate test process with a small descriptor limit. Recovery must return successfully, preserve ordinary files, and clean abandoned names in reachable directories. Do not alter existing expectations. Duplicate search: recovery/depth and ENAMETOOLONG. #802 owns this shared cleanup fix; add this evidence there. ## F2 — P2: thumbnail temporary files bypass active cleanup ownership Evidence: `crates/calternal-fs/src/thumbnails.rs:133` creates a `.calternal-tmp-` file directly. `ThumbnailTemp` (`:90`) stores no `TemporaryGuard`. The new recovery sweep at `journal.rs:265` deletes such regular files unless the temporary registry contains their parent and name. The creation does not register them. `publish_thumbnail_for` (`:175`) can also return on a sync or rename error without removing the temporary. The documented concurrent-recovery invariant therefore excludes an active thumbnail. Recovery can remove its name before publication. Publication can then fail despite intact rendered bytes. An I/O failure can also leave the name until the next recovery. This affects Derived data, not content files. No concurrent failure was reproduced. Rule: #802; DESIGN §39; reuse gate. Fix: create thumbnail temporaries through `Root::temp` and retain its guard inside `ThumbnailTemp` until publication or discard. Keep the existing WebP validation and cache identity. Use the same registry lock as other writers. Test idea: create a thumbnail temporary, run recovery through a second Root
Author
Owner

F1 repair committed on job/mediafix: b5d95ced3f57e5024b894eee322ed542a4fed0bf.

Evidence: the old implementation failed the new child-process regression with NameTooLong while scanning a directory tree whose relative path exceeded PATH_MAX. The fixed walk opens each child relative to its held parent with the existing resolve flags. It holds at most 24 directory cursors, skips unreadable or deeper optional branches, and still scans all reachable entries without an entry-count cutoff. The test runs recovery with RLIMIT_NOFILE=32, confirms a reachable abandoned name is removed, and confirms an ordinary file remains unchanged.

Focused test output:

running 1 test
test journal::recovery_tests::recovery_scans_long_paths_with_bounded_descriptors ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 69 filtered out; finished in 0.01s

Decision not specified by DESIGN: use a 24-directory descriptor cap; skip deeper/unreadable branches because this cleanup is optional and must not prevent Instance startup. Final calternal-fs gates remain queued after the other review fix.

F1 repair committed on `job/mediafix`: `b5d95ced3f57e5024b894eee322ed542a4fed0bf`. Evidence: the old implementation failed the new child-process regression with `NameTooLong` while scanning a directory tree whose relative path exceeded PATH_MAX. The fixed walk opens each child relative to its held parent with the existing resolve flags. It holds at most 24 directory cursors, skips unreadable or deeper optional branches, and still scans all reachable entries without an entry-count cutoff. The test runs recovery with RLIMIT_NOFILE=32, confirms a reachable abandoned name is removed, and confirms an ordinary file remains unchanged. Focused test output: ``` running 1 test test journal::recovery_tests::recovery_scans_long_paths_with_bounded_descriptors ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 69 filtered out; finished in 0.01s ``` Decision not specified by DESIGN: use a 24-directory descriptor cap; skip deeper/unreadable branches because this cleanup is optional and must not prevent Instance startup. Final calternal-fs gates remain queued after the other review fix.
Author
Owner

F2 repair committed on job/mediafix: dcd502dfb3844adf40edcb247957848d592a6d9e.

Both regressions failed on the old code. Recovery removed the active thumbnail's private name, and a rejected publication left its private name behind. thumbnail_temp() now creates the file through the shared temporary registry and retains its guard through publication or discard. The fixed .webp suffix is preserved. Drop removes the name on early publication errors; after successful rename, the guard releases the old temporary name.

Focused output:

running 7 tests
test thumbnails::tests::failed_thumbnail_publication_removes_its_temporary ... ok
test thumbnails::tests::recovery_preserves_active_thumbnail_temporary_until_publication ... ok
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 65 filtered out; finished in 1.64s

The full calternal-fs gates will run after F3 so the crate is verified once on the final combined code.

F2 repair committed on `job/mediafix`: `dcd502dfb3844adf40edcb247957848d592a6d9e`. Both regressions failed on the old code. Recovery removed the active thumbnail's private name, and a rejected publication left its private name behind. `thumbnail_temp()` now creates the file through the shared temporary registry and retains its guard through publication or discard. The fixed `.webp` suffix is preserved. Drop removes the name on early publication errors; after successful rename, the guard releases the old temporary name. Focused output: ``` running 7 tests test thumbnails::tests::failed_thumbnail_publication_removes_its_temporary ... ok test thumbnails::tests::recovery_preserves_active_thumbnail_temporary_until_publication ... ok test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 65 filtered out; finished in 1.64s ``` The full calternal-fs gates will run after F3 so the crate is verified once on the final combined code.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal#802
No description provided.