Skip to content

Commit b2ddb11

Browse files
committed
ci: run privileged workflows from main; pin actions; harden CI
> ⚠️ **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.) - 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.
1 parent cfd51a8 commit b2ddb11

6 files changed

Lines changed: 174 additions & 16 deletions

File tree

.github/CODEOWNERS

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,6 @@
1-
* @uber/submitqueue-admins @uber/submitqueue-maintainers @behinddwalls @sbalabanov
1+
* @uber/submitqueue-admins @uber/submitqueue-maintainers @behinddwalls @sbalabanov
2+
3+
# CI workflows, composite actions, and other automation are privileged
4+
# (secrets, write tokens). Require admin review for any change under .github/.
5+
# CODEOWNERS is last-match-wins, so this overrides the `*` rule for these paths.
6+
/.github/ @uber/submitqueue-admins

.github/actions/setup/action.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ runs:
1717
using: composite
1818
steps:
1919
- name: Setup Bazel cache
20-
uses: actions/cache@v4
20+
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
2121
with:
2222
path: |
2323
~/.cache/bazel-disk-cache

.github/workflows/ci.yml

Lines changed: 118 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@ on:
2020
permissions:
2121
contents: read
2222

23+
# Cancel superseded runs for the same PR to save runner minutes. Never cancel
24+
# in-progress runs for `push` (main) or `merge_group` — those must complete so
25+
# the default branch and merge queue always have a definitive result.
26+
concurrency:
27+
group: ci-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
28+
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
29+
2330
jobs:
2431
# ---------------------------------------------------------------------------
2532
# LINT (gofmt via Go SDK, yamlfmt via go run)
@@ -29,7 +36,11 @@ jobs:
2936
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
3037
runs-on: ubuntu-latest
3138
steps:
32-
- uses: actions/checkout@v4
39+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
40+
with:
41+
# This job executes untrusted PR code (make build/test/lint). Don't
42+
# leave the GITHUB_TOKEN in the workspace git config while it runs.
43+
persist-credentials: false
3344
- uses: ./.github/actions/setup
3445

3546
- name: Run linters
@@ -43,7 +54,11 @@ jobs:
4354
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
4455
runs-on: ubuntu-latest
4556
steps:
46-
- uses: actions/checkout@v4
57+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
58+
with:
59+
# This job executes untrusted PR code (make build/test/lint). Don't
60+
# leave the GITHUB_TOKEN in the workspace git config while it runs.
61+
persist-credentials: false
4762
- uses: ./.github/actions/setup
4863

4964
- name: Check module files are tidy
@@ -60,7 +75,11 @@ jobs:
6075
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
6176
runs-on: ubuntu-latest
6277
steps:
63-
- uses: actions/checkout@v4
78+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
79+
with:
80+
# This job executes untrusted PR code (make build/test/lint). Don't
81+
# leave the GITHUB_TOKEN in the workspace git config while it runs.
82+
persist-credentials: false
6483
- uses: ./.github/actions/setup
6584

6685
- name: Build project
@@ -77,7 +96,11 @@ jobs:
7796
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
7897
runs-on: ubuntu-latest
7998
steps:
80-
- uses: actions/checkout@v4
99+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
100+
with:
101+
# This job executes untrusted PR code (make build/test/lint). Don't
102+
# leave the GITHUB_TOKEN in the workspace git config while it runs.
103+
persist-credentials: false
81104
- uses: ./.github/actions/setup
82105

83106
- name: Run E2E tests
@@ -88,7 +111,11 @@ jobs:
88111
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
89112
runs-on: ubuntu-latest
90113
steps:
91-
- uses: actions/checkout@v4
114+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
115+
with:
116+
# This job executes untrusted PR code (make build/test/lint). Don't
117+
# leave the GITHUB_TOKEN in the workspace git config while it runs.
118+
persist-credentials: false
92119
- uses: ./.github/actions/setup
93120

94121
- name: Run Gateway integration tests
@@ -99,7 +126,11 @@ jobs:
99126
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
100127
runs-on: ubuntu-latest
101128
steps:
102-
- uses: actions/checkout@v4
129+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
130+
with:
131+
# This job executes untrusted PR code (make build/test/lint). Don't
132+
# leave the GITHUB_TOKEN in the workspace git config while it runs.
133+
persist-credentials: false
103134
- uses: ./.github/actions/setup
104135

105136
- name: Run Orchestrator integration tests
@@ -113,7 +144,11 @@ jobs:
113144
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
114145
runs-on: ubuntu-latest
115146
steps:
116-
- uses: actions/checkout@v4
147+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
148+
with:
149+
# This job executes untrusted PR code (make build/test/lint). Don't
150+
# leave the GITHUB_TOKEN in the workspace git config while it runs.
151+
persist-credentials: false
117152
- uses: ./.github/actions/setup
118153
- uses: ./.github/actions/run-bazel-test
119154
with:
@@ -124,7 +159,11 @@ jobs:
124159
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
125160
runs-on: ubuntu-latest
126161
steps:
127-
- uses: actions/checkout@v4
162+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
163+
with:
164+
# This job executes untrusted PR code (make build/test/lint). Don't
165+
# leave the GITHUB_TOKEN in the workspace git config while it runs.
166+
persist-credentials: false
128167
- uses: ./.github/actions/setup
129168
- uses: ./.github/actions/run-bazel-test
130169
with:
@@ -135,7 +174,11 @@ jobs:
135174
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
136175
runs-on: ubuntu-latest
137176
steps:
138-
- uses: actions/checkout@v4
177+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
178+
with:
179+
# This job executes untrusted PR code (make build/test/lint). Don't
180+
# leave the GITHUB_TOKEN in the workspace git config while it runs.
181+
persist-credentials: false
139182
- uses: ./.github/actions/setup
140183
- uses: ./.github/actions/run-bazel-test
141184
with:
@@ -149,12 +192,71 @@ jobs:
149192
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
150193
runs-on: ubuntu-latest
151194
steps:
152-
- uses: actions/checkout@v4
195+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
196+
with:
197+
# This job executes untrusted PR code (make build/test/lint). Don't
198+
# leave the GITHUB_TOKEN in the workspace git config while it runs.
199+
persist-credentials: false
153200
- uses: ./.github/actions/setup
154201
- uses: ./.github/actions/run-bazel-test
155202
with:
156203
target: //test/integration/submitqueue/core/consumer/...
157204

205+
# ---------------------------------------------------------------------------
206+
# WORKFLOW SECURITY LINT
207+
#
208+
# Guards against regressions in the workflows themselves: actionlint checks
209+
# general validity; zizmor audits for GitHub Actions security smells
210+
# (dangerous triggers, unpinned `uses:`, credential persistence, template
211+
# injection). Keeps the pull_request_target / SHA-pinning hardening from
212+
# silently eroding in future edits.
213+
# ---------------------------------------------------------------------------
214+
workflow-security:
215+
name: Workflow Security Lint
216+
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
217+
runs-on: ubuntu-latest
218+
steps:
219+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
220+
with:
221+
persist-credentials: false
222+
- name: actionlint
223+
uses: raven-actions/actionlint@205b530c5d9fa8f44ae9ed59f341a0db994aa6f8 # v2.1.2
224+
- name: zizmor
225+
uses: zizmorcore/zizmor-action@5f14fd08f7cf1cb1609c1e344975f152c7ee938d # v0.5.6
226+
with:
227+
# Pin the zizmor TOOL version (distinct from the action tag above) for
228+
# reproducible audits matching local validation.
229+
version: "1.25.2"
230+
# Fail the job on findings without requiring GitHub Advanced Security
231+
# / SARIF upload (which needs security-events: write and degrades on
232+
# fork PRs). Keeps the gate self-contained.
233+
advanced-security: false
234+
235+
# ci.yml and the composite actions it calls run untrusted PR code under
236+
# the `pull_request` trigger. A repository secret referenced anywhere on
237+
# that path is reachable by a malicious PR and can be exfiltrated on the
238+
# PR run (before any review), so this path must stay secret-free — route
239+
# any secret-bearing step through a SEPARATE trusted workflow
240+
# (workflow_run / pull_request_target) that does not execute PR code.
241+
#
242+
# This guard catches ACCIDENTAL reintroduction by honest contributors; a
243+
# malicious actor controlling ci.yml could delete the guard itself, so the
244+
# real defenses remain (a) secrets scoped to a main-only Environment so a
245+
# PR-triggered job cannot obtain them, and (b) CODEOWNERS review on
246+
# .github/. GITHUB_TOKEN (least-privilege, read-only here) is allowlisted.
247+
- name: Guard — no repository secrets on the untrusted-code path
248+
run: |
249+
hits="$(grep -rnE '\$\{\{[^}]*secrets\.' \
250+
.github/workflows/ci.yml .github/actions \
251+
| grep -vE 'secrets\.GITHUB_TOKEN' || true)"
252+
if [ -n "$hits" ]; then
253+
echo "::error::Repository secret referenced on the untrusted-code CI path (ci.yml / composite actions):" >&2
254+
echo "$hits" >&2
255+
echo "Move secret-bearing steps to a separate trusted workflow that does not run PR code." >&2
256+
exit 1
257+
fi
258+
echo "OK: no repository secrets referenced in ci.yml or its composite actions."
259+
158260
# ---------------------------------------------------------------------------
159261
# REQUIRED CHECKS GATE
160262
#
@@ -179,12 +281,17 @@ jobs:
179281
- queue-integration-test
180282
- storage-integration-test
181283
- consumer-integration-test
284+
- workflow-security
182285
steps:
183286
- name: Fail if any required check did not succeed
184287
if: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }}
288+
# Pass the job-results context via env (not inline ${{ }} in the script)
289+
# so there is no template expansion inside the run block.
290+
env:
291+
NEEDS_JSON: ${{ toJSON(needs) }}
185292
run: |
186293
echo "One or more required checks did not succeed:" >&2
187-
echo '${{ toJSON(needs) }}' >&2
294+
echo "$NEEDS_JSON" >&2
188295
exit 1
189296
190297
- name: All required checks passed

