Skip to content

Read a caller's path as a path where it exists, and let a caller mark one - #608

Open
ronleizrowice-ant wants to merge 3 commits into
mainfrom
fix/literal-path-entries
Open

ronleizrowice-ant wants to merge 3 commits into
mainfrom
fix/literal-path-entries

Conversation

@ronleizrowice-ant

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

Copy link
Copy Markdown
Collaborator

A path inside a folder whose name holds [, ], * or ? was read as a pattern, so a caller's deny there was lost on every platform. An entry is now also read as the path it spells when that part of it exists on disk, and a caller can mark an entry as a path outright.

What was wrong

Whether an entry of denyRead, allowRead, allowWrite or denyWrite is a pattern is decided by looking for glob characters in what the caller wrote. A directory may have them in its name: [WIP] project, notes (draft?), build*. A caller that passes absolute paths inside one had each of them read as a pattern:

  • Linux, write lists: an entry that still has glob characters after a trailing /** is removed was dropped (bubblewrap has no patterns). denyWrite: ['/w/[WIP] project/keep'] was not applied; allowWrite: ['/w/[WIP] project'] made nothing writable.
  • Linux, read lists: the entry was expanded as a glob, [WIP] as a character class; it matched nothing and the deny was lost.
  • macOS: the entry compiled to a regex that covers /w/W project/keep and not the real folder.
  • Windows: [ and ] are not pattern characters in an entry without * or ?, so plain paths were fine, but a pattern beneath such a folder read the folder's brackets as a class.

The library's own paths (the mandatory denies, credential masks) already travelled as literals. This is about the caller's.

What changes

New module src/sandbox/path-entries.ts, where the reading of an entry is decided once and carried with it; no later stage looks at its characters again.

  1. Both readings, when the path exists. Of the path components that hold glob characters, the leading ones that exist on disk as spelled are names. When all of them exist, the entry is ALSO the path it spells (what follows need not exist). When a pattern follows an existing one, as in /w/[WIP] project/**/.env, the entry is ALSO that pattern beneath the folder, the folder taken as the name it is. The pattern reading stays as it was.
  2. A deny takes every reading; an allow only a path that is there as itself. For a deny, a component exists when anything has its name, a symbolic link included, or when it cannot be looked at: a deny that a command could switch off by making a parent unsearchable would be no deny. For an allow, no component from the first with glob characters on may be a symbolic link or beyond looking at: whoever can write the directory could plant one, and the allow would open what it points at.
  3. An explicit marker. Each of the four lists also takes { "path": "...", "literal": true }: a path and never a pattern, whether or not it exists. The schema, the CLI settings file, updateConfig, per-wrap customConfig and getConfig() take and give back the new form. Anything else in the object is refused.
  4. Literal end to end. FsReadRestrictionConfig and FsWriteRestrictionConfig carry the names in literalDenyOnly, literalAllowWithinDeny, literalAllowOnly, literalDenyWithinAllow. The Linux wrapper takes every path it is handed for a name (an eslint rule now forbids the sniffing form of normalizePathForSandbox in the two Linux files); the macOS builder emits a subpath/literal filter beside the regex, and escapes the anchor of a pattern beneath a named folder.

Who sees a difference

  • A project in such a folder: all four lists are enforced.
  • An entry with no such folder on disk: nothing. The computed configurations, the bubblewrap arguments and the macOS profile are byte-identical to main for a corpus of ordinary paths and patterns.
  • Embedders: the four lists are FilesystemPathEntry[] (string | { path, literal: true }); code that reads an entry as a string stops compiling. Without a compile error: the restriction configs may carry the literal* lists, so hand them on whole; an empty denyOnly no longer means nothing is read-denied.
  • Beneath a directory that cannot be searched when the command is wrapped, an ordinary deny pattern is also taken for the path it spells, and denied like any path there that cannot be looked at. That denies more than main, never less.
  • Cost: one lstat per ordinary pattern per wrap (about 1 ms for 200 patterns here).

Known limits

  • Linux write lists still take no patterns: a pattern beneath such a folder is skipped there.
  • What exists is looked at when the command is wrapped (Windows: at initialize()).
  • Inside build* or notes (draft?) the folder's name also matches itself as a pattern, so a deny ending in /** is applied as the pattern it is; write it without the /** to have an allowRead beneath it open.
  • A marked allow is resolved like any plain path, links included. credentials.files takes no marker.
  • Windows: * and ? cannot be part of a name, so a marked path holding one is skipped; a UNC pattern is a pattern only.

