diff --git a/src/sandbox/read-deny-glob.ts b/src/sandbox/read-deny-glob.ts index 342805143..0a1d7915c 100644 --- a/src/sandbox/read-deny-glob.ts +++ b/src/sandbox/read-deny-glob.ts @@ -1,5 +1,6 @@ import { logForDebugging } from '../utils/debug.js' import { + type GlobWalkListings, isAtOrUnder, normalizePathForSandbox, pathSpellings, @@ -70,15 +71,18 @@ function collapseReadDenyLocations({ * covering it. The Linux wrapper binds nothing back beneath one: what the * pattern matches under an allowed path in there was never found, and would * come back unmasked. + * @param listings - one map for all the patterns of a configuration. */ export function expandReadDenyGlobLinux( globPattern: string, reExposedPaths: readonly string[], unlistableDirs?: Set, + listings?: GlobWalkListings, ): string[] { const walk = walkGlobPattern(globPattern, { withDirectoryForm: true, followSymlinkedDirectories: true, + listings, }) // Where a path the walk reported really lives: the denyRead loop mounts an // entry there, whatever spelling named it. diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index 91daa448a..b8e6c8611 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -85,6 +85,7 @@ import { normalizePathForSandbox, removeTrailingGlobSuffix, expandGlobPattern, + type GlobWalkListings, attributionKeyFor, decodeSandboxedCommand, encodeSandboxedCommand, @@ -1331,10 +1332,16 @@ function getFsReadConfig(): FsReadRestrictionConfig { ) const reExposedPaths = [...allowPaths, ...getFsWriteConfig().allowOnly] const unlistableDenyDirs = new Set() + const listings: GlobWalkListings = new Map() const denyPaths = resolveReadPathEntries( unionDenyReadPaths(config.filesystem.denyRead, credentialRestrictions), pattern => - expandReadDenyGlobLinux(pattern, reExposedPaths, unlistableDenyDirs), + expandReadDenyGlobLinux( + pattern, + reExposedPaths, + unlistableDenyDirs, + listings, + ), credentialRestrictions.degradeToDenyPaths, ) @@ -1717,13 +1724,19 @@ async function wrapWithSandbox( } const reExposedPaths = [...expandedAllowRead, ...writeConfig.allowOnly] const unlistableDenyDirs = new Set() + const listings: GlobWalkListings = new Map() const expandedDenyRead = resolveReadPathEntries( unionDenyReadPaths( customConfig?.filesystem?.denyRead ?? config?.filesystem.denyRead ?? [], credentialRestrictions, ), pattern => - expandReadDenyGlobLinux(pattern, reExposedPaths, unlistableDenyDirs), + expandReadDenyGlobLinux( + pattern, + reExposedPaths, + unlistableDenyDirs, + listings, + ), credentialRestrictions.degradeToDenyPaths, ) readConfig = { diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index 23859d190..7fa101423 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -1137,6 +1137,10 @@ export interface ExpandGlobOptions { caseInsensitive?: boolean } +/** Successful listings, by real directory: a second position, or a second + * pattern, reads the same entries. */ +export type GlobWalkListings = Map + /** What one recursive walk of a glob's base directory found; see {@link walkGlobPattern}. */ export interface GlobWalk { /** Where the walk started, with symlinks resolved (the spelling itself @@ -1488,6 +1492,9 @@ export function walkGlobPattern( opts: ExpandGlobOptions & { withDirectoryForm?: boolean followSymlinkedDirectories?: boolean + /** Handed to every walk of one configuration, so that patterns with a + * base in common list each directory once between them. */ + listings?: GlobWalkListings } = {}, ): GlobWalk { const walk: GlobWalk = { @@ -1550,9 +1557,7 @@ export function walkGlobPattern( } /** The positions each real directory has been listed for. */ const listedFor = new Map>() - /** Successful listings, by real directory: a second position reads the - * same entries. */ - const listings = new Map() + const listings: GlobWalkListings = opts.listings ?? new Map() const pending: Frame[] = [] /** A filesystem call on a real path, and on a shorter name for it when that * fails. The real path crosses no link, so a long chain of them cannot @@ -1636,6 +1641,17 @@ export function walkGlobPattern( continue } for (const entry of entries) { + // Nearly every entry of a large tree: a plain file the pattern does not + // match, which nothing below records. A pattern that splits is matched + // by name, so no path need be spelled to find that out. + if ( + positions.splits && + !entry.isDirectory() && + !entry.isSymbolicLink() && + !positions.matches(fresh, entry.name, '') + ) { + continue + } const fullPath = path.join(dir, entry.name) const realPath = path.join(real, entry.name) const candidate = toForwardSlashes(fullPath) diff --git a/test/sandbox/glob-expand.test.ts b/test/sandbox/glob-expand.test.ts index ec0625c9f..9f0d90ef8 100644 --- a/test/sandbox/glob-expand.test.ts +++ b/test/sandbox/glob-expand.test.ts @@ -595,6 +595,46 @@ describe.if(!isWindows)('walkGlobPattern', () => { } }) + it('lists a directory once for all the patterns handed the same listings', () => { + const root = realPath(mkdtempSync(join(tmpdir(), 'glob-walk-shared-'))) + try { + mkdirSync(join(root, 'a', 'b'), { recursive: true }) + writeFileSync(join(root, 'a', '.env'), '') + writeFileSync(join(root, 'a', 'b', 'id.pem'), '') + writeFileSync(join(root, 'a', 'b', 'notes.txt'), '') + const patterns = ['**/.env', '**/*.pem', '**/*.key'].map(p => + join(root, p), + ) + const alone = patterns.map(p => walkGlobPattern(p).matches) + + const listed: string[] = [] + const readdirSync = fs.readdirSync + const readdirSpy = spyOn(fs, 'readdirSync').mockImplementation((( + ...args: Parameters + ) => { + listed.push(String(args[0])) + return readdirSync(...args) + }) as typeof fs.readdirSync) + let together + try { + const listings = new Map() + together = patterns.map(p => walkGlobPattern(p, { listings }).matches) + } finally { + readdirSpy.mockRestore() + } + + expect(listed.sort()).toEqual( + [root, join(root, 'a'), join(root, 'a', 'b')].sort(), + ) + expect(together).toEqual(alone) + expect(together.flat().sort()).toEqual( + [join(root, 'a', '.env'), join(root, 'a', 'b', 'id.pem')].sort(), + ) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + it('lists a directory again under a name the pattern tells apart', () => { // vault is reached by its own name, which matches nothing, and through // config/secrets, the only spelling `**/secrets/*.pem` matches. However @@ -1185,6 +1225,53 @@ describe.if(isLinux)('getFsReadConfig with glob patterns on Linux', () => { await SandboxManager.reset() }) + it('lists a directory once for all the denyRead patterns of a configuration', async () => { + const { SandboxManager } = await import( + '../../src/sandbox/sandbox-manager.js' + ) + + await SandboxManager.reset() + await SandboxManager.initialize({ + network: { + allowedDomains: [], + deniedDomains: [], + }, + filesystem: { + denyRead: ['*.env', '*.txt', '*.pem'].map(p => join(RAW_TEST_DIR, p)), + allowWrite: ['/tmp'], + denyWrite: [], + }, + }) + + const realTestDir = realPath(RAW_TEST_DIR) + const timesListedBy = async (call: () => unknown): Promise => { + let times = 0 + const readdirSync = fs.readdirSync + const readdirSpy = spyOn(fs, 'readdirSync').mockImplementation((( + ...args: Parameters + ) => { + if (String(args[0]) === realTestDir) times++ + return readdirSync(...args) + }) as typeof fs.readdirSync) + try { + await call() + } finally { + readdirSpy.mockRestore() + } + return times + } + try { + expect(await timesListedBy(() => SandboxManager.getFsReadConfig())).toBe( + 1, + ) + expect( + await timesListedBy(() => SandboxManager.wrapWithSandbox('true')), + ).toBe(1) + } finally { + await SandboxManager.reset() + } + }) + it('should pass non-glob paths through unchanged on Linux', async () => { const { SandboxManager } = await import( '../../src/sandbox/sandbox-manager.js'