Skip to content

Glob [!x] and [^x] now negate (such allow rules widen); placeholders are literal - #573

Open
ronleizrowice-ant wants to merge 15 commits into
mainfrom
fix/glob-to-regex-negation-and-placeholders
Open

ronleizrowice-ant wants to merge 15 commits into
mainfrom
fix/glob-to-regex-negation-and-placeholders

Conversation

@ronleizrowice-ant

@ronleizrowice-ant ronleizrowice-ant commented Sep 19, 2026 •

Copy link
Copy Markdown
Collaborator

globToRegex compiles a glob for all three backends. It never negated a bracket set, so [!x] and [^x] matched only ! (or ^) and x, and a path component spelled __GLOBSTAR__ came back as a wildcard.

What changes

  • A leading ! or ^ negates a set; a negated set never matches /.
  • The pattern is read once, with no placeholders: literal text is never read as a wildcard.
  • Every [ that opens no set is literal, an empty set ([], [!]) included.
  • Inside a set only \ is escaped, and a member - is written first: the one spelling JavaScript and the macOS profile's regex engine read alike.
  • The Linux read-deny walk drops two fences for patterns it could not read back.

Who sees a difference

  • An ALLOW rule written [!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.
  • [!] and [^] are literal text now: an old deny /x/[!] no longer covers /x/!.
  • Patterns with two unclosed [ (/d/[[) used to throw and now compile, as literal text.
  • Linux: patterns holding __GLOBSTAR or a second unclosed [ are now walked through symlinked directories.
  • macOS: a rule with a set like [a-] loads; sandbox-exec rejected the whole profile before.

Known limits

  • macOS: a deny with a negated set whose members sit next to / in character order ([!0-9], [!.]) is wider than written, never narrower. More in the README.
  • A set that holds a wildcard ([*]) is still not read as a set, and [z-a] does not compile, as before.
  • Windows was not run.

Merge order

One README line conflicts with #575 and #607. The external #581 reads a rooted brackets-only path as literal, which would undo the negated set: land one. #608's rule for a lone [ should match this one's for empty sets.

Testing

Compiler swept against a reference matcher (189,751 set bodies x 95 characters). A macOS-only test runs seven compiled sets under real sandbox-exec: it passes on CI's macOS arm64 and x86-64 legs, so both engines read the new spellings alike.

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.
…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.

@antdres antdres left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread README.md Outdated
- `**` - Matches any characters including `/` (e.g., `src/**/*.ts` matches all `.ts` files in `src/`)
- `?` - Matches any single character except `/` (e.g., `file?.txt` matches `file1.txt`)
- `[abc]` - Matches any character in the set (e.g., `file[0-9].txt` matches `file3.txt`)
- `[!abc]` and `[^abc]` - Matches any character outside the set, and never `/` (e.g., `file[!0-9].txt` matches `filea.txt` but not `file3.txt`)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one probably needs a release note. Any existing rule written [!x] or [^x] now covers the opposite set, allow rules included. An allowRead or allowWrite like /data/[!a]*, which used to match only names starting with ! or a, now matches almost every name in that directory. Someone upgrading would get a wider allow without changing their config.

For context, I compared the old and new compiler on 529 patterns: 44 realistic denies plus every glob string in the repo's tests and docs. Only the negated-set, empty-set and second-unclosed-[ cases changed, so this is the one user-visible change worth calling out.


Generated by Claude Code

Comment thread src/sandbox/sandbox-utils.ts Outdated
Comment on lines +1180 to +1184
// A `-` first among the members would read as a range with that `/`.
if (set.negated && globPattern[i] === '-') {
regex += '\\-'
i++
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this \- escape been checked under sandbox-exec? The live macOS run in the description covers [!a], which has no leading -. In JavaScript, [^/\-a] reads as "not /, - or a". If the profile's regex engine treats a backslash inside brackets as a literal character, the way POSIX bracket expressions do, it would read \-a as the range from \ to a. A deny like [!-a]x would then fail to cover _x and would match -x, which is narrower than written.

It's a narrow corner, but a deny with [!-a] or [!-.] under sandbox-exec would settle it. If the escape doesn't hold there, putting the - last among the members would avoid needing it.


Generated by Claude Code

antdres
antdres previously approved these changes Sep 20, 2026

@antdres antdres left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@ronleizrowice-ant ronleizrowice-ant changed the title globToRegex: negate bracket sets, and compile from tokens so no path text can become a wildcard Glob [!x] and [^x] now negate (such allow rules widen); placeholders are literal Sep 28, 2026
…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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants