diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index 73affb426..8e880e161 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -1288,6 +1288,10 @@ function unionDenyReadPaths( * credential file that degraded to deny) — each names one file on disk, so * it is passed through whatever characters it contains. Expanded as a * pattern, a name holding `[` matches nothing and the deny is lost. + * + * Each path once, where it first came up: every `**` pattern over one base + * returns the directories it could not list, and the Linux wrapper looks at + * every entry it is given. */ function* resolveReadPathEntries( kind: PathListKind, @@ -1296,7 +1300,7 @@ function* resolveReadPathEntries( literalPaths: readonly string[] = [], ): Steps { const literal = new Set(literalPaths) - const resolved: string[] = [] + const resolved = new Set() for (const p of spelledOf(paths)) { const stripped = removeTrailingGlobSuffix(p) if ( @@ -1304,14 +1308,19 @@ function* resolveReadPathEntries( !literal.has(p) && containsGlobChars(stripped) ) { - resolved.push( - ...(yield* withOtherReadings(p, stripped, kind, expandGlob)), - ) + for (const found of yield* withOtherReadings( + p, + stripped, + kind, + expandGlob, + )) { + resolved.add(found) + } } else { - resolved.push(stripped) + resolved.add(stripped) } } - return resolved + return [...resolved] } /** diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index 409899cc9..30c5af5a2 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -1185,9 +1185,10 @@ export interface ExpandGlobOptions { anchor?: string } -/** Successful listings, by real directory: a second position, or a second - * pattern, reads the same entries. */ -export type GlobWalkListings = Map +/** What listing a real directory gave, its entries or the error that has it + * denied whole: a second position, or a second pattern, reads the same + * answer. */ +export type GlobWalkListings = Map export type GlobWalkOptions = ExpandGlobOptions & { withDirectoryForm?: boolean @@ -1742,11 +1743,15 @@ export function* walkGlobPatternSteps( if (fresh.length === 0) continue yield for (const p of fresh) listed.add(p) - let entries = listings.get(real) + let entries: fs.Dirent[] try { - entries ??= onRealPath(real, frame.short, p => - fs.readdirSync(p, { withFileTypes: true }), - ) + const known = listings.get(real) + if (known instanceof Error) throw known + entries = + known ?? + onRealPath(real, frame.short, p => + fs.readdirSync(p, { withFileTypes: true }), + ) listings.set(real, entries) } catch (err) { const errorCode = (err as NodeJS.ErrnoException | undefined)?.code @@ -1755,6 +1760,10 @@ export function* walkGlobPatternSteps( { level: errorCode === 'ENOENT' ? 'info' : 'warn' }, ) if (errorCode !== 'ENOENT') { + // Kept, or every pattern would try the directory again: one that + // would have cleared meanwhile hides more. An absence is not kept. It + // is the one answer that denies nothing, so each pattern asks. + listings.set(real, err instanceof Error ? err : new Error(String(err))) walk.unlisted.push(dir) if (real !== dir) walk.realOf.set(dir, real) } diff --git a/test/sandbox/glob-expand.test.ts b/test/sandbox/glob-expand.test.ts index 97de366d3..32948f29f 100644 --- a/test/sandbox/glob-expand.test.ts +++ b/test/sandbox/glob-expand.test.ts @@ -641,6 +641,62 @@ describe.if(!isWindows)('walkGlobPattern', () => { } }) + it.if(process.getuid?.() !== 0)( + 'tries a directory it cannot list once for all the patterns handed the same listings', + () => { + const root = realPath(mkdtempSync(join(tmpdir(), 'glob-walk-failed-'))) + const locked = join(root, 'locked') + const open = join(root, 'open') + try { + mkdirSync(open) + writeFileSync(join(open, 'id.pem'), '') + mkdirSync(locked) + chmodSync(locked, 0o000) + const patterns = ['**/.env', '**/*.pem', '**/*.key'].map(p => + join(root, p), + ) + const alone = patterns.map(p => walkGlobPattern(p).unlisted) + + const tried: string[] = [] + const readdirSync = fs.readdirSync + const readdirSpy = spyOn(fs, 'readdirSync').mockImplementation((( + ...args: Parameters + ) => { + const dir = String(args[0]) + const askedBefore = tried.includes(dir) + tried.push(dir) + // Absent when the first pattern asks, and there for the others. + if (dir === open && !askedBefore) { + throw Object.assign(new Error('gone'), { code: 'ENOENT' }) + } + return readdirSync(...args) + }) as typeof fs.readdirSync) + let together + try { + const listings = new Map() + together = patterns.map(p => walkGlobPattern(p, { listings })) + } finally { + readdirSpy.mockRestore() + } + + expect(tried.filter(dir => dir === locked)).toEqual([locked]) + // Every pattern still has it to deny whole. + expect(together.map(walk => walk.unlisted)).toEqual(alone) + expect(alone).toEqual(patterns.map(() => [locked])) + // An absence denies nothing, so it is nobody's answer but the asker's. + expect(tried.filter(dir => dir === open)).toEqual([open, open]) + expect(together.map(walk => walk.matches)).toEqual([ + [], + [join(open, 'id.pem')], + [], + ]) + } finally { + chmodSync(locked, 0o755) + 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 @@ -1722,6 +1778,41 @@ describe.if(isLinux)('getFsReadConfig with glob patterns on Linux', () => { } }) + it.if(process.getuid?.() !== 0)( + 'gives each path once, however many patterns came to it', + async () => { + const { SandboxManager } = await import( + '../../src/sandbox/sandbox-manager.js' + ) + const root = realPath(mkdtempSync(join(tmpdir(), 'glob-walk-once-'))) + const locked = join(root, 'locked') + mkdirSync(locked) + chmodSync(locked, 0o000) + writeFileSync(join(root, 'id.pem'), '') + + await SandboxManager.reset() + await SandboxManager.initialize({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { + // All three could not list `locked`, and two match the file. + denyRead: ['**/*.pem', '**/id.*', '**/.env'].map(p => join(root, p)), + allowWrite: [], + denyWrite: [], + }, + }) + try { + expect(SandboxManager.getFsReadConfig().denyOnly).toEqual([ + join(root, 'id.pem'), + locked, + ]) + } finally { + await SandboxManager.reset() + chmodSync(locked, 0o755) + rmSync(root, { recursive: true, force: true }) + } + }, + ) + for (const list of ['denyRead', 'allowRead'] as const) { it(`gives up a wrap whose signal is aborted while a ${list} pattern is walked`, async () => { const { SandboxManager } = await import(