Linux: pin ancestor directories of deny binds against rename - #514
Open
ronleizrowice-ant wants to merge 13 commits into
Open
Linux: pin ancestor directories of deny binds against rename#514ronleizrowice-ant wants to merge 13 commits into
ronleizrowice-ant wants to merge 13 commits into
Conversation
Each denyWrite bind or read-deny file mask makes only its destination a mountpoint; the directories between it and the covering allowed write root could be renamed, carrying the bind along and leaving the path recreatable unprotected. Emit a self --bind for each such directory so rename/rmdir on it fail EBUSY, seeded from both deny binds and file masks, skipping allowed write roots, deny dests and any directory that contains a read-deny tmpfs. Pins ride the existing emission filter and tmpfs/mask re-application passes, which now compare recorded and canonical spellings, replay the read section's actual restores instead of re-deriving them, and refuse restores that would bury an earlier read-deny mount. Only ENOENT/ENOTDIR count as absence in the pin walk; an unverifiable component aborts the wrap. Remote-Dev: homespace
computeAncestorPins is a pure, exported walk with injected probes and direct unit tests. The unverifiable-component abort now names the path and the remedy. The nested-repo behavioral test keeps the repo within the default scan depth, quotes its rename paths correctly, and a new case pins the depth rule. README documents pinned-directory behavior. Remote-Dev: homespace
…ready covers Expanding denyRead ['/'] into per-child tmpfs mounts also emitted them for children the caller explicitly allowed (/bin, /usr, /lib, ...). On merged-/usr hosts /bin, /sbin and /lib* are symlinks into /usr, so those synthetic denies landed at /usr/bin, /usr/lib, ... and the read-deny burial veto then refused to restore /usr, leaving nothing executable inside the sandbox (the three 'denyRead at filesystem root' tests in allow-read.test.ts). A child whose landing location an allowRead entry equals or contains is now skipped at expansion; explicit deny entries and the veto itself are unchanged. Also corrects the scan depth in the nested-repository pin test: ripgrep's --max-depth counts the matched file, so a/b/c/.git/config needs 5. Remote-Dev: homespace
… mount A pin's only job is to make the directory a mountpoint so rename/rmdir on it fail EBUSY; the kernel checks that against every mount on the dentry, so a pin buried under later mounts still works. Emitting each pin as '--ro-bind P P' straight after '--ro-bind / /', before the allow roots, deny binds, tmpfs units and masks, means: - no writable bind is ever placed on a directory the sandboxed process can create, so a component swapped for a symlink between the walk and bwrap's mount yields at worst a redundant read-only view under the allow root instead of a writable rbind of the parent tree; - nothing has to be excluded or re-applied around pins (a pin at or below a deny dest, inside a carve-out, or above a read-deny tmpfs is fine — those mounts land on top), so the exclusion probes and the carve-out clause go; - no vfsmount boundary sits on the lookup path, so renames across a pinned directory no longer fail EXDEV; only rename/rmdir of the pinned directory itself fail; - an unverifiable pin component drops that pin instead of aborting every command. Tests re-pointed to the new plan shape; new cases for a pin above a read-deny tmpfs, a pin inside a denied directory, the no-EXDEV property under bwrap, and the relative-seed guard in computeAncestorPins. README updated. Remote-Dev: homespace
No behaviour change: - pushReadDenyDirMounts returns the write paths it actually re-bound instead of recording them in both a local array and an optional out-parameter; the read section reads the return value. - The ancestor-pin walk seeds from denyWriteRawDests, which the deny loop already keys by every dest it produces, rather than re-collecting dests from denyWriteArgs. - The root-expansion allowRead check drops `form === child`: child and allowPath go through the same memoized mountForms, so an allow entry equal to the child always also matches the child's canonical location. - The allowRead restore compares the resolved path against the entry itself; readAllowPaths arrive slash-free from normalizePathForSandbox.
Conflicts in src/sandbox/linux-sandbox-utils.ts with anthropics#502's root-aware containment helpers: the two re-application filters keep this branch's canonical-location and mount-form tests and now go through isStrictlyUnder / isAtOrUnder; the stub-skip INVARIANT comment carries main's wording plus this branch's rationale.
The pin and read-deny code compared paths with a local pathSep() and inline `x === d || x.startsWith(d + '/')` tests, two of which were not root-aware. They now go through isAtOrUnder / isStrictlyUnder from sandbox-utils, which the rest of the file already uses, and pathSep is gone. The two root-unaware sites only see a write path of '/' when it lies beneath a read-deny tmpfs, which cannot happen (a '/' deny is expanded into its children), so no emitted plan changes.
…ad the emitted record Realpaths were memoized by a helper that a config-derived list of read-deny locations filled before the mandatory-deny scan's await. That scan can run arbitrarily long, so a symlink retargeted meanwhile left every later predicate (the restore's covering test, the emission filter, the burial veto) comparing against where the link used to point. canonicalForm is now the memoized primitive, nothing calls it before the await, and the stub-skip veto inputs use it instead of resolving by hand. A test retargets a denyRead symlink from a stand-in ripgrep and checks the write path under the new target is the one restored. The config-derived list is gone. The re-application vetoes against the record of read-deny mounts actually emitted (tmpfs units, file masks and now the credential masks), which is what the first emission already did: the derived list missed root-expanded children and the implicit ssh_config.d tmpfs and counted entries that never mount. The pin phase reads the same record. It runs after the read section and seeds from the deny dests and the emitted masks rather than re-deriving which denyRead entries become masks, and it is one function, ancestorPinArgs. A directory that is missing, cannot be inspected or has a symlink component gets no pin, under one policy: the walk used to keep a pin whose stat failed with EACCES and then drop it when lstat failed the same way. computeAncestorPins is no longer exported for its unit test; the wrap-level suites cover the walk. Shapes: one record per tmpfs unit instead of two index-parallel lists, a named-field argument for pushReadDenyDirMounts, a three-state union for a pin path component.
A typed fixture tree instead of Record<string, unknown> and a cast; run() pins LC_ALL=C for the strerror matches; blocks that execute bwrap assert the command's DONE marker so a rejected mount plan cannot pass them; the exchange-rename probe is its own case, skipped without python3, rather than a silent `true`; a space-terminated --tmpfs match where one path is a prefix of another; the git case turns commit signing off so a global commit.gpgsign cannot fail it. The four-in-one rename case and the implicit-tmpfs file repeated single-behaviour cases and are removed, as are comments that narrate the assertion beneath them.
Each existing directory up to the outermost allowed write root, and what rm -rf leaves behind; the mount-ordering clause a reader cannot act on is dropped.
Contributor
Author
|
Merged
Local run (Linux x86-64, bubblewrap 0.11, unprivileged userns): eslint and |
…d-denied directories A read-denied directory did not seed ancestor pins, so `mv proj/other proj/o2` still moved its tmpfs aside and the next command, no longer finding proj/other/secrets, left it readable. Every read-deny mount now seeds the walk: tmpfs units as well as file and credential masks. The read section keeps one record of what it emitted, tmpfs units with the restores they made and file masks, each with its landing: where the mount sits in the sandbox. bwrap resolves a destination in the root built so far, so a symlink an earlier tmpfs has hidden is created literally on that tmpfs and never reaches its host target. The emission filter used to treat such an entry as mounted at its host realpath and dropped the deny binds under that target as hidden, mandatory ones included; it now decides by the landing. The pins and both re-application passes read the same record. Entries are processed shallow-first by canonical depth rather than by spelling. A unit can then restore its write and allowRead paths unconditionally, since nothing emitted so far can lie inside one, so the burial veto and its bookkeeping are gone. The veto dropped a whole allowed write root (often the cwd) whenever a symlink-spelled entry inside it happened to sort first, and dropped an allowRead carve-out in the re-application pass whenever any read-deny entry lay inside it. bwrap cannot mount onto a symlink that dangles in the new root. A destination that lands somewhere an earlier tmpfs hid is therefore spelled by its landing, and an allowRead carve-out is bound at its resolved path. On a merged-/usr host a root read-deny produced `--tmpfs /usr` followed by `--tmpfs /lib32` and the sandbox did not start; that also happens before this branch, depending on readdir order. An allowRead entry spelled through a symlink is a carve-out of a unit when its target lies inside the denied directory (/lib/x under a deny of /lib or /). The previous guard skipped every symlink-spelled entry. A denyRead path that exists but cannot be inspected (EACCES after chmod 000 on its parent, which a same-uid command can undo from inside the next sandbox) was treated as absent and got a pin but no mask. The deepest directory above it that can be inspected is now hidden instead, with nothing restored inside it. ENOENT, ENOTDIR, ELOOP and ENAMETOOLONG still mean absent. The pin walk applies the same split: a missing directory gets no pin, an uninspectable one is pinned anyway, and the per-component lstat walk that turned "cannot inspect" into a silently dropped pin is removed. When a denyWrite bind re-exposes a read-denied directory and its tmpfs is re-applied, an allowed write path inside comes back read-only: it lies under the deny too. The previous filter always reduced the restore list to nothing, so the carve-out vanished. The write-allowlist predicates go back to plain prefix tests. Made root-aware, an allowOnly of '/' contained every path: every cwd stub and a pin for each ancestor up to the root appeared where only `--bind / /` was emitted before, and with denyWithinAllow ['/'] and any read-deny tmpfs the stubs for absent cwd dotfiles landed after the read-only root, where bwrap cannot create them. An earlier commit on this branch called those sites unreachable; they are reachable with allowWrite ['/']. The stub-skip predictor now takes its tmpfs set from the same root expansion as the read section, so a root child that an allowRead covers no longer keeps a stub for a tmpfs that is never mounted. Test hygiene: the exchange-rename probe uses the process's own libc and skips where it has no renameat2 wrapper, the FIFO case skips without mkfifo, the nested-repo commit ignores a global hooksPath, and a pin that is the same string as a deny bind is counted rather than located.
A read-denied directory now pins its ancestors as well, so spell out what "a protected path" covers.
Contributor
Author
|
Second round on top of
Not fixed here, stated in the description: pins roughly double the profile size, so Verified on Linux x86-64 under real bubblewrap 0.11 with |
…links to The previous commit accepted an allowRead entry as a carve-out of a read-denied directory when the entry's canonical form lay inside it. An entry that merely points at the directory from outside then counted: with allowWrite [proj], denyRead [~/.ssh] and allowRead [proj/docs], a command that ran `rm -rf docs; ln -s ~/.ssh docs` (or a repository that ships that link) made the next wrap emit `--tmpfs ~/.ssh --ro-bind ~/.ssh ~/.ssh`, and the key was readable. The same test decided which root children a root read-deny skips as covered, so `docs -> /etc` left /etc unhidden. An allowed path is now bound back only when its name lives inside the denied directory: its parent directories are resolved, its last component is kept as written, and that location must be at or under where the tmpfs landed, as must what the path resolves to, which is where the bind goes. /lib/x86_64-linux-gnu under a deny of / or /lib on a merged-/usr host is still restored (its parent /lib resolves to /usr/lib, so the name lives inside the /usr tmpfs); proj/docs -> ~/.ssh is not (the name lives in proj). An entry that is itself a symlink restores nothing. The file-mask exemption follows the same rule: a mask is lifted only by an entry that names that very file, through whatever symlinked directories, not by a symlink to it. A planted sub/.env.example -> ../.env matched by both an allowRead glob and a denyRead glob leaves .env masked.
Contributor
Author
|
One more commit, The rule is now the one #503 uses too, so the two PRs converge: an Six tests fail on |
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
2 participants
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.
Summary
On Linux a deny bind, a read-deny file mask or a read-deny tmpfs makes only its destination a mountpoint. The directories between it and the covering allowed write root carry no mount, so a sandboxed command can rename one of them and have the mount travel with it. For a write deny the path can then be recreated unprotected (
mv .git aside && mkdir .git && … > .git/hooks/pre-commit); for a read deny the next command's wrap no longer finds the path and leaves it readable under its new name (mv proj/other proj/o2, thencat proj/o2/secrets/key).Each existing directory between such a mount and its outermost covering allowed write root now gets a read-only self-bind (a "pin"), emitted straight after
--ro-bind / /and so beneath every other mount. That is enough forrename/rmdir/RENAME_EXCHANGEon it to failEBUSY; permissions, deny binds, tmpfs units and masks are decided by the mounts above it, and no mount boundary appears on the lookup path, so renames inside, out of or across a pinned directory behave as before (noEXDEV). A directory that is missing gets no pin. One that exists but cannot be inspected is pinned anyway; if bwrap cannot bind it the sandbox does not start, which is where the deny loop already lands for an unverifiable path.About half the source diff is the read section, which the pins depend on being exact about where mounts land:
allowReadpaths unconditionally, because nothing emitted so far can lie inside one; the outcome no longer depends on the order same-depth entries are listed in, and no write root is dropped.allowReadcarve-out is bound at its resolved path. bwrap cannot mount onto a symlink that dangles in the new root: on a merged-/usrhostdenyRead: ['/']produced--tmpfs /usrfollowed by--tmpfs /lib32, and the sandbox did not start (this fails onmaintoo, depending onreaddirorder).allowReadentry re-allows the name it is, not what it links to. It is bound back over a read-deny tmpfs only when its name lives inside the denied directory (its parent directories resolved, its last component as written) and it resolves to somewhere inside it too, which is where the bind goes. So/lib/x86_64-linux-gnuunder a deny of/libor/on a merged-/usrhost is restored (the name lives inside the/usrtmpfs), while a link planted at an allowed path (proj/docs -> ~/.ssh, withdenyRead: ['~/.ssh']andallowRead: ['proj/docs']) restores nothing, and an entry that is itself a symlink never does. The same rule decides which root children a root read-deny skips as covered, and a file mask is lifted only by an entry that names that very file, not by a symlink to it (sub/.env.example -> ../.envmatched by both anallowReadand adenyReadglob leaves.envmasked).denyRead: ['/']skips a root child anallowReadentry covers, and the stub-skip predictor uses the same expansion.denyReadpath that exists but cannot be inspected (EACCESafterchmod 000on its parent, which a same-uid command can undo from inside the next sandbox) is no longer treated as absent: the deepest directory above it that can be inspected is hidden instead, with nothing restored inside it.ENOENT,ENOTDIR,ELOOPandENAMETOOLONGstill mean absent.denyWritebind re-exposes a read-denied directory and its tmpfs is re-applied, an allowed write path inside comes back read-only (the deny wins) instead of writable (main) or not at all.This supersedes #485 (both of its commits are kept with their authorship); #503 rebases onto it.
Blast radius: Linux only. A config with no deny path, file mask or read-denied directory under an allowed write root gets no pin. Not behind a flag: the pins add mountpoints, not permissions.
A
/write root is unchanged frommain.allowOnly: ['/']contains only/itself for the purposes of denies, stubs and pins, so it still emits--bind / /and nothing else, andallowOnly: ['/']withdenyWithinAllow: ['/']still skips the stubs for absent cwd dotfiles (bwrap could not create them on a read-only root). Both are pinned by tests.Argument size. Pins add one
--ro-bind P Pper distinct ancestor, roughly doubling the profile for repositories with many nested deny paths. The wrapped command is a singlesh -cstring, so it meets LinuxMAX_ARG_STRLEN(131072 bytes) at about half the repository size it did before: measured with a 44-character cwd, N first-level nested repos (X/.git/config) cross at N≈334 instead of N≈913, and NX/.vscode/settings.jsonat N≈515 instead of N≈968. #504 moves the arguments to--argstransport and removes the limit; until it lands, very large monorepos hitArgument list too longsooner.Known follow-up.
EBUSYonrename/rmdirof a pinned directory is a new sandbox-caused failure that the Linux violation monitor cannot attribute yet: the directory is insideallowWriteand under nodenyWriteentry, so the event is dropped and stderr shows a bare "Device or resource busy". Teaching the monitor about pinned directories is left to a follow-up.Test plan
bun teston Linux with unprivileged bubblewrap 0.11.2. Same failure set asmainon that machine (environment-only); CI is the real check. Every regression test below fails on the previous head of this branch.linux-ancestor-pin.test.tsdrives the walk throughwrapCommandWithSandboxLinux(leaf to root, nested write roots, pins at and below other deny dests, above a read-deny tmpfs, inside a denied directory, above a read-denied directory) and, under bwrap, checksmv/rmdir/exchange-rename of a pinned directory failEBUSYwhile work inside it, renames across it andgit commitsucceed; that a read-denied directory cannot be renamed out from under its tmpfs; that a carve-out inside a denied directory comes back read-only; and that a sandbox with a symlink and its target directory both read-denied starts and honours the carve-out.linux-ancestor-pin-errno.test.ts: a missing directory gets no pin, an uninspectable one is still pinned, an uninspectabledenyReadpath hides the directory above it, masks (a FIFO included) seed the walk and an unmasked entry does not.linux-mount-plan-record.test.ts: the plan is the same whichever order a symlink-spelled entry is listed in and keeps the write root; deny binds under the target of an already-hidden symlink are kept;allowReadcarve-outs reached through symlinked parent directories are restored, while an entry that is a symlink into a denied directory, to a denied file or to a root child restores and lifts nothing (also driven under bwrap: a planteddocs -> ~/.sshleaves the key unreadable); re-application restores a carve-out and then the deeper deny; adenyReadsymlink retargeted during the scan is resolved afterwards; and the two/-root plans matchmain.