Skip to content

ci: run privileged workflows from main; pin actions; harden CI#203

Merged
behinddwalls merged 1 commit into
mainfrom
fix-ci
Jun 5, 2026
Merged

ci: run privileged workflows from main; pin actions; harden CI#203
behinddwalls merged 1 commit into
mainfrom
fix-ci

Conversation

@behinddwalls
Copy link
Copy Markdown
Collaborator

@behinddwalls behinddwalls commented Jun 5, 2026

⚠️ 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.

@behinddwalls behinddwalls marked this pull request as ready for review June 5, 2026 02:57
@behinddwalls behinddwalls requested review from a team and sbalabanov as code owners June 5, 2026 02:57
> ⚠️ **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.
@behinddwalls behinddwalls merged commit 4d65022 into main Jun 5, 2026
15 checks passed
@behinddwalls behinddwalls deleted the fix-ci branch June 5, 2026 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants