Repository navigation
Keep the cache fallback in the temporary directory out of other users' reach - #431
Merged
Merged
Conversation
…' reach Without HOME or XDG_CACHE_HOME the disk cache used <tmp>/com.request-dl-nio.Swift.Cache, a path any user can predict, in a directory any user can write to. It now uses <tmp>/request-dl-nio-<euid> when that is a real directory of the process's user that nobody else can read or rename away, and otherwise a directory with an unguessable name that lasts as long as the process. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #431 +/- ##
==========================================
+ Coverage 94.63% 94.64% +0.01%
==========================================
Files 464 465 +1
Lines 19367 19423 +56
==========================================
+ Hits 18327 18382 +55
- Misses 1040 1041 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The fallback chain is a function of its own now (sharedCachesDirectory(under:)), the unguessable part of a name can be given, and the tests cover a parent that is a file, a directory that cannot be made, a name that is taken, every name taken, and a fallback that finds nothing. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
o-nnerb
added a commit
that referenced
this pull request
Oct 19, 2026
…es and events on a server, cancelled image load (#435) ## What **The tests the audit listed as not written (V5).** Tests only, no source change. | Audit | Test | What it shows | |---|---|---| | **N3** | `CacheReplayTests`: `readPiece_whenTheFileCannotBeRead_isUnreadable` (mode `000`, skipped as root) and `readPiece_whenADirectoryTookTheFilesPlace_isUnreadable` | An I/O error that is not a missing file is as unreadable as a removed file: a miss, not a failure | | **N1** | `InternalsURLSessionUploadFileTests`: `write_whenCancelledWhileSpilling_throwsCancellationAndLeavesNoFileOpen` and `write_whenCancelledBeforeTheThreshold_throwsCancellationWithoutTouchingDisk` | A request body drained slowly (a wait that throws `CancellationError`) and cancelled while it is written to its temporary file: the stream is closed from a cancelled task and nothing leaks | | **L1** | `AsyncBytesServerTests.lines_whenTheReaderPauses_theServerIsHeldBack` (every executor) | A reader that stops after one line of a 48 MiB body stops draining the socket: the server sees a stalled connection, the body is not read into memory to keep up | | **L1** | `AsyncBytesServerTests.events_whenALineNeverEnds_failsAndTheTransferIsCancelled` (every executor) | A response that never ends a line fails with `AsyncBytesItemTooLargeError` once it is over the limit, and the connection is closed (the server's writes fail), long before the body could have been read | | **N10** | `RDLImageLoaderNetworkTests.cancellingTheOnlyCaller_stopsTheDownloadOnTheWire` | A real request cancelled through `RDLImageLoader`: the server holds a 128 MiB download after 1 MiB, the only call is cancelled, the server is let go, and the body did not reach the client (`isBodyComplete == false`, `bodyLength < length`) | ## Checked - **N1:** closing the stream without `Internals.uncancellable` in `FileStreamBuffer.close()` makes the first N1 test end the process with `Fatal error: Leaking file descriptor`, which is what N1 fixed. Restored. - **N10:** not cancelling the download when the last call leaves makes the test fail (`isBodyComplete` is `true`). Restored. - **L1, readers:** without the server's stall detection the test fails at its bound. (The `events` test and the second N3 test were not mutation-checked: the first has no cheap way to fail without an unbounded wait, and the second shares its shape with the existing "file gone" test.) - macOS full suite and the URLSession-only build pass locally. `swift format lint --strict` is clean on the directories touched. The new network tests run on macOS here; Linux and the simulators are for CI. ## Not in this pull request - **N1, the other close sites** (the URL helpers `create`, `readData`, `write`): they need a hook between opening and closing the file, since `openFile` fails first in a cancelled task. Unchanged. - **N7:** already covered by `subscript_whenAnotherInstanceWritesAfterAMiss_findsTheEntryOnceTheIntervalPassed`, which uses a second `DiskStorage` over the same directory for the other process (the index is in the process, so the directory is the only shared state). - **I2:** covered by `RedirectErrorTests` on every executor since #421. - **S5:** the other-uid part needs root; the rule is parameterized by owner in #431. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: brennobemoura <37243584+brennobemoura@users.noreply.github.com> Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
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
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.
What
When the disk cache has no per-user directory to go to, it no longer uses a directory in the shared temporary directory that another user could have made first. (Audit E12, the
/tmpfallback.)The cache lives in
Library/Cacheson Darwin and in$XDG_CACHE_HOMEor$HOME/.cacheon Linux. Only a process with neither (noHOME, noXDG_CACHE_HOME: a launch daemon, some CI runners and containers) fell back to the temporary directory, to<tmp>/com.request-dl-nio.Swift.Cache: a path every user can predict, in a directory every user can write to. A user who got there first could create that directory, which made it theirs, and then rename it from under the process whenever they liked (shown with two uids on Linux). The entries are encrypted and authenticated (EncryptionKey), so what that could do was cost the victim its cache and read the names of the directories, which encode the URLs; this closes the door instead of relying on the key.The change
The fallback is now
Internals'FilePath.sharedCachesDirectory, resolved once per process:<tmp>/request-dl-nio-<euid>, ifFilePath.ownedDirectory(named:under:owner:)trusts it. It creates the directory owner only when it is not there, and trusts it whenlstat, so a symbolic link someone planted is not followed), owned by the process's user, that group and others cannot even read; and<tmp>/request-dl-nio-<random 64 bits>, created owner only withmkdir(which fails if the name exists), if the first is not trusted. It lasts as long as the process does: nobody else could have made it first, and the cache does not survive a restart in this case.lstatto ask (Windows), as it always was.A directory that fails the check is left alone, not repaired or removed: it is someone else's.
What changes for whom
HOMEorXDG_CACHE_HOME: nothing.<tmp>/com.request-dl-nio.Swift.Cache/<suite>to<tmp>/request-dl-nio-<euid>/com.request-dl-nio.Swift.Cache/<suite>. The entries under the old path are not read again and nothing removes them (the same thing that happened when the cache moved out of/tmpfor everyone else; that part of E12 is still open).os(Linux), so no XDG directory is looked for): the same as Linux withoutHOME. The temporary directory of an app is its own, so the check passes and the only difference is therequest-dl-nio-<euid>level.lstatto ask; the temporary directory is per user there).Tests
InternalsCacheDirectoryTests, with who owns what as a parameter (two users cannot be had in a test):0755,0750,0770,0707), or when a symbolic link is there;0777, no sticky bit); used when it is world-writable and sticky (1777); not used when the parent is missing;nilwithout a parent;Checked by weakening each check (the sticky/permission rule on the parent, the owner-only rule, and
lstatforstat): the tests for the parent, for the mode and for the symbolic link fail.macOS passes locally; Linux, Android and the static SDK (musl) are for CI: the code is POSIX and uses
Darwin,Glibc,MuslorAndroidformkdir,lstatandgeteuid.🤖 Generated with Claude Code