From ec2fc06e4ea3bae5bb9ce8eaee4045089f9ce98a Mon Sep 17 00:00:00 2001 From: Ron Leizrowice Date: Fri, 25 Sep 2026 22:16:47 +0000 Subject: [PATCH 1/4] Read a caller's path as a path where it exists, and let a caller mark 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. --- eslint.config.js | 21 ++ src/index.ts | 1 + src/sandbox/linux-sandbox-utils.ts | 31 +- src/sandbox/macos-sandbox-utils.ts | 118 +++++-- src/sandbox/path-entries.ts | 462 +++++++++++++++++++++++++++ src/sandbox/read-deny-glob.ts | 10 +- src/sandbox/sandbox-config.ts | 40 ++- src/sandbox/sandbox-manager.ts | 180 ++++++++--- src/sandbox/sandbox-schemas.ts | 21 ++ src/sandbox/sandbox-utils.ts | 131 ++++++-- src/sandbox/windows-sandbox-utils.ts | 48 ++- 11 files changed, 940 insertions(+), 123 deletions(-) create mode 100644 src/sandbox/path-entries.ts diff --git a/eslint.config.js b/eslint.config.js index b7783d1c9..95e8f4423 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -133,5 +133,26 @@ export default [ reportUnusedDisableDirectives: false, }, }, + { + // bubblewrap takes no patterns, so every path that reaches the Linux + // backend is a name. normalizePathForSandbox with one argument decides + // pattern or name from the characters of the path, which would read a + // name with `[`, `*` or `?` in it as a pattern again. + files: [ + 'src/sandbox/linux-sandbox-utils.ts', + 'src/sandbox/read-deny-glob.ts', + ], + rules: { + 'no-restricted-syntax': [ + 'error', + { + selector: + "CallExpression[callee.name='normalizePathForSandbox'][arguments.length=1]", + message: + 'Every path here is a name: pass { literal: true }, so that glob characters in it are not read as a pattern.', + }, + ], + }, + }, prettierRecommended, ] diff --git a/src/index.ts b/src/index.ts index d0a06954e..0884a9bb5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,6 +8,7 @@ export type { SandboxRuntimeConfig, NetworkConfig, FilesystemConfig, + FilesystemPathEntry, CredentialsConfig, CredentialFileConfig, CredentialEnvVarConfig, diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index 4579a9424..f5d531694 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -9,6 +9,7 @@ import { endianness, tmpdir } from 'node:os' import path, { join } from 'node:path' import { ripGrep, type RipgrepConfig } from '../utils/ripgrep.js' import { buildJavaToolOptions } from './java-proxy-agent.js' +import { readNamesOf, writeNamesOf } from './path-entries.js' import { generateProxyEnvVars, buildPosixGitSafeDirEnv, @@ -1771,7 +1772,7 @@ async function generateFilesystemArgs( let readAllowPathsMemo: string[] | undefined const readAllowPaths = (): string[] => (readAllowPathsMemo ??= (readConfig?.allowWithinDeny || []).map(p => - normalizePathForSandbox(p), + normalizePathForSandbox(p, { literal: true }), )) // What the read section mounts for one denyRead entry: a tmpfs (directory) // or a /dev/null mask (anything else) on the entry itself; nothing when it @@ -1867,7 +1868,7 @@ async function generateFilesystemArgs( if (!readConfig) return [] const entries: string[] = [] for (const p of readConfig.denyOnly || []) { - if (normalizePathForSandbox(p) !== '/') { + if (normalizePathForSandbox(p, { literal: true }) !== '/') { entries.push(p) continue } @@ -1921,7 +1922,7 @@ async function generateFilesystemArgs( let readDenyPlanMemo: ReadDenyPlanEntry[] | undefined const readDenyPlan = (): ReadDenyPlanEntry[] => (readDenyPlanMemo ??= readDenyEntries() - .map(p => normalizePathForSandbox(p)) + .map(p => normalizePathForSandbox(p, { literal: true })) .sort((a, b) => canonicalDepth(a) - canonicalDepth(b)) .map(normalizedPath => { const mount = readDenyMountOf(normalizedPath) @@ -1949,10 +1950,10 @@ async function generateFilesystemArgs( // Allow writes to specific paths for (const pathPattern of writeConfig.allowOnly || []) { - // normalizePathForSandbox already strips a trailing slash from every - // spelling it does not take for a glob; this strip covers the ones it - // exempts — a literal directory named with glob characters, spelled - // '/[id]/'. Allow paths are recorded slash-free because every + // Every path here is a name, and normalizePathForSandbox strips a + // trailing slash from a name, so the strip below has nothing left to + // do; it stays as the guard of what is recorded. Allow paths are + // recorded slash-free because every // downstream comparison — the deny loop's within-allowlist gate, // findSymlinkInPath's mask scoping, the emission filter's re-expose // check, the denyRead re-bind and its allowRead skip, and the stub-skip @@ -1962,7 +1963,7 @@ async function generateFilesystemArgs( // instead of per-predicate. ('/' itself is kept; the empty spelling an // empty $HOME expands '~' to is NOT the root, and falls out at the // existence check below.) - const normalized = normalizePathForSandbox(pathPattern) + const normalized = normalizePathForSandbox(pathPattern, { literal: true }) const normalizedPath = normalized === '' ? '' : normalized.replace(/\/+$/, '') || '/' @@ -2154,7 +2155,7 @@ async function generateFilesystemArgs( // directory (the re-check below); an emitted one missing from the record // only costs a spurious abort. for (const pathPattern of denyPaths) { - const rawPath = normalizePathForSandbox(pathPattern) + const rawPath = normalizePathForSandbox(pathPattern, { literal: true }) if (rawPath.startsWith('/dev/')) { continue } @@ -2303,7 +2304,7 @@ async function generateFilesystemArgs( return covered } for (const pathPattern of denyPaths) { - const rawPath = normalizePathForSandbox(pathPattern) + const rawPath = normalizePathForSandbox(pathPattern, { literal: true }) // Skip /dev/* paths since --dev /dev already handles them if (rawPath.startsWith('/dev/')) { @@ -2963,8 +2964,6 @@ export async function wrapCommandWithSandboxLinux( proxyAuthToken, caCertPath, javaAgentJarPath, - readConfig, - writeConfig, unsetEnvVars, setEnvVars, maskedFileBinds, @@ -2982,6 +2981,14 @@ export async function wrapCommandWithSandboxLinux( observeSocketPath, abortSignal, } = params + // bubblewrap takes no patterns, so every path that reaches this backend is + // a name: the manager has expanded or dropped what was a pattern. The + // literal lists are therefore more entries of the lists they belong to, and + // are folded into them here, once, so that nothing below can read a list + // and miss them. A caller of this function may still hand it a pattern: + // in `allowOnly` one that is no name is dropped by the fold. + const readConfig = readNamesOf(params.readConfig) + const writeConfig = writeNamesOf(params.writeConfig) // Determine if we have restrictions to apply // Read: denyOnly pattern - empty array means no restrictions diff --git a/src/sandbox/macos-sandbox-utils.ts b/src/sandbox/macos-sandbox-utils.ts index 4c6a3a4df..08982949f 100644 --- a/src/sandbox/macos-sandbox-utils.ts +++ b/src/sandbox/macos-sandbox-utils.ts @@ -19,6 +19,7 @@ import { getDangerousDirectories, } from './sandbox-utils.js' import { shouldIgnoreViolation } from './sandbox-violation-store.js' +import { literalReadings, type PathListKind } from './path-entries.js' import type { FsReadRestrictionConfig, @@ -162,16 +163,16 @@ function pathFilter(entry: PathEntry): string { /** * Compiles `entry`'s pattern with one of the shared string compilers in - * `sandbox-utils.ts`. An entry the library anchored at a directory - * (`anchor`) keeps that directory as a literal — the cwd is a name on disk - * and may itself contain `[`, `*` or `?` — so only the tail below it goes - * through the compiler, and the anchor is spliced back in escaped. Every - * compiler there returns '^…$'. + * `sandbox-utils.ts`. An entry anchored at a directory (`anchor`) keeps + * that directory as a literal — the cwd, or a directory a caller's pattern + * lies beneath, is a name on disk and may itself contain `[`, `*` or `?` — + * so only the tail below it goes through the compiler, and the anchor is + * spliced back in escaped. Every compiler there returns '^…$'. * * This is the whole difference between the entry-taking wrappers here and * the string-taking compilers they call: those see a spelling in which - * every character is glob syntax, which is right for what a caller wrote - * and wrong for what the library computed. + * every character is glob syntax, which is right for the pattern reading + * of what a caller wrote and wrong for a name on disk. */ function anchorRegex( entry: GlobPathEntry, @@ -251,9 +252,11 @@ export type PathEntry = glob: true path: string /** - * Set when the library anchored this glob at a directory of its own - * (the cwd, for the mandatory `**\/` patterns): the prefix of - * `path` that is a literal path rather than part of the pattern. + * Set when this glob is anchored at a directory that is a name on + * disk: the cwd, for the mandatory `**\/` patterns, or a + * directory that exists and holds glob characters, for a caller's + * pattern beneath it. The prefix of `path` that is a literal path + * rather than part of the pattern. */ anchor?: string } @@ -267,12 +270,16 @@ function anchoredGlobEntry(anchor: string, pattern: string): PathEntry { } /** - * A spelling that came from the caller's config: `*`, `?` and `[…]` in - * what the caller wrote are the glob syntax it asked for. The decision is - * made on that raw spelling, because resolving a relative or `~` path can - * splice in a cwd or home directory whose own name contains those - * characters — read back, they would turn the caller's `secrets` into a - * pattern that never matches the directory it was resolved to. + * A spelling that came from the caller's config, read by its characters: + * with `*`, `?` or `[…]` in what the caller wrote it is a pattern. The + * decision is made on that raw spelling, because resolving a relative or + * `~` path can splice in a cwd or home directory whose own name contains + * those characters — read back, they would turn the caller's `secrets` into + * a pattern that never matches the directory it was resolved to. + * + * Those characters may also be part of a name that exists, and then the + * spelling is that name as well: {@link alsoReadAs} gives the readings this + * one leaves out. */ function toPathEntry(pathPattern: string): PathEntry { return containsGlobChars(pathPattern) @@ -289,7 +296,8 @@ function toPathEntry(pathPattern: string): PathEntry { * `subpath` filter whatever characters it contains — sniffed as a pattern, * a component like `a[b` would become a character class and the filter * would no longer match the path it was built from (the rule would be - * inert). Never use this for a caller's spelling. + * inert). A path the caller marked literal is the same thing and comes + * through here too. Never use this for a spelling the caller did not mark. */ function toLiteralPathEntry(literalPath: string): PathEntry { return { @@ -298,6 +306,36 @@ function toLiteralPathEntry(literalPath: string): PathEntry { } } +/** + * The readings a caller's spellings have beside {@link toPathEntry}'s: the + * name a spelling with glob characters also is, and its pattern beneath a + * directory that holds such characters in its own name. Empty for every + * spelling no part of which is such a name on disk. + */ +function alsoReadAs( + spellings: readonly string[] | undefined, + kind: PathListKind, +): PathEntry[] { + return (spellings ?? []).flatMap(spelling => literalReadings(spelling, kind)) +} + +/** + * One of the caller's lists as entries: each spelling as its characters + * read, then the other readings of the spellings, then the paths that + * arrived marked literal. + */ +function callerEntries( + kind: PathListKind, + spellings: readonly string[] | undefined, + literalPaths: readonly string[] | undefined, +): PathEntry[] { + return [ + ...(spellings ?? []).map(toPathEntry), + ...alsoReadAs(spellings, kind), + ...(literalPaths ?? []).map(toLiteralPathEntry), + ] +} + /** Does a subtree-extended deny glob regex cover `entry`'s region? */ function denyGlobCovers(denyRegex: RegExp, entry: PathEntry): boolean { return denyRegex.test(entrySamplePath(entry)) @@ -322,7 +360,7 @@ interface ResolvedReadConfig { function resolveReadConfig( config: FsReadRestrictionConfig, - writeAllowPaths: readonly string[] | undefined, + writeRoots: readonly PathEntry[], /** * Read denies the library resolved itself — the masked credential files, * which macOS degrades to a deny. Already literal: each names the file @@ -331,12 +369,19 @@ function resolveReadConfig( libraryDenies: readonly PathEntry[], ): ResolvedReadConfig { return { - denies: [...(config.denyOnly || []).map(toPathEntry), ...libraryDenies], + denies: [ + ...callerEntries('deny', config.denyOnly, config.literalDenyOnly), + ...libraryDenies, + ], // Non-glob spellings arrive slash-free from normalizePathForSandbox — // the nested-deny re-emit matches by `path + '/'` prefix, which a // preserved trailing slash would defeat ('//'). - allows: (config.allowWithinDeny || []).map(toPathEntry), - writeRoots: (writeAllowPaths || []).map(toPathEntry), + allows: callerEntries( + 'allow', + config.allowWithinDeny, + config.literalAllowWithinDeny, + ), + writeRoots: [...writeRoots], } } @@ -863,13 +908,25 @@ function generateReadRules( return rules } +/** + * What a write config allows, as entries. Decided once per profile and + * handed to the read section and the write section alike, so the two cannot + * see different answers for one entry. + */ +function writeRootEntries( + config: FsWriteRestrictionConfig | undefined, +): PathEntry[] { + return callerEntries('allow', config?.allowOnly, config?.literalAllowOnly) +} + /** * Generate filesystem write rules for sandbox profile */ function generateWriteRules( config: FsWriteRestrictionConfig | undefined, logTag: string, - allowGitConfig = false, + allowGitConfig: boolean, + writeRoots: readonly PathEntry[], ): string[] { if (!config) { return [`(allow file-write*)`] @@ -879,8 +936,8 @@ function generateWriteRules( // Generate allow rules const allowFilters = new Set() - for (const pathPattern of config.allowOnly || []) { - allowFilters.add(pathFilter(toPathEntry(pathPattern))) + for (const entry of writeRoots) { + allowFilters.add(pathFilter(entry)) } rules.push(...renderRule('allow', ['file-write*'], allowFilters, logTag)) @@ -889,8 +946,10 @@ function generateWriteRules( // the mandatory entries carry their own literal/glob split. const denyEntries = [ ...(config.denyWithinAllow || []).map(toPathEntry), + ...(config.literalDenyWithinAllow ?? []).map(toLiteralPathEntry), ...macGetMandatoryDenyEntries(allowGitConfig), ] + denyEntries.push(...alsoReadAs(config.denyWithinAllow, 'deny')) const { groups, rest: ungrouped } = groupLiteralDenyPaths(denyEntries) const groupFilters = groups.map(literalDenyGroupFilters) @@ -1231,6 +1290,7 @@ function generateSandboxProfile({ } profile.push('') + const writeRoots = writeRootEntries(writeConfig) // Read rules // Pass write-allowed paths so that move-blocking deny rules in the read section // can be overridden for paths where file deletion should be permitted. @@ -1238,7 +1298,7 @@ function generateSandboxProfile({ readConfig || libraryDenyEntries.length > 0 ? resolveReadConfig( readConfig ?? { denyOnly: [] }, - writeConfig?.allowOnly, + writeRoots, libraryDenyEntries, ) : undefined @@ -1248,7 +1308,9 @@ function generateSandboxProfile({ // Write rules profile.push('; File write') - profile.push(...generateWriteRules(writeConfig, logTag, allowGitConfig)) + profile.push( + ...generateWriteRules(writeConfig, logTag, allowGitConfig, writeRoots), + ) // Read-denied paths inside write roots: the read section's unlink/create // re-allow for write roots (and the write section's file-write* allows) @@ -1347,7 +1409,9 @@ export function wrapCommandWithSandboxMacOS( // Read: denyOnly pattern - empty array means no restrictions // Write: allowOnly pattern - undefined means no restrictions, any config means restrictions const hasReadRestrictions = - (readConfig && readConfig.denyOnly.length > 0) || + (readConfig && + (readConfig.denyOnly.length > 0 || + (readConfig.literalDenyOnly?.length ?? 0) > 0)) || libraryDenyEntries.length > 0 const hasWriteRestrictions = writeConfig !== undefined const hasEnvRestrictions = diff --git a/src/sandbox/path-entries.ts b/src/sandbox/path-entries.ts new file mode 100644 index 000000000..c55b546a6 --- /dev/null +++ b/src/sandbox/path-entries.ts @@ -0,0 +1,462 @@ +/** + * How an entry of `denyRead`, `allowRead`, `allowWrite` or `denyWrite` is + * read: as the pattern its characters spell, as the path they name, or both. + * + * An entry is a pattern when it holds `*`, `?`, `[` or `]`. A directory may + * hold those characters in its own name (`[WIP] project`, `notes (draft?)`), + * and a caller that lists paths inside one means the paths. So an entry with + * those characters keeps the pattern reading it always had, and is ALSO read + * as what it names wherever the part of it that holds the characters exists + * on disk. A caller that wants the name and nothing else marks the entry + * `{ path, literal: true }`. + * + * The decision is made here and nowhere else, once per entry per wrap, by + * the first stage that needs it: the manager on Linux (bubblewrap takes no + * patterns, so the manager is where they are expanded), the profile builder + * on macOS, `expandWindowsFsPaths` on Windows. What comes out is carried with + * the entry from then on; no later stage looks at its characters again. + */ + +import * as fs from 'node:fs' +import * as path from 'node:path' +import { getPlatform } from '../utils/platform.js' +import { + collapseInteriorSpellings, + containsGlobChars, + containsGlobCharsForPlatform, + expandTilde, + expandWindowsEnvRefs, + isAbsenceErrno, + isUncPath, + markedLiteralPath, + normalizePathForSandbox, + removeTrailingGlobSuffix, + stripExtendedPathPrefix, + toForwardSlashes, +} from './sandbox-utils.js' +import type { FilesystemPathEntry } from './sandbox-config.js' +import type { + FsReadRestrictionConfig, + FsWriteRestrictionConfig, +} from './sandbox-schemas.js' + +export { markedLiteralPath } + +/** + * Which way a list cuts. A reading added to a deny can only deny more, so a + * deny takes every reading that may hold. A reading added to an allow opens + * something, so an allow takes one only for a path that is there exactly as + * spelled. + */ +export type PathListKind = 'allow' | 'deny' + +/** + * One reading of an entry. The same shape as the macOS builder's `PathEntry`, + * so a reading is one without conversion. + */ +export type PathReading = + /** The entry is the name of this path. */ + | { glob: false; path: string } + /** + * The entry is a pattern beneath `anchor`, a directory taken as the name it + * is. `path` is `anchor` followed by the tail, and only the tail is + * pattern. + */ + | { glob: true; path: string; anchor: string } + +/** + * The spellings and the marked paths of a list, apart. Throws on an entry + * that is neither: `initialize()` does not run the schema, and reading + * `{ path }` either way would be a guess about a deny. + */ +export function splitPathEntries( + entries: readonly FilesystemPathEntry[] | undefined, +): { spelled: string[]; marked: string[] } { + const spelled: string[] = [] + const marked: string[] = [] + for (const entry of entries ?? []) { + if (typeof entry === 'string') spelled.push(entry) + else marked.push(markedLiteralPath(entry)) + } + return { spelled, marked } +} + +/** The spellings among the entries of a list: what may be a pattern. */ +export function spelledOf( + entries: readonly FilesystemPathEntry[] | undefined, +): string[] { + return splitPathEntries(entries).spelled +} + +/** + * A key that tells entries apart by value: a marked entry is a new object + * after every clone of the config, and `/x` is not `{ path: '/x' }`. + */ +export function pathEntryKey(entry: FilesystemPathEntry): string { + return typeof entry === 'string' + ? `s:${entry}` + : `l:${markedLiteralPath(entry)}` +} + +/** + * The spelling made absolute the way {@link normalizePathForSandbox} does it + * for a pattern, with no symlink resolved and a trailing separator kept: the + * path the caller wrote, which is what the disk is asked about. Undefined + * for a Windows UNC path, which is never probed (see {@link isUncPath}). + */ +function spelledAbsolute(spelling: string): string | undefined { + const onWindows = getPlatform() === 'windows' + let spelled = spelling + if (onWindows) { + spelled = stripExtendedPathPrefix(expandWindowsEnvRefs(spelled)) + if (isUncPath(spelled)) return undefined + } + const expanded = expandTilde(spelled) + const absolute = + expanded === spelled && !path.isAbsolute(spelled) + ? path.resolve(process.cwd(), spelled) + : expanded + return onWindows + ? toForwardSlashes(absolute) + : collapseInteriorSpellings(absolute) +} + +/** + * Whether something may be at `p`, for a deny. A symbolic link counts, + * dangling or not: the name is taken. So does a path that could not be + * looked at, because unreadable now is not absent: a command running as the + * same user can make a parent unsearchable and undo that from inside the + * next sandbox, and a deny it could switch off that way would be no deny. + */ +function mayBeThere(p: string): boolean { + try { + fs.lstatSync(p) + return true + } catch (err) { + return !isAbsenceErrno(err) + } +} + +/** + * Whether `p` is there and is not a symbolic link, for an allow. A link with + * the name an entry spells is not the path the caller meant: whoever can + * write the directory that holds it can plant one, and the allow would then + * open what the link points at. Anything that cannot be looked at is not + * there. + */ +function isThereUnlinked(p: string): boolean { + try { + return !fs.lstatSync(p).isSymbolicLink() + } catch { + return false + } +} + +/** Whether a pattern can be walked beneath `p`, under the rule of `kind`. */ +function isDirectoryFor(kind: PathListKind, p: string): boolean { + try { + return kind === 'allow' + ? fs.lstatSync(p).isDirectory() + : fs.statSync(p).isDirectory() + } catch (err) { + return kind === 'deny' && !isAbsenceErrno(err) + } +} + +/** + * The readings a caller's spelling has BESIDE the one its characters give + * it. Empty for a spelling that does not read as a pattern (it is a name + * already) and for a pattern none of whose glob characters are part of a + * name on disk, which is every ordinary pattern: that one costs a single + * `lstat`. + * + * A trailing `/**` is set aside first. 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 NAME it spells. What + * follows the last of them need not exist. + * - When a pattern follows an existing one, the entry is also that pattern + * BENEATH the directory, which is taken as the name it is. There is one + * such reading for each of the existing components, the longest first, so + * creating or removing a directory adds or removes a reading and never + * turns one into another. + * + * Whether a spelling is a pattern at all is decided on what the caller + * wrote (`isPattern`, the platform's own check by default), as it always + * was. Which components hold glob characters is decided by + * {@link containsGlobChars} on every platform, because those are the + * characters the pattern compiler and the walk take for syntax. + * + * For a deny, a component exists when something has its name, a symbolic + * link included, or when it cannot be looked at. For an allow it exists only + * as itself: no component from the first with glob characters on may be a + * symbolic link, and none may be beyond looking at. That goes for a name to + * its end and for a pattern up to its first pattern component, from where + * the walk of an allow goes through no link. + */ +export function literalReadings( + spelling: string, + kind: PathListKind, + opts: { isPattern?: (spelling: string) => boolean } = {}, +): PathReading[] { + const isPattern = opts.isPattern ?? containsGlobCharsForPlatform + const stripped = removeTrailingGlobSuffix(spelling) + if (!isPattern(stripped)) return [] + const spelled = spelledAbsolute(stripped) + if (spelled === undefined) return [] + + const parts = spelled.split('/') + const globAt = parts.flatMap((part, i) => + containsGlobChars(part) ? [i] : [], + ) + const first = globAt[0] + const last = globAt[globAt.length - 1] + if (first === undefined || last === undefined) return [] + const prefix = (i: number): string => parts.slice(0, i + 1).join('/') || '/' + + const isThere = kind === 'deny' ? mayBeThere : isThereUnlinked + let reach = first - 1 + while (reach < last && isThere(prefix(reach + 1))) reach++ + if (reach < first) return [] + + const readings: PathReading[] = [] + if ( + reach === last && + (kind === 'deny' || restIsUnlinked(parts.length - 1, last, prefix)) + ) { + readings.push({ + glob: false, + path: normalizePathForSandbox(stripped, { literal: true }), + }) + } + + // The tail keeps a trailing separator, which makes a pattern match nothing + // (`/x/*/`): stripped, an allow spelled so would start matching every + // child. The `/**` set aside above goes back on, for the walk's directory + // form. + const setAside = stripped === spelling ? '' : '/**' + for (let n = globAt.length - 2; n >= 0; n--) { + const at = globAt[n]! + if (at > reach) continue + // As far as the directory's own name goes on: up to the next component + // with glob characters, or to where the disk ends. + const next = globAt[n + 1]! + let end = Math.min(next - 1, reach) + while (end >= at && !isDirectoryFor(kind, prefix(end))) end-- + if (end < at) continue + // The walk starts at the anchor followed by what the tail spells before + // its first pattern component, and resolves that: a link there would + // have an allow list what the link points at. + if (kind === 'allow' && !restIsUnlinked(next - 1, end, prefix)) continue + const anchor = normalizePathForSandbox(prefix(end), { literal: true }) + readings.push({ + glob: true, + anchor, + path: anchor + spelled.slice(prefix(end).length) + setAside, + }) + } + return readings +} + +/** + * Whether no component after `from`, up to `to`, is a symbolic link or + * beyond looking at. What is not there yet is neither. + */ +function restIsUnlinked( + to: number, + from: number, + prefix: (i: number) => string, +): boolean { + for (let i = from + 1; i <= to; i++) { + try { + if (fs.lstatSync(prefix(i)).isSymbolicLink()) return false + } catch (err) { + return isAbsenceErrno(err) + } + } + return true +} + +/** Whether a spelling that reads as a pattern is also the name of a path. */ +export function hasNameReading(spelling: string, kind: PathListKind): boolean { + return literalReadings(spelling, kind).some(reading => !reading.glob) +} + +/** + * What a Linux read entry resolves to: what `expandGlob` finds for the + * pattern, exactly as it returns it, then what the entry's other readings + * add. `name` is the entry as a name, in the caller's spelling like every + * other name in these lists. + */ +export function withOtherReadings( + spelling: string, + name: string, + kind: PathListKind, + expandGlob: (pattern: string, anchor?: string) => string[], +): string[] { + const expansion = expandGlob(spelling) + const seen = new Set(expansion) + const added: string[] = [] + for (const reading of literalReadings(spelling, kind)) { + const found = reading.glob + ? expandGlob(reading.path, reading.anchor) + : [name] + for (const p of found) { + if (seen.has(p)) continue + seen.add(p) + added.push(p) + } + } + return [...expansion, ...added] +} + +/** + * The read rules `getDefaultWritePaths()` is given for a policy: its + * entries, the credential denies, and beside each spelling the name it also + * is, handed over as a marked path. + */ +export function readRulesOf( + denyRead: readonly FilesystemPathEntry[], + allowRead: readonly FilesystemPathEntry[] | undefined, + credentialDenies: readonly string[], +): { + denyRead: FilesystemPathEntry[] + allowRead: FilesystemPathEntry[] | undefined +} { + const withNames = ( + entries: readonly FilesystemPathEntry[], + kind: PathListKind, + ): FilesystemPathEntry[] => [ + ...entries, + ...spelledOf(entries).flatMap(spelling => + literalReadings(spelling, kind).flatMap( + (reading): FilesystemPathEntry[] => + reading.glob ? [] : [{ path: reading.path, literal: true }], + ), + ), + ] + return { + denyRead: withNames([...denyRead, ...credentialDenies], 'deny'), + allowRead: + allowRead === undefined ? undefined : withNames(allowRead, 'allow'), + } +} + +/** `lists` without the keys whose list is empty. */ +function nonEmptyLists>( + lists: T, +): Partial { + return Object.fromEntries( + Object.entries(lists).filter(([, list]) => list.length > 0), + ) as Partial +} + +/** + * The `literal…` lists of a read config: the marked entries of the two read + * lists. A key is present only when its list is not empty, so a config + * without marked entries gives the object it always gave. + */ +export function literalReadLists( + denyRead: readonly FilesystemPathEntry[] | undefined, + allowRead: readonly FilesystemPathEntry[] | undefined, +): Pick { + return nonEmptyLists({ + literalDenyOnly: splitPathEntries(denyRead).marked, + literalAllowWithinDeny: splitPathEntries(allowRead).marked, + }) +} + +/** The `literal…` lists of a write config; see {@link literalReadLists}. */ +export function literalWriteLists( + allowWrite: readonly FilesystemPathEntry[] | undefined, + denyWrite: readonly FilesystemPathEntry[] | undefined, +): Pick< + FsWriteRestrictionConfig, + 'literalAllowOnly' | 'literalDenyWithinAllow' +> { + return nonEmptyLists({ + literalAllowOnly: splitPathEntries(allowWrite).marked, + literalDenyWithinAllow: splitPathEntries(denyWrite).marked, + }) +} + +/** + * Every path a write config allows, whichever list it came in on. Whoever + * asks "may the command write here" asks this, not `allowOnly` alone: a + * path the caller marked literal is in `literalAllowOnly` and nowhere else. + */ +export function writeRootsOf( + config: Pick, +): string[] { + return [...(config.allowOnly || []), ...(config.literalAllowOnly ?? [])] +} + +/** + * The paths bound back over a read deny: the read allows and the write + * allows, the marked ones among them. A deny glob's matches are collapsed + * against these, and one left out loses the matches beneath it their mask. + */ +export function reExposedBy( + expandedAllowRead: readonly string[], + allowRead: readonly FilesystemPathEntry[] | undefined, + writeConfig: FsWriteRestrictionConfig, +): string[] { + return [ + ...expandedAllowRead, + ...splitPathEntries(allowRead).marked, + ...writeRootsOf(writeConfig), + ] +} + +/** + * A read config for a backend to which every path is a name (Linux): the + * literal lists folded into the lists they are more entries of, and gone as + * keys, so that whatever reads `denyOnly` has read them all. + */ +export function readNamesOf( + config: FsReadRestrictionConfig | undefined, +): FsReadRestrictionConfig | undefined { + if (!config) return config + const { literalDenyOnly, literalAllowWithinDeny, ...rest } = config + return { + ...rest, + denyOnly: [...(config.denyOnly || []), ...(literalDenyOnly ?? [])], + ...(config.allowWithinDeny || literalAllowWithinDeny + ? { + allowWithinDeny: [ + ...(config.allowWithinDeny ?? []), + ...(literalAllowWithinDeny ?? []), + ], + } + : {}), + } +} + +/** + * The same for a write config; see {@link readNamesOf}. A string of + * `allowOnly` that holds glob characters is kept only when it is also the + * name of a path (see {@link literalReadings}), the condition on which the + * manager keeps one. Any other is a pattern that a caller of the backend + * handed it, and no name: resolving a name folds a `..` by the text, so + * `/home/*\/../.ssh` would make `/home/.ssh` writable where nothing is named + * `*`. A path the caller marked is resolved like any name. + */ +export function writeNamesOf( + config: FsWriteRestrictionConfig | undefined, +): FsWriteRestrictionConfig | undefined { + if (!config) return config + const { literalAllowOnly, literalDenyWithinAllow, ...rest } = config + return { + ...rest, + allowOnly: writeRootsOf({ + allowOnly: (config.allowOnly || []).filter( + p => !containsGlobChars(p) || hasNameReading(p, 'allow'), + ), + literalAllowOnly, + }), + denyWithinAllow: [ + ...(config.denyWithinAllow || []), + ...(literalDenyWithinAllow ?? []), + ], + } +} diff --git a/src/sandbox/read-deny-glob.ts b/src/sandbox/read-deny-glob.ts index 342805143..c65d26f67 100644 --- a/src/sandbox/read-deny-glob.ts +++ b/src/sandbox/read-deny-glob.ts @@ -63,7 +63,8 @@ function collapseReadDenyLocations({ * `**\/build/**` yields one mount per `build/` directory. A match reached * through a symlink is listed where it really lives, and a directory the walk * could not list is denied whole. Sorted, so an ancestor precedes its - * descendants. + * descendants. With `anchor`, the pattern is walked beneath that directory, + * which is taken as the name it is (see `ExpandGlobOptions.anchor`). * * @param unlistableDirs - receives the returned locations that hide something * the walk could not enumerate, whether by being that directory or by @@ -75,10 +76,12 @@ export function expandReadDenyGlobLinux( globPattern: string, reExposedPaths: readonly string[], unlistableDirs?: Set, + anchor?: string, ): string[] { const walk = walkGlobPattern(globPattern, { withDirectoryForm: true, followSymlinkedDirectories: true, + anchor, }) // Where a path the walk reported really lives: the denyRead loop mounts an // entry there, whatever spelling named it. @@ -156,8 +159,11 @@ export function expandReadDenyGlobLinux( addLocation(standIn, candidate) } + // Allow paths, which reach this backend as names whatever they hold. const reExposed = new Set( - reExposedPaths.flatMap(p => pathSpellings(normalizePathForSandbox(p))), + reExposedPaths.flatMap(p => + pathSpellings(normalizePathForSandbox(p, { literal: true })), + ), ) const mounts = collapseReadDenyLocations({ locations, diff --git a/src/sandbox/sandbox-config.ts b/src/sandbox/sandbox-config.ts index 6a2b32767..3769dba4d 100644 --- a/src/sandbox/sandbox-config.ts +++ b/src/sandbox/sandbox-config.ts @@ -134,6 +134,29 @@ const addressRangeSchema = z */ const filesystemPathSchema = z.string().min(1, 'Path cannot be empty') +/** + * An entry of `denyRead`, `allowRead`, `allowWrite` or `denyWrite`: a + * spelling, which is a pattern when it reads as one, or a path marked + * literal, which is never a pattern whether or not it exists. The mark is + * `literal: true` and nothing else: an object without it, or with a key + * this does not know, is refused rather than read one way or the other. + */ +const filesystemPathEntrySchema = z.union( + [ + filesystemPathSchema, + z.object({ path: filesystemPathSchema, literal: z.literal(true) }).strict(), + ], + { + errorMap: (issue, ctx) => + issue.code === z.ZodIssueCode.invalid_union + ? { + message: + 'Expected a path, or { "path": "", "literal": true }', + } + : { message: ctx.defaultError }, + }, +) + /** * Schema for an absolute path to an external binary. * Relative paths are rejected to prevent PATH/CWD-based hijacking — these @@ -315,7 +338,7 @@ const extractPatternSchema = z.string().superRefine((val, ctx) => { export const CredentialFileConfigSchema = z.object({ path: filesystemPathSchema.describe( 'Path to a credential file or directory. Supports the same path forms as ' + - 'filesystem.denyRead (absolute paths and ~ expansion).', + 'a string in filesystem.denyRead (absolute paths and ~ expansion).', ), mode: credentialModeSchema.describe('Access mode for this path'), extract: extractPatternSchema @@ -901,19 +924,21 @@ export const FilesystemConfigSchema = z.object({ 'is trusted with full host filesystem access. Network and credential-env restrictions ' + 'still apply. On Linux, /dev is still replaced by the bwrap minimal devtmpfs.', ), - denyRead: z.array(filesystemPathSchema).describe('Paths denied for reading'), + denyRead: z + .array(filesystemPathEntrySchema) + .describe('Paths denied for reading'), allowRead: z - .array(filesystemPathSchema) + .array(filesystemPathEntrySchema) .optional() .describe( 'Paths to re-allow reading within denied regions (takes precedence over denyRead). ' + 'Use with denyRead to deny a broad region then allow back specific subdirectories.', ), allowWrite: z - .array(filesystemPathSchema) + .array(filesystemPathEntrySchema) .describe('Paths allowed for writing'), denyWrite: z - .array(filesystemPathSchema) + .array(filesystemPathEntrySchema) .describe('Paths denied for writing (takes precedence over allowWrite)'), allowGitConfig: z .boolean() @@ -1195,10 +1220,14 @@ export const SandboxRuntimeConfigSchema = z // under it is not a hole. const fsEnforced = !cfg.filesystem.disabled if (fsEnforced) { + // A marked entry is never a glob, so a separator at its end is the + // end of a name. for (const [idx, p] of cfg.filesystem.denyRead.entries()) { + if (typeof p !== 'string') continue addInertSlashedDenyGlobIssue(p, ['filesystem', 'denyRead', idx], ctx) } for (const [idx, p] of cfg.filesystem.denyWrite.entries()) { + if (typeof p !== 'string') continue addInertSlashedDenyGlobIssue(p, ['filesystem', 'denyWrite', idx], ctx) } } @@ -1459,6 +1488,7 @@ export type MitmProxyConfig = z.infer export type ParentProxyConfig = z.infer export type NetworkConfig = z.infer export type FilesystemConfig = z.infer +export type FilesystemPathEntry = z.infer export type CredentialMode = z.infer export type CredentialFileConfig = z.infer export type CredentialEnvVarConfig = z.infer< diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index 91daa448a..51ae92ab0 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -1,3 +1,15 @@ +import { + hasNameReading, + literalReadLists, + literalWriteLists, + pathEntryKey, + reExposedBy, + readRulesOf, + spelledOf, + withOtherReadings, + writeRootsOf, + type PathListKind, +} from './path-entries.js' import { createHttpProxyServer } from './http-proxy.js' import { createSocksProxyServer } from './socks-proxy.js' import type { SocksProxyWrapper } from './socks-proxy.js' @@ -30,6 +42,7 @@ import * as fs from 'fs' import { randomBytes } from 'node:crypto' import type { CredentialsConfig, + FilesystemPathEntry, SandboxRuntimeConfig, SeccompConfig, } from './sandbox-config.js' @@ -699,9 +712,10 @@ async function initialize( if (enableLogMonitor && getPlatform() === 'linux') { // The monitor compares paths the kernel reported, so its lists are // expanded the way the wrapper expands them (getFsWriteConfig folds in - // the default write paths and drops the globs bwrap cannot take; - // normalizePathForSandbox resolves `~`, relative spellings and symlinks), - // plus the built-in write denies the wrapper always applies. + // the default write paths and drops the patterns bwrap cannot take, so + // what is left are names; normalizePathForSandbox resolves `~`, relative + // spellings and symlinks), plus the built-in write denies the wrapper + // always applies. // It does not reproduce the wrapper's existence and boundary-symlink // filters, nor the ripgrep scan for nested dangerous paths; and it still // reports writes bwrap permits through `--dev`, `--proc` and the tmpfs @@ -719,12 +733,15 @@ async function initialize( // out because it is read-denied sits under that deny's writable // tmpfs, so a write there is permitted and is not a violation. allowWritePaths: [ - ...monitoredWrites.allowOnly, + ...writeRootsOf(monitoredWrites), ...(config.filesystem.disabled ? [] : getDefaultWritePaths()), - ].map(p => normalizePathForSandbox(p)), + ].map(p => normalizePathForSandbox(p, { literal: true })), denyWritePaths: [ ...monitoredWrites.denyWithinAllow.map(p => - normalizePathForSandbox(p), + normalizePathForSandbox(p, { literal: true }), + ), + ...(monitoredWrites.literalDenyWithinAllow ?? []).map(p => + normalizePathForSandbox(p, { literal: true }), ), // filesystem.disabled reaches the wrapper as `writeConfig === // undefined`, which skips every bind and the built-in denies with @@ -1223,21 +1240,21 @@ function getCredentialDenyReadPaths( * expansion and no credential masking, because getFsWriteConfig() is a * getter callers reach from permission checks and render paths. A mask entry * that degrades to a deny is a file and cannot cover a directory, so - * leaving those out changes nothing. + * leaving those out changes nothing. An entry with glob characters that is + * also the name of a path counts as that name too. */ function defaultWritePathsUnder({ denyRead, allowRead, credentials, }: { - denyRead: readonly string[] - allowRead: readonly string[] | undefined + denyRead: readonly FilesystemPathEntry[] + allowRead: readonly FilesystemPathEntry[] | undefined credentials: CredentialsConfig | undefined }): string[] { - return getDefaultWritePaths({ - denyRead: [...denyRead, ...getCredentialDenyReadPaths(credentials)], - allowRead, - }) + return getDefaultWritePaths( + readRulesOf(denyRead, allowRead, getCredentialDenyReadPaths(credentials)), + ) } /** @@ -1255,9 +1272,15 @@ function unionDenyReadPaths( } /** - * Strip a trailing `/**` from each read-path entry and, on Linux, replace - * any remaining glob with what `expandGlob` returns for it (bubblewrap takes - * concrete paths only). Other platforms match globs natively. + * Strip a trailing `/**` from each spelling of a read list and, on Linux, + * replace one that still has glob characters with what it resolves to + * (bubblewrap takes concrete paths only): what `expandGlob` returns for the + * pattern, then the name the spelling also is when that exists, and what + * the pattern matches beneath a directory that exists and has the + * characters in its own name (see path-entries.ts). Other platforms match + * globs natively, and their backends decide the other readings themselves. + * Entries the caller marked literal are not among the result: they travel + * in the `literal…` lists. * * `literalPaths` are the ones the library resolved itself (a masked * credential file that degraded to deny) — each names one file on disk, so @@ -1265,31 +1288,43 @@ function unionDenyReadPaths( * pattern, a name holding `[` matches nothing and the deny is lost. */ function resolveReadPathEntries( - paths: readonly string[], - expandGlob: (pattern: string) => string[], + kind: PathListKind, + paths: readonly FilesystemPathEntry[], + expandGlob: (pattern: string, anchor?: string) => string[], literalPaths: readonly string[] = [], ): string[] { const literal = new Set(literalPaths) - return paths.flatMap(p => { + return spelledOf(paths).flatMap(p => { const stripped = removeTrailingGlobSuffix(p) return getPlatform() === 'linux' && !literal.has(p) && containsGlobChars(stripped) - ? expandGlob(p) + ? withOtherReadings(p, stripped, kind, expandGlob) : [stripped] }) } /** - * Strip a trailing `/**` and drop what is still a glob on Linux: bwrap needs - * real paths. macOS subpath matching is recursive, so the strip is harmless - * there and the filter never fires. + * Strip a trailing `/**` from each spelling of a write list and, on Linux, + * drop one that still has glob characters unless it is also the name of a + * path that exists: bwrap needs real paths and nothing expands a write + * pattern, so such an entry is applied as the name and its pattern reading + * is not. macOS subpath matching is recursive, so the strip is harmless + * there and the filter never fires. Entries the caller marked literal are + * not among the result: they travel in the `literal…` lists. */ -function stripWriteGlobs(paths: readonly string[]): string[] { - return paths +function stripWriteGlobs( + kind: PathListKind, + paths: readonly FilesystemPathEntry[], +): string[] { + return spelledOf(paths) .map(p => removeTrailingGlobSuffix(p)) .filter(p => { - if (getPlatform() === 'linux' && containsGlobChars(p)) { + if ( + getPlatform() === 'linux' && + containsGlobChars(p) && + !hasNameReading(p, kind) + ) { logForDebugging(`[Sandbox] Skipping glob write pattern on Linux: ${p}`) return false } @@ -1297,8 +1332,8 @@ function stripWriteGlobs(paths: readonly string[]): string[] { }) } -function expandAllowReadGlob(pattern: string): string[] { - const expanded = expandGlobPattern(pattern) +function expandAllowReadGlob(pattern: string, anchor?: string): string[] { + const expanded = expandGlobPattern(pattern, { anchor }) logForDebugging( `[Sandbox] Expanded allowRead glob pattern "${pattern}" to ${expanded.length} paths on Linux`, ) @@ -1308,9 +1343,11 @@ function expandAllowReadGlob(pattern: string): string[] { /** * The read policy of the initialized config, for inspection and display. * On Linux, denyRead globs are collapsed to covering directory mounts against - * this config's allowRead and {@link getFsWriteConfig}'s allowOnly, so + * this config's allowRead and {@link getFsWriteConfig}'s write roots, so * `denyOnly` is only sound alongside that write config and must not be handed - * to wrapCommandWithSandboxLinux with a different one. + * to wrapCommandWithSandboxLinux with a different one. Entries the caller + * marked literal are in `literalDenyOnly` and `literalAllowWithinDeny`, as + * spelled, and the object is to be passed on whole. */ function getFsReadConfig(): FsReadRestrictionConfig { if (!config || config.filesystem.disabled) { @@ -1326,15 +1363,29 @@ function getFsReadConfig(): FsReadRestrictionConfig { // allowRead (re-allow within denied regions) is resolved first: the // denyRead glob expansion collapses against it. const allowPaths = resolveReadPathEntries( + 'allow', config.filesystem.allowRead ?? [], expandAllowReadGlob, ) - const reExposedPaths = [...allowPaths, ...getFsWriteConfig().allowOnly] + const reExposedPaths = reExposedBy( + allowPaths, + config.filesystem.allowRead, + getFsWriteConfig(), + ) const unlistableDenyDirs = new Set() const denyPaths = resolveReadPathEntries( - unionDenyReadPaths(config.filesystem.denyRead, credentialRestrictions), - pattern => - expandReadDenyGlobLinux(pattern, reExposedPaths, unlistableDenyDirs), + 'deny', + unionDenyReadPaths( + spelledOf(config.filesystem.denyRead), + credentialRestrictions, + ), + (pattern, anchor) => + expandReadDenyGlobLinux( + pattern, + reExposedPaths, + unlistableDenyDirs, + anchor, + ), credentialRestrictions.degradeToDenyPaths, ) @@ -1342,6 +1393,10 @@ function getFsReadConfig(): FsReadRestrictionConfig { denyOnly: denyPaths, allowWithinDeny: allowPaths, unlistableDenyDirs: [...unlistableDenyDirs], + ...literalReadLists( + config.filesystem.denyRead, + config.filesystem.allowRead, + ), } } @@ -1354,8 +1409,8 @@ function getFsWriteConfig(): FsWriteRestrictionConfig { return { allowOnly: ['/'], denyWithinAllow: [] } } - const allowPaths = stripWriteGlobs(config.filesystem.allowWrite) - const denyPaths = stripWriteGlobs(config.filesystem.denyWrite) + const allowPaths = stripWriteGlobs('allow', config.filesystem.allowWrite) + const denyPaths = stripWriteGlobs('deny', config.filesystem.denyWrite) const allowOnly = [ ...defaultWritePathsUnder({ @@ -1369,6 +1424,10 @@ function getFsWriteConfig(): FsWriteRestrictionConfig { return { allowOnly, denyWithinAllow: denyPaths, + ...literalWriteLists( + config.filesystem.allowWrite, + config.filesystem.denyWrite, + ), } } @@ -1450,10 +1509,14 @@ function rawWindowsFsInputs(c: SandboxRuntimeConfig) { } } -function setEq(a: readonly string[], b: readonly string[]): boolean { +/** Compared by value: a marked entry is a new object after every clone. */ +function setEq( + a: readonly FilesystemPathEntry[], + b: readonly FilesystemPathEntry[], +): boolean { if (a.length !== b.length) return false - const s = new Set(a) - return b.every(x => s.has(x)) + const s = new Set(a.map(pathEntryKey)) + return b.every(x => s.has(pathEntryKey(x))) } function sameRawWindowsFsInputs( @@ -1672,6 +1735,7 @@ async function wrapWithSandbox( let readConfig: FsReadRestrictionConfig | undefined if (!fsDisabled) { const userAllowWrite = stripWriteGlobs( + 'allow', customConfig?.filesystem?.allowWrite ?? config?.filesystem.allowWrite ?? [], @@ -1690,10 +1754,15 @@ async function wrapWithSandbox( ...userAllowWrite, ], denyWithinAllow: stripWriteGlobs( + 'deny', customConfig?.filesystem?.denyWrite ?? config?.filesystem.denyWrite ?? [], ), + ...literalWriteLists( + customConfig?.filesystem?.allowWrite ?? config?.filesystem.allowWrite, + customConfig?.filesystem?.denyWrite ?? config?.filesystem.denyWrite, + ), } // Credential deny paths are unioned with the caller's denyRead — never @@ -1702,6 +1771,7 @@ async function wrapWithSandbox( // collapsed against the paths that re-expose contents under a denied // directory (allowRead + allowWrite), so both must be final here. const expandedAllowRead = resolveReadPathEntries( + 'allow', customConfig?.filesystem?.allowRead ?? config?.filesystem.allowRead ?? [], expandAllowReadGlob, ) @@ -1715,21 +1785,37 @@ async function wrapWithSandbox( if (javaAgentJarPath) { expandedAllowRead.push(javaAgentJarPath) } - const reExposedPaths = [...expandedAllowRead, ...writeConfig.allowOnly] + const reExposedPaths = reExposedBy( + expandedAllowRead, + customConfig?.filesystem?.allowRead ?? config?.filesystem.allowRead, + writeConfig, + ) const unlistableDenyDirs = new Set() const expandedDenyRead = resolveReadPathEntries( + 'deny', unionDenyReadPaths( - customConfig?.filesystem?.denyRead ?? config?.filesystem.denyRead ?? [], + spelledOf( + customConfig?.filesystem?.denyRead ?? config?.filesystem.denyRead, + ), credentialRestrictions, ), - pattern => - expandReadDenyGlobLinux(pattern, reExposedPaths, unlistableDenyDirs), + (pattern, anchor) => + expandReadDenyGlobLinux( + pattern, + reExposedPaths, + unlistableDenyDirs, + anchor, + ), credentialRestrictions.degradeToDenyPaths, ) readConfig = { denyOnly: expandedDenyRead, allowWithinDeny: expandedAllowRead, unlistableDenyDirs: [...unlistableDenyDirs], + ...literalReadLists( + customConfig?.filesystem?.denyRead ?? config?.filesystem.denyRead, + customConfig?.filesystem?.allowRead ?? config?.filesystem.allowRead, + ), } } @@ -2360,7 +2446,11 @@ function annotateStderrWithSandboxFailures( * fully supported on Linux. Returns empty array on macOS or when * sandboxing is disabled. * - * Patterns ending with /** are excluded since they work as subpaths. + * Patterns ending with /** are excluded since they work as subpaths, and so + * are entries marked literal, which are no patterns. An entry that is also + * the name of a path that exists is applied as that name and is still + * returned: its pattern reading is not applied, and what is returned must + * not depend on what is on the disk, which a sandboxed command can change. */ function getLinuxGlobPatternWarnings(): string[] { // Only warn on Linux/WSL (bubblewrap doesn't support globs) @@ -2379,6 +2469,7 @@ function getLinuxGlobPatternWarnings(): string[] { ] for (const path of allPaths) { + if (typeof path !== 'string') continue // Strip trailing /** since that's just a subpath (directory and everything under it) const pathWithoutTrailingStar = removeTrailingGlobSuffix(path) @@ -2396,6 +2487,7 @@ function getLinuxGlobPatternWarnings(): string[] { ...config.filesystem.denyRead, ...(config.filesystem.allowRead ?? []), ]) { + if (typeof path !== 'string') continue const baseDir = globPatternBaseDir(normalizePathForSandbox(path)) if ( containsGlobChars(removeTrailingGlobSuffix(path)) && diff --git a/src/sandbox/sandbox-schemas.ts b/src/sandbox/sandbox-schemas.ts index ce4243109..bd382ea70 100644 --- a/src/sandbox/sandbox-schemas.ts +++ b/src/sandbox/sandbox-schemas.ts @@ -16,10 +16,19 @@ * one — stays denied. * * This is maximally permissive by default - only explicitly denied paths are blocked. + * + * A `literal…` list holds MORE entries for the list it is named after, not a + * part of it: paths the caller marked `{ path, literal: true }`. They are + * names, in which `[`, `*` and `?` are characters and never pattern syntax, + * so no backend expands or compiles them. The key is absent when there are + * none. Whatever reads `denyOnly` to learn what is denied has to read + * `literalDenyOnly` too. */ export interface FsReadRestrictionConfig { denyOnly: string[] allowWithinDeny?: string[] + literalDenyOnly?: string[] + literalAllowWithinDeny?: string[] /** * The `denyOnly` entries that stand for a directory a glob expansion could * not list. Nothing is bound back beneath one — neither an @@ -41,10 +50,22 @@ export interface FsReadRestrictionConfig { * * This is maximally restrictive by default - only explicitly allowed paths are writable. * Note: Empty `allowOnly` means NO paths are writable (unlike read's empty denyOnly). + * + * A `literal…` list holds MORE entries for the list it is named after, as in + * {@link FsReadRestrictionConfig}: names and never patterns, among them the + * paths the caller marked `{ path, literal: true }`. What the command may + * write is `allowOnly` and `literalAllowOnly` together. */ export interface FsWriteRestrictionConfig { allowOnly: string[] + literalAllowOnly?: string[] denyWithinAllow: string[] + /** + * More paths to deny within `allowOnly`, which the library read off the + * disk itself. They are names, not spellings a caller wrote: `[`, `*` and + * `?` in them are characters of the name and never pattern syntax. + */ + literalDenyWithinAllow?: string[] } /** diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index 23859d190..d5306cb4f 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -136,6 +136,38 @@ export function containsGlobCharsForPlatform(p: string): boolean { : containsGlobChars(p) } +/** + * An entry of `denyRead`, `allowRead`, `allowWrite` or `denyWrite` as + * configured: a spelling, which is a pattern when it reads as one, or a path + * marked literal, which is never a pattern. + */ +export type PathListEntry = string | { path: string; literal: true } + +/** + * The path of an entry that is not a spelling. Throws unless the entry is + * `{ path: string, literal: true }`: a config reaches the library without + * going through the schema when an embedder builds it in code, and taking + * `{ path }` for a name or for a pattern would be a guess about a deny. + */ +export function markedLiteralPath( + entry: Exclude, +): string { + const candidate: { path?: unknown; literal?: unknown } | null = entry + if ( + candidate === null || + typeof candidate !== 'object' || + typeof candidate.path !== 'string' || + candidate.path === '' || + candidate.literal !== true + ) { + throw new TypeError( + 'A filesystem path entry must be a path, or { path, literal: true }; ' + + `got ${JSON.stringify(entry) ?? String(entry)}`, + ) + } + return candidate.path +} + /** * Strip the Win32 `\\?\` extended-path prefix so the residue is * a conventional absolute path (drive-letter or UNC) with no `?` @@ -343,7 +375,7 @@ export function expandWindowsEnvRefs(p: string): string { * Runs of `/` collapsed to one and `/./` components dropped, leaving the rest * of the spelling (a trailing `/` or `/.`, a `..`) to the caller. POSIX only. */ -function collapseInteriorSpellings(pathPattern: string): string { +export function collapseInteriorSpellings(pathPattern: string): string { return pathPattern.replace(/\/{2,}/g, '/').replace(/\/\.(?=\/)/g, '') } @@ -381,14 +413,16 @@ function warnIfParentRefUnfolded(normalizedPath: string): string { * Returns the absolute path with symlinks resolved (or normalized glob pattern) * * `opts.literal` marks a path that names one file or directory rather - * than matching several: one the library computed itself, or a caller - * spelling that carried no glob character — resolving such a spelling can - * splice in a cwd or home directory whose own name does. The glob - * branches are skipped for it, so a component like `a[b` is resolved and - * later compiled as the name it is. A spelling the caller wrote with `*`, - * `?` or `[…]` in it keeps the character sniffing: there the brackets are - * the glob syntax it asked for. The interior collapse below is not one of - * the glob branches: `//` and `/./` are dead spellings either way. + * than matching several: one the library computed itself, one the caller + * marked literal, a caller spelling that carried no glob character — + * resolving such a spelling can splice in a cwd or home directory whose own + * name does — or the name a spelling with glob characters also is (see + * path-entries.ts). The glob branches are skipped for it, so a component + * like `a[b` is resolved and later compiled as the name it is. Without it, + * a spelling with `*`, `?` or `[…]` in it keeps the character sniffing: + * that is the pattern reading of what the caller wrote, where the brackets + * are glob syntax. The interior collapse below is not one of the glob + * branches: `//` and `/./` are dead spellings either way. */ export function normalizePathForSandbox( pathPattern: string, @@ -570,10 +604,15 @@ const HOME_CONVENIENCE_WRITE_DIRS: readonly string[] = [ * still counts (which only ever drops a convenience path), and a glob whose * match is a symlink to one of these directories is not seen. A glob * `allowRead` entry is not counted as re-opening anything. + * + * An entry marked `{ path, literal: true }` is a name whatever characters it + * holds. A spelling is read by its characters alone here: the name that a + * spelling with glob characters also is when it exists on disk is for the + * caller to add as a marked entry, which the sandbox manager does. */ export function getDefaultWritePaths(readRules?: { - denyRead: readonly string[] - allowRead?: readonly string[] + denyRead: readonly PathListEntry[] + allowRead?: readonly PathListEntry[] }): string[] { const home = homedir() const keptDirs = @@ -592,8 +631,8 @@ export function getDefaultWritePaths(readRules?: { */ function homeDirsNotReadDenied( home: string, - denyRead: readonly string[], - allowRead: readonly string[] = [], + denyRead: readonly PathListEntry[], + allowRead: readonly PathListEntry[] = [], ): readonly string[] { // Rules are compared as normalizePathForSandbox spells them, and on macOS // that resolves /tmp and /var to /private/... for a path that exists. A @@ -604,8 +643,11 @@ function homeDirsNotReadDenied( ] const denies = denyRead.map(entry => readRuleCovers(entry)) const reopened = allowRead - .map(entry => removeTrailingGlobSuffix(entry)) - .filter(entry => !containsGlobCharsForPlatform(entry)) + .flatMap(entry => { + if (typeof entry !== 'string') return [markedLiteralPath(entry)] + const stripped = removeTrailingGlobSuffix(entry) + return containsGlobCharsForPlatform(stripped) ? [] : [stripped] + }) .map(entry => normalizePathForSandbox(entry, { literal: true })) return HOME_CONVENIENCE_WRITE_DIRS.filter(rel => { const spellings = homes.map(h => path.join(h, rel)) @@ -629,9 +671,15 @@ function homeDirsNotReadDenied( * is resolved: `*`, `?` and `[…]` there are the syntax it asked for, while * resolving splices in a cwd or home directory that may carry those * characters in its own name. A spelling without them is normalized as the - * name it is. + * name it is, and so is an entry marked literal, whatever it holds. */ -function readRuleCovers(entry: string): (p: string) => boolean { +function readRuleCovers(entry: PathListEntry): (p: string) => boolean { + if (typeof entry !== 'string') { + const rule = normalizePathForSandbox(markedLiteralPath(entry), { + literal: true, + }) + return p => isAtOrUnder(p, rule) + } const stripped = removeTrailingGlobSuffix(entry) if (containsGlobCharsForPlatform(stripped)) { try { @@ -1117,10 +1165,11 @@ export function globToRegex(globPattern: string): string { * already does (a deny masks the whole subtree). Only ever widens a deny. * * Takes a whole pattern, so every character in it is glob syntax: right for - * a spelling the caller wrote, which is what {@link readRuleCovers} passes. - * A pattern the library anchored at a directory of its own goes through the - * macOS `denyGlobEntryRegex`, which splices that directory back in escaped - * and calls this for the tail. + * the pattern reading of a spelling the caller wrote, which is what + * {@link readRuleCovers} passes. A pattern anchored at a directory that is a + * name on disk (one of the library's own, or one that exists and holds glob + * characters) goes through the macOS `denyGlobEntryRegex`, which splices + * that directory back in escaped and calls this for the tail. */ export function denyGlobRegex(normalizedGlob: string): string { // globToRegex() always returns '^…$'. @@ -1135,6 +1184,13 @@ export interface ExpandGlobOptions { * don't need it). */ caseInsensitive?: boolean + /** + * A directory the pattern is walked beneath, taken as the name it is: a + * leading run of `globPath` that exists on disk and may itself hold `[`, + * `*` or `?`. Only what follows it is pattern. The glob dialect has no + * escape, so this is the one way to say where such a name ends. + */ + anchor?: string } /** What one recursive walk of a glob's base directory found; see {@link walkGlobPattern}. */ @@ -1456,7 +1512,8 @@ function globPositions( * component), which {@link walkGlobPattern} refuses to expand. * * @param normalizedPattern - a pattern already through - * {@link normalizePathForSandbox} (and, on Windows, {@link toForwardSlashes}) + * {@link normalizePathForSandbox} (and, on Windows, {@link toForwardSlashes}), + * or the tail of one that is walked beneath an anchor */ export function globPatternBaseDir(normalizedPattern: string): string { const staticPrefix = normalizedPattern.split(/[*?[\]]/)[0] @@ -1500,9 +1557,20 @@ export function walkGlobPattern( realOf: new Map(), } - const normalizedPattern = toForwardSlashes(normalizePathForSandbox(globPath)) - const baseDir = globPatternBaseDir(normalizedPattern) - if (baseDir === '' || baseDir === '/') { + // Beneath an anchor the pattern is the tail alone, rooted at the anchor: + // no character of the anchor is compiled, and the walk starts at the + // anchor plus the tail's own static directory. + const anchor = + opts.anchor === undefined ? undefined : toForwardSlashes(opts.anchor) + const normalizedPattern = + anchor === undefined + ? toForwardSlashes(normalizePathForSandbox(globPath)) + : toForwardSlashes(globPath).slice(anchor.length) + const patternBaseDir = globPatternBaseDir(normalizedPattern) + const baseDirBelowAnchor = patternBaseDir === '/' ? '' : patternBaseDir + const baseDir = + anchor === undefined ? patternBaseDir : anchor + baseDirBelowAnchor + if (anchor === undefined && (baseDir === '' || baseDir === '/')) { logForDebugging( `[Sandbox] Glob pattern has no literal directory to start from, skipping: ${globPath}`, { level: 'warn' }, @@ -1606,7 +1674,9 @@ export function walkGlobPattern( dir: baseDir, real: baseReal, short: baseDir.length < baseReal.length ? baseDir : baseReal, - positions: baseDir.split('/').reduce(positions.next, positions.start), + positions: (anchor === undefined ? baseDir : baseDirBelowAnchor) + .split('/') + .reduce(positions.next, positions.start), }) for (let frame = pending.pop(); frame !== undefined; frame = pending.pop()) { const { dir, real } = frame @@ -1638,7 +1708,14 @@ export function walkGlobPattern( for (const entry of entries) { const fullPath = path.join(dir, entry.name) const realPath = path.join(real, entry.name) - const candidate = toForwardSlashes(fullPath) + // A pattern that does not split is matched by its whole spelling, and + // beneath an anchor that is the spelling from the anchor on. It is never + // listed through a link, so every path it sees starts with the anchor. + const spelled = toForwardSlashes(fullPath) + const candidate = + anchor !== undefined && spelled.startsWith(anchor) + ? spelled.slice(anchor.length) + : spelled const isMatch = positions.matches(fresh, entry.name, candidate) if (isMatch) walk.matches.push(fullPath) if (entry.isDirectory()) { diff --git a/src/sandbox/windows-sandbox-utils.ts b/src/sandbox/windows-sandbox-utils.ts index ece8fd95b..19c24250c 100644 --- a/src/sandbox/windows-sandbox-utils.ts +++ b/src/sandbox/windows-sandbox-utils.ts @@ -24,8 +24,9 @@ export { isUncPath, } from './sandbox-utils.js' import { certThumbprint, generateCa, validateCaPair } from './mitm-ca.js' +import { literalReadings, markedLiteralPath } from './path-entries.js' import type { SandboxDependencyCheck } from './linux-sandbox-utils.js' -import type { SrtWinConfig } from './sandbox-config.js' +import type { FilesystemPathEntry, SrtWinConfig } from './sandbox-config.js' /** * Windows sandbox backend. @@ -1688,15 +1689,38 @@ export function uninstallWindowsSandbox( * `srt-win` soft-drops a missing UNC deny target rather than * materializing a placeholder chain on an SMB share. A UNC **glob** * still walks the share (user-trusted). + * + * `[` and `]` are characters of a name here, but the walk that expands a + * glob reads them as a character class on every platform. So a glob that + * lies beneath a directory with brackets in its name, which the walk alone + * would expand to nothing, is also expanded beneath that directory taken as + * the name it is (see path-entries.ts). A UNC glob is not: finding the + * directory would probe the share. + * + * An entry marked `{ path, literal: true }` is a literal whatever it holds. + * One that holds `*` or `?` is skipped: no Win32 name has them, so it names + * nothing that can exist, and `srt-win` refuses the characters outright. */ export function expandWindowsFsPaths( - patterns: readonly string[], + patterns: readonly FilesystemPathEntry[], opts?: { mode?: 'grant' | 'deny' }, ): string[] { const out = new Set() - for (const raw of patterns) { - const norm = normalizePathForSandbox(raw) - const isGlob = containsGlobCharsWin(norm) + for (const entry of patterns) { + const marked = typeof entry !== 'string' + const raw = marked ? markedLiteralPath(entry) : entry + const norm = normalizePathForSandbox( + raw, + marked ? { literal: true } : undefined, + ) + if (marked && containsGlobCharsWin(norm)) { + logForDebugging( + `[Sandbox Windows] Skipping a literal path that no file can have: ${norm}`, + { level: 'warn' }, + ) + continue + } + const isGlob = !marked && containsGlobCharsWin(norm) // UNC literal: pass raw (no stat) — see {@link isUncPath}. A // UNC glob falls through to expandGlobPattern below. if (isUncPath(norm) && !isGlob) { @@ -1704,7 +1728,19 @@ export function expandWindowsFsPaths( continue } const candidates = isGlob - ? expandGlobPattern(norm, { caseInsensitive: true }) + ? [ + ...expandGlobPattern(norm, { caseInsensitive: true }), + ...literalReadings(raw, opts?.mode === 'deny' ? 'deny' : 'allow', { + isPattern: containsGlobCharsWin, + }).flatMap(reading => + reading.glob + ? expandGlobPattern(reading.path, { + caseInsensitive: true, + anchor: reading.anchor, + }) + : [], + ), + ] : [norm] for (const c of candidates) { const st = fs.statSync(c, { throwIfNoEntry: false }) From 4c9c77d40f1dcc3cb291faa18581a499953bb7e7 Mon Sep 17 00:00:00 2001 From: Ron Leizrowice Date: Fri, 25 Sep 2026 22:16:47 +0000 Subject: [PATCH 2/4] test: entries inside a folder named like a pattern, and marked entries 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. --- test/config-validation.test.ts | 115 ++ test/sandbox/glob-expand.test.ts | 227 ++- test/sandbox/literal-path-entries.test.ts | 2262 +++++++++++++++++++++ 3 files changed, 2603 insertions(+), 1 deletion(-) create mode 100644 test/sandbox/literal-path-entries.test.ts diff --git a/test/config-validation.test.ts b/test/config-validation.test.ts index f2864ac23..3a9eb1532 100644 --- a/test/config-validation.test.ts +++ b/test/config-validation.test.ts @@ -525,6 +525,121 @@ describe('Config Validation', () => { }) }) + describe('path entries marked literal', () => { + type List = 'denyRead' | 'allowRead' | 'allowWrite' | 'denyWrite' + const lists: List[] = ['denyRead', 'allowRead', 'allowWrite', 'denyWrite'] + + function parse(list: List, entry: unknown) { + return SandboxRuntimeConfigSchema.safeParse({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { + denyRead: [], + allowWrite: [], + denyWrite: [], + [list]: [entry], + }, + }) + } + + test.each(lists)('%s takes a path marked literal beside a string', list => { + const marked = { path: '/work/*.env', literal: true as const } + const result = parse(list, marked) + expect(result.success).toBe(true) + expect(result.success && result.data.filesystem[list]).toEqual([marked]) + const plain = parse(list, '/work/keep') + expect(plain.success && plain.data.filesystem[list]).toEqual([ + '/work/keep', + ]) + }) + + test.each(lists)( + '%s takes ~ and relative spellings marked literal', + list => { + for (const path of ['~/notes (draft?)', './build*', '[WIP] project']) { + expect(parse(list, { path, literal: true }).success).toBe(true) + } + }, + ) + + test.each([ + ['no mark', { path: '/work/x' }], + ['a mark that is false', { path: '/work/x', literal: false }], + ['a mark that is a string', { path: '/work/x', literal: 'true' }], + ['a mark that is a number', { path: '/work/x', literal: 1 }], + ['no path', { literal: true }], + ['a path that is not a string', { path: ['/work/x'], literal: true }], + ['a number', 42], + ['null', null], + ['a list', ['/work/x']], + ])('refuses an entry with %s', (_what, entry) => { + for (const list of lists) { + const result = parse(list, entry) + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error.issues[0]?.path.slice(0, 3)).toEqual([ + 'filesystem', + list, + 0, + ]) + } + } + }) + + test('says what an entry may be when it is neither', () => { + const result = parse('denyRead', { path: '/work/x', literal: false }) + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error.issues[0]?.message).toBe( + 'Expected a path, or { "path": "", "literal": true }', + ) + } + }) + + test('refuses a key it does not know, rather than ignore it', () => { + const result = parse('denyRead', { + path: '/work/x', + literal: true, + recursive: true, + }) + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error.issues[0]?.code).toBe('unrecognized_keys') + } + }) + + test('refuses an empty path, marked or not, with the same message', () => { + for (const entry of ['', { path: '', literal: true }]) { + const result = parse('allowWrite', entry) + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error.issues[0]?.message).toBe('Path cannot be empty') + } + } + }) + + test('takes a separator at the end of a marked deny, which is no glob', () => { + // The same spelling as a string is a pattern that can match no path, + // and is refused; marked, it is a name and the separator ends it. + for (const list of ['denyRead', 'denyWrite'] as const) { + expect(parse(list, '/data/*/').success).toBe(false) + expect(parse(list, { path: '/data/*/', literal: true }).success).toBe( + true, + ) + } + }) + + test('does not take the mark on a credential file', () => { + const result = SandboxRuntimeConfigSchema.safeParse({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { denyRead: [], allowWrite: [], denyWrite: [] }, + credentials: { + files: [{ path: { path: '~/.netrc', literal: true }, mode: 'deny' }], + }, + }) + expect(result.success).toBe(false) + }) + }) + describe('bwrapPath / socatPath', () => { const base = { network: { allowedDomains: [], deniedDomains: [] }, diff --git a/test/sandbox/glob-expand.test.ts b/test/sandbox/glob-expand.test.ts index ec0625c9f..c5998451d 100644 --- a/test/sandbox/glob-expand.test.ts +++ b/test/sandbox/glob-expand.test.ts @@ -14,7 +14,9 @@ import { symlinkSync, } from 'node:fs' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { basename, join } from 'node:path' +import { literalReadings } from '../../src/sandbox/path-entries.js' +import type { FilesystemPathEntry } from '../../src/sandbox/sandbox-config.js' import { expandGlobPattern, expandTilde, @@ -835,6 +837,229 @@ describe('expandWindowsFsPaths literal branch', () => { }) }) +// ============================================================================ +// expandWindowsFsPaths — beneath a directory with brackets in its name +// ============================================================================ + +/** + * `[` and `]` are characters of a name on Windows, and a path that holds + * them and no `*` or `?` is a literal. A pattern beneath such a directory is + * expanded by the walk every platform shares, which reads the brackets as a + * character class: on its own it finds nothing, and the deny or grant is + * lost. expandWindowsFsPaths is plain TypeScript, so what it does with such + * entries is pinned here on every host; the spellings only Windows has are + * in the suite below this one. + */ +describe('expandWindowsFsPaths beneath a directory named [WIP] project', () => { + let root: string + let project: string + let envFiles: string[] + + beforeAll(() => { + root = realPath(mkdtempSync(join(tmpdir(), 'win-literal-'))) + project = join(root, '[WIP] project') + mkdirSync(join(project, 'keep'), { recursive: true }) + mkdirSync(join(project, 'sub', 'deep'), { recursive: true }) + envFiles = [ + join(project, '.env'), + join(project, 'sub', '.env'), + join(project, 'sub', 'deep', '.env'), + ] + for (const file of envFiles) writeFileSync(file, '') + writeFileSync(join(project, 'sub', 'readme'), '') + }) + + afterAll(() => { + rmSync(root, { recursive: true, force: true }) + }) + + it('takes a path inside it for the path it is', () => { + const kept = join(project, 'keep') + expect(expandWindowsFsPaths([kept])).toEqual([kept]) + expect(expandWindowsFsPaths([kept], { mode: 'deny' })).toEqual([kept]) + }) + + it('passes a deny for a path inside it that is not there yet, and drops the grant', () => { + const notYet = join(project, 'not-yet') + expect(expandWindowsFsPaths([notYet], { mode: 'deny' })).toEqual([notYet]) + expect(expandWindowsFsPaths([notYet], { mode: 'grant' })).toEqual([]) + }) + + it.each(['deny', 'grant'] as const)( + 'expands a pattern beneath it (%s)', + mode => { + expect( + expandWindowsFsPaths([`${project}/**/.env`], { mode }).sort(), + ).toEqual([...envFiles].sort()) + expect( + expandWindowsFsPaths([`${project}/sub/*`], { mode }).sort(), + ).toEqual( + [ + join(project, 'sub', '.env'), + join(project, 'sub', 'deep'), + join(project, 'sub', 'readme'), + ].sort(), + ) + }, + ) + + it('lists a path once that more than one reading finds', () => { + const out = expandWindowsFsPaths( + [`${project}/**/.env`, `${project}/*/.env`, join(project, 'sub', '.env')], + { mode: 'deny' }, + ) + expect(out.sort()).toEqual([...envFiles].sort()) + }) + + it('takes a marked entry for a literal, whatever it holds', () => { + const kept = join(project, 'keep') + const marked: FilesystemPathEntry = { path: kept, literal: true } + expect(expandWindowsFsPaths([marked])).toEqual([kept]) + const notYet: FilesystemPathEntry = { + path: join(project, '[later]'), + literal: true, + } + expect(expandWindowsFsPaths([notYet], { mode: 'deny' })).toEqual([ + join(project, '[later]'), + ]) + expect(expandWindowsFsPaths([notYet], { mode: 'grant' })).toEqual([]) + }) + + it('skips a marked entry with a character no Windows name can hold', () => { + // srt-win refuses `*` and `?` outright, and nothing can have the name. + for (const path of [`${project}/*.env`, join(project, 'what?')]) { + const marked: FilesystemPathEntry = { path, literal: true } + expect(expandWindowsFsPaths([marked], { mode: 'deny' })).toEqual([]) + expect(expandWindowsFsPaths([marked], { mode: 'grant' })).toEqual([]) + } + }) + + it('refuses an entry that is neither a path nor a marked path', () => { + for (const entry of [ + { path: project }, + { path: project, literal: false }, + ]) { + expect(() => + expandWindowsFsPaths([entry as unknown as FilesystemPathEntry]), + ).toThrow(TypeError) + } + }) + + it.if(!isWindows)( + 'does not expand a grant beneath a link that has the name the pattern spells', + () => { + const work = join(root, 'work') + mkdirSync(join(root, 'vault', 'pub'), { recursive: true }) + mkdirSync(work, { recursive: true }) + writeFileSync(join(root, 'vault', 'pub', 'key'), '') + symlinkSync(join(root, 'vault'), join(work, '[ab]')) + const pattern = `${work}/[ab]/pub/*` + expect(expandWindowsFsPaths([pattern], { mode: 'grant' })).toEqual([]) + expect(expandWindowsFsPaths([pattern], { mode: 'deny' })).toEqual([ + join(work, '[ab]', 'pub', 'key'), + ]) + }, + ) + + it.if(!isWindows)( + 'does not expand a grant beneath a link that lies between such a directory and the first wildcard', + () => { + const work = join(root, 'work-with-link-inside') + const vault = join(root, 'vault-behind-link') + mkdirSync(join(vault, 'deep', 'pub'), { recursive: true }) + mkdirSync(join(work, '[ab]'), { recursive: true }) + symlinkSync(vault, join(work, '[ab]', 'mid')) + const pattern = `${work}/[ab]/mid/*/pub` + expect(expandWindowsFsPaths([pattern], { mode: 'grant' })).toEqual([]) + expect(expandWindowsFsPaths([pattern], { mode: 'deny' })).toEqual([ + join(work, '[ab]', 'mid', 'deep', 'pub'), + ]) + }, + ) +}) + +describe.if(isWindows)( + 'expandWindowsFsPaths beneath a bracketed directory, as Windows spells paths', + () => { + let root: string + let project: string + + beforeAll(() => { + root = mkdtempSync(join(tmpdir(), 'win-literal-')) + project = join(root, '[WIP] project') + mkdirSync(join(project, 'sub', 'deep'), { recursive: true }) + for (const dir of ['', 'sub', join('sub', 'deep')]) { + writeFileSync(join(project, dir, '.env'), '') + } + writeFileSync(join(project, 'sub', 'readme'), '') + }) + + afterAll(() => { + rmSync(root, { recursive: true, force: true }) + }) + + /** Every result names an existing `.env`, and there are three of them. */ + function expectTheEnvFiles(out: string[]): void { + expect(out.map(p => basename(p))).toEqual(['.env', '.env', '.env']) + expect(new Set(out.map(p => p.toLowerCase())).size).toBe(3) + for (const p of out) expect(existsSync(p)).toBe(true) + } + + it('expands a pattern spelled with a drive letter and backslashes', () => { + expect(project).toMatch(/^[A-Za-z]:\\/) + expectTheEnvFiles( + expandWindowsFsPaths([`${project}\\**\\.env`], { mode: 'deny' }), + ) + }) + + it('expands one spelled with forward slashes and a lower-case drive letter', () => { + const spelled = + project[0]!.toLowerCase() + project.slice(1).replace(/\\/g, '/') + expectTheEnvFiles( + expandWindowsFsPaths([`${spelled}/**/.env`], { mode: 'deny' }), + ) + expectTheEnvFiles( + expandWindowsFsPaths([`${spelled}/**/.env`], { mode: 'grant' }), + ) + }) + + it('expands one spelled with mixed separators and the extended-length prefix', () => { + expectTheEnvFiles( + expandWindowsFsPaths([`\\\\?\\${project}/sub/..\\**/.env`], { + mode: 'deny', + }), + ) + }) + + it('takes a marked path spelled with backslashes, and skips one with a wildcard', () => { + const out = expandWindowsFsPaths( + [ + { path: `${project}\\sub`, literal: true }, + { path: `${project}\\*.env`, literal: true }, + ], + { mode: 'deny' }, + ) + expect(out.map(p => basename(p))).toEqual(['sub']) + }) + + it('gives a UNC pattern no reading that would probe the share', () => { + // Deciding on a reading looks at the disk, and looking at a UNC path + // is a request to the host it names. + for (const unc of [ + '\\\\srt-no-such-host\\share\\[WIP] project\\**\\.env', + '\\\\?\\UNC\\srt-no-such-host\\share\\[WIP] project\\*.env', + '//srt-no-such-host/share/[WIP] project/*.env', + ]) { + for (const kind of ['deny', 'allow'] as const) { + expect( + literalReadings(unc, kind, { isPattern: containsGlobCharsWin }), + ).toEqual([]) + } + } + }) + }, +) + // ============================================================================ // isUncPath — broker never stats `\\server\…` with real-user creds // ============================================================================ diff --git a/test/sandbox/literal-path-entries.test.ts b/test/sandbox/literal-path-entries.test.ts new file mode 100644 index 000000000..f29601008 --- /dev/null +++ b/test/sandbox/literal-path-entries.test.ts @@ -0,0 +1,2262 @@ +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + spyOn, +} from 'bun:test' +import { spawnSync } from 'node:child_process' +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + realpathSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs' +import { connect } from 'node:net' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { SandboxManager } from '../../src/sandbox/sandbox-manager.js' +import { wrapCommandWithSandboxLinux } from '../../src/sandbox/linux-sandbox-utils.js' +import * as linuxViolationMonitorModule from '../../src/sandbox/linux-violation-monitor.js' +import { + type LinuxViolationMonitorOptions, + startLinuxSandboxViolationMonitor, +} from '../../src/sandbox/linux-violation-monitor.js' +import { wrapCommandWithSandboxMacOS } from '../../src/sandbox/macos-sandbox-utils.js' +import { + literalReadings, + type PathReading, + pathEntryKey, + readNamesOf, + splitPathEntries, + writeNamesOf, + writeRootsOf, +} from '../../src/sandbox/path-entries.js' +import type { FilesystemPathEntry } from '../../src/sandbox/sandbox-config.js' +import { + denyGlobRegex, + expandGlobPattern, + getDefaultWritePaths, + globToRegex, + normalizePathForSandbox, +} from '../../src/sandbox/sandbox-utils.js' +import { + loadConfig, + loadConfigFromString, +} from '../../src/utils/config-loader.js' +import { bwrapCanNamespace } from '../helpers/bwrap-namespace.js' +import { isLinux, isWindows } from '../helpers/platform.js' + +/** + * A directory may hold `*`, `?`, `[` or `]` in its name. An entry of + * `denyRead`, `allowRead`, `allowWrite` or `denyWrite` that lies inside one + * has those characters in it, and read as a pattern alone it matches + * nothing: the deny is lost and the allow opens nothing. Such an entry is + * read as the pattern AND as the path it spells wherever the part that holds + * the characters exists on disk; an entry marked `{ path, literal: true }` + * is a path and nothing else. + * + * The Linux suites run real commands under bubblewrap. The macOS suites + * read the generated profile, which is string building and runs on any + * POSIX host; nothing here runs a profile under sandbox-exec. + */ + +/** The folder every suite works in: a name a pattern reads as a class. */ +const PROJECT = '[WIP] project' + +const WRAP_TIMEOUT_MS = 60_000 +const RUN_TIMEOUT_MS = 15_000 + +function freshRoot(): string { + return realpathSync(mkdtempSync(join(tmpdir(), 'literal-paths-'))) +} + +/** A path as one shell word. */ +function q(p: string): string { + return `'${p.replace(/'/g, `'\\''`)}'` +} + +/** `text` as it appears inside a regular expression that matches it. */ +function escapedForRegex(text: string): string { + return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +} + +/** + * The filter a spelling compiles to when its characters are read as a + * pattern, which is the reading every entry with glob characters has. + */ +function patternFilter(spelling: string, kind: 'allow' | 'deny'): string { + const compile = kind === 'deny' ? denyGlobRegex : globToRegex + return `(regex ${JSON.stringify(compile(normalizePathForSandbox(spelling)))})` +} + +const subpath = (p: string): string => `(subpath ${JSON.stringify(p)})` +const literalFilter = (p: string): string => `(literal ${JSON.stringify(p)})` + +/** Every `(regex "…")` the profile emits. */ +function emittedRegexes(profile: string): RegExp[] { + return [...profile.matchAll(/\(regex ("(?:[^"\\]|\\.)*")\)/g)].map( + match => new RegExp(JSON.parse(match[1]!) as string), + ) +} + +/** Every `(subpath "…")` the profile emits. */ +function emittedSubpaths(profile: string): string[] { + return [...profile.matchAll(/\(subpath ("(?:[^"\\]|\\.)*")\)/g)].map( + match => JSON.parse(match[1]!) as string, + ) +} + +/** The rule of `profile` that starts with `head`, up to its closing line. */ +function ruleOf(profile: string, head: string): string { + const start = profile.indexOf(head) + if (start < 0) return '' + const end = profile.indexOf('(with message', start) + return profile.slice(start, end) +} + +function macProfile( + readConfig: { denyOnly: string[]; allowWithinDeny?: string[] } | undefined, + writeConfig: { allowOnly: string[]; denyWithinAllow: string[] } | undefined, +): string { + return wrapCommandWithSandboxMacOS({ + command: 'true', + needsNetworkRestriction: false, + readConfig, + writeConfig, + }) +} + +const noNetwork = { allowedDomains: [], deniedDomains: [] } + +type FilesystemLists = { + denyRead?: FilesystemPathEntry[] + allowRead?: FilesystemPathEntry[] + allowWrite?: FilesystemPathEntry[] + denyWrite?: FilesystemPathEntry[] +} + +async function initialize(filesystem: FilesystemLists): Promise { + await SandboxManager.reset() + await SandboxManager.initialize( + { + network: noNetwork, + filesystem: { + denyRead: [], + allowWrite: [], + denyWrite: [], + ...filesystem, + }, + }, + undefined, + false, + ) +} + +/** + * Runs `payload` under the policy, behind an `echo BOOTED`, so a sandbox + * that failed to start cannot read as a payload that was refused. + */ +async function sandboxed( + filesystem: FilesystemLists, + payload: string, + opts: { cwd: string; perWrap?: FilesystemLists; keepMountPoints?: boolean }, +): Promise<{ stdout: string; stderr: string }> { + await initialize(opts.perWrap ? {} : filesystem) + const wrapped = await SandboxManager.wrapWithSandbox( + `echo BOOTED; ${payload}`, + undefined, + opts.perWrap + ? { + filesystem: { + denyRead: [], + allowWrite: [], + denyWrite: [], + ...opts.perWrap, + }, + } + : undefined, + ) + const result = spawnSync(wrapped, { + shell: true, + encoding: 'utf8', + cwd: opts.cwd, + timeout: RUN_TIMEOUT_MS, + }) + if (!opts.keepMountPoints) SandboxManager.cleanupAfterCommand() + expect(result.stdout).toContain('BOOTED') + return { stdout: result.stdout, stderr: result.stderr } +} + +// ============================================================================ +// Deciding the reading +// ============================================================================ + +describe.if(!isWindows)('literalReadings', () => { + let root: string + let project: string + const savedCwd = process.cwd() + + beforeAll(() => { + root = freshRoot() + project = join(root, PROJECT) + mkdirSync(join(project, 'keep'), { recursive: true }) + mkdirSync(join(project, 'src'), { recursive: true }) + mkdirSync(join(project, '[ab]'), { recursive: true }) + writeFileSync(join(project, 'file'), '') + mkdirSync(join(root, 'plain', '[ab]'), { recursive: true }) + mkdirSync(join(root, 'plain', 'a'), { recursive: true }) + mkdirSync(join(root, 'plain', 'b'), { recursive: true }) + mkdirSync(join(root, 'star'), { recursive: true }) + writeFileSync(join(root, 'star', '*.env'), '') + writeFileSync(join(root, 'star', 'a.env'), '') + for (const name of [ + 'open[bracket', + 'close]bracket', + 'build*', + 'notes (draft?)', + 'curly{a,b}', + ]) { + mkdirSync(join(root, name, 'out'), { recursive: true }) + } + mkdirSync(join(root, 'target', 'keep'), { recursive: true }) + symlinkSync(join(root, 'target'), join(root, 'li[n]k')) + symlinkSync(join(root, 'nowhere'), join(root, 'dang[l]ing')) + mkdirSync(join(project, 'real'), { recursive: true }) + symlinkSync(join(root, 'target'), join(project, 'linked')) + }) + + afterAll(() => { + process.chdir(savedCwd) + rmSync(root, { recursive: true, force: true }) + }) + + afterEach(() => { + process.chdir(savedCwd) + }) + + const name = (path: string): PathReading => ({ glob: false, path }) + const beneath = (anchor: string, tail: string): PathReading => ({ + glob: true, + anchor, + path: anchor + tail, + }) + const bothKinds = (spelling: string) => ({ + deny: literalReadings(spelling, 'deny'), + allow: literalReadings(spelling, 'allow'), + }) + const forBoth = (readings: PathReading[]) => ({ + deny: readings, + allow: readings, + }) + + it('reads a path inside the folder as the name it spells', () => { + expect(bothKinds(project)).toEqual(forBoth([name(project)])) + expect(bothKinds(join(project, 'keep'))).toEqual( + forBoth([name(join(project, 'keep'))]), + ) + }) + + it('does not ask for what follows the glob characters to exist', () => { + const notYet = join(project, 'keep', 'not-there-yet') + expect(bothKinds(notYet)).toEqual(forBoth([name(notYet)])) + }) + + it('reads a pattern beneath the folder as that pattern beneath its name', () => { + expect(bothKinds(`${project}/**/.env`)).toEqual( + forBoth([beneath(project, '/**/.env')]), + ) + expect(bothKinds(`${project}/*.pem`)).toEqual( + forBoth([beneath(project, '/*.pem')]), + ) + }) + + it('takes the longest run of directories that exists as the anchor', () => { + expect(bothKinds(`${project}/src/**/.env`)).toEqual( + forBoth([beneath(join(project, 'src'), '/**/.env')]), + ) + expect(bothKinds(`${project}/absent/**/.env`)).toEqual( + forBoth([beneath(project, '/absent/**/.env')]), + ) + }) + + it('reads a wildcard as a name only when a file has that very name', () => { + expect(bothKinds(join(root, 'star', '*.env'))).toEqual( + forBoth([name(join(root, 'star', '*.env'))]), + ) + expect(bothKinds(join(root, 'plain', '*.env'))).toEqual(forBoth([])) + }) + + it('sets a trailing /** aside for the decision and gives it back to a pattern', () => { + expect(bothKinds(`${project}/**`)).toEqual(forBoth([name(project)])) + expect(bothKinds(`${project}/keep/**`)).toEqual( + forBoth([name(join(project, 'keep'))]), + ) + expect(bothKinds(`${project}/**/build/**`)).toEqual( + forBoth([beneath(project, '/**/build/**')]), + ) + }) + + it('strips a trailing separator from a name and keeps it in a pattern', () => { + expect(bothKinds(`${project}/keep/`)).toEqual( + forBoth([name(join(project, 'keep'))]), + ) + expect(bothKinds(`${project}/*/`)).toEqual( + forBoth([beneath(project, '/*/')]), + ) + }) + + it('folds a parent reference in a name', () => { + expect(bothKinds(`${project}/keep/../src`)).toEqual( + forBoth([name(join(project, 'src'))]), + ) + }) + + it('leaves a relative name inside a bracketed cwd alone', () => { + process.chdir(project) + // No glob character in what the caller wrote: it is a name already, and + // every backend takes it for one. + expect(bothKinds('./keep')).toEqual(forBoth([])) + expect(bothKinds('keep')).toEqual(forBoth([])) + }) + + it('reads a relative pattern inside a bracketed cwd beneath the cwd', () => { + process.chdir(project) + expect(bothKinds('**/.env')).toEqual( + forBoth([beneath(project, '/**/.env')]), + ) + expect(bothKinds('./*.env')).toEqual(forBoth([beneath(project, '/*.env')])) + expect(bothKinds('src/**/.env')).toEqual( + forBoth([beneath(join(project, 'src'), '/**/.env')]), + ) + }) + + it('reads a ~ spelling beneath a home directory with brackets in its name', () => { + const home = join(root, PROJECT) + const script = ` + const { literalReadings } = await import(${JSON.stringify( + join(import.meta.dir, '../../src/sandbox/path-entries.ts'), + )}) + console.log(JSON.stringify({ + name: literalReadings('~/keep', 'deny'), + pattern: literalReadings('~/**/.env', 'deny'), + allow: literalReadings('~/src/*.ts', 'allow'), + }))` + const result = spawnSync(process.execPath, ['-e', script], { + env: { ...process.env, HOME: home }, + encoding: 'utf8', + timeout: WRAP_TIMEOUT_MS, + }) + expect(result.status).toBe(0) + expect(JSON.parse(result.stdout.trim().split('\n').at(-1) ?? '')).toEqual({ + name: [], + pattern: [beneath(home, '/**/.env')], + allow: [beneath(join(home, 'src'), '/*.ts')], + }) + }) + + it.each(['open[bracket', 'close]bracket', 'build*', 'notes (draft?)'])( + 'reads a path inside a directory named %p as a name', + dir => { + const inside = join(root, dir, 'out') + expect(bothKinds(inside)).toEqual(forBoth([name(inside)])) + }, + ) + + it('adds nothing for braces, which are no glob characters', () => { + expect(bothKinds(join(root, 'curly{a,b}', 'out'))).toEqual(forBoth([])) + }) + + it('adds nothing for an entry without glob characters', () => { + expect(bothKinds(join(root, 'plain', 'a'))).toEqual(forBoth([])) + expect(bothKinds(join(root, 'absent'))).toEqual(forBoth([])) + }) + + it('adds nothing for a pattern none of which exists as a name', () => { + expect(bothKinds(join(root, 'plain', '**', '.env'))).toEqual(forBoth([])) + expect(bothKinds(join(root, '[nope]', 'x'))).toEqual(forBoth([])) + expect(bothKinds(join(root, 'plain', '[xy]', 'key'))).toEqual(forBoth([])) + }) + + it('reads [ab] as the class and as the directory of that name', () => { + // The pattern reading, which covers `a` and `b`, is the one every entry + // has; the name is what this adds. + const inClass = join(root, 'plain', '[ab]', 'secret') + expect(bothKinds(inClass)).toEqual(forBoth([name(inClass)])) + }) + + it('gives one reading for each name the entry may hold, the longest first', () => { + expect(bothKinds(`${project}/[ab]/secret`)).toEqual( + forBoth([ + name(join(project, '[ab]', 'secret')), + beneath(project, '/[ab]/secret'), + ]), + ) + expect(bothKinds(`${project}/[ab]/**/.env`)).toEqual( + forBoth([ + beneath(join(project, '[ab]'), '/**/.env'), + beneath(project, '/[ab]/**/.env'), + ]), + ) + }) + + it('never turns one reading into another when a directory appears', () => { + const entry = `${project}/[cd]/**/.env` + const before = literalReadings(entry, 'deny') + expect(before).toEqual([beneath(project, '/[cd]/**/.env')]) + mkdirSync(join(project, '[cd]')) + try { + const after = literalReadings(entry, 'deny') + expect(after).toEqual([ + beneath(join(project, '[cd]'), '/**/.env'), + ...before, + ]) + } finally { + rmSync(join(project, '[cd]'), { recursive: true, force: true }) + } + }) + + it('anchors a pattern at the nearest directory when a file is in the way', () => { + // As beneath any folder: the walk then finds the file where it looked + // for a directory. + expect(literalReadings(`${project}/file/**/x`, 'deny')).toEqual([ + beneath(project, '/file/**/x'), + ]) + }) + + describe('a prefix that is a symbolic link', () => { + it('counts for a deny, which can only deny more', () => { + const inside = join(root, 'li[n]k', 'keep') + expect(literalReadings(inside, 'deny')).toEqual([name(inside)]) + expect(literalReadings(`${root}/li[n]k/**/x`, 'deny')).toEqual([ + beneath(join(root, 'li[n]k'), '/**/x'), + ]) + }) + + it('does not count for an allow, which would open what it points at', () => { + expect(literalReadings(join(root, 'li[n]k', 'keep'), 'allow')).toEqual([]) + expect(literalReadings(join(root, 'li[n]k'), 'allow')).toEqual([]) + expect(literalReadings(`${root}/li[n]k/**/x`, 'allow')).toEqual([]) + }) + + it('counts for a deny when it dangles, as a name and not as a directory', () => { + const dangling = join(root, 'dang[l]ing') + expect(literalReadings(dangling, 'deny')).toEqual([name(dangling)]) + expect(literalReadings(`${dangling}/**/x`, 'deny')).toEqual([]) + expect(literalReadings(dangling, 'allow')).toEqual([]) + expect(literalReadings(`${dangling}/**/x`, 'allow')).toEqual([]) + }) + + it('ends the name of an allow wherever it lies after the glob characters', () => { + const through = join(project, 'linked', 'keep') + expect(literalReadings(through, 'deny')).toEqual([name(through)]) + expect(literalReadings(through, 'allow')).toEqual([]) + expect(literalReadings(join(project, 'real', 'keep'), 'allow')).toEqual([ + name(join(project, 'real', 'keep')), + ]) + }) + + it('ends the pattern of an allow when it lies before the first pattern component', () => { + // The walk starts at the folder followed by `linked`, resolved: it + // would list what the link points at. + for (const tail of ['/linked/*/x', '/linked/*', '/real/../linked/**/x']) { + expect(literalReadings(project + tail, 'allow')).toEqual([]) + } + expect(literalReadings(`${project}/linked/*/x`, 'deny')).toEqual([ + beneath(join(project, 'linked'), '/*/x'), + ]) + // Not there yet is no link, and neither is a directory. + expect(literalReadings(`${project}/absent/sub/*`, 'allow')).toEqual([ + beneath(project, '/absent/sub/*'), + ]) + expect(literalReadings(`${project}/real/*/x`, 'allow')).toEqual([ + beneath(join(project, 'real'), '/*/x'), + ]) + // From the first pattern component on, the walk meets the link itself + // and does not list it. + expect(literalReadings(`${project}/*/linked/x`, 'allow')).toEqual([ + beneath(project, '/*/linked/x'), + ]) + }) + }) + + it.skipIf(process.getuid?.() === 0)( + 'takes a path it cannot look at as present for a deny and absent for an allow', + () => { + const locked = join(root, 'locked') + const inside = join(locked, '[x]', 'secret') + mkdirSync(join(locked, '[x]'), { recursive: true }) + chmodSync(locked, 0o000) + try { + expect(literalReadings(inside, 'deny')).toEqual([name(inside)]) + expect(literalReadings(inside, 'allow')).toEqual([]) + } finally { + chmodSync(locked, 0o755) + } + }, + ) + + describe('a pattern beneath the folder', () => { + let ordinary: string + + beforeAll(() => { + ordinary = join(root, 'ordinary') + for (const base of [project, ordinary]) { + mkdirSync(join(base, 'deep', 'er'), { recursive: true }) + for (const file of ['a]x', 'deep/a]x', 'deep/er/b]x', 'deep/.env']) { + writeFileSync(join(base, file), '') + } + } + }) + + const found = (base: string, tail: string, kind: 'allow' | 'deny') => + literalReadings(base + tail, kind) + .flatMap(reading => + reading.glob + ? expandGlobPattern(reading.path, { anchor: reading.anchor }) + : [], + ) + .map(match => match.slice(base.length)) + .sort() + + it.each(['/**/.env', '/deep/*', '/**/er/*', '/*/*]x'])( + 'finds for %p what the same pattern finds beneath an ordinary folder', + tail => { + const expected = expandGlobPattern(ordinary + tail) + .map(match => match.slice(ordinary.length)) + .sort() + expect(expected.length).toBeGreaterThan(0) + expect(found(project, tail, 'deny')).toEqual(expected) + expect(found(project, tail, 'allow')).toEqual(expected) + }, + ) + + it.each([ + ['/**/[a*]x', ['/a]x', '/deep/a]x']], + ['/[a*]x', ['/a]x']], + ['/deep/[a*]x', ['/deep/a]x']], + ])( + 'matches %p, which cannot be split, by its spelling from the anchor on', + (tail, expected) => { + // A wildcard inside a bracket expression: the walk matches such a + // pattern against whole paths, and beneath an anchor those are paths + // from the anchor on. + expect( + expandGlobPattern(ordinary + tail) + .map(match => match.slice(ordinary.length)) + .sort(), + ).toEqual(expected) + expect(found(project, tail, 'deny')).toEqual(expected) + expect(found(project, tail, 'allow')).toEqual(expected) + }, + ) + + it('keeps a trailing separator, with which a pattern matches nothing', () => { + expect(expandGlobPattern(`${ordinary}/*/`)).toEqual([]) + expect(found(project, '/*/', 'allow')).toEqual([]) + expect(found(project, '/*/', 'deny')).toEqual([]) + }) + }) +}) + +describe('path entries as configured', () => { + it('tells spellings from marked paths', () => { + expect( + splitPathEntries([ + '/a/*.env', + { path: '/a/*.env', literal: true }, + '/b', + { path: '~/c', literal: true }, + ]), + ).toEqual({ spelled: ['/a/*.env', '/b'], marked: ['/a/*.env', '~/c'] }) + expect(splitPathEntries(undefined)).toEqual({ spelled: [], marked: [] }) + }) + + it.each([ + ['an object without the mark', { path: '/a' }], + ['a mark that is not true', { path: '/a', literal: false }], + ['a mark that is a string', { path: '/a', literal: 'true' }], + ['a path that is not a string', { path: 1, literal: true }], + ['an empty path', { path: '', literal: true }], + ['null', null], + ['a number', 1], + ['a list', ['/a']], + ])('refuses %s rather than guess', (_what, entry) => { + // initialize() does not run the schema, so this is the only check an + // embedder's hand-built config meets. + expect(() => + splitPathEntries([entry as unknown as FilesystemPathEntry]), + ).toThrow(TypeError) + }) + + it('keys a marked path apart from the same spelling, and by value', () => { + expect(pathEntryKey('/a')).not.toBe( + pathEntryKey({ path: '/a', literal: true }), + ) + expect(pathEntryKey({ path: '/a', literal: true })).toBe( + pathEntryKey(structuredClone({ path: '/a', literal: true as const })), + ) + }) + + it('folds the literal lists into the lists they are more entries of', () => { + expect( + readNamesOf({ + denyOnly: ['/d'], + allowWithinDeny: ['/d/a'], + unlistableDenyDirs: ['/u'], + literalDenyOnly: ['/l*'], + literalAllowWithinDeny: ['/d/l*'], + }), + ).toEqual({ + denyOnly: ['/d', '/l*'], + allowWithinDeny: ['/d/a', '/d/l*'], + unlistableDenyDirs: ['/u'], + }) + expect(readNamesOf({ denyOnly: ['/d'] })).toEqual({ denyOnly: ['/d'] }) + expect( + readNamesOf({ denyOnly: [], literalAllowWithinDeny: ['/a'] }), + ).toEqual({ denyOnly: [], allowWithinDeny: ['/a'] }) + expect( + writeNamesOf({ + allowOnly: ['/w'], + denyWithinAllow: ['/w/d'], + literalAllowOnly: ['/l*'], + literalDenyWithinAllow: ['/w/l*'], + }), + ).toEqual({ allowOnly: ['/w', '/l*'], denyWithinAllow: ['/w/d', '/w/l*'] }) + expect(readNamesOf(undefined)).toBeUndefined() + expect(writeNamesOf(undefined)).toBeUndefined() + }) + + it('counts a marked allow among what a write config allows', () => { + expect( + writeRootsOf({ allowOnly: ['/w'], literalAllowOnly: ['/l*'] }), + ).toEqual(['/w', '/l*']) + expect(writeRootsOf({ allowOnly: ['/w'] })).toEqual(['/w']) + }) +}) + +// ============================================================================ +// Linux: real commands under bubblewrap +// ============================================================================ + +describe.if(isLinux)( + 'Linux: entries inside a folder named [WIP] project', + () => { + const CAN_RUN = bwrapCanNamespace() + const savedCwd = process.cwd() + let root: string + let project: string + + beforeEach(() => { + root = freshRoot() + project = join(root, PROJECT) + mkdirSync(join(project, 'keep'), { recursive: true }) + mkdirSync(join(project, 'real'), { recursive: true }) + mkdirSync(join(project, 'secrets', 'public'), { recursive: true }) + mkdirSync(join(project, 'sub', 'deep'), { recursive: true }) + writeFileSync(join(project, 'keep', 'file'), 'original\n') + writeFileSync(join(project, 'real', 'file'), 'original\n') + writeFileSync(join(project, 'secret.txt'), 'SECRET-FILE\n') + writeFileSync(join(project, 'secrets', 'key'), 'SECRET-DIR\n') + writeFileSync(join(project, 'secrets', 'public', 'ok'), 'PUBLIC\n') + writeFileSync(join(project, '.env'), 'ENV-TOP\n') + writeFileSync(join(project, 'sub', '.env'), 'ENV-SUB\n') + writeFileSync(join(project, 'sub', 'deep', '.env'), 'ENV-DEEP\n') + writeFileSync(join(project, 'sub', 'readme'), 'README\n') + // Outside every allowed path, so the scan for dangerous files in the + // working directory adds no binds of its own. + process.chdir(root) + }) + + afterEach(async () => { + process.chdir(savedCwd) + SandboxManager.cleanupAfterCommand() + await SandboxManager.reset() + rmSync(root, { recursive: true, force: true }) + }) + + it.skipIf(!CAN_RUN)( + 'makes the project writable when it is allowed', + async () => { + const { stdout } = await sandboxed( + { allowWrite: [project] }, + `echo hi > ${q(join(project, 'new'))} && echo WROTE`, + { cwd: root }, + ) + expect(stdout).toContain('WROTE') + expect(readFileSync(join(project, 'new'), 'utf8')).toBe('hi\n') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'enforces a denyWrite inside it', + async () => { + const kept = join(project, 'keep', 'file') + const { stdout } = await sandboxed( + { allowWrite: [root], denyWrite: [join(project, 'keep')] }, + `echo tampered > ${q(kept)} && echo WROTE; ` + + `echo fine > ${q(join(project, 'sub', 'other'))} && echo SIBLING`, + { cwd: root }, + ) + expect(stdout).not.toContain('WROTE') + expect(stdout).toContain('SIBLING') + expect(readFileSync(kept, 'utf8')).toBe('original\n') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'hides a denyRead file and a denyRead directory inside it', + async () => { + const { stdout } = await sandboxed( + { denyRead: [join(project, 'secret.txt'), join(project, 'secrets')] }, + `cat ${q(join(project, 'secret.txt'))} ${q(join(project, 'secrets', 'key'))}; ` + + `cat ${q(join(project, 'sub', 'readme'))}`, + { cwd: root }, + ) + expect(stdout).not.toContain('SECRET-FILE') + expect(stdout).not.toContain('SECRET-DIR') + expect(stdout).toContain('README') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'opens an allowRead carve-out inside a denied directory', + async () => { + const { stdout } = await sandboxed( + { + denyRead: [join(project, 'secrets')], + allowRead: [join(project, 'secrets', 'public')], + }, + `cat ${q(join(project, 'secrets', 'key'))} ${q(join(project, 'secrets', 'public', 'ok'))}`, + { cwd: root }, + ) + expect(stdout).not.toContain('SECRET-DIR') + expect(stdout).toContain('PUBLIC') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN).each(['build*', 'notes (draft?)'])( + 'opens a carve-out inside a folder named %p, whose name a pattern matches as itself', + async name => { + // The deny is spelled without a `/**`: with one it is the pattern as + // well in such a folder, under which every entry beneath the + // directory keeps a mask of its own. + const folder = join(root, name) + mkdirSync(join(folder, 'secrets', 'public'), { recursive: true }) + writeFileSync(join(folder, 'secrets', 'key'), 'SECRET-DIR\n') + writeFileSync(join(folder, 'secrets', 'public', 'ok'), 'PUBLIC\n') + const { stdout } = await sandboxed( + { + denyRead: [join(folder, 'secrets')], + allowRead: [join(folder, 'secrets', 'public')], + }, + `cat ${q(join(folder, 'secrets', 'key'))} ${q(join(folder, 'secrets', 'public', 'ok'))}`, + { cwd: root }, + ) + expect(stdout).not.toContain('SECRET-DIR') + expect(stdout).toContain('PUBLIC') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'masks .env at every depth for a recursive deny beneath it', + async () => { + const { stdout } = await sandboxed( + { denyRead: [`${project}/**/.env`] }, + `cat ${q(join(project, '.env'))} ${q(join(project, 'sub', '.env'))} ${q(join(project, 'sub', 'deep', '.env'))}; ` + + `cat ${q(join(project, 'sub', 'readme'))}`, + { cwd: root }, + ) + expect(stdout).not.toMatch(/ENV-(TOP|SUB|DEEP)/) + expect(stdout).toContain('README') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'masks the same for a relative recursive deny when it is the cwd', + async () => { + process.chdir(project) + const { stdout } = await sandboxed( + { denyRead: ['**/.env'] }, + `cat .env sub/.env sub/deep/.env; cat sub/readme`, + { cwd: project }, + ) + expect(stdout).not.toMatch(/ENV-(TOP|SUB|DEEP)/) + expect(stdout).toContain('README') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'applies a deny beneath an allow that is spelled through a parent reference', + async () => { + // The allow is recorded where it resolves to. Left as spelled, the + // deny beneath it would be judged outside every allowed path and + // skipped. + const denied = join(project, 'real', 'file') + const { stdout } = await sandboxed( + { + allowWrite: [`${project}/keep/../real`], + denyWrite: [denied], + }, + `echo tampered > ${q(denied)} && echo WROTE; ` + + `echo fine > ${q(join(project, 'real', 'other'))} && echo SIBLING`, + { cwd: root }, + ) + expect(stdout).not.toContain('WROTE') + expect(stdout).toContain('SIBLING') + expect(readFileSync(denied, 'utf8')).toBe('original\n') + }, + WRAP_TIMEOUT_MS, + ) + + it('keeps the entries in the write config and resolves the read ones', async () => { + await initialize({ + denyRead: [join(project, 'secret.txt'), `${project}/**/.env`], + allowRead: [join(project, 'secrets', 'public')], + allowWrite: [project], + denyWrite: [join(project, 'keep'), `${project}/**/*.lock`], + }) + const write = SandboxManager.getFsWriteConfig() + expect(write.allowOnly).toContain(project) + // Write lists take no pattern on Linux, beneath this folder or any other. + expect(write.denyWithinAllow).toEqual([join(project, 'keep')]) + const read = SandboxManager.getFsReadConfig() + expect(read.denyOnly).toEqual([ + join(project, 'secret.txt'), + join(project, '.env'), + join(project, 'sub', '.env'), + join(project, 'sub', 'deep', '.env'), + ]) + expect(read.allowWithinDeny).toEqual([join(project, 'secrets', 'public')]) + }) + + it('still reports every write entry whose pattern is not applied', async () => { + // Whether an entry is also a name depends on the disk, which the + // command can change; what is reported does not. + await initialize({ + allowWrite: [project, { path: join(root, 'marked*'), literal: true }], + denyWrite: [join(project, 'keep'), `${project}/**/*.lock`], + denyRead: [`${project}/**/.env`, { path: '/*/x', literal: true }], + }) + expect(SandboxManager.getLinuxGlobPatternWarnings()).toEqual([ + project, + join(project, 'keep'), + `${project}/**/*.lock`, + ]) + }) + + it.skipIf(!CAN_RUN)( + 'takes the strings a direct caller hands the wrapper as names', + async () => { + const kept = join(project, 'keep', 'file') + const wrapped = await wrapCommandWithSandboxLinux({ + command: + `echo BOOTED; echo tampered > ${q(kept)} && echo WROTE; ` + + `cat ${q(join(project, 'secret.txt'))}; ` + + `echo fine > ${q(join(project, 'new'))} && echo SIBLING`, + needsNetworkRestriction: false, + readConfig: { denyOnly: [join(project, 'secret.txt')] }, + writeConfig: { + allowOnly: [`${project}/`], + denyWithinAllow: [join(project, 'keep')], + }, + }) + const result = spawnSync(wrapped, { + shell: true, + encoding: 'utf8', + cwd: root, + timeout: RUN_TIMEOUT_MS, + }) + expect(result.stdout).toContain('BOOTED') + expect(result.stdout).not.toContain('WROTE') + expect(result.stdout).not.toContain('SECRET-FILE') + expect(result.stdout).toContain('SIBLING') + expect(readFileSync(kept, 'utf8')).toBe('original\n') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN).each([ + ['*/../keep', 'keep/file'], + ['[ab]/..', 'secret.txt'], + ])( + 'makes nothing writable for a direct caller that spells %p, which is not there', + async (tail, target) => { + // Nothing beneath the project is named `*` or `[ab]`. Resolved as a + // name the string loses the component with its parent reference, and + // what is left names a directory that is there. + const kept = join(project, target) + const before = readFileSync(kept, 'utf8') + const wrapped = await wrapCommandWithSandboxLinux({ + command: `echo BOOTED; echo tampered > ${q(kept)} && echo WROTE; echo END`, + needsNetworkRestriction: false, + readConfig: { denyOnly: [] }, + writeConfig: { + allowOnly: [`${project}/${tail}`], + denyWithinAllow: [], + }, + }) + const result = spawnSync(wrapped, { + shell: true, + encoding: 'utf8', + cwd: root, + timeout: RUN_TIMEOUT_MS, + }) + expect(result.stdout).toContain('BOOTED') + expect(result.stdout).not.toContain('WROTE') + expect(result.stdout).toContain('END') + expect(readFileSync(kept, 'utf8')).toBe(before) + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'makes a path writable that is spelled through a folder that is not there, as beneath any folder', + async () => { + // The project's own name holds the glob characters and exists, so the + // string is the name of a path; a name is resolved, parent reference + // and all. + const { stdout } = await sandboxed( + { allowWrite: [`${project}/absent/../real`] }, + `echo hi > ${q(join(project, 'real', 'new'))} && echo WROTE; ` + + `echo hi > ${q(join(project, 'keep', 'new'))} && echo ESCAPED`, + { cwd: root }, + ) + expect(stdout).toContain('WROTE') + expect(stdout).not.toContain('ESCAPED') + }, + WRAP_TIMEOUT_MS, + ) + }, +) + +/** + * What exists is looked at when a command is wrapped, and an earlier command + * may have put it there. A deny takes every reading that may hold, so + * nothing the command creates can switch one off. An allow takes the path + * only as itself: a link with the name an entry spells is not the path. + */ +describe.if(isLinux)( + 'Linux: what a command can change before the next wrap', + () => { + const CAN_RUN = bwrapCanNamespace() + const savedCwd = process.cwd() + let root: string + let work: string + let vault: string + + beforeEach(() => { + root = freshRoot() + work = join(root, 'work') + vault = join(root, 'vault') + mkdirSync(join(work, 'proj', 'pub'), { recursive: true }) + mkdirSync(join(vault, 'pub'), { recursive: true }) + mkdirSync(join(vault, 'deep', 'pub'), { recursive: true }) + writeFileSync(join(work, 'proj', 'pub', 'ok'), 'PROJECT-PUBLIC\n') + writeFileSync(join(vault, 'pub', 'key'), 'VAULT-SECRET\n') + writeFileSync(join(vault, 'deep', 'pub', 'key'), 'VAULT-DEEP-SECRET\n') + process.chdir(root) + }) + + afterEach(async () => { + process.chdir(savedCwd) + SandboxManager.cleanupAfterCommand() + await SandboxManager.reset() + rmSync(root, { recursive: true, force: true }) + }) + + const readVault = (): string => + `cat ${q(join(vault, 'pub', 'key'))} ${q(join(vault, 'deep', 'pub', 'key'))} 2>/dev/null; echo END` + + it.skipIf(!CAN_RUN).each([ + ['work/*/pub', '*'], + ['work/*/**/pub', '*'], + ['work/[ab]/pub', '[ab]'], + ['work/?/pub', '?'], + ])( + 'does not open a denied directory through a link named like %p spells it', + async (entry, linkName) => { + const policy = { + denyRead: [vault], + allowRead: [join(root, entry)], + allowWrite: [work], + } + // The command may write `work`, so it can plant the link itself. + const planted = await sandboxed( + policy, + `ln -s ${q(vault)} ${q(join(work, linkName))} && echo PLANTED`, + { cwd: root }, + ) + expect(planted.stdout).toContain('PLANTED') + await initialize(policy) + const allowed = SandboxManager.getFsReadConfig().allowWithinDeny ?? [] + expect(allowed.filter(p => p.includes(linkName))).toEqual([]) + const { stdout } = await sandboxed(policy, readVault(), { cwd: root }) + expect(stdout).not.toContain('SECRET') + expect(stdout).toContain('END') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'does not open one through a link further along the path either', + async () => { + const policy = { + denyRead: [vault], + allowRead: [join(work, '[ab]', 'mid', 'pub')], + allowWrite: [work], + } + const planted = await sandboxed( + policy, + `mkdir ${q(join(work, '[ab]'))} && ln -s ${q(vault)} ${q(join(work, '[ab]', 'mid'))} && echo PLANTED`, + { cwd: root }, + ) + expect(planted.stdout).toContain('PLANTED') + const { stdout } = await sandboxed(policy, readVault(), { cwd: root }) + expect(stdout).not.toContain('SECRET') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN).each([ + ['[ab]/mid/*/pub', '[ab]', 'mid'], + ['[ab]/mid/**/key', '[ab]', 'mid'], + ['*/dist/*', '*', 'dist'], + ])( + 'does not open a denied directory through a link that a pattern spelled %p starts beneath', + async (entry, dirName, linkName) => { + // The directory is one the command made, with the name the entry + // spells, and the link inside it is what the pattern would be walked + // from. + const policy = { + denyRead: [vault], + allowRead: [join(work, entry)], + allowWrite: [work], + } + const planted = await sandboxed( + policy, + `mkdir ${q(join(work, dirName))} && ln -s ${q(vault)} ${q(join(work, dirName, linkName))} && echo PLANTED`, + { cwd: root }, + ) + expect(planted.stdout).toContain('PLANTED') + await initialize(policy) + expect(SandboxManager.getFsReadConfig().allowWithinDeny).toEqual([]) + const { stdout } = await sandboxed(policy, readVault(), { cwd: root }) + expect(stdout).not.toContain('SECRET') + expect(stdout).toContain('END') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'hides a file for a read deny spelled through a link with glob characters in its name', + async () => { + // For a deny a link of that name counts: the entry names the file, + // by whichever of its names the command asks for it. + const target = join(root, 'target') + mkdirSync(target) + writeFileSync(join(target, 'token'), 'THROUGH-LINK\n') + symlinkSync(target, join(root, 'li[n]k')) + const entry = join(root, 'li[n]k', 'token') + const policy = { denyRead: [entry] } + await initialize(policy) + expect(SandboxManager.getFsReadConfig().denyOnly).toEqual([entry]) + const { stdout } = await sandboxed( + policy, + `cat ${q(entry)} ${q(join(target, 'token'))} 2>/dev/null; echo END`, + { cwd: root }, + ) + expect(stdout).not.toContain('THROUGH-LINK') + expect(stdout).toContain('END') + }, + WRAP_TIMEOUT_MS, + ) + + it('adds no name for a write allow spelled through a link with glob characters in its name', async () => { + symlinkSync(vault, join(work, '[ab]')) + await initialize({ + allowWrite: [join(work, '[ab]', 'pub'), join(work, '[ab]')], + }) + expect( + SandboxManager.getFsWriteConfig().allowOnly.filter(p => + p.startsWith(root), + ), + ).toEqual([]) + }) + + it.skipIf(!CAN_RUN)( + 'keeps a class deny in force when a directory with the name of the class appears', + async () => { + const bracketed = join(root, PROJECT) + mkdirSync(join(bracketed, 'a'), { recursive: true }) + writeFileSync(join(bracketed, 'a', '.env'), 'ENV-A\n') + const policy = { + denyRead: [`${bracketed}/[ab]/**/.env`], + allowWrite: [bracketed], + } + const read = `cat ${q(join(bracketed, 'a', '.env'))} ${q(join(bracketed, '[ab]', '.env'))} 2>/dev/null; echo END` + expect( + (await sandboxed(policy, read, { cwd: root })).stdout, + ).not.toContain('ENV-') + const made = await sandboxed( + policy, + `mkdir ${q(join(bracketed, '[ab]'))} && echo ENV-LITERAL > ${q(join(bracketed, '[ab]', '.env'))} && echo MADE`, + { cwd: root }, + ) + expect(made.stdout).toContain('MADE') + expect( + (await sandboxed(policy, read, { cwd: root })).stdout, + ).not.toContain('ENV-') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN || process.getuid?.() === 0)( + 'keeps a write deny in force when the folder cannot be looked at', + async () => { + const locked = join(root, 'locked') + const kept = join(locked, PROJECT, 'keep') + mkdirSync(kept, { recursive: true }) + writeFileSync(join(kept, 'file'), 'original\n') + chmodSync(locked, 0o000) + try { + // The command runs as the same user, so it can undo the chmod. + const { stdout } = await sandboxed( + { allowWrite: [root], denyWrite: [kept] }, + `chmod 755 ${q(locked)}; echo tampered > ${q(join(kept, 'file'))} && echo WROTE; echo END`, + { cwd: root }, + ) + expect(stdout).not.toContain('WROTE') + expect(stdout).toContain('END') + } finally { + chmodSync(locked, 0o755) + } + expect(readFileSync(join(kept, 'file'), 'utf8')).toBe('original\n') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN || process.getuid?.() === 0)( + 'takes any write deny with glob characters for the path it spells when the folder cannot be looked at', + async () => { + // Whether anything in the folder has the name cannot be told, so an + // ordinary pattern is taken for a path as well, and is applied like + // a path that is not there: the name is kept from being created, and + // what the pattern would match is no more denied than anywhere else. + const locked = join(root, 'locked') + mkdirSync(locked) + writeFileSync(join(locked, 'a.pem'), 'original\n') + const wrapsWith = async ( + name: string, + ): Promise<{ denied: string[]; stdout: string }> => { + const policy = { + allowWrite: [root], + denyWrite: [join(locked, name)], + } + chmodSync(locked, 0o000) + try { + await initialize(policy) + const denied = SandboxManager.getFsWriteConfig().denyWithinAllow + const { stdout } = await sandboxed( + policy, + `chmod 755 ${q(locked)}; echo x > ${q(join(locked, name))} && echo CREATED; ` + + `echo t >> ${q(join(locked, 'a.pem'))} && echo WROTE; echo END`, + { cwd: root }, + ) + return { denied, stdout } + } finally { + chmodSync(locked, 0o755) + } + } + const leftBy = async (name: string): Promise => { + const policy = { + allowWrite: [root], + denyWrite: [join(locked, name)], + } + chmodSync(locked, 0o000) + try { + await sandboxed(policy, 'echo RAN', { cwd: root }) + } finally { + chmodSync(locked, 0o755) + } + const left = readdirSync(locked).filter(entry => entry !== 'a.pem') + for (const entry of left) { + rmSync(join(locked, entry), { recursive: true, force: true }) + } + return left + } + + for (const name of ['absent', '*.pem']) { + const { denied, stdout } = await wrapsWith(name) + expect(denied).toEqual([join(locked, name)]) + expect(stdout).not.toContain('CREATED') + expect(stdout).toContain('WROTE') + expect(stdout).toContain('END') + expect(existsSync(join(locked, name))).toBe(false) + } + // The folder cannot be cleaned by the host while it cannot be looked + // at, for a name and for a pattern alike. + const leftByName = await leftBy('absent') + expect(await leftBy('*.pem')).toEqual( + leftByName.map(entry => (entry === 'absent' ? '*.pem' : entry)), + ) + }, + WRAP_TIMEOUT_MS, + ) + }, +) + +// ============================================================================ +// macOS: the generated profile +// ============================================================================ + +describe.if(!isWindows)( + 'macOS profile: entries inside a folder named [WIP] project', + () => { + let root: string + let project: string + + beforeAll(() => { + root = freshRoot() + project = join(root, PROJECT) + mkdirSync(join(project, 'keep'), { recursive: true }) + mkdirSync(join(project, 'secrets', 'public'), { recursive: true }) + mkdirSync(join(project, 'sub'), { recursive: true }) + writeFileSync(join(project, 'secret.txt'), '') + writeFileSync(join(project, '.env'), '') + }) + + afterAll(() => { + rmSync(root, { recursive: true, force: true }) + }) + + it('allows writes to the project by subpath', () => { + const profile = macProfile(undefined, { + allowOnly: [project], + denyWithinAllow: [], + }) + const allow = ruleOf(profile, '(allow file-write*') + expect(allow).toContain(subpath(project)) + // The pattern reading is still there, as for any entry. + expect(allow).toContain(patternFilter(project, 'allow')) + }) + + it('denies a write inside it by subpath, and pins the folder', () => { + const kept = join(project, 'keep') + const profile = macProfile(undefined, { + allowOnly: [root], + denyWithinAllow: [kept], + }) + expect(ruleOf(profile, '(deny file-write*')).toContain(subpath(kept)) + const pinned = ruleOf( + profile, + '(deny file-write-unlink file-write-create', + ) + expect(pinned).toContain(subpath(kept)) + expect(pinned).toContain(literalFilter(project)) + expect(profile).toContain(patternFilter(kept, 'deny')) + }) + + it('denies a read of a file and of a directory inside it by subpath', () => { + const file = join(project, 'secret.txt') + const dir = join(project, 'secrets') + const profile = macProfile({ denyOnly: [file, dir] }, undefined) + const deny = ruleOf(profile, '(deny file-read*') + expect(deny).toContain(subpath(file)) + expect(deny).toContain(subpath(dir)) + expect(deny).toContain(patternFilter(file, 'deny')) + }) + + it('re-allows a carve-out inside a denied directory by subpath', () => { + const dir = join(project, 'secrets') + const open = join(dir, 'public') + const profile = macProfile( + { denyOnly: [dir], allowWithinDeny: [open] }, + undefined, + ) + expect(ruleOf(profile, '(allow file-read*\n')).toContain(subpath(open)) + // A deny nested in an allow lands again after it; here the allow is the + // nested one, so the deny is not repeated by name. + const denies = profile.split('(deny file-read*\n') + expect(denies[1]).toContain(subpath(dir)) + }) + + it('denies a recursive pattern beneath it with the folder escaped', () => { + const profile = macProfile( + { denyOnly: [`${project}/**/.env`] }, + undefined, + ) + const anchored = `^${escapedForRegex(project)}/(.*/)?\\.env(/.*)?$` + expect(profile).toContain(`(regex ${JSON.stringify(anchored)})`) + expect(profile).toContain(patternFilter(`${project}/**/.env`, 'deny')) + const regexes = emittedRegexes(profile) + for (const denied of [ + '.env', + 'sub/.env', + 'sub/deep/.env', + '.env/inside', + ]) { + expect(regexes.some(re => re.test(join(project, denied)))).toBe(true) + } + expect(regexes.some(re => re.test(join(project, 'sub', 'readme')))).toBe( + false, + ) + expect( + ruleOf(profile, '(deny file-write-unlink file-write-create'), + ).toContain(literalFilter(project)) + }) + + it('takes the project as a write root for the rules that keep denied paths in place', () => { + const profile = macProfile( + { denyOnly: [join(project, 'secrets')] }, + { allowOnly: [project], denyWithinAllow: [] }, + ) + expect( + ruleOf(profile, '(allow file-write-unlink file-write-create'), + ).toContain(subpath(project)) + const tail = profile.slice( + profile.indexOf('keep read-denied paths inside write roots in place'), + ) + expect(tail).toContain(subpath(join(project, 'secrets'))) + }) + + it('adds no reading for a link with the name an allow spells', () => { + const work = join(root, 'work') + mkdirSync(join(root, 'vault', 'pub'), { recursive: true }) + mkdirSync(work, { recursive: true }) + symlinkSync(join(root, 'vault'), join(work, '[ab]')) + const entry = join(work, '[ab]', 'pub') + const asAllow = macProfile( + { denyOnly: [join(root, 'vault')], allowWithinDeny: [entry] }, + { allowOnly: [entry], denyWithinAllow: [] }, + ) + expect(emittedSubpaths(asAllow)).not.toContain(entry) + expect(emittedSubpaths(asAllow)).not.toContain(join(root, 'vault', 'pub')) + const asDeny = macProfile({ denyOnly: [entry] }, undefined) + expect(ruleOf(asDeny, '(deny file-read*')).toContain(subpath(entry)) + const asWriteDeny = macProfile(undefined, { + allowOnly: [root], + denyWithinAllow: [entry], + }) + expect(ruleOf(asWriteDeny, '(deny file-write*')).toContain(subpath(entry)) + }) + + it('compiles an entry as before when none of it is a name on disk', () => { + const profile = macProfile( + { denyOnly: ['/srv/[ab]/secrets', '/srv/**/.env'] }, + { allowOnly: ['/srv/build*'], denyWithinAllow: ['/srv/*.lock'] }, + ) + expect( + emittedSubpaths(profile).filter(p => p.startsWith('/srv')), + ).toEqual([]) + }) + }, +) + +// ============================================================================ +// Both readings are both +// ============================================================================ + +describe.if(!isWindows)('an entry that is a pattern and a name', () => { + const CAN_RUN = isLinux && bwrapCanNamespace() + const savedCwd = process.cwd() + let root: string + let entry: string + const dirs = ['[ab]', 'a', 'b', 'c'] + + beforeAll(() => { + root = freshRoot() + entry = join(root, '[ab]', 'secret') + for (const dir of dirs) { + mkdirSync(join(root, dir), { recursive: true }) + writeFileSync(join(root, dir, 'secret'), `SECRET-${dir}\n`) + } + process.chdir(root) + }) + + afterAll(async () => { + process.chdir(savedCwd) + await SandboxManager.reset() + rmSync(root, { recursive: true, force: true }) + }) + + it.if(isLinux)( + 'resolves to the class matches and to the name on Linux', + async () => { + await initialize({ denyRead: [entry] }) + expect(SandboxManager.getFsReadConfig().denyOnly.sort()).toEqual( + ['[ab]', 'a', 'b'].map(dir => join(root, dir, 'secret')).sort(), + ) + }, + ) + + it.if(isLinux)( + 'lists a path once that the pattern and the name both give', + async () => { + const starred = join(root, 'c', '*') + writeFileSync(starred, '') + try { + await initialize({ denyRead: [starred], allowRead: [starred] }) + const read = SandboxManager.getFsReadConfig() + const both = [join(root, 'c', '*'), join(root, 'c', 'secret')].sort() + expect([...read.denyOnly].sort()).toEqual(both) + expect([...(read.allowWithinDeny ?? [])].sort()).toEqual(both) + } finally { + rmSync(starred) + } + }, + ) + + it.skipIf(!CAN_RUN)( + 'denies all three under bubblewrap', + async () => { + const { stdout } = await sandboxed( + { denyRead: [entry] }, + dirs.map(dir => `cat ${q(join(root, dir, 'secret'))}`).join('; '), + { cwd: root }, + ) + expect(stdout).not.toMatch(/SECRET-(\[ab\]|a|b)\n/) + expect(stdout).toContain('SECRET-c') + }, + WRAP_TIMEOUT_MS, + ) + + it('covers all three in the macOS profile', () => { + const profile = macProfile({ denyOnly: [entry] }, undefined) + const deny = ruleOf(profile, '(deny file-read*') + const regexes = emittedRegexes(deny) + const subpaths = emittedSubpaths(deny) + const covered = (p: string): boolean => + regexes.some(re => re.test(p)) || + subpaths.some(s => p === s || p.startsWith(s + '/')) + for (const dir of ['[ab]', 'a', 'b']) { + expect(covered(join(root, dir, 'secret'))).toBe(true) + } + expect(covered(join(root, 'c', 'secret'))).toBe(false) + }) +}) + +// ============================================================================ +// Entries with no such name on disk: nothing changes +// ============================================================================ + +/** + * Ordinary entries and ordinary patterns, under a root that does not exist. + * The expectations are what the code gave for them before an entry could + * have a second reading; they must stay what they are, byte for byte. That + * goes for every entry none of whose glob characters can be part of a name + * on disk. Beneath a directory that cannot be looked at they can, for all + * anyone can tell, and there a deny is also the path it spells: see "takes + * any write deny with glob characters for the path it spells" above. + */ +describe.if(!isWindows)( + 'entries none of which is a name with glob characters', + () => { + const ROOT = '/srt-literal-path-entries' + const lists = { + denyRead: [ + `${ROOT}/secrets`, + `${ROOT}/home/.ssh/**`, + `${ROOT}/proj/**/.env`, + `${ROOT}/proj/*.pem`, + `${ROOT}/proj/[ab]/key`, + `${ROOT}/proj/file?.txt`, + `${ROOT}/proj/**/build/**`, + ], + allowRead: [ + `${ROOT}/secrets/public`, + `${ROOT}/proj/**/public`, + `${ROOT}/home/.ssh/known_hosts`, + ], + allowWrite: [ + `${ROOT}/proj`, + `${ROOT}/out/**`, + `${ROOT}/build/*`, + `${ROOT}/cache/[0-9]`, + ], + denyWrite: [ + `${ROOT}/proj/.git`, + `${ROOT}/proj/**/node_modules`, + `${ROOT}/proj/locked/**`, + `${ROOT}/proj/?.lock`, + ], + } + const savedCwd = process.cwd() + + beforeAll(() => { + // The built-in write denies are spelled from the working directory and + // its ancestors. From the root they are the same on every host. + process.chdir('/') + }) + + afterAll(async () => { + process.chdir(savedCwd) + await SandboxManager.reset() + }) + + it.if(isLinux)( + 'gives the read and write config it gave before', + async () => { + await initialize(lists) + const read = SandboxManager.getFsReadConfig() + expect(read.denyOnly).toEqual([ + '/srt-literal-path-entries/secrets', + '/srt-literal-path-entries/home/.ssh', + ]) + expect(read.allowWithinDeny).toEqual([ + '/srt-literal-path-entries/secrets/public', + '/srt-literal-path-entries/home/.ssh/known_hosts', + ]) + expect(read.unlistableDenyDirs).toEqual([]) + expect(Object.keys(read).sort()).toEqual([ + 'allowWithinDeny', + 'denyOnly', + 'unlistableDenyDirs', + ]) + + const write = SandboxManager.getFsWriteConfig() + expect(write.allowOnly).toEqual([ + ...getDefaultWritePaths(), + '/srt-literal-path-entries/proj', + '/srt-literal-path-entries/out', + ]) + expect(write.denyWithinAllow).toEqual([ + '/srt-literal-path-entries/proj/.git', + '/srt-literal-path-entries/proj/locked', + ]) + expect('literalAllowOnly' in write).toBe(false) + // Absent or empty: the two mean the same for this list. + expect(write.literalDenyWithinAllow ?? []).toEqual([]) + + expect(SandboxManager.getLinuxGlobPatternWarnings()).toEqual([ + '/srt-literal-path-entries/build/*', + '/srt-literal-path-entries/cache/[0-9]', + '/srt-literal-path-entries/proj/**/node_modules', + '/srt-literal-path-entries/proj/?.lock', + ]) + }, + ) + + it('gives the macOS file rules it gave before', () => { + const wrapped = macProfile( + { denyOnly: lists.denyRead, allowWithinDeny: lists.allowRead }, + { allowOnly: lists.allowWrite, denyWithinAllow: lists.denyWrite }, + ) + expect(fileRulesOf(wrapped)).toBe(EXPECTED_MACOS_FILE_RULES) + }) + }, +) + +/** + * The file rules of the profile inside a wrapped command, with the log tag, + * which differs from one process to the next, replaced. + */ +function fileRulesOf(wrapped: string): string { + const from = wrapped.indexOf('; File read') + const to = wrapped.indexOf("' ", from) + return wrapped.slice(from, to).replace(/CMD64_.*?_SBX/g, '') +} + +const EXPECTED_MACOS_FILE_RULES = String.raw`; File read +(allow file-read*) +(deny file-read* + (subpath "/srt-literal-path-entries/secrets") + (regex "^/srt-literal-path-entries/home/\\.ssh/.*(/.*)?$") + (regex "^/srt-literal-path-entries/proj/(.*/)?\\.env(/.*)?$") + (regex "^/srt-literal-path-entries/proj/[^/]*\\.pem(/.*)?$") + (regex "^/srt-literal-path-entries/proj/[ab]/key(/.*)?$") + (regex "^/srt-literal-path-entries/proj/file[^/]\\.txt(/.*)?$") + (regex "^/srt-literal-path-entries/proj/(.*/)?build/.*(/.*)?$") + (with message "")) +(allow file-read* + (subpath "/srt-literal-path-entries/secrets/public") + (regex "^/srt-literal-path-entries/proj/(.*/)?public$") + (subpath "/srt-literal-path-entries/home/.ssh/known_hosts") + (with message "")) +(deny file-read* + (require-all (regex "^/srt-literal-path-entries/home/\\.ssh/.*(/.*)?$") (require-not (subpath "/srt-literal-path-entries/home/.ssh/known_hosts"))) + (regex "^/srt-literal-path-entries/proj/(.*/)?\\.env(/.*)?$") + (regex "^/srt-literal-path-entries/proj/[^/]*\\.pem(/.*)?$") + (regex "^/srt-literal-path-entries/proj/[ab]/key(/.*)?$") + (regex "^/srt-literal-path-entries/proj/file[^/]\\.txt(/.*)?$") + (regex "^/srt-literal-path-entries/proj/(.*/)?build/.*(/.*)?$") + (with message "")) +(allow file-read-metadata + (vnode-type DIRECTORY)) +(deny file-write-unlink file-write-create + (subpath "/srt-literal-path-entries/secrets") + (literal "/srt-literal-path-entries") + (regex "^/srt-literal-path-entries/home/\\.ssh/.*(/.*)?$") + (literal "/srt-literal-path-entries/home/.ssh") + (literal "/srt-literal-path-entries/home") + (regex "^/srt-literal-path-entries/proj/(.*/)?\\.env(/.*)?$") + (literal "/srt-literal-path-entries/proj") + (regex "^/srt-literal-path-entries/proj/[^/]*\\.pem(/.*)?$") + (regex "^/srt-literal-path-entries/proj/[ab]/key(/.*)?$") + (regex "^/srt-literal-path-entries/proj/file[^/]\\.txt(/.*)?$") + (regex "^/srt-literal-path-entries/proj/(.*/)?build/.*(/.*)?$") + (with message "")) +(allow file-write-unlink file-write-create + (subpath "/srt-literal-path-entries/proj") + (regex "^/srt-literal-path-entries/out/.*$") + (regex "^/srt-literal-path-entries/build/[^/]*$") + (regex "^/srt-literal-path-entries/cache/[0-9]$") + (with message "")) + +; File write +(allow file-write* + (subpath "/srt-literal-path-entries/proj") + (regex "^/srt-literal-path-entries/out/.*$") + (regex "^/srt-literal-path-entries/build/[^/]*$") + (regex "^/srt-literal-path-entries/cache/[0-9]$") + (with message "")) +(deny file-write* + (subpath "/srt-literal-path-entries/proj/.git") + (regex "^/srt-literal-path-entries/proj/(.*/)?node_modules(/.*)?$") + (regex "^/srt-literal-path-entries/proj/locked/.*(/.*)?$") + (regex "^/srt-literal-path-entries/proj/[^/]\\.lock(/.*)?$") + (subpath "/.gitconfig") + (regex "^/(.*/)?\\.gitconfig(/.*)?$") + (subpath "/.gitmodules") + (regex "^/(.*/)?\\.gitmodules(/.*)?$") + (subpath "/.bashrc") + (regex "^/(.*/)?\\.bashrc(/.*)?$") + (subpath "/.bash_profile") + (regex "^/(.*/)?\\.bash_profile(/.*)?$") + (subpath "/.zshrc") + (regex "^/(.*/)?\\.zshrc(/.*)?$") + (subpath "/.zprofile") + (regex "^/(.*/)?\\.zprofile(/.*)?$") + (subpath "/.profile") + (regex "^/(.*/)?\\.profile(/.*)?$") + (subpath "/.ripgreprc") + (regex "^/(.*/)?\\.ripgreprc(/.*)?$") + (subpath "/.mcp.json") + (regex "^/(.*/)?\\.mcp\\.json(/.*)?$") + (subpath "/.vscode") + (regex "^/(.*/)?\\.vscode/.*(/.*)?$") + (subpath "/.idea") + (regex "^/(.*/)?\\.idea/.*(/.*)?$") + (subpath "/.claude/commands") + (regex "^/(.*/)?\\.claude/commands/.*(/.*)?$") + (subpath "/.claude/agents") + (regex "^/(.*/)?\\.claude/agents/.*(/.*)?$") + (subpath "/.git/hooks") + (regex "^/(.*/)?\\.git/hooks/.*(/.*)?$") + (subpath "/.git/config") + (regex "^/(.*/)?\\.git/config(/.*)?$") + (with message "")) +(deny file-write-unlink file-write-create + (subpath "/srt-literal-path-entries/proj/.git") + (literal "/srt-literal-path-entries/proj") + (literal "/srt-literal-path-entries") + (regex "^/srt-literal-path-entries/proj/(.*/)?node_modules(/.*)?$") + (regex "^/srt-literal-path-entries/proj/locked/.*(/.*)?$") + (literal "/srt-literal-path-entries/proj/locked") + (regex "^/srt-literal-path-entries/proj/[^/]\\.lock(/.*)?$") + (subpath "/.gitconfig") + (regex "^/(.*/)?\\.gitconfig(/.*)?$") + (subpath "/.gitmodules") + (regex "^/(.*/)?\\.gitmodules(/.*)?$") + (subpath "/.bashrc") + (regex "^/(.*/)?\\.bashrc(/.*)?$") + (subpath "/.bash_profile") + (regex "^/(.*/)?\\.bash_profile(/.*)?$") + (subpath "/.zshrc") + (regex "^/(.*/)?\\.zshrc(/.*)?$") + (subpath "/.zprofile") + (regex "^/(.*/)?\\.zprofile(/.*)?$") + (subpath "/.profile") + (regex "^/(.*/)?\\.profile(/.*)?$") + (subpath "/.ripgreprc") + (regex "^/(.*/)?\\.ripgreprc(/.*)?$") + (subpath "/.mcp.json") + (regex "^/(.*/)?\\.mcp\\.json(/.*)?$") + (subpath "/.vscode") + (regex "^/(.*/)?\\.vscode/.*(/.*)?$") + (subpath "/.idea") + (regex "^/(.*/)?\\.idea/.*(/.*)?$") + (subpath "/.claude/commands") + (literal "/.claude") + (regex "^/(.*/)?\\.claude/commands/.*(/.*)?$") + (subpath "/.claude/agents") + (regex "^/(.*/)?\\.claude/agents/.*(/.*)?$") + (subpath "/.git/hooks") + (literal "/.git") + (regex "^/(.*/)?\\.git/hooks/.*(/.*)?$") + (subpath "/.git/config") + (regex "^/(.*/)?\\.git/config(/.*)?$") + (with message "")) + +; File read: keep read-denied paths inside write roots in place +(deny file-write-unlink + (require-all (regex "^/srt-literal-path-entries/home/\\.ssh/.*(/.*)?$") (require-not (subpath "/srt-literal-path-entries/home/.ssh/known_hosts"))) + (regex "^/srt-literal-path-entries/proj/(.*/)?\\.env(/.*)?$") + (regex "^/srt-literal-path-entries/proj/[^/]*\\.pem(/.*)?$") + (regex "^/srt-literal-path-entries/proj/[ab]/key(/.*)?$") + (regex "^/srt-literal-path-entries/proj/file[^/]\\.txt(/.*)?$") + (regex "^/srt-literal-path-entries/proj/(.*/)?build/.*(/.*)?$") + (with message ""))` + +// ============================================================================ +// The marker +// ============================================================================ + +describe.if(!isWindows)('an entry marked literal', () => { + const CAN_RUN = isLinux && bwrapCanNamespace() + const savedCwd = process.cwd() + let root: string + let dir: string + let starred: string + + beforeEach(() => { + root = freshRoot() + dir = join(root, 'x') + starred = join(dir, '*.env') + mkdirSync(dir, { recursive: true }) + writeFileSync(join(dir, 'a.env'), 'ENV-A\n') + process.chdir(root) + }) + + afterEach(async () => { + process.chdir(savedCwd) + SandboxManager.cleanupAfterCommand() + await SandboxManager.reset() + rmSync(root, { recursive: true, force: true }) + }) + + const marked = (path: string): FilesystemPathEntry => ({ + path, + literal: true, + }) + + it('travels in the literal lists, as spelled, and is never expanded', async () => { + await initialize({ + denyRead: [marked(starred), marked('~/x/[y]')], + allowRead: [marked(join(dir, 'a?'))], + allowWrite: [marked(join(root, 'out*')), root], + denyWrite: [marked(join(root, 'keep/**'))], + }) + const read = SandboxManager.getFsReadConfig() + expect(read.denyOnly).toEqual([]) + expect(read.allowWithinDeny).toEqual([]) + expect(read.literalDenyOnly).toEqual([starred, '~/x/[y]']) + expect(read.literalAllowWithinDeny).toEqual([join(dir, 'a?')]) + const write = SandboxManager.getFsWriteConfig() + expect(write.allowOnly).toEqual([...getDefaultWritePaths(), root]) + expect(write.literalAllowOnly).toEqual([join(root, 'out*')]) + expect(write.denyWithinAllow).toEqual([]) + // A `/**` at the end of a marked path is part of the name. + expect(write.literalDenyWithinAllow).toEqual([join(root, 'keep/**')]) + if (isLinux) + expect(SandboxManager.getLinuxGlobPatternWarnings()).toEqual([]) + }) + + it.skipIf(!CAN_RUN)( + 'denies the file of that name and not what the pattern would match', + async () => { + const read = `cat ${q(join(dir, 'a.env'))}; cat ${q(starred)} 2>/dev/null; echo END` + const before = await sandboxed({ denyRead: [marked(starred)] }, read, { + cwd: root, + }) + expect(before.stdout).toContain('ENV-A') + writeFileSync(starred, 'ENV-STAR\n') + const after = await sandboxed({ denyRead: [marked(starred)] }, read, { + cwd: root, + }) + expect(after.stdout).toContain('ENV-A') + expect(after.stdout).not.toContain('ENV-STAR') + }, + WRAP_TIMEOUT_MS, + ) + + it('compiles to a subpath and to no regex in the macOS profile', () => { + const profile = wrapCommandWithSandboxMacOS({ + command: 'true', + needsNetworkRestriction: false, + readConfig: { + denyOnly: [], + literalDenyOnly: [starred], + literalAllowWithinDeny: [join(dir, 'a?')], + }, + writeConfig: { + allowOnly: [], + denyWithinAllow: [], + literalAllowOnly: [join(root, 'out*')], + literalDenyWithinAllow: [join(root, 'keep[1]')], + }, + }) + expect(ruleOf(profile, '(deny file-read*')).toContain(subpath(starred)) + expect(ruleOf(profile, '(allow file-read*\n')).toContain( + subpath(join(dir, 'a?')), + ) + expect(ruleOf(profile, '(allow file-write*')).toContain( + subpath(join(root, 'out*')), + ) + expect(ruleOf(profile, '(deny file-write*')).toContain( + subpath(join(root, 'keep[1]')), + ) + // What the characters of each path would match as a pattern is matched + // by no regex of the profile. + const regexes = emittedRegexes(profile) + for (const matched of [ + join(dir, 'a.env'), + join(dir, 'ab'), + join(root, 'outX'), + join(root, 'keep1'), + ]) { + expect(regexes.some(re => re.test(matched))).toBe(false) + } + }) + + it('is a read restriction on its own in the macOS profile', () => { + const wrapped = wrapCommandWithSandboxMacOS({ + command: 'true', + needsNetworkRestriction: false, + readConfig: { denyOnly: [], literalDenyOnly: [starred] }, + writeConfig: undefined, + }) + expect(wrapped).toContain(subpath(starred)) + }) + + it.skipIf(!CAN_RUN)( + 'is a read restriction on its own for the Linux wrapper', + async () => { + writeFileSync(starred, 'ENV-STAR\n') + const wrapped = await wrapCommandWithSandboxLinux({ + command: `cat ${q(starred)} ${q(join(dir, 'a.env'))}`, + needsNetworkRestriction: false, + readConfig: { denyOnly: [], literalDenyOnly: [starred] }, + writeConfig: undefined, + }) + const result = spawnSync(wrapped, { + shell: true, + encoding: 'utf8', + cwd: root, + timeout: RUN_TIMEOUT_MS, + }) + expect(result.stdout).toContain('ENV-A') + expect(result.stdout).not.toContain('ENV-STAR') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'makes a marked path writable, and a marked path inside it read-only', + async () => { + const out = join(root, 'out*') + mkdirSync(join(out, 'keep[1]'), { recursive: true }) + writeFileSync(join(out, 'keep[1]', 'file'), 'original\n') + mkdirSync(join(root, 'outside')) + const { stdout } = await sandboxed( + { + allowWrite: [marked(out)], + denyWrite: [marked(join(out, 'keep[1]'))], + }, + `echo hi > ${q(join(out, 'new'))} && echo WROTE; ` + + `echo tampered > ${q(join(out, 'keep[1]', 'file'))} && echo TAMPERED; ` + + `echo hi > ${q(join(root, 'outside', 'new'))} && echo ESCAPED`, + { cwd: root }, + ) + expect(stdout).toContain('WROTE') + expect(stdout).not.toContain('TAMPERED') + expect(stdout).not.toContain('ESCAPED') + expect(readFileSync(join(out, 'keep[1]', 'file'), 'utf8')).toBe( + 'original\n', + ) + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'denies a write to a path that is not there yet, in a folder that is not there yet', + async () => { + // An absent deny path gets a placeholder at its first missing + // component, which keeps the command from creating it; the placeholder + // is removed from the host once the command is done. + const folder = join(root, '[new]') + const secret = join(folder, 'secret') + const { stdout } = await sandboxed( + { allowWrite: [root], denyWrite: [marked(secret)] }, + `mkdir -p ${q(folder)} 2>/dev/null; echo x > ${q(secret)} && echo CREATED; ` + + `echo fine > ${q(join(root, 'other'))} && echo SIBLING`, + { cwd: root, keepMountPoints: true }, + ) + expect(stdout).not.toContain('CREATED') + expect(stdout).toContain('SIBLING') + expect(existsSync(secret)).toBe(false) + SandboxManager.cleanupAfterCommand() + expect(existsSync(folder)).toBe(false) + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'denies a read of a path that is not there yet from the wrap after it appears', + async () => { + // Nothing is mounted for a read deny whose path is absent, marked or + // not: there is nothing to hide. The path is looked at again for the + // next command. + const late = join(root, '[late]', 'secret') + const policy = { denyRead: [marked(late)] } + const read = `cat ${q(late)} 2>/dev/null; echo END` + expect((await sandboxed(policy, read, { cwd: root })).stdout).toContain( + 'END', + ) + mkdirSync(join(root, '[late]')) + writeFileSync(late, 'LATE-SECRET\n') + const { stdout } = await sandboxed(policy, read, { cwd: root }) + expect(stdout).not.toContain('LATE-SECRET') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'keeps the masks of a deny pattern beneath a marked allow', + async () => { + // The matches of a deny pattern are collapsed: one that a directory + // match above it already hides gets no mount of its own, unless a path + // that is bound back over that directory lies between the two. A + // marked allow is bound back like any other, so the match beneath it + // has to keep its mask. + const vault = join(root, 'tree', 'vault.key') + const open = join(vault, 'open[1]') + mkdirSync(open, { recursive: true }) + writeFileSync(join(vault, 'top'), 'TOP\n') + writeFileSync(join(open, 'ok'), 'OPEN\n') + writeFileSync(join(open, 'inner.key'), 'INNER\n') + const { stdout } = await sandboxed( + { + denyRead: [`${root}/tree/**/*.key`], + allowRead: [marked(open)], + }, + `cat ${q(join(vault, 'top'))} ${q(join(open, 'inner.key'))} 2>/dev/null; ` + + `cat ${q(join(open, 'ok'))}`, + { cwd: root }, + ) + expect(stdout).not.toContain('TOP') + expect(stdout).not.toContain('INNER') + expect(stdout).toContain('OPEN') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'keeps them beneath a marked write allow too', + async () => { + const vault = join(root, 'tree', 'vault.key') + const out = join(vault, 'out[1]') + mkdirSync(out, { recursive: true }) + writeFileSync(join(vault, 'top'), 'TOP\n') + writeFileSync(join(out, 'inner.key'), 'INNER\n') + const { stdout } = await sandboxed( + { + denyRead: [`${root}/tree/**/*.key`], + allowWrite: [marked(out)], + }, + `cat ${q(join(vault, 'top'))} ${q(join(out, 'inner.key'))} 2>/dev/null; ` + + `echo hi > ${q(join(out, 'new'))} && echo WROTE`, + { cwd: root }, + ) + expect(stdout).not.toContain('TOP') + expect(stdout).not.toContain('INNER') + expect(stdout).toContain('WROTE') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'reaches the wrap from a per-command config', + async () => { + writeFileSync(starred, 'ENV-STAR\n') + const { stdout } = await sandboxed( + {}, + `cat ${q(join(dir, 'a.env'))}; cat ${q(starred)} 2>/dev/null; echo END`, + { cwd: root, perWrap: { denyRead: [marked(starred)] } }, + ) + expect(stdout).toContain('ENV-A') + expect(stdout).not.toContain('ENV-STAR') + }, + WRAP_TIMEOUT_MS, + ) + + it.skipIf(!CAN_RUN)( + 'reaches the wrap from a per-command config in each of the four lists', + async () => { + const out = join(root, 'out*') + const kept = join(out, 'keep[1]', 'file') + const secrets = join(root, 'secrets?') + const open = join(secrets, 'open[1]') + mkdirSync(join(out, 'keep[1]'), { recursive: true }) + mkdirSync(open, { recursive: true }) + writeFileSync(kept, 'original\n') + writeFileSync(join(secrets, 'key'), 'SECRET\n') + writeFileSync(join(open, 'ok'), 'OPEN\n') + const { stdout } = await sandboxed( + {}, + `echo hi > ${q(join(out, 'new'))} && echo WROTE; ` + + `echo tampered > ${q(kept)} && echo TAMPERED; ` + + `cat ${q(join(secrets, 'key'))} ${q(join(open, 'ok'))} 2>/dev/null; echo END`, + { + cwd: root, + perWrap: { + denyRead: [marked(secrets)], + allowRead: [marked(open)], + allowWrite: [marked(out)], + denyWrite: [marked(join(out, 'keep[1]'))], + }, + }, + ) + expect(stdout).toContain('WROTE') + expect(stdout).not.toContain('TAMPERED') + expect(stdout).not.toContain('SECRET') + expect(stdout).toContain('OPEN') + expect(stdout).toContain('END') + expect(readFileSync(kept, 'utf8')).toBe('original\n') + }, + WRAP_TIMEOUT_MS, + ) + + it.if(isLinux)( + 'is in the lists the violation monitor judges a write by', + async () => { + const out = join(root, 'out*') + const kept = join(out, 'keep[1]') + mkdirSync(kept, { recursive: true }) + let handedOver: LinuxViolationMonitorOptions | undefined + const spy = spyOn( + linuxViolationMonitorModule, + 'startLinuxSandboxViolationMonitor', + ).mockImplementation((_callback, opts) => { + handedOver = opts + return { + observeSocketPath: undefined, + ready: Promise.resolve(), + stop: () => {}, + } + }) + try { + await SandboxManager.reset() + await SandboxManager.initialize( + { + network: noNetwork, + filesystem: { + denyRead: [], + allowWrite: [marked(out)], + denyWrite: [marked(kept)], + }, + }, + undefined, + true, + ) + } finally { + spy.mockRestore() + } + expect(handedOver?.allowWritePaths).toContain(out) + expect(handedOver?.denyWritePaths).toContain(kept) + + // The listener itself, with what the manager handed over: a write the + // sandbox permits is no violation, and one it refuses is. + const lines: string[] = [] + const monitor = startLinuxSandboxViolationMonitor( + violation => lines.push(violation.line), + handedOver!, + ) + await monitor.ready + try { + await new Promise((resolve, reject) => { + const client = connect(monitor.observeSocketPath!, () => { + client.write( + [join(out, 'new'), join(kept, 'file')] + .map(path => JSON.stringify({ syscall: 'openat', path })) + .join('\n') + '\n', + ) + client.end() + }) + client.on('close', () => resolve()) + client.on('error', reject) + }) + await new Promise(resolve => setTimeout(resolve, 50)) + expect(lines).toEqual([`deny openat ${join(kept, 'file')}`]) + } finally { + monitor.stop() + } + }, + WRAP_TIMEOUT_MS, + ) + + it('comes back from getConfig() and updateConfig() as it went in', async () => { + const filesystem = { + denyRead: [marked(starred), join(dir, 'plain')], + allowRead: [marked(join(dir, 'a?'))], + allowWrite: [marked(join(root, 'out*'))], + denyWrite: [marked(join(root, 'keep[1]'))], + } + await initialize(filesystem) + expect(SandboxManager.getConfig()?.filesystem).toEqual(filesystem) + const updated = { + ...filesystem, + denyRead: [marked(join(dir, '[later]'))], + } + SandboxManager.updateConfig({ network: noNetwork, filesystem: updated }) + expect(SandboxManager.getConfig()?.filesystem).toEqual(updated) + expect(SandboxManager.getFsReadConfig().literalDenyOnly).toEqual([ + join(dir, '[later]'), + ]) + }) + + it('comes back from a settings file and from a control line as it went in', () => { + const config = { + network: noNetwork, + filesystem: { + denyRead: [marked(starred), '/plain/*.pem'], + allowRead: [marked('~/a?')], + allowWrite: [marked('./out*')], + denyWrite: [marked(join(root, 'keep[1]/'))], + }, + } + const file = join(root, 'settings.json') + writeFileSync(file, JSON.stringify(config)) + const loaded = loadConfig(file) + expect(loaded.kind).toBe('ok') + expect(loaded.kind === 'ok' && loaded.config.filesystem).toEqual( + config.filesystem, + ) + expect(loadConfigFromString(JSON.stringify(config))?.filesystem).toEqual( + config.filesystem, + ) + }) + + it('is refused by a settings file when the mark is not the mark', () => { + const file = join(root, 'settings.json') + writeFileSync( + file, + JSON.stringify({ + network: noNetwork, + filesystem: { + denyRead: [{ path: starred, literal: false }], + allowWrite: [], + denyWrite: [], + }, + }), + ) + const loaded = loadConfig(file) + expect(loaded.kind).toBe('invalid') + expect(loaded.kind === 'invalid' && loaded.reason).toContain( + 'filesystem.denyRead.0: Expected a path, or { "path": "", "literal": true }', + ) + }) + + it('is refused at the wrap when a config that skipped the schema holds half a mark', async () => { + await initialize({ + denyRead: [{ path: starred } as unknown as FilesystemPathEntry], + }) + const refusal = 'must be a path, or { path, literal: true }' + expect(() => SandboxManager.getFsReadConfig()).toThrow(refusal) + expect(() => SandboxManager.getFsWriteConfig()).toThrow(refusal) + // eslint-disable-next-line @typescript-eslint/await-thenable -- bun:test types .rejects.toThrow() as void; the await is required at runtime + await expect(SandboxManager.wrapWithSandbox('true')).rejects.toThrow( + refusal, + ) + }) +}) + +describe.if(!isWindows)( + 'default write paths under read rules with a name in them', + () => { + const script = (body: string): string => ` + const { getDefaultWritePaths } = await import(${JSON.stringify( + join(import.meta.dir, '../../src/sandbox/sandbox-utils.ts'), + )}) + const { SandboxManager } = await import(${JSON.stringify( + join(import.meta.dir, '../../src/sandbox/sandbox-manager.ts'), + )}) + const kept = paths => paths.filter(p => p.endsWith('/debug') || p.endsWith('/_logs')).map(p => p.split('/').slice(-2).join('/')) + ${body}` + + function under(home: string, body: string): unknown { + const result = spawnSync(process.execPath, ['-e', script(body)], { + env: { ...process.env, HOME: home }, + encoding: 'utf8', + timeout: WRAP_TIMEOUT_MS, + }) + if (result.status !== 0) throw new Error(result.stderr) + return JSON.parse(result.stdout.trim().split('\n').at(-1) ?? '') + } + + let root: string + + beforeAll(() => { + root = freshRoot() + for (const home of ['home', '[home]']) { + mkdirSync(join(root, home, '.claude', 'debug'), { recursive: true }) + mkdirSync(join(root, home, '.npm', '_logs'), { recursive: true }) + } + }) + + afterAll(() => { + rmSync(root, { recursive: true, force: true }) + }) + + it('drops a directory a marked deny covers and keeps one a marked allow re-opens', () => { + expect( + under( + join(root, 'home'), + `console.log(JSON.stringify({ + denied: kept(getDefaultWritePaths({ denyRead: [{ path: '~/.claude', literal: true }] })), + reopened: kept(getDefaultWritePaths({ + denyRead: [{ path: '~/.claude', literal: true }], + allowRead: [{ path: '~/.claude/debug', literal: true }], + })), + asPattern: kept(getDefaultWritePaths({ denyRead: [{ path: '~/.c*', literal: true }] })), + }))`, + ), + ).toEqual({ + denied: ['.npm/_logs'], + reopened: ['.npm/_logs', '.claude/debug'], + asPattern: ['.npm/_logs', '.claude/debug'], + }) + }) + + it('counts a deny spelled inside a home directory with brackets in its name', () => { + const home = join(root, '[home]') + expect( + under( + home, + `const config = filesystem => ({ network: { allowedDomains: [], deniedDomains: [] }, filesystem: { allowWrite: [], denyWrite: [], ...filesystem } }) + await SandboxManager.initialize(config({ denyRead: [${JSON.stringify(join(home, '.claude'))}] }), undefined, false) + const denied = kept(SandboxManager.getFsWriteConfig().allowOnly) + await SandboxManager.reset() + await SandboxManager.initialize(config({ + denyRead: [${JSON.stringify(join(home, '.claude'))}], + allowRead: [${JSON.stringify(join(home, '.claude', 'debug'))}], + }), undefined, false) + const reopened = kept(SandboxManager.getFsWriteConfig().allowOnly) + await SandboxManager.reset() + console.log(JSON.stringify({ denied, reopened })) + process.exit(0)`, + ), + ).toEqual({ + denied: ['.npm/_logs'], + reopened: ['.npm/_logs', '.claude/debug'], + }) + }) + }, +) From c0abeccd4b018a7e086e50a5dd1ff310663f4a74 Mon Sep 17 00:00:00 2001 From: Ron Leizrowice Date: Fri, 25 Sep 2026 22:16:47 +0000 Subject: [PATCH 3/4] README: paths inside a folder named like a pattern, and the literal marker --- README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6116ecb15..a067d8539 100644 --- a/README.md +++ b/README.md @@ -435,7 +435,7 @@ Examples: bubblewrap binds concrete paths, so glob support is narrower than on macOS: -- `allowWrite` / `denyWrite` take literal paths. A trailing `/**` is dropped (`src/**` means `src`); any other glob pattern there is skipped. +- `allowWrite` / `denyWrite` take literal paths. A trailing `/**` is dropped (`src/**` means `src`); any other glob pattern there is skipped, unless the entry is also the name of a path that exists (see "All platforms" below), and then that path is what is applied. - `denyRead` / `allowRead` accept the same glob syntax as macOS, expanded to the entries that exist when the command is wrapped, so a file that appears later is not covered. The pattern needs a literal directory to start from (a relative pattern starts at the current directory): one with a wildcard in its first path component, such as `/**/*.pem` or `/opt*/keys/**`, is skipped on Linux. Only directories the pattern can match beneath are listed (`certs/*.pem` lists `certs` alone). - A directory matched by a `denyRead` pattern ending in `/**` that holds at least one entry when the command is wrapped becomes one tmpfs mount, like a directory listed in `denyRead` literally: inside the sandbox it is an EMPTY WRITABLE directory, so a command that used to write through a read-denied `build/` still writes, into the tmpfs, and loses that output when the command exits. A file added to the directory on the host afterwards is hidden too. A matched directory that is empty when the command is wrapped gets no mount (a matched symlink to a directory always gets one, on the directory it leads to). An `allowRead` beneath a mounted directory is bound back over the tmpfs, but each entry beneath it that the pattern matches keeps its own mask: under a `/**` pattern that is every entry there, so only what is created beneath the `allowRead` later is readable. - A directory the expansion cannot list is denied as a whole, and nothing is bound back beneath the mount that hides it, `allowRead` and `allowWrite` paths included: what the pattern matches under them cannot be found. A `denyRead` entry that cannot be inspected (its parent directory is readable but not searchable, say), or that leads to `/`, hides the nearest directory above it instead, in the same way. @@ -455,6 +455,11 @@ Examples: - Paths can be absolute (e.g., `/home/user/.ssh`) or relative to the current working directory (e.g., `./src`) - `~` 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 `/**`. +- 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, 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 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`. 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. #### Other Configuration @@ -621,7 +626,7 @@ Running under a distinct user SID structurally closes the surrogate-spawn class - `filesystem.allowRead` → an inheriting `READ|EXECUTE` ALLOW ACE - `filesystem.denyRead` / `filesystem.denyWrite` → an inheriting DENY ACE on the target, plus an inheriting `FILE_DELETE_CHILD` DENY on its parent — together with the withheld `FILE_DELETE_CHILD` on the working-tree grant, this stops the sandboxed process from renaming or deleting a denied path via its parent directory -`reset()` removes every ACE this session added (refcounted across this user's concurrent hosts via the per-user session DB; a crash-recovery pass on the next `initialize()` cleans up after an unclean exit). Directory targets are supported (the ACEs inherit to the whole subtree). Glob patterns are expanded to concrete paths at `initialize()` time — a matching path that appears later is not covered. +`reset()` removes every ACE this session added (refcounted across this user's concurrent hosts via the per-user session DB; a crash-recovery pass on the next `initialize()` cleans up after an unclean exit). Directory targets are supported (the ACEs inherit to the whole subtree). Glob patterns are expanded to concrete paths at `initialize()` time — a matching path that appears later is not covered. In an entry without `*` or `?`, `[` and `]` are characters of the name. In a pattern they are read as a character class, as on the other platforms, and the pattern is also expanded beneath a folder that has them in its name. ### TLS termination on Windows From 3386decceb726a1a4fbc715e32a3b6984a6faf6f Mon Sep 17 00:00:00 2001 From: Ron Leizrowice Date: Mon, 28 Sep 2026 13:50:22 +0000 Subject: [PATCH 4/4] Shorten the comments this change adds 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. --- src/sandbox/linux-sandbox-utils.ts | 15 +-- src/sandbox/macos-sandbox-utils.ts | 56 ++++------ src/sandbox/path-entries.ts | 127 +++++++++------------- src/sandbox/read-deny-glob.ts | 3 +- src/sandbox/sandbox-config.ts | 5 +- src/sandbox/sandbox-manager.ts | 49 ++++----- src/sandbox/sandbox-schemas.ts | 20 +--- src/sandbox/sandbox-utils.ts | 30 ++--- src/sandbox/windows-sandbox-utils.ts | 13 +-- test/sandbox/glob-expand.test.ts | 11 +- test/sandbox/literal-path-entries.test.ts | 108 ++++++------------ 11 files changed, 164 insertions(+), 273 deletions(-) diff --git a/src/sandbox/linux-sandbox-utils.ts b/src/sandbox/linux-sandbox-utils.ts index f5d531694..b8553d603 100644 --- a/src/sandbox/linux-sandbox-utils.ts +++ b/src/sandbox/linux-sandbox-utils.ts @@ -1951,9 +1951,8 @@ async function generateFilesystemArgs( // Allow writes to specific paths for (const pathPattern of writeConfig.allowOnly || []) { // Every path here is a name, and normalizePathForSandbox strips a - // trailing slash from a name, so the strip below has nothing left to - // do; it stays as the guard of what is recorded. Allow paths are - // recorded slash-free because every + // name's trailing slash; the strip below stays as a guard. Allow paths + // are recorded slash-free because every // downstream comparison — the deny loop's within-allowlist gate, // findSymlinkInPath's mask scoping, the emission filter's re-expose // check, the denyRead re-bind and its allowRead skip, and the stub-skip @@ -2981,12 +2980,10 @@ export async function wrapCommandWithSandboxLinux( observeSocketPath, abortSignal, } = params - // bubblewrap takes no patterns, so every path that reaches this backend is - // a name: the manager has expanded or dropped what was a pattern. The - // literal lists are therefore more entries of the lists they belong to, and - // are folded into them here, once, so that nothing below can read a list - // and miss them. A caller of this function may still hand it a pattern: - // in `allowOnly` one that is no name is dropped by the fold. + // bubblewrap takes no patterns, so every path here is a name. The literal + // lists are folded into the lists they belong to, once, so that nothing + // below can read a list and miss them; a pattern in `allowOnly` that is no + // name is dropped by the fold (see writeNamesOf). const readConfig = readNamesOf(params.readConfig) const writeConfig = writeNamesOf(params.writeConfig) diff --git a/src/sandbox/macos-sandbox-utils.ts b/src/sandbox/macos-sandbox-utils.ts index 08982949f..34c477fca 100644 --- a/src/sandbox/macos-sandbox-utils.ts +++ b/src/sandbox/macos-sandbox-utils.ts @@ -163,16 +163,16 @@ function pathFilter(entry: PathEntry): string { /** * Compiles `entry`'s pattern with one of the shared string compilers in - * `sandbox-utils.ts`. An entry anchored at a directory (`anchor`) keeps - * that directory as a literal — the cwd, or a directory a caller's pattern - * lies beneath, is a name on disk and may itself contain `[`, `*` or `?` — - * so only the tail below it goes through the compiler, and the anchor is - * spliced back in escaped. Every compiler there returns '^…$'. + * `sandbox-utils.ts`. An entry anchored at a directory + * (`anchor`) keeps that directory as a literal — it is a name on disk + * and may itself contain `[`, `*` or `?` — so only the tail below it goes + * through the compiler, and the anchor is spliced back in escaped. Every + * compiler there returns '^…$'. * * This is the whole difference between the entry-taking wrappers here and * the string-taking compilers they call: those see a spelling in which - * every character is glob syntax, which is right for the pattern reading - * of what a caller wrote and wrong for a name on disk. + * every character is glob syntax, which is right for what a caller wrote + * as a pattern and wrong for a name on disk. */ function anchorRegex( entry: GlobPathEntry, @@ -253,10 +253,9 @@ export type PathEntry = path: string /** * Set when this glob is anchored at a directory that is a name on - * disk: the cwd, for the mandatory `**\/` patterns, or a - * directory that exists and holds glob characters, for a caller's - * pattern beneath it. The prefix of `path` that is a literal path - * rather than part of the pattern. + * disk (the cwd, for the mandatory `**\/` patterns, or an + * existing one that holds glob characters): the prefix of + * `path` that is a literal path rather than part of the pattern. */ anchor?: string } @@ -270,16 +269,13 @@ function anchoredGlobEntry(anchor: string, pattern: string): PathEntry { } /** - * A spelling that came from the caller's config, read by its characters: - * with `*`, `?` or `[…]` in what the caller wrote it is a pattern. The - * decision is made on that raw spelling, because resolving a relative or - * `~` path can splice in a cwd or home directory whose own name contains - * those characters — read back, they would turn the caller's `secrets` into - * a pattern that never matches the directory it was resolved to. - * - * Those characters may also be part of a name that exists, and then the - * spelling is that name as well: {@link alsoReadAs} gives the readings this - * one leaves out. + * A spelling that came from the caller's config: `*`, `?` and `[…]` in + * what the caller wrote are read as glob syntax here. The decision is + * made on that raw spelling, because resolving a relative or `~` path can + * splice in a cwd or home directory whose own name contains those + * characters — read back, they would turn the caller's `secrets` into a + * pattern that never matches the directory it was resolved to. + * {@link alsoReadAs} gives the readings this one leaves out. */ function toPathEntry(pathPattern: string): PathEntry { return containsGlobChars(pathPattern) @@ -296,8 +292,7 @@ function toPathEntry(pathPattern: string): PathEntry { * `subpath` filter whatever characters it contains — sniffed as a pattern, * a component like `a[b` would become a character class and the filter * would no longer match the path it was built from (the rule would be - * inert). A path the caller marked literal is the same thing and comes - * through here too. Never use this for a spelling the caller did not mark. + * inert). Never use this for a spelling the caller did not mark literal. */ function toLiteralPathEntry(literalPath: string): PathEntry { return { @@ -307,10 +302,8 @@ function toLiteralPathEntry(literalPath: string): PathEntry { } /** - * The readings a caller's spellings have beside {@link toPathEntry}'s: the - * name a spelling with glob characters also is, and its pattern beneath a - * directory that holds such characters in its own name. Empty for every - * spelling no part of which is such a name on disk. + * The readings a caller's spellings have beside {@link toPathEntry}'s; see + * {@link literalReadings}. */ function alsoReadAs( spellings: readonly string[] | undefined, @@ -319,11 +312,7 @@ function alsoReadAs( return (spellings ?? []).flatMap(spelling => literalReadings(spelling, kind)) } -/** - * One of the caller's lists as entries: each spelling as its characters - * read, then the other readings of the spellings, then the paths that - * arrived marked literal. - */ +/** One of the caller's lists as entries, in every reading each has. */ function callerEntries( kind: PathListKind, spellings: readonly string[] | undefined, @@ -910,8 +899,7 @@ function generateReadRules( /** * What a write config allows, as entries. Decided once per profile and - * handed to the read section and the write section alike, so the two cannot - * see different answers for one entry. + * handed to the read and the write section, so the two see the same answer. */ function writeRootEntries( config: FsWriteRestrictionConfig | undefined, diff --git a/src/sandbox/path-entries.ts b/src/sandbox/path-entries.ts index c55b546a6..52c335e26 100644 --- a/src/sandbox/path-entries.ts +++ b/src/sandbox/path-entries.ts @@ -2,19 +2,14 @@ * How an entry of `denyRead`, `allowRead`, `allowWrite` or `denyWrite` is * read: as the pattern its characters spell, as the path they name, or both. * - * An entry is a pattern when it holds `*`, `?`, `[` or `]`. A directory may - * hold those characters in its own name (`[WIP] project`, `notes (draft?)`), - * and a caller that lists paths inside one means the paths. So an entry with - * those characters keeps the pattern reading it always had, and is ALSO read - * as what it names wherever the part of it that holds the characters exists - * on disk. A caller that wants the name and nothing else marks the entry - * `{ path, literal: true }`. + * An entry with `*`, `?`, `[` or `]` is a pattern. A directory may hold those + * in its own name (`[WIP] project`), so such an entry is ALSO read as what it + * names wherever the part that holds the characters exists on disk. An entry + * marked `{ path, literal: true }` is the name and nothing else. * - * The decision is made here and nowhere else, once per entry per wrap, by - * the first stage that needs it: the manager on Linux (bubblewrap takes no - * patterns, so the manager is where they are expanded), the profile builder - * on macOS, `expandWindowsFsPaths` on Windows. What comes out is carried with - * the entry from then on; no later stage looks at its characters again. + * The decision is made here only, once per entry per wrap, by the first + * stage that needs it (the manager on Linux, the profile builder on macOS, + * `expandWindowsFsPaths` on Windows); no later stage reads the characters. */ import * as fs from 'node:fs' @@ -43,31 +38,25 @@ import type { export { markedLiteralPath } /** - * Which way a list cuts. A reading added to a deny can only deny more, so a - * deny takes every reading that may hold. A reading added to an allow opens - * something, so an allow takes one only for a path that is there exactly as - * spelled. + * Which way a list cuts. A deny takes every reading that may hold, since one + * more can only deny more; an allow takes one only for a path that is there + * exactly as spelled. */ export type PathListKind = 'allow' | 'deny' -/** - * One reading of an entry. The same shape as the macOS builder's `PathEntry`, - * so a reading is one without conversion. - */ +/** One reading of an entry, shaped as the macOS builder's `PathEntry`. */ export type PathReading = /** The entry is the name of this path. */ | { glob: false; path: string } /** * The entry is a pattern beneath `anchor`, a directory taken as the name it - * is. `path` is `anchor` followed by the tail, and only the tail is - * pattern. + * is: `path` is `anchor` plus the tail, and only the tail is pattern. */ | { glob: true; path: string; anchor: string } /** * The spellings and the marked paths of a list, apart. Throws on an entry - * that is neither: `initialize()` does not run the schema, and reading - * `{ path }` either way would be a guess about a deny. + * that is neither; see {@link markedLiteralPath}. */ export function splitPathEntries( entries: readonly FilesystemPathEntry[] | undefined, @@ -99,10 +88,9 @@ export function pathEntryKey(entry: FilesystemPathEntry): string { } /** - * The spelling made absolute the way {@link normalizePathForSandbox} does it - * for a pattern, with no symlink resolved and a trailing separator kept: the - * path the caller wrote, which is what the disk is asked about. Undefined - * for a Windows UNC path, which is never probed (see {@link isUncPath}). + * The spelling made absolute as {@link normalizePathForSandbox} does for a + * pattern: no symlink resolved, a trailing separator kept. Undefined for a + * Windows UNC path, which is never probed (see {@link isUncPath}). */ function spelledAbsolute(spelling: string): string | undefined { const onWindows = getPlatform() === 'windows' @@ -123,10 +111,9 @@ function spelledAbsolute(spelling: string): string | undefined { /** * Whether something may be at `p`, for a deny. A symbolic link counts, - * dangling or not: the name is taken. So does a path that could not be - * looked at, because unreadable now is not absent: a command running as the - * same user can make a parent unsearchable and undo that from inside the - * next sandbox, and a deny it could switch off that way would be no deny. + * dangling or not, and so does a path that cannot be looked at: otherwise a + * command running as the same user could switch the deny off by making a + * parent unsearchable, and undo that inside the next sandbox. */ function mayBeThere(p: string): boolean { try { @@ -138,11 +125,9 @@ function mayBeThere(p: string): boolean { } /** - * Whether `p` is there and is not a symbolic link, for an allow. A link with - * the name an entry spells is not the path the caller meant: whoever can - * write the directory that holds it can plant one, and the allow would then - * open what the link points at. Anything that cannot be looked at is not - * there. + * Whether `p` is there and is not a symbolic link, for an allow. Whoever can + * write the directory that holds `p` can plant a link of that name, and the + * allow would open what it points at. What cannot be looked at is not there. */ function isThereUnlinked(p: string): boolean { try { @@ -165,34 +150,28 @@ function isDirectoryFor(kind: PathListKind, p: string): boolean { /** * The readings a caller's spelling has BESIDE the one its characters give - * it. Empty for a spelling that does not read as a pattern (it is a name - * already) and for a pattern none of whose glob characters are part of a - * name on disk, which is every ordinary pattern: that one costs a single - * `lstat`. + * it. Empty for a spelling that is no pattern, and for a pattern none of + * whose glob characters are part of a name on disk: that costs one `lstat`. * - * A trailing `/**` is set aside first. Of the path components that hold glob + * A trailing `/**` is set aside first. Of the 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 NAME it spells. What * follows the last of them need not exist. * - When a pattern follows an existing one, the entry is also that pattern - * BENEATH the directory, which is taken as the name it is. There is one - * such reading for each of the existing components, the longest first, so - * creating or removing a directory adds or removes a reading and never - * turns one into another. + * BENEATH the directory. There is one such reading per existing component, + * the longest first, so creating or removing a directory adds or removes a + * reading and never turns one into another. * - * Whether a spelling is a pattern at all is decided on what the caller - * wrote (`isPattern`, the platform's own check by default), as it always - * was. Which components hold glob characters is decided by - * {@link containsGlobChars} on every platform, because those are the - * characters the pattern compiler and the walk take for syntax. + * Whether a spelling is a pattern is decided on what the caller wrote + * (`isPattern`); which components hold glob characters, by + * {@link containsGlobChars} on every platform, since those are what the + * pattern compiler and the walk take for syntax. * - * For a deny, a component exists when something has its name, a symbolic - * link included, or when it cannot be looked at. For an allow it exists only - * as itself: no component from the first with glob characters on may be a - * symbolic link, and none may be beyond looking at. That goes for a name to - * its end and for a pattern up to its first pattern component, from where - * the walk of an allow goes through no link. + * A component exists for a deny as {@link mayBeThere} has it. For an allow + * no component from the first with glob characters on may be a symbolic + * link or beyond looking at: a name to its end, a pattern up to its first + * pattern component, from where the walk of an allow goes through no link. */ export function literalReadings( spelling: string, @@ -231,9 +210,8 @@ export function literalReadings( } // The tail keeps a trailing separator, which makes a pattern match nothing - // (`/x/*/`): stripped, an allow spelled so would start matching every - // child. The `/**` set aside above goes back on, for the walk's directory - // form. + // (`/x/*/`): stripped, an allow spelled so would match every child. The + // `/**` set aside above goes back on, for the walk's directory form. const setAside = stripped === spelling ? '' : '/**' for (let n = globAt.length - 2; n >= 0; n--) { const at = globAt[n]! @@ -244,9 +222,8 @@ export function literalReadings( let end = Math.min(next - 1, reach) while (end >= at && !isDirectoryFor(kind, prefix(end))) end-- if (end < at) continue - // The walk starts at the anchor followed by what the tail spells before - // its first pattern component, and resolves that: a link there would - // have an allow list what the link points at. + // The walk resolves what the tail spells before its first pattern + // component: a link there would have an allow list what it points at. if (kind === 'allow' && !restIsUnlinked(next - 1, end, prefix)) continue const anchor = normalizePathForSandbox(prefix(end), { literal: true }) readings.push({ @@ -283,10 +260,9 @@ export function hasNameReading(spelling: string, kind: PathListKind): boolean { } /** - * What a Linux read entry resolves to: what `expandGlob` finds for the - * pattern, exactly as it returns it, then what the entry's other readings - * add. `name` is the entry as a name, in the caller's spelling like every - * other name in these lists. + * What a Linux read entry resolves to: what `expandGlob` returns for the + * pattern, then what the entry's other readings add. `name` is the entry as + * a name, in the caller's spelling. */ export function withOtherReadings( spelling: string, @@ -311,9 +287,8 @@ export function withOtherReadings( } /** - * The read rules `getDefaultWritePaths()` is given for a policy: its - * entries, the credential denies, and beside each spelling the name it also - * is, handed over as a marked path. + * The read rules `getDefaultWritePaths()` is given: the entries, the + * credential denies, and the name each spelling also is, as a marked path. */ export function readRulesOf( denyRead: readonly FilesystemPathEntry[], @@ -342,7 +317,6 @@ export function readRulesOf( } } -/** `lists` without the keys whose list is empty. */ function nonEmptyLists>( lists: T, ): Partial { @@ -353,8 +327,7 @@ function nonEmptyLists>( /** * The `literal…` lists of a read config: the marked entries of the two read - * lists. A key is present only when its list is not empty, so a config - * without marked entries gives the object it always gave. + * lists. A key is present only when its list is not empty. */ export function literalReadLists( denyRead: readonly FilesystemPathEntry[] | undefined, @@ -435,11 +408,9 @@ export function readNamesOf( /** * The same for a write config; see {@link readNamesOf}. A string of * `allowOnly` that holds glob characters is kept only when it is also the - * name of a path (see {@link literalReadings}), the condition on which the - * manager keeps one. Any other is a pattern that a caller of the backend - * handed it, and no name: resolving a name folds a `..` by the text, so - * `/home/*\/../.ssh` would make `/home/.ssh` writable where nothing is named - * `*`. A path the caller marked is resolved like any name. + * name of a path (see {@link literalReadings}). Any other is a pattern and + * no name: resolving a name folds a `..` by the text, so `/home/*\/../.ssh` + * would make `/home/.ssh` writable where nothing is named `*`. */ export function writeNamesOf( config: FsWriteRestrictionConfig | undefined, diff --git a/src/sandbox/read-deny-glob.ts b/src/sandbox/read-deny-glob.ts index c65d26f67..782f12569 100644 --- a/src/sandbox/read-deny-glob.ts +++ b/src/sandbox/read-deny-glob.ts @@ -63,8 +63,7 @@ function collapseReadDenyLocations({ * `**\/build/**` yields one mount per `build/` directory. A match reached * through a symlink is listed where it really lives, and a directory the walk * could not list is denied whole. Sorted, so an ancestor precedes its - * descendants. With `anchor`, the pattern is walked beneath that directory, - * which is taken as the name it is (see `ExpandGlobOptions.anchor`). + * descendants. `anchor` is as in `ExpandGlobOptions.anchor`. * * @param unlistableDirs - receives the returned locations that hide something * the walk could not enumerate, whether by being that directory or by diff --git a/src/sandbox/sandbox-config.ts b/src/sandbox/sandbox-config.ts index 3769dba4d..1b6530d87 100644 --- a/src/sandbox/sandbox-config.ts +++ b/src/sandbox/sandbox-config.ts @@ -137,9 +137,8 @@ const filesystemPathSchema = z.string().min(1, 'Path cannot be empty') /** * An entry of `denyRead`, `allowRead`, `allowWrite` or `denyWrite`: a * spelling, which is a pattern when it reads as one, or a path marked - * literal, which is never a pattern whether or not it exists. The mark is - * `literal: true` and nothing else: an object without it, or with a key - * this does not know, is refused rather than read one way or the other. + * `literal: true`, which never is. An object without the mark, or with a + * key this does not know, is refused rather than read either way. */ const filesystemPathEntrySchema = z.union( [ diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index 51ae92ab0..b1d23c5a0 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -712,10 +712,9 @@ async function initialize( if (enableLogMonitor && getPlatform() === 'linux') { // The monitor compares paths the kernel reported, so its lists are // expanded the way the wrapper expands them (getFsWriteConfig folds in - // the default write paths and drops the patterns bwrap cannot take, so - // what is left are names; normalizePathForSandbox resolves `~`, relative - // spellings and symlinks), plus the built-in write denies the wrapper - // always applies. + // the default write paths and drops the patterns, so only names are left; + // normalizePathForSandbox resolves `~`, relative spellings and symlinks), + // plus the built-in write denies the wrapper always applies. // It does not reproduce the wrapper's existence and boundary-symlink // filters, nor the ripgrep scan for nested dangerous paths; and it still // reports writes bwrap permits through `--dev`, `--proc` and the tmpfs @@ -1272,15 +1271,12 @@ function unionDenyReadPaths( } /** - * Strip a trailing `/**` from each spelling of a read list and, on Linux, - * replace one that still has glob characters with what it resolves to - * (bubblewrap takes concrete paths only): what `expandGlob` returns for the - * pattern, then the name the spelling also is when that exists, and what - * the pattern matches beneath a directory that exists and has the - * characters in its own name (see path-entries.ts). Other platforms match - * globs natively, and their backends decide the other readings themselves. - * Entries the caller marked literal are not among the result: they travel - * in the `literal…` lists. + * Strip a trailing `/**` from each read-path entry and, on Linux, replace + * any remaining glob with what `expandGlob` returns for it (bubblewrap takes + * concrete paths only). Other platforms match globs natively. + * On Linux the entry's other readings are added (see `withOtherReadings`); + * elsewhere the backend decides them. Entries the caller marked literal are + * not in the result: they travel in the `literal…` lists. * * `literalPaths` are the ones the library resolved itself (a masked * credential file that degraded to deny) — each names one file on disk, so @@ -1305,13 +1301,11 @@ function resolveReadPathEntries( } /** - * Strip a trailing `/**` from each spelling of a write list and, on Linux, - * drop one that still has glob characters unless it is also the name of a - * path that exists: bwrap needs real paths and nothing expands a write - * pattern, so such an entry is applied as the name and its pattern reading - * is not. macOS subpath matching is recursive, so the strip is harmless - * there and the filter never fires. Entries the caller marked literal are - * not among the result: they travel in the `literal…` lists. + * Strip a trailing `/**` and drop what is still a glob on Linux: bwrap needs + * real paths. macOS subpath matching is recursive, so the strip is harmless + * there and the filter never fires. + * A glob that is also the name of a path that exists is kept, as that name. + * Marked entries are not in the result, as in {@link resolveReadPathEntries}. */ function stripWriteGlobs( kind: PathListKind, @@ -1345,9 +1339,8 @@ function expandAllowReadGlob(pattern: string, anchor?: string): string[] { * On Linux, denyRead globs are collapsed to covering directory mounts against * this config's allowRead and {@link getFsWriteConfig}'s write roots, so * `denyOnly` is only sound alongside that write config and must not be handed - * to wrapCommandWithSandboxLinux with a different one. Entries the caller - * marked literal are in `literalDenyOnly` and `literalAllowWithinDeny`, as - * spelled, and the object is to be passed on whole. + * to wrapCommandWithSandboxLinux with a different one. Marked entries are + * in the `literal…` lists, as spelled: pass the object on whole. */ function getFsReadConfig(): FsReadRestrictionConfig { if (!config || config.filesystem.disabled) { @@ -1509,7 +1502,6 @@ function rawWindowsFsInputs(c: SandboxRuntimeConfig) { } } -/** Compared by value: a marked entry is a new object after every clone. */ function setEq( a: readonly FilesystemPathEntry[], b: readonly FilesystemPathEntry[], @@ -2446,11 +2438,10 @@ function annotateStderrWithSandboxFailures( * fully supported on Linux. Returns empty array on macOS or when * sandboxing is disabled. * - * Patterns ending with /** are excluded since they work as subpaths, and so - * are entries marked literal, which are no patterns. An entry that is also - * the name of a path that exists is applied as that name and is still - * returned: its pattern reading is not applied, and what is returned must - * not depend on what is on the disk, which a sandboxed command can change. + * Patterns ending with /** are excluded since they work as subpaths. + * So are entries marked literal. An entry that is also the name of a path + * that exists is still returned: what is returned must not depend on what + * is on the disk, which a sandboxed command can change. */ function getLinuxGlobPatternWarnings(): string[] { // Only warn on Linux/WSL (bubblewrap doesn't support globs) diff --git a/src/sandbox/sandbox-schemas.ts b/src/sandbox/sandbox-schemas.ts index bd382ea70..09ac5a30b 100644 --- a/src/sandbox/sandbox-schemas.ts +++ b/src/sandbox/sandbox-schemas.ts @@ -17,12 +17,10 @@ * * This is maximally permissive by default - only explicitly denied paths are blocked. * - * A `literal…` list holds MORE entries for the list it is named after, not a - * part of it: paths the caller marked `{ path, literal: true }`. They are - * names, in which `[`, `*` and `?` are characters and never pattern syntax, - * so no backend expands or compiles them. The key is absent when there are - * none. Whatever reads `denyOnly` to learn what is denied has to read - * `literalDenyOnly` too. + * A `literal…` list holds MORE entries for the list it is named after: the + * paths the caller marked `{ path, literal: true }`, names that no backend + * expands or compiles. The key is absent when there are none. Whatever reads + * `denyOnly` to learn what is denied has to read `literalDenyOnly` too. */ export interface FsReadRestrictionConfig { denyOnly: string[] @@ -52,19 +50,13 @@ export interface FsReadRestrictionConfig { * Note: Empty `allowOnly` means NO paths are writable (unlike read's empty denyOnly). * * A `literal…` list holds MORE entries for the list it is named after, as in - * {@link FsReadRestrictionConfig}: names and never patterns, among them the - * paths the caller marked `{ path, literal: true }`. What the command may - * write is `allowOnly` and `literalAllowOnly` together. + * {@link FsReadRestrictionConfig}: what the command may write is `allowOnly` + * and `literalAllowOnly` together. */ export interface FsWriteRestrictionConfig { allowOnly: string[] literalAllowOnly?: string[] denyWithinAllow: string[] - /** - * More paths to deny within `allowOnly`, which the library read off the - * disk itself. They are names, not spellings a caller wrote: `[`, `*` and - * `?` in them are characters of the name and never pattern syntax. - */ literalDenyWithinAllow?: string[] } diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index d5306cb4f..a8009549c 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -137,17 +137,15 @@ export function containsGlobCharsForPlatform(p: string): boolean { } /** - * An entry of `denyRead`, `allowRead`, `allowWrite` or `denyWrite` as - * configured: a spelling, which is a pattern when it reads as one, or a path - * marked literal, which is never a pattern. + * An entry of a filesystem list as configured: a spelling, which is a + * pattern when it reads as one, or a path marked literal, which never is. */ export type PathListEntry = string | { path: string; literal: true } /** * The path of an entry that is not a spelling. Throws unless the entry is - * `{ path: string, literal: true }`: a config reaches the library without - * going through the schema when an embedder builds it in code, and taking - * `{ path }` for a name or for a pattern would be a guess about a deny. + * `{ path: string, literal: true }`: a config built in code skips the + * schema, and reading `{ path }` either way would be a guess about a deny. */ export function markedLiteralPath( entry: Exclude, @@ -419,9 +417,8 @@ function warnIfParentRefUnfolded(normalizedPath: string): string { * name does — or the name a spelling with glob characters also is (see * path-entries.ts). The glob branches are skipped for it, so a component * like `a[b` is resolved and later compiled as the name it is. Without it, - * a spelling with `*`, `?` or `[…]` in it keeps the character sniffing: - * that is the pattern reading of what the caller wrote, where the brackets - * are glob syntax. The interior collapse below is not one of the glob + * a spelling with `*`, `?` or `[…]` in it keeps the character sniffing, its + * pattern reading. The interior collapse below is not one of the glob * branches: `//` and `/./` are dead spellings either way. */ export function normalizePathForSandbox( @@ -605,10 +602,9 @@ const HOME_CONVENIENCE_WRITE_DIRS: readonly string[] = [ * match is a symlink to one of these directories is not seen. A glob * `allowRead` entry is not counted as re-opening anything. * - * An entry marked `{ path, literal: true }` is a name whatever characters it - * holds. A spelling is read by its characters alone here: the name that a - * spelling with glob characters also is when it exists on disk is for the - * caller to add as a marked entry, which the sandbox manager does. + * An entry marked `{ path, literal: true }` is a name whatever it holds. A + * spelling is read by its characters alone: the name it also is when that + * exists on disk is for the caller to add as a marked entry. */ export function getDefaultWritePaths(readRules?: { denyRead: readonly PathListEntry[] @@ -1167,8 +1163,7 @@ export function globToRegex(globPattern: string): string { * Takes a whole pattern, so every character in it is glob syntax: right for * the pattern reading of a spelling the caller wrote, which is what * {@link readRuleCovers} passes. A pattern anchored at a directory that is a - * name on disk (one of the library's own, or one that exists and holds glob - * characters) goes through the macOS `denyGlobEntryRegex`, which splices + * name on disk goes through the macOS `denyGlobEntryRegex`, which splices * that directory back in escaped and calls this for the tail. */ export function denyGlobRegex(normalizedGlob: string): string { @@ -1557,9 +1552,8 @@ export function walkGlobPattern( realOf: new Map(), } - // Beneath an anchor the pattern is the tail alone, rooted at the anchor: - // no character of the anchor is compiled, and the walk starts at the - // anchor plus the tail's own static directory. + // Beneath an anchor the pattern is the tail alone, so no character of the + // anchor is compiled; the walk starts at the anchor plus the tail's base. const anchor = opts.anchor === undefined ? undefined : toForwardSlashes(opts.anchor) const normalizedPattern = diff --git a/src/sandbox/windows-sandbox-utils.ts b/src/sandbox/windows-sandbox-utils.ts index 19c24250c..962914d86 100644 --- a/src/sandbox/windows-sandbox-utils.ts +++ b/src/sandbox/windows-sandbox-utils.ts @@ -1691,15 +1691,14 @@ export function uninstallWindowsSandbox( * still walks the share (user-trusted). * * `[` and `]` are characters of a name here, but the walk that expands a - * glob reads them as a character class on every platform. So a glob that - * lies beneath a directory with brackets in its name, which the walk alone - * would expand to nothing, is also expanded beneath that directory taken as - * the name it is (see path-entries.ts). A UNC glob is not: finding the - * directory would probe the share. + * glob reads them as a character class. So a glob beneath a directory with + * brackets in its name is also expanded beneath that directory taken as the + * name it is (see path-entries.ts). A UNC glob is not: finding the directory + * would probe the share. * * An entry marked `{ path, literal: true }` is a literal whatever it holds. - * One that holds `*` or `?` is skipped: no Win32 name has them, so it names - * nothing that can exist, and `srt-win` refuses the characters outright. + * One that holds `*` or `?` is skipped: no Win32 name has them, and + * `srt-win` refuses the characters outright. */ export function expandWindowsFsPaths( patterns: readonly FilesystemPathEntry[], diff --git a/test/sandbox/glob-expand.test.ts b/test/sandbox/glob-expand.test.ts index c5998451d..9867e31d5 100644 --- a/test/sandbox/glob-expand.test.ts +++ b/test/sandbox/glob-expand.test.ts @@ -842,13 +842,10 @@ describe('expandWindowsFsPaths literal branch', () => { // ============================================================================ /** - * `[` and `]` are characters of a name on Windows, and a path that holds - * them and no `*` or `?` is a literal. A pattern beneath such a directory is - * expanded by the walk every platform shares, which reads the brackets as a - * character class: on its own it finds nothing, and the deny or grant is - * lost. expandWindowsFsPaths is plain TypeScript, so what it does with such - * entries is pinned here on every host; the spellings only Windows has are - * in the suite below this one. + * `[` and `]` are characters of a name on Windows, but the walk that expands + * a pattern reads them as a character class: beneath such a directory it + * finds nothing on its own, and the deny or grant is lost. Pinned here on + * every host; the spellings only Windows has are in the suite below. */ describe('expandWindowsFsPaths beneath a directory named [WIP] project', () => { let root: string diff --git a/test/sandbox/literal-path-entries.test.ts b/test/sandbox/literal-path-entries.test.ts index f29601008..0dbfe441d 100644 --- a/test/sandbox/literal-path-entries.test.ts +++ b/test/sandbox/literal-path-entries.test.ts @@ -57,17 +57,12 @@ import { bwrapCanNamespace } from '../helpers/bwrap-namespace.js' import { isLinux, isWindows } from '../helpers/platform.js' /** - * A directory may hold `*`, `?`, `[` or `]` in its name. An entry of - * `denyRead`, `allowRead`, `allowWrite` or `denyWrite` that lies inside one - * has those characters in it, and read as a pattern alone it matches - * nothing: the deny is lost and the allow opens nothing. Such an entry is - * read as the pattern AND as the path it spells wherever the part that holds - * the characters exists on disk; an entry marked `{ path, literal: true }` - * is a path and nothing else. + * Entries inside a directory with `*`, `?`, `[` or `]` in its name, which + * match nothing when read as a pattern alone, and entries marked + * `{ path, literal: true }`: see src/sandbox/path-entries.ts. * - * The Linux suites run real commands under bubblewrap. The macOS suites - * read the generated profile, which is string building and runs on any - * POSIX host; nothing here runs a profile under sandbox-exec. + * The Linux suites run real commands under bubblewrap. The macOS suites read + * the generated profile on any POSIX host; none runs it under sandbox-exec. */ /** The folder every suite works in: a name a pattern reads as a class. */ @@ -85,15 +80,11 @@ function q(p: string): string { return `'${p.replace(/'/g, `'\\''`)}'` } -/** `text` as it appears inside a regular expression that matches it. */ function escapedForRegex(text: string): string { return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') } -/** - * The filter a spelling compiles to when its characters are read as a - * pattern, which is the reading every entry with glob characters has. - */ +/** The filter a spelling compiles to when read as a pattern. */ function patternFilter(spelling: string, kind: 'allow' | 'deny'): string { const compile = kind === 'deny' ? denyGlobRegex : globToRegex return `(regex ${JSON.stringify(compile(normalizePathForSandbox(spelling)))})` @@ -102,14 +93,12 @@ function patternFilter(spelling: string, kind: 'allow' | 'deny'): string { const subpath = (p: string): string => `(subpath ${JSON.stringify(p)})` const literalFilter = (p: string): string => `(literal ${JSON.stringify(p)})` -/** Every `(regex "…")` the profile emits. */ function emittedRegexes(profile: string): RegExp[] { return [...profile.matchAll(/\(regex ("(?:[^"\\]|\\.)*")\)/g)].map( match => new RegExp(JSON.parse(match[1]!) as string), ) } -/** Every `(subpath "…")` the profile emits. */ function emittedSubpaths(profile: string): string[] { return [...profile.matchAll(/\(subpath ("(?:[^"\\]|\\.)*")\)/g)].map( match => JSON.parse(match[1]!) as string, @@ -323,8 +312,7 @@ describe.if(!isWindows)('literalReadings', () => { it('leaves a relative name inside a bracketed cwd alone', () => { process.chdir(project) - // No glob character in what the caller wrote: it is a name already, and - // every backend takes it for one. + // No glob character in what the caller wrote: it is a name already. expect(bothKinds('./keep')).toEqual(forBoth([])) expect(bothKinds('keep')).toEqual(forBoth([])) }) @@ -388,8 +376,7 @@ describe.if(!isWindows)('literalReadings', () => { }) it('reads [ab] as the class and as the directory of that name', () => { - // The pattern reading, which covers `a` and `b`, is the one every entry - // has; the name is what this adds. + // The class is the pattern reading every entry has; this adds the name. const inClass = join(root, 'plain', '[ab]', 'secret') expect(bothKinds(inClass)).toEqual(forBoth([name(inClass)])) }) @@ -426,8 +413,7 @@ describe.if(!isWindows)('literalReadings', () => { }) it('anchors a pattern at the nearest directory when a file is in the way', () => { - // As beneath any folder: the walk then finds the file where it looked - // for a directory. + // The walk then finds the file where it looked for a directory. expect(literalReadings(`${project}/file/**/x`, 'deny')).toEqual([ beneath(project, '/file/**/x'), ]) @@ -466,8 +452,7 @@ describe.if(!isWindows)('literalReadings', () => { }) it('ends the pattern of an allow when it lies before the first pattern component', () => { - // The walk starts at the folder followed by `linked`, resolved: it - // would list what the link points at. + // The walk would start at `linked`, resolved, and list what it points at. for (const tail of ['/linked/*/x', '/linked/*', '/real/../linked/**/x']) { expect(literalReadings(project + tail, 'allow')).toEqual([]) } @@ -548,8 +533,7 @@ describe.if(!isWindows)('literalReadings', () => { 'matches %p, which cannot be split, by its spelling from the anchor on', (tail, expected) => { // A wildcard inside a bracket expression: the walk matches such a - // pattern against whole paths, and beneath an anchor those are paths - // from the anchor on. + // pattern against whole paths, which beneath an anchor start at it. expect( expandGlobPattern(ordinary + tail) .map(match => match.slice(ordinary.length)) @@ -591,8 +575,7 @@ describe('path entries as configured', () => { ['a number', 1], ['a list', ['/a']], ])('refuses %s rather than guess', (_what, entry) => { - // initialize() does not run the schema, so this is the only check an - // embedder's hand-built config meets. + // initialize() runs no schema: this is a hand-built config's only check. expect(() => splitPathEntries([entry as unknown as FilesystemPathEntry]), ).toThrow(TypeError) @@ -753,8 +736,7 @@ describe.if(isLinux)( 'opens a carve-out inside a folder named %p, whose name a pattern matches as itself', async name => { // The deny is spelled without a `/**`: with one it is the pattern as - // well in such a folder, under which every entry beneath the - // directory keeps a mask of its own. + // well, under which every entry beneath keeps a mask of its own. const folder = join(root, name) mkdirSync(join(folder, 'secrets', 'public'), { recursive: true }) writeFileSync(join(folder, 'secrets', 'key'), 'SECRET-DIR\n') @@ -806,9 +788,8 @@ describe.if(isLinux)( it.skipIf(!CAN_RUN)( 'applies a deny beneath an allow that is spelled through a parent reference', async () => { - // The allow is recorded where it resolves to. Left as spelled, the - // deny beneath it would be judged outside every allowed path and - // skipped. + // The allow is recorded where it resolves to: left as spelled, the + // deny beneath it would be judged outside every allowed path. const denied = join(project, 'real', 'file') const { stdout } = await sandboxed( { @@ -899,9 +880,8 @@ describe.if(isLinux)( ])( 'makes nothing writable for a direct caller that spells %p, which is not there', async (tail, target) => { - // Nothing beneath the project is named `*` or `[ab]`. Resolved as a - // name the string loses the component with its parent reference, and - // what is left names a directory that is there. + // Nothing is named `*` or `[ab]`. Resolved as a name, the string would + // lose that component to its parent reference and name what is there. const kept = join(project, target) const before = readFileSync(kept, 'utf8') const wrapped = await wrapCommandWithSandboxLinux({ @@ -931,8 +911,7 @@ describe.if(isLinux)( 'makes a path writable that is spelled through a folder that is not there, as beneath any folder', async () => { // The project's own name holds the glob characters and exists, so the - // string is the name of a path; a name is resolved, parent reference - // and all. + // string is a name, and a name is resolved, parent reference and all. const { stdout } = await sandboxed( { allowWrite: [`${project}/absent/../real`] }, `echo hi > ${q(join(project, 'real', 'new'))} && echo WROTE; ` + @@ -948,10 +927,9 @@ describe.if(isLinux)( ) /** - * What exists is looked at when a command is wrapped, and an earlier command - * may have put it there. A deny takes every reading that may hold, so - * nothing the command creates can switch one off. An allow takes the path - * only as itself: a link with the name an entry spells is not the path. + * What exists is looked at on every wrap, and an earlier command may have + * put it there: nothing it creates may switch a deny off, and a link with + * the name an entry spells is not the path an allow takes. */ describe.if(isLinux)( 'Linux: what a command can change before the next wrap', @@ -1042,9 +1020,8 @@ describe.if(isLinux)( ])( 'does not open a denied directory through a link that a pattern spelled %p starts beneath', async (entry, dirName, linkName) => { - // The directory is one the command made, with the name the entry - // spells, and the link inside it is what the pattern would be walked - // from. + // The command made the directory the entry spells, and the link + // inside it is what the pattern would be walked from. const policy = { denyRead: [vault], allowRead: [join(work, entry)], @@ -1156,10 +1133,9 @@ describe.if(isLinux)( it.skipIf(!CAN_RUN || process.getuid?.() === 0)( 'takes any write deny with glob characters for the path it spells when the folder cannot be looked at', async () => { - // Whether anything in the folder has the name cannot be told, so an - // ordinary pattern is taken for a path as well, and is applied like - // a path that is not there: the name is kept from being created, and - // what the pattern would match is no more denied than anywhere else. + // Whether anything in the folder has the name cannot be told, so a + // pattern is also applied like a path that is not there: the name is + // kept from being created, and what it would match is not denied. const locked = join(root, 'locked') mkdirSync(locked) writeFileSync(join(locked, 'a.pem'), 'original\n') @@ -1458,13 +1434,9 @@ describe.if(!isWindows)('an entry that is a pattern and a name', () => { // ============================================================================ /** - * Ordinary entries and ordinary patterns, under a root that does not exist. - * The expectations are what the code gave for them before an entry could - * have a second reading; they must stay what they are, byte for byte. That - * goes for every entry none of whose glob characters can be part of a name - * on disk. Beneath a directory that cannot be looked at they can, for all - * anyone can tell, and there a deny is also the path it spells: see "takes - * any write deny with glob characters for the path it spells" above. + * Ordinary entries and patterns, under a root that does not exist: no glob + * character of theirs can be part of a name on disk, so each has its + * pattern reading alone, and what it resolves to is pinned byte for byte. */ describe.if(!isWindows)( 'entries none of which is a name with glob characters', @@ -1564,10 +1536,7 @@ describe.if(!isWindows)( }, ) -/** - * The file rules of the profile inside a wrapped command, with the log tag, - * which differs from one process to the next, replaced. - */ +/** The file rules of a wrapped command's profile, log tag replaced. */ function fileRulesOf(wrapped: string): string { const from = wrapped.indexOf('; File read') const to = wrapped.indexOf("' ", from) @@ -1813,8 +1782,7 @@ describe.if(!isWindows)('an entry marked literal', () => { expect(ruleOf(profile, '(deny file-write*')).toContain( subpath(join(root, 'keep[1]')), ) - // What the characters of each path would match as a pattern is matched - // by no regex of the profile. + // No regex of the profile matches what a path would match as a pattern. const regexes = emittedRegexes(profile) for (const matched of [ join(dir, 'a.env'), @@ -1889,8 +1857,7 @@ describe.if(!isWindows)('an entry marked literal', () => { 'denies a write to a path that is not there yet, in a folder that is not there yet', async () => { // An absent deny path gets a placeholder at its first missing - // component, which keeps the command from creating it; the placeholder - // is removed from the host once the command is done. + // component, which is removed from the host once the command is done. const folder = join(root, '[new]') const secret = join(folder, 'secret') const { stdout } = await sandboxed( @@ -1912,8 +1879,7 @@ describe.if(!isWindows)('an entry marked literal', () => { 'denies a read of a path that is not there yet from the wrap after it appears', async () => { // Nothing is mounted for a read deny whose path is absent, marked or - // not: there is nothing to hide. The path is looked at again for the - // next command. + // not; the path is looked at again for the next command. const late = join(root, '[late]', 'secret') const policy = { denyRead: [marked(late)] } const read = `cat ${q(late)} 2>/dev/null; echo END` @@ -1931,11 +1897,9 @@ describe.if(!isWindows)('an entry marked literal', () => { it.skipIf(!CAN_RUN)( 'keeps the masks of a deny pattern beneath a marked allow', async () => { - // The matches of a deny pattern are collapsed: one that a directory - // match above it already hides gets no mount of its own, unless a path - // that is bound back over that directory lies between the two. A - // marked allow is bound back like any other, so the match beneath it - // has to keep its mask. + // A match of a deny pattern that a directory match above it already + // hides gets no mount of its own, unless a path bound back over that + // directory lies between the two. A marked allow is one such path. const vault = join(root, 'tree', 'vault.key') const open = join(vault, 'open[1]') mkdirSync(open, { recursive: true })