Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,9 @@ 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.
- 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, 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, 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.
- For embedders: the four lists of `SandboxRuntimeConfig` are of type `FilesystemPathEntry[]`, which is `(string | { path: string; literal: true })[]`, and `getConfig()` gives entries back in the form they were given. Code that reads an entry as a string stops compiling; `typeof entry === 'string' ? entry : entry.path` is its path. Three things change without a compile error. A list joined into text prints `[object Object]` for a marked entry. `getFsReadConfig()` and `getFsWriteConfig()` carry marked entries in `literalDenyOnly`, `literalAllowWithinDeny`, `literalAllowOnly` and `literalDenyWithinAllow`, each of which may be absent when it has no entries, so hand those objects on whole instead of rebuilding them from `denyOnly` and `allowOnly`; `writeRootsOf(getFsWriteConfig())` is every path the command may write. And an empty `denyOnly` no longer means that nothing is read-denied. `getDefaultWritePaths()` and `expandWindowsFsPaths()` take the lists as configured. `wrapCommandWithSandboxLinux()` takes every path it is handed for a name, as bubblewrap does, and skips a string in `allowOnly` that has `*`, `?`, `[` or `]` in it unless something is there under that very spelling.

#### Other Configuration

Expand Down
21 changes: 21 additions & 0 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,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,
]
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export type {
SandboxRuntimeConfig,
NetworkConfig,
FilesystemConfig,
FilesystemPathEntry,
CredentialsConfig,
CredentialFileConfig,
CredentialEnvVarConfig,
Expand Down Expand Up @@ -124,6 +125,7 @@ export {

// Utility functions
export { getDefaultWritePaths } from './sandbox/sandbox-utils.js'
export { writeRootsOf } from './sandbox/path-entries.js'

// Platform utilities
export { getWslVersion } from './utils/platform.js'
Expand Down
28 changes: 16 additions & 12 deletions src/sandbox/linux-sandbox-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { endianness, tmpdir } from 'node:os'
import path, { join } from 'node:path'
import { ripGrep, RipgrepError, type RipgrepConfig } from '../utils/ripgrep.js'
import { buildJavaToolOptions } from './java-proxy-agent.js'
import { readNamesOf, writeNamesOf } from './path-entries.js'
import {
generateProxyEnvVars,
buildPosixGitSafeDirEnv,
Expand Down Expand Up @@ -1945,7 +1946,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
Expand Down Expand Up @@ -2041,7 +2042,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
}
Expand Down Expand Up @@ -2095,7 +2096,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)
Expand Down Expand Up @@ -2123,10 +2124,9 @@ 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
// '<dir>/[id]/'. Allow paths are recorded slash-free because every
// Every path here is a name, and normalizePathForSandbox strips a
// 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
Expand All @@ -2136,7 +2136,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(/\/+$/, '') || '/'

Expand Down Expand Up @@ -2328,7 +2328,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
}
Expand Down Expand Up @@ -2477,7 +2477,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/')) {
Expand Down Expand Up @@ -3163,8 +3163,6 @@ export async function wrapCommandWithSandboxLinux(
proxyAuthToken,
caCertPath,
javaAgentJarPath,
readConfig,
writeConfig,
unsetEnvVars,
setEnvVars,
maskedFileBinds,
Expand All @@ -3182,6 +3180,12 @@ export async function wrapCommandWithSandboxLinux(
observeSocketPath,
abortSignal,
} = params
// 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)

// Determine if we have restrictions to apply
// Read: denyOnly pattern - empty array means no restrictions
Expand Down
56 changes: 45 additions & 11 deletions src/sandbox/macos-sandbox-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,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). Never use this for a caller's spelling.
* inert). Never use this for a spelling the caller did not mark literal.
*/
function toLiteralPathEntry(literalPath: string): PathEntry {
return {
Expand All @@ -298,6 +298,17 @@ function toLiteralPathEntry(literalPath: string): PathEntry {
}
}

/** One of the caller's lists as entries: spellings, then marked paths. */
function callerEntries(
spellings: readonly string[] | undefined,
literalPaths: readonly string[] | undefined,
): PathEntry[] {
return [
...(spellings ?? []).map(toPathEntry),
...(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))
Expand All @@ -322,7 +333,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
Expand All @@ -331,12 +342,18 @@ function resolveReadConfig(
libraryDenies: readonly PathEntry[],
): ResolvedReadConfig {
return {
denies: [...(config.denyOnly || []).map(toPathEntry), ...libraryDenies],
denies: [
...callerEntries(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 ('<dir>//').
allows: (config.allowWithinDeny || []).map(toPathEntry),
writeRoots: (writeAllowPaths || []).map(toPathEntry),
allows: callerEntries(
config.allowWithinDeny,
config.literalAllowWithinDeny,
),
writeRoots: [...writeRoots],
}
}

Expand Down Expand Up @@ -863,13 +880,24 @@ function generateReadRules(
return rules
}

/**
* What a write config allows, as entries: read once per profile, for the
* read and the write section alike, so neither can miss the marked paths.
*/
function writeRootEntries(
config: FsWriteRestrictionConfig | undefined,
): PathEntry[] {
return callerEntries(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*)`]
Expand All @@ -879,8 +907,8 @@ function generateWriteRules(

// Generate allow rules
const allowFilters = new Set<string>()
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))

Expand All @@ -889,6 +917,7 @@ function generateWriteRules(
// the mandatory entries carry their own literal/glob split.
const denyEntries = [
...(config.denyWithinAllow || []).map(toPathEntry),
...(config.literalDenyWithinAllow ?? []).map(toLiteralPathEntry),
...macGetMandatoryDenyEntries(allowGitConfig),
]

Expand Down Expand Up @@ -1231,14 +1260,15 @@ 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.
const resolvedRead =
readConfig || libraryDenyEntries.length > 0
? resolveReadConfig(
readConfig ?? { denyOnly: [] },
writeConfig?.allowOnly,
writeRoots,
libraryDenyEntries,
)
: undefined
Expand All @@ -1248,7 +1278,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)
Expand Down Expand Up @@ -1347,7 +1379,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 =
Expand Down
Loading
Loading