@@ -250,16 +250,19 @@ Propagation rules: |
| 250 | 250 | |
| 251 | 251 | - Reading `shithub.event.X` → `Tainted: true` (always, including |
| 252 | 252 | missing-path null results). |
| 253 | | -- Reading any other namespace → `Tainted: false`. |
| 253 | +- Reading any other namespace → `Tainted: false`, except `env.X` |
| 254 | + preserves the taint of the resolved env value. This closes the |
| 255 | + escape where an event-derived value is first assigned to env and |
| 256 | + then interpolated through `${{ env.X }}`. |
| 254 | 257 | - Binary op (`==`, `!=`, `&&`, `||`) → tainted if either operand is. |
| 255 | 258 | - Unary op (`!`) → tainted iff its operand is. |
| 256 | 259 | - Function call (`contains`, `startsWith`, `endsWith`) → tainted |
| 257 | 260 | if any argument is. |
| 258 | 261 | |
| 259 | | -The runner (S41d) consumes `Tainted` and refuses to interpolate |
| 260 | | -tainted values into shell strings. Instead, tainted values are |
| 261 | | -bound to `${SHITHUB_INPUT_xx}` envvars set by the runner with no |
| 262 | | -shell expansion. The author writes: |
| 262 | +The runner consumes `Tainted` and refuses to interpolate tainted |
| 263 | +values into shell strings. Instead, tainted values are bound to |
| 264 | +runner-owned `SHITHUB_INPUT_xx` envvars and the shell source only |
| 265 | +references those placeholders. The author writes: |
| 263 | 266 | |
| 264 | 267 | ```yaml |
| 265 | 268 | - run: echo "PR title was: ${{ shithub.event.pull_request.title }}" |
@@ -268,18 +271,32 @@ shell expansion. The author writes: |
| 268 | 271 | The runner sees a tainted reference; it compiles the step to: |
| 269 | 272 | |
| 270 | 273 | ```bash |
| 271 | | -SHITHUB_INPUT_01="$user_pr_title" exec sh -c 'echo "PR title was: $SHITHUB_INPUT_01"' |
| 274 | +SHITHUB_INPUT_0="$user_pr_title" exec sh -c 'echo "PR title was: $SHITHUB_INPUT_0"' |
| 272 | 275 | ``` |
| 273 | 276 | |
| 274 | | -…where `$user_pr_title` is set via Go's `cmd.Env`, never via |
| 275 | | -shell-string interpolation. Backticks, `$()`, `;`, `&&` — none of |
| 276 | | -those work as injection vectors when the value never reaches the |
| 277 | | -shell parser. |
| 278 | | - |
| 279 | | -Tests for this contract live in `internal/actions/expr/eval_test.go` |
| 280 | | -under `TestEval_Taint*`. **Do not** weaken them in a later PR |
| 281 | | -without an audit-checkpoint review — they're explicitly load-bearing |
| 282 | | -for S41e's threat model. |
| 277 | +…where `$user_pr_title` is set via Go's `cmd.Env`, never inserted into |
| 278 | +the shell source string. Backticks, `$()`, `;`, `&&` — none of those |
| 279 | +work as command-injection vectors when the value reaches the shell as |
| 280 | +environment data instead of syntax. |
| 281 | + |
| 282 | +The shared renderer lives in `internal/runner/exec`, so future engines |
| 283 | +consume the same injection boundary instead of reimplementing it. The |
| 284 | +runner claim payload includes `workflow_runs.event_payload`; without |
| 285 | +that field, the runner cannot evaluate and taint |
| 286 | +`${{ shithub.event.* }}` references. |
| 287 | + |
| 288 | +Tests for this contract live in `internal/actions/expr/eval_test.go`, |
| 289 | +`internal/runner/exec/render_test.go`, and |
| 290 | +`internal/runner/engine/docker_test.go`. **Do not** weaken them in a |
| 291 | +later PR without an audit-checkpoint review — they're explicitly |
| 292 | +load-bearing for S41e's threat model. |
| 293 | + |
| 294 | +Runner log chunks pass through `internal/runner/scrub` before they are |
| 295 | +posted to the API. It masks exact secret values and preserves enough |
| 296 | +tail bytes between chunks to catch a secret split across chunk |
| 297 | +boundaries. S41e follow-up work wires resolved workflow secrets into |
| 298 | +the runner/API mask set and adds server-side defense in depth before |
| 299 | +persisting chunks. |
| 283 | 300 | |
| 284 | 301 | ## `shithub.event` payload schema (v1) |
| 285 | 302 | |