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
21 changes: 15 additions & 6 deletions src/sandbox/sandbox-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -1296,22 +1300,27 @@ function* resolveReadPathEntries(
literalPaths: readonly string[] = [],
): Steps<string[]> {
const literal = new Set(literalPaths)
const resolved: string[] = []
const resolved = new Set<string>()
for (const p of spelledOf(paths)) {
const stripped = removeTrailingGlobSuffix(p)
if (
getPlatform() === 'linux' &&
!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]
}

/**
Expand Down
23 changes: 16 additions & 7 deletions src/sandbox/sandbox-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, fs.Dirent[]>
/** 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<string, fs.Dirent[] | Error>

export type GlobWalkOptions = ExpandGlobOptions & {
withDirectoryForm?: boolean
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
Expand Down
91 changes: 91 additions & 0 deletions test/sandbox/glob-expand.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof fs.readdirSync>
) => {
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
Expand Down Expand Up @@ -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(
Expand Down
Loading