Repository navigation
Read brackets that spell no set of characters as text, so no path entry keeps a sandbox from starting - #687
Merged
ronleizrowice-ant merged 17 commits intoOct 12, 2026
Conversation
…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.
The spy type is not disposable under the type packages the test typecheck runs with, so `using spy = spyOn(...)` does not compile there. Each becomes a const restored in a finally, as the other suites do it, with the same scope the declaration had.
…base Two things the earlier commits on this branch got wrong. A pattern that cannot be read one component at a time denies whole a link that leads out of its starting directory, since nothing in there is listed under any name. "Out of the starting directory" included every ANCESTOR of it, and the test that leaves a link leading up alone came after that branch, where such a pattern never reaches it. So with an unsplittable deny glob, one `ln -s ../.. x` in the tree put a tmpfs over the directory holding the project, with nothing bound back beneath it, for every later command. A link that leads back up to the starting directory or above it is now left alone, as it is for a pattern that can be followed. The refusal of a filesystem root as a base was written for `/`, which is refused so that a whole filesystem is not listed. It also matched a drive root and a share root, so every Windows pattern whose only literal directory is one (`\\server\share\*.pem`, `D:\**\*.key`) matched nothing and was not enforced, where the last release walks and enforces them. Only `/` and a pattern with no literal directory are refused now. A base never carries a trailing separator; a drive root is handed to the filesystem as `C:/`, since `C:` alone is that drive's current directory. The README also says what is still not covered: a link whose target cannot be looked at, and that the pattern would continue through rather than match, is not followed and nothing is denied for it.
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.
…e alone again For a pattern that cannot be followed one path component at a time (a set holding a wildcard, `id[*].pem`), the walk denied the target of every symbolic link to a directory outside the base, anywhere in the base's subtree, whether or not the pattern could match beneath it, and bound nothing back under it. One link such as `vendor/sdk/conf -> /usr` hid /usr, and every sandboxed command failed to start. Anything able to write the tree can make such a link. That rule is removed: such a pattern descends no symbolic link, as before this branch, and the README says what that leaves uncovered. Also: the over-long-pattern fallback logs once and says why; the comment on the piece cap no longer names a number that is not true.
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.
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.
…enied too A matching link whose own stat or realpath failed by the real path is covered under its own spelling. Inside a directory the same pattern denies, that spelling is hidden by the directory above it, and what the link leads to, outside that directory, got no mount, though the short name had said where it is. That place is now a match as well. The link stays covered as spelled.
Conflicts with #672 in the walk's listing call and two README bullets: the listing asks for the type of an entry listed without one, and passes what counts as an absence and what to do with a directory that was not read.
A path entry whose brackets hold a range that runs backwards, such as a folder really named [user-id], compiled to no regular expression, and so did an ordinary relative pattern used from inside such a folder. Every wrap then threw. globToRegex judges one bracket expression at a time: one that would not compile is the text it is, and the rest of the pattern keeps its meaning. What it returns always compiles, on every platform alike.
#575 is on main as one commit. Against main this branch holds its own change and nothing else.
No brackets keep a pattern from compiling, but its size can: a name of about 1 MiB. readRuleCovers keeps its catch for that, so getFsWriteConfig() answers. What such a rule covers cannot be told, so it covers. The pattern is run once inside the catch: V8 finds it too large only then.
ronleizrowice-ant
deleted the
fix/a-path-entry-whose-pattern-does-not-compile
branch
October 12, 2026 14:16
This was referenced Oct 15, 2026
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.
The fault
A path entry whose brackets hold a range that runs backwards compiles to no regular expression: a folder really named
[user-id](r-i), orbackup[2024-01-15]. So does an ordinary relative pattern such as**/.envwhen the working directory lies inside such a folder, as route folders of web frameworks are named.Every wrap then throws, so no command can be sandboxed:
getFsReadConfig().allowReadanddenyWritethe string goes to Seatbelt as it is.initialize().The change
globToRegexjudges one bracket expression at a time. One that would not compile is the text it is, as POSIX reads a[that opens no set. The rest of the pattern keeps its meaning:app/**/[user-id]/*.envis a directory named[user-id]at any depth.No brackets keep what it returns from compiling, on every platform alike. So the walk's
SyntaxErrorgoes. ThecatchinreadRuleCoversstays for a pattern too large to compile (a name of about 1 MiB): it now counts as covering, and the pattern is run once inside thecatch, since V8 finds it too large only then. SogetFsWriteConfig()answers for such an entry in Node too, where it throws onmain. A line underSRT_DEBUGnames the brackets.Every pattern that compiled gives the same expression: 2,932,441 of 3,000,000 random patterns compiled before, and all are byte-identical. No schema change, no added file system call.
Not covered
mainrefuses the wrap), 1.2 s against 0.4 s where all do. An ordinary pattern costs the same.getFsReadConfig()throw, as onmain.**/[slug]/*.envdoes not match a folder really named[slug].[a-]on macOS, which JavaScript takes and Seatbelt may not (Glob [!x] and [^x] now negate (such allow rules widen); a set is written as both regex engines read it #573).Testing
sandbox-exec; the macOS profile read on any POSIX host; the Windows expansion; two properties. 49 tests fail onmain's source. 21 mutations, each failing a test.main: six configurations that throw there run here, and refuse or allow exactly the named paths. A folderubeside[user-id]is untouched.file[0-9].txt,[WIP] projectand*.pembehave as onmain.On the macOS jobs 41 rows pass under
sandbox-exec, on both architectures. On the Windows jobs the nine bracket rows of the files they run pass.