mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 08:37:49 +00:00
Bound cwd permission probes during terminal session creation (#33643)
* Bound cwd permission probes and share pending enumerations * Skip non-macOS cwd permission probes during session creation - Prevent stalled network paths from consuming filesystem workers
This commit is contained in:
@@ -163,7 +163,12 @@ describe('resetFolderAccessForDaemon runs the remedy', () => {
|
||||
cwdClass: 'documents',
|
||||
freshDaemonAccess: 'denied'
|
||||
}
|
||||
opendirMock.mockReturnValue(new Promise<never>(() => {}))
|
||||
let answer: () => void = () => {}
|
||||
opendirMock.mockReturnValue(
|
||||
new Promise((resolve) => {
|
||||
answer = () => resolve(fakeDir())
|
||||
})
|
||||
)
|
||||
getMismatchMock.mockReturnValue(denied)
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
@@ -182,6 +187,8 @@ describe('resetFolderAccessForDaemon runs the remedy', () => {
|
||||
cwd_class: 'documents'
|
||||
})
|
||||
} finally {
|
||||
answer()
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
@@ -4,6 +4,8 @@ import { opendir } from 'node:fs/promises'
|
||||
/** `denied` is the only outcome that proves a permission refusal; `other` keeps unknown errors apart. */
|
||||
export type DirectoryEnumerationOutcome = 'ok' | 'denied' | 'missing' | 'other'
|
||||
|
||||
const pendingEnumerations = new Map<string, Promise<DirectoryEnumerationOutcome>>()
|
||||
|
||||
function errorCode(error: unknown): string | undefined {
|
||||
if (typeof error !== 'object' || error === null || !('code' in error)) {
|
||||
return undefined
|
||||
@@ -21,7 +23,18 @@ function outcomeForError(error: unknown): DirectoryEnumerationOutcome {
|
||||
}
|
||||
|
||||
/** Probe enumeration because TCC can reject opendir even when access() succeeds. */
|
||||
export async function enumerateDirectoryOnce(path: string): Promise<DirectoryEnumerationOutcome> {
|
||||
export function enumerateDirectoryOnce(path: string): Promise<DirectoryEnumerationOutcome> {
|
||||
const pending = pendingEnumerations.get(path)
|
||||
if (pending) {
|
||||
return pending
|
||||
}
|
||||
// Keep an uninterruptible probe shared until it settles, even if its callers stop waiting.
|
||||
const enumeration = enumerateDirectory(path).finally(() => pendingEnumerations.delete(path))
|
||||
pendingEnumerations.set(path, enumeration)
|
||||
return enumeration
|
||||
}
|
||||
|
||||
async function enumerateDirectory(path: string): Promise<DirectoryEnumerationOutcome> {
|
||||
let dir: Dir | undefined
|
||||
try {
|
||||
dir = await opendir(path)
|
||||
|
||||
@@ -1,17 +1,23 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { SubprocessHandle } from './session-subprocess-handle'
|
||||
import { TerminalHost, type TerminalHostOptions } from './terminal-host'
|
||||
import { TerminalHost } from './terminal-host'
|
||||
import { validateWorkingDirectoryAsync } from '../providers/working-directory-validation'
|
||||
|
||||
const { opendirMock } = vi.hoisted(() => ({ opendirMock: vi.fn() }))
|
||||
const { opendirMock, statMock } = vi.hoisted(() => ({
|
||||
opendirMock: vi.fn(),
|
||||
statMock: vi.fn()
|
||||
}))
|
||||
// TCC directory enumeration can remain pending while a permission sheet is open.
|
||||
vi.mock('node:fs/promises', async (importOriginal) => ({
|
||||
...(await importOriginal<Record<string, unknown>>()),
|
||||
opendir: opendirMock
|
||||
opendir: opendirMock,
|
||||
stat: statMock
|
||||
}))
|
||||
|
||||
vi.mock('../pty-descendant-termination', () => ({ killWithDescendantSweep: vi.fn() }))
|
||||
|
||||
const close = vi.fn(async () => {})
|
||||
const spawnSubprocess = vi.fn(() => createMockSubprocess())
|
||||
|
||||
function dirReading(read: () => unknown): { read: () => unknown; close: () => Promise<void> } {
|
||||
return { read, close }
|
||||
@@ -21,6 +27,33 @@ function failWith(code: string): never {
|
||||
throw Object.assign(new Error(code), { code })
|
||||
}
|
||||
|
||||
function stallEnumeration(stage: 'open' | 'read' | 'close') {
|
||||
let release!: () => void
|
||||
const pending = new Promise<void>((resolve) => {
|
||||
release = resolve
|
||||
})
|
||||
const directory = {
|
||||
read: async () => {
|
||||
if (stage === 'read') {
|
||||
await pending
|
||||
}
|
||||
return null
|
||||
},
|
||||
close: vi.fn(async () => {
|
||||
if (stage === 'close') {
|
||||
await pending
|
||||
}
|
||||
})
|
||||
}
|
||||
opendirMock.mockImplementation(async () => {
|
||||
if (stage === 'open') {
|
||||
await pending
|
||||
}
|
||||
return directory
|
||||
})
|
||||
return { release, directory }
|
||||
}
|
||||
|
||||
function createMockSubprocess(): SubprocessHandle {
|
||||
let onExitCb: ((code: number) => void) | null = null
|
||||
return {
|
||||
@@ -49,26 +82,29 @@ describe('TerminalHost cwd readability verdict', () => {
|
||||
|
||||
beforeEach(() => {
|
||||
close.mockReset()
|
||||
spawnSubprocess.mockClear()
|
||||
statMock.mockReset()
|
||||
opendirMock.mockReset().mockReturnValue(dirReading(() => ({ name: 'entry' })))
|
||||
platformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' })
|
||||
const spawnSubprocess: TerminalHostOptions['spawnSubprocess'] = () => createMockSubprocess()
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'darwin' })
|
||||
host = new TerminalHost({ spawnSubprocess })
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
vi.useRealTimers()
|
||||
await host.dispose()
|
||||
if (platformDescriptor) {
|
||||
Object.defineProperty(process, 'platform', platformDescriptor)
|
||||
}
|
||||
})
|
||||
|
||||
const create = (sessionId: string, cwd?: string) =>
|
||||
const create = (sessionId: string, cwd?: string, signal?: AbortSignal) =>
|
||||
host.createOrAttach({
|
||||
sessionId,
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
...(cwd ? { cwd } : {}),
|
||||
...(signal ? { cancelSignal: signal, isCanceled: () => signal.aborted } : {}),
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
@@ -116,10 +152,167 @@ describe('TerminalHost cwd readability verdict', () => {
|
||||
expect(opendirMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each(['linux', 'win32'])('skips TCC enumeration on %s', async (platform) => {
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: platform })
|
||||
expect((await create('non-macos', '/work/repo')).cwdReadableByDaemon).toBeUndefined()
|
||||
expect(opendirMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps native UNC creation within the validation lane while healthy creation proceeds', async () => {
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
vi.useFakeTimers()
|
||||
let release = () => {}
|
||||
const unavailable = new Promise<void>((resolve) => {
|
||||
release = resolve
|
||||
})
|
||||
opendirMock.mockImplementation(async () => {
|
||||
await unavailable
|
||||
return dirReading(() => null)
|
||||
})
|
||||
const server = '\\\\unreachable-smb\\share'
|
||||
let active = 0
|
||||
let peak = 0
|
||||
statMock.mockImplementation(async (cwd: string) => {
|
||||
if (cwd.startsWith(server)) {
|
||||
active += 1
|
||||
peak = Math.max(peak, active)
|
||||
try {
|
||||
await unavailable
|
||||
} finally {
|
||||
active -= 1
|
||||
}
|
||||
}
|
||||
return { isDirectory: () => true }
|
||||
})
|
||||
await host.dispose()
|
||||
host = new TerminalHost({
|
||||
spawnSubprocess: async (opts) => {
|
||||
if (opts.cwd) {
|
||||
await validateWorkingDirectoryAsync(opts.cwd, { signal: opts.cancelSignal })
|
||||
}
|
||||
return createMockSubprocess()
|
||||
}
|
||||
})
|
||||
const abort = new AbortController()
|
||||
const firstPath = `${server}\\first`
|
||||
const canceled = create('unc-canceled', firstPath, abort.signal)
|
||||
void canceled.catch(() => {})
|
||||
const pending = [
|
||||
create('unc-second', `${server}\\second`),
|
||||
create('unc-queued', `${server}\\third`)
|
||||
]
|
||||
try {
|
||||
await vi.advanceTimersByTimeAsync(1_100)
|
||||
expect(active + opendirMock.mock.calls.length).toBe(2)
|
||||
abort.abort()
|
||||
await expect(canceled).rejects.toThrow('was canceled')
|
||||
pending.push(create('unc-canceled', firstPath))
|
||||
await vi.advanceTimersByTimeAsync(1_100)
|
||||
expect(statMock).toHaveBeenCalledTimes(2)
|
||||
expect((await create('healthy', 'C:\\healthy')).isNew).toBe(true)
|
||||
expect(active).toBe(2)
|
||||
} finally {
|
||||
release()
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
await Promise.allSettled([canceled, ...pending])
|
||||
}
|
||||
expect(peak).toBe(2)
|
||||
expect(host.listSessions()).toHaveLength(4)
|
||||
expect(statMock).toHaveBeenCalledTimes(4)
|
||||
})
|
||||
|
||||
it('omits the verdict on attach to an existing session', async () => {
|
||||
await create('attach', '/work/repo')
|
||||
const attached = await create('attach', '/work/repo')
|
||||
expect(attached.isNew).toBe(false)
|
||||
expect(attached.cwdReadableByDaemon).toBeUndefined()
|
||||
})
|
||||
|
||||
it.each(['open', 'read', 'close'] as const)(
|
||||
'starts a shell when the diagnostic directory %s remains pending',
|
||||
async (stage) => {
|
||||
vi.useFakeTimers()
|
||||
const { release, directory } = stallEnumeration(stage)
|
||||
const creation = create(`slow-${stage}`, `/work/slow-${stage}`)
|
||||
try {
|
||||
await vi.advanceTimersByTimeAsync(1_000)
|
||||
expect(host.listSessions()).toHaveLength(1)
|
||||
await expect(creation).resolves.toMatchObject({ isNew: true })
|
||||
expect((await creation).cwdReadableByDaemon).toBeUndefined()
|
||||
} finally {
|
||||
release()
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
}
|
||||
expect(directory.close).toHaveBeenCalledOnce()
|
||||
}
|
||||
)
|
||||
|
||||
it('reuses a pending directory probe across timeouts and probes again after it settles', async () => {
|
||||
vi.useFakeTimers()
|
||||
const { release } = stallEnumeration('open')
|
||||
try {
|
||||
const first = create('first-prompt', '/work/prompt')
|
||||
await vi.advanceTimersByTimeAsync(1_000)
|
||||
expect(host.listSessions()).toHaveLength(1)
|
||||
await first
|
||||
const second = create('second-prompt', '/work/prompt')
|
||||
await vi.advanceTimersByTimeAsync(1_000)
|
||||
expect(host.listSessions()).toHaveLength(2)
|
||||
await second
|
||||
expect(opendirMock).toHaveBeenCalledOnce()
|
||||
} finally {
|
||||
release()
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
}
|
||||
expect((await create('after-prompt', '/work/prompt')).cwdReadableByDaemon).toBe(true)
|
||||
expect(opendirMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('cancels a pending probe without spawning and releases the session id for a retry', async () => {
|
||||
vi.useFakeTimers()
|
||||
const { release, directory } = stallEnumeration('open')
|
||||
const abort = new AbortController()
|
||||
const creation = create('canceled-prompt', '/work/canceled-prompt', abort.signal)
|
||||
let error: unknown
|
||||
void creation.catch((rejection: unknown) => {
|
||||
error = rejection
|
||||
})
|
||||
try {
|
||||
abort.abort()
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
expect(spawnSubprocess).not.toHaveBeenCalled()
|
||||
expect(host.listSessions()).toEqual([])
|
||||
expect(error).toMatchObject({ message: 'Attach canceled for session canceled-prompt' })
|
||||
const retry = create('canceled-prompt', '/work/canceled-prompt')
|
||||
await vi.advanceTimersByTimeAsync(1_000)
|
||||
expect(host.listSessions()).toHaveLength(1)
|
||||
await expect(retry).resolves.toMatchObject({ isNew: true })
|
||||
expect(opendirMock).toHaveBeenCalledOnce()
|
||||
} finally {
|
||||
release()
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
}
|
||||
expect(directory.close).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('does not launch a shell when shutdown starts during a pending probe', async () => {
|
||||
vi.useFakeTimers()
|
||||
const { release } = stallEnumeration('open')
|
||||
const creation = create('shutdown-prompt', '/work/shutdown-prompt')
|
||||
let error: unknown
|
||||
void creation.catch((rejection: unknown) => {
|
||||
error = rejection
|
||||
})
|
||||
const disposal = host.dispose()
|
||||
try {
|
||||
await vi.advanceTimersByTimeAsync(1_000)
|
||||
expect(spawnSubprocess).not.toHaveBeenCalled()
|
||||
expect(error).toMatchObject({ message: 'Terminal host is shutting down' })
|
||||
await disposal
|
||||
expect(host.listSessions()).toEqual([])
|
||||
} finally {
|
||||
release()
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -21,6 +21,7 @@ import type { LegacyRestoreDetails } from './legacy-restore-records'
|
||||
import { resolveDaemonSessionScrollbackRows } from './daemon-session-scrollback-window'
|
||||
import { TerminalAttachCanceledError } from './daemon-errors'
|
||||
import { waitForTerminalAttachOperation } from './terminal-attach-cancellation'
|
||||
import { sharedPromiseSettlementWaiters } from '../../shared/promise-settlement-waiters'
|
||||
import { SessionNotFoundError } from './types'
|
||||
import { resolveWslSessionContext } from './wsl-session-context'
|
||||
import { recordSessionSpawnPaneKey } from './terminal-host-session-listing'
|
||||
@@ -145,9 +146,12 @@ async function spawnAndPublishSession(
|
||||
onPublished: (sessionId: string, isAlive: boolean) => void
|
||||
): Promise<CreateOrAttachResult> {
|
||||
const { size, wslDistro } = ctx
|
||||
// Probe the requested cwd before the shell can fall back to another path.
|
||||
// Only macOS needs the TCC probe; other hosts must not pin extra filesystem workers.
|
||||
const cwdReadableByDaemon =
|
||||
opts.cwd && !wslDistro ? await isCwdReadableByThisProcess(opts.cwd) : null
|
||||
process.platform === 'darwin' && opts.cwd && !wslDistro
|
||||
? await isCwdReadableByThisProcess(opts.cwd, opts)
|
||||
: null
|
||||
deps.assertCreateAllowed()
|
||||
const subprocess = await deps.spawnSubprocess({
|
||||
sessionId: opts.sessionId,
|
||||
cols: size.cols,
|
||||
@@ -310,6 +314,21 @@ function describeAgentRestore(
|
||||
}
|
||||
|
||||
// TCC may reject directory enumeration despite successful access(); only explicit permission denial counts.
|
||||
async function isCwdReadableByThisProcess(cwd: string): Promise<boolean> {
|
||||
return (await enumerateDirectoryOnce(cwd)) !== 'denied'
|
||||
async function isCwdReadableByThisProcess(
|
||||
cwd: string,
|
||||
opts: Pick<InternalCreateOrAttachOptions, 'sessionId' | 'cancelSignal'>
|
||||
): Promise<boolean | null> {
|
||||
const outcome = await sharedPromiseSettlementWaiters(enumerateDirectoryOnce(cwd))
|
||||
.wait({
|
||||
timeoutMs: 1_000,
|
||||
signal: opts.cancelSignal,
|
||||
createAbortError: () => new TerminalAttachCanceledError(opts.sessionId)
|
||||
})
|
||||
.catch((error: unknown) => {
|
||||
if (error instanceof TerminalAttachCanceledError) {
|
||||
throw error
|
||||
}
|
||||
return null
|
||||
})
|
||||
return outcome === null ? null : outcome !== 'denied'
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user