Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,27 @@ and eval jobs retain their canonical settings. Adding or removing the modifier
restarts the active sweep. Fast-mode runs are not eligible for artifact reuse
after merge.

## Trusted External-Fork Sweep Dispatch (PoC)

Public-fork `pull_request` workflows receive no repository secrets. For an
external PR, the ordinary `run-sweep.yml` run therefore validates the
changelog but does not fan out onto GPU runners. A maintainer with `write`,
`maintain`, or `admin` permission can add any modifier labels first, then apply
one primary sweep label to approve the PR's exact current head SHA.
`trusted-external-sweep.yml` then dispatches `e2e-tests.yml` from `main`, pins
both the approved head and GitHub's merge SHA, and runs the generated matrix
with the trusted workflow's secrets.

The approval is revision-specific. A later push is not trusted automatically;
remove and re-add the primary sweep label to approve the new SHA. The trusted
dispatcher never checks out or executes PR code itself.

This proof of concept produces benchmark and evaluation artifacts through the
End-to-End Tests workflow. Those runs are not yet eligible for
`/reuse-sweep-run`, which currently accepts only `run-sweep.yml` runs. The PoC
also fans out the selected matrix immediately; it does not reproduce
`run-sweep.yml`'s canary-first sequencing.

## Reusing an Approved PR Full Sweep

`[skip-sweep]` skips PR benchmark setup only; changelog and reuse checks still
Expand Down
156 changes: 135 additions & 21 deletions .github/workflows/e2e-tests.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
name: End-to-End Tests
run-name: e2e Test - ${{ inputs.test-name || inputs.generate-cli-command || github.event.inputs.generate-cli-command }}

permissions:
contents: read

on:
workflow_dispatch:
inputs:
generate-cli-command:
description: "Command passed to generate matrix script"
required: true
required: false
type: string
default: ""
test-name:
description: "Name for this test run"
required: false
Expand Down Expand Up @@ -36,12 +40,48 @@
required: false
type: string
default: ""
changelog-base-ref:
description: "Trusted-dispatch mode: base SHA for process_changelog.py"
required: false
type: string
default: ""
changelog-head-ref:
description: "Trusted-dispatch mode: exact approved PR head SHA"
required: false
type: string
default: ""
trim-conc:
description: "Trim each configuration to its minimum concurrency"
required: false
type: boolean
default: false
all-evals:
description: "Expand evaluation selection to every fixed-sequence configuration"
required: false
type: boolean
default: false
evals-only:
description: "Suppress throughput and run only evaluations"
required: false
type: boolean
default: false
fail-fast:
description: "Cancel the rest of each matrix after its first failure"
required: false
type: boolean
default: false
pr-labels-json:
description: "Labels from the source PR for priority scoring"
required: false
type: string
default: "[]"
Comment on lines +43 to +77

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 BLOCKING: The workflow_dispatch trigger now defines 14 inputs, but GitHub enforces a hard maximum of 10 inputs per workflow_dispatch event.

Why it matters: Dispatching this workflow fails with you may only define up to 10 'inputs' for a 'workflow_dispatch' event — which breaks the new trusted-external-sweep dispatcher, the existing claude.yml e2e automation, and manual runs from the Actions UI. Since this PR is already merged, e2e-tests.yml on main is likely un-dispatchable right now; a one-off manual dispatch will confirm.

