Read brackets that spell no set of characters as text, so no path entry keeps a sandbox from starting

README.md

@@ -538,7 +538,7 @@ Paths support git-style glob patterns on macOS, similar to `.gitignore` syntax:
 - `*` - Matches any characters except `/` (e.g., `*.ts` matches `foo.ts` but not `foo/bar.ts`)
 - `**` - 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]` - Matches any character in the set (e.g., `file[0-9].txt` matches `file3.txt`). Brackets that spell no set, because a range in them runs backwards (`[user-id]`, `backup[2024-01-15]`), are the text they are, on every platform; the rest of the entry is still a pattern (`app/**/[user-id]/*.env`)
 
 Examples:
 
@@ -573,7 +573,7 @@ Examples:
 - `~` expands to the user's home directory
 - A deny glob must not end in a separator: the separator becomes part of the compiled pattern, so the pattern can match no path. `denyRead`, `denyWrite` and a `mode: "deny"` credential file reject such an entry at config validation — write `/data/*`, or add a `**` segment to match at any depth.
 - A directory may have `*`, `?`, `[` or `]` in its name (`[WIP] project`, `notes (draft?)`). An entry with those characters is read as a pattern, as described above, and **also** as the path it spells when the part of it that holds those characters exists on disk. With a folder `/work/[WIP] project` there, `/work/[WIP] project/keep` is both the pattern and the path, and `/work/[WIP] project/**/.env` is both the pattern and the pattern `**/.env` beneath that folder. A deny covers every reading and an allow allows every reading. What exists is looked at when the command is wrapped (on Windows, at `initialize()`), so a folder created later counts from the next command on (on Windows, from the next `initialize()`). For a deny, a symbolic link of that name counts as existing, and so does a path that cannot be looked at, whatever the entry: beneath a directory that cannot be searched when the command is wrapped, an ordinary pattern such as `/locked/*.pem` is also taken for the path it spells, and denied like any path there that cannot be looked at. For an allow the path has to be there as itself: if any component, from the first one with such a character on, is a symbolic link, the entry is a pattern only.
-- A `*` or `?` in a folder's name also matches itself as a pattern, and so does a `[` or `]` without its partner, so inside `build*` or `notes (draft?)` the pattern reading of an entry finds the path too. That shows in a deny ending in `/**`: it is applied as the pattern it is, under which on Linux every entry beneath the directory keeps its own mask (see above) and an `allowRead` beneath it opens only what is created later. Write such a deny without the `/**`.
+- A `*` or `?` in a folder's name also matches itself as a pattern, and so do a `[` or `]` without its partner and brackets that spell no set, so inside `build*`, `notes (draft?)` or `[user-id]` the pattern reading of an entry finds the path too. That shows in a deny ending in `/**`: it is applied as the pattern it is, under which on Linux every entry beneath the directory keeps its own mask (see above) and an `allowRead` beneath it opens only what is created later. Write such a deny without the `/**`.
 - To name a path and nothing else, write `{ "path": "/work/*.env", "literal": true }` in place of the string. Such an entry is never a pattern, whether or not the path exists, so it can deny a file that is not there yet in a folder that is not there yet. `~` and relative paths are resolved as for a string, symbolic links are followed as for any path without such characters (a marked allow has no check for them), and a `/**` at the end is part of the name. `denyRead`, `allowRead`, `allowWrite` and `denyWrite` take this form; `credentials.files` does not. An object without `"literal": true`, or with any other key, is rejected.
 - Not covered: on Linux, `allowWrite` and `denyWrite` still take no patterns, so a pattern beneath such a folder is skipped there, and `getLinuxGlobPatternWarnings()` still names every write entry with those characters that is not marked, and every read entry whose pattern is skipped, whether or not it is applied as a path. On Linux, a `denyRead` for a path that does not exist when the command is wrapped hides nothing in that command, marked or not. On Linux, the violation monitor takes its lists at `initialize()`: a folder created later counts for the commands wrapped after it, and the monitor does not learn of it. On macOS, an allow that is also the name of a path allows everything beneath that path, as any name does, where its pattern allows only what it matches: under `allowWrite: ["/work/*"]` a command can create a folder named `*` in `/work`, or rename one to that, and from the next command on write everything in it (under `allowRead`, read everything in it, whatever a `denyRead` pattern covers there). On Windows, `*` and `?` cannot be part of a name, so a marked path that holds one is skipped, and a UNC pattern is read as a pattern only, because finding out what exists would mean asking the share.
 - For embedders: the four lists of `SandboxRuntimeConfig` are of type `FilesystemPathEntry[]`, which is `(string | { path: string; literal: true })[]`, and `getConfig()` gives entries back in the form they were given. Code that reads an entry as a string stops compiling; `typeof entry === 'string' ? entry : entry.path` is its path. Three things change without a compile error. A list joined into text prints `[object Object]` for a marked entry. `getFsReadConfig()` and `getFsWriteConfig()` carry marked entries in `literalDenyOnly`, `literalAllowWithinDeny`, `literalAllowOnly` and `literalDenyWithinAllow`, each of which may be absent when it has no entries, so hand those objects on whole instead of rebuilding them from `denyOnly` and `allowOnly`; `writeRootsOf(getFsWriteConfig())` is every path the command may write. And an empty `denyOnly` no longer means that nothing is read-denied. `getDefaultWritePaths()` and `expandWindowsFsPaths()` take the lists as configured. `wrapCommandWithSandboxLinux()` takes every path it is handed for a name, as bubblewrap does, and skips a string in `allowOnly` that has such characters and is not the name of a path by the rule above.

src/sandbox/sandbox-utils.ts

@@ -698,10 +698,12 @@ function readRuleCovers(entry: FilesystemPathEntry): (p: string) => boolean {
   if (containsGlobCharsForPlatform(stripped)) {
     try {
       const regex = new RegExp(denyGlobRegex(normalizePathForSandbox(stripped)))
+      // V8 finds a pattern too large only when it first runs it.
+      regex.test('')
       return p => regex.test(p)
     } catch {
-      // Brackets that do not form a valid class. The entry may be a literal
-      // file name, so it is compared as one.
+      // Too large to compile. What it covers cannot be told, so it covers.
+      return () => true
     }
   }
   const rule = normalizePathForSandbox(stripped, { literal: true })
@@ -1203,12 +1205,45 @@ export function encodedCommandFromProxyUser(
  * - ? 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)
  *
+ * Brackets that would not compile, because a range in them runs backwards
+ * (`[user-id]`: `r-i`) or a second `[` is left open, are the text they are, as
+ * POSIX reads a `[` that opens no set. The rest of the pattern keeps its
+ * meaning. So what is returned always compiles, and no entry keeps a sandbox
+ * from starting.
+ *
  * Exported for testing and shared between macOS sandbox profiles and Linux glob expansion.
  */
 export function globToRegex(globPattern: string): string {
+  // Every other part is a bracket expression, up to the first `]` as a regular
+  // expression reads one. The last part holds each `[` that is left open.
+  const sources = globPattern.split(/(\[[^\]]*\])/).map(part => {
+    const source = globPartToRegex(part)
+    if (isRegex(source)) return source
+    logForDebugging(
+      `[Sandbox] The brackets in ${part} spell no set of characters, so they are read as text: ${globPattern}`,
+      { level: 'warn' },
+    )
+    return part
+      .split(/([[\]])/)
+      .map((text, i) => (i % 2 ? `\\${text}` : globPartToRegex(text)))
+      .join('')
+  })
+  return `^${sources.join('')}$`
+}
+
+function isRegex(source: string): boolean {
+  try {
+    new RegExp(source)
+    return true
+  } catch {
+    return false
+  }
+}
+
+/** {@link globToRegex} for one bracket expression, or for what lies between two. */
+function globPartToRegex(part: string): string {
   return (
-    '^' +
-    globPattern
+    part
       // Escape regex special characters (except glob chars * ? [ ])
       .replace(/[.^$+{}()|\\]/g, '\\$&')
       // Escape unclosed brackets (no matching ])
@@ -1220,8 +1255,7 @@ export function globToRegex(globPattern: string): string {
       .replace(/\?/g, '[^/]') // ? matches single character except /
       // Restore placeholders
       .replace(/__GLOBSTAR_SLASH__/g, '(.*/)?') // **/ matches zero or more dirs
-      .replace(/__GLOBSTAR__/g, '.*') + // ** matches anything including /
-    '$'
+      .replace(/__GLOBSTAR__/g, '.*') // ** matches anything including /
   )
 }
 
@@ -1448,11 +1482,18 @@ function globPieces(pattern: string, flags: string): GlobPiece[] | undefined {
     if (bracket !== undefined) {
       if (/[*?]/.test(bracket)) return undefined
       const source = sourceOf(bracket)
-      pieces.push(
-        new RegExp(`^${source}$`, flags).test('/')
-          ? { source, canBeSeparator: true }
-          : { source },
-      )
+      const regex = new RegExp(`^${source}$`, flags)
+      if (regex.test(bracket)) {
+        // Read as the text it is, where a separator is one like any other.
+        for (const [i, name] of source.split('/').entries()) {
+          if (i > 0) pieces.push('/')
+          pieces.push({ source: name })
+        }
+      } else {
+        pieces.push(
+          regex.test('/') ? { source, canBeSeparator: true } : { source },
+        )
+      }
     } else if (stars !== undefined) {
       // globToRegex takes `**/` first, so the last two of a run before a
       // separator go with it. What is left of the run is `**` pairs from the
@@ -1484,23 +1525,12 @@ function globPositions(
   normalizedPattern: string,
   flags: string,
 ): GlobPositions {
+  const regex = new RegExp(globToRegex(normalizedPattern), flags)
   const directoryForm = removeTrailingGlobSuffix(normalizedPattern)
-  let regex: RegExp
-  let directoryRegex: RegExp | undefined
-  try {
-    regex = new RegExp(globToRegex(normalizedPattern), flags)
-    directoryRegex =
-      directoryForm !== normalizedPattern
-        ? new RegExp(globToRegex(directoryForm), flags)
-        : undefined
-  } catch (err) {
-    // A pattern that is no regular expression would match nothing and deny
-    // nothing, silently: it is raised as the configuration error it is.
-    throw new SyntaxError(
-      `Glob pattern ${normalizedPattern} does not compile, so nothing can match it: ${err}`,
-      { cause: err },
-    )
-  }
+  const directoryRegex =
+    directoryForm !== normalizedPattern
+      ? new RegExp(globToRegex(directoryForm), flags)
+      : undefined
   const unsplit: GlobPositions = {
     splits: false,
     start: [0],

test/sandbox/brackets-that-spell-no-set.test.ts

@@ -0,0 +1,221 @@
+import { afterAll, beforeAll, describe, expect, it } from 'bun:test'
+import { spawnSync } from 'node:child_process'
+import { mkdirSync, realpathSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { dirname, join } from 'node:path'
+import { wrapCommandWithSandboxMacOS } from '../../src/sandbox/macos-sandbox-utils.js'
+import type { SandboxRuntimeConfig } from '../../src/sandbox/sandbox-config.js'
+import { SandboxManager } from '../../src/sandbox/sandbox-manager.js'
+import { bwrapCanNamespace } from '../helpers/bwrap-namespace.js'
+import { isLinux, isMacOS, isWindows } from '../helpers/platform.js'
+
+/**
+ * An entry with brackets that spell no set of characters, because a range in
+ * them runs backwards (`[user-id]`: `r-i`). A route folder is named so. The
+ * brackets are the text they are, in every list and on every platform, and
+ * the rest of the entry is the pattern it was.
+ */
+
+const ROOT = join(
+  isWindows ? tmpdir() : realpathSync(tmpdir()),
+  'brackets-no-set-' + Date.now(),
+)
+const TREE = join(ROOT, 'tree')
+
+/** The files a command tries, each under the letter it answers with. */
+const FILES = {
+  S: '[user-id]/secret',
+  E: '[user-id]/a.env',
+  D: 'deep/[user-id]/a.env',
+  // What a set of the letters between the brackets would match.
+  U: 'u/a.env',
+  P: 'plain',
+}
+const ALL = Object.keys(FILES).join('')
+const without = (letters: string): string =>
+  [...ALL].filter(letter => !letters.includes(letter)).join('')
+
+/** An entry beneath TREE, the files it covers, and the regular expression it is. */
+const ENTRIES: Array<[string, string, string]> = [
+  ['[z-a]', '', '\\[z-a\\]'],
+  ['[user-id]', 'SE', '\\[user-id\\]'],
+  ['[user-id]/secret', 'S', '\\[user-id\\]/secret'],
+  ['**/[user-id]/*.env', 'ED', '(.*/)?\\[user-id\\]/[^/]*\\.env'],
+]
+
+type Policy = Pick<SandboxRuntimeConfig, 'filesystem' | 'credentials'>
+const nothingListed = { denyRead: [], allowWrite: [], denyWrite: [] }
+
+/** A list: the policy with `entry` in it, and what is left of ALL to a command
+ *  that reads, or writes, when the entry covers `covered`. */
+const LISTS: Record<
+  string,
+  {
+    policy: (entry: string) => Policy
+    writes?: true
+    left: (covered: string) => string
+  }
+> = {
+  denyRead: {
+    policy: entry => ({ filesystem: { ...nothingListed, denyRead: [entry] } }),
+    left: without,
+  },
+  'credential deny': {
+    policy: entry => ({
+      filesystem: nothingListed,
+      credentials: { files: [{ path: entry, mode: 'deny' }] },
+    }),
+    left: without,
+  },
+  allowRead: {
+    policy: entry => ({
+      filesystem: { ...nothingListed, denyRead: [TREE], allowRead: [entry] },
+    }),
+    left: covered => covered,
+  },
+  denyWrite: {
+    policy: entry => ({
+      filesystem: { ...nothingListed, allowWrite: [TREE], denyWrite: [entry] },
+    }),
+    writes: true,
+    left: without,
+  },
+  allowWrite: {
+    policy: entry => ({
+      filesystem: { ...nothingListed, allowWrite: [entry] },
+    }),
+    writes: true,
+    left: covered => covered,
+  },
+}
+
+const stdoutOf = (wrapped: string): string =>
+  spawnSync(wrapped, { shell: true, encoding: 'utf8', timeout: 20000 }).stdout
+
+const CASES = Object.entries(LISTS).flatMap(([list, how]) =>
+  ENTRIES.map(([entry, covered, source]) => ({
+    list,
+    entry,
+    covered,
+    source,
+    ...how,
+  })),
+)
+
+describe.if(!isWindows)('brackets that spell no set of characters', () => {
+  beforeAll(() => {
+    for (const file of Object.values(FILES)) {
+      mkdirSync(dirname(join(TREE, file)), { recursive: true })
+      writeFileSync(join(TREE, file), 'content')
+    }
+  })
+
+  afterAll(() => {
+    rmSync(ROOT, { recursive: true, force: true })
+  })
+
+  describe.if((isLinux && bwrapCanNamespace()) || isMacOS)(
+    'what a command reads and writes',
+    () => {
+      beforeAll(async () => {
+        await SandboxManager.reset()
+        await SandboxManager.initialize({
+          network: { allowedDomains: [], deniedDomains: [] },
+          filesystem: nothingListed,
+        })
+      })
+
+      afterAll(async () => {
+        await SandboxManager.reset()
+      })
+
+      it.each(CASES)(
+        '$list $entry',
+        async ({ entry, covered, policy, writes, left }) => {
+          const attempts = Object.entries(FILES).map(([letter, file]) => {
+            const path = `'${join(TREE, file)}'`
+            // Opened to append and left as it is.
+            const attempt = writes ? `: >> ${path}` : `cat ${path} > /dev/null`
+            return `( ${attempt} ) 2> /dev/null && printf ${letter}`
+          })
+          const wrapped = await SandboxManager.wrapWithSandbox(
+            // 'ran' tells a command that was refused all from one that never ran.
+            `echo ran; ${attempts.join('; ')}`,
+            undefined,
+            policy(join(TREE, entry)),
+          )
+          // Linux takes no pattern in a write list, whatever it holds.
+          const applies = !(isLinux && writes && entry.includes('*'))
+          expect(stdoutOf(wrapped)).toBe(`ran\n${left(applies ? covered : '')}`)
+        },
+        60000,
+      )
+
+      it('an ordinary relative pattern, from inside such a folder', async () => {
+        const cwd = process.cwd()
+        process.chdir(join(TREE, '[user-id]'))
+        try {
+          const wrapped = await SandboxManager.wrapWithSandbox(
+            'echo ran; cat a.env secret',
+            undefined,
+            { filesystem: { ...nothingListed, denyRead: ['*.env'] } },
+          )
+          expect(stdoutOf(wrapped)).toBe('ran\ncontent')
+        } finally {
+          process.chdir(cwd)
+        }
+      }, 60000)
+
+      it.if(isLinux)('reports the lists of the session', async () => {
+        await SandboxManager.reset()
+        await SandboxManager.initialize({
+          network: { allowedDomains: [], deniedDomains: [] },
+          filesystem: {
+            ...nothingListed,
+            denyRead: [join(TREE, '**/[user-id]/*.env'), join(TREE, '[z-a]')],
+            allowRead: [join(TREE, '[user-id]/secret')],
+          },
+        })
+        const { denyOnly, allowWithinDeny } = SandboxManager.getFsReadConfig()
+        expect(denyOnly.sort()).toEqual([
+          join(TREE, FILES.E),
+          join(TREE, FILES.D),
+        ])
+        expect(allowWithinDeny).toEqual([join(TREE, FILES.S)])
+      })
+    },
+  )
+
+  // Read on any POSIX host. The builder compiles some of them itself, and
+  // Seatbelt is handed them all.
+  it.each(CASES)(
+    'macOS profile, $list $entry: every regex compiles',
+    ({ list, entry, source, policy }) => {
+      const { filesystem, credentials } = policy(join(TREE, entry))
+      const credentialDenyOnly = credentials?.files?.map(file => file.path)
+      const profile = wrapCommandWithSandboxMacOS({
+        command: 'true',
+        needsNetworkRestriction: false,
+        readConfig: {
+          denyOnly: [
+            ...(filesystem.denyRead as string[]),
+            ...(credentialDenyOnly ?? []),
+          ],
+          credentialDenyOnly,
+          allowWithinDeny: filesystem.allowRead as string[] | undefined,
+        },
+        writeConfig: {
+          allowOnly: filesystem.allowWrite as string[],
+          denyWithinAllow: filesystem.denyWrite as string[],
+        },
+      })
+      const regexes = [
+        ...profile.matchAll(/\(regex ("(?:[^"\\]|\\.)*")\)/g),
+      ].map(match => JSON.parse(match[1]!) as string)
+      for (const regex of regexes) expect(() => new RegExp(regex)).not.toThrow()
+      expect(regexes.map(regex => regex.split('/tree/')[1])).toContain(
+        source + (list.startsWith('allow') ? '$' : '(/.*)?$'),
+      )
+    },
+  )
+})