Repository navigation
Glob [!x] and [^x] now negate (such allow rules widen); a set is written as both regex engines read it - #573
Open
ronleizrowice-ant wants to merge 21 commits into
Open
ronleizrowice-ant wants to merge 21 commits into
ronleizrowice-ant wants to merge 21 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.
This was referenced Sep 19, 2026
Closed
antdres
reviewed
Sep 20, 2026
antdres
left a comment
Collaborator
There was a problem hiding this comment.
Two non-blocking notes inline. The rewrite itself looks right to me. I fuzzed it against a separately written reference matcher, and the Linux walk agrees with the full-path regex on patterns that spell __GLOBSTAR__ or carry a second unclosed [, so removing the two checks there is safe.
Generated by Claude Code
antdres
previously approved these changes
Sep 20, 2026
antdres
left a comment
Collaborator
There was a problem hiding this comment.
Approving. The two inline notes (a release note for the [!x] flip, and a sandbox-exec check of the \- escape) don't block this.
Generated by Claude Code
…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.
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.
Comments only: the JavaScript emitted with comments removed is byte-identical for every file, and the test names are unchanged. Kept: every invariant and what could be done without it, the documentation of exported names, and the reasons for what is not done the obvious way. Cut: how the code came to be as it is, narration of the lines that follow, and explanations repeated at several sites.
…exec Inside a set only a backslash is escaped now, on every path. A macOS sandbox profile's regex engine takes a backslash inside a set for a member, so `[^/\$]` also left out `\`: a deny of `[!$]` was narrower there than written. JavaScript needs none of those escapes inside a class. A negated set that holds a wildcard keeps a leading `-` first, not as a range from the `/`. New macOS-only test: deny rules compiled from seven sets run under real sandbox-exec and must hide exactly the names JavaScript says they match. New test for the two fences the Linux walk dropped: a placeholder name and a second unclosed `[` are followed through a link. README: an example that holds for a macOS deny as well.
dylan-conway
previously approved these changes
Sep 28, 2026
main has read a pattern by a parser of its own since this branch left it
(src/sandbox/glob-pattern.ts): once, into tokens, with no placeholders, with
every bracket that spells no set taken as text and a wildcard in a set taken
as a member. That overtakes most of this branch: the single reading, the
literal `[`, the empty set, and the two fences the Linux walk dropped. In the
three files git marks the tree is main's, and what is left of this branch is
put into the parser:
- A `!` or `^` straight after the `[` negates the set. The token says so, and
its members are what follows. A negated set never matches a `/`, so the
walk never takes one for a separator.
- `[!]` and `[^]` have no members, and `[!z-a]` runs backwards: they spell no
set and are text, by the rule main has for `[]` and `[z-a]`.
- A set is written the way JavaScript and the regex engine of a macOS sandbox
profile read alike: each member as it is, a backslash doubled, a `-` that
is a member first. main escaped `.^$+{}()|\` in a set, where that engine
takes the backslash for one more member, and left `[a-]` as written, which
that engine refuses, and the whole profile with it.
Where main reads differently from this branch's base, the cases follow main:
a range written backwards is text and compiles, and `[!-*]` is a set of two.
Tests: the parser's cases and its reference matcher read negation; the
property that holds the walk and the regular expression together draws
negated sets; this branch's cases for what each set holds and how it is
written are kept, and so is the macOS suite under sandbox-exec.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv
Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
…property The sets run under sandbox-exec held a `-` only where they were not negated. `[!-a]` is written `[^-/a]`, the `-` between the `^` and the `/`. The reference matcher's paths had no `,`, so the property could not see where a range that ends at a `-` is cut: `[+--]` is the `-` and `+` to `,`. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
ronleizrowice-ant
enabled auto-merge (squash)
October 20, 2026 06:43
ronleizrowice-ant
disabled auto-merge
October 20, 2026 09:56
ronleizrowice-ant
added a commit
that referenced
this pull request
Oct 20, 2026
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
A review of the whole diff asked what the profile's regex engine does with what is written for a set. It was measured with sandbox-exec on macOS 14, 15 and 26, in a deny with its tail, in an allow and in a carve-out. - A negated set is misread as `[^/]` only in a rule with the tail, and only where two neighbours are written apart. They are written as one range now (`[!0-9]` is `[^/-9]`), which leaves a `-` beside a `,` or a `.`. - `[[.]` made the whole profile fail to load, and `[[:alpha:]]` was read as a POSIX class. The members go in ascending order, in which no `[` is followed by `.`, `:` or `=`. - A range that starts or ends at a `-` is the `-` and the rest of it. - The macOS test runs what is emitted, tail included, over every set of one or two of the characters the engines disagree about, in all three rules. - The set tables in glob-expand.test.ts repeated those of two other files and are gone: 256 added lines there are 27. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
A second review, of the whole diff as it now stands. Sorted by code unit, the halves of two characters outside the basic plane parted: `[😀😁]` came out with a lone surrogate, which reaches a macOS profile as the text `\ude01`. Only the ASCII members are sorted and merged; the rest follow as written. Tests: the separator rule of a negated set is shown in the walk too, the structural check compares the polarity of what is written with the pattern's, and three more sets go under sandbox-exec: a range that starts at a backslash, and a `-` with one and with both of its neighbours. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FydxqUESqxwSCVFnhk3gfv Co-authored-by: ronleizrowice <ronleizrowice@anthropic.com>
ronleizrowice-ant
enabled auto-merge (squash)
October 20, 2026 13:06
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>
ronleizrowice-ant
disabled auto-merge
November 11, 2026 16:13
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
[!x]and[^x]never negated a bracket set: they matched only!(or^) andx. And a set was written in a way the regex engine of a macOS sandbox profile reads differently from JavaScript, or refuses.Size
+327 −31 in 6 files: 96 lines in one source file (
src/sandbox/glob-pattern.ts), 5 of README, the rest tests. It was +640 before the tests were cut.Rebuilt on 2026-10-20.
mainhas read a pattern by a parser of its own since #687 and #718. That overtakes most of what this pull request held when it was approved on 2026-09-28, so the source change is new and wants reading again.What changes
!or^straight after the[negates the set; a negated set never matches/. The token saysnegated, and its members are what follows.[!]and[^]have no members and[!z-a]runs backwards: they spell no set and are text, by the rulemainhas for[]and[z-a].setToRegex, whose comment lists the four things the profile's engine does):mainescaped.^$+{}()|\in a set, where that engine takes the backslash for one more member;[is followed by.,:or=:[[.]made the whole profile fail to load and[[:alpha:]]was read as a POSIX class;-first, and a range that starts or ends at one as the-and the rest:[a-]and[--9]are refused;[!0-9]is[^/-9],[!abc]is[^/a-c]). Written apart, such a set is read as[^/]in a deny;What was measured
With
sandbox-execon CI's macOS 14, 15 and 26 legs, the same on all three. The misreading of a negated set happens only in a rule that carries the deny's tail(/.*)?$, and only where two neighbours are written apart. In an allow rule and in arequire-notcarve-out every form is read as JavaScript reads it, so no allow is wider than written. That was the open question of the earlier description.Who sees a difference
[!x]or[^x]used to grant two names and now grants almost every name: review such rules. A deny written that way now covers what it says. Release-note it.[!]and[^]are literal text now: an old deny/x/[!]no longer covers/x/!.[a-]or[[.]loads. A set with.,$,+and the like no longer holds\as well.[ab],[0-9]), so the profilesmainpins are unchanged.Known limits
-and one of,and., not both ([!-.],[!+--]), covers every character but/: wider than written, never narrower. AnallowReadcannot re-open what such a deny covers beyond its words.?and a set match no character outside ASCII; elsewhere they match one UTF-16 unit, so no character beyond the basic plane.file[!0-9].txtdoes not coverfile😀.txtanywhere.?is like that onmain.xfor every set, so a negated deny set that leaves outxgets no carve-out. It fails closed.Merge order
No conflict with the other open pull requests as they stand.
Testing
test/sandbox/macos-glob-sets.test.ts: how 16 sets are written; that no set body of up to four of eleven characters is written in a form the engine was seen to refuse or misread; and, on macOS, 288 sets under realsandbox-exec, each in a deny with its tail, in an allow and in a carve-out, against 18 file names, compared with JavaScript's reading.glob-expand.test.tsrepeated those of two other files and are gone.main merged in on 2026-11-06
mainat27fce0dis merged in at8a4a93f, without a conflict. What this pull request changes is the same as before, +327 −31 in 6 files.main merged in again on 2026-11-09
mainat951e641is merged in at9713af9, without a conflict. What this pull request changes is the same as before.Claude session #01FydxqU
Generated by Claude Code