Fix: The four new booleans are all derivable from pr-labels-json (that's exactly how trusted-external-sweep.yml computes them before dispatching): drop trim-conc, all-evals, evals-only, and fail-fast from the workflow_dispatch inputs, derive them inside get-jobs from the labels JSON (strategy blocks can read needs.get-jobs.outputs.*), and stop passing them from the dispatcher. That brings the count to 10. The workflow_call block has no 10-input limit and can keep them.

Fix this →

workflow_call:
inputs:
generate-cli-command:
description: "Command passed to generate matrix script"
required: true
required: false
type: string
default: ""
test-name:
description: "Name for this test run"
required: false
Expand Down Expand Up @@ -70,6 +110,41 @@
required: false
type: string
default: ""
changelog-base-ref:
description: "Trusted-dispatch mode: base SHA for process_changelog.py"
required: false
type: string
default: ""
changelog-head-ref:
description: "Trusted-dispatch mode: exact approved PR head SHA"
required: false
type: string
default: ""
trim-conc:
description: "Trim each configuration to its minimum concurrency"
required: false
type: boolean
default: false
all-evals:
description: "Expand evaluation selection to every fixed-sequence configuration"
required: false
type: boolean
default: false
evals-only:
description: "Suppress throughput and run only evaluations"
required: false
type: boolean
default: false
fail-fast:
description: "Cancel the rest of each matrix after its first failure"
required: false
type: boolean
default: false
pr-labels-json:
description: "Labels from the source PR for priority scoring"
required: false
type: string
default: "[]"

jobs:
get-jobs:
Expand All @@ -88,6 +163,8 @@
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
ref: ${{ inputs.ref }}
fetch-depth: 0
persist-credentials: false

- name: Checkout code (default)
if: ${{ !inputs.ref || inputs.ref == '' }}
Expand All @@ -105,15 +182,50 @@
- uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0
- id: get-jobs
env:
PR_LABELS: ${{ toJson(github.event.pull_request.labels.*.name) }}
PR_LABELS: ${{ inputs.pr-labels-json || toJson(github.event.pull_request.labels.*.name) }}
CHANGELOG_BASE_REF: ${{ inputs.changelog-base-ref }}
CHANGELOG_HEAD_REF: ${{ inputs.changelog-head-ref }}
TRIM_CONC: ${{ inputs.trim-conc }}
ALL_EVALS: ${{ inputs.all-evals }}
EVALS_ONLY: ${{ inputs.evals-only }}
run: |
CONFIG_JSON=$(uv run --no-project --with pydantic --with pyyaml --python 3.12 \
${GITHUB_WORKSPACE}/utils/matrix_logic/generate_sweep_configs.py \
${{ inputs.generate-cli-command || github.event.inputs.generate-cli-command }})
PRIORITY_ROOT="${GITHUB_WORKSPACE}"
if [ -d "${GITHUB_WORKSPACE}/.ci-priority" ]; then
PRIORITY_ROOT="${GITHUB_WORKSPACE}/.ci-priority"
fi
if [ -n "$CHANGELOG_BASE_REF" ] || [ -n "$CHANGELOG_HEAD_REF" ]; then
if [ -z "$CHANGELOG_BASE_REF" ] || [ -z "$CHANGELOG_HEAD_REF" ]; then
echo "Both changelog-base-ref and changelog-head-ref are required" >&2
exit 1
fi
CMD=(
uv run --no-project --with pydantic --with pyyaml --python 3.12
"${PRIORITY_ROOT}/utils/process_changelog.py"
--changelog-file "${GITHUB_WORKSPACE}/perf-changelog.yaml"
--base-ref "$CHANGELOG_BASE_REF"
--head-ref "$CHANGELOG_HEAD_REF"
)
if [ "$TRIM_CONC" = "true" ]; then
CMD+=(--trim-conc)
fi
if [ "$ALL_EVALS" = "true" ]; then
CMD+=(--all-evals)
fi
if [ "$EVALS_ONLY" = "true" ]; then
CMD+=(--evals-only)
fi
RAW_CONFIG_JSON=$("${CMD[@]}")
CONFIG_JSON=$(python3 -c 'import json,sys; data=json.load(sys.stdin); rows=[row for family in ("single_node","multi_node") for group in data.get(family,{}).values() for row in group]; rows.extend(row for family in ("evals","agentic_evals","multinode_evals") for row in data.get(family,[])); print(json.dumps(rows))' <<<"$RAW_CONFIG_JSON")
else
GENERATE_COMMAND="${{ inputs.generate-cli-command || github.event.inputs.generate-cli-command }}"
if [ -z "$GENERATE_COMMAND" ]; then
echo "generate-cli-command is required outside trusted changelog dispatch mode" >&2
exit 1
fi

Check failure on line 224 in .github/workflows/e2e-tests.yml

View check run for this annotation

Claude / Claude Code Review

Trusted-dispatch changelog path executes untrusted PR's generate_sweep_configs.py

The trusted-dispatch `get-jobs` step invokes the pinned `.ci-priority/utils/process_changelog.py`, but that script's internal `subprocess.run`/config-load calls use the repo-relative paths `utils/matrix_logic/generate_sweep_configs.py` and `configs/{amd,nvidia}-master.yaml` (from `utils/constants.py`) with no `cwd=` override, so they resolve against the step's actual working directory — the untrusted external-fork checkout at `GITHUB_WORKSPACE` — rather than the trusted `.ci-priority` copy. This
Comment on lines +196 to +224

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The trusted-dispatch get-jobs step invokes the pinned .ci-priority/utils/process_changelog.py, but that script's internal subprocess.run/config-load calls use the repo-relative paths utils/matrix_logic/generate_sweep_configs.py and configs/{amd,nvidia}-master.yaml (from utils/constants.py) with no cwd= override, so they resolve against the step's actual working directory — the untrusted external-fork checkout at GITHUB_WORKSPACE — rather than the trusted .ci-priority copy. This lets an external contributor supply their own generate_sweep_configs.py and/or a doctored configs/*-master.yaml in their fork, defeating the config validation and matrix generation that the .ci-priority isolation was built to protect, with the resulting matrix flowing into downstream secrets: inherit benchmark jobs. Fix by passing cwd=PRIORITY_ROOT (or absolutizing GENERATE_SWEEPS_PY_SCRIPT/MASTER_CONFIGS) in utils/process_changelog.py, or by exporting PRIORITY_ROOT for the subprocess to use.

Extended reasoning...

The mechanism, confirmed against the actual source. utils/process_changelog.py (unmodified by this PR) imports GENERATE_SWEEPS_PY_SCRIPT = "utils/matrix_logic/generate_sweep_configs.py" and MASTER_CONFIGS = ["configs/amd-master.yaml", "configs/nvidia-master.yaml"] from utils/constants.py" — both repo-relative. Line 173 calls master_config = load_config_files(MASTER_CONFIGS)to validate that every changelog entry'sconfig-keysexist in the master config (raisingValueErrorotherwise, lines 104-119). Lines 211-233 and 248-274 then callsubprocess.run(["python3", GENERATE_SWEEPS_PY_SCRIPT, "test-config", ..., "--config-files", MASTER_CONFIGS, ...])with nocwd=argument, so both the script path and the config-file arguments resolve against whatever directory the parent process is running in.\n\nIne2e-tests.yml's get-jobsstep, that directory isGITHUB_WORKSPACE. In trusted-dispatch mode the 'Checkout code (ref)' step checks out inputs.ref (pull.merge_commit_sha, the *untrusted* external-fork merge revision) directly into GITHUB_WORKSPACEwith nopath:override, while the trusted tooling is separately checked out under.ci-priority(pinned togithub.workflow_sha). PRIORITY_ROOTis correctly set to.ci-priority, so process_changelog.pyitself — and its own top-level imports, which resolve viasys.path[0] = the script's own directory — do run the trusted copy. But the *relative* paths inside it (GENERATE_SWEEPS_PY_SCRIPT, MASTER_CONFIGS) are resolved by Python/the OS against the process cwd, which is GITHUB_WORKSPACE— the untrusted checkout — notPRIORITY_ROOT.\n\n**Why this defeats the isolation, not just 'runs PR code.'** trusted-external-sweep.yml always dispatches e2e-tests.ymlwithchangelog-base-ref/changelog-head-refset, so every approved external-fork run takes this branch unconditionally. Two independent attacks follow from the same root cause: (1) the external contributor can put arbitrary Python in their ownutils/matrix_logic/generate_sweep_configs.py, which then executes inside the trusted, secrets-context e2e-tests.ymlrun; and (2), even without any code execution, the contributor can simply edit their ownconfigs/amd-master.yaml/configs/nvidia-master.yamlto add a fabricated config entry (e.g. with an attacker-controlled containerimage) and reference that key from perf-changelog.yaml. Because load_config_files(MASTER_CONFIGS)reads the *fork's* master-config files, the fabricated key passes the 'must exist in master configs' check that.ci-priority's trusted process_changelog.pyis supposed to enforce, andgenerate_sweep_configs.py(also resolved from the fork) will happily emit a matrix row carrying that entry. That row is exactly the JSON thatget-jobsoutputs and that thetest-sweep-jobs consume withsecrets: inherit.\n\n**Why nothing else in the workflow catches this.** The maintainer's trust decision, per the PR description, is scoped to 'the approved merge revision' being executed by the benchmark jobs — but the entire reason process_changelog.pyandci_priority.pyare routed through.ci-priorityin the first place is to keep the *matrix-generation and priority-scoring control plane* on trusted code so an external label-approval doesn't also hand the contributor free rein over what configuration/image ends up in the secrets-bearing matrix. That is precisely the protection this bug removes for two of.ci-priority's three components (generate_sweep_configs.pyand the master configs;ci_priority.pyalone remains safely pinned since it's invoked with an explicit"/utils/ci_priority.py"path and--policy "/configs/ci-priority.yaml"). It is true that the get-jobs step itself has no secrets in its own env (persist-credentials: false, workflow-level permissions: contents: read), so this is not a direct secrets-exfiltration primitive from inside get-jobs— but that's not the threat this isolation exists to prevent. The threat is exactly what this bug enables: attacker-influenced data/code reaching the config-generation step that feedssecrets: inheritGPU jobs downstream.\n\n**Step-by-step PoC.** (1) An external contributor forks the repo and, in their PR branch, adds a new keyevil-keytoconfigs/nvidia-master.yamlwhoseimagefield points at a Docker image they control (crafted to exfiltrate the runner's inherited secrets or otherwise misbehave). (2) They add a corresponding entry inperf-changelog.yamlreferencingconfig-keys: [evil-key]. (3) A maintainer, believing they are approving only the exact reviewed source diff, applies full-sweep-fail-fast(one of the primary sweep labels) to the PR. (4)trusted-external-sweep.ymlvalidates the maintainer's permission and the head SHA, then dispatchese2e-tests.ymlonmainwithref=pull.merge_commit_sha, changelog-base-ref, changelog-head-ref. (5) In get-jobs, GITHUB_WORKSPACEis checked out to the merge commit (containing the attacker'sevil-keyentry and doctored master config);.ci-priorityholds the trustedprocess_changelog.py. (6) process_changelog.pyruns from.ci-prioritybut callsload_config_files(MASTER_CONFIGS)andsubprocess.run(["python3", GENERATE_SWEEPS_PY_SCRIPT, ...])with cwd defaulting toGITHUB_WORKSPACE— the attacker's checkout — soevil-keyis found 'valid' and a matrix row containing the attacker's image is generated. (7) That row is emitted viaGITHUB_OUTPUT, consumed by e.g. test-sweep-single-node, which runs benchmark-tmpl.ymlwithsecrets: inheritand the attacker-chosenimage. The maintainer never intended to approve a new config entry or image — only the reviewed diff — yet it reaches a secrets-bearing job.\n\n**Fix.** Pass cwd=os.environ["PRIORITY_ROOT"](or an equivalent absolute base) to bothsubprocess.runcalls inutils/process_changelog.py, and resolve MASTER_CONFIGS/GENERATE_SWEEPS_PY_SCRIPTrelative to that same trusted root before callingload_config_files/subprocess.run`, rather than relying on the ambient process cwd.

CONFIG_JSON=$(uv run --no-project --with pydantic --with pyyaml --python 3.12 \
"${GITHUB_WORKSPACE}/utils/matrix_logic/generate_sweep_configs.py" \
$GENERATE_COMMAND)
fi
score_matrix() {
local family="$1"
uv run --no-project --with pyyaml --python 3.12 "${PRIORITY_ROOT}/utils/ci_priority.py" \
Expand All @@ -129,21 +241,23 @@
MULTI=$(echo "$CONFIG_JSON" | python3 -c "import sys,json; d=json.load(sys.stdin); print(json.dumps([x for x in d if 'prefill' in x and x.get('scenario-type') != 'agentic-coding' and not x.get('eval-only', False)]))" | score_matrix multi)
EVALS=$(echo "$CONFIG_JSON" | python3 -c "import sys,json; d=json.load(sys.stdin); print(json.dumps([x for x in d if 'prefill' not in x and x.get('scenario-type') != 'agentic-coding' and x.get('run-eval', False)]))" | score_matrix eval)
MULTI_EVAL=$(echo "$CONFIG_JSON" | python3 -c "import sys,json; d=json.load(sys.stdin); print(json.dumps([x for x in d if 'prefill' in x and x.get('run-eval', False)]))" | score_matrix multi-eval)
echo "agentic-config=$AGENTIC" >> $GITHUB_OUTPUT
echo "agentic-eval-config=$AGENTIC_EVAL" >> $GITHUB_OUTPUT
echo "multi-node-agentic-config=$MULTI_AGENTIC" >> $GITHUB_OUTPUT
echo "single-node-config=$SINGLE" >> $GITHUB_OUTPUT
echo "multi-node-config=$MULTI" >> $GITHUB_OUTPUT
echo "eval-config=$EVALS" >> $GITHUB_OUTPUT
echo "multi-node-eval-config=$MULTI_EVAL" >> $GITHUB_OUTPUT
{
echo "agentic-config=$AGENTIC"
echo "agentic-eval-config=$AGENTIC_EVAL"
echo "multi-node-agentic-config=$MULTI_AGENTIC"
echo "single-node-config=$SINGLE"
echo "multi-node-config=$MULTI"
echo "eval-config=$EVALS"
echo "multi-node-eval-config=$MULTI_EVAL"
} >> "$GITHUB_OUTPUT"

test-sweep-multi-node:
needs: get-jobs
if: ${{ needs.get-jobs.outputs.multi-node-config != '[]' }}
uses: ./.github/workflows/benchmark-multinode-tmpl.yml
name: multi-node /
strategy:
fail-fast: false
fail-fast: ${{ inputs.fail-fast }}
matrix:
config: ${{ fromJson(needs.get-jobs.outputs.multi-node-config) }}
secrets: inherit
Expand Down Expand Up @@ -194,7 +308,7 @@
uses: ./.github/workflows/benchmark-multinode-tmpl.yml
name: multi-node eval /
strategy:
fail-fast: false
fail-fast: ${{ inputs.fail-fast }}
matrix:
config: ${{ fromJson(needs.get-jobs.outputs.multi-node-eval-config) }}
secrets: inherit
Expand Down Expand Up @@ -247,7 +361,7 @@
uses: ./.github/workflows/benchmark-tmpl.yml
name: agentic /
strategy:
fail-fast: false
fail-fast: ${{ inputs.fail-fast }}
matrix:
config: ${{ fromJson(needs.get-jobs.outputs.agentic-config) }}
secrets: inherit
Expand Down Expand Up @@ -291,7 +405,7 @@
uses: ./.github/workflows/benchmark-tmpl.yml
name: agentic eval /
strategy:
fail-fast: false
fail-fast: ${{ inputs.fail-fast }}
matrix:
config: ${{ fromJson(needs.get-jobs.outputs.agentic-eval-config) }}
secrets: inherit
Expand Down Expand Up @@ -332,7 +446,7 @@
uses: ./.github/workflows/benchmark-multinode-tmpl.yml
name: multi-node agentic /
strategy:
fail-fast: false
fail-fast: ${{ inputs.fail-fast }}
matrix:
config: ${{ fromJson(needs.get-jobs.outputs.multi-node-agentic-config) }}
secrets: inherit
Expand Down Expand Up @@ -388,7 +502,7 @@
uses: ./.github/workflows/benchmark-tmpl.yml
name: single-node /
strategy:
fail-fast: false
fail-fast: ${{ inputs.fail-fast }}
matrix:
config: ${{ fromJson(needs.get-jobs.outputs.single-node-config) }}
secrets: inherit
Expand Down Expand Up @@ -425,7 +539,7 @@
uses: ./.github/workflows/benchmark-tmpl.yml
name: eval /
strategy:
fail-fast: false
fail-fast: ${{ inputs.fail-fast }}
matrix:
config: ${{ fromJson(needs.get-jobs.outputs.eval-config) }}
secrets: inherit
Expand Down Expand Up @@ -498,7 +612,7 @@
run: pip install PyGithub

- name: Calculate success rate
run: python3 utils/calc_success_rate.py $STATS_FILENAME
run: python3 utils/calc_success_rate.py "$STATS_FILENAME"

- uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/run-sweep.yml
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ jobs:
(
github.event_name == 'pull_request' &&
!github.event.pull_request.draft &&
github.event.pull_request.head.repo.full_name == github.repository &&
needs.check-changelog.outputs.skip-pr-sweep != 'true' &&
(
contains(github.event.pull_request.labels.*.name, 'sweep-enabled') ||
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/test-changelog-gate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ on:
- ".github/workflows/run-sweep.yml"
- ".github/workflows/speedbench-al.yml"
- ".github/workflows/test-changelog-gate.yml"
- ".github/workflows/trusted-external-sweep.yml"
- "benchmarks/benchmark_lib.sh"
- "configs/ci-priority.yaml"
- "benchmarks/multi_node/amd_utils/job.slurm"
Expand Down
Loading