Repository navigation
Mandatory denies: submodule git dirs, .git pointer files, nested hooks, gitignored paths - #569
Draft
ronleizrowice-ant wants to merge 158 commits into
Draft
ronleizrowice-ant wants to merge 158 commits into
ronleizrowice-ant wants to merge 158 commits into
Conversation
… dirs, .git pointer files, and gitignored paths
The mandatory write-denies keep a sandboxed command from leaving behind
something the host's git later executes. Four shapes slipped through:
- Linux: the ripgrep scan matched a nested repository's hook FILES
(`**/.git/hooks/**`), one segment deeper than its config, so at the
default depth a repository directly under cwd had .git/config denied
but .git/hooks writable. The scan now also matches `**/.git/HEAD`, and
any file inside a `.git/` marks a repository whose hooks/ and config are
denied, as for cwd's own .git.
- Linux: rg honoured .gitignore/.ignore/.rgignore, which the command can
write, so one command could hide a nested repository from the next
command's scan. The scan passes --no-ignore.
- Both: a submodule's git directory under .git/modules/<name>/ (its
hooks/ and config) matched nothing. Linux walks .git/modules for git
directories; macOS adds **/.git/modules/**/{hooks/**,config}.
- Both: a `.git` FILE (linked worktree or submodule checkout) could be
repointed at a directory the command prepared. An existing one is now
read-only — on macOS by vnode type, so .git directories are untouched
and creating a new pointer is still allowed — and the hooks/config it
leads to are denied: the named git directory's, or for a worktree the
commondir's (the main repository's) plus an existing config.worktree.
Also: the Linux match-to-directory mapping compared single segments
against the two-segment names .claude/commands and .claude/agents, so a
nested one got per-file binds and new files stayed creatable; names now
match as segment runs on the cwd-relative path.
No conflicts.
…ative to cwd macOS: the create re-allow for `.git` pointer files followed the write denies unconditionally, so a command could create `sub/.git` under a cwd that allowOnly never covered. It is now intersected with the allowed write paths and dropped when there are none. The filter helper is no longer exported. Linux: rg prefixes each match with its target, so the segment scan also looked at cwd's own ancestors. With cwd under a directory named like a dangerous one (~/.vscode/ext/foo), a dangerous file below cwd mapped to a deny of that ancestor and was itself left writable. Matches are now taken relative to cwd. Also: - An unreadable directory under cwd made rg exit 2 and the scan discard every match it had printed. ripGrep now throws RipgrepError carrying the partial matches, and the scan keeps them and warns. - A nested repository's .git/modules is walked like cwd's own, matching what the macOS pattern already covers. - config.worktree is denied whether or not it exists yet, like hooks/ and config. - gitFileDenyPaths and submoduleGitDirs treat only ENOENT/ENOTDIR as "absent" and log anything else. - The git deny-path helpers move out of sandbox-utils.ts into mandatory-deny-paths.ts. - Tests: a regression case for each of the above and for the HEAD-only nested repository (allowGitConfig), multi-behaviour cases split, positive controls for the worktree cases, the unused `append` option dropped.
commondir names the git directory whose hooks and config git reads, in any git directory and not only a linked worktree's, so one write to it redirects every deny below it. config.worktree is read instead of config wherever extensions.worktreeConfig is on. Both now come from gitDirDenyPaths, so every git directory gets them on both platforms. A repository is recognised by any regular file directly inside its .git (one `**/.git/*` glob replaces three) and a submodule git directory by any of HEAD/config/hooks/objects, so moving HEAD aside no longer hides either from the next command's scan. Detection no longer depends on allowGitConfig, which governs what is denied and not what is found. The three discovery paths fail closed. A directory ripgrep could not read is denied whole, from the paths it names in its own diagnostics; a directory the .git/modules walk could not list is denied whole; an unstattable pointer target is denied at the deepest ancestor still reachable; and a scan that does not finish inside its timeout aborts the wrap rather than sandboxing with a deny list of unknown completeness. A pointer target is followed only when it looks like a git directory, so `gitdir: ..` cannot point the deny list at an ordinary config/ tree, and an absent target is denied so the command cannot create it and fill it with hooks before the host's git first uses it. Pointer files are read as git reads them: a bounded prefix of a regular file, opened non-blocking, a prefix check and no backtracking regex, so a FIFO or a very large file left at that path cannot block or slow the host. On macOS the two mid-`**` module globs are gone, replaced by literal enumeration of the working directory's own submodule git directories plus a single-segment nested pattern - exact paths with ancestor pins, and no more denying refs/heads/config or a submodule named config. An existing .git pointer also gets a trailing file-write-unlink deny, which the read section's unlink re-allow for write roots was winning over whenever a read config was present. On Linux, placeholder destinations are deduped through seenDenyWrite, so one destination cannot take two different placeholder mounts and abort bwrap for every later command in that directory. ripgrep runs --null, so a path containing a newline is not split in two and a killed run's truncated tail is dropped rather than denied. The .git/modules walk has its own depth bound instead of borrowing the scan-depth knob, logs when the bound cuts it, and follows a symlinked modules entry.
Integration cases for each gap: removing and renaming over a pointer file with a read config present, commondir and config.worktree in the working directory's repository, in a submodule git directory and in a nested repository, a repository and a submodule git directory whose HEAD was moved aside, a pointer naming an ordinary directory, a dangling pointer's target, a directory the scan could not read, and a scan that does not finish. Unit cases for the helpers: the pointer parse, the size cap, a FIFO at the pointer and at the commondir, multi-segment and nested submodule names, a symlinked modules entry, an unlistable directory and the walk's depth bound. ripGrep gains cases for the NUL split, the dropped truncated tail and the stderr its caller denies from. Also drops a duplicated cleanupBwrapMountPoints() call and a comment describing behaviour this branch had already replaced, and gates the unreadable-directory case on the platform rather than on getuid, which is undefined (and so not 0) on Windows.
Spells out commondir and config.worktree and why they are denied, the difference between the enumerated working-directory repository and the pattern-matched nested ones, and the fail-closed scan. Lists the git operations that no longer work inside the sandbox, and states the residual: a command can still rename aside the directory holding a pointer and create a fresh one in its place, which the ancestor pins block on Linux within the scan depth and for the macOS literal denies.
The metadata read stopped at 8 KiB and gave up at that size, so a .git file padded past it protected only the pointer file itself while git still followed it: a valid gitdir line with 9,000 newline bytes after it is a pointer git rev-parse accepts before and after the padding, and the target's hooks and config went back to being writable inside the sandbox. git's read_gitfile_gently refuses a gitfile only past 1 MiB, and below that reads it whole: the prefix sits at offset 0, \n and \r are stripped from the end of the whole file rather than a line being taken from it, the path ends at the first NUL, and nothing is trimmed. commondir is read the same way minus the prefix, and with no size limit at all. This now follows those rules on bytes rather than on a decoded first line, sizing the read from the fstat -- one byte past it, so a file that grew since is read on rather than taken for whole -- and re-applying the bound to what actually arrived, since readSync can also come back short. Where the rules cannot be followed the wrap is refused rather than applied with a deny list that may name the wrong directory: a commondir larger than is read here, and a path whose bytes are not valid UTF-8, where a JavaScript string of them opens a different file than git does. That is the call already made for a scan that does not finish in time. A target too long for the filesystem goes the other way and is followed nowhere: git's own stat of it fails, and no command can create anything there either, so there is nothing below it to protect.
The padded pointer gets a case through the real wrapper on both platforms, and unit cases cover both sides of the 1 MiB bound, a path spanning lines, NUL truncation, CRLF, a padded commondir, an untrimmed one, and the two paths that refuse to sandbox. Hand-written cases only cover the shapes their author thought of, and the input that matters is the one this resolves confidently to one directory while git opens another. So each shape is also built on disk with a real target git directory and resolved twice, once here and once by git rev-parse in the pointer's own directory with the git environment scrubbed: wherever git follows a pointer, that directory's hooks, config, config.worktree and commondir have to be in the deny list, or the wrap has to have been refused. Denying more than git follows is fine; denying less fails. The corpus crosses prefix spellings, trailers, path spellings and target shapes, and a seeded generator mixes the same ingredients across another hundred and fifty cases. It skips where git is not installed.
The paragraph on the git denies said which paths are protected but not how the pointer leading to them is read, which is where the size bound and the refusal to sandbox now live.
macOS filesystems reject a directory name that is not valid UTF-8 (mkdir fails with EILSEQ), and the target has to exist for git to follow the pointer to it. The refusal itself is platform-independent and stays covered everywhere by the unit case, which needs no such directory.
…paper path.resolve folds .. lexically, so `gitdir: link/../evil` resolved to <checkout>/evil while the kernel follows link first and hands git the directory under link's target instead: the deny list named a directory git never opens, and left the hooks of the one it does open writable. The same held for the directory the pointer file itself sits in, when the checkout is reached through a symlink and .. pops its physical parent, and for a commondir read out of either. A .. in any of those values now also gets a physical walk: component by component from the real path of the directory the value was read in, following each symlink where it is met, taking the rest of the path as written past a component that is not there (the kernel cannot traverse one either, so nothing beyond it can redirect the path), and giving up on a loop at the same 40 hops the kernel allows, which hands the loop itself to the existing unreadable handling and denies the deepest directory that still reads. Where the walk lands somewhere other than the lexical fold, both directories are denied; where it lands on the same object by another name -- a temporary directory under a /var that is a symlink to /private/var -- nothing is added, so no deny that exists today changes its spelling. fs.realpathSync is not that walk: it runs its argument through the same lexical folding before resolving it, and it cannot reach a target that does not exist yet, which is the one that has to be denied so the sandboxed command cannot create it and fill it with hooks.
The walk that finds submodule git directories is bounded at ten levels, and on reaching the bound it only logged and moved on. Everything else this cannot inspect fails closed -- a directory ripgrep could not read is denied whole, so is one the walk could not list, and a pointer target that cannot be inspected is denied at its deepest reachable ancestor -- which left the bound as the one unknown still writable: a submodule git directory nested eleven levels down under .git/modules kept its hooks/ and config, the pair these denies exist to protect. The directory the walk stops at now joins the ones it could not list and is denied whole, covering whatever is beneath it without inspecting it. The bound is counted per .git/modules tree, so that directory is always at least ten segments inside one and can never be the repository or anything above it. Reaching it takes a submodule name of ten path segments or five levels of nesting, which no real tree has; a tree that does gets that one directory read-only inside the sandbox. Also records, at the two "is this a git directory" predicates, why they differ. The .git/modules walk takes config and hooks as markers because its input is a tree git itself created, so a looser test only ever denies more. A gitdir:/commondir target is named by file content a sandboxed command can write, so the same test there would let that content aim a deny at any directory that merely holds a config or hooks.
… directory A git directory exactly at the walk's depth bound went into both lists: into gitDirs, so its hooks and config were denied by path, and into unreadableDirs, so the backends denied it whole and took its objects, refs and index with it. Ten levels of ordinary nested submodules were enough to reach it, and git inside that submodule then failed on add, commit and fetch, where before only hooks and config were denied. What the walk would have descended into is denied instead: the modules directory beneath a git directory, absent or not, since a sandboxed command must not be able to create one and hide a git directory inside; otherwise the directory itself, as before. The depth-bound tests fold into one parameterised case asserting the whole scan shape, with its segment count derived from the constant rather than repeated. The Linux arm asserts the argv and, where bubblewrap can build its namespaces, runs the wrapped command: a write to objects/ succeeds inside the bound git directory while hooks/, config and creating modules/ fail.
…ne costs The SubmoduleScan doc named two producers and the README one. There are three — a directory the walk cannot list, an entry it cannot stat, and each branch that reaches the depth bound — and for the first two what is recorded is the deepest ancestor this process can reach, which can be .git/modules itself. Say what such a deny costs: everything beneath it is read-only, a submodule's objects, refs and index included. gitDirKind's doc claimed the walk's looser marker set cannot be aimed anywhere. It can, through a symlinked entry under .git/modules, which is exactly what the narrower set here is for.
Three conflicts, all in the macOS mandatory denies, plus one clean hunk that combined wrongly. macGetMandatoryDenyPatterns / macGetMandatoryDenyEntries: main's entry-typed signature and its dangerous-file and dangerous-directory blocks, with this branch's git blocks in place of main's two. The '**/.git' and '**/.git/modules/*' patterns go through main's cwd-anchored constructor, so the cwd is escaped into the regex instead of being read back as glob syntax, and every path the producers in mandatory-deny-paths.ts read off disk becomes a literal entry: the cwd's own git directory, the submodule git directories, the directories the walk could not see through, and a pointer's resolved gitdir and commondir. A submodule is named by whatever the sandboxed command left under .git/modules, and a name like a[b/c]d compiled as a glob is a character class matching neither it nor anything else. gitPointerFilter: main's pathFilter takes an entry, so its '**/.git' is anchored at the cwd for the same reason. generateWriteRules merged cleanly and correctly: main's grouped entry denies with this branch's pointer deny, create allow and trailing unlink deny around them. macos-literal-deny-brackets.test.ts merged cleanly but kept main's '**/.git/hooks/**' in its subtree assertion, which this branch replaced with '**/.git/hooks' (a deny regex covers the subtree, so the directory vnode is covered too). Expectation updated; what the case asserts, that the cwd stays literal in the pattern, is unchanged.
A submodule's name is its path and the sandboxed command chooses it, so `.git/modules/a[b/c]d` is an ordinary name for one; so is the directory a `.git` pointer file names, and the one its `commondir` names. Sniffed for glob characters, each compiles to a character class that matches neither the directory it was built from nor anything else, and the deny covers nothing. Profile cases, on any POSIX host: a submodule git directory and a directory the modules walk stops at are denied by subpath; no emitted regex carries a bracket that came off the filesystem; the patterns that have no path to enumerate - a nested repository's submodule, and the `.git` pointer filter - are anchored at the cwd with its own brackets escaped; a pointer file's target and its commondir's target are denied by subpath. Under real sandbox-exec: a hook planted in a bracketed submodule git directory is refused, a write in the directory the walk stopped at is refused, and a write elsewhere in that git directory still succeeds.
README.md conflicted where both sides rewrote the same part of the mandatory deny section: main (#514) inserted its "Pinned directories (Linux)" paragraphs between the "Note (Linux)" note and the search-depth note, and this branch had rewritten both of those notes. Resolved as the union, in main's order: this branch's rewritten notes, main's pin paragraphs between them, this branch's search-depth line last. src/sandbox/linux-sandbox-utils.ts merged cleanly and was checked hunk by hunk: main's changes (#503, #514, #536, #555) sit in generateFilesystemArgs below the mandatory scan, which main does not touch, and this branch's seenDenyWrite guard kept its place inside the absent-path branch, above the empty-directory bind. One assertion had to move with the merge: a test read "the git directory at the modules-walk depth bound is not denied whole" as the absence of a --ro-bind of that directory onto itself, which main's ancestor pins now spell for every ancestor of a deny. It asserts the position instead - every self-bind of that directory comes before the write root's own bind, which is what buries a pin and what a deny bind is emitted after.
The suite carried its own memoised copy of the bwrap namespace probe. The same probe is now a helper every other Linux suite imports, so use that one.
Denying a path that is not there means mounting something at it, and two of the paths these denies name are ones git reads back: `commondir` and `config.worktree`. The placeholder was /dev/null, which a bind mount serves with nodev, so git found a `commondir` it could not read and refused to run at all - "fatal: failed to read .../commondir: Permission denied" from every git command in the repository, not just the write the deny is for. An empty file is no better: git rejects a commondir it reads zero bytes from. Mount a placeholder git reads as no redirect instead: `.` for `commondir`, which is where git looks when a git directory has no commondir, and nothing for `config.worktree`, which reads as an empty config does. The deny is unchanged - the mount is read-only, so neither file can be written or replaced - and a real one is still bound from itself. A file already in place holding no bytes takes the placeholder too. That is what bubblewrap's own mount point for the absent case looks like until cleanup removes it, so without this a second wrap over the same repository would bind that empty file and break git again.
…-hooks-depth src/sandbox/linux-sandbox-utils.ts conflicted in the mandatory-deny producer. #535 split it in two: linuxGetCwdMandatoryDenyPaths(), exported so the violation monitor judges a refused write against the same definition the wrap applies, and linuxGetMandatoryDenyPaths(), which adds what its ripgrep scan finds. This branch had rewritten both halves in place. Resolved so the split keeps its point: the cwd half is this branch's version of it - the repository's hooks, config, commondir and config.worktree, its submodule git directories, and the pointer-file case - so the violation monitor sees all of them and not just hooks and config. The walk both halves need is now one function, gitDirTreeDenyPaths(), and the scan half keeps this branch's doc, its RipgrepConfig signature and its per-repository dedup, whose set starts seeded with the cwd's own git directory: the cwd half has walked that one already, and every file the scan lists beneath it leads back to it. Everything else merged cleanly and was checked hunk by hunk: #534's capability start-up check, #553's typed profile error and #535's monitor work sit elsewhere in the file, and the deny loop this branch adds to is untouched by all three.
…-hooks-depth Nothing conflicted. main's only commit since the last merge drops a duplicate normalizePathForSandbox import from src/sandbox/sandbox-manager.ts, a file this branch does not touch; the merge takes that deletion unchanged.
…-hooks-depth Two files conflicted. src/sandbox/linux-sandbox-utils.ts, in the absent-path branch of the deny loop. #524 rewrote it: one placeholder per destination, recorded by the index of its source so a later deny reaching the same destination can upgrade that source to the directory form, and one shared empty directory behind every directory placeholder. This branch had added a two-line guard there against the same collision, and a placeholder git can read for the leaf case. Resolved on #524's version: its dedup replaces the guard, and the leaf keeps this branch's placeholder - for a leaf whose basename is a file git reads back (commondir, config.worktree) the source is that placeholder rather than /dev/null, which git cannot read and which makes it refuse to run in the repository at all. #524 also added a branch above this one that takes an empty, write-bit-free file at a deny path for a mount point an earlier sandbox left behind and covers it with /dev/null. A commondir left by an earlier wrap is exactly that shape, and /dev/null is what it must not be covered with, so a path with a placeholder is left to the existing-path branch below, which binds the placeholder over an empty one. Without that gate two live arms fail, here and in test/sandbox/linux-ancestor-pin.test.ts: the second wrap over a repository reports "fatal: failed to read .../commondir". test/sandbox/macos-literal-deny-brackets.test.ts, where both sides appended a describe block at the end of the file. Both kept, in that order. Everything else merged cleanly and was read hunk by hunk. #506 touches this branch in three places: the empty-$HOME guard in the Linux allow loop, which sits above the deny loop; denyGlobRegex moving to sandbox-utils, which the macOS backend this branch also edits now imports; and the interior-spelling canonicalisation in normalizePathForSandbox, whose output is what this branch reads a deny path's basename from.
…rom the path The two files git reads back inside a git directory, commondir and config.worktree, were written out twice: once as deny paths and once as the stand-in each needs where it is absent, in a switch whose default arm handed a redirect file added later a silent /dev/null. Both now come off one table, which carries the rationale the five copies around it repeated, and each entry says whether it is denied even where the caller allows writes to the config. The stand-in is also chosen from the resolved deny path rather than from the entry the scan produced it with. Caller denies are applied first and win the dedup on that resolved path, so a denyWrite spelled ./.git/commondir, with a tilde, or with a trailing slash arrived with no stand-in and bound /dev/null - and git refuses to run in a repository whose commondir it cannot read, which is the failure this branch exists to prevent. Every spelling of one file shares a basename, and a file of that name outside a git directory loses nothing by being read the same way: the bind is read-only in both arms.
…ect file
Denying a path that is not there means mounting something at it, and bubblewrap
makes the mount point itself: an empty file, on the HOST. For the two files git
reads back inside a git directory that is a fault the whole machine sees. git
refuses to run at all in a repository whose commondir it cannot read, and reads
no bytes as exactly that, so from the moment a wrap is built until its cleanup,
every git command outside the sandbox in that repository - another terminal, an
editor, the embedder's own - failed with "fatal: failed to read .../commondir",
and a process killed before its cleanup left that state for good. The
placeholder this branch added fixed only what the sandbox reads.
The wrap now writes the mount point itself, before bubblewrap starts, holding
the placeholder, and tracks it: the host reads a redirect git accepts for as
long as the command runs, and the cleanup takes the file away while it still
holds exactly what was written there. A file left behind by a process that
could not clean up is recognised on the next wrap - its own bytes, or the empty
file an older release left, which is no one's since git refuses a commondir of
zero bytes - and is repaired and removed with the rest. An empty
config.worktree is legitimate and is claimed only with the shape bubblewrap
leaves (read-only, empty, one link). Nothing writes over a redirect git can
read.
What is bound over the mount point is a copy in a store, not the file. The
placeholders used to be minted one per deny per wrap into a fresh mkdtemp that
nothing ever removed, and nothing bound that directory read-only inside the
sandbox: with a TMPDIR under an allowed write path the sandboxed command could
rewrite the source in place - the bind exposes the inode - and choose what git
resolved its own git directory to. They now live in the masked-file store,
which is one file per distinct content for the life of the process, mode 0600,
and pinned read-only in every sandbox that mounts one, next to the two mount
sources that already are. The store is emptied by the forced cleanup, which
reset() and the process-exit handler call.
The placeholder is also built only once the bind is known to land, so one
dropped as hidden by a read-deny tmpfs leaves nothing behind, and the swap is
skipped where the file already holds the placeholder.
Neither the mount point nor the store file can always be written - a read-only
temporary directory, a read-only git directory. There is one decision for both,
logged once and fail-closed: /dev/null is the only deny left, and it costs the
repository every git command inside the sandbox and, through the mount point,
outside it as well, so the command is refused with
LinuxSandboxProfileError('deny_placeholder_unavailable') rather than run behind
a deny that breaks it. The one mode that is not a reason to refuse is the 0444
bubblewrap leaves on its own mount points, which is chmod'd first.
The Linux violation monitor judges a refused write against the same cwd deny paths a wrap applies, and this branch gave that producer a way to throw: a commondir past the size it reads, or a path whose bytes are not valid UTF-8, refuses the wrap rather than sandboxing with a deny list that may name the wrong directory. The monitor calls it once, from initialize(), with no catch - so standing in such a repository failed the whole session's start-up, not one command. It now asks through a wrapper that falls back to the cwd's plain deny paths - its dangerous files and directories, and either the .git pointer file itself or that git directory's own hooks, config and redirect files - and warns. What this decides is which refused write the monitor reports; every wrap in the repository still refuses its command outright until the metadata is readable.
…s paths The per-command ripgrep scan is where every nested repository below the working directory is found, so a scan that does not deliver one leaves that repository's hooks and config writable. Two arms of the catch still ran the command: - an error that is not a RipgrepError, which is the scan not running at all: the binary could not be spawned, or this process is out of descriptors. That is the missing dependency the start-up check refuses on, met later. An abort is the caller's own answer and travels as the AbortError it is. - a run that exited with an error status while naming no path under the working directory on stderr. An unreadable directory is denied whole in its place; nothing stands in for any other failure, so whatever the run did not reach stayed writable. Both refuse the wrap now, as deny_scan_failed on LinuxSandboxProfileError, which the timed-out case carries too instead of a bare Error. A failed scan is still sandboxed on where every line it printed names a path under the working directory that is then denied whole.
The scan fails for real on a directory the process cannot read, and the test box and CI run as root, where there is none: none of those paths were exercised. A fake rg does it anywhere - a partial listing with an unreadable directory named on stderr (the listed repository denied, the named directory denied whole, the wrap standing), a run that outlives a short timeout, one that exits with an error status naming no path, one that cannot be spawned, and an aborted signal that comes back as itself. test/utils/ripgrep.test.ts pins the carrying of a failed run's matches and diagnostics the same way. Where the uid allows it the real thing still does it under bubblewrap: one wrapped command takes a directory to mode 000 and the next is still refused the nested repository's hooks. README: the refusals a failed scan now makes, and where a missing ripgrep is caught instead.
…ctories here The per-command ripgrep scan is where every nested repository below the working directory is found, and a run that fails still lists most of the tree. Which of those runs could be carried on from was decided by parsing paths out of stderr: each line cut at the first ": ", the rest taken as a path under the working directory and denied whole. A directory's name is chosen by whoever created it and reaches stderr unescaped, so that parse was the name's to steer. A directory named "src: x" holding an unreadable "locked" gave "<cwd>/src" - a directory that was scanned and must not be denied - while the unreadable one was never denied and the wrap went ahead: the fail-open this scan exists to close. One named ".: x" aimed it at the working directory itself; a trailing space was trimmed off; a newline in a name split one line into two, and the second named no path, which refused every command in the tree. No path comes out of stderr any more. The run is classified by the "(os error N)" tokens anywhere in it, which a name can only add to: - every token ENOENT: entries that went away while rg walked. There is nothing at those paths to deny, and mounting one would plant a file on the host where the command deleted one. The listing still counts. - any EACCES: the wrapper finds the unreadable directories itself, with a bounded readdir of the working directory to the scan's own depth, symlinks not followed and node_modules skipped as the scan's globs skip it, and denies each whole. The walk spends what the scan left of its time budget; a walk that finds none refuses, since the run failed over something this cannot see. - anything else, or nothing at all: refused, with the raw stderr in the message. Also: node_modules is excluded as a directory as well as by its contents, so an unreadable one no longer fails the run over a directory the scan never wanted; the scan runs with --no-config, since RIPGREP_CONFIG_PATH routinely points inside a project at a file a sandboxed command can write and one --max-filesize line in it empties the scan with no error; and a run killed by something other than the timeout is reported as the signal it was. RipgrepConfig.timeoutMs joins the schema the type already had it in.
…d deny where a deny will do GitMetadataError escaped the wrap raw: no name, no code, not exported. It is raised by following a .git pointer or a commondir, and creating a new .git pointer inside an allowed write path is deliberately allowed, so one command could leave every later wrap in the tree failing with an untyped error - the command that would delete the file among them. It now carries a name and a code and is exported from the package root, which is the macOS profile generator's own refusal type; on Linux the wrap turns it into a LinuxSandboxProfileError carrying deny_git_metadata_unreadable, and both name the file, since lifting the refusal takes a command run outside the sandbox. A read error on a followed target's commondir - the file is there and cannot be read - propagated to the outer catch, which logged it and returned the deny paths gathered so far: the main repository whose hooks a linked worktree's commits run was then never denied at all. That refuses now. A target the walk classified as unreadable is skipped instead of read: it is already denied whole, and git, running as the same user, cannot read through it either. A directory named by a pointer or a commondir that is not a git directory no longer has its commondir followed either, which is the same rule its own deny paths already obeyed. Nested-repository detection needs a regular file directly inside .git. With allowGitConfig false the config deny keeps one there and it cannot be removed from inside the sandbox; with allowGitConfig true a command could delete them all and hide the repository from the next command's scan, so that case walks the working directory as well and denies the .git directories it finds. Smaller, in the same area: a stat of cwd/.git that fails for anything but absence denies it whole rather than skipping the enumeration, on both backends; the violation monitor's fallback tells an absent .git from a directory, so it no longer hands out four denies the wrapper never makes; gitDirTreeDenyPaths moves beside the producers both backends call; the 40-hop symlink bound is one constant; and the GitMetadata reads are exhaustive switches rather than a ternary with an undefined arm.
Denying a commondir or a config.worktree that is not there means writing a mount point for it, on the host, where every git command outside the sandbox reads it. Five things about that file were wrong. - It was written 0644, and the only recogniser of one left behind by a killed process requires no write bits, so an empty config.worktree left that way was never claimed and never removed. Written 0444 now, which is the mode bubblewrap gives its own. - A git directory this process cannot write - a root-owned vendored checkout, a read-only mount, one the command itself made unwritable - made the exclusive create fail and refused every command in the project. The git directory is bound read-only whole instead: it needs nothing written to the host, costs what was already unwritable, and closes the chmod route a command could otherwise take to create the file itself. deny_placeholder_unavailable is left for a host that genuinely cannot take the file. - The empty-file repair arm had no shape check, so a zero-byte commondir a host git was between creating and writing was chmod'd, overwritten and later unlinked. It is left exactly as found now, and only covered for the length of the command. - A mount point already holding the placeholder was bound from itself. Another process wrapping a command in the same repository claims the same file, and its cleanup unlinks it: the bind then has no source, or loses it mid-run. The store's copy is bound instead. - A wrap that threw after the files were written left them on the host with no sandbox running, because the catch only decremented the counter. It calls the cleanup, which decrements and, with nothing else running, takes the files away. The store write, which unlinks and rewrites on every call, is made once per placeholder per wrap; the first try around the exclusive create no longer covers it, so its own typed refusal is not re-wrapped or logged twice; and the two parallel tables of mount points are one map from a mount point to the bytes this wrapper wrote there.
…her wrap could not record What is kept in memory is kept by every clean-up but the one that releases everything. Two things followed that should not have: - The directory that mount points bind from stayed in memory after a manifest named it. A clean-up after one command kept it and dropped that manifest, so from then on only this process knew of it, and killed, left it for good. A published manifest now takes it out of memory like a mount point the plan made. - A plan that made a mount point marked it as its own though an earlier wrap, which could record it nowhere, relied on it. The manifest then took it over, and the clean-up after that command removed it under the earlier sandbox. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Since the places are made at the head of a plan, the call before the pins did one thing more: it settled where this wrap's manifest would go, so that a directory of the process's own was pinned in the same plan. No case held that, and the wrap is right without it. A manifest that went to a directory the pins do not cover has the wrap plan again, which a case does hold, and the next plan finds the directory settled. So the second call goes, and the `recording` parameter with it. A process that can write none of the shared places plans its first recording wrap twice. Every other wrap saves the second look at the five places: with twenty manifests there, `lstat` 66 to 57, `open` 28 to 24, `access` 6 to 3. The hook of the cases that act between the deny loop and the pins counted calls of `mkdirSync`. It is keyed on the one question a plan asks there. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Brought in by the commit before last. Once a manifest named the directory that mount points bind from, the process no longer kept it in memory. So with that manifest lost, to a cleaner of temp dirs say, no clean-up of the process removed the directory, and every later wrap went on binding from it, also after the temp dir had changed. It is this process's own: a clean-up with none of its commands outstanding takes it for removal, whatever the manifests say. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
…spelled out
What is no JSON is in doubt for half a second only, so a case that plants
it and is slow to wrap would see no refusal. `{}` is in doubt for an hour.
Two more captures of a refusal use the fixture, and a doc comment is back
on the function it describes.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
f203eb6 was wrong to cut that. It relied on the wrap planning again when its manifest went to a directory the pins do not cover, and that is asked of a manifest. Where none can be written, on a full disk say, there is none to ask it of: the command went out with the process's own directory, just made, neither bound nor pinned. With the temp dir in a write root the command could write a manifest there, and the host's next clean-up removed an empty file outside every write root on its word. It also had such a process plan twice after every forced clean-up, which removes that directory, and not once in its life as that message said. And a wrap given up between the two plans left the directory behind at exit. So it is settled before the pins as it was, and listed with the others. Without the second look at the shared places, which is what cost: with twenty manifests there, `lstat` 66 at fdce419, 58 now, `open` 28 and 24. The case is what was missing: neither shared name will do, and the wrap publishes once and binds the directory, with the manifest written and with the disk full, also after a forced clean-up. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
This reverts commit 8997f0d. It took the directory for removal at every clean-up with no command outstanding, for as long as the process remembered the name. So where the name outlived the directory, or a sandbox still running kept it, each later clean-up went on record and read everything, with nothing to remove, and other processes' wraps waited for it: ten records in five idle clean-ups, none before. It removed the directory from under a running sandbox whose manifest and record were both lost. And it made two cases pass by itself that hold the tracking from when the directory is made. What it mended is a case of a limit the README states: what a lost manifest named stays. For this directory less than that. The next wrap of the process that binds from it names it again, and it goes with that manifest. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
It asks three times whether what is no manifest is in doubt, and planted what is so for half a second. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
…be written Older than this round: reproduced at fdce419, and the two conditions it hangs on read the same in the head that was approved. A wrap settles where its manifest goes before the pins only when it has mount points of its own. One that relies on another sandbox's alone learns that after the pins, which are among what it relies on, and settles as it publishes. Written, the manifest has the wrap plan again. Not written, on a full disk say, there is no manifest to ask that of, and the command went out with the directory, just made, neither bound nor pinned: with the temp dir in a write root the command could write a manifest there, and the host's next clean-up removed an empty file outside every write root on its word. So it is closed where the directory is made, whichever way a wrap came there: a directory only this process knows, and with nothing in it, is removed again when the manifest it was made for cannot be written. One that holds an earlier manifest stays, and was settled before this plan began, so this plan has bound it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Each is bound where it really is, and last, on top of every deny. The plan makes sure at its head that each is a directory and no link, and asked where it really is only at the pins, the whole deny loop later. A name swapped for a link to the home directory in between had that bound last, on top of a read deny under it, and the command read what was denied. Since fcd46bc for a wrap with no mount point, since f203eb6 for every wrap: before those, the look came right before the pins. Only what the environment names can be swapped, and only by a process of the user's that has not got it bound, such as a sandbox of a process with another temp dir. Now it is asked at once. A name swapped later is bound by that name, and bubblewrap refuses to mount on a link. The hook of the cases that act before the pins hung on that question. It is keyed on the same question about the directory mount points bind from, so the one case whose first plan had none has one more deny. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
…it asks of a wrap Exported since 260e0dc, it was the one function here that makes something on the host without asking for the platform first. Its callers did that for it until then. Its doc comment says what a caller takes on, and the one that lists what a wrap binds points to it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
`src/` is byte for byte what it was there. That is the last head a review found free of regressions in the wrap path. Its nine findings were small, and each of my three rounds of answers to them brought in worse: - f203eb6 cut the second look at the manifest directories, and with it the settling of a directory of the process's own before the pins. On a full disk that directory went out neither bound nor pinned. - 8997f0d made every later clean-up a full pass in three states. Reverted in b936bc7 already. - 260e0dc settled again, without the look. A name swapped for a link during the deny loop was then followed, for every wrap. - 5bb12e9 removed a directory that could not be written, also one that commands already handed out bind, so that the next one made was in their reach. On a disk that takes no directory either, the process recorded nothing for the rest of its life. - 57a7678 resolved the directories at the head of the plan. One that cannot be resolved then counted as a guess against every placeholder the plan would have skipped, and the command could not start. All of it lies in some twenty lines, in states that are rare: no shared name can be used, the disk is full, or another process of the user's races the plan. CI was green on every one of these heads. What stays is tests. Junk that is in doubt for an hour and not for half a second, the refusal fixture, a hook that no longer counts calls, and two cases for what the second look does, which no case held: a directory of the process's own is bound by the plan that settles on it, written or not, and a link put at a shared name during the plan is not followed. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Its swap hangs on the wrap making the directory that mount points bind from, which a wrap does only with none left from an earlier case. In file order the case before it had cleaned up, under a filter or in a random order it failed. It now removes what is left first, and acts only on that one directory being made. Three clean-ups in `finally` could throw and replace the failure they followed. Two titles said of a mount point what is true of its manifest, one claimed the second look for every wrap, one still had the directory settled on as the wrap records, and a comment said more of read denies than holds. One of three rounds in a loop held nothing the others do not. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEZ8LUnnuZdEgiFocuvVDm Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
…ted-repo-hooks-depth Its 35 commits since cdfb8a8, up to db71e7c. Joined by hand: a plan reads the manifests before it makes anything and still leaves a file git reads back to be written after the manifest is published; where the manifest goes is settled before the pins, after the store's copies are made; the pass that is settled on is still the only one published. The README's long lines were merged word by word. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
ronleizrowice-ant
marked this pull request as draft
October 21, 2026 02:54
An absent `commondir` or `config.worktree` was denied by writing the file on the host and binding a copy of the same bytes, kept in a directory under the temp dir, over it. That directory was one more thing to hold: it was bound read-only only in the wraps that used it, never looked at again once made, never remade once removed, and left behind by a killed process. The file is now bound read-only onto itself and the directory is gone, with the change to the masked file store that served it. - The file is made through a descriptor of the git directory that followed no link, whose path must still be the one the plan looked at. Written by path, it went wherever a command already running had pointed the directory. - Nothing found at the name is written over. An empty `commondir` that a manifest names is no longer repaired: the git directory is denied whole for that wrap, and the file goes with the next clean-up. - A full disk or a spent quota counts as a directory that takes no file, so the git directory is denied whole and `rm` can still be run. - Every refusal of one plan is learned in that plan. Learned one at a time, three such directories used up the plans a wrap has. The tests of all this are one file now, about 900 lines shorter. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ
A repository's git directory keeps others: its submodules' under `modules`, its linked worktrees' under `worktrees`. Those keep others again, and not all of them were found. Each of these let a sandboxed command leave a hook, or a `commondir`, for a git run on the host afterwards: - The submodules of a linked worktree live under the WORKTREE's git directory, `.git/worktrees/<id>/modules/`, which nothing walked. The two walks now call each other, over one set of the directories visited, by real path, so that a link leading back up ends the branch. - A git directory reached through a `.git` file, and the one its `commondir` names, had their own entries denied and nothing of what they keep. A session in a linked worktree that may write the repository's git directory, which it needs in order to commit, could write that repository's submodules' hooks and its sibling worktrees' `commondir`. `gitFileDenies` now names the git directories it reached and both backends enumerate them like any other. - `worktrees` being a link was not held, as `modules` being one is. - A `modules` beneath a directory taken for a git directory was never taken for one itself, so a `HEAD` left in `vendor` took the denies off the submodule `vendor/modules`. It is looked at like any other directory. - A directory that file content names stopped being a git directory once `HEAD` and `objects` were renamed aside: one command moved them, the next wrote the hooks and moved them back. There is one test now for what a git directory is, and `config` and `hooks`, which are denied and so stay, count in it. - Where a submodule is degraded to one bind, what a worktree of its that is reached through a link is denied by was dropped with the rest. A marker counts only as the kind of entry git reads there, so the submodule `vendor/config/lib` no longer makes a git directory of `vendor`, which cost it its git writes. One case stays open and is in the README: a submodule with `refs`, `logs` or the like in the middle of its name, below a planted marker. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ
- What is still to be walked is a stack. It was an array taken from at the front and rebuilt at each link, which a value of half a million components made quadratic where the runtime copies, and it was spread into one call's arguments, which overflows. - A link whose target is not valid UTF-8 was read as a string that names another path. It is refused like a pointer's value of such bytes, with `deny_git_metadata_unreadable` on Linux: the deny used to land on the link itself, and bubblewrap then never started, with no code to go by. - The deny that stands in for an entry of `.git/modules` that cannot be looked behind is aimed where the chain lands, not where one hop folds to on paper. - A tree's deny paths are added to the plan one at a time, not as arguments. - macOS: one time budget for all that is read off disk, where every deny path had ten seconds of its own, and one call for the entries. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ
Lock-outs a sandboxed command could plant: - An entry linked at a write root that is a file, /dev/null among the default ones, refused every later command: the guard against denying the work tree whole listed it, and took ENOTDIR for a tree it could not read. A file holds no tree, and the deny stands. - A directory under `.git/worktrees` made unreadable refused every later command over the `commondir` in it. It is denied whole, as the README said. Fail-open or wrong: - A `.git` file that could not be read for any reason at all was denied alone. Only what the host's git cannot read either is passed over now. Out of descriptors, or a disk that fails, refuses. - `gitdir: ` followed by a NUL: git joins the empty path to the directory that holds the file and opens that. So does this now. - `ripgrep.timeoutMs` above what a timer takes was taken for one millisecond. The schema refuses it. The violation monitor's fallback, for a repository that cannot be enumerated, looked at the entries of `.git` again and could fail again. It is path arithmetic on the shape of `.git` now. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ
… size of - The level the passes start from was counted down to a step at a time. Where the entries a step took are not among what is counted (git directories the command may not write), each step took a few and gave nothing back, and every step goes over the whole deny list. A step that gives nothing back is the last now, as it is for the passes. The function sits with the rest of the arithmetic, where it can be tested by itself. - The warning that says what was degraded resolved each git directory once for every whole-directory deny of the plan. Once each will do. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ
What is denied, where, what it costs, which refusals a command can plant and what stays open. The reasoning behind each call is in the source, next to the code that makes it. Three statements were wrong and are corrected: `config.worktree` is read on top of `config`, not instead of it. A `denyWrite` entry does not lift `deny_unresolvable_git_entry`, which the refusal's own message said too. A directory made unreadable does refuse the command where it is the working directory, or holds it or a write root. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ
The comments of the new source say what each thing is for and what it guards against, in about 430 lines less. The compiled code is the same but for: - `gitFileDenyPaths`, which only tests called, is gone for `gitFileDenies().denyPaths`; - `repositorySubmodules` and `repositoryWorktrees`, near copies of each other, are one `degradableGitDirs`; - the two pushes of a hop in `gitDirDenies` are one. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ
…thing Repaired: - 'gives a walk made after the scan a budget of its own' needed two things to happen inside one millisecond and failed about once in a hundred runs. The clock now jumps as the stand-in scan ends. - The case for the clock being looked at inside one wide directory made fifty thousand links and went by the wall clock. Three hundred files and a clock that jumps after the first look. - The work tree guard's cases counted every `--ro-bind X X`, ancestor pins among them, so most passed with every whole-directory deny removed. They look above the bind that makes the working directory writable. - 'holds a modules directory that is a symlink' asserted on a directory the link does not lead to. Dropped: two live cases of seven hundred submodules each, one of which never degraded anything and the other repeats unit cases; the live half of a third; a parity case that called nothing of the library. Folded: twelve one-write cases into a table, one platform condition for the unit cases, one afterEach in place of seven try/finally. These suites take 85 s here, down from 132 s. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ
A review of b163513..e992a35 found that those fixes had brought in four regressions and a lock-out. Each is closed here, with a case that fails without its fix. - One set of visited directories served every walk, whatever a directory was visited as. A command could link a `modules` into a `worktrees`, to have it visited there first, as what is denied the least, and the walk that would have denied its submodules then skipped it. The other way round cost a linked worktree its `gitdir` deny. A visit as one thing no longer stands in for a visit as another, and a `modules` is walked from the top even where it was walked into from elsewhere. - With the `modules` of a git directory looked at like any other directory, a `HEAD` left in it made its entries git's own by name, and hid the nested submodule `lfs/client`. It is still taken for what it holds, and its entries are all names again. - A link whose target is not valid UTF-8 refused the command wherever it was met. Under a `.git/modules` or a `.git/worktrees`, where a command can make one, it costs what any link there does: the directory holding it, whole. A directory there whose NAME is no text was passed over in silence, and is answered the same way. - An empty `commondir` had the git directory denied whole at once. Making the file and writing it are two steps, so that was also what a second command wrapped at the same moment found, and its `git commit` then failed. It is looked at again on the next plan. What a killed wrap left is collected in between and made anew; only what stays empty has the directory denied whole. - What a git directory behind a `.git` file keeps is enumerated only where the command may write some of it. In a linked worktree with the default write roots every command walked the whole repository for nothing, and one stale sibling worktree refused them all. - A step of the collapse counts only what takes a mount, so it goes on past entries that give nothing back instead of giving up on stepping. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ
A review of the last round found that the set of visited directories, shared between repositories and keyed by what a directory was visited as, still let a planted link have a real directory visited first and hide it. One rule instead. A directory that is where its name says, with no link on the way from the root of the walk, is only stood in for by a walk that came to it so. A visit through a link is cut short after the first. That ends a link that leads back up, and keeps many links or many .git files into one directory from costing as many walks of it. - The entries of the modules of a git directory are names: only a file makes a git directory of it, so a nested submodule named hooks does not. - A link whose target is not valid UTF-8 refuses where something is behind it, and has the directory holding it denied whole, on both platforms, where nothing is. - A git directory and a linked worktree are denied under each name that leads to them, which on macOS is what holds the link. - macOS: the trees behind the working directory's .git file are enumerated wherever a write root is a pattern. - A repository none of whose git directories takes a mount gives nothing back, rather than taking one from what the next gives. - The cases that make a name that is not valid UTF-8 run on Linux only: the macOS file systems take no such name. Left out, and in the README: where a commondir with a `..` in it folds to on paper when it is read under a second name. Where the kernel lands is denied whichever name led there. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
…rectory A third review of the fixes found no link that hides anything any more, and these. A walk was cut short only after what a link leads to had been listed, and a git directory was recorded under each name that led to it, for macOS to hold the link by. So many links to one large directory cost their product, and on Linux every name counted as mounts that landed on the same paths and had other repositories degraded. The entry link is now a hop like the links after it: macOS holds it by its own name, Linux by the directory holding it, as before. What a directory is read as follows from where it was met, so the cut comes before anything is listed, and a second name is neither listed nor recorded. git opens what is in a git directory by name, which takes leave to search the directory and none to list it. - A linked worktree's that cannot be listed has its commondir followed all the same. One that cannot be searched is denied whole, and refuses nothing. - THREAT: `chmod 311` or `000` on the git directory the working directory's .git file names took every deny off the repository that keeps it, whose git directory such a session is commonly let write. Where the directory is, `<dir>/worktrees/<id>`, names that repository as well. - A directory that is denied whole because it cannot be looked into is named by where it really is: the name of a link holds the link, and is no deny of what it leads to. A link whose target is not valid UTF-8 refuses only where a directory is behind it. In the modules of a git directory `hooks` and `objects` are names whatever they are, a link too. In the README as a limit: a nested checkout made unlistable hides its .git file from the scan, and with it a repository outside the working directory. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
…t is A fourth review found three things the last commit brought in. - gitDirKind asked for leave to search the directory a .git file names. One that can be listed and not searched then counted as unreadable, its one deny was the directory whole, and for a write root that is dropped: a `chmod 600` took every deny off where the command used to be refused. Taken back. - The cut before the one listing of a directory with a name git keeps for itself came before the link was followed, so a second `refs` link to one directory was not held. Taken back. Many such links cost a listing each, as they did. - The directory a walk starts from and the `modules` of a git directory were read as one thing. The second may be a git directory itself, and a walk a command had start there first kept it from being recorded. Two things now. And these, which were there before. - THREAT: the work-tree guard took a whole deny that lands on a write root for the holder of a link wherever it could list the directory. One that can be listed and not searched stands in for a tree that cannot be read as well: refused, as one that cannot be listed is. - THREAT: the directory a .git file or a commondir names, where it cannot be read, was denied by the name the value ends in. Where that is a link it is a hop, which Linux drops and macOS matches against the link alone. Named by where it really is. - A link under `worktrees` that leads nowhere for the kernel can fold on paper onto a directory that is there: read once, like one that resolves. - A link whose target is not valid UTF-8 refuses nothing where it is a loop or leads below a file, under Bun as under Node. - One quadratic filter outside every time budget. In the README as limits: on macOS a whole deny that lands on a write root is dropped whether or not the directory can be looked into, and a linked entry of a git directory that is itself reached through a link is not named as a removal names it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
A review of the last commit alone found no regression, and one variant left open in each of two of its fixes. - THREAT: where a directory ABOVE what a link leads to is shut, the deepest directory that could be reached was looked for along the path as spelled, which climbs to the directory holding the link. Nothing behind the link was denied, and a command that had done the `chmod` undid it in the next wrap and wrote the hooks. Looked for along the path the kernel walks now, for what a .git file or a commondir names and for what a walk cannot list. - A link whose target is not valid UTF-8 and is a name no file can have refused every later command. It is nothing, like a loop. The README said of every linked worktree's git directory what holds for one reached from the repository that keeps it, and did not list the refusal for one a .git file names that can be listed and not searched. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
macOS refuses realpath on a directory without leave to read it, and the test directory is a real path already. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WzCURdLN6GNmf5ogwTo1vZ Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Collaborator
Author
|
The macOS failures on Generated by Claude Code |
This branch has not been 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
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.
Human: @ronleizrowice-ant
Requested by Ron · project thread
The mandatory write denies stop a sandboxed command from leaving behind something the host's git then runs: a hook, or a
core.fsmonitorline in a config. With the default configuration several ordinary layouts slipped through (nested repositories below the scan's depth, submodule git directories, linked worktrees,.gitpointer files). This closes them and makes discovery fail closed.What changes
hooks/,config,config.worktreeandcommondirare denied in every git directory that is found: the working directory's, nested repositories (Linux), every submodule under amodules, every linked worktree under aworktrees, and what each of those keeps in turn..gitpointer file is read-only. The git directory it names, the one that directory'scommondirnames, and what they keep are covered where the command may write any of it.EACCESorENOENT.commondirorconfig.worktreegets a mount point git can read, on linux: a placeholder is removed only when no running sandbox relies on it #584's manifests.The README sections "Which git directories are covered", "Refusals a wrapped command can plant" and the known limits beside them are the reference.
Size
Beyond #584 (
db71e7c..cac279c): source and README +3,882 −261, tests +12,322 −1,274. Most of the source is three files:mandatory-deny-paths.ts(what is denied, shared by both platforms),linux-deny-collapse.ts(the degrade steps), and the parts oflinux-sandbox-utils.tsthat scan, guard and mount.How it was reviewed
The whole diff beyond #584 was reviewed in three ranges at
b163513, then fixed, cut, and reviewed again after every round of fixes. Each pass read the source and attacked it with real git and bubblewrap and, from the second on, a differential fuzzer againstb163513.db71e7c..b1635135cf3747..e992a35(8 steps; README halved, 430 comment lines cut, tests repaired and cut)b163513..e992a352e7e8e7b163513..2e7e8e748e04eb(the walk redesigned)b163513..48e04ebab26e00b163513..ab26e00f3cdda2ab26e00..f3cdda2cac279cSaid plainly: passes 1 to 4 each found that the fixes for the pass before had broken something (a deny lost, a new refusal, a cost a command can multiply), next to holes that were there from the start. Pass 5 is the first that found no regression.
cac279citself has had the checks below and no pass of its own.The walk, since it was what kept breaking
One rule (
isCutShort): a directory that is where its name says, with no link on the way from the root of the walk, is only ever stood in for by a walk that came to it the same way. A visit through a link is cut short after the first. What a directory is read as follows from where it was met, so the cut comes before anything is listed. That ends links that lead back up, keeps many links or many.gitfiles into one directory from costing as many walks, and a link planted to have a directory visited first under another name takes nothing from the walk that comes to it from where it is.Known limits
All are in the README. None is a step back from
main, which follows no pointer and enumerates no submodule or worktree.refs,lfs,info,logs, ...) is left at one listing..gititself being a symlink is neither held nor warned about.chmod) hides its.gitfile from the scan, and with it a repository outside the working directory that the command may write.commondirwith a..in it folds to on paper, read under a second name, is not denied. Where the kernel lands is..gitfile are enumerated. A profile is bounded byARG_MAX.Two questions of scope
About half of all findings sat in two mechanisms. Cutting either makes the diff smaller and gives something up:
linux-deny-collapse.ts, about 400 lines). Without them a superproject past about 590 submodule git directories is refused withtoo_many_argumentsrather than degraded.GitChainHop). Without it only the two ends of a chain are denied, and a command can retarget a link between them.Testing
cac279c: 4,752 pass. The 84 that fail are by name the 84 that fail on linux: a placeholder is removed only when no running sandbox relies on it #584'sdb71e7con the same machine (no network, nosocat, no seccomp helper there). The sandbox suites also pass on bubblewrap 0.4.1.5de9ab5after it changes one line of one test, which macOS failed sinceab26e00: it calledrealpathon a directory it had made unreadable.chmodlimit above.chmod, planted markers, planted.gitandcommondirfiles), run after every round, 12,600 trees on the last commit and about 25,000 in pass 5: no new refusal beyond the listed ones, and no real path left undenied on Linux outside the limits above..gitfiles by submodules, 300 levels of nesting): linear, at or below whatb163513takes.Claude session #01WzCURd
Generated by Claude Code