feat(bash): add stdin + extra env to the executor seam as a trusted-plugin surface
The hooks subsystem runs external hook commands the Claude Code / Codex way: JSON payload on stdin, context in CLAUDE_PROJECT_DIR / CLAUDE_PLUGIN_ROOT env. Reusing the ctx.bash seam for that needs two new inputs — but stdin and arbitrary env are exactly what dsh-bash-local's credential scrub exists to keep away from model-driven commands. So this adds them as a TRUSTED-PLUGIN surface: - BashExecRequest + BashExecSpec gain optional `stdin` and `env`. They are plain optionals on the resolved spec (not required-but-nullable like `owner`): a missing one means "none", the safe default, not a security footgun. - dsh-bash-local threads them through resolve/run/start. `env` merges AFTER the credential scrub, so a trusted caller's explicit entry wins even on a credential-shaped name — the scrub guards the harness's OWN ambient creds from model-driven commands, not a trusted plugin. stdin is always a pipe, closed immediately (with bytes when supplied, empty otherwise — EOF as before); an EPIPE from a child that exits without reading is swallowed. - The model-facing dsh-tool-bash NEVER forwards model input into stdin/env (its request is command/workdir/timeoutMs/signal/owner only). A regression guard drives the real tool with adversarial args and asserts the request carries neither field — proven to go red if the consumer ever forwards them. Configurable scrub (in an earlier sketch) is dropped as speculative: the explicit `env` field already gives a trusted caller full control, and no caller needs to broaden the ambient scrub. Documented in a new architecture RFC, the bash.md type-equiv blocks, and the three bash READMEs.
This commit is contained in:
@@ -40,6 +40,10 @@ These tools own how their calls render in a UI (an editor's tool-call card) via
|
||||
|
||||
When a background task finishes, a short notice is injected into the owning agent's session (`agent.inject()`, source `{kind: 'plugin', plugin: 'tool-bash'}`). The owning agent is found by its session token: the listener reads `ctx.bash.ownerOf(task.id)` and scans `ctx.get('agents')?.list()` for an agent whose `session.header.id` matches (read via `ctx.get` — `onTaskDone` runs on the bash fiber, a foreign fiber, so the `ctx.agents` proxy would throw). If no live agent carries that token — e.g. the owning session disconnected and its agent was disposed while the task ran on — the notice is dropped cleanly. Injection is **durable context for the next model request, not a wake-up** — an idle agent stays idle until something sends a message. That's why the tool descriptions tell the model to poll with `bash_output`.
|
||||
|
||||
## Trusted-plugin boundary: env / stdin are never model-driven
|
||||
|
||||
The `BashExecRequest` seam carries optional `stdin` and `env` (a **trusted-plugin surface** used by the hooks bridges to feed a hook command its JSON payload and `CLAUDE_*` env). This tool deliberately **never** threads model input into either: its request is built from `command`/`workdir`/`timeoutMs`/`signal`/`owner` only, so a model that includes `env` or `stdin` keys in its tool arguments has them ignored — it cannot smuggle an environment variable or stdin payload past `dsh-bash-local`'s credential scrub. A regression guard (the "trusted-plugin boundary" tests) drives the real tool with adversarial args and asserts the resulting request carries neither field. See [the trusted-plugin RFC](../../../docs/rfc/implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md).
|
||||
|
||||
## Permissions
|
||||
|
||||
`TODO(permissions)`: commands run with the executor's full authority. The permission/sandbox seam is the `tools/execute` waterfall (veto or ask) plus sandboxing `BashExecutor` implementations — see docs/architecture.md. `@cordisjs/plugin-capability` (a named-permission service with a session `test()`) is a candidate building block for that work.
|
||||
|
||||
@@ -863,3 +863,99 @@ describe('tool-owned UI presentation (presentCall / presentResult)', () => {
|
||||
expect(ctx.tools.get('bash')?.presentCall?.({ command: 'ls' })).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('trusted-plugin boundary: the model-facing bash tool never sets env/stdin', () => {
|
||||
/**
|
||||
* Records every {@link BashExecRequest} the consumer hands to `resolve()`, so a
|
||||
* test can assert what the model-facing tool DID and DID NOT forward. `stdin`
|
||||
* and `env` are a TRUSTED-PLUGIN surface (in-process plugins only); the `bash`
|
||||
* tool must never thread model-supplied input into them, even when the model
|
||||
* smuggles extra keys into the tool arguments. Foreground `run()` returns a
|
||||
* canned result; `start()` is unused here.
|
||||
*/
|
||||
class RecordingBashExecutor extends BashExecutor {
|
||||
readonly requests: BashExecRequest[] = []
|
||||
resolve(request: BashExecRequest): BashExecSpec {
|
||||
this.requests.push(request)
|
||||
return {
|
||||
command: request.command,
|
||||
workdir: request.workdir ?? process.cwd(),
|
||||
timeoutMs: request.timeoutMs ?? 0,
|
||||
...request.signal ? { signal: request.signal } : {},
|
||||
...request.stdin !== undefined ? { stdin: request.stdin } : {},
|
||||
...request.env !== undefined ? { env: request.env } : {},
|
||||
owner: request.owner,
|
||||
}
|
||||
}
|
||||
run(): Promise<BashRunResult> {
|
||||
return Promise.resolve({
|
||||
exitCode: 0, signal: null, timedOut: false, aborted: false, timeoutMs: 0,
|
||||
stdout: { text: 'ok', truncated: false }, stderr: { text: '', truncated: false },
|
||||
})
|
||||
}
|
||||
start(): BashTask { throw new Error('unused') }
|
||||
get(): BashTask | undefined { return undefined }
|
||||
ownerOf(): OwnerToken | undefined { return undefined }
|
||||
list(): BashTask[] { return [] }
|
||||
readOutput(): BashTaskRead { throw new Error('unused') }
|
||||
kill(): boolean { return false }
|
||||
}
|
||||
|
||||
async function setupRecording() {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(AgentRegistry)
|
||||
await ctx.plugin(RecordingBashExecutor)
|
||||
await ctx.plugin(ToolBash)
|
||||
return { ctx, bash: ctx.bash as RecordingBashExecutor }
|
||||
}
|
||||
|
||||
it('does not forward env/stdin even when the model smuggles them as extra arguments', async () => {
|
||||
const { ctx, bash } = await setupRecording()
|
||||
// Adversarial args: the model includes `env` and `stdin` keys (and a
|
||||
// credential-shaped value) hoping they reach the executor. The bash tool's
|
||||
// schema ignores unknown keys, and execute() builds the request from only
|
||||
// command/workdir/timeoutMs/signal — so the recorded request carries NEITHER.
|
||||
await ctx.tools.execute({
|
||||
callId: CallId('boundary-1'),
|
||||
name: 'bash',
|
||||
arguments: {
|
||||
command: 'echo hi',
|
||||
description: 'echo',
|
||||
env: { SNEAKY_API_KEY: 'leak' },
|
||||
stdin: 'malicious payload',
|
||||
},
|
||||
})
|
||||
expect(bash.requests).toHaveLength(1)
|
||||
const request = bash.requests[0]!
|
||||
expect(request.command).toBe('echo hi')
|
||||
expect('env' in request).toBe(false)
|
||||
expect('stdin' in request).toBe(false)
|
||||
})
|
||||
|
||||
it('a background bash call likewise carries no env/stdin', async () => {
|
||||
const { ctx, bash } = await setupRecording()
|
||||
// start() throws in this recorder, but resolve() runs first and records the
|
||||
// request — which is all this boundary assertion needs.
|
||||
await ctx.tools.execute({
|
||||
callId: CallId('boundary-2'),
|
||||
name: 'bash',
|
||||
arguments: {
|
||||
command: 'sleep 1',
|
||||
description: 'sleep',
|
||||
run_in_background: true,
|
||||
env: { TOKEN: 'leak' },
|
||||
stdin: 'x',
|
||||
},
|
||||
})
|
||||
expect(bash.requests).toHaveLength(1)
|
||||
const request = bash.requests[0]!
|
||||
expect('env' in request).toBe(false)
|
||||
expect('stdin' in request).toBe(false)
|
||||
// The owner token IS set on a background call (the isolation fence) — proving
|
||||
// the recorder sees the real request the consumer built, so the absent
|
||||
// env/stdin above is a real negative, not a recorder that drops everything.
|
||||
expect('owner' in request).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user