Skip to content

fix(sandbox): confine child file reads - #841

Open
rustytrees wants to merge 2 commits into
indaco:mainfrom
rustytrees:security/sandbox-read-confinement
Open

fix(sandbox): confine child file reads#841
rustytrees wants to merge 2 commits into
indaco:mainfrom
rustytrees:security/sandbox-read-confinement

Conversation

@rustytrees

@rustytrees rustytrees commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Description

The macOS sandbox permits the metadata and system runtime reads needed to launch ordinary tools, but denies file contents under user, application, package-manager, device, and mutable system roots. Formula data is readable only under the current keg and Malt prefix.

Formula tools such as fontconfig also need the standard system and per-user font directories during post-install. Those directories receive narrow read grants; the rest of HOME remains inaccessible. Generated Ruby wrappers receive a literal read grant, and system Ruby starts with RubyGems disabled so it does not scan host-installed gems under /Library/Ruby.

The regression tests run real sandboxed processes. One confirms that a child cannot copy an arbitrary file from HOME into the prefix. Another confirms that an official-style post-install command can read a font from ~/Library/Fonts.

Related Issue

Closes #847.

Notes for Reviewers

The security regression failed before the original fix because the outside source was copied. A compatibility regression added during catalog testing also failed before the follow-up commit: the sandbox denied a font under ~/Library/Fonts, leaving the fast suite at 2,424 of 2,425 tests.

Verification after both fixes completed with:

  • zig build test-one: 2,425 passed
  • zig build test: 5,062 passed, 3 skipped; exit code 0
  • zig build: 8 of 8 steps succeeded
  • zig fmt --check on all changed Zig files
  • git verify-commit HEAD

Deny file contents under mutable and user-data roots while preserving the metadata and system runtime reads macOS processes need. Grant formula data and generated Ruby scripts explicitly, and start system Ruby without scanning host-installed gems.
@indaco

indaco commented Aug 13, 2026

Copy link
Copy Markdown
Owner

@rustytrees Thanks for the contribution! One small request: could you please instruct your agent to follow the PR template provided in the repository for PRs? It helps keep the PRs consistent and easier to review. Thanks!

Formula tools such as fontconfig scan system and per-user font directories during post-install. Keep those directories readable without opening the rest of HOME. Pass HOME to each sandbox profile and cover both the allowed font read and a denied sibling file.

@indaco indaco left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rustytrees Thanks for this one - I reproduced the issue and the fix does what it claims. Rendering the profile and running it under real sandbox-exec, a fenced child on main reads $HOME/.ssh/id_rsa and exits 0, and since file-write* covers <prefix>/share it can land the copy in the keg. With this branch that read is refused and ~/Library/Fonts still works. Good catch, and thank you for the compatibility regression around fontconfig - that is the right instinct.

Two things before I can merge.

1. Two grants use literal where they need subpath (blocking)

/private/var/db/timezone and /Library/Apple are directories, but (literal ...) matches only the directory node itself, so nothing inside them is granted. The deny on (subpath "/private") therefore takes the whole timezone database with it.

Measured against the profile this branch renders, unmodified:

                            main    this PR
/usr/share/zoneinfo/UTC     OK      DENY     (it is a symlink into /private/var/db/timezone)
/etc/localtime              OK      DENY
TZ=Europe/Rome date         CEST    UTC      <- wrong, and silent

Every fenced child silently loses local time. Nothing errors, the clock is just wrong.

Changing those two rules to subpath fixes it - I verified:

TZ=Europe/Rome date  ->  Tue Aug 18 10:48:21 CEST 2026

/Library/Apple has the same shape and is dead as written, since Rosetta lives under /Library/Apple/usr/....

2. (subpath "/private") also removes all of /etc (needs a decision)

/etc is a symlink to /private/etc, so the blanket deny takes it too. Measured main -> this PR:

/etc/ssl/cert.pem            OK -> DENY
/etc/paths                   OK -> DENY
/etc/hosts                   OK -> DENY
/Library/Preferences/*       OK -> DENY
/opt/homebrew/*              OK -> DENY

The cert bundle is the one I care about. I am not asking you to revert the deny - the tighter default is the right direction - but I would like /private/etc granted back, or at minimum /etc/ssl. If you think that reopens something meaningful, say so and I will take the stricter version instead.

For the record, so it does not muddy this: creating files in /tmp and mktemp in $TMPDIR already fail on main. file-write-create was never granted there, so that is not a regression from this PR.

On the tests

The profile tests assert on rendered text, and only two of them spawn a real sandbox-exec. None of them would have caught the literal/subpath bug, which is the kind of mistake this change is most exposed to. Could you add a test that actually runs something under the rendered profile and asserts an outcome rather than a substring? Reading a file under each granted root would be enough, and a TZ assertion would have caught this directly.

--disable-gems is fine - your description explains why it is needed once /Library is denied. I will note it in the changelog as a behaviour change.

Happy to take 1 and 2 as a follow-up commit here rather than a new PR.

@indaco

indaco commented Aug 18, 2026

Copy link
Copy Markdown
Owner

Answering the /etc question so you can do the fix and the rebase in one pass.

Grant (subpath "/private/etc/ssl") and nothing else. Keep the /private deny as you wrote it.

I need to correct my own reasoning first: I said the cert bundle was what I cared about. That was wrong - the profile denies network, so a fenced child cannot do TLS and the CA bundle is irrelevant. The real dependency is narrower. LibreSSL reads its config unconditionally at startup:

fopen('/private/etc/ssl/openssl.cnf','rb'): Operation not permitted

so any post-install step that shells out to openssl prints three error lines, and anything that depends on the config misbehaves.

Everything else I probed is fine with /private denied: id, whoami and dscl all work (macOS resolves users through Directory Services over mach-lookup, not /etc/passwd), plus sw_vers and shell startup.

Worth knowing for the other half of the review: timezone was never an /etc problem. With only the literal -> subpath change applied and /private still denied, TZ=Europe/Rome date returns CEST and /usr/share/zoneinfo reads fine. So the two issues are independent - D-1 is the whole timezone fix.

Confirmed with the narrow grant in place:

openssl version     -> LibreSSL 3.3.6   (clean)
$HOME/.ssh/id_rsa   -> denied
/etc/passwd         -> denied
/etc/hosts          -> denied

So the full ask on this PR is: the two literal -> subpath rules, plus (subpath "/private/etc/ssl"), plus a rebase onto current main. Note #846 has to land after this one - you both rewrite spawnFenced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: sandboxed post-install commands can read arbitrary user files

2 participants