fix(review): reconcile sandbox and approval contracts
This commit is contained in:
@@ -35,7 +35,8 @@
|
||||
* `dsh-tool-bash` through `ctx.approval` — this executor's contribution is the
|
||||
* per-call `sandboxMode` override it honors in {@link resolve}: an escalated
|
||||
* call runs (and classifies, and reports) under ITS granted mode while every
|
||||
* neighboring call keeps the configured default.
|
||||
* neighboring call keeps its session's standing mode (or the configured
|
||||
* default when that session has no override).
|
||||
*
|
||||
* @module @deepseek-ai/dsh-bash-sandbox
|
||||
*/
|
||||
@@ -138,17 +139,13 @@ function matchesSignature(exitCode: number | null, stderr: string, signatures: r
|
||||
/**
|
||||
* Sandbox-consuming bash executor. Registers as `ctx.bash` (loading it
|
||||
* INSTEAD OF `dsh-bash-local`, together with a `ctx.sandbox` provider, is
|
||||
* the whole swap — the tool layer is untouched). The DEFAULT mode is fixed at
|
||||
* config time for the executor's lifetime; a single call escalates past it
|
||||
* only through the request-level `sandboxMode` override its {@link resolve}
|
||||
* stamps onto the spec (granted upstream via `ctx.approval` — the
|
||||
* sandbox RFC § Escalation). The model learns of the sandbox only through
|
||||
* result facts: the static bash tool description explains the denial marker,
|
||||
* and every run's `result.sandbox` carries the mode it executed under and how
|
||||
* completely the runner enforced it. Runtime default-mode switching and a
|
||||
* current-mode prompt statement are deliberately absent until a config
|
||||
* surface exists to drive them (TODO(sandbox-config): the sandbox RFC's
|
||||
* future-work list brings both with the per-session config options).
|
||||
* the whole swap — the tool layer is untouched). Its configured mode is the
|
||||
* fallback exposed by {@link sandboxMode}; `dsh-tool-bash` folds a session's
|
||||
* durable `bash/sandbox-mode` override and stamps the effective mode onto each
|
||||
* request, while an approved escalation may stamp a strictly wider mode for
|
||||
* one call. The tool's per-agent prompt section states that same effective
|
||||
* mode, and each run's `result.sandbox` reports what actually executed plus
|
||||
* enforcement completeness.
|
||||
*/
|
||||
export class SandboxBashExecutor extends LocalBashExecutor {
|
||||
static inject = ['sandbox']
|
||||
|
||||
@@ -76,11 +76,12 @@ export abstract class BashExecutor extends Service {
|
||||
/**
|
||||
* The sandbox mode this executor confines commands under BY DEFAULT, or
|
||||
* `undefined` when it does not sandbox at all — the capability fact the
|
||||
* tool layer reads to advertise escalation honestly (a mode-widening lever
|
||||
* is only offered when a sandboxing executor is mounted to honor it, and
|
||||
* only for modes strictly wider than this one). Composition truth, not
|
||||
* configuration: the base class reports `undefined`; a sandboxing
|
||||
* implementation overrides the getter with its configured mode.
|
||||
* tool and ACP layers read to advertise sandbox controls honestly. The
|
||||
* getter proves a sandboxing executor is mounted and supplies its fallback
|
||||
* mode; a session override may make the effective mode narrower or wider,
|
||||
* so strict escalation widening is checked per call rather than encoded in
|
||||
* this default-relative capability fact. The base class reports
|
||||
* `undefined`; a sandboxing implementation overrides the getter.
|
||||
* @returns the configured default mode of a sandboxing executor;
|
||||
* `undefined` for an executor that never confines.
|
||||
*/
|
||||
|
||||
@@ -52,7 +52,7 @@ The `BashExecRequest` seam carries optional `stdin` and `env`, used by the hooks
|
||||
|
||||
Commands run with the executor's full authority unless a sandboxing executor ([`dsh-bash-sandbox`](../bash-sandbox/)) confines them — the deny-only sandbox reports denials as result facts, rendered here as the denial marker; per-call allow/deny/ask policy is the `tools/pre-execute` waterfall (see docs/architecture.md).
|
||||
|
||||
On top of a denial sits the escalation gate ([the sandbox RFC § Escalation](../../../docs/rfc/implemented/feature/2026-07-06-sandbox.md)): an escalating call (`sandbox_permissions` + `justification`) resolves [`ctx.approval`](../../approval/approval/README.md) BEFORE anything executes — `allowed-once` stamps the granted mode onto the bash request as the seam-level `sandboxMode` override (that one call runs, classifies, and reports under the wider mode; its neighbors keep the session's effective mode), while `rejected`/`cancelled`/`unavailable` and the no-service / no-agent paths each fail closed with their own error text and execute nothing. The seam is consumed opportunistically (`ctx.get('approval')`, the dsh-tools ask-routing pattern); the grant is consumed by the very call that asked, and nothing is stored. The static description teaches — and a denied result itself prompts, via the escalation-available marker appended exactly when the fields are advertised — the SAME-TURN flow: on a denial a wider mode would cure, retry the exact command once with `sandbox_permissions` (the narrowest mode that suffices) + `justification` immediately, without detouring through chat (the approval prompt IS the user's consent); never speculatively — an escalation is grounded in a real denial (up-front only when the session already denied the same access), a prompt-stated approvals-disabled policy turns the exception off entirely, and a rejected escalation is final for that command.
|
||||
On top of a denial sits the escalation gate ([the sandbox RFC § Escalation](../../../docs/rfc/implemented/feature/2026-07-06-sandbox.md)): an escalating call (`sandbox_permissions` + `justification`) resolves [`ctx.approval`](../../ui/user-approval/README.md) BEFORE anything executes — `allowed-once` stamps the granted mode onto the bash request as the seam-level `sandboxMode` override (that one call runs, classifies, and reports under the wider mode; its neighbors keep the session's effective mode), while `rejected`/`cancelled`/`unavailable` and the no-service / no-agent paths each fail closed with their own error text and execute nothing. The seam is consumed opportunistically (`ctx.get('approval')`, the dsh-tools ask-routing pattern); the grant is consumed by the very call that asked, and nothing is stored. The static description teaches — and a denied result itself prompts, via the escalation-available marker appended exactly when the fields are advertised — the SAME-TURN flow: on a denial a wider mode would cure, retry the exact command once with `sandbox_permissions` (the narrowest mode that suffices) + `justification` immediately, without detouring through chat (the approval prompt IS the user's consent); never speculatively — an escalation is grounded in a real denial (up-front only when the session already denied the same access), a prompt-stated approvals-disabled policy turns the exception off entirely, and a rejected escalation is final for that command.
|
||||
|
||||
## Per-session mode switching
|
||||
|
||||
|
||||
@@ -23,7 +23,7 @@
|
||||
"license": "BSD-3-Clause",
|
||||
"peerDependencies": {
|
||||
"@deepseek-ai/dsh-agent": "^0.0.1",
|
||||
"@deepseek-ai/dsh-approval": "^0.0.1",
|
||||
"@deepseek-ai/dsh-user-approval": "^0.0.1",
|
||||
"@deepseek-ai/dsh-bash": "^0.0.1",
|
||||
"@deepseek-ai/dsh-llm": "^0.0.1",
|
||||
"@deepseek-ai/dsh-sandbox": "^0.0.1",
|
||||
@@ -34,7 +34,7 @@
|
||||
"devDependencies": {
|
||||
"@deepseek-ai/dsh-agent": "workspace:^",
|
||||
"@deepseek-ai/dsh-agent-loop": "workspace:^",
|
||||
"@deepseek-ai/dsh-approval": "workspace:^",
|
||||
"@deepseek-ai/dsh-user-approval": "workspace:^",
|
||||
"@deepseek-ai/dsh-bash": "workspace:^",
|
||||
"@deepseek-ai/dsh-bash-local": "workspace:^",
|
||||
"@deepseek-ai/dsh-bash-sandbox": "workspace:^",
|
||||
|
||||
@@ -65,7 +65,7 @@ import type {} from '@deepseek-ai/dsh-system-prompt'
|
||||
// Side-effect type import: declaration-merges `ctx.approval`, consumed
|
||||
// opportunistically by the escalation gate (`ctx.get('approval')` — the seam
|
||||
// stays optional at runtime, same pattern as dsh-tools' ask routing).
|
||||
import type {} from '@deepseek-ai/dsh-approval'
|
||||
import type {} from '@deepseek-ai/dsh-user-approval'
|
||||
import type { SandboxMode } from '@deepseek-ai/dsh-sandbox'
|
||||
import { BashTaskId, OwnerToken, effectiveSandboxMode } from '@deepseek-ai/dsh-bash'
|
||||
import type { BashRunResult, BashTask, CollectedOutput } from '@deepseek-ai/dsh-bash'
|
||||
@@ -471,12 +471,12 @@ export function apply(ctx: Context): void {
|
||||
}
|
||||
})
|
||||
|
||||
// The escalation surface exists exactly when the mounted executor confines
|
||||
// under a default that has a strictly wider mode to escalate to — a lever
|
||||
// is never advertised that the composition cannot honor. Registration time
|
||||
// is the right read: the executor's default is config-fixed for its
|
||||
// lifetime, and an executor swap restarts this fiber (static inject) and
|
||||
// re-registers the schema.
|
||||
// The escalation surface exists whenever the mounted executor confines.
|
||||
// Its enum is the closed target vocabulary, deliberately NOT cut down by
|
||||
// the configured default: a session may switch to a narrower effective mode
|
||||
// while sharing this globally registered schema. Strict widening therefore
|
||||
// belongs to the per-call check below. An executor swap restarts this fiber
|
||||
// (static inject) and re-registers the schema.
|
||||
const defaultMode = ctx.bash.sandboxMode
|
||||
const escalationModes: readonly SandboxMode[] = defaultMode === undefined ? [] : ESCALATION_TARGETS
|
||||
|
||||
@@ -505,8 +505,7 @@ export function apply(ctx: Context): void {
|
||||
*/
|
||||
const approveEscalation = async (mode: string, justification: string, exec: ToolExecution): Promise<SandboxMode> => {
|
||||
// Schema validation only checks ADVERTISED keys, so an unadvertised
|
||||
// `sandbox_permissions` (no sandboxing executor, or a `danger-full-access`
|
||||
// default with nothing wider) still reaches execute — reject it here so a
|
||||
// `sandbox_permissions` (no sandboxing executor) still reaches execute — reject it here so a
|
||||
// human is never prompted to "escalate" a sandbox that is not there. When
|
||||
// the fields ARE advertised, the registry's SchemaSpec enum has already
|
||||
// pinned `mode` to this ladder for every caller.
|
||||
@@ -540,8 +539,8 @@ export function apply(ctx: Context): void {
|
||||
...exec.signal ? { signal: exec.signal } : {},
|
||||
})
|
||||
switch (outcome) {
|
||||
// The SchemaSpec enum already pinned `mode` to this executor's wider
|
||||
// ladder; the cast records that validated fact.
|
||||
// The SchemaSpec enum already pinned `mode` to the closed target
|
||||
// vocabulary; the per-call check above proved it is strictly wider.
|
||||
case 'allowed-once': return mode as SandboxMode
|
||||
case 'rejected': throw new Error(`the user rejected escalating this command to "${mode}"`)
|
||||
case 'cancelled': throw new Error(`approval for escalating to "${mode}" was cancelled`)
|
||||
|
||||
@@ -16,8 +16,8 @@ import { SandboxBashExecutor } from '@deepseek-ai/dsh-bash-sandbox'
|
||||
import { SandboxProvider } from '@deepseek-ai/dsh-sandbox'
|
||||
import type { ConfinedArgv } from '@deepseek-ai/dsh-sandbox'
|
||||
import { LocalSandboxProvider } from '@deepseek-ai/dsh-sandbox-local'
|
||||
import ApprovalService from '@deepseek-ai/dsh-approval'
|
||||
import type { ApprovalOutcome } from '@deepseek-ai/dsh-approval'
|
||||
import ApprovalService from '@deepseek-ai/dsh-user-approval'
|
||||
import type { ApprovalOutcome } from '@deepseek-ai/dsh-user-approval'
|
||||
import * as ToolBash from '@deepseek-ai/dsh-tool-bash'
|
||||
import { renderResult } from '@deepseek-ai/dsh-tool-bash'
|
||||
|
||||
@@ -27,6 +27,13 @@ const spillDir = mkdtempSync(join(tmpdir(), 'dsh-tool-bash-spec-'))
|
||||
// profile args up to `--` and execs the command unconfined — deterministic
|
||||
// without a host bwrap.
|
||||
const PASSTHROUGH_RUNNER = ['bash', '-c', 'while [ "$1" != "--" ]; do shift; done; shift; exec "$@"', 'passthrough-runner']
|
||||
const PASSTHROUGH_RUNNER_CONFIG = {
|
||||
runnerCommand: PASSTHROUGH_RUNNER,
|
||||
// The script has no pre-exec failure path; the provider still requires an
|
||||
// explicit dialect so a future script change cannot silently turn runner
|
||||
// failure into an ordinary command result.
|
||||
runnerFailureSignatures: ['passthrough-runner: profile rejected'],
|
||||
}
|
||||
|
||||
async function setup() {
|
||||
const ctx = new Context()
|
||||
@@ -1022,7 +1029,7 @@ describe('sandbox rendering', () => {
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(AgentRegistry)
|
||||
await ctx.plugin(LocalSandboxProvider, { runnerCommand: PASSTHROUGH_RUNNER })
|
||||
await ctx.plugin(LocalSandboxProvider, PASSTHROUGH_RUNNER_CONFIG)
|
||||
await ctx.plugin(SandboxBashExecutor, { graceMs: 200 })
|
||||
const bash = ctx.bash as SandboxBashExecutor
|
||||
bash.internals = { spillDir }
|
||||
@@ -1113,12 +1120,31 @@ describe('sandbox rendering', () => {
|
||||
expect(text(read)).not.toContain('file access denied')
|
||||
})
|
||||
|
||||
it('classifies an executable configured runner that refuses its profile before the command runs', async () => {
|
||||
const signature = 'custom-runner-rejected'
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(LocalSandboxProvider, {
|
||||
runnerCommand: ['bash', '-c', `printf '${signature}\\n' >&2; exit 125`, 'custom-runner'],
|
||||
runnerFailureSignatures: [signature],
|
||||
})
|
||||
await ctx.plugin(SandboxBashExecutor, { graceMs: 200 })
|
||||
const bash = ctx.bash as SandboxBashExecutor
|
||||
bash.internals = { spillDir }
|
||||
|
||||
await expect(bash.run(bash.resolve({ command: 'echo command-must-not-run' })))
|
||||
.rejects.toMatchObject({ code: 'SANDBOX_UNAVAILABLE' })
|
||||
|
||||
const task = bash.start(bash.resolve({ command: 'echo command-must-not-run' }))
|
||||
await task.done
|
||||
expect(task.sandbox).toEqual({ mode: 'read-only', denied: false, enforcement: 'full', runnerFailed: true })
|
||||
})
|
||||
|
||||
it('reports a real denial end-to-end through the shipping sandbox executor', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(AgentRegistry)
|
||||
await ctx.plugin(LocalSandboxProvider, { runnerCommand: PASSTHROUGH_RUNNER })
|
||||
await ctx.plugin(LocalSandboxProvider, PASSTHROUGH_RUNNER_CONFIG)
|
||||
await ctx.plugin(SandboxBashExecutor, { graceMs: 200 })
|
||||
const bash = ctx.bash as SandboxBashExecutor
|
||||
bash.internals = { spillDir }
|
||||
@@ -1141,7 +1167,7 @@ describe('sandbox escalation (sandbox_permissions / justification)', () => {
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(AgentRegistry)
|
||||
await ctx.plugin(LocalSandboxProvider, { runnerCommand: PASSTHROUGH_RUNNER })
|
||||
await ctx.plugin(LocalSandboxProvider, PASSTHROUGH_RUNNER_CONFIG)
|
||||
await ctx.plugin(SandboxBashExecutor, { graceMs: 200, ...mode !== undefined ? { mode } : {} })
|
||||
const bash = ctx.bash as SandboxBashExecutor
|
||||
bash.internals = { spillDir }
|
||||
@@ -1363,7 +1389,7 @@ describe('per-session sandbox mode (the bash/sandbox-mode fold)', () => {
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(AgentRegistry)
|
||||
await ctx.plugin(LocalSandboxProvider, { runnerCommand: PASSTHROUGH_RUNNER })
|
||||
await ctx.plugin(LocalSandboxProvider, PASSTHROUGH_RUNNER_CONFIG)
|
||||
await ctx.plugin(SandboxBashExecutor, { graceMs: 200, mode })
|
||||
;(ctx.bash as SandboxBashExecutor).internals = { spillDir }
|
||||
if (opts.approval === true) await ctx.plugin(ApprovalService)
|
||||
|
||||
@@ -30,7 +30,7 @@
|
||||
"path": "../../core/system-prompt"
|
||||
},
|
||||
{
|
||||
"path": "../../approval/approval"
|
||||
"path": "../../ui/user-approval"
|
||||
},
|
||||
{
|
||||
"path": "../../sandbox/sandbox"
|
||||
|
||||
Reference in New Issue
Block a user