Replies: 5 comments
|
Verified against rc.2 (HEAD b150a55). This is the second independent report of this bug — #3519 (hytime, 2026-08-20) described the identical failure on the identical path, and the community fix discussion there is directly applicable. The one-line fix works for the approveEscalation internal path, but there is an ordering point that makes it incomplete: the same-mode request is rejected BEFORE approveEscalation is even reached.
packages/sandbox/sandbox/src/escalation.ts:157-164: approveEscalation checks
The three consumers (tool-bash, tool-pwsh, tool-fs) validate the escalation args BEFORE resolving the standing policy. In tool-bash:67 and tool-fs sandbox.ts:88, validateEscalationArgs rejects an empty justification first; only after validation passes does the consumer resolve policy.mode and call approveEscalation. So in the approval:never (danger-full-access preset) configuration your report describes, the call dies at the empty-justification validation, before the same-mode comparison can apply. Fixing approveEscalation alone leaves that path broken. The correct shape is the one the #3519 thread converged on: resolve the standing policy first, compare the requested sandbox_permissions against the effective mode, and when they are equal, skip validation and approval entirely and run under the standing policy. That is a shared normalizeEscalationMode helper (requested, effective) returning undefined for equal-or-absent, called before the existing validateEscalationArgs in all three consumers. It is deliberately placed before validation because a no-op grant should not require a justification at all.
The danger-full-access preset ships approval: never, and under approval: never every genuine escalation fails (no one can be asked). So the same-mode request is the ONLY sandbox_permissions request that can ever succeed in that configuration. Rejecting it makes the field universally poisonous there — the model's schema-prefilled habit (which you note is encouraged-against but tolerated) turns every call into a hard error. The fix restores the field's actual designed contract: the schema text (packages/sandbox/sandbox/src/index.ts:263) frames sandbox_permissions as "only valid as a one-shot retry of a command the sandbox just denied" — the same-mode no-op is the safe interpretation of that retry.
The same-mode early return must compare against the EFFECTIVE mode (the standing policy), not merely the requested mode being present. The consumers already compute effectiveMode from the standing policy before escalating, so the normalization sits between policy resolution and validation. Genuine widening, downgrading, and absent-policy cases must still fall through to the existing fail-closed path unchanged — your table's last three rows pin exactly that.
This is now a two-report family (#3519 + this) with an agreed fix shape and a PR-ready blueprint in the discussion thread. Worth consolidating both threads: the maintainers get one patch (shared helper + consumer reordering + escalation.spec.ts same-mode matrix) closing both reports. Your verification table is a strong addition to that matrix. Thanks for the clean report — the table especially. It closes the loop on the family. |
|
Thanks for the precise review — the ordering point is exactly right. We have now implemented the full agreed shape locally against the installed 0.1.1-rc.2 build:
Verified locally with 11 checks: same-mode Happy to share the exact diff, or to test a maintainer patch — consolidating with #3519 (shared helper + consumer reorder + a same-mode matrix in |
|
The 11-check matrix is exactly the acceptance matrix the maintainer patch needs — same-mode with/without justification succeeding, genuine widening still failing closed, narrowing still "not strictly wider", absent/no-backend unchanged. Two follow-ups from re-reading rc.2, so the consolidation does not quietly re-introduce the model-side habit: 1. Consumer coverage is confirmed complete — no fourth enforcement point exists. 2. The model-facing contract texts are now stale and should ride along in the same patch.
After the fix, the same-mode no-op succeeds with no justification and never consults the approval channel — so "requires justification and user approval" is false for exactly the path the fix enables. More importantly, that wording is the text that trains the model to keep prefilling the pairing — the habit at the root of both #4359 and #3519. The fix tolerates the habit; only a schema-text sync stops it. Suggested shape (keeps the one-shot-retry contract): "Only valid as a one-shot retry of a command the sandbox just denied; escalating to a strictly wider mode requires justification and user approval; a retry at the call's current effective mode is accepted as a no-op." The (One text that does not need changing: the Happy to review the diff against rc.2 when you share it, and to draft the |
|
Here is the full diff of the local implementation against the published What changed, per file:
Verified with the 11-check matrix: same-mode diff --git a/node_modules/@deepseek-ai/dsh-sandbox/lib/index.js b/node_modules/@deepseek-ai/dsh-sandbox/lib/index.js
index 71d1cda..2f24f56 100644
--- a/node_modules/@deepseek-ai/dsh-sandbox/lib/index.js
+++ b/node_modules/@deepseek-ai/dsh-sandbox/lib/index.js
@@ -43,7 +43,9 @@ const ESCALATION_TARGETS = ["workspace-write", "danger-full-access"];
* Validate the escalation argument pairing a tool schema cannot express:
* `sandbox_permissions` and `justification` travel together — an approval
* prompt without a reason, or a reason driving nothing, is a malformed ask —
-* and the justification must be a non-empty sentence.
+* and the justification must be a non-empty sentence. Exception: an
+* equal-mode (no-op) ask is normalized away by {@link normalizeEscalationMode}
+* before this validation runs, so it never requires a justification.
* @param sandboxPermissions - the raw `sandbox_permissions` argument, if given.
* @param justification - the raw `justification` argument, if given.
*/
@@ -53,6 +55,23 @@ function validateEscalationArgs(sandboxPermissions, justification) {
if (justification !== void 0 && justification.trim().length === 0) throw new Error("invalid justification: expected a non-empty sentence");
}
/**
+* Resolve a call's escalation ask against its effective mode: `undefined`
+* when the request is absent OR asks for the mode the call already runs under
+* (a no-op grant — nothing widens, so no justification and no approval are
+* ever required), otherwise the requested mode unchanged so the existing
+* validation and fail-closed approval sequence applies. Consumers MUST call
+* this BEFORE `validateEscalationArgs`: an equal-mode ask must not be
+* rejected for lacking a justification, and the approval channel must not be
+* consulted for a grant that changes nothing.
+* @param requestedMode - the raw `sandbox_permissions` argument, if given.
+* @param effectiveMode - the standing policy's mode, if any.
+* @returns `undefined` for absent/equal requests, else the requested mode.
+*/
+function normalizeEscalationMode(requestedMode, effectiveMode) {
+ if (requestedMode === void 0 || requestedMode === effectiveMode) return void 0;
+ return requestedMode;
+}
+/**
* The model-facing denial marker — the one vocabulary both enforcing families
* teach and report, so the model recognizes a policy denial identically
* whether the kernel refused a bash file effect or the filesystem provider's
@@ -84,13 +103,19 @@ function escalationHintMarker(subject) {
* non-widening request, a missing approval service, an agent-less execution,
* a rejection, a cancellation, an unanswerable ask) — the tool registry turns
* the throw into the call's isError result, and nothing has run. A
-* non-widening request never prompts a human.
+* non-widening request never prompts a human; a request for the mode the call
+* already runs under is granted immediately as a no-op — nothing widens, so
+* nothing needs approval.
* @param request - the escalation to judge (see {@link EscalationRequest}).
* @param approval - the approval ingredients the tool holds (see {@link EscalationApproval}).
* @returns the granted mode, consumed by the one call that asked.
*/
async function approveEscalation(request, approval) {
const { requestedMode: mode, effectiveMode, justification, subject } = request;
+ // A request for the mode the call already runs under is a no-op grant: the
+ // call is already at that level, so nothing widens and nothing needs
+ // approval — a same-mode ask must not be rejected as a failed escalation.
+ if (mode === effectiveMode) return mode;
if (!(WIDER_MODES[effectiveMode] ?? []).includes(mode)) throw new Error(`sandbox escalation to "${mode}" is not strictly wider than this call's current "${effectiveMode}" mode`);
if (approval.approver === void 0) throw new Error(`sandbox escalation to "${mode}" requires approval, but no approval service is composed`);
if (approval.agent === void 0) throw new Error(`sandbox escalation to "${mode}" requires approval, but the call has no agent to route it through`);
@@ -198,4 +223,4 @@ var SandboxProvider = class extends Service {
}
};
//#endregion
-export { ESCALATION_TARGETS, SANDBOX_UNAVAILABLE, SandboxProvider, SandboxProvider as default, SandboxUnavailableError, WIDER_MODES, approveEscalation, canonicalPath, escalationHintMarker, sandboxDenialMarker, validateEscalationArgs, writableRoots };
+export { ESCALATION_TARGETS, SANDBOX_UNAVAILABLE, SandboxProvider, SandboxProvider as default, SandboxUnavailableError, WIDER_MODES, approveEscalation, canonicalPath, escalationHintMarker, normalizeEscalationMode, sandboxDenialMarker, validateEscalationArgs, writableRoots };
diff --git a/node_modules/@deepseek-ai/dsh-tool-bash/lib/index.js b/node_modules/@deepseek-ai/dsh-tool-bash/lib/index.js
index e597cad..23bed60 100644
--- a/node_modules/@deepseek-ai/dsh-tool-bash/lib/index.js
+++ b/node_modules/@deepseek-ai/dsh-tool-bash/lib/index.js
@@ -2,7 +2,7 @@ import z from "@deepseek-ai/schemastery";
import { isAbsolute, resolve } from "node:path";
import { TOOL_ABORTED, defineTool } from "@deepseek-ai/dsh-tools";
import { HarnessError } from "@deepseek-ai/dsh-llm";
-import { ESCALATION_TARGETS, approveEscalation, canonicalPath, escalationHintMarker, sandboxDenialMarker, validateEscalationArgs } from "@deepseek-ai/dsh-sandbox";
+import { ESCALATION_TARGETS, approveEscalation, canonicalPath, escalationHintMarker, normalizeEscalationMode, sandboxDenialMarker, validateEscalationArgs } from "@deepseek-ai/dsh-sandbox";
import { DSH_ENV_PREFIX, parseExitStatus } from "@deepseek-ai/dsh-shell";
//#region lib/types/background.js
/**
@@ -120,7 +120,6 @@ function validateBashArgs(args) {
if (args.command.trim().length === 0) throw new Error("invalid command: expected a non-empty string");
if (args.description.trim().length === 0) throw new Error("invalid description: expected a non-empty string");
if (args.timeoutMs !== void 0 && (!Number.isFinite(args.timeoutMs) || args.timeoutMs <= 0)) throw new Error(`invalid timeoutMs: expected a positive number, got ${JSON.stringify(args.timeoutMs)}`);
- validateEscalationArgs(args.sandbox_permissions, args.justification);
}
function bashDescription(backgroundEnabled, escalationModes) {
const background = backgroundEnabled ? "Set `run_in_background: true` for long-running commands: the call returns a job id immediately; read its output with `job_output` and stop it with `job_kill`." : "Background execution is not available; long-running commands must finish within the timeout.";
@@ -286,11 +285,11 @@ function apply(ctx, config = {}) {
sandbox_permissions: {
type: "string",
enum: [...escalationModes],
- description: "The wider sandbox mode this command needs. Only valid as a one-shot retry of a command the sandbox just denied; requires justification and user approval."
+ description: "The wider sandbox mode this command needs. Only valid as a one-shot retry of a command the sandbox just denied; escalating to a strictly wider mode requires justification and user approval; a retry at the call's current effective mode is accepted as a no-op."
},
justification: {
type: "string",
- description: "Required with sandbox_permissions: one sentence for the user explaining why this exact command needs the wider access."
+ description: "Required with sandbox_permissions for a strictly wider mode: one sentence for the user explaining why this exact command needs the wider access."
}
} : {}
},
@@ -384,9 +383,14 @@ function apply(ctx, config = {}) {
}]
},
async execute(args, exec) {
- validateBashArgs(args);
const standingPolicy = resolveSandboxPolicy(exec);
- const approvedMode = args.sandbox_permissions !== void 0 && args.justification !== void 0 ? await approveBashEscalation(args.sandbox_permissions, args.justification, exec, standingPolicy) : void 0;
+ // A no-op escalation ask (absent, or requesting the mode the call
+ // already runs under) must skip pairing validation and the approval
+ // channel entirely: it is granted by the standing policy itself.
+ const escalationMode = normalizeEscalationMode(args.sandbox_permissions, standingPolicy?.mode);
+ validateBashArgs(args);
+ if (escalationMode !== void 0) validateEscalationArgs(args.sandbox_permissions, args.justification);
+ const approvedMode = escalationMode !== void 0 && args.justification !== void 0 ? await approveBashEscalation(args.sandbox_permissions, args.justification, exec, standingPolicy) : void 0;
const policy = approvedMode === void 0 ? standingPolicy : {
...standingPolicy,
mode: approvedMode
diff --git a/node_modules/@deepseek-ai/dsh-tool-fs/lib/index.js b/node_modules/@deepseek-ai/dsh-tool-fs/lib/index.js
index e720306..e037813 100644
--- a/node_modules/@deepseek-ai/dsh-tool-fs/lib/index.js
+++ b/node_modules/@deepseek-ai/dsh-tool-fs/lib/index.js
@@ -1,7 +1,7 @@
import z from "@deepseek-ai/schemastery";
import { defineTool } from "@deepseek-ai/dsh-tools";
import { FsError } from "@deepseek-ai/dsh-fs";
-import { ESCALATION_TARGETS, approveEscalation, canonicalPath, escalationHintMarker, sandboxDenialMarker, validateEscalationArgs } from "@deepseek-ai/dsh-sandbox";
+import { ESCALATION_TARGETS, approveEscalation, canonicalPath, escalationHintMarker, normalizeEscalationMode, sandboxDenialMarker, validateEscalationArgs } from "@deepseek-ai/dsh-sandbox";
import { structuredPatch } from "diff";
import { basename, extname } from "node:path";
import { AttachmentError, AttachmentId } from "@deepseek-ai/dsh-attachment";
@@ -1124,11 +1124,11 @@ var FsSandboxController = class {
sandbox_permissions: {
type: "string",
enum: [...this.escalationModes],
- description: "The wider sandbox mode this file operation needs. Only valid as a one-shot retry of an operation the sandbox just denied; requires justification and user approval."
+ description: "The wider sandbox mode this file operation needs. Only valid as a one-shot retry of an operation the sandbox just denied; escalating to a strictly wider mode requires justification and user approval; a retry at the call's current effective mode is accepted as a no-op."
},
justification: {
type: "string",
- description: "Required with sandbox_permissions: one sentence for the user explaining why this exact file operation needs the wider access."
+ description: "Required with sandbox_permissions for a strictly wider mode: one sentence for the user explaining why this exact file operation needs the wider access."
}
};
}
@@ -1145,9 +1145,12 @@ var FsSandboxController = class {
* unsandboxed backend.
*/
async resolvePolicy(toolName, args, exec) {
- validateEscalationArgs(args.sandbox_permissions, args.justification);
const standingPolicy = this.policy?.resolve({ ...exec.agent ? { session: exec.agent.session } : {} });
- if (args.sandbox_permissions === void 0 || args.justification === void 0) return standingPolicy;
+ // A no-op escalation ask (absent, or requesting the mode the call
+ // already runs under) is granted by the standing policy itself: it
+ // must skip pairing validation AND the approval channel entirely.
+ if (normalizeEscalationMode(args.sandbox_permissions, standingPolicy?.mode) === void 0) return standingPolicy;
+ validateEscalationArgs(args.sandbox_permissions, args.justification);
if (this.escalationModes.length === 0) throw new Error("sandbox_permissions is not available in this composition (no sandboxing filesystem to escalate)");
const policy = standingPolicy;
const approvedMode = await approveEscalation({
diff --git a/node_modules/@deepseek-ai/dsh-tool-pwsh/lib/index.js b/node_modules/@deepseek-ai/dsh-tool-pwsh/lib/index.js
index 95efaab..cdb8749 100644
--- a/node_modules/@deepseek-ai/dsh-tool-pwsh/lib/index.js
+++ b/node_modules/@deepseek-ai/dsh-tool-pwsh/lib/index.js
@@ -2,7 +2,7 @@ import { isAbsolute, resolve } from "node:path";
import z from "@deepseek-ai/schemastery";
import { TOOL_ABORTED, defineTool } from "@deepseek-ai/dsh-tools";
import { HarnessError } from "@deepseek-ai/dsh-llm";
-import { ESCALATION_TARGETS, approveEscalation, escalationHintMarker, sandboxDenialMarker, validateEscalationArgs } from "@deepseek-ai/dsh-sandbox";
+import { ESCALATION_TARGETS, approveEscalation, escalationHintMarker, normalizeEscalationMode, sandboxDenialMarker, validateEscalationArgs } from "@deepseek-ai/dsh-sandbox";
import { parseExitStatus } from "@deepseek-ai/dsh-shell";
//#region lib/types/background.js
/**
@@ -136,7 +136,6 @@ function validatePwshArgs(args) {
if (args.command.trim().length === 0) throw new Error("invalid command: expected a non-empty string");
if (args.description.trim().length === 0) throw new Error("invalid description: expected a non-empty string");
if (args.timeoutMs !== void 0 && (!Number.isFinite(args.timeoutMs) || args.timeoutMs <= 0)) throw new Error(`invalid timeoutMs: expected a positive number, got ${JSON.stringify(args.timeoutMs)}`);
- validateEscalationArgs(args.sandbox_permissions, args.justification);
}
function pwshDescription(backgroundEnabled, escalationModes) {
const base = "Execute a PowerShell command (`pwsh -Command`) and return its stdout/stderr. Each call runs in a fresh pwsh process: no state (cwd, variables, functions) persists between calls — pass `workdir` instead of using `cd`. Paths use native Windows form (`C:\\...`); read environment variables with `$env:NAME`. Non-zero exits are reported as `[exit code: N]`. Current harness environment facts are exposed through managed `$env:DSH_*` variables; inspect them when needed. Commands may run under a file sandbox; a blocked file operation is reported as `[sandbox: file access denied under <mode> mode]` — a policy denial, not a bug in the command; do not retry another way. Long output is truncated to its tail; the full output is saved to a file whose path is reported when available. On Windows a force-killed command settles as `[exit code: 1]` without a signal marker — treat it as an interruption, not a command failure. " + (backgroundEnabled ? "Set `run_in_background: true` for long-running commands: the call returns a job id immediately; read its output with `job_output` and stop it with `job_kill`." : "Background execution is not available; long-running commands must finish within the timeout.");
@@ -260,11 +259,11 @@ function apply(ctx, config = {}) {
sandbox_permissions: {
type: "string",
enum: [...escalationModes],
- description: "The wider sandbox mode this command needs. Only valid as a one-shot retry of a command the sandbox just denied; requires justification and user approval."
+ description: "The wider sandbox mode this command needs. Only valid as a one-shot retry of a command the sandbox just denied; escalating to a strictly wider mode requires justification and user approval; a retry at the call's current effective mode is accepted as a no-op."
},
justification: {
type: "string",
- description: "Required with sandbox_permissions: one sentence for the user explaining why this exact command needs the wider access."
+ description: "Required with sandbox_permissions for a strictly wider mode: one sentence for the user explaining why this exact command needs the wider access."
}
} : {}
},
@@ -358,9 +357,14 @@ function apply(ctx, config = {}) {
}]
},
async execute(args, exec) {
- validatePwshArgs(args);
const standingPolicy = resolveSandboxPolicy(exec);
- const approvedMode = args.sandbox_permissions !== void 0 && args.justification !== void 0 ? await approvePwshEscalation(args.sandbox_permissions, args.justification, exec, standingPolicy) : void 0;
+ // A no-op escalation ask (absent, or requesting the mode the call
+ // already runs under) must skip pairing validation and the approval
+ // channel entirely: it is granted by the standing policy itself.
+ const escalationMode = normalizeEscalationMode(args.sandbox_permissions, standingPolicy?.mode);
+ validatePwshArgs(args);
+ if (escalationMode !== void 0) validateEscalationArgs(args.sandbox_permissions, args.justification);
+ const approvedMode = escalationMode !== void 0 && args.justification !== void 0 ? await approvePwshEscalation(args.sandbox_permissions, args.justification, exec, standingPolicy) : void 0;
const policy = approvedMode === void 0 ? standingPolicy : {
...standingPolicy,
mode: approvedMode |
|
Reviewed the full diff against rc.2 — the shape is exactly the agreed blueprint, and the schema wording you adopted is the suggested contract text almost verbatim ("escalating to a strictly wider mode requires justification and user approval; a retry at the call's current effective mode is accepted as a no-op"). The JSDoc sync (escalation.ts pairing-exception note + the approveEscalation doc line) and the The no-op short-circuit regresses the stray-justification contract to fail-openIn rc.2, In the patched flow,
So a malformed ask that rc.2 rejects now passes silently. The intent of the fix is to skip validation for an equal-mode no-op — the absent case should still validate the pairing. Suggested adjustment, one conditional per consumer: if (args.sandbox_permissions !== undefined || args.justification !== undefined) validateEscalationArgs(args.sandbox_permissions, args.justification)i.e. validate whenever either field is present, regardless of the normalization result:
The approval-channel skip for no-ops is untouched; only the pairing-validation gate moves back to "any escalation field present". Minor notes (no change needed)
Suggested spec additionAdd one row to the escalation matrix: With that conditional, the patch is family-complete: shared helper + consumer reorder + approveEscalation defense-in-depth + schema contract sync + the three-state matrix — one PR closing both #3519 and #4359. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
环境 / Environment
@deepseek-ai/dsh-sandbox0.1.1-rc.2（DSH Desktop），Windowsdanger-full-access；审批（approval prompts）已禁用packages/sandbox/sandbox/src/escalation.ts同样存在该问题复现步骤 / Steps to reproduce
当会话当前沙箱模式已经是
danger-full-access时，任何write/edit/pwsh/bash调用只要携带sandbox_permissions: "danger-full-access"（并附justification），都会在执行前被拒绝：根因 / Root cause
approveEscalation的严格更宽校验（WIDER_MODES[effectiveMode].includes(mode)）要求请求模式必须严格宽于当前有效模式；而WIDER_MODES对danger-full-access没有任何更宽条目，同级请求也不在允许列表中，于是被当作非法升级拒绝。但同级请求本质上是 no-op：调用本来就运行在该模式下，没有任何权限变更，既不需要审批，也不应被拒绝。该请求通常来自模型预填了 schema 中公开的
sandbox_permissions可选字段（当审批被禁用时，系统提示也建议不要设置该字段，但冗余字段不应导致调用失败）。期望行为 / Expected behavior
请求模式 == 当前有效模式时直接放行（返回当前模式，不触发审批提示）。
建议修复 / Suggested fix
一行，位于严格更宽校验之前（fs / pwsh / bash 三个工具族都委托给
approveEscalation，一处修复全覆盖）：验证 / Verification
本地已按此补丁验证 5 个分支，全部符合预期：
danger-full-accessworkspace-writenot strictly wider（行为不变）无安全影响：同级放行不产生任何权限提升。
（注：按 CONTRIBUTING 说明，此处以 Discussion 形式报告 bug；如后续接受外部 PR，可基于本报告直接提交补丁。）
All reactions