From 5533bb783a87d5430253ea76667b119d13075a36 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 2 Jul 2026 04:30:40 +0800 Subject: [PATCH] docs(bash): purge remaining trusted-plugin wording caught in review Codex review of the reframe found stale "trusted-plugin surface/boundary" wording still in review-relevant spots the first pass missed: - docs/rfc/README.md index title for the RFC. - packages/bash/bash-local/src/run.ts (childEnv JSDoc + SpawnSpec stdin/env JSDoc + the spawn stdin comment) and src/index.ts (resolve carry-through comment); run.ts also pointed at a tool-bash README section name that no longer exists. - the two bash-local test descriptors (run.spec.ts / executor.spec.ts). - the tool-bash guard test's `boundary-*` call ids and one "boundary assertion" comment (renamed to `no-forward-*`). All reworded to the scrub-is-the-control framing (or neutral wording). The RFC FILENAME keeps `-trusted-plugin-surface` as a stable id (many links point at it; the index title and content are corrected). No code or behavior change. --- docs/rfc/README.md | 2 +- packages/bash/bash-local/src/index.ts | 4 +-- packages/bash/bash-local/src/run.ts | 28 ++++++++++--------- .../bash/bash-local/tests/executor.spec.ts | 2 +- packages/bash/bash-local/tests/run.spec.ts | 2 +- packages/bash/tool-bash/tests/tools.spec.ts | 6 ++-- 6 files changed, 23 insertions(+), 21 deletions(-) diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 5c47c98599..f40d47a3e9 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -121,7 +121,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Branded IDs everywhere they belong](implemented/architecture/2026-06-20-branded-ids.md) | 2026-06-20 | | [Extract example apps into packages](implemented/architecture/2026-06-20-extract-example-app-packages.md) | 2026-06-20 | | [Event-domain semantics — session is the fact log, agent is the live surface](implemented/architecture/2026-06-30-event-domain-semantics.md) | 2026-06-30 | -| [stdin + extra env on the bash seam — a trusted-plugin surface](implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md) | 2026-06-30 | +| [stdin + extra env on the bash seam](implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md) | 2026-06-30 | ### Process diff --git a/packages/bash/bash-local/src/index.ts b/packages/bash/bash-local/src/index.ts index 53c369a24c..b1ea729fb4 100644 --- a/packages/bash/bash-local/src/index.ts +++ b/packages/bash/bash-local/src/index.ts @@ -116,8 +116,8 @@ export class LocalBashExecutor extends BashExecutor { workdir: request.workdir ?? this.config.cwd ?? process.cwd(), timeoutMs, ...request.signal ? { signal: request.signal } : {}, - // Carry the trusted-plugin stdin/env through verbatim — optional, no - // config default (absent means none). env merges AFTER the scrub in run.ts. + // Carry stdin/env through verbatim — optional, no config default (absent + // means none). env merges AFTER the scrub in run.ts. ...request.stdin !== undefined ? { stdin: request.stdin } : {}, ...request.env !== undefined ? { env: request.env } : {}, // Carry the owner through verbatim (required-but-nullable on the spec): diff --git a/packages/bash/bash-local/src/run.ts b/packages/bash/bash-local/src/run.ts index 5b67bbdc1b..de3d880c7c 100644 --- a/packages/bash/bash-local/src/run.ts +++ b/packages/bash/bash-local/src/run.ts @@ -48,12 +48,13 @@ export const SENSITIVE_ENV_PATTERN = /KEY|SECRET|TOKEN/i * * Layering matters: the scrub drops `process.env` credentials, then * `ENV_OVERRIDES` forces the model-friendly terminal vars, then `extra` is - * merged LAST so a TRUSTED-PLUGIN entry wins even when its name matches the - * scrub pattern (the scrub guards against leaking the HARNESS's ambient - * credentials into model-driven commands; an in-process plugin that explicitly - * sets a var has taken responsibility for it). `extra` is NEVER model-supplied - * — `dsh-tool-bash` does not forward model input here (see its README, § - * "Trusted-plugin boundary"). + * merged LAST so an explicit caller entry wins even when its name matches the + * scrub pattern (the scrub is the control that stops the HARNESS's ambient + * credentials leaking into a spawned command; a caller that explicitly sets a + * var named a value it already holds, not that ambient secret). `extra` is set + * by in-process plugins (the hooks bridges), not the model — `dsh-tool-bash` + * builds its request from named fields only and does not forward model input + * here (see its README, § "The tool builds its request from named args only"). */ export function childEnv(extra?: Record): NodeJS.ProcessEnv { const env: NodeJS.ProcessEnv = {} @@ -75,14 +76,15 @@ export interface SpawnSpec { signal?: AbortSignal | undefined /** * Bytes to write to the child's stdin, then close it. Absent (or empty) - * leaves stdin closed/empty. A TRUSTED-PLUGIN surface (see {@link SpawnSpec}'s - * consumer `dsh-bash`); never carries model input. + * leaves stdin closed/empty. Set by in-process plugins (the hooks bridges); + * the model-facing `dsh-tool-bash` tool does not thread model input here. */ stdin?: string | undefined /** * Extra environment entries, merged onto the scrubbed env AFTER the * credential scrub and the model-friendly overrides (so an explicit entry - * wins). A TRUSTED-PLUGIN surface; never carries model input. + * wins). Set by in-process plugins; the model-facing tool does not forward + * model input here. */ env?: Record | undefined } @@ -297,10 +299,10 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB } // stdin is ALWAYS a pipe (kept literal so the typed spawn overload guarantees - // non-null stdout/stderr) and is closed immediately: with bytes when a - // trusted plugin supplied stdin, empty otherwise. A closed empty pipe gives a - // reading child EOF exactly as `/dev/null` would, so the no-stdin path (every - // model-driven call) is unchanged. + // non-null stdout/stderr) and is closed immediately: with bytes when a caller + // supplied stdin, empty otherwise. A closed empty pipe gives a reading child + // EOF exactly as `/dev/null` would, so the no-stdin path (every model-driven + // call) is unchanged. const child = spawn('bash', ['-c', spec.command], { cwd: spec.cwd, env: childEnv(spec.env), diff --git a/packages/bash/bash-local/tests/executor.spec.ts b/packages/bash/bash-local/tests/executor.spec.ts index 03f602a2f1..cf6d1c267e 100644 --- a/packages/bash/bash-local/tests/executor.spec.ts +++ b/packages/bash/bash-local/tests/executor.spec.ts @@ -110,7 +110,7 @@ describe('LocalBashExecutor.run', () => { it('resolve() carries stdin/env onto the spec, and run() threads them to the command', async () => { const { bash } = await setup() const spec = bash.resolve({ command: 'cat; echo "[$DSH_SEAM_VAR]"', stdin: 'piped\n', env: { DSH_SEAM_VAR: 'env-ok' } }) - // resolve() keeps the trusted-plugin fields verbatim (optional, no default). + // resolve() keeps the stdin/env fields verbatim (optional, no default). expect(spec.stdin).toBe('piped\n') expect(spec.env).toEqual({ DSH_SEAM_VAR: 'env-ok' }) const result = await bash.run(spec) diff --git a/packages/bash/bash-local/tests/run.spec.ts b/packages/bash/bash-local/tests/run.spec.ts index 3859f2ac39..395f442dad 100644 --- a/packages/bash/bash-local/tests/run.spec.ts +++ b/packages/bash/bash-local/tests/run.spec.ts @@ -158,7 +158,7 @@ describe('runBash', () => { }) }) -describe('stdin and extra env (trusted-plugin surface)', () => { +describe('stdin and extra env (set by in-process plugins)', () => { it('writes stdin to the command and closes it', async () => { const result = await runBash(spec('cat', { stdin: 'hello from stdin\n' })).done expect(result.exitCode).toBe(0) diff --git a/packages/bash/tool-bash/tests/tools.spec.ts b/packages/bash/tool-bash/tests/tools.spec.ts index 79e8ea183d..8fd131d251 100644 --- a/packages/bash/tool-bash/tests/tools.spec.ts +++ b/packages/bash/tool-bash/tests/tools.spec.ts @@ -924,7 +924,7 @@ describe('the model-facing bash tool builds its request from named args only (no // var or feed stdin via shell syntax anyway; this just keeps the request // shape honest so a future `...args` spread can't silently forward input.) await ctx.tools.execute({ - callId: CallId('boundary-1'), + callId: CallId('no-forward-1'), name: 'bash', arguments: { command: 'echo hi', @@ -943,9 +943,9 @@ describe('the model-facing bash tool builds its request from named args only (no 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. + // request — which is all this no-forward assertion needs. await ctx.tools.execute({ - callId: CallId('boundary-2'), + callId: CallId('no-forward-2'), name: 'bash', arguments: { command: 'sleep 1',