Repository navigation
Draft, not for merging: seven pull requests merged together - #751
Draft
ronleizrowice-ant wants to merge 150 commits into
Draft
ronleizrowice-ant wants to merge 150 commits into
ronleizrowice-ant wants to merge 150 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.
The Linux and macOS backends resolve the dangerous files and directories, plus .git/hooks and .git/config, against the working directory and deny writes to them whatever the configuration says. Windows had no counterpart. windowsGetCwdMandatoryDenyPaths reads the same two definitions, so the three platforms protect the same names. It returns only the paths that are there: a Windows deny is an ACE and an ACE needs an object, and the placeholder chain srt-win materializes for a missing deny target is permanent by design, which is the right answer for a denyWrite entry someone asked for and the wrong one for a name nobody did. The existence filter also covers what the Linux producer spells out as a .git-is-a-directory gate: nothing exists beneath a .git pointer file.
computeWindowsFsAccessSet unions the producer into denyWrite, so the paths are stamped once at initialize() alongside the configured denies. A path that is both granted and denied gets both ACEs and srt-win writes the deny first, so allowWrite does not lift one; filesystem.allowGitConfig lifts the .git/config deny as it does on the other platforms, and joins the raw inputs updateConfig's short-circuit compares. The M rows drive it on the Windows legs: a write to .git/hooks/pre-commit and to .bashrc refused with the hooks directory explicitly granted, an ordinary project file written, allowGitConfig lifting one deny and not the other, and git status still working with the set stamped.
The mandatory-deny section carried a Linux note and no Windows one, and the Windows known-limitations list named five other gaps but not this one.
…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.
… hand @types/node goes from ^18 to ^22 and @types/bun from ^1.3.2 to ^1.4.2, which is what CI runs. Two things surfaced. The CONNECT handler's `head` is reassigned with bytes read off the socket, so it is annotated as a plain Buffer. And the Bun type package redeclares `connect` for 'node:tls' in a way that, against @types/node 22, takes and returns an untyped value, so the four call sites pin their options with `satisfies ConnectionOptions`, and a guard in parent-proxy.test.ts fails the typecheck if ConnectionOptions ever stops being a real type. Adds `npm run test:node`, which runs the two checks that start the library under node rather than Bun. The emitted JavaScript changes only by comment lines.
Linux legs install ripgrep 14.1.1 from the upstream release by sha256 instead of taking whatever apt serves, and run the suites that exercise the scan a second time against 15.2.0: the two differ in exit status for an unreadable directory at --max-depth. The main legs move to Node 22 and run `npm run test:node` on Node 20, 22 and 24; publish moves from Node 18, below the package's floor, to 22. `npm install` becomes `npm ci`. Actions are pinned by commit with the version in a trailing comment.
The string globToRegex returns is compiled twice: by JavaScript, and by the regex engine of a macOS sandbox profile. The two read a `-` inside a set alike in one place only, first among the members. A negated set whose first member is `-` was written `[^/\-a]`. The profile's engine takes a backslash inside a set for a member, so there that is "not `/`, and not `\` to `a`": a deny `[!-a]x` let `_x`, `^x` and `]x` through and refused `-x`. Writing the `-` last is no way out: that engine rejects a set that ends in one character and a `-` (`[a-]`, `[^/a-]`) as an unterminated bracket expression, and the whole profile with it. That part is older than this branch: a positive set was always passed through as written, so a rule holding `[a-]` already gave a profile that does not load. A wildcard-free set holding a `-` of its own is now compiled as a unit: the `-` goes first (`[-a]`, `[^-/a]`), a range that starts or ends at one becomes the `-` and the rest of the range (`[!--0]` is `[^-/.-0]`, `[+--]` is `[-+-,]`), and behind it only a backslash gets a backslash. Every other set compiles to the string it did before. Tests: the meaning of 24 set bodies in three spellings against JavaScript's own reading, exact strings for the shapes measured under sandbox-exec, and a rule checked over every generated set: a member `-` only first, none before the closing `]`, no backslash but a backslash's.
Node 20 is past its end of life, and nothing here is built or tested on it first: the legs build on 22. engines becomes >=22, the steps that ran the node-side checks on Node 20 go, and the package verification in release.yml covers 22 and 24. An install on Node 20 with engine-strict set is now refused, where it used to go through.
The suites run the sources unbundled. An application that embeds the library ships it in one file, minified and compiled to bytecode, where a function's name, its source text and the files beside it on disk are all gone, and nothing here would notice code that leans on one of them. `npm run test:bundled` compiles a small program that imports dist/ that way and runs it: it initialises the manager, wraps real commands, and checks on the host what they wrote, that the network was closed, that the command's own exit status came back and that the mount point for a denied path is gone afterwards. It imports the built package, so it has a tsconfig of its own that is checked after the build rather than with the rest of test/.
Three things CI did not see. The package as an embedder ships it, which `npm run test:bundled` now runs on every leg that can wrap. musl: the container job repeats its two runs in alpine:3.22, with the seccomp helper built static against musl's libseccomp and Alpine's own bubblewrap, socat and ripgrep. And the newest macOS beside the two images the macOS legs name, since what Seatbelt and the profile's regex engine accept has moved between releases.
An embedding application is built into code-split ESM chunks as often as into one flat file, and a split bundle evaluates modules in another order with the library in a chunk of its own. The runner now builds the program both ways, and the program reaches the library through import() so that it really does sit in a second chunk. Each executable is copied, with the seccomp helper, into an empty directory and run from there. An embedded copy has no vendor/, no node_modules and no package.json beside it, so a lookup that starts from the library's own location has to find nothing and not matter. On Linux the program also provokes a refusal, a profile with more deny paths than bubblewrap takes arguments for, and checks that the error is still an instance of the exported class and still carries the name and the code a caller branches on, with every identifier shortened. Inside a container the program asks for the weaker nested sandbox, as the container suite does: bubblewrap cannot mount /proc there.
Both container steps piped bun's installer into a shell. They now fetch the 1.4.2 release archive for the architecture, glibc or musl, check it against a sha256 recorded here and install the one binary, the way the ripgrep steps do.
…r names An application that has the seccomp helper and ripgrep compiled into its own executable reaches them by running itself under another name: `seccomp.argv0` with `applyPath: '/proc/self/fd/3'`, fd 3 of every wrapped command being an open handle on the executable, and `ripgrep.argv0` with the executable as the command. No suite runs that arrangement, and it is a path of its own through the wrap: the ARGV0 assignment ahead of the helper, a helper path that is used as given and never looked for, and no helper file anywhere to find. The smoke program now plays such an executable. Started as `apply-seccomp` or as `rg` it runs the real one; and on Linux the runner runs each build a second time configured that way, checking that the scan did start it as `rg`, that the wrapped command did reach the helper through fd 3, and that it ran under the seccomp filter.
Under Node, whichSync ran `which` by that bare name, so whatever `which` came first on PATH was executed, on the host, on every wrap: the shell a command runs in is resolved through it each time. Under Bun it was Bun.which, which executes nothing. It is now one search made inside the process on both runtimes: each PATH entry in order, the first holding a regular file of that name this process may execute. A name that already carries a directory is checked where it is and not searched for. Bun.which is not used any more, so that the two runtimes cannot drift: on Bun 1.4.2 it does not see a PATH changed inside the process, and ignores an empty entry.
…rite bubblewrap, socat and ripgrep are run by the library itself, on the host, outside the sandbox, and were named bare: `bwrap` at the head of the wrapped command, `socat` for the bridges, `rg` for the scan every wrap makes. On macOS the wrapped command began with a bare `env`, ahead of the hard-pathed sandbox-exec, and the violation monitor ran a bare `log`. Whoever runs a bare name takes it from PATH, and PATH often leads with a directory the sandboxed command may write: under `npx` and `npm run` it is <project>/node_modules/.bin, with the project in allowWrite. A file of that name left there by one command was run, unsandboxed, by the next wrap. The rule is about the file that gets chosen: the library never executes on the host a file the sandbox policy lets the wrapped command write. A helper is looked up inside the process, and a copy is passed over when the file as spelled, any symbolic link followed on the way to it, or the file it resolves to lies at or under an allowed write path; relative and empty PATH entries are never used. The result is an absolute path, used wherever the bare name was. A remembered result is looked at again under the current write paths on every use. With nothing left, the dependency check reports an error naming the copies passed over and a wrap throws LinuxSandboxProfileError `host_helper_unavailable`, rather than fall back to the bare name. A policy that allows writes everywhere gives the rule nothing to prefer and is not filtered. `/usr/bin/env` and `/usr/bin/log` are fixed paths on macOS, as sandbox-exec already was. The shell a command runs in keeps the whole PATH: it runs inside the sandbox. A helper the operator names (bwrapPath, socatPath, a ripgrep.command with a directory, seccomp applyPath) is run as given, and the dependency check warns when the sandboxed command may write it. The uid-0 probe runs the resolved bubblewrap's own --version inside its user namespace instead of a bare `true` looked up with the whole filesystem bound writable.
Where the seccomp helper, or the JVM agent's jar, is not beside the library, a global install is looked for by asking `npm root -g`. That went through /bin/sh and the PATH as it was, and npm is a script that finds `node` by name, so resolving one file would not have been enough: it is the PATH the child gets that has to leave the writable places out. hostSearchPath() gives that PATH, by the same test as for a host helper (the entry as spelled, any link on the way, where it resolves; relative and empty entries never), and the manager passes it from the two places the look-up is reached with a configuration in hand: initialize(), for the jar, and the asynchronous dependency check, for the helper. The first answer is kept for the life of the process, so those are what decide in the ordinary order of calls. A synchronous dependency check made before either still uses the PATH as it is; the README says so. The README's earlier sentence named the helper alone. The jar's look-up goes the same way and is reached more often: a build from source has no jar beside it.
hostSearchPath() left out the entries of PATH that lie in, or lead into, a place the sandboxed command may write. What a child runs is a file, though, and a link kept in a directory nobody can write may still lead to a file somebody can: a `node` in /usr/local/bin linked into a version manager's directory that is an allowed write path. The file-level test already made for bubblewrap, socat and ripgrep is now put to each entry's copy of the programs the child is known to look up, and an entry whose copy would be refused is left out with it. The manager passes `npm` and `node` for `npm root -g`.
Two ways the judgement of an entry's copy of a program differed from what the shell that does the look-up would do. The candidate was built with path.join, which folds `..` away by spelling, while the shell resolves it through the file system: through a link, `<dir>/link/..` is another directory than `<dir>`. It is now the entry, a slash and the name, and the walk resolves the rest. And a copy was only judged if it was an executable file at that moment, so a link into a writable place with nothing behind it yet, or a file not yet made executable, let the entry through for the sandboxed command to complete afterwards. Whatever is at the name is now judged; only a name that is not there at all has nothing to say.
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com> # Conflicts: # src/index.ts
The reason given was too wide: it is srt that hands MXC a write deny as a full one. The schema text, the policy mapping in mxc-sandbox-utils.ts and the Windows notes in the README name the backend they hold for, and the case reads the request of a command as well as the one initialize() probes. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
ronleizrowice-ant
changed the title
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
Collaborator
Author
|
A test that runs out of time fails without an error object, so it would leave no annotation. That would fit a slow runner, but it is a guess: the job's log cannot be read from where I run. I will re-run the job once when the run has finished. A second failure I will take as real, and have the workflow on this draft name the tests that failed. Later, 23:55Z: the one re-run passed, so 9bc15b9 is green on all 9 legs. What ended the first run is still unknown. 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
Draft, not for merging. Seven open pull requests merged together on top of
main, so that CI runs the combination on every platform before any of them lands. Each is reviewed and merged in its own pull request, and this branch is thrown away afterwards. It does for these seven what #624 did up to 2026-10-09.What is in it
Real merges of #589, #592, #573, #607, #579, #693 and #594, each at the head that has
mainat951e641(2026-11-09) merged in. Thewip/directory holds the tools the local checks were run with.The joins
Only #693 collides with others, so it is best landed last. Whichever lands second needs these.
ReadDenyGlobOptionsgainsmatchedLinks, and both call sites pass{ anchor, budget, listings, matchedLinks }.linux-sandbox-utils.ts, andgenerateFilesystemArgs(..., abortSignal, writable, linksAreHeld), whose call passeswritableand thenwhyNotHeld === undefined.whyLinksAreNotHeld(helperInUse, config, writable)handswritabletogetApplySeccompBinaryPath(), andhost-helpers.test.tscounts[6, 1, 1]lookups instead of[4, 1, 1]. Without thesemainwould turn red with no conflict shown.Every other pair merges without a conflict, in either order.
What the merge of 2026-11-06 found
mainin theISandboxManagerinterface. Both sides are kept.main's newwindows.backend: 'mxc'with no conflict shown: the mandatory write denies would have reached MXC as full denies. It is settled in windows: apply the mandatory write denies, and keep the directories above a kept deny in place #579.What the merge of 2026-11-09 found
mainininitialize(), wheremainadded astopIfReset()after the look for the JVM agent's jar. Both sides are kept.awaittoinitialize(), andmain's changes topersist.rslie apart from windows: apply the mandatory write denies, and keep the directories above a kept deny in place #579's.Testing
On Linux x86-64 with bubblewrap 0.12.0, the whole suite fails the same 48 names here as on
mainat951e641on the same machine, which has no outside network and a newer Bun than CI. Both type checks and eslint are clean under #589's typings.Claude session #01FydxqU
Generated by Claude Code