docs: clarify shell-step interpolation safety (#3719)

* docs: clarify shell-step interpolation safety

Shell step `run` fields are executed by the system shell and `{{ ... }}`
expressions are substituted as raw, unquoted text. Document that untrusted
sources — workflow `inputs.*` and prior-step output, including AI-generated
`prompt` output — must be quoted, enum-constrained, validated, or gated before
they reach a `run` field.

- docs/reference/workflows.md: add an "Interpolation and shell safety" section.
- workflows/README.md: add a warning under the Shell Steps example, link to the
  new section, and quote the `inputs.project_dir` example.
- workflows/PUBLISHING.md: strengthen the interpolation guidance and call out
  prior-step/agent output as untrusted.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c0fd6900-69bf-4fcb-b377-de37f98f5835

* docs: correct shell-step interpolation guidance

Address review feedback that the previous wording over-promised. Clarify that
none of the mitigations neutralise a hostile interpolated value:

- Quoting is not a security boundary — there is no shell-escaping filter, and a
  value containing the matching quote can break out. Present quoting as
  correctness handling for already-constrained values only.
- Remove the "pass data via environment or files" guidance: ShellStep has no
  `env` mapping (it only copies the process environment and sets
  SPECKIT_WORKFLOW_DIR), so that transport does not exist.
- Drop the claim that routing through a command/prompt step validates or safely
  binds a value; it does not.
- Correct the gate guidance: a gate renders only its own message/show_file and
  does not inspect, resolve, or sanitise the following step. Authors must
  surface the exact command/data in the gate themselves, and approval does not
  neutralise an injectable interpolation.

Frame constraining values at the source (enum/allowlist) as the only reliable
control, and keeping unconstrained values out of `run` fields entirely.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c0fd6900-69bf-4fcb-b377-de37f98f5835

* docs: remove unsafe interpolation from example and gate guidance

Address further review feedback:

- workflows/README.md: the shell example interpolated an unconstrained path
  into shell source, which contradicted the warning beneath it. Shell steps
  already run from the project root, so drop the `cd '{{ inputs.project_dir }}'`
  prefix and model a plain `run: "npm test"` with no interpolation.
- docs/reference/workflows.md: GateStep prints `message` verbatim with no
  control-character stripping (stripping applies only to `show_file` path and
  contents), so recommending that authors surface untrusted data in `message`
  was itself unsafe — agent/caller output could inject terminal escapes to
  alter or hide the prompt. Direct authors to keep `message` to trusted text
  and surface untrusted material via `show_file`, whose contents are stripped.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c0fd6900-69bf-4fcb-b377-de37f98f5835

* docs: use correct prompt-step output key in example

A `prompt` step stores agent-generated text under `output.stdout`, not
`output.value`, so the example expression `{{ steps.plan.output.value }}`
would resolve to None. Reference `output.stdout` so the example correctly
demonstrates untrusted agent output flowing into a shell step.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c0fd6900-69bf-4fcb-b377-de37f98f5835

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c0fd6900-69bf-4fcb-b377-de37f98f5835

docs/reference/workflows.md

@@ -502,6 +502,32 @@ args: "{{ inputs.spec }}"
 message: "{{ status | default('pending') }}"
 ```
 
+### Interpolation and shell safety
+
+Expressions are resolved by **plain string substitution** — the value of `{{ ... }}` is spliced into the surrounding text exactly as-is, with no quoting or escaping added. That is convenient for building `args` and `message` strings, but it has an important consequence for `shell` steps: a `run` field is handed to the system shell (`/bin/sh -c` on POSIX), so any interpolated value is interpreted as **shell syntax**, not just data.
+
+If an interpolated value can contain characters like `;`, `|`, `&`, `$( )`, backticks, or quotes, it can change or extend the command that actually runs. This matters most when the value is not fully under the workflow author's control:
+
+- **Workflow `inputs.*`** — supplied by whoever runs the workflow.
+- **A prior step's output**, e.g. `{{ steps.plan.output.stdout }}` — for a `prompt` step this is **text produced by the AI agent**, which can in turn be influenced by files, tickets, or web content the agent read. Treat agent output as untrusted when it flows into a `shell` step.
+
+There is **no shell-escaping filter** in the expression language and **no sandbox** around a `shell` step, so none of the practices below can be treated as a guarantee that a hostile value is neutralised. The only reliable control is to constrain what an interpolated value *can* be, and to keep values you cannot constrain out of `run` fields entirely. Scrutinise every `run` field that interpolates a value you do not control, and at minimum:
+
+- **Constrain the value at the source with `enum`/an allowlist.** When `inputs.*` feeds a `run` field, restrict it to a fixed set of known-safe values so a caller cannot supply arbitrary shell text at all. This is the strongest control the engine offers — prefer it over any downstream mitigation.
+
+  ```yaml
+  inputs:
+    target:
+      type: string
+      enum: [staging, production]   # caller cannot inject arbitrary text
+  ```
+
+- **Keep unconstrained values out of `run`.** If a value cannot be constrained to an allowlist — most agent/`prompt` output — do not interpolate it into a `run` field. Branch on it with `if`/`switch` against fixed conditions, or act on it in a `command`/`prompt` step rather than a shell command built from it.
+- **Quoting is not a security boundary.** Surrounding a substitution with quotes (`'{{ inputs.x }}'`) helps the shell treat a *trusted* value as a single argument and avoids word-splitting on spaces, but a value that itself contains the matching quote character can still break out and inject shell syntax. Quote for correctness on constrained values; never rely on quoting to make an *unconstrained* substitution safe.
+- **Gates do not inspect the next step, and `message` is printed verbatim.** A `gate` step renders only its own `message`/`show_file` — it does not display, resolve, or sanitise the command that follows it, and approval never neutralises an injectable interpolation. Do **not** interpolate raw untrusted data into `message`: it is printed as-is with no control-character stripping, so agent or caller output could inject terminal/ANSI escapes that alter or hide the approval prompt. Keep `message` to trusted, constrained text, and surface untrusted material for review via `show_file` instead — its path and contents are control/ANSI-stripped before display.
+
+A `shell` step is an arbitrary-command primitive by design; these practices reduce exposure and keep *which* command runs under the author's control, but they do not eliminate the risk of interpolating values you do not fully control.
+
 ## Shell Step Environment Variables
 
 Shell steps automatically receive the following environment variables:

workflows/PUBLISHING.md

@@ -280,7 +280,8 @@ Workflow `shell` steps execute their `run` field through `/bin/sh` (POSIX) or th
 Catalog-listed workflows are reviewed at submission time (see [Verification Process](#verification-process)), but you should still treat every install as code-execution from an untrusted source until you have read the `workflow.yml`:
 
 - **Before installing a workflow**, fetch the raw YAML and audit every `shell` step's `run` field directly. `specify workflow info <name>` only shows metadata (name, version, inputs, step IDs/types) — not the shell content that would actually execute.
-- **Prefer explicit commands over interpolation** in `run` blocks: `{{ inputs.something }}` substitutions should be quoted and constrained via `enum` so a malicious input can't inject shell syntax.
+- **Constrain interpolated values, don't just quote them** in `run` blocks: expressions are spliced in as raw text with no automatic escaping, and there is no shell-escaping filter, so quoting is not a security boundary. Restrict `{{ inputs.something }}` substitutions to a fixed set with `enum`/an allowlist so a malicious input can't inject shell syntax; treat quoting only as correctness handling for already-constrained values.
+- **Treat prior-step output as untrusted too** — `{{ steps.*.output.* }}` from a `prompt` step is AI-generated text that upstream content can influence. Don't interpolate agent output into a `run` field at all when you can't constrain it; branch on it with `if`/`switch` or act on it in a non-shell step instead.
 - **Limit privilege**: shell steps inherit the user's environment. Workflows that need elevated access (sudo, secrets, GitHub tokens) should call them out explicitly in the README so reviewers can spot the requirement.
 - **Authors**: if your workflow has shell steps that look risky out of context (deletions, network calls, credential reads), document the rationale in your README. Maintainers will reject submissions whose shell steps can't be justified at review time.
 

workflows/README.md

@@ -111,10 +111,22 @@ Run a shell command and capture output:
 ```yaml
 - id: run-tests
   type: shell
-  run: "cd {{ inputs.project_dir }} && npm test"
+  run: "npm test"       # runs from the project root; no interpolation needed
   timeout: 1800   # Optional: max seconds before the command is killed (default 300)
 ```
 
+> ⚠️ **Constrain interpolated values in `run` fields.** A `run` field is executed
+> by the system shell, and `{{ ... }}` expressions are substituted as raw text
+> with no automatic quoting or escaping. An `inputs.*` value or prior-step output
+> (including AI-generated `prompt` output) is parsed as shell syntax and can
+> change the command that runs. The only reliable control is to restrict such a
+> value at the source with an `enum`/allowlist, or to keep values you cannot
+> constrain out of `run` entirely. Quoting a substitution helps a trusted value
+> survive word-splitting but is **not** a security boundary — there is no
+> shell-escaping filter, and a value containing the matching quote can still
+> break out. See
+> [Interpolation and shell safety](../docs/reference/workflows.md#interpolation-and-shell-safety).
+
 `timeout` is the maximum time in seconds the command may run before it is
 killed and the step fails; it must be a positive number and defaults to
 `300` (five minutes) when omitted. Raise it for long-running gates such as