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.
This commit is contained in:
@@ -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):
|
||||
|
||||
@@ -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<string, string>): 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<string, string> | 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),
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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',
|
||||
|
||||
Reference in New Issue
Block a user