From e758a723e283863f46133e12f6bf1f24512e37ff Mon Sep 17 00:00:00 2001 From: dsh-slopo agent Date: Tue, 22 Sep 2026 07:12:03 +0100 Subject: [PATCH] Fix: the tool substituted the deployment sandbox default for the session's dsh-bash-sandbox is the composed ctx.shell, and its resolve() stamps `request.sandboxPolicy ?? ctx.sandboxPolicy.resolve()`. Omitting the policy therefore does NOT mean "unconfined" -- it silently substitutes the DEPLOYMENT default (`DSH_PERMISSION_MODE ?? 'workspace-write'`) for the session's preset. On a host with no sandbox runner that confines a danger-full-access session and fails with SandboxUnavailableError before slopo is ever reached, which is exactly what happened: the tool was unusable while bash worked. Resolve the calling session's policy the way dsh-tool-bash does, and pass it on every call. A confining executor with no ctx.sandboxPolicy is now refused outright rather than guessed at. That fix exposes a second problem worth naming. Slopo's state lives under $DSH_HOME, which is outside the workspace root a workspace-write sandbox permits writing to, so under that preset nothing would work. The tool now refuses before running and names both ways out -- run under danger-full-access, or set stateDir to a directory inside the workspace -- instead of letting a write fail halfway through an embed. Five tests pin this: the session's policy is resolved exactly once and travels with the session, a missing policy service is refused, state outside the sandbox is refused before any execution, state inside it proceeds, and status/configure never touch the shell at all. --- README.md | 15 ++++++ index.js | 54 ++++++++++++++++++++- test/plugin.test.js | 112 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 177 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 853f71b..a4fa5d3 100644 --- a/README.md +++ b/README.md @@ -190,6 +190,21 @@ embedding failure — onto a message and a next step. **`--config-version 1` is an alarm, not boilerplate.** It makes slopo fail if its agent-facing contract has moved on, which is the signal that the skill text in `skills/` needs re-porting from upstream. +**The sandbox can block slopo's state.** Slopo's index, config and log live +under `$DSH_HOME`, which is outside the workspace that a `workspace-write` +sandbox permits writing to. The tool detects that and refuses *before* running, +naming both ways out, rather than letting a write fail halfway through an embed: +run the session under the `danger-full-access` preset, or set `stateDir` to a +directory inside the workspace. + +**What the tool does with the sandbox policy, and why it matters.** The mounted +`ctx.shell` on a confined deployment is `dsh-bash-sandbox`, whose `resolve()` +stamps `request.sandboxPolicy ?? ctx.sandboxPolicy.resolve()`. Omitting the +policy therefore does *not* mean "unconfined" — it silently substitutes the +**deployment** default (`DSH_PERMISSION_MODE ?? 'workspace-write'`), which is not +the session's preset. That is what makes a `danger-full-access` session fail with +`SandboxUnavailableError` on a host with no sandbox runner. The tool resolves the +session's policy the way `dsh-tool-bash` does and passes it per call. ## Slopo is an external process, not a library diff --git a/index.js b/index.js index a02343e..6c072b9 100644 --- a/index.js +++ b/index.js @@ -26,7 +26,7 @@ import { existsSync } from 'node:fs' import { mkdir, readFile, writeFile } from 'node:fs/promises' -import { join } from 'node:path' +import { join, sep } from 'node:path' import { fileURLToPath } from 'node:url' import { defineTool } from '@deepseek-ai/dsh-tools' @@ -497,6 +497,50 @@ export function apply(ctx, config) { }) } + // Resolve the CALLING SESSION's sandbox policy, the way the bash tool + // does. `dsh-bash-sandbox.resolve()` stamps + // `request.sandboxPolicy ?? ctx.sandboxPolicy.resolve()`, so omitting it + // does not mean "unconfined" — it silently substitutes the DEPLOYMENT + // default (`DSH_PERMISSION_MODE ?? 'workspace-write'`) for the session's + // preset. That confines a call the session asked to run unconfined, and + // on a host with no sandbox runner it fails with SandboxUnavailableError + // before slopo is ever reached. + const sandboxDefault = shell.sandboxMode + const policyService = sandboxDefault === undefined ? undefined : ctx.get('sandboxPolicy') + if (sandboxDefault !== undefined && policyService === undefined) { + return foreground({ + ok: false, + exitCode: null, + errorCode: 'NO_SANDBOX_POLICY', + message: 'The mounted shell executor confines commands, but ctx.sandboxPolicy is absent.', + remedy: 'Compose @deepseek-ai/dsh-sandbox-policy, or mount an unconfining shell executor.', + }) + } + const sandboxPolicy = policyService?.resolve( + exec.agent === undefined ? {} : { session: exec.agent.session }, + ) + + // Slopo's state is deliberately outside the workspace, which a + // workspace-write sandbox forbids writing to. Say so before a run and + // name both ways out, rather than letting a write fail mid-embed. + if ( + sandboxPolicy !== undefined && + sandboxPolicy.mode !== 'danger-full-access' && + !isInside(sandboxPolicy.workspaceRoot, paths.stateDir) + ) { + return foreground({ + ok: false, + exitCode: null, + errorCode: 'STATE_OUTSIDE_SANDBOX', + message: + `This session runs under the "${sandboxPolicy.mode}" sandbox, which permits writes only under ` + + `${sandboxPolicy.workspaceRoot} — but slopo's index, config and log live in ${paths.stateDir}.`, + remedy: + 'Either run this session under the danger-full-access preset, or set `stateDir` to a directory ' + + 'inside the workspace using action "configure".', + }) + } + /** Run one command through the shell seam. */ const run = async (command, { env, timeoutMs, signal } = {}) => { const result = await shell.run( @@ -507,6 +551,8 @@ export function apply(ctx, config) { ...(signal === undefined ? {} : { signal }), // Merges over a parent-derived base, so PATH and HOME survive. ...(env === undefined ? {} : { env }), + // Omitting this substitutes the deployment default; see above. + ...(sandboxPolicy === undefined ? {} : { sandboxPolicy }), }), ) return { @@ -626,3 +672,9 @@ async function readLogTail(path) { return '' } } + +/** True when `path` is `root` itself or sits underneath it. */ +function isInside(root, path) { + const prefix = root.endsWith(sep) ? root : root + sep + return path === root || path.startsWith(prefix) +} diff --git a/test/plugin.test.js b/test/plugin.test.js index 6e3ae73..1fd7949 100644 --- a/test/plugin.test.js +++ b/test/plugin.test.js @@ -76,15 +76,34 @@ const shell = { * `jobs` is deliberately absent, which forces the tool down its foreground path * so the assertions can see a result rather than a job handle. */ -function harness(overrides = {}, { withSettings = true, withJobs = false } = {}) { +function harness(overrides = {}, { withSettings = true, withJobs = false, sandboxMode, sandboxPolicyResolver } = {}) { const config = { ...FULL_CONFIG, ...overrides } - const state = { tools: new Map(), updates: [], provider: undefined, namespace: undefined, shell, jobs: undefined } + // A per-harness shell, so a test can vary sandboxMode and every resolved + // request is recorded for assertion. + const scopedShell = { + ...shell, + sandboxMode, + resolve(request) { + state.shellRequests.push(request) + return { workdir: process.cwd(), timeoutMs: 600_000, stdoutMaxBytes: 10_000_000, ...request } + }, + } + const state = { + tools: new Map(), + updates: [], + provider: undefined, + namespace: undefined, + shell: scopedShell, + shellRequests: [], + jobs: undefined, + } const ctx = { logger: () => ({ info() {}, warn() {}, debug() {}, error() {} }), get(name) { - if (name === 'shell') return shell + if (name === 'shell') return scopedShell if (name === 'jobs') return withJobs ? state.jobs : undefined + if (name === 'sandboxPolicy') return sandboxPolicyResolver return undefined }, inject(deps, callback) { @@ -323,3 +342,90 @@ test( assert.match(reviewed.report, /\* /) }, ) + +test('the sandbox policy is resolved for the CALLING SESSION, not the deployment default', async () => { + // dsh-bash-sandbox stamps `request.sandboxPolicy ?? ctx.sandboxPolicy.resolve()`. + // Omitting it substitutes the DEPLOYMENT default, which is what made a + // danger-full-access session fail with SandboxUnavailableError before slopo + // was ever reached. + const seen = [] + const { state } = harness( + { embeddingModel: 'ollama/example:latest', embeddingDimensions: 768 }, + { + sandboxMode: 'workspace-write', + sandboxPolicyResolver: { + resolve(arg) { + seen.push(arg) + return { mode: 'danger-full-access', workspaceRoot: '/some/workspace' } + }, + }, + }, + ) + const tool = state.tools.get('slopo') + await tool.execute({ action: 'review', foreground: true }, execContext('/tmp/nonexistent-repo-xyz')) + assert.equal(seen.length, 1, 'the policy must be resolved exactly once') + assert.ok(seen[0].session !== undefined, 'the calling session must travel with the policy') + assert.equal(state.shellRequests.length, 1) + assert.deepEqual(state.shellRequests[0].sandboxPolicy, { + mode: 'danger-full-access', + workspaceRoot: '/some/workspace', + }) +}) + +test('a confining executor with no policy service is refused rather than guessed at', async () => { + const { state } = harness( + { embeddingModel: 'ollama/example:latest', embeddingDimensions: 768 }, + { sandboxMode: 'workspace-write' }, + ) + const tool = state.tools.get('slopo') + const value = await tool.execute({ action: 'review' }, execContext('/tmp')) + assert.equal(value.ok, false) + assert.equal(value.errorCode, 'NO_SANDBOX_POLICY') + assert.deepEqual(state.shellRequests, [], 'nothing may be executed') +}) + +test('a workspace-write session is told its state lives outside the sandbox', async () => { + const { state } = harness( + { embeddingModel: 'ollama/example:latest', embeddingDimensions: 768 }, + { + sandboxMode: 'workspace-write', + sandboxPolicyResolver: { resolve: () => ({ mode: 'workspace-write', workspaceRoot: '/some/workspace' }) }, + }, + ) + const tool = state.tools.get('slopo') + const value = await tool.execute({ action: 'review' }, execContext('/some/workspace')) + assert.equal(value.ok, false) + assert.equal(value.errorCode, 'STATE_OUTSIDE_SANDBOX') + assert.match(value.message, /workspace-write/) + assert.match(value.remedy, /danger-full-access/) + assert.match(value.remedy, /stateDir/) + assert.deepEqual(state.shellRequests, [], 'it must not run where it cannot write') +}) + +test('a workspace-write session with stateDir inside the workspace proceeds', async () => { + const workspace = mkdtempSync(join(tmpdir(), 'dsh-slopo-ws-')) + const { state } = harness( + { + embeddingModel: 'ollama/example:latest', + embeddingDimensions: 768, + stateDir: join(workspace, '.slopo'), + }, + { + sandboxMode: 'workspace-write', + sandboxPolicyResolver: { resolve: () => ({ mode: 'workspace-write', workspaceRoot: workspace }) }, + }, + ) + const tool = state.tools.get('slopo') + const value = await tool.execute({ action: 'review', foreground: true }, execContext(workspace)) + assert.notEqual(value.errorCode, 'STATE_OUTSIDE_SANDBOX') + assert.ok(state.shellRequests.length > 0, 'it should have attempted the run') + assert.deepEqual(state.shellRequests[0].sandboxPolicy, { mode: 'workspace-write', workspaceRoot: workspace }) +}) + +test('configure and status never touch the shell, so they work under any sandbox', async () => { + const { state } = harness({}, { sandboxMode: 'workspace-write' }) + const tool = state.tools.get('slopo') + await tool.execute({ action: 'status' }, execContext('/some/workspace')) + await tool.execute({ action: 'configure', config: { embeddingModel: 'x' } }, execContext('/some/workspace')) + assert.deepEqual(state.shellRequests, []) +})