sandbox: keep system temp dirs writable in offline installs - #24034
Merged
Merged
Conversation
The offline install branch (formulae with a fetch phase) allowed only HOMEBREW_TEMP, dropping the per-user /private/var/folders/…/[C,T]/ and /private/var/tmp allowances the normal install path gets. macOS tooling writes there via confstr(3), ignoring TMPDIR: Xcode 27's SwiftPM/XCBuild build service (T/TemporaryDirectory.*, T/TemporaryItems) aborts every swift build with a misleading buildpath manifest.json error, mktemp with an inherited default TMPDIR fails, and clang's module cache under the per-user C dir breaks cold-cache cgo builds. Split allow_write_temp_and_cache into allow_write_system_temp plus the HOMEBREW_CACHE allowance and call the former from the offline branch, which keeps the download cache read-only there as designed.
This comment was marked as low quality.
This comment was marked as low quality.
MikeMcQuaid
requested changes
Sep 19, 2026
MikeMcQuaid
left a comment
MikeMcQuaid
left a comment
Member
There was a problem hiding this comment.
Thanks! Want to do some local analysis and testing here.
- Check that system temporary writes preserve private socket grants, child temporary paths and offline build restrictions. - Keep shared temporary sockets denied while allowing sockets in each sandbox's private directory. - Reuse the shared cache allowance on Linux to avoid duplicated rules.
MikeMcQuaid
approved these changes
Sep 19, 2026
MikeMcQuaid
left a comment
MikeMcQuaid
left a comment
Member
There was a problem hiding this comment.
looks good! tweaked slightly and added another regression test. thanks!
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.
brew benchmarkresults.brewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?AI-assisted contribution by Claude Code (Fable 5) of ~95% of the work (diagnosis, fix, spec, validation); results reviewed by the user.
What: splits
Sandbox#allow_write_temp_and_cacheintoallow_write_system_temp(system temporary directories, incl. the macOS per-user/private/var/folders/…/[C,T]/dirs) plus theHOMEBREW_CACHEallowance, and makesFormulaInstaller's offline-install branch (formulae with afetchphase) callallow_write_system_tempinstead of allowing onlyHOMEBREW_TEMP. A spec asserts the new method keeps the per-user dirs writable without opening the download cache.Why: the offline branch narrows the sandbox to keep
HOMEBREW_CACHEread-only, but as a side effect it also drops the per-user system temp/cache dirs that the normal install path allows. Plenty of Apple tooling writes there viaconfstr(3), ignoring$TMPDIR, sofetch-phase formulae hit failures that plain formulae never see:T/TemporaryDirectory.*andT/TemporaryItems; when denied, everyswift buildin afetch-phase formula aborts with a misleadingunable to write manifest to '<buildpath>/.build/out/…/XCBuildData/manifest.json'error. With SwiftPM'sswiftbuildbuild system now the default (native is deprecated), this blocks sandboxing any Swift formula.mktempwith an inherited defaultTMPDIR(seen locally with redis'sdeploy.shand pulumi duringfetch-phase conversion work).Cdir (seen as CI-only failures on cgo + Apple-framework builds, e.g. aws-vault/ipatool conversion attempts; locally masked by a warm cache since reads stay allowed).Reproduce: with Xcode 27 selected and any Swift formula converted to a
fetchphase (e.g. adddef fetch; system "swift", "package", "resolve", "--disable-sandbox"; end+deny_network_access!tomint):Bisected by dumping the generated seatbelt profile and replaying it via
sandbox-execon a plainswift build: the captured 161-line profile reproduces the failure outside brew; adding the single line(allow file-write* (subpath "$(getconf DARWIN_USER_TEMP_DIR)"))makes it pass. With this PR applied, the samebrew install --build-from-source mintsucceeds end-to-end.Not changed: the offline branch still denies
HOMEBREW_CACHEwrites (apart from the package-manager caches) and all network, per its design.Security posture: this grants fetch-phase installs nothing that every normal formula build does not already have —
allow_write_temp_and_cacheincludes these exact allowances on the ordinary install path today — so the offline sandbox stays strictly tighter than the normal one (network denied, download cache read-only), just no longer accidentally tighter in a way Apple tooling cannot tolerate. The pre-existing breadth of the[C,T]grant (e.g. a build can write another process's clang module cache) is unchanged by this PR; if narrowing is preferred, a follow-up could restrict both paths to the specific patterns tooling needs (T/TemporaryDirectory.*,T/TemporaryItems,C/clang/ModuleCache, …) — kept out of this parity fix to limit regression risk.