feat(bash): generalize managed shell environment
This commit is contained in:
@@ -14,9 +14,9 @@ Add `stdin?: string` and `env?: Record<string, string>` to **both** `BashExecReq
|
||||
|
||||
Three deliberate choices:
|
||||
|
||||
1. **The model-facing `bash` tool does NOT expose `stdin`/`env` as parameters** — not as a security wall, but because bash syntax already covers the model's needs, so duplicating them as tool params would be redundant surface. [dsh-tool-bash](../../../../packages/bash/tool-bash)'s `bash` tool builds its `BashExecRequest` from named schema fields and may add harness-owned environment such as the current [session identity and JSONL location](../feature/2026-07-10-agent-session-identity-and-log-location.md); a model that includes `env`/`stdin` keys in its tool-call arguments has them ignored and cannot replace that overlay. Regression guards drive the real tool with extra args and assert no model-provided field enters the request. In-process plugins (the hooks bridges, native plugins) that construct a `BashExecRequest` directly set the fields; the seam imposes no access policy (consistent with how `owner` works — the executor stores but never interprets it).
|
||||
1. **The model-facing `bash` tool does NOT expose `stdin`/`env` as parameters** — not as a security wall, but because bash syntax already covers the model's needs, so duplicating them as tool params would be redundant surface. [dsh-tool-bash](../../../../packages/bash/tool-bash)'s `bash` tool builds its `BashExecRequest` from named schema fields; a model that includes `env`/`stdin` keys has them ignored. Harness-owned variables use the distinct `dshEnv` channel added by the [session environment decision](../feature/2026-07-10-agent-session-identity-and-log-location.md), so ordinary `env` cannot replace them. In-process plugins such as hook bridges construct requests directly and set ordinary `stdin`/`env`; the seam otherwise imposes no access policy.
|
||||
|
||||
2. **`env` merges AFTER the credential scrub, so an explicit caller entry always wins** — even a credential-shaped name. This is correct because the scrub's job is narrow: stop the harness's *ambient* `process.env` credentials from leaking into a spawned command. A caller that explicitly sets a var has named a value it already holds (not the ambient secret), so the scrub is not a constraint on it. `childEnv(extra?)` layers `scrub(process.env)` → `ENV_OVERRIDES` (the model-friendly `TERM=dumb` etc.) → `extra`, last-wins.
|
||||
2. **`env` merges AFTER the credential scrub, so an explicit caller entry wins even on a credential-shaped name.** The later managed-namespace decision reserves `DSH_*`: ambient entries are removed, ordinary `env` cannot set them, and trusted `dshEnv` merges last. The complete order is `scrub(process.env, including DSH_*)` → `ENV_OVERRIDES` → ordinary `env` → `dshEnv`.
|
||||
|
||||
3. **`stdin`/`env` are required-absent-OK (plain optional) on the resolved spec, NOT required-but-nullable like `owner`.** `owner` is required-but-nullable because a *silently* missing owner yields an unowned, cross-session-readable task — a security footgun that a visible `undefined` guards against. `stdin`/`env` have no such hazard: a missing one means "no stdin / no extra env", which is the safe, ordinary case (every model-driven call). So they stay plain optionals, matching `signal`.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user