.github/workflows/rebase-stack.yml

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,26 @@
3737

3838
name: Rebase Stacked PRs
3939

40+
# SECURITY: this workflow is triggered by `pull_request_target`, NOT
41+
# `pull_request`. The distinction is critical because this job holds a
42+
# privileged, repo-write PAT (STACK_REBASE_TOKEN).
43+
#
44+
# - `pull_request` runs the workflow file from the PR's HEAD ref.
45+
# A contributor could edit THIS file in their own PR
46+
# (e.g. drop the `merged` guard), close the PR, and
47+
# run arbitrary code with the PAT. Same-repo branch
48+
# PRs (the stacked-PR model) DO receive secrets, so
49+
# this is exploitable — a secret-exfiltration hole.
50+
# - `pull_request_target` always runs the workflow file from the DEFAULT
51+
# branch (main). PR edits to this file are ignored,
52+
# so the privileged logic is fixed at main's version.
53+
#
54+
# `pull_request_target` is safe HERE specifically because the job never builds
55+
# or executes PR code: it only replays commits with `git rebase` (which runs
56+
# nothing from the tree) and validates the resulting diff is byte-identical
57+
# before force-pushing. It does not run `make`, tests, or any PR script.
4058
on:
41-
pull_request:
59+
pull_request_target:
4260
types:
4361
- closed
4462

@@ -52,8 +70,11 @@ jobs:
5270
name: Rebase Stack
5371
if: github.event.pull_request.merged == true
5472
runs-on: ubuntu-latest
73+
# Scope the privileged PAT to a protected Environment restricted to `main`
74+
# so the secret cannot be used from any other ref/context.
75+
environment: stack-rebase
5576
steps:
56-
- uses: actions/checkout@v4
77+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
5778
with:
5879
# Fetch full history so rebase --onto works correctly.
5980
fetch-depth: 0

.github/workflows/tag-go-pseudoversion.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ jobs:
2424
name: Create and push pseudo-version tag
2525
runs-on: ubuntu-latest
2626
steps:
27-
- uses: actions/checkout@v4
27+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
2828
with:
2929
fetch-depth: 0
3030

.github/zizmor.yml

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# zizmor (GitHub Actions security auditor) configuration.
2+
#
3+
# Every ignore below is an INTENTIONAL, reviewed exception with a concrete
4+
# justification — not a blanket mute. New findings outside these are expected
5+
# to fail the `workflow-security` CI job.
6+
rules:
7+
# rebase-stack.yml deliberately uses `pull_request_target` so the privileged
8+
# workflow definition (which holds the STACK_REBASE_TOKEN PAT) is always taken
9+
# from `main`, never from PR-controlled content. It never builds or runs PR
10+
# code — only `git rebase` replay + byte-identical diff validation before
11+
# force-push — so the usual pull_request_target code-execution risk does not
12+
# apply. See the SECURITY comment block at the top of that workflow.
13+
dangerous-triggers:
14+
ignore:
15+
- rebase-stack.yml
16+
# These workflows MUST persist the checkout credentials because they push:
17+
# - rebase-stack.yml force-pushes rebased branches using STACK_REBASE_TOKEN
18+
# - tag-go-pseudoversion.yml pushes pseudo-version tags using the default token
19+
# Neither uploads build artifacts, so persisted credentials cannot leak via
20+
# the artipacked vector. CI jobs that DO run untrusted PR code set
21+
# persist-credentials: false in ci.yml.
22+
artipacked:
23+
ignore:
24+
- rebase-stack.yml
25+
- tag-go-pseudoversion.yml

0 commit comments

Comments
 (0)