From 519ea684b6ea9278bf9c97055281e8465484fcad Mon Sep 17 00:00:00 2001 From: tietoumiao <343426914+tietoumiao@users.noreply.github.com> Date: Mon, 2 Nov 2026 01:04:52 +0800 Subject: [PATCH] fix(shells): declare meta tool names and refuse a host-binary argv Declare the progressive-disclosure, PTC batch and PTC SDK loader tools in their own x-tool-namespaces entry so the manifest advertises the surface the plugin publishes, and assert that every x-components name reaches a namespace. Refuse a confined argv whose head is the host Electron binary: without ELECTRON_RUN_AS_NODE it opens a second GUI instance that exits 0 without running the command, which was indistinguishable from success. Report EXECUTOR_UNAVAILABLE instead of a silent pass. --- docs/CHANGELOG.md | 2 ++ dsh-plugin.json | 12 ++++++++ scripts/validate-manifest.mjs | 40 ++++++++++++++++++++------ src/shells/host-runner.ts | 50 +++++++++++++++++++++++++++++++++ src/shells/session-execution.ts | 15 ++++++++-- test/manifest.test.mjs | 36 +++++++++++++++++++++--- test/shells-host.test.mjs | 32 +++++++++++++++++++++ 7 files changed, 173 insertions(+), 14 deletions(-) create mode 100644 src/shells/host-runner.ts diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 5e7a499c..8713130c 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -7,6 +7,8 @@ this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +- Declare the progressive-disclosure, PTC batch and PTC SDK loader tools in their own `x-tool-namespaces` entry, so the manifest's advertised tool surface matches the namespaces it publishes under, and refuse a confined argv that would start the host Electron binary instead of reporting its exit 0 as success (#484). +- 将渐进披露、PTC batch 与 PTC SDK 加载工具声明为独立的 `x-tool-namespaces` 条目，使 manifest 宣传的工具面与其发布的命名空间一致；受限 argv 若会启动宿主 Electron 二进制，则显式拒绝，不再把它的 exit 0 当成成功（#484）。 - Keep Native file routing separate from the independent Bash prompt section, so resident Native + Shell consumers do not repeat Bash guidance or advertise host PowerShell routing. Keep deployment guidance single-owned across Bash, Native and discovery sections, including withdrawal and reload. - 将 Native 文件路由提示与独立 Bash 提示段分开，避免 Native + Shell 常驻消费者重复 Bash 指导或把 Bash 宣传为宿主 PowerShell 路由。部署指导在 Bash、Native 与发现提示段之间仅保留一份，并覆盖撤回与重载。 - Update preset hints and rendered settings UI to describe default Bash/PowerShell coexistence; hiding host PowerShell remains an explicit choice. diff --git a/dsh-plugin.json b/dsh-plugin.json index 7307e2f2..a1304711 100644 --- a/dsh-plugin.json +++ b/dsh-plugin.json @@ -100,6 +100,18 @@ "bash" ], "shellPermission": "Mode row enabled with the matching preset AND publishBashTool AND authoritative per-session DSH policy; restricted modes require host confinement" + }, + { + "serverName": "dsh-ops", + "transport": "in-process", + "source": "the plugin's progressive-disclosure, PTC batch and PTC SDK loader surface", + "publicPrefix": "ops_", + "tools": [ + "batch", + "load_sdk", + "discover_tools" + ], + "shellPermission": "No command execution of its own: batch is gated by the same per-session DSH policy and root as the file mutations it plans or runs, and load_sdk/discover_tools only select capabilities already published to that session" } ], "x-components": [ diff --git a/scripts/validate-manifest.mjs b/scripts/validate-manifest.mjs index 9f826b95..8cdf1c2e 100644 --- a/scripts/validate-manifest.mjs +++ b/scripts/validate-manifest.mjs @@ -28,7 +28,7 @@ import { publicToolName, renderToolingPolicy, } from '../lib/policy.js' -import { NATIVE_FILE_TOOLS } from '../lib/capabilities/registry.js' +import { DISCOVERY_TOOL_NAME, NATIVE_FILE_TOOLS, PTC_TOOLS, SDK_CAPABILITIES } from '../lib/capabilities/registry.js' import { resolvePtcConfig } from '../lib/ptc/index.js' import { resolveTuiConfig } from '../lib/components/tui/index.js' import { resolvePanelConfig } from '../lib/components/panel/index.js' @@ -295,24 +295,48 @@ const equippedPolicy = renderToolingPolicy({ levels: { files: true, bash: true, pwsh: true }, }) -// Every namespace after the first describes tools this plugin registers itself, -// so those names are checked against `IN_PROCESS_TOOLS` and against the policy -// text that has to name them. Written against whatever the manifest declares. +// The routing table is deliberately compact, so only the routed in-process tools +// have to be named by it; a PTC-only API or the SDK loader is never advertised there. check( IN_PROCESS_TOOLS.every((toolName) => equippedPolicy.includes(publicToolName(toolName))), 'the tools this plugin registers itself are named by the tooling policy', `policy never names ${IN_PROCESS_TOOLS.filter((name) => !equippedPolicy.includes(publicToolName(name))).map(publicToolName).join(', ')}`, ) +// Every namespace after the first describes tools this plugin registers itself. +// The names come from the capability catalogue, not from a hard-coded list: the +// shell row, the PTC-only APIs the compact native routing table never advertises, +// the PTC SDK loader and the progressive-disclosure entry. +const ownSurface = new Set([ + ...IN_PROCESS_TOOLS, + ...PTC_TOOLS.filter((toolName) => !NATIVE_FILE_TOOLS.includes(toolName)), + ...SDK_CAPABILITIES.map((capability) => capability.rawName), + DISCOVERY_TOOL_NAME, +]) +const declaredSurface = new Set(namespaces[0]?.tools ?? []) for (const [index, entry] of namespaces.slice(1).entries()) { const declared = Array.isArray(entry?.tools) ? entry.tools : [] - const identified = declared.filter((toolName) => typeof toolName === 'string' - && IN_PROCESS_TOOLS.includes(toolName) - && equippedPolicy.includes(publicToolName(toolName))) + const identified = declared.filter((toolName) => typeof toolName === 'string' && ownSurface.has(toolName)) check( declared.length > 0 && identified.length === declared.length, `every tool of the namespace at index ${index + 1} is published under ${TOOL_PREFIX} by this plugin itself`, - `declared ${JSON.stringify(entry?.tools)} vs in-process ${IN_PROCESS_TOOLS.map(publicToolName).join(', ') || '(none)'}`, + `declared ${JSON.stringify(entry?.tools)} vs own surface ${[...ownSurface].map(publicToolName).join(', ') || '(none)'}`, ) + for (const toolName of declared) { + check(!declaredSurface.has(toolName), `${publicToolName(String(toolName))} is declared by exactly one namespace`) + declaredSurface.add(toolName) + } +} +// The advertised component surface must be the declared namespace surface (#484): +// a name that only reaches x-components[].tools is invisible to a namespace consumer. +for (const component of manifestRaw.contributes?.['x-components'] ?? []) { + for (const name of Array.isArray(component?.tools) ? component.tools : []) { + const raw = typeof name === 'string' && name.startsWith(TOOL_PREFIX) ? name.slice(TOOL_PREFIX.length) : undefined + check( + raw !== undefined && declaredSurface.has(raw), + `${String(name)} is declared by an x-tool-namespaces entry`, + `declared surface ${[...declaredSurface].map(publicToolName).join(', ')}`, + ) + } } const sections = manifestRaw.contributes?.['x-prompt-sections'] ?? [] diff --git a/src/shells/host-runner.ts b/src/shells/host-runner.ts new file mode 100644 index 00000000..47d543db --- /dev/null +++ b/src/shells/host-runner.ts @@ -0,0 +1,50 @@ +/** + * Refuse a confined argv that would start the host GUI instead of the command. + * + * The Windows ACL sandbox provider builds its invocation as + * `[process.execPath, , …]`. Under DSH Desktop `process.execPath` is the + * Electron GUI binary, so spawning that argv unchanged opens a second GUI + * instance: the host's single-instance lock makes it exit 0 without ever running + * the command, and an exit-0/no-output result is indistinguishable from success. + * Nothing else in this plugin adapts that argv, so the refusal lives here rather + * than reporting a silent success. + */ + +/** The host runtime a confined argv may name. */ +export interface HostRuntime { + /** The binary the confined argv may start. */ + execPath: string + /** Whether that binary is an Electron application rather than a Node runtime. */ + electron: boolean +} + +/** The variable Electron reads to behave as a plain Node runtime. */ +const NODE_MODE = 'ELECTRON_RUN_AS_NODE' + +/** + * The host runtime as this process sees it. + * @returns the exec path and whether it is an Electron application. + */ +export function currentHostRuntime(): HostRuntime { + return { execPath: process.execPath, electron: process.versions.electron !== undefined } +} + +/** + * Whether the child environment already asks Electron to behave as Node. + * Windows environment names are case-insensitive, so a lower-cased variant counts. + */ +function nodeModeRequested(env: NodeJS.ProcessEnv): boolean { + return Object.entries(env).some(([key, value]) => key.toUpperCase() === NODE_MODE && value === '1') +} + +/** + * Refuse a confined argv that would start the host GUI instead of the command. + * @param argv - the argv the confinement provider returned. + * @param env - the environment the confined child will actually receive. + * @param host - the host runtime; injected so the guard is testable off-Electron. + * @returns the failure text, or `undefined` when the argv is safe to spawn. + */ +export function hostRunnerRefusal(argv: readonly string[], env: NodeJS.ProcessEnv = {}, host: HostRuntime = currentHostRuntime()): string | undefined { + if (!host.electron || argv[0] !== host.execPath || nodeModeRequested(env)) return undefined + return 'EXECUTOR_UNAVAILABLE: the confined argv starts the host Electron binary without ELECTRON_RUN_AS_NODE, so it would open a second GUI instance that exits 0 without running the command. This is not a user authorization denial and no command executed. Restore a confinement provider that returns a Node-runnable argv, or run the confined child with ELECTRON_RUN_AS_NODE=1; changing the DSH selector is not a repair. Do not retry.' +} diff --git a/src/shells/session-execution.ts b/src/shells/session-execution.ts index 12a3f77c..00a18839 100644 --- a/src/shells/session-execution.ts +++ b/src/shells/session-execution.ts @@ -6,6 +6,8 @@ import { bashToolDefinition, withBashStatus } from './tool.js' import { confinedBash } from '../bash-ipc.js' import { prepareWorkspace } from '../workspace-preparation.js' import { executionRestriction } from '../permissions.js' +import { shellEnvOverrides } from './constants.js' +import { hostRunnerRefusal } from './host-runner.js' export function startSessionBash({ ctx, bash, active }: { ctx: ShellContext; bash: { file: string; source: string }; active(): boolean }, args: Record, exec: ShellCall, standing: ShellPolicy, policy: ShellPolicy, dshEnv: NodeJS.ProcessEnv, options: ExecutionOptions): ShellExecution { @@ -23,8 +25,11 @@ export function startSessionBash({ ctx, bash, active }: { ctx: ShellContext; bas try { runtime = policy.mode === 'danger-full-access' ? { file: bash.file } : confinedBash(bash.file, { source: bash.source }) } catch { throw new Error('EXECUTOR_UNAVAILABLE: the restricted Bash runtime is unavailable or incompatible; this is not a user authorization denial. Restore the verified pinned Bash payload and IPC runtime; changing the DSH selector is not a repair. Do not retry or bypass confinement.') } let confinement: ConfinedArgv | undefined + // The child environment is `ENV_OVERRIDES` over these overrides and nothing else, + // so this is also the environment the host-runner guard has to read. + const childEnv = { ...runtime.env, ...dshEnv } const definition = bashToolDefinition({ file: runtime.file, source: bash.source, - env: { ...runtime.env, ...dshEnv }, subprocess: ctx.get('subprocess'), + env: childEnv, subprocess: ctx.get('subprocess'), prepareArgv: async (argv: string[], signal: AbortSignal) => { signal.throwIfAborted() assertAuthority() @@ -38,7 +43,13 @@ export function startSessionBash({ ctx, bash, active }: { ctx: ShellContext; bas } signal.throwIfAborted() assertAuthority() - return confinement?.argv ?? argv + const prepared = confinement?.argv ?? argv + // A confinement provider that hands back the host binary itself — or a bashPath + // pointed at it — would start a second GUI instance under Electron and exit 0 + // without running the command. Refuse instead of reporting that as success. + const refusal = hostRunnerRefusal(prepared, shellEnvOverrides(globalThis.process.env, childEnv)) + if (refusal) throw new Error(refusal) + return prepared }, projectResult: (value: ShellResult) => { const stderr = `${value.stderr?.text ?? ''}\n${value.error ?? ''}` diff --git a/test/manifest.test.mjs b/test/manifest.test.mjs index 20b5f901..d85dbc37 100644 --- a/test/manifest.test.mjs +++ b/test/manifest.test.mjs @@ -17,7 +17,7 @@ import { publicToolName, renderToolingPolicy, } from '../lib/policy.js' -import { NATIVE_FILE_TOOLS } from '../lib/capabilities/registry.js' +import { DISCOVERY_TOOL_NAME, IN_PROCESS_TOOLS, NATIVE_FILE_TOOLS, PTC_TOOLS, SDK_CAPABILITIES } from '../lib/capabilities/registry.js' import { COMPONENTS, componentEnabled, componentEntry } from '../lib/components/defaults.js' import { assert, PACKAGE_ROOT, report, test } from './lib/harness.mjs' @@ -75,8 +75,17 @@ await test('every declared tool is one the policy actually names', () => { // The FastCtx background backend was removed: no namespace may still advertise it. assert.equal(namespaces.some((entry) => entry.serverName === 'fastctx' || entry.transport === 'stdio'), false) - // Every further namespace is this plugin's own in-process surface, so its - // names are checked against the shell capability and against the policy. + // Every further namespace is this plugin's own in-process surface, so its names + // are checked against the capability catalogue rather than a hard-coded list: the + // shell row, the PTC-only APIs the compact native routing table never advertises, + // the PTC SDK loader and the progressive-disclosure entry. + const ownSurface = new Set([ + ...IN_PROCESS_TOOLS, + ...PTC_TOOLS.filter((name) => !NATIVE_FILE_TOOLS.includes(name)), + ...SDK_CAPABILITIES.map((capability) => capability.rawName), + DISCOVERY_TOOL_NAME, + ]) + const declared = new Set() for (const [offset, entry] of namespaces.slice(1).entries()) { const index = offset + 1 assert.ok( @@ -85,12 +94,31 @@ await test('every declared tool is one the policy actually names', () => { ) for (const tool of entry.tools) { assert.ok( - ['bash', 'pwsh'].includes(tool), + ownSurface.has(tool), `the namespace at index ${index} declares ${tool}, which the plugin does not register itself`, ) + assert.equal(declared.has(tool), false, `${tool} is declared by two namespaces`) + declared.add(tool) + } + // The routing table stays compact: it advertises the file, shell and discovery + // rows only, so a PTC-only API or the SDK loader is legitimately absent from it. + for (const tool of entry.tools.filter((name) => IN_PROCESS_TOOLS.includes(name))) { assert.ok(policy.includes(publicToolName(tool)), `the policy never names ${publicToolName(tool)}`) } } + // The declared surface has to cover the names the component advertises (#484): a + // tool that appears only in x-components[].tools is invisible to anything that + // derives the tool surface from the namespaces. + const declaredSurface = new Set([...namespaces[0].tools, ...declared]) + for (const component of raw.contributes['x-components']) { + for (const name of component.tools) { + assert.ok(name.startsWith(TOOL_PREFIX), `${name} must carry the public prefix`) + assert.ok( + declaredSurface.has(name.slice(TOOL_PREFIX.length)), + `the manifest advertises ${name} but no x-tool-namespaces entry declares it`, + ) + } + } }) await test('the declared namespace documents the upstream names and the public prefix', () => { diff --git a/test/shells-host.test.mjs b/test/shells-host.test.mjs index 61d045f5..cadcc4ad 100644 --- a/test/shells-host.test.mjs +++ b/test/shells-host.test.mjs @@ -2,6 +2,7 @@ import { assertSupportedJsonSchema, validateJsonSchemaValue } from '@deepseek-ai import { bashToolDefinition } from '../lib/shells.js' import { hostBashDefinition } from '../lib/shells/host-tool.js' import { startSessionBash } from '../lib/shells/session-execution.js' +import { hostRunnerRefusal } from '../lib/shells/host-runner.js' import { startBashJob, waitOnBashJob } from '../lib/shells/jobs.js' import { sessionWorkdir } from '../lib/session-shells.js' import { hostForeground } from '../lib/shells/host-contract.js' @@ -354,4 +355,35 @@ await test('timeout cannot claim no effects while an invoked asynchronous starte assert.equal(stopped, true); assert.equal(jobs.outcome.status, 'killed') }) +await test('a confined argv that starts the host Electron binary is refused, not reported as success (#484)', () => { + const host = { execPath: 'C:/DSH/DSH Desktop Beta.exe', electron: true } + // The shape the Windows ACL provider returns: [process.execPath, runner, …]. + const runner = [host.execPath, 'C:/DSH/resources/app/lib/dsh-sandbox-windows-acl/lib/runner.js', + '--workspace', TEST_HOME, '--', 'bash.exe', '-c', 'echo hello'] + assert.match(hostRunnerRefusal(runner, {}, host), /^EXECUTOR_UNAVAILABLE: /) + // An ordinary bash argv and a non-Electron host keep spawning their own exec path. + assert.equal(hostRunnerRefusal(['C:/bash/usr/bin/bash.exe', '-c', 'echo hello'], {}, host), undefined) + assert.equal(hostRunnerRefusal(runner, {}, { execPath: host.execPath, electron: false }), undefined) + // A child that already asks Electron to behave as Node runs the runner correctly. + assert.equal(hostRunnerRefusal(runner, { ELECTRON_RUN_AS_NODE: '1' }, host), undefined) + // Windows environment names are case-insensitive, so a lower-cased variant counts too. + assert.equal(hostRunnerRefusal(runner, { electron_run_as_node: '1' }, host), undefined) +}) + +await test('the session executor refuses the host binary instead of spawning a second GUI instance (#484)', async () => { + const f = fixture({ file: process.execPath }) + // This suite runs under plain Node; declare the Desktop runtime for this case only. + Object.defineProperty(process.versions, 'electron', { value: '34.0.0', configurable: true, enumerable: true, writable: true }) + try { + const value = await f.definition.execute({ command: 'echo hello', description: 'Run one confined command' }, f.exec) + assert.equal(f.calls.length, 0, 'the host binary must never be spawned') + assert.equal(value.status, 'failed') + assert.match(value.error, /EXECUTOR_UNAVAILABLE: the confined argv starts the host Electron binary/) + } finally { delete process.versions.electron } + // Without the Desktop runtime the same argv is a plain Node invocation and stays spawnable. + const plain = fixture({ file: process.execPath }) + const spawned = await plain.definition.execute({ command: 'echo hello', description: 'Run one plain command' }, plain.exec) + assert.equal(plain.calls.length, 1); assert.equal(spawned.error, undefined) +}) + report('shells-host')