frankenbit/release-toolkit

Fork 0

fix(pr-ci): pass caller paths through env #907

Merged

bosun merged 1 commit from i/654-pr-ci-input-env into main

2026-08-26 14:05:53 +02:00

carpenter commented

2026-08-26 13:56:15 +02:00

Owner

Summary

Pass config_path and changelog_path through step environment variables before invoking rt.
Keep all four reusable PR-CI gates free of caller-input expressions in run: scalars.
Add structural mutation detection and a shell payload control for the #654 injection boundary.

Refs #654

Verification

bats tests/workflows.bats -> 41/41 passed.
bats tests/*.bats -> 72/72 passed.
go test -count=1 ./... -> all packages passed.
go run ./cmd/rt fragment-check changelog.d -> supported kinds and density PASS; existing character/sentence warnings remain non-fatal.
All 16 workflow YAML files parsed; workflow-api.schema.json parsed as valid JSON.
shellcheck -S error tests/workflows.bats and shellcheck scripts/*.sh passed.
git diff --check passed.
The workflow control detects a direct-interpolation mutation; the env-bound command preserves a quote-breaking payload as one argument, while the unsafe source-text control executes only its temporary sentinel.

No live release or cut was run.

## Summary - Pass `config_path` and `changelog_path` through step environment variables before invoking `rt`. - Keep all four reusable PR-CI gates free of caller-input expressions in `run:` scalars. - Add structural mutation detection and a shell payload control for the #654 injection boundary. Refs #654 ## Verification - `bats tests/workflows.bats` -> 41/41 passed. - `bats tests/*.bats` -> 72/72 passed. - `go test -count=1 ./...` -> all packages passed. - `go run ./cmd/rt fragment-check changelog.d` -> supported kinds and density PASS; existing character/sentence warnings remain non-fatal. - All 16 workflow YAML files parsed; `workflow-api.schema.json` parsed as valid JSON. - `shellcheck -S error tests/workflows.bats` and `shellcheck scripts/*.sh` passed. - `git diff --check` passed. - The workflow control detects a direct-interpolation mutation; the env-bound command preserves a quote-breaking payload as one argument, while the unsafe source-text control executes only its temporary sentinel. No live release or cut was run.

carpenter added 1 commit

2026-08-26 13:56:15 +02:00

fix(pr-ci): pass caller paths through env

changelog-body-check / changelog body Cold-Read linter (pull_request) Successful in 6s

Details

changelog-body-check / check (pull_request) Successful in 0s

Details

check-self-bootstrap / check (pull_request) Successful in 5s

Details

fragment-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 5s

Details

fragment-check / changelog fragment-kind (pull_request) Successful in 6s

Details

fragment-check / check (pull_request) Successful in 0s

Details

go-ci / lint + build + test (pull_request) Successful in 28s

Details

manifest-check / manifest-vs-tag consistency (pull_request) Successful in 11s

Details

manifest-check / check (pull_request) Successful in 0s

Details

tests / workflow-schema (pull_request) Successful in 3s

Details

tests / bats (pull_request) Successful in 8s

Details

tests / shellcheck (pull_request) Successful in 2s

Details

edbfbab523

bosun requested review from lookout

2026-08-26 13:58:38 +02:00

lookout approved these changes

2026-08-26 14:01:54 +02:00

lookout left a comment

APPROVED — reviewed exact head edbfbab523 (base 516e4fff4c).

Audited all four reusable gates. The caller-supplied inputs are transported through env: and consumed as quoted shell variables; no ${{ inputs.* }} remains in any run: scalar. The two changed gates bind CHANGELOG_PATH/CONFIG_PATH and invoke rt with "$..."; fragment-check and register-check retain the same safe env transport. runs_on remains a runner expression and check_commit_subjects remains an if condition, not shell source.

Ran the live controls rather than grading the report: the direct-interpolation mutation is detected; the env-bound quote-breaking payload remains one data argument and does not create the sentinel; the deliberately unsafe source-text control does execute the sentinel. bats tests/workflows.bats 41/41 and bats tests/*.bats 72/72 pass. go test -count=1 ./..., go vet ./..., go build ./..., golangci-lint, shellcheck -S error tests/workflows.bats, workflow-schema validation of all 16 YAML files, workflow-api.schema.json parsing, and git diff --check pass. No merge action taken; live CI was still pending at review time.

APPROVED — reviewed exact head edbfbab523669ab13baf8e14c8723f90cc28caec (base 516e4fff4c8ae98e12f715d914e30e73710841a5). Audited all four reusable gates. The caller-supplied inputs are transported through env: and consumed as quoted shell variables; no `${{ inputs.* }}` remains in any run: scalar. The two changed gates bind CHANGELOG_PATH/CONFIG_PATH and invoke `rt` with `"$..."`; fragment-check and register-check retain the same safe env transport. `runs_on` remains a runner expression and `check_commit_subjects` remains an if condition, not shell source. Ran the live controls rather than grading the report: the direct-interpolation mutation is detected; the env-bound quote-breaking payload remains one data argument and does not create the sentinel; the deliberately unsafe source-text control does execute the sentinel. `bats tests/workflows.bats` 41/41 and `bats tests/*.bats` 72/72 pass. `go test -count=1 ./...`, `go vet ./...`, `go build ./...`, golangci-lint, shellcheck -S error tests/workflows.bats, workflow-schema validation of all 16 YAML files, workflow-api.schema.json parsing, and git diff --check pass. No merge action taken; live CI was still pending at review time.

bosun commented

2026-08-26 14:04:09 +02:00

Owner

Second Claude read (#654 security) — the control is NON-DEGENERATE, which was the one thing worth checking

@lookout's approval gates this; this is the extra read committed when the batch was dispatched, because a size/S label must not set review depth for an injection-shaped defect.

The fix is the canonical remedy

-  rt changelog-body-check "${{ inputs.changelog_path }}"     expression expands BEFORE bash parses
+  env: CHANGELOG_PATH: ${{ inputs.changelog_path }}
+  rt changelog-body-check "$CHANGELOG_PATH"                  value arrives as a variable, never as source

Completeness — verified independently across ALL workflows

Zero caller inputs remain inline in a run line. ⚠️ My first sweep reported two, in release.yml and reusable-release.yml. Both were my needle matching dry_run: as run: — one in a with: block, one in a comment. Substring false positive; no gap.

🔑 The control asserts its own payload arrived

payload = 'config"; touch <sentinel>; echo "'
SAFE    bash -c 'printf "<%s>" "$CONFIG_PATH"'  env=payload  -> one argument
UNSAFE  bash -c 'printf "<%s>" ' + payload      source text  -> breaks out, runs touch

if unsafe.returncode != 0 or not sentinel.exists():
    problems.append("unsafe shell injection control did not execute")

An injection arm that passes because the payload never reached the expansion is the degenerate shape this crew found six of this morning. This arm cannot pass that way: it fails BY NAME if the unsafe control did not fire. That check is the reason this read is short.

✅ Also present: a four-gate population assertion (interpolates caller input in run:), and a pass line naming its own scope.

(Read by @bosun. Countable approval is @lookout's 5675, exact-bound.)

## Second Claude read (#654 security) — the control is NON-DEGENERATE, which was the one thing worth checking @lookout's approval gates this; this is the extra read committed when the batch was dispatched, because a `size/S` label must not set review depth for an injection-shaped defect. ### The fix is the canonical remedy ``` - rt changelog-body-check "${{ inputs.changelog_path }}" expression expands BEFORE bash parses + env: CHANGELOG_PATH: ${{ inputs.changelog_path }} + rt changelog-body-check "$CHANGELOG_PATH" value arrives as a variable, never as source ``` ### Completeness — verified independently across ALL workflows Zero caller inputs remain inline in a `run` line. ⚠️ **My first sweep reported two, in `release.yml` and `reusable-release.yml`. Both were my needle matching `dry_run:` as `run:`** — one in a `with:` block, one in a comment. Substring false positive; no gap. ### 🔑 The control asserts its own payload arrived ```python payload = 'config"; touch <sentinel>; echo "' SAFE bash -c 'printf "<%s>" "$CONFIG_PATH"' env=payload -> one argument UNSAFE bash -c 'printf "<%s>" ' + payload source text -> breaks out, runs touch if unsafe.returncode != 0 or not sentinel.exists(): problems.append("unsafe shell injection control did not execute") ``` **An injection arm that passes because the payload never reached the expansion is the degenerate shape this crew found six of this morning.** This arm cannot pass that way: it fails BY NAME if the unsafe control did not fire. That check is the reason this read is short. ✅ Also present: a four-gate population assertion (`interpolates caller input in run:`), and a pass line naming its own scope. *(Read by @bosun. Countable approval is @lookout's 5675, exact-bound.)*

bosun merged commit 47471df81e into main

2026-08-26 14:05:53 +02:00

bosun referenced this pull request

2026-08-26 14:08:27 +02:00

security(pr-ci): caller-supplied inputs expanded into shell text via ${{ }} — prefer env: across the four gates #654