ci: run privileged workflows from main; pin actions; harden CI#203
Merged
Conversation
>⚠️ **P0 — required before merge.** `STACK_REBASE_TOKEN` is currently a plain > repository secret, so any same-repo PR can exfiltrate it today by referencing > `${{ secrets.STACK_REBASE_TOKEN }}` in a modified/added workflow — it runs on > the PR, *before any review*, and log-masking is trivially bypassed. The > `pull_request_target` change here does **not** close that (an attacker uses a > different workflow to read the ambient secret). The actual fix is moving the > PAT into a `main`-only Environment so PR-triggered jobs cannot obtain it — see > item **#1** in "Required manual follow-up". Treat that as blocking. ## Summary ### Why? A review of `.github/workflows/` found one privilege-escalation hole and several supply-chain / least-privilege gaps. GitHub runs a `pull_request` workflow from the PR's HEAD ref, so a PR can modify the workflow that runs on it. `rebase-stack.yml` — which holds a repo-write PAT (STACK_REBASE_TOKEN) and triggers on `pull_request: closed` — was therefore exploitable: a contributor could rewrite the file in their own PR (dropping the `merged` guard), close it, and run arbitrary code with the PAT. Same-repo branch PRs (the stacked-PR model) DO receive secrets, so this was reachable by any contributor with push access or a compromised account. Privileged automation must run its definition from `main`, never from PR-controlled content. CI test jobs must still execute PR code — that is the purpose of a test pipeline — so the goal is to lock down *privileged* execution and *supply-chain* references, not to stop PR testing. The defense rests on an invariant: the untrusted-code path (`ci.yml` + its composite actions) carries no secrets and a read-only token, so there is nothing for a malicious PR to steal. ### What? - rebase-stack.yml: trigger `pull_request` -> `pull_request_target`, so GitHub always runs main's version of the file regardless of PR content. Safe here because the job never builds/runs PR code — it only replays commits via `git rebase` and validates the diff is byte-identical before force-pushing. Adds `environment: stack-rebase` to scope the PAT to a protected, main-only environment. - ci.yml: `persist-credentials: false` on every checkout (jobs execute untrusted PR code); PR-only `concurrency` cancellation (never cancels push/merge_group); new `workflow-security` job (actionlint + zizmor) wired into the required-checks gate; the gate's debug echo now passes `toJSON(needs)` via env to avoid in-script template expansion. - workflow-security: added a guard step that fails CI if `ci.yml` or its composite actions reference a repository secret (GITHUB_TOKEN allowlisted). This keeps the untrusted-code path secret-free, catching accidental reintroduction of a reachable secret. (It catches honest mistakes; the malicious case is covered by environment-scoped secrets + CODEOWNERS review.) - run-bazel-test composite action: pass `inputs.target` via env instead of interpolating `${{ }}` into the shell, closing a template-injection. - All third-party actions pinned to immutable commit SHAs (checkout, cache, actionlint, zizmor) so a moved tag cannot inject unapproved code. - .github/zizmor.yml: documents the intentional, reviewed exceptions (dangerous-triggers on the deliberate pull_request_target; artipacked on the two workflows that must persist credentials to push). - CODEOWNERS: require admin review for any change under `.github/`. ## Test Plan - ✅ `actionlint .github/workflows/*.yml` — no issues. - ✅ `zizmor --persona=regular .github/workflows/` (offline + online) — "No findings to report" (3 intentional ignores, documented in zizmor.yml). - ✅ Secrets guard: passes on the current secret-free path, and fires correctly when a `${{ secrets.* }}` reference is introduced. - ✅ All workflow/action YAML parses; no unpinned action refs remain. - Post-merge verification: open a PR that edits rebase-stack.yml (add an `echo PWNED` step, drop the `merged` guard) and CLOSE it without merging — confirm GitHub runs main's version (no PWNED, job skipped). Then merge a 2-PR stack and confirm children are still rebased/retargeted as before. ## Required manual follow-up (repo / org settings) These cannot be expressed in files and must be applied by a maintainer. - [ ] **🔴 P0 (blocking): Create the `stack-rebase` Environment**, restrict its deployment branches to `main`, and move `STACK_REBASE_TOKEN` into it so the PAT is unreachable from any PR-triggered job. Until this is done the PAT is exfiltratable today (see the callout above), and `rebase-stack.yml` (which now declares `environment: stack-rebase`) will not run. - [ ] **Settings → Actions → General → Fork pull request workflows from outside collaborators**: "Require approval for all external contributors" so PR code never auto-executes for new contributors. - [ ] **Settings → Actions → General → Workflow permissions**: default `GITHUB_TOKEN` to read-only; uncheck "Allow GitHub Actions to create and approve pull requests" unless required. - [ ] **Branch protection / ruleset on `main`**: require PR review, require review from Code Owners (so `/.github/` changes need admin approval), require the `Required Checks` status, block force-pushes. - [ ] **(Optional)** Restrict allowed actions to selected/verified creators with pinned SHAs.
mnoah1
approved these changes
Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why?
A review of
.github/workflows/found one privilege-escalation hole andseveral supply-chain / least-privilege gaps. GitHub runs a
pull_requestworkflow from the PR's HEAD ref, so a PR can modify the workflow that runs
on it.
rebase-stack.yml— which holds a repo-write PAT (STACK_REBASE_TOKEN)and triggers on
pull_request: closed— was therefore exploitable: acontributor could rewrite the file in their own PR (dropping the
mergedguard), close it, and run arbitrary code with the PAT. Same-repo branch PRs
(the stacked-PR model) DO receive secrets, so this was reachable by any
contributor with push access or a compromised account. Privileged automation
must run its definition from
main, never from PR-controlled content.CI test jobs must still execute PR code — that is the purpose of a test
pipeline — so the goal is to lock down privileged execution and
supply-chain references, not to stop PR testing. The defense rests on an
invariant: the untrusted-code path (
ci.yml+ its composite actions) carriesno secrets and a read-only token, so there is nothing for a malicious PR to
steal.
What?
pull_request->pull_request_target, soGitHub always runs main's version of the file regardless of PR content.
Safe here because the job never builds/runs PR code — it only replays
commits via
git rebaseand validates the diff is byte-identical beforeforce-pushing. Adds
environment: stack-rebaseto scope the PAT to aprotected, main-only environment.
persist-credentials: falseon every checkout (jobs executeuntrusted PR code); PR-only
concurrencycancellation (never cancelspush/merge_group); new
workflow-securityjob (actionlint + zizmor) wiredinto the required-checks gate; the gate's debug echo now passes
toJSON(needs)via env to avoid in-script template expansion.ci.ymlor itscomposite actions reference a repository secret (GITHUB_TOKEN allowlisted).
This keeps the untrusted-code path secret-free, catching accidental
reintroduction of a reachable secret. (It catches honest mistakes; the
malicious case is covered by environment-scoped secrets + CODEOWNERS review.)
inputs.targetvia env instead ofinterpolating
${{ }}into the shell, closing a template-injection.actionlint, zizmor) so a moved tag cannot inject unapproved code.
(dangerous-triggers on the deliberate pull_request_target; artipacked on
the two workflows that must persist credentials to push).
.github/.Test Plan
actionlint .github/workflows/*.yml— no issues.zizmor --persona=regular .github/workflows/(offline + online) —"No findings to report" (3 intentional ignores, documented in zizmor.yml).
correctly when a
${{ secrets.* }}reference is introduced.echo PWNEDstep, drop themergedguard) and CLOSE it without merging —confirm GitHub runs main's version (no PWNED, job skipped). Then merge a
2-PR stack and confirm children are still rebased/retargeted as before.
Required manual follow-up (repo / org settings)
These cannot be expressed in files and must be applied by a maintainer.
stack-rebaseEnvironment, restrict itsdeployment branches to
main, and moveSTACK_REBASE_TOKENinto it sothe PAT is unreachable from any PR-triggered job. Until this is done the
PAT is exfiltratable today (see the callout above), and
rebase-stack.yml(which now declares
environment: stack-rebase) will not run.outside collaborators: "Require approval for all external
contributors" so PR code never auto-executes for new contributors.
GITHUB_TOKENto read-only; uncheck "Allow GitHub Actions to create andapprove pull requests" unless required.
main: require PR review, requirereview from Code Owners (so
/.github/changes need admin approval),require the
Required Checksstatus, block force-pushes.with pinned SHAs.