Not done here

On main and here alike, an allowWithinDeny spelled with .. after a wildcard passed DIRECTLY to wrapCommandWithSandboxLinux is resolved by its text; the manager never produces one.

Merge order

Late. It touches the call sites of all four lists in sandbox-manager.ts and both backends, so it conflicts textually with #569 (macos-sandbox-utils.ts), #574 (linux-sandbox-utils.ts), #575 (sandbox-utils.ts, its test and the README), #595 (sandbox-manager.ts) and #607 (read-deny-glob.ts, sandbox-manager.ts); it merges cleanly with the other open PRs. Whichever merges after needs a merge of main.

One thing a merge will NOT flag. A literal... list holds more entries, not a part of the plain list, so code that asks "may the command write here" has to ask writeRootsOf(config) and not allowOnly alone, or it does not see a root the caller marked. Every reader here does. #569, #594 and #595 each add readers of allowOnly on lines that do not conflict; whichever side merges second has to route them through writeRootsOf() by hand. #595's literalDenyWithinAllow has the same meaning here.

Testing

test/sandbox/literal-path-entries.test.ts (new, 109 cases), plus schema cases and walk cases. Deciding the reading: every shape above, ~ and relative spellings, each glob character, a link or a dangling link in the prefix, an unsearchable parent. Linux, by real bubblewrap runs inside [WIP] project: the project writable when allowed, a denyWrite enforced with the host file unchanged, a denyRead file and directory unreadable, a carve-out working, a recursive deny masking .env at two depths; each fails on main. Planted links: an allow never follows one. macOS profile text for the same, including the escaped anchor. Both readings at once ([ab] beside a and b). The marker through every entry point, all four lists, and the violation monitor. The differential against main for ordinary entries. 48 mutations of the change: 45 fail a test. Of the three that survive, one is caught by the new lint rule instead, one is in code only Windows reaches, and one is a difference that is allowed on purpose (an empty literal list means the same as an absent one).

Both typechecks, eslint and prettier clean. Both runtimes. Whole suite on Linux x86-64, Bun 1.4.2: identical failing names to main. Not run here: enforcement under sandbox-exec, and the Windows cases; CI runs both.

… one

Whether an entry of denyRead, allowRead, allowWrite or denyWrite is a
pattern is decided by looking for `*`, `?`, `[` and `]` in what the
caller wrote. A directory may have those characters in its name, and a
caller that lists absolute paths inside `[WIP] project` had every one of
them read as a pattern: on Linux a write entry was dropped and a read
entry expanded to nothing; on macOS the rule compiled to a regex that
covers `W project` and not the folder. The deny was lost on both.

An entry with those characters keeps the pattern reading it always had,
and is now also read as what it names wherever the part of it that
holds the characters exists on disk: as the path it spells when all of
it does, and as the rest of the pattern beneath the folder when a
pattern follows, the folder taken as the name it is. A deny takes every
reading, and counts a name as there when anything has it, a symbolic
link included, or when it cannot be looked at. An allow takes a reading
only for a path that is there as itself: no component from the first
with such a character on may be a symbolic link.

Each of the four lists also takes `{ path, literal: true }`, a path and
never a pattern whether or not it exists.

The decision is made in one new module, path-entries.ts, once per entry
per wrap, and carried with the entry: the restriction configs hold the
names in literalDenyOnly, literalAllowWithinDeny, literalAllowOnly and
literalDenyWithinAllow, the Linux wrapper takes every path it is handed
for a name, and the macOS builder emits a literal filter beside the
regex and escapes the anchor of a pattern beneath a named folder. A lint
rule keeps the sniffing form of normalizePathForSandbox out of the two
Linux files. An entry with no such folder on disk gives what it gave.
How an entry is read, for every shape of spelling, each glob character,
a link or a dangling link in the prefix and an unsearchable parent. By
real bubblewrap runs inside `[WIP] project`: the project writable when
allowed, a denyWrite enforced with the file on the host unchanged, a
denyRead file and directory unreadable, a carve-out working, a recursive
deny masking `.env` at two depths. An allow never follows a planted
link. The macOS profile for the same, with the anchor escaped. Both
readings at once. The marker through the schema, the getters,
updateConfig, a per-command config in all four lists and the violation
monitor. And that entries with no such folder on disk give
configurations, bubblewrap arguments and a macOS profile identical to
what they gave before.

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.

1 participant