Saved-search Undo lists the whole Trash; one non-UTF-8 name breaks every Undo #979

Open
opened 2026-10-03 06:46:20 +00:00 by kayg · 1 comment
Owner

Found in review of job/undo-a11y (2026-10-03)

crates/calternal-search/src/saved.rs:801 (undo_trash_saved) lists and stats the whole Trash on every saved-search Undo to find one entry name. trash_original + Root::read_trash already validate that name.

Problems:

  1. Cost grows with Trash size (one stat per entry) on every Undo.
  2. One non-UTF-8 name anywhere in the User's Trash makes the listing fail, so every saved-search Undo for that User returns 400.

Fix: drop the listing; rely on trash_original + read_trash (size check before a capped read stays). Regression tests: a Trash with a non-UTF-8 entry name and an Undo still works; Undo cost does not depend on Trash entry count.
Not a merge blocker (non-security, rare). Owner of the fix: whoever touches saved.rs next, or the 7b round if cheap.

## Found in review of job/undo-a11y (2026-10-03) `crates/calternal-search/src/saved.rs:801` (`undo_trash_saved`) lists and stats the whole Trash on every saved-search Undo to find one entry name. `trash_original` + `Root::read_trash` already validate that name. Problems: 1. Cost grows with Trash size (one stat per entry) on every Undo. 2. One non-UTF-8 name anywhere in the User's Trash makes the listing fail, so every saved-search Undo for that User returns 400. Fix: drop the listing; rely on `trash_original` + `read_trash` (size check before a capped read stays). Regression tests: a Trash with a non-UTF-8 entry name and an Undo still works; Undo cost does not depend on Trash entry count. Not a merge blocker (non-security, rare). Owner of the fix: whoever touches saved.rs next, or the 7b round if cheap.
Author
Owner

Fixed on job/undo-a11y at 13e28c4a031e78590740e287443618c257852380 (not on dev yet).

undo_trash_saved no longer lists or stats the Trash. It validates the name with trash_original and read_trash, then checks the size with metadata() on the open fd before the capped read. A missing, rejected or non-file entry returns 404. A changed hash or an occupied ID still returns 409.

New tests in crates/calternal-search/src/saved.rs:

  • trash_undo_works_beside_a_non_utf8_trash_entry: the Trash holds an entry whose name is not UTF-8. list_trash fails, but Undo still restores the search.
  • trash_undo_opens_only_its_own_entry: 64 other entries, and the .Trash/files folder is set to mode 0300 so readdir fails. Undo still returns 404 for a missing name and 409 for a changed hash, so it never lists the folder. The real Undo then returns 200 after the mode is set back, because the restore journal needs to read the folder.
  • trash_undo_refuses_an_oversized_entry: an entry over the cap returns 404 because of the fd size check.

Gates:

$ cargo fmt --check
(no output, exit 0)
$ cargo clippy -p calternal-search -p calternal-fs --all-targets -- -D warnings
    Checking calternal-search v0.0.1 (/home/kayg/Developer/calternal-wt/undo-a11y/crates/calternal-search)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 19.72s
$ cargo test -p calternal-search
test result: ok. 47 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 1.06s   (lib)
test result: ok. 1 passed; 0 failed; 0 ignored; ...   (ask_evaluation)
test result: ok. 1 passed; 0 failed; 0 ignored; ...   (citations)
test result: ok. 21 passed; 0 failed; 0 ignored; ...  (indexer)
test result: ok. 5 passed; 0 failed; 0 ignored; ...   (operators)
test result: ok. 2 passed; 0 failed; 0 ignored; ...   (query_properties)
test result: ok. 1 passed; 0 failed; 2 ignored; ...   (relevance)
test result: ok. 4 passed; 0 failed; 0 ignored; ...   (retrieval)
Fixed on `job/undo-a11y` at `13e28c4a031e78590740e287443618c257852380` (not on dev yet). `undo_trash_saved` no longer lists or stats the Trash. It validates the name with `trash_original` and `read_trash`, then checks the size with `metadata()` on the open fd before the capped read. A missing, rejected or non-file entry returns 404. A changed hash or an occupied ID still returns 409. New tests in `crates/calternal-search/src/saved.rs`: - `trash_undo_works_beside_a_non_utf8_trash_entry`: the Trash holds an entry whose name is not UTF-8. `list_trash` fails, but Undo still restores the search. - `trash_undo_opens_only_its_own_entry`: 64 other entries, and the `.Trash/files` folder is set to mode 0300 so `readdir` fails. Undo still returns 404 for a missing name and 409 for a changed hash, so it never lists the folder. The real Undo then returns 200 after the mode is set back, because the restore journal needs to read the folder. - `trash_undo_refuses_an_oversized_entry`: an entry over the cap returns 404 because of the fd size check. Gates: ``` $ cargo fmt --check (no output, exit 0) $ cargo clippy -p calternal-search -p calternal-fs --all-targets -- -D warnings Checking calternal-search v0.0.1 (/home/kayg/Developer/calternal-wt/undo-a11y/crates/calternal-search) Finished `dev` profile [unoptimized + debuginfo] target(s) in 19.72s $ cargo test -p calternal-search test result: ok. 47 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 1.06s (lib) test result: ok. 1 passed; 0 failed; 0 ignored; ... (ask_evaluation) test result: ok. 1 passed; 0 failed; 0 ignored; ... (citations) test result: ok. 21 passed; 0 failed; 0 ignored; ... (indexer) test result: ok. 5 passed; 0 failed; 0 ignored; ... (operators) test result: ok. 2 passed; 0 failed; 0 ignored; ... (query_properties) test result: ok. 1 passed; 0 failed; 2 ignored; ... (relevance) test result: ok. 4 passed; 0 failed; 0 ignored; ... (retrieval) ```
Commenting is not possible because the repository is archived.
No labels
No milestone
No project
No assignees
1 participant
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-v0#979
No description provided.