Skip to content

Commit 582af33

Browse files
committed
docs+ci: download fork-comment artifact to runner.temp (CodeQL artifact-poisoning)
1 parent f059f8e commit 582af33

2 files changed

Lines changed: 11 additions & 6 deletions

File tree

.github/workflows/bumpy-comment.yaml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,15 @@ jobs:
2727
- run: bun install
2828
- run: bun run --filter @varlock/bumpy build
2929
- run: bun install # link the freshly-built CLI bin
30-
# Download AFTER checkout — checkout cleans the workspace and would wipe it.
30+
# Download to runner.temp (OUTSIDE the checkout) so the untrusted artifact can't
31+
# overwrite our trusted files.
3132
- uses: actions/download-artifact@v4
3233
continue-on-error: true # the check may not have produced a comment
3334
with:
3435
name: bumpy-comment
35-
path: ./bumpy-comment
36+
path: ${{ runner.temp }}/bumpy-comment
3637
run-id: ${{ github.event.workflow_run.id }}
3738
github-token: ${{ github.token }}
38-
- run: bunx @varlock/bumpy ci comment --body-file ./bumpy-comment/comment.md
39+
- run: bunx @varlock/bumpy ci comment --body-file "$RUNNER_TEMP/bumpy-comment/comment.md"
3940
env:
4041
GH_TOKEN: ${{ github.token }}

docs/github-actions.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,14 @@ jobs:
8585
continue-on-error: true # the check may not have produced a comment
8686
with:
8787
name: bumpy-comment
88-
path: ./bumpy-comment
88+
# Download OUTSIDE the checkout (runner.temp) so the untrusted artifact
89+
# can't overwrite trusted files.
90+
path: ${{ runner.temp }}/bumpy-comment
8991
run-id: ${{ github.event.workflow_run.id }}
9092
github-token: ${{ github.token }}
9193
- run: |
9294
VERSION=$(jq -r '.devDependencies["@varlock/bumpy"] // .dependencies["@varlock/bumpy"]' package.json | sed 's/[\^~]//')
93-
bunx "@varlock/bumpy@$VERSION" ci comment --body-file ./bumpy-comment/comment.md
95+
bunx "@varlock/bumpy@$VERSION" ci comment --body-file "$RUNNER_TEMP/bumpy-comment/comment.md"
9496
env:
9597
GH_TOKEN: ${{ github.token }}
9698
```
@@ -99,7 +101,9 @@ Point `workflows: [...]` at the **name** of whatever runs your check (your exist
99101

100102
> **Heads-up: it won't run until it's on your default branch.** `workflow_run` always uses the workflow file as it exists on the default branch, so the poster doesn't fire for the PR that _adds_ it — fork comments start working once `bumpy-comment.yaml` is merged to `main`. (Same-repo PRs are unaffected; they comment directly from the check.)
101103

102-
> **The one safety rule.** The uploaded artifact is **untrusted** — it's produced by the unprivileged `pull_request` run from fork-controlled inputs (the comment is rendered from the PR's own bump files, and the run may execute fork code), so treat its contents as attacker-controlled. `bumpy ci comment` uses the body only as comment text and resolves the **target PR from the trusted `workflow_run` event** (`head_sha`), never from the artifact — that's what stops a fork from redirecting the comment onto a different PR or issue. Don't override it with a `--pr` derived from artifact data.
104+
> **The one safety rule.** The uploaded artifact is **untrusted** — it's produced by the unprivileged `pull_request` run from fork-controlled inputs (the comment is rendered from the PR's own bump files, and the run may execute fork code), so treat its contents as attacker-controlled. Two things keep it safe: (1) download it to `runner.temp`, **outside the checkout**, so it can't overwrite trusted files; and (2) `bumpy ci comment` uses the body only as comment text and resolves the **target PR from the trusted `workflow_run` event** (`head_sha`), never from the artifact — that's what stops a fork from redirecting the comment onto a different PR or issue. Don't override it with a `--pr` derived from artifact data.
105+
106+
> **If CodeQL flags this step (`actions/artifact-poisoning`):** that query fires on any `workflow_run` workflow that consumes an artifact from the triggering run. Downloading to `runner.temp` as above addresses its core recommendation (extract to a folder that can't overwrite existing files). If your repo's query is strict enough to still flag the download, it's a false positive you can dismiss — the body is used only as comment text and the PR is resolved from the trusted event, never the artifact.
103107

104108
> **Why can't the fork run post it itself?** A fork's `pull_request` token is read-only at issuance and enforced server-side — REST, GraphQL, `gh`, and raw `curl` all 403 on a comment write, and secrets aren't exposed either. The write has to originate from a privileged base-repo run, which is exactly what `workflow_run` provides.
105109

0 commit comments

Comments
 (0)