fix(sandbox): preserve valueless spawn failures (round 2)
This commit is contained in:
@@ -247,12 +247,12 @@ export class LocalBashExecutor extends BashExecutor {
|
|||||||
}
|
}
|
||||||
proc.exitCode = outcome.exitCode
|
proc.exitCode = outcome.exitCode
|
||||||
proc.signal = outcome.signal
|
proc.signal = outcome.signal
|
||||||
this.onProcessDone(proc, collected.stderr.readFrom(0).text)
|
this.onProcessDone(proc, collected.stderr.readFrom(0).text, false)
|
||||||
}, (error: unknown) => {
|
}, (error: unknown) => {
|
||||||
// Background spawn failures settle as killed and surface through the read path.
|
// Background spawn failures settle as killed and surface through the read path.
|
||||||
proc.status = 'killed'
|
proc.status = 'killed'
|
||||||
spawnFailureNote = `spawn failed: ${String(error)}`
|
spawnFailureNote = `spawn failed: ${String(error)}`
|
||||||
this.onProcessDone(proc, spawnFailureNote, error)
|
this.onProcessDone(proc, spawnFailureNote, true, error)
|
||||||
}),
|
}),
|
||||||
readOutput: (): BashProcessRead => {
|
readOutput: (): BashProcessRead => {
|
||||||
const out = collected.stdout.readFrom(stdoutOffset)
|
const out = collected.stdout.readFrom(stdoutOffset)
|
||||||
@@ -292,9 +292,10 @@ export class LocalBashExecutor extends BashExecutor {
|
|||||||
* empty.
|
* empty.
|
||||||
* @param _proc - the settled process handle.
|
* @param _proc - the settled process handle.
|
||||||
* @param _stderr - the process's retained stderr tail used by subclasses for settlement classification.
|
* @param _stderr - the process's retained stderr tail used by subclasses for settlement classification.
|
||||||
* @param _spawnError - the original spawn rejection; absent after a process successfully started.
|
* @param _spawnFailed - whether the subprocess promise rejected before a process started.
|
||||||
|
* @param _spawnError - the original spawn rejection reason, which may itself be undefined.
|
||||||
*/
|
*/
|
||||||
protected onProcessDone(_proc: BashProcess, _stderr: string, _spawnError?: unknown): void {}
|
protected onProcessDone(_proc: BashProcess, _stderr: string, _spawnFailed: boolean, _spawnError?: unknown): void {}
|
||||||
}
|
}
|
||||||
|
|
||||||
export default LocalBashExecutor
|
export default LocalBashExecutor
|
||||||
|
|||||||
@@ -124,13 +124,13 @@ export class SandboxBashExecutor extends LocalBashExecutor {
|
|||||||
* Stamp per-process sandbox facts before `done` settles. Full-access processes
|
* Stamp per-process sandbox facts before `done` settles. Full-access processes
|
||||||
* have no facts; signal deaths are not denials.
|
* have no facts; signal deaths are not denials.
|
||||||
*/
|
*/
|
||||||
protected override onProcessDone(proc: BashProcess, stderr: string, spawnError?: unknown): void {
|
protected override onProcessDone(proc: BashProcess, stderr: string, spawnFailed: boolean, spawnError?: unknown): void {
|
||||||
const facts = this.processFacts.get(proc)
|
const facts = this.processFacts.get(proc)
|
||||||
if (facts !== undefined) {
|
if (facts !== undefined) {
|
||||||
this.processFacts.delete(proc)
|
this.processFacts.delete(proc)
|
||||||
// A rejected spawn never started the confined launch. Otherwise runner
|
// A rejected spawn never started the confined launch. Otherwise runner
|
||||||
// failure outranks denial because its diagnostics may contain denial terms.
|
// failure outranks denial because its diagnostics may contain denial terms.
|
||||||
const runnerFailed = spawnError !== undefined
|
const runnerFailed = spawnFailed
|
||||||
|| classifyRunnerFailure(proc.exitCode, stderr, facts.runnerFailureRules) !== undefined
|
|| classifyRunnerFailure(proc.exitCode, stderr, facts.runnerFailureRules) !== undefined
|
||||||
proc.sandbox = {
|
proc.sandbox = {
|
||||||
mode: facts.mode,
|
mode: facts.mode,
|
||||||
@@ -139,7 +139,7 @@ export class SandboxBashExecutor extends LocalBashExecutor {
|
|||||||
...(runnerFailed ? { runnerFailed } : {}),
|
...(runnerFailed ? { runnerFailed } : {}),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
super.onProcessDone(proc, stderr, spawnError)
|
super.onProcessDone(proc, stderr, spawnFailed, spawnError)
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -16,6 +16,7 @@ import type { ConfinedArgv, SandboxExecutionPolicy, SandboxMode, SandboxPolicy }
|
|||||||
import { SandboxPolicyService } from '@deepseek-ai/dsh-sandbox-policy'
|
import { SandboxPolicyService } from '@deepseek-ai/dsh-sandbox-policy'
|
||||||
import { SandboxBashExecutor } from '@deepseek-ai/dsh-bash-sandbox'
|
import { SandboxBashExecutor } from '@deepseek-ai/dsh-bash-sandbox'
|
||||||
import LocalSubprocessService from '@deepseek-ai/dsh-subprocess-local'
|
import LocalSubprocessService from '@deepseek-ai/dsh-subprocess-local'
|
||||||
|
import type { SubprocessHandle, SubprocessOutputReader } from '@deepseek-ai/dsh-subprocess'
|
||||||
import { classifyDenial, classifyRunnerFailure } from '../src/helpers.ts'
|
import { classifyDenial, classifyRunnerFailure } from '../src/helpers.ts'
|
||||||
import type { Config } from '@deepseek-ai/dsh-bash-sandbox'
|
import type { Config } from '@deepseek-ai/dsh-bash-sandbox'
|
||||||
|
|
||||||
@@ -372,6 +373,36 @@ describe('background sandbox facts', () => {
|
|||||||
expect(accounting.size).toBe(0)
|
expect(accounting.size).toBe(0)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('classifies a spawn rejection whose reason is undefined', async () => {
|
||||||
|
const { ctx, bash } = await setup()
|
||||||
|
const emptyReader: SubprocessOutputReader = {
|
||||||
|
readFrom: () => ({ text: '', nextOffset: 0, lossy: false }),
|
||||||
|
}
|
||||||
|
vi.spyOn(ctx.subprocess, 'spawn').mockReturnValue({
|
||||||
|
pid: -1,
|
||||||
|
stdin: undefined,
|
||||||
|
stdout: undefined,
|
||||||
|
stderr: undefined,
|
||||||
|
collected: { stdout: emptyReader, stderr: emptyReader },
|
||||||
|
// Arbitrary subprocess providers can reject without a value; that edge is the point of this test.
|
||||||
|
// oxlint-disable-next-line typescript/prefer-promise-reject-errors
|
||||||
|
done: Promise.reject(undefined),
|
||||||
|
terminate: vi.fn(),
|
||||||
|
waitForExit: async () => true,
|
||||||
|
} satisfies SubprocessHandle)
|
||||||
|
|
||||||
|
const task = bash.start(bash.resolve({ command: 'true' }))
|
||||||
|
await task.done
|
||||||
|
|
||||||
|
expect(task.readOutput().delta).toContain('spawn failed: undefined')
|
||||||
|
expect(task.sandbox).toEqual({
|
||||||
|
mode: 'read-only',
|
||||||
|
denied: false,
|
||||||
|
enforcement: 'full',
|
||||||
|
runnerFailed: true,
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
it('stamps a settled denial: nonzero exit + permission stderr under a confined mode', async () => {
|
it('stamps a settled denial: nonzero exit + permission stderr under a confined mode', async () => {
|
||||||
const { bash } = await setup()
|
const { bash } = await setup()
|
||||||
const task = bash.start(bash.resolve({ command: 'echo "x: Permission denied" >&2; exit 1' }))
|
const task = bash.start(bash.resolve({ command: 'echo "x: Permission denied" >&2; exit 1' }))
|
||||||
|
|||||||
Reference in New Issue
Block a user