Repository navigation
Draft, not for merging: every ready fix merged together - #624
Draft
ronleizrowice-ant wants to merge 204 commits into
Draft
ronleizrowice-ant wants to merge 204 commits into
ronleizrowice-ant wants to merge 204 commits into
Conversation
The five cases under "adjacent aliasing behaviors" in allow-read.test.ts called a local record() helper that console.log()s a line. None of them held an expect, so they spawned sandbox-exec on every macOS leg and could not fail. Two of them cover read-deny bypasses: if a build started letting a hard link or a clone reach a denied file, the log line would change from "not readable" to "READABLE" and CI would stay green. Each case now asserts the direction this build shows: the link and the clone are never created and the secret never appears, the case-folded and firmlink spellings do not reach the file, and the standalone (deny file-link) probe is denied by name rather than by a profile that failed to compile. The two cases that depend on the machine are decided while the block is collected and skipped where they cannot run, rather than returning from the middle of the body.
… reference matcher globToRegex is the one place a glob deny becomes a pattern on all three backends, and its test surface was four cases covering *, **/, ? and a trailing **. Nothing exercised the documented [abc] form or the escape for a bracket that opens no set. Added cases for a digit and a letter range, for the two spellings used for negation elsewhere (both land in the set as members, so such a pattern matches fewer names than its author meant), and for an unclosed [ and a stray ]. Added a property that checks the compiled regex against a hand-written backtracking matcher over generated patterns and paths, so the two implementations check each other on inputs nobody picked. The property found that a path component spelled like one of the internal placeholders (__GLOBSTAR__, __GLOBSTAR_SLASH__) is restored as a wildcard. That case is marked failing rather than fixed here, so it reds the suite if it is ever passed.
… reached The idempotence property over expandWindowsFsPaths says it catches normalize-divergence between the literal and the glob branch, but its corpus was five files named f0.txt..f4.txt under a mkdtemp directory and the generator only ever selects members of it. Neither spelling can hold * or ?, so the branch key was false on every sample and expandGlobPattern was never called across the whole run. The corpus now also holds f*.txt and f?.txt, so a sample can select a pattern alongside literals and the second pass re-normalises concrete results the first pass expanded.
Every .req in the vendored suite lists its headers lowercase and already sorted, and the one header the test adds itself lands in its sorted position too. The signer lowercases and sorts the set it is handed, so neither step was ever exercised: deleting both sort calls leaves all 22 vectors passing. Each vector now signs a second time with the same set reversed and upper-cased, and must produce the same canonical request, string to sign and Authorization. The fixture count is pinned exactly instead of a lower bound, with a note naming the upstream vectors for header order and key case that are not vendored.
Five tests in the "Sandbox Integration" block opened with `if (checkLinuxDependencies().errors.length > 0) return`, and one of them also returned when the apply-seccomp binary did not resolve. A machine without bwrap or socat ran the whole block green, having wrapped nothing, which is the failure the block exists to catch. The suite's own first test wrapped its real assertions in `if (depCheck.errors.length === 0)`, so it passed whether or not the condition its name states held. The dependency check is now a beforeAll that asserts, the first test asserts unconditionally, and the two early returns for a missing binary became assertions on the architectures that have one.
src/index.ts is what package.json points main and types at, and nothing checked what it exports. Three test files import through the barrel, for two of the names; everything else in the test tree is imported from its implementation module, so dropping an export breaks embedders while the suite stays green, and tsc building declarations cannot notice a removal. Added a test that imports the entry point and compares the sorted runtime export names against a checked-in list, so a removal has to edit the list in the same commit. Types are erased before the test can see them; the list says so, since catching a removed type needs a declaration snapshot the package does not build.
The comment above the fixture writes said "ALL dangerous files from DANGEROUS_FILES", but the nine names were written out by hand and asserted by nine hand-written blocks, and the four directories the same way. The file never imported either constant, so a name added to DANGEROUS_FILES or getDangerousDirectories() got no test and a backend that stopped denying one of them was only caught for the names somebody had copied. Both loops now read the constants the two backends read. Each entry keeps the block it had; a new entry brings its own.
…ation-and-placeholders
…stays literal globToRegex parked `**` under the literal strings __GLOBSTAR__ and __GLOBSTAR_SLASH__ while it rewrote `*` and `?`, then restored them by name, so a path component actually spelled that way came back as a wildcard: `/tmp/__GLOBSTAR__/x` compiled to `^/tmp/.*/x$`, and `/tmp/__GLOBSTAR_SLASH__x` no longer matched itself. The pattern is now read once, left to right, and the regex is emitted as it goes, so nothing is parked under a marker and no text in a pattern can be taken for one. What it emits is unchanged for every other pattern, bar two corners of the bracket syntax that the rewriting left as it found them: a `[` that opens no set is escaped wherever it stands rather than only the first one, where two of them used to leave a regex that would not compile, and a set with no members (`[]`) is not a set, so its brackets are characters of the path. The walk over a read-deny glob compiles the pattern piece by piece and fenced off the two shapes it could not read back from what the rewriting made of them: one spelling a placeholder, and one with a second `[` that nothing closes. Neither is a shape of its own any more, so both fences are gone and such a pattern is followed a name at a time like the rest.
The doc comment promised gitignore-style matching, but neither spelling of a negated set was one: `[^0-9]` had its `^` escaped into the set and `[!0-9]` kept the `!` as a member, so both compiled to a positive set holding one character more than its author wrote. A deny written that way denied the opposite of what it said. A `!` or `^` first inside a set now negates it, and a negated set never matches a path separator, which is one character of one name the way the rest of the syntax reads it. `/data/[!a]*` compiles to `^/data/[^/a][^/]*$` where it used to compile to `^/data/[!a][^/]*$`. A `-` first among the members of a negated set is escaped, so that it stays that character rather than reading as a range with the separator the set already excludes.
…er has The glob syntax the README lists did not carry negation, which both spellings of a bracket set now do. On macOS a deny written that way is read by the profile's own regex engine as any character but `/` where the set's members sit next to `/` in the character order, so it covers the characters it lists as well: measured with `sandbox-exec`, and named where the syntax is. The Linux walk reads a pattern with a second unclosed `[` the way it reads the rest, so it is no longer among the patterns matched against whole paths with every directory listed.
…or on one path.dirname keeps the separator of a root it returns, so the base of a pattern whose first wildcard sits right below one is 'C:/', or '//server/share/' on a share. Split into path components to seed the walk, either ends in an empty name no position can consume: the automaton starts nowhere, the pattern matches nothing, and the entry is dropped with no error and no warning. On Windows that is every denyRead, denyWrite, allowRead and allowWrite glob of the shape C:\Users*\**\.env. A base now never carries a separator, and one that is a filesystem root -- '/', a drive root, the root of a UNC share -- is refused and warned about the way the POSIX root already was, in the walk and in the manager's glob-pattern warnings, through one predicate so that the two cannot drift.
…ough a link A pattern that cannot be read one path component at a time is matched against whole paths, and no directory is listed through a symlink under it. A link leading out of the walk's tree was dropped: what the pattern matches in there is found under no other name, so the deny lost it. Such a link is now recorded as a place the walk could not enumerate, which the read-deny expansion denies whole and binds nothing back beneath. A link within the tree still loses nothing, since every directory there is listed under its own name. The link the walk follows is now named where it is followed, which is the one place that knows it: what is found through it is reported where it really lives, so the warning about a mount outside the pattern's base cannot name it.
The positions a directory had been listed for were marked before the listing and never rolled back, so a listing that failed answered for every later name that led there: a transient EMFILE, or a real path too long to name, dropped every match beneath the directory for the rest of the walk. A failure is uncached again, with one record per real directory, so the directory is named once in `unlisted` however many names fail on it, and a retry costs one listing per name that leads to it -- each of them an entry of a directory that is itself listed once per position. A filesystem call also no longer answers from a second name for anything but a path too long to be one. Every other errno belongs to the object rather than to the name, and the shorter name crosses links a sandboxed command owns: EACCES on the real path followed by ENOENT on the short one read as "nothing is there", which is the fail-open the unlistable and uninspectable branches exist to prevent.
Both of globPositions' regular expressions are compiled inside the try that guards the automaton, and a pattern that compiles into no regular expression at all is raised under its own name rather than thrown from wherever the sandbox happened to be setting up. A pattern the automaton cannot be built for -- including one so long that building it would overflow the stack, which a bound on the piece count now refuses -- falls back to matching whole real paths with a warning that says so, rather than quietly covering less: on a deny path that fallback also stops the walk descending symlinked directories. Beside that: the directory-form automaton is built only for the caller that asks for it, the positions are no longer sorted on every step for an order nothing reads, and the state written back into the automaton is held rather than cast.
… the separator Nothing in the tree held the walk's automaton and globToRegex together, though they are the two halves of one rule and are edited by different hands. A seeded, bounded property test compares them over generated trees and patterns: what the walk returns must be exactly what the regular expression matches beneath the pattern's base. It takes about 30ms. Also: a bracket range that holds the separator ([+-9]) joins the set that does ([s/]), and the fixture that builds a path near PATH_MAX no longer spins for ever when the temporary directory is exactly the length that leaves it nothing to add.
A link back up the tree is not descended, so it denies what it reaches only when the link is itself a match; the README said it always did. It now also says what a pattern that cannot be split does with a link that leads out of the starting directory.
`npm run build` succeeding says the sources compile, not that an installed dist/index.js loads: a specifier the build does not emit, a file left out of `files`, or an export dropped from src/index.ts reaches npm either way, and the first consumer to import the package root finds it. The release workflow packs the tarball and checks its contents; on Windows it also resolves and runs the prebuilt srt-win.exe from an installed package. Nothing imported the package root. A new release job packs, installs the tarball into an empty directory and imports `@anthropic-ai/sandbox-runtime` from there on node 20, the floor `engines` claims, failing when SandboxManager, SandboxViolationStore, SandboxRuntimeConfigSchema or getDefaultWritePaths is missing. `publish` waits for it. The same body runs on the Linux x86-64 test leg, where dist/ is already built, so a change that breaks the package root fails on the pull request rather than at the release.
The README said the network lists take effect and filesystem rules do not. It did not say what a line has to contain, which other keys are live, or what Windows does with one. A line is validated against the settings-file schema, so it is a whole config and not a patch: `network` and `filesystem` are required, and a key it omits goes back to its default. `credentials.sigv4` is live for the same reason the network lists are - the signing hook reads it per request - and the resolved-address check is rebuilt from the line. Everything else waits for the next run: the filesystem rules and the credential masks were compiled when the command was wrapped, and the running proxy servers hold the parent proxy they started with. On Windows a changed file-access set is not applied at all, and is reported only under SRT_DEBUG. Also recorded: the reader starts after the sandbox is up, so a line written before then waits rather than being lost; a blank line is ignored; and on Windows the command is handed the three standard streams alone, so the control descriptor is not among its descriptors.
The schema has sixteen top-level keys. "Other Configuration" listed five, and network, filesystem, mandatoryDenySearchDepth and the Windows block have sections of their own, which left allowPty, bwrapPath, socatPath, seccomp, ripgrep, git and credentials with no entry anywhere - three of them with no mention in the README at all. Each gets a line, from what the code does with it: the seatbelt rules allowPty adds, that an explicit bwrapPath or socatPath is a directive rather than a hint (a path that is not executable is an error, with no PATH lookup behind it), what seccomp.argv0 asks of the caller, that ripgrep.args go before srt's own, that git.safeDirectories grants no write, and the two modes a credentials entry has along with the TLS termination masking requires.
One file can be named more than once: in filesystem.denyRead under two
spellings, in filesystem.denyRead and again in credentials.files, or twice
in credentials.files. Every spelling resolves to one place, and the read-deny
loop skipped an entry only where a denyRead tmpfs already hid it, which a
file mask is not — so each spelling emitted its own file mount at that one
destination, and the credential masks added theirs on top.
bubblewrap before 0.5.0 refuses to start on the second of them: its
ensure_file() returns early only for a regular file and creates a file over
anything else, so the mount lands on the character device the first
/dev/null mask left and creat() hits the read-only mount that mask just
made ("Can't create file at <dest>: Permission denied").
The credential masks are now collected first, keyed by where they land, so
two spellings of one file keep one fake: the last, which is the one
bubblewrap left in force. The read-deny loop skips a file whose destination
a mask already covers or that an earlier entry already masked. The mount
that used to win is the mount still emitted.
…ound Two things need 0.5.0, both of them changes to how bubblewrap prepares the mount point for a file bind. Before it, ensure_file() takes only a regular file for one and creates a file over anything else, so a denyRead entry or credential mask naming a fifo, socket or device node blocks or fails instead of binding over it; and it creates that file mode 0666 rather than 0444, so the recovery that keys off a read-only leftover does not recognise a mount point an interrupted sandbox left behind. checkDependencies() probes bwrap --version once per binary and reports both, naming the version found. A warning, not a refusal: every mount plan this library builds starts on 0.4.0 and later.
…tions A file mask is a --ro-bind of something other than the destination itself. Two of them at one destination with nothing between that mounts at or above it is a mount whose only effect is to replace the one before it, and the shape bubblewrap before 0.5.0 refuses to start on. The corpus spells one file through the routes a caller and the credentials block reach it by; a mask re-applied above a write deny that re-exposed it has that deny in between and is not one of these. Also covers the version warning: a stub bwrap that reports 0.4.1 draws one naming that version, a newer one draws none, and one that will not answer --version draws none either.
RHEL 8 and 9 ship 0.4.0 and 0.4.1, the oldest bubblewrap in current distribution use, and one of those refuses to start on a plan that mounts twice at one destination. Built with autotools, which that release predates meson for, and pinned by commit like the 0.12.0 job. stale-mount-point is the one mount-plan suite left out: it asserts the read-only mode bubblewrap gives a mount point it creates, which it does only from 0.5.0 on.
Every mount plan starts on 0.4.0 and later; a read deny or credential mask on a path that is not a regular file, and the recovery of a mount point an interrupted sandbox left behind, need 0.5.0.
bwrap applies mounts in order, so the second of two masks at one destination was the one in force before they were collapsed into one. Nothing else says that is the one kept.
…ation-and-placeholders Conflicts, and how each was resolved: - test/sandbox/glob-expand.test.ts: main carries the same nine tests, from the squash of #570, which this branch carried as commits of its own, and this branch then flipped two of them. Ours whole: the file as it stood before this branch's own change is byte for byte main's, so taking ours keeps one copy of each test, with the placeholder case a passing `it` and the bracket-set case the negation tests. Nothing else conflicted. The rest of #570 is identical on both sides, and main's own changes to src/sandbox/sandbox-utils.ts and README.md sit far from the compiler and the glob syntax this branch edits, so both merged cleanly and were read again afterwards: the merged tree differs from main by this branch's three files alone, hunk for hunk as before the merge.
…rnel say so A deny on a path that does not exist needs a mount point on the host, and unlinking one while a sandbox is still bound over it detaches that mount inside the sandbox: the denied path can then be created, and what is written there lands on the host. So a mount point may only be removed when no running sandbox relies on it. Nothing a process counts for itself can decide that. A second srt process sees another's live mount point as a leftover and takes it away when it exits; a caller that crashes, cleans up early, twice, or never, each gets a different answer. Ask the kernel instead. Each wrap writes a manifest naming the mount points it relies on into a per-user runtime directory and points bubblewrap's --lock-file at it: bubblewrap's sandbox init opens that path and holds an fcntl read lock on it for exactly the sandbox's lifetime, which the kernel drops however the sandbox ends, SIGKILL included. /proc/locks then answers, for any process and at any moment, whether a sandbox that named a mount point is still running. The lock belongs to the sandbox's init process, not to the command: bubblewrap opens the manifest with O_CLOEXEC, so the descriptor does not survive the exec, and a POSIX lock is only dropped by the process that took it. Every error reads as "still in use": an unreadable /proc/locks, a directory that cannot be listed, a manifest that cannot be read while a sandbox holds its lock, and a manifest younger than the moment bubblewrap takes to reach that lock.
wrapCommandWithSandboxLinux() named every mount point it made in a set in
this process's memory, and cleanupBwrapMountPoints() counted the wraps it had
been told about and emptied that set when the count reached zero. The count
only ever described this process, so a second srt process took away a live
sandbox's mount points, and a caller that crashed, cleaned up early, twice or
never broke the count itself.
Each wrap now names its mount points in a manifest and passes
--ro-bind <dir> <dir> --lock-file <manifest> to bubblewrap, and the cleanup
is a garbage collect against those manifests: it is safe from any process, at
any time, any number of times, and it also takes away what other srt
processes have finished with. The counter and the exactly-once contract are
gone; cleanupBwrapMountPoints() keeps its signature, and `force` no longer
changes what is removed.
The manifest directory is bound read-only after every allow and deny bind, so
a sandboxed command can neither delete nor rewrite a manifest, and so
bubblewrap can open one whatever the profile has mounted over its directory.
A command wrapped but not started when a cleanup runs no longer keeps its
mount points: the cleanup releases them, and bubblewrap then refuses to start
that command ("Unable to open lock file") rather than running it with a mount
point another call has taken away.
The new suite drives the cases a count in one process's memory could not answer: two concurrent processes, where the second's cleanup used to take the first's live mount point away and the survivor could then create the denied path; a cleanup that comes early and twice while the sandbox runs; a process killed before it can clean up, whose manifest and mount point the next cleanup collects; a sandboxed command that tries to delete, plant and rewrite manifests; twelve wrap-and-run rounds against a process collecting in a tight loop; a command wrapped before the cleanup that released it; and a wrap that threw letting go of the mount points it had named. Five of the seven fail on the previous behaviour. Two tests in stale-mount-point.test.ts described the counter: one expected a mount point to wait for a second wrap that had produced no running sandbox, which the kernel now answers instead, and one collected a killed process's mount point before the grace that covers a sandbox still starting.
No conflicts.
…urce The import of the packed package is overtaken: verify-package installs the tarball that publish uploads and imports its root, on every pull request and release. Both workflows are main's again. Five statements of the --control-fd section were false or too wide, and one behaviour is new since the wrap walks in turns: a line that is waiting when srt starts to read may be the config the command is wrapped with.
A read pattern with a wildcard in its first path component has no literal directory to start a walk from, so Linux skips it and it hides nothing. getLinuxGlobPatternWarnings() names such denyRead and allowRead entries. The path of a `mode: "deny"` credential file joins the read denies and is skipped the same way, and was not named. Nothing that is enforced changes.
…throw The write lists hold entries now, so both call sites go through writeRootsOf(): read as strings, a root marked literal was missed, and that type-checked. The check only advises. A malformed entry, or a working directory that is gone, made it throw out of checkDependencies() and initialize(), where main returns. Whatever goes wrong in it now counts as no warning: whoever enforces a config says what is wrong with it.
The walk was rewritten under this branch. What is left of it on today's: - a drive root, and the root of a share, is a base like any other; - a directory that is not there under one name is asked for again under its others. A failure is kept for the patterns that follow only in a map the caller shares between them, so a walk by itself still tries each name; - the shorter name is tried when the real path is too long to name, and for no other error; - a pattern that is no regular expression says which pattern it is. The cap of 1024 pieces is gone. main splits such patterns, and capped they were matched against real paths only, which lost a match behind a link. Three refactors that changed no behaviour are gone too.
…that fails reset() is what the 'exit' handler runs, and it is async: at 'exit' it gets as far as its first await. A process that ends with process.exit() and has not awaited reset() left in the temp directory the two directories of a generated CA, one of them with its private key, the mux proxy's socket, the masked-file store and, now and then, a Linux bridge socket. The handler now removes them synchronously, after reset() has started. Only paths this process made and still holds: no listing of the temp directory, no path out of the configuration. A CA pair the configuration names is left alone, as before. An initialize() that rejects after it has made a CA removes it there: a step can fail before the clean-up is registered.
…nder a running one The clean-up after a command defers the removal of its mount points while other sandboxes are counted as active. - A wrap that was aborted or threw gave its count back and removed nothing, so what an earlier command had deferred to it stayed on the host. - A forced clean-up zeroes the count under a wrap still in flight. Failing, that wrap took the count of a sandbox started since; succeeding, it went uncounted and its caller's clean-up took it. Either way the mount points of a running sandbox were removed, and its denies with them. - The removal went in insertion order, so a directory mount point that another sandbox had been wrapped under was met while it was not empty, skipped, and forgotten. It goes deepest first.
Some file systems list names without types. Every is…() of such an entry answers false, so the walk neither went into it nor recorded it, and a deny pattern missed all it matches beneath, silently. An entry listed with a type costs no call. One without is asked for: lstat, then stat and realpath. One that nothing can be learned about is taken for a directory whose listing has failed, which the walk already denies whole.
Such a file, in a writable directory, was taken for a mount point a killed sandbox had left behind: covered with /dev/null, tracked, and unlinked by the next clean-up. Its look proves nothing. It can be the caller's own file, or the mount point of a sandbox another process is still running, whose deny goes with it. The guess is taken out. An existing file at a write-deny path is bound read-only onto itself like any other and is never removed. Given up: after a hard kill the empty mount points stay on the host until someone removes them, and a wrap that finds another process's mount point fails to start if its owner removes it first.
Only an answer was kept, so a probe that timed out, failed or printed no version was repeated at every ask: three times in one start-up, each blocking the event loop for up to the 5 s limit. No answer is kept like an answer, under the same key. The version decides one advisory warning and nothing else, so a kept failure can only leave that warning out.
It said to reduce what the configuration expands to. The arguments can be the mandatory denies', which follow from what the working directory holds and are in no list of the configuration: each git repository one level down costs twelve. Then nobody could follow the advice. Both refusals now state the two shares and what lowers each. Only the text changes: not the cap, what is protected, the arguments, or the codes.
Its side of both blocks in the manager. Three of its cases expect the dependency check to pass over a policy it cannot read. Here, on Linux, the check looks for the host's helpers outside the write paths before it comes to the warning, and stops there, so the cases say so by platform.
README only now: it gave up its step that imports the packed package, so that step leaves the workflow here too.
No conflict, and nothing else here touches that list.
No conflict. Its suite counts what the library leaves in the temp directory; here the directory of the mount-point manifests is there to outlive the process, so the count leaves it out.
… the scan Wraps walk in turns, so several interleave on the one thread, and each keeps all it has listed until it ends: peak memory, and the time the event loop is held, grew with the wraps in flight. Parallel walks gain nothing there. They now take their walks first come, first served. An attempt waits before it reads anything, so it goes by what it finds afterwards. The place is given up on every way out, and a waiter whose signal is aborted leaves at once.
One block, in the README's sentence on bubblewrap versions: this branch's sentence, with the new clause on the probe. The probe for --disable-userns decides an argument, so it goes on keeping answers only.
Nothing changes here. The manifests had already taken the guess out of this branch: only a manifest makes a path a mount point, and what none names is the caller's own. So this branch keeps its side of all fourteen blocks, and its own suite.
…anup Joined with the manifests. Here a running sandbox answers for its own mount points and they already go deepest first, so what the count protects is the manifest and the --args profile of a command that is wrapped and has not started. Taken over: forced cleanups are counted, a wrap handed out after one is put back in the count, and a wrap that throws gives its place back through the cleanup unless one came between. Three of its cases assert a removal that is deferred, which it is not here; they now assert what is: the manifest of a command that never started. Two cases are new, for a command wrapped since a forced cleanup.
Composed with the manifests: the plan's return value, which here also holds the manifest and what stays on the command line, gains the mandatory denies' share, and the renderer takes it beside the started record. In the README the new sentence on the limit, with this branch's list of codes.
… head The limit of 1024 pieces goes: past it a pattern was no longer listed through symlinks, so a deny could be lost. Three refactors without effect are taken back. The listing is written as upstream has it now, with the budget's charge kept between the step and the listing.
Joined with the walk's other two changes here. A position is still marked after a listing that succeeded, and the budget's charge stays between the step and the listing. A failed listing is not kept in a walk's own map, so that a second name gets its try; what could not be learnt of an entry is kept in either map, since nothing else says it may be a directory. The comment that the two made untrue together now says both.
Joined with the budget. The line that empties the wrap's listings before the scan named nothing here, where the budget's expander made that map: a type error, with no conflict marker. The wrap now makes the map, hands it to the expander and empties it. The budget's clock starts after the wait in line. One of its cases compares two wrapped commands, which differ here in the name of the manifest; the helper that takes it out moves to module scope.
FsReadRestrictionConfig carries credentialDenyOnly, the denyOnly entries that come from credentials.files, and ownAllowWithinDeny, the files the library adds to allowWithinDeny itself. A credential deny has the library's own files as its only exceptions. denyRead and allowRead rank as documented. On macOS every read deny is also emitted at the canonical location of its path.
The short name was asked after ENAMETOOLONG only. That masked less than main: after ENOENT on a listing, after any failure of a link's own stat or realpath, and for what a link leads out of a directory that is denied whole. It is asked after any failure again, as on main. A real path that failed with anything but an absence or ENAMETOOLONG still has the directory denied whole, whatever the short name says; what that lists is walked as well. Of two failures the one that is not an absence counts. The base of a pattern is main's code with the posix dirname: stripping a separator lost patterns with a doubled one before the first wildcard. A pattern that does not compile is a SyntaxError again, with its cause.
…ypings With @types/node 22 and @types/bun 1.4.2 together, connect() of node:tls is any, so sixteen callback parameters in five new test files were implicitly any. And FileSink.write() may return a promise there, which openFdStreams() left floating: it is chained before flush(), so that its failure reaches the stream's callback.
Two blocks in the manager: resolveReadDenies() takes this branch's expander where main has an inline closure, so both lists share one budget and one set of listings, and the wrap's map is still handed in and emptied.
…added Joined with the walk's other changes here: the budget's function and its charge between the step and the listing stay, and the entries of a listing go through withType inside the new retry, under the name the listing was made by.
A config with half a mark is refused by initialize() on macOS as well: the search for a global npm goes by the write paths there too. The case says so for every platform again. Since the walk's second round, what is kept of a directory whose real path failed to list depends on the name the walk came by: the failure, or what a shorter name listed. The case forced neither, so it went by the order readdir gave. It now forces both, and asserts what holds in both: the pattern that saw the failure has the directory denied whole.
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
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.
Draft, not for merging. Every open fix that is ready, merged together on top of
main, so that CI runs the combination on every platform and the joins between them are worked out once. Each change is reviewed and merged in its own pull request; this branch is thrown away afterwards.Two are in that are not up for merging as they stand: #573, which is out of the next release, and #575, whose second round (
b1b6258) is in and has not been reviewed.What is in it
Real merges, in this order: #595, #585, #580, #573, #575, #607, #620 + #621 + #622, #588 + #617, #594, #591, #584, #578, #589, #592; then
mainat 0.0.78, #629, #627 and #628; thenmainup to #638 (new to the branch: #636, #638 and #644; it held #620 to #622 already), #639, #630, #643 and #645; thenmainat 0.0.79 (new to the branch: #655, #660 and the version), #591 again at its new head, and #656; thenmainup to #585 (new to the branch: #662, #663, #665 and #666), #595 and #578 again at their new heads, #669, #670, #674, #673, #671, #675, #575 again at7ec70d0, #672, #679 at135aaaa, #680 at101ac8e, and #575 once more atb1b6258.Not in it: #615 and #579 (not ready), #569 and #574 (to be re-derived on top of #584 first), #577 (overtaken), #664 (the version, which goes last).
Joins that no conflict marker shows
Whichever of each pair lands second needs the same change.
--disable-usernsprobe lookedbwrapup onPATHand ran it on the host, which Take host-side helpers only from where the sandboxed command cannot write #594 forbids. It asks the copy the wrap resolved.bwrapand linux: a placeholder is removed only when no running sandbox relies on it #584's record together. Tests: a wrap with mount points starts with a shell, so "the first word" is no longer bubblewrap, and bubblewrap comes by its path: new test helperbwrapOf().anchoroption, a directory taken as the name it is #620: one options object,{ anchor?, budget? }, both passed on. A resolution that drops either type-checks.listings.sizeis gone after Glob walk: a drive root is a base, and two ways a directory could be taken for absent #575. Notrecords.size, which counts directories that could not be listed.{ path, literal: true }#621 with Take host-side helpers only from where the sandboxed command cannot write #594 and Warn when the library is installed where a sandboxed command can write it #595: both read the write roots throughwriteRootsOf(), or miss a path marked literal.initialize()builds the search path for a global npm from the write paths on every platform, so the third expects the refusal there.--versionspawns also dropped Take host-side helpers only from where the sandboxed command cannot write #594's uid-0 probe.withOtherReadings()of Read a path entry as the path it spells, where that exists #622 included, which git did not mark.anchoroption, a directory taken as the name it is #620's anchor, and linux: a wrap that starts over reads the disk again #644 took the walks out again, so that join is gone. Two things from it are worth keeping. A cache needs every input in its key: beneath an anchor the same characters are a name,withOtherReadings()walks one entry both ways in one attempt, and without the anchor in the key a.envinside[WIP] projectwas not masked; git marked nothing. And a join test can pass for another reason than its name: "takes the budget of the configuration that replaced the one it started with" replaces the configuration during the allow walk, so the first attempt already reads the new budget. A case beside it replaces it during the deny walk.anchoroption, a directory taken as the name it is #620 to Read a path entry as the path it spells, where that exists #622: the walk, the deny expansion and its test are the branch's own from before linux: a wrap that starts over keeps what it has walked, and ends #638. In the manager, linux: a wrap that starts over reads the disk again #644's two checks after theawaits sit around the branch's'allow'andreadDenyGlobExpander. A wrap that starts over makes a new budget and new listings again.main.mainthe count of wrapped commands defers the removal of mount points. Here a running sandbox answers for its own mount points and they already go deepest first, so what the count protects is the manifest and the--argsprofile of a command that is wrapped and has not started: a count that is one short costs a start. Taken over in those terms: forced cleanups are counted, a wrap handed out after one is put back in the count, and a wrap that throws gives its place back through the cleanup unless one came between. Three of linux: remove mount points when the last sandbox is done #671's cases assert a deferred removal, which there is none of here; they assert the deferred manifest. Two cases are new. If linux: a placeholder is removed only when no running sandbox relies on it #584 lands after linux: remove mount points when the last sandbox is done #671, this is the join to make.{ args, onTheLine, manifest, started, mandatoryDenyArgs }, and the renderer takes the share beside the started record.bwrap --versionprobe that got no answer #674 with linux: with no seccomp helper, have bubblewrap disable user namespaces #617: nothing to join, and worth knowing: the probe for--disable-usernsdecides an argument, so it goes on keeping answers only.withTypeinside the new retry, under the name the listing was made by.mainas well: whichever lands second needs this.readdirgave; it forces both now.resolveReadDenies()takes the branch'sreadDenyGlobExpander()wheremainhas an inline closure: one budget and one set of listings for both lists.filterRequest, and options that close the ways around a per-request policy #663, Terminate TLS in-process on the tunnel socket, with no per-tunnel listener #665 and srt-proxy: SRT's HTTP proxy with an external decider #666 (found by CI here): under the typings ci: pin ripgrep, Node, type packages and actions, and raise engines to Node 22 #589 installs (@types/node22 with@types/bun1.4.2)connectofnode:tlsisany, so sixteen callback parameters in five new test files are implicitlyany; they are annotated. AndFileSink.write()may return a promise there, whichopenFdStreams()left floating; it is chained beforeflush(), so that its failure reaches the stream's callback. If ci: pin ripgrep, Node, type packages and actions, and raise engines to Node 22 #589 lands after them it needs both.mainwith Glob walk: ananchoroption, a directory taken as the name it is #620 to Read a path entry as the path it spells, where that exists #622: before they landed each tookmainin, and made there three joins already made here. Of the nine blocks git marks the branch keeps its own side; what is new to it is comments.enginesismain's>=22.12.0.with { type: 'json' }: six of seven cases failed on both. ci: run the Docker legs on Node 22, not on the image's Node 18 #643 binds the runner's Node 22 in. The same holds onmain.PATH.main's squashes of linux: make the read-deny glob walk about six times faster #627, linux: the wrap walks its patterns in turns, and stops when its signal is aborted #628 and linux: one mount per destination, so bubblewrap 0.4 starts #580: they conflict with the same changes merged here before. Nothing is new in them (same trees in the conflicting files; same patch id for linux: one mount per destination, so bubblewrap 0.4 starts #580), so the branch's side is kept and its tree does not change.catch, the abort is looked at first, then what ripgrep listed.anchoroption, a directory taken as the name it is #620 to Read a path entry as the path it spells, where that exists #622 (test): one case said that what a write-deny pattern would match inside a folder that cannot be looked at is still written. Below the working directory linux: deny every dangerous name of a subdirectory, not only those three levels down #629 denies such a folder whole. The case now runs from two working directories and says both.mainwith Document what a --control-fd line changes, and the config keys the reference left out #578, ci: pin ripgrep, Node, type packages and actions, and raise engines to Node 22 #589 and ci: run the package as an embedder ships it, on musl and on macOS 26 #592:release.ymlismain's. Their changes to it are not carried.Checked
On the head with the stack for the next release (
2b01c47), against the head with #656:**read deny, a denied host; names with brackets; the cases of the reviews of linux: a wrap that starts over keeps what it has walked, and ends #638 and linux: one wrap, one working directory, and no object taken for the same install #655; the ask callback's answers and the per-command lists through the rewritten proxy; three endings of a process with TLS termination on.process.exit()leaves nothing of the library's in the temp directory, and neither does a rejectedinitialize(). linux: remember abwrap --versionprobe that got no answer #674: abwrapstand-in that sleeps on--versionis asked once. linux: the refusal at 9000 bwrap arguments says whose they are #675: 800 repositories one folder down are refused, with 9,636 of 9,760 arguments named as the built-in write protection, and both ways out that the message names run. Glob walk: ask for the type of an entry that was listed without one #672: with listings faked to give no types,**/.envrefuses the file at every depth. linux: remove mount points when the last sandbox is done #671, with its own scripts: nothing left after an aborted, a refused and a restarted wrap, and the denied name refused in everyreset()sequence.**rules over 400,150 files: peak memory 289, 481 and 480 MB on Node, 303, 519 and 438 MB on Bun; one set of mounts; an aborted waiter leaves at once.main's. The whole suite fails the same names as the head before (they need outbound network), nothing that passed fails, 602 new cases pass. The 0.4.1 list: 511 pass. The Node step and the bundled package pass.mainan inline closure keeps the map alive, here nothing captures it, and the runtime lets it go after its last use. The line is kept because it does not depend on that.On the head with #645, against the head before:
**rules: one and the same wrapped command on both heads; the matching file refused under real bubblewrap. With every file system call delayed by 1 ms, one wrap: 2.9 / 6.4 / 25.2 s before, 2.0 / 2.1 / 2.7 s now (5 / 20 / 100 rules).On the head with #644:
updateConfig()during the wrap, Node 22 / Bun 1.4.2: once halfway 1.4 / 1.5 of an undisturbed wrap; every 250 ms 1.4 / 1.7; the thread held for one walk where a third attempt was needed. A budget that arrives 250 ms into the wrap refuses it at 254 to 259 ms.On the head with #636, #638, #639 and #630 (the head after it adds #643, one workflow file):
anchoroption, a directory taken as the name it is #620: each fails a test. Three new cases intest/sandbox/read-deny-walk-joins.test.ts.**deny rules over 400,150 files,updateConfig()during the walk (the same content once and every 250 ms, a deny added, a new pattern every 150 ms): 0.7 to 1.1 of an undisturbed wrap, the command equal to an undisturbed one, the added deny applied;**/.envwritten beneath a folder named[WIP] project: both files there are unreadable in the sandbox;sleepplanted first onPATHis not run;On the head with #627, #628 and #629:
test/sandbox/read-deny-walk-joins.test.tsis new.**deny rules over 400,150 files: 6.4 s becomes 1.2 s, the event loop is held for 10 to 18 ms at a stretch, and the mounts are identical;On the heads before it:
mainon the same machine.bwrapplanted first onPATHin a writable directory is not run, with a seccomp helper, without one, and with mount points in play; a budget of five entries refuses a**/.envdeny inside a folder named[WIP] project, and the default budget denies the file; 3,900 commands from one, two and three processes in one project: none refused, no denied write landed, nothing left behind.