Repository navigation
feat(core): remove a file on Unix by moving it aside and identifying the open object (#445) - #1069
Merged
Merged
Conversation
…rt only (#445) The removal primitives of the pinned module exist only with the test-only feature and have no caller. On Unix an object is moved into a quarantine directory with RENAME_NOREPLACE, the open object is identified there and only then unlinked, with the link count checked afterwards; a swap of the last name or a manipulation inside the quarantine directory is moved back and never unlinked. No exclusivity of that directory is assumed: the one window no Unix primitive closes is forced in a test and reported as Disturbed. On Windows the object is opened through the handle-relative lookup with delete access and read-only sharing, so no writer can change the authenticated bytes, and the POSIX delete disposition is set on that same handle. There is no FILE_FLAG_DELETE_ON_CLOSE and no fallback. Leftovers of a crash are finished only after authenticating them again. A scan of the sources and a compile of a dependent with and without the feature keep the primitives out of every other build.
…on (#445) The first draft of the removal was one pull request of 2,172 meaningful lines, over the 1,200-line hard-advisory tier. This commit keeps what has to land first: the API behind test-support, the proofs that it exists in no other build (a scan of the sources and a compile of a dependent with and without the feature), PinnedDir::sync, and the contract with the limit no Unix primitive can remove. Every removal answers Unsupported here; the Unix file removal, the Unix directory removal with the leftovers, and the Windows delete follow as stacked pull requests, each with its own tests.
…the open object (#445) The authenticated file is moved into a quarantine directory with RENAME_NOREPLACE, the open object is identified there and only then unlinked. Anything else is put back, never unlinked. Every failure after the move is handled at one point: the entry goes back, or the outcome is Stranded and says where it is. The parent is synced whenever it changed. The quarantine directory is not assumed to be exclusive. The one window no Unix primitive closes is forced in a test and reported as Disturbed: detected after the fact by the link count of the authenticated object, not prevented.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedEnable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Next included review available in 53 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Reviewer's GuideThis PR implements test-support-only authenticated file removal on Unix by moving the bound entry into a randomized private quarantine, verifying the open object by identity, and unlinking only after verification, with rollback, stranded and race-detection outcomes, parent synchronization, and extensive adversarial tests; directories, leftovers, Windows, and production callers remain unsupported. Sequence diagram for authenticated Unix file removalsequenceDiagram
participant Caller
participant PinnedDir
participant Quarantine
participant Observer
participant ParentFS
Caller->>PinnedDir: remove_file(name, authenticate, observer)
PinnedDir->>PinnedDir: open_file(name)
PinnedDir->>Caller: authenticate(PinnedFile)
Caller-->>PinnedDir: authenticated
PinnedDir->>Observer: Checked
PinnedDir->>Quarantine: create()
Quarantine->>ParentFS: mkdirat + fchmod 0700
PinnedDir->>Quarantine: adopt(parent, name)
Quarantine->>ParentFS: renameat_with(RENAME_NOREPLACE)
PinnedDir->>Observer: MovedAside
PinnedDir->>Quarantine: inspect(file)
Quarantine->>ParentFS: stat_child + openat
alt authenticated object identified
PinnedDir->>Observer: Identified
PinnedDir->>Quarantine: unlink_verified(object, file)
Quarantine->>ParentFS: unlinkat
alt link count decreased
PinnedDir->>Observer: BeforeSync
PinnedDir->>ParentFS: sync()
PinnedDir-->>Caller: Removed
else object disturbed
PinnedDir-->>Caller: Disturbed
end
else foreign or swapped object
PinnedDir->>Quarantine: restore()
Quarantine->>ParentFS: renameat_with(RENAME_NOREPLACE)
PinnedDir->>Observer: BeforeSync
PinnedDir->>ParentFS: sync()
PinnedDir-->>Caller: Swapped or Stranded
end
Flow diagram for Unix authenticated file removal outcomesflowchart TD
A["Authenticate open file"] --> B["Create private quarantine"]
B --> C["Move name with RENAME_NOREPLACE"]
C --> D["Inspect and open quarantined entry"]
D --> E{"Identity matches authenticated object?"}
E -- No --> F["Restore entry"]
F --> G["Sync parent"]
G --> H["Swapped"]
E -- Yes --> I["Unlink quarantined entry"]
I --> J{"Authenticated link count decreased?"}
J -- Yes --> K["Sync parent"]
K --> L["Removed"]
J -- No --> M["Disturbed"]
D --> N["Failure after move"]
N --> O{"Restore succeeds?"}
O -- Yes --> P["Return failure after rollback"]
O -- No --> Q["Stranded in quarantine"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
codeant-ai
Bot
added
the
size:XXL
CodeAnt Nitpicks1 code suggestion1. After a post-move failure, successful restoration returns an error before
|
CodeAnt PR Risk: Medium Risk
Assessed commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be6136b610
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… its own (#445) CodeScene counted the table as part of a 73-line test; the table is data, the test is the loop over it.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…unix-file-removal # Conflicts: # CHANGELOG.md # crates/worldscript-secure-storage/src/pinned/tests/mod.rs # crates/worldscript-secure-storage/src/pinned/tests/remove.rs # crates/worldscript-secure-storage/src/pinned/unix/removal.rs # docs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
[check-pr-size] PR size is over the target tier (normal profile): 7 files, 1109 meaningful lines, 9 commits — limit ≤8 files / ≤400 lines / ≤6 commits. Consider splitting into smaller, independently reviewable PRs. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: qnbs/WorldScript-Studio/.coderabbit.yaml
- Review profile: CHILL
- Plan: Essentials
- Run ID:
b6bb2535-55fb-4269-bb5c-fe664e0656c1
📒 Files selected for processing: 7
CHANGELOG.mdcrates/worldscript-secure-storage/Cargo.tomlcrates/worldscript-secure-storage/src/pinned/tests/mod.rscrates/worldscript-secure-storage/src/pinned/tests/remove.rscrates/worldscript-secure-storage/src/pinned/tests/remove_unix.rscrates/worldscript-secure-storage/src/pinned/unix/removal.rsdocs/native/r15/GATE4D-JOURNAL-DURABLE-EVIDENCE.md
Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…tory that was made (#445) Review findings on the Unix file removal: - A failure after the move that put the entry back, or left it in the quarantine directory, returned without syncing the parent. The sync is now attempted after both (best effort, so it does not replace the failure being reported) and the BeforeSync stage is reached, which a test observes. - The quarantine directory was removed by its name alone, so a directory that an actor put at that name after renaming the held one away could be removed. The cleanup now checks that the name still leads to the directory that was created; a test forces the swap. - The permission-based Stranded test failed for root. It now skips with a message for root, and the same logic is covered without permissions by a case that blocks the put-back by taking the name again.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb9eb87011
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…creation race (#445) More review findings on the Unix file removal: - The path where the name vanished before the move created and removed a quarantine directory and reported no change; the parent is now synced. - An entry that stays in the quarantine directory (put-back blocked, or stranded) is retained with a best-effort sync of that directory as well as of the parent. - That the directory behind the quarantine name is the one mkdirat made cannot be proved, since mkdirat returns no descriptor. The documentation says so and why it does not widen the window the contract already states.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54dcf086fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ove, refuse an unopenable one (#445) Further review findings on the Unix file removal: - An entry that stays in the quarantine directory is only reported as being there if the recorded name still leads to that directory. If an actor renamed it away the outcome is Disturbed, which now also covers an entry that cannot be found by name. A test forces the rename. - A failure to move the entry at all (renameat2 unsupported or failing) created and removed a quarantine directory without syncing the parent; it now goes through the same best-effort sync as a rollback. - A umask that takes the owner's read access away leaves a quarantine directory that cannot be opened. The removal is refused (fail closed) and a test pins that, instead of the documentation promising more.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…own part (#445) The Unix file removal had grown to 1,306 meaningful lines through review waves, over the 1,200-line tier. Its hardening against a same-user actor in the quarantine directory and the durability of the failure paths (sync after a rollback or a stranded entry, an identity-checked cleanup of the quarantine directory, a quarantine that was renamed away, an unopenable one, the creation race stated) is a separate proof boundary and is re-applied as the next part on top of this one. What stays here is the removal itself with its single rollback point, the Stranded outcome, the sync of changes that are made, the forced swap tests and the link-count window. The root-skip of the permission case stays.
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
Owner
Author
|
Scope note: the hardening commits |
qnbs
added a commit
that referenced
this pull request
Oct 16, 2026
…crash (#445) (#1074) * feat(core): remove a directory on Unix and finish the leftovers of a crash (#445) Fresh re-cut of #1070 on the hardened main (1f37cd8), not a retarget: that PR was built on the history of the squash-merged #1069 and its code predates the quarantine sync, the identity checks and the fail-closed look that main has now. Its one own change is re-applied on that code, with its tests. test-support only, no caller, no new unsafe, no new package. Windows still answers Unsupported. - remove_empty_dir opens the child directory (no link followed), refuses one that has entries and moves it through the same quarantine directory; it is identified by the open directory and unlinked with AT_REMOVEDIR. A directory that fills after the check goes back (NotEmpty), through the step that syncs the quarantine directory like any put-back. - remove_leftover_file takes a .h1q-* directory by its generated name only (the prefix and exactly 32 lowercase hexadecimal digits, otherwise InvalidName before anything is opened), requires a directory of the effective user without access for group or others, authenticates the file in it again and unlinks it only if accepted. A file that does not authenticate and anything that is not a regular file (a link, a directory) stay and give Refused; an empty generated directory is removed. - On the hardened code the leftover removal syncs the quarantine directory it unlinks in and says Removed(NotConfirmed) if that fails; an entry that vanished before the unlink is reported like one that vanished before. Tests (own file): a directory swapped or filled after the check, an empty directory that wins the last window (Disturbed), a simulated crash and its recovery, the name grammar (short, long, upper case, not hexadecimal), a foreign object and a link left in the quarantine directory, a stranded entry (put-back blocked, proved to be there) finished by the recovery, and the failed sync of the held directory. The earlier permission-based stranded case is now Disturbed on main (a failed look proves nothing) and is not asserted here. Mutation-checked: no sync in the leftover removal, no refusal of a foreign entry, a loosened name grammar and an ignored authentication each fail a case. The PR number in the changelog entry is to be set to the number of this PR when it is opened. Issue #359. * docs(changelog): set the PR number of the Unix directory and leftover entry (#445) * fix(core): report a swapped-in non-empty directory as disturbed, state the limits of the leftover recovery (#445) Carries the findings that were open on the superseded #1070 into this PR, checked against the code of main (most were already closed by the hardening it has: the rollback sync, the sync of the held directory, the identity-checked cleanup, the root-safe test). A directory that cannot be removed because it has entries goes back where it was and was reported as NotEmpty. If an actor won the last window by moving the authenticated empty directory away and putting a non-empty one in its place, the refusal was about that directory and said nothing about ours (Codex P2 on #1070). The unlink refusal now checks that what is inside is still the authenticated directory (held open, so its identity is unique); if not, it goes back under the name all the same and the outcome is Disturbed. A put-back that fails stays a failure that ends in Stranded or Disturbed as before. Stated, not changed: an empty foreign directory that wins the same window is still removed instead of the authenticated one (no Unix primitive unlinks "this inode"; it holds no data, and the removal reports Disturbed), and the leftover recovery reclaims files, so an empty directory left by a crashed directory removal stays in its .h1q-* directory (Sourcery on #1070). Both are in the changelog, the evidence document and the API documentation. Test: a non-empty directory swapped in at the last window gives Disturbed with the original directory intact and the swapped-in one back under the name. Mutation-checked: reporting NotEmpty for any directory fails it. test-support only, no caller, no new unsafe, no new package. Issue #359. * fix(core): tell a removed directory from a moved one on macOS by its path, not its link count (#445) The hosted macOS evidence of this PR failed the removal of an empty directory: the outcome was Disturbed instead of Removed(Confirmed). The check after the unlink asked the open directory whether its link count had dropped, which holds on Linux, where a removed directory that is still open reports zero. macOS keeps reporting the old count (or cannot be asked), so the check could never confirm a removal there. The superseded #1070 never ran on a hosted OS (a stacked PR gets no hosted evidence), so this was the first time that path met macOS; the other directory cases passed there only because they expect Disturbed. For a directory on Apple targets the signal is now that the open descriptor no longer has a path: fcntl(F_GETPATH), rustix::fs::getpath, fails with "no such file" for a removed directory and still answers for one that an actor moved away and that is therefore not the one that was removed. Anything else, such as a path that is still there or another error, stays Disturbed (a removal that cannot be checked afterwards says nothing). Linux and files are unchanged. No new package and no new feature: getpath needs rustix's alloc, which std already enables. Whether macOS answers as described is not known from here; the hosted macOS run of this commit is the evidence, and the changelog and the evidence document state the signal as the one that run showed. test-support only, no caller, no new unsafe. Issue #359. * test(core): TEMPORARY probe of what macOS reports for a removed, a moved and a live open directory (#445) Always fails on purpose so the hosted macOS log prints the answer; removed again in the next commit. Not part of the change. * test(core): TEMPORARY macOS probe, do not create the probe file before the directory is removed (#445) Still temporary and removed again in the next commit. * fix(core): remove a directory on Linux only, answer Unsupported where the system cannot tell afterwards (#445) The hosted macOS evidence of this PR failed the removal of an empty directory (Disturbed instead of Removed). A first attempt to read the removal from fcntl(F_GETPATH) was also refused by macOS, so the answer was measured instead of guessed: a temporary probe on the hosted runner (commits b59fe38 and 8e294a2, removed here) compared an open directory that was removed, one that was only moved and one that was untouched. link count 2 / 2 / 2 (unchanged for the removed one) F_GETPATH the old path for the removed one (stale) openat(".."), statat(".") succeed for all three create an entry in it fails with ENOENT only for the removed one The removal of a directory counts only if the open directory can be asked afterwards whether it was the one that lost its name. Linux answers (a removed directory that is still open reports a link count of zero). On macOS the only signal is a write into a directory that, in the attack case, an actor moved away and that belongs to another path, and it fails for lack of permission on a read-only directory. That is not a check to build a success on. remove_empty_dir therefore answers Unsupported on any Unix system but Linux, after the checks that need no such signal (a link, a missing name, a directory that has entries) and before anything is moved. The directory cases move to a Linux-only test file; on the other systems the same call is pinned to Unsupported with nothing moved, and the generic directory test expects it. The leftover recovery (files) is unchanged and runs on both. Whether and how macOS gets directory removal is left to the decision about the production use. The changelog, the evidence document and the API documentation state the limit and the measured behaviour. test-support only, no caller, no new unsafe, no new package. Issue #359. * fix(core): sync the parent when a leftover's quarantine directory goes, look at a restored directory again, recover an empty directory leftover (#445) Three findings on 2989ba8, all valid. A leftover removal removes the emptied quarantine directory with discard, which changes the parent. When the removal itself changed nothing (a refusal) or failed, that change was never synced: Unchanged skipped the sync, and an error returned before the settle. A crash could then bring the empty directory back although the caller had been told the result (Sourcery, Codex P2 and a CodeAnt nitpick, independently). discard now reports whether it removed the directory; the parent is synced in that case for a refusal and for an error, and the result of the removal is kept. A directory that cannot be removed because it has entries goes back under its name, and was looked at before the rename. An actor that swaps another directory in between the look and the rename had it restored and reported as NotEmpty (Codex P2). It is now looked at under its name after it went back, against the identity of the open authenticated directory, and anything else is Disturbed. The earlier look is gone: it was the same check at a worse place. A crashed directory removal leaves an empty directory as x in its quarantine directory, and the leftover recovery only took files (Sourcery, on #1070 and again here). An empty directory holds nothing, so it needs no authentication and goes with the same unlink and the same check as the removal of a directory, on Linux; elsewhere it is refused, a directory with entries stays (NotEmpty), and a link or a FIFO stays (Refused). The temporary macOS probe thread (Codex P1) is obsolete: the probe was removed in the previous commit and the hosted macOS run is green. Tests: the parent sync for a refusal and an error with the entry taken, and not when the entry stays; the recovery of an empty directory after a crash during a directory removal, with the callback never asked; a directory with entries stays; Refused for an empty directory where the removal cannot be checked. Each of the three changes is mutation-checked. test-support only, no caller, no new unsafe, no new package. Issue #359. * docs(core): say that the recovery refuses a link or a FIFO and that only an explicit call takes a leftover (#445) A leftover quarantine directory is not removed by a sweep; the explicit recovery takes a file or an empty directory and refuses anything else, which stays. Wording only (CodeRabbit on ec7ba93), plus one spelling nit in the changelog. No code change.
This was referenced Oct 16, 2026
qnbs
added a commit
that referenced
this pull request
Oct 16, 2026
…#1075) * feat(core): remove on Windows through the authenticated handle (#445) Fresh re-cut of #1071 on the main that carries the Unix hardening and the directory removal, not a retarget: that PR was stacked on the history of the squash-merged #1069 and #1070. Its one own change (bdf2011) is re-applied here with its tests, and the two findings that were open on it are dealt with. test-support only, no caller. One new unsafe call (the disposition, one fixed structure, its layout pinned by a test), no new package. - The object is opened through the handle-relative lookup of every child, with delete access and a share mode of read only, authenticated on that handle and then given the POSIX delete disposition on the same handle: no FILE_FLAG_DELETE_ON_CLOSE, no fallback to the classic disposition, a read-only file is refused. A directory is removed the same way and a directory with entries refuses (NotEmpty). The removal is reported as Removed(NotConfirmed): Windows offers no directory sync. A leftover removal is Unsupported, because Windows creates no quarantine directory. - The cases of remove.rs now run on Windows too. The directory case there expects the removal on Linux and on Windows and Unsupported on any other Unix system, which the Unix re-cut introduced and which would otherwise have failed the hosted Windows run. Findings carried from #1071: - Codex P2: ERROR_INVALID_FUNCTION, which an older Windows or a driver gives when it does not implement the information class, was not mapped and became a plain I/O error. Every code that means "no POSIX delete" now maps to Unsupported through one function, known_refusal, that a test pins (including that an unrelated code stays an error). - Sourcery: a writable memory-mapped view that another process created earlier and keeps after closing its handle is not stopped by the read-only sharing rule, so it can still change the authenticated bytes before the disposition. No sharing mode closes this; it is stated in the module documentation, the evidence document and the changelog as a same-user residual, not claimed closed, and not tested because a hosted run cannot be made to hold such a mapping reliably. Checked here only by compiling for x86_64-pc-windows-msvc (fmt, clippy -D warnings); the hosted Windows run is the evidence. The PR number in the changelog entry is set when this PR is opened. Issue #359. * docs(changelog): set the PR number of the Windows removal entry (#445) * docs(core): say that the removal binds the object, not its bytes, on Unix and on Windows (#445) CodeAnt asked whether the mapped-writer limit on Windows is acceptable for this slice. Looking at it showed that the Unix removal has the same limit in a wider form and never said so: there is no sharing mode, so a same-user process with a writable descriptor or a writable shared mapping can change the bytes between the authentication and the unlink. Both are now stated in the module documentation, the evidence document and the changelog as parts of the same-user residual, not closed. Documentation only. * fix(core): look at a directory again right before its disposition on Windows (#445) Codex P2 on 7c43e60: the share mode of a directory handle cannot stop a writer from converting the empty directory into a reparse point in place (the module already says so for the pinned directory), and the observer, a caller's code, runs between the check and the disposition. The disposition would then have removed the converted entry and reported it as Removed although the API refuses links. The held handle is now checked a second time right before the disposition: a junction made in that window is left, its target is untouched and the removal answers Redirected. This narrows the window and does not close it; a file cannot be converted because its handle shares no write access. Test: the directory is converted into a junction at the Checked stage with the existing convert_in_place helper. It runs on Windows only, so the hosted Windows run is the evidence (checked here by compiling for the Windows target); it is not mutation-checked locally. Documentation, changelog and evidence document say the same. test-support only, no new unsafe, no new package. Issue #359. * docs(core): limit the Windows write protection claim to handles (#445) CodeRabbit on 140e981: the changelog said no writer can change the authenticated bytes while its own entry notes that a writable mapped view that existed before can. The claim is now limited to handles that open the file, and the test header says the same. Wording only.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #359 (R-15 Gate 4, slice H1a-2), part 2 of 5 (base
main; #1068 is merged).test-supportonly: no production caller, no production deletion release.What this adds
The Unix removal of a file:
PinnedDir::remove_authenticated_fileon Unix. The authenticated file is held open;renameat2(RENAME_NOREPLACE)moves whatever the name is bound to into a private quarantine directory (generated name, mode 0700 set withfchmodso a restrictive umask cannot break it); the open object is identified there; only then it is unlinked. Anything else (a live file, a link, a directory, a file this process cannot open) is put back or, if the name was taken, kept in the quarantine directory; never unlinked. Every failure after the move is handled at one point: the entry goes back, or the outcome isStranded { cause }and says where it is. The parent is synced whenever it changed (a failed sync is an error;sync()repeats it). Directories and leftovers still answerUnsupported(part 3); Windows is part 4.rustixgets itsprocessfeature (geteuid); no new package, no newunsafe.The window, stated (needs the maintainer's decision before H1b)
The quarantine directory is not assumed exclusive. After the last identification no Unix primitive unlinks "this inode"; an actor that wins that one race can have its own file removed instead. It is detected after the fact, not prevented (
Disturbed, by the link count of the authenticated object; a heuristic, as Codex pointed out on the first draft) and is never a success. A test forces exactly that window and pins the outcome.Proof
The observer forces each interleaving; the baseline (naive check-then-
unlinkatagainst the same swap) destroys the live file, so the hook is not vacuous. Swaps of the last name, of a directory / symlink / FIFO / unopenable file (identified by inode), a name retaken while the entry is aside, a swap inside the quarantine directory before identification, the last window (foreign file / directory / hard link), vanished name, renamed root, mode and umask, failure injected at each stage, failed sync for a removal and a put-back,Stranded(quarantine loses its search permission; the entry stays the authenticated inode). Mutation-checked (identity comparison: 5 cases fail; link-count verification: the forced window; rollback of a failed inspection: theStrandedcase). Local: fmt, clippy-D warningson Linux /x86_64-pc-windows-msvc/aarch64-apple-darwin, lib and isolation tests; macOS and Windows execution is the hosted runners' job.Review findings of the first draft that this part answers
CodeRabbit / CodeAnt / Codex on the first draft (#1068 history): rollback after a failed inspection, umask 0200, the honesty of the link-count check, sync after a put-back, CodeScene argument counts. Each is answered on its thread with this commit.
Status:
H1A_2_UNIX_FILE_REMOVAL=IMPLEMENTED_TEST_SUPPORT_ONLY,H1_PROVED=NO,R1_PRODUCTION_RECLAMATION=NOT_ADMITTED,GATE4_TERMINAL=NO.Update: the hardening moved to #1072
Through its review waves this part grew to 1,306 meaningful lines (tier 1,200). The hardening against a same-user actor in the quarantine directory and the durability of the failure paths (commits
eb9eb870,54dcf086,60d6db2b) was cut out with one revert commit (586f6638, no force-push) and is re-applied in #1072 (part 2b), stacked on this PR. This head (1,109 lines) keeps the removal itself with its single rollback point,Stranded, the sync of the changes that are made, the forced-swap tests and the link-count window. Replies on the threads that cite the three commits refer to #1072 now.