From 3c51335bf9f2cbc8fafd692010f9fc5bd966ac1e Mon Sep 17 00:00:00 2001 From: Donkoko Date: Tue, 28 Jul 2026 13:32:11 +0300 Subject: [PATCH 1/6] ci: run PR tests on pull_request instead of pull_request_target actions/checkout now refuses an explicit fork-PR ref inside a pull_request_target workflow, which broke every fork PR at the checkout step in under 10s. None of the three active jobs consume a secret, so the trusted context bought them nothing. Drops the authorize gate and the ref: override, pins least-privilege permissions, stops persisting git credentials, and replaces the broken cancel-workflow-action with native concurrency. Refs #2385 --- .github/workflows/test.yml | 81 ++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 35 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 74ce79b98c..a8b31b65a2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,7 +1,25 @@ -name: ๐Ÿงช Test PR from fork +name: ๐Ÿงช Test + +# SECURITY INVARIANT โ€” read this before adding a job to this workflow. +# +# This workflow runs on `pull_request`, so fork PRs execute here. GitHub gives +# those runs a read-only GITHUB_TOKEN and NO repository secrets โ€” that is +# precisely what makes it safe to run untrusted contributor code automatically. +# +# Therefore: any job that consumes a secret MUST NOT live in this workflow. +# Secret-bearing jobs (e.g. the Playwright/E2E suite below) belong in a separate +# workflow gated behind the `external` environment, and must never check out +# fork PR code in that trusted context. +# +# Do NOT "fix" a fork PR checkout failure by setting `allow-unsafe-pr-checkout: +# true` on a checkout step. That re-enables the "pwn request" attack class that +# actions/checkout began blocking in July 2026. on: - pull_request_target: + pull_request: + # Called by deploy.yml on pushes to main/dev. The secrets below are declared + # for the E2E job that currently lives commented-out at the bottom of this + # file; deploy.yml passes all six, so this block must not be removed. workflow_call: secrets: SESSION_SECRET: @@ -17,28 +35,32 @@ on: DATABASE_URL: required: true -jobs: - authorize: - environment: ${{ github.event_name == 'pull_request_target' && - github.event.pull_request.head.repo.full_name != github.repository && - 'external' || '' }} +# Least privilege, pinned explicitly. The repo default is already `read`, but +# pinning it means a future org-level default change cannot silently escalate +# these jobs. `contents: read` is what actions/checkout needs to fetch. +permissions: + contents: read - runs-on: ubuntu-latest - steps: - - run: echo โœ“ +concurrency: + # PR number for PR runs; run_id on the workflow_call path so a push to main + # gets a unique group and can never be cancelled by a PR run. Note that in a + # reusable workflow the `github` context belongs to the CALLER. + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} + cancel-in-progress: true +jobs: lint: - needs: authorize name: โฌฃ ESLint runs-on: ubuntu-latest steps: - - name: ๐Ÿ›‘ Cancel Previous Runs - uses: styfle/cancel-workflow-action@0.11.0 - - name: โฌ‡๏ธ Checkout repo uses: actions/checkout@v4 with: - ref: ${{ github.event.pull_request.head.sha || github.ref }} + # No `ref:` โ€” the default checkout resolves to the PR merge ref and is + # exempt from actions/checkout's fork-PR guard. No credentials + # persisted: nothing after this step needs git auth, so the token + # never lands in .git/config where fork code could read it. + persist-credentials: false - name: ๐Ÿ“ฆ Setup pnpm uses: pnpm/action-setup@v4 @@ -55,17 +77,13 @@ jobs: run: pnpm run lint typecheck: - needs: authorize name: สฆ TypeScript runs-on: ubuntu-latest steps: - - name: ๐Ÿ›‘ Cancel Previous Runs - uses: styfle/cancel-workflow-action@0.11.0 - - name: โฌ‡๏ธ Checkout repo uses: actions/checkout@v4 with: - ref: ${{ github.event.pull_request.head.sha || github.ref }} + persist-credentials: false - name: ๐Ÿ“ฆ Setup pnpm uses: pnpm/action-setup@v4 @@ -82,17 +100,13 @@ jobs: run: pnpm run typecheck vitest: - needs: authorize name: โšก Vitest runs-on: ubuntu-latest steps: - - name: ๐Ÿ›‘ Cancel Previous Runs - uses: styfle/cancel-workflow-action@0.11.0 - - name: โฌ‡๏ธ Checkout repo uses: actions/checkout@v4 with: - ref: ${{ github.event.pull_request.head.sha || github.ref }} + persist-credentials: false - name: ๐Ÿ“ฆ Setup pnpm uses: pnpm/action-setup@v4 @@ -111,19 +125,16 @@ jobs: - name: โšก Run vitest run: pnpm --filter @shelf/webapp test -- --run + # NOTE: this job consumes secrets โ€” see the SECURITY INVARIANT at the top of + # this file. Do NOT re-enable it here. It belongs in its own workflow gated + # behind the `external` environment, and must not check out fork PR code. # playwright: - # needs: authorize # name: ๐ŸŽญ Playwright # timeout-minutes: 60 # runs-on: ubuntu-latest # steps: - # - name: ๐Ÿ›‘ Cancel Previous Runs - # uses: styfle/cancel-workflow-action@0.11.0 - # - name: โฌ‡๏ธ Checkout repo - # uses: actions/checkout@v3 - # with: - # ref: ${{ github.event.pull_request.head.sha || github.ref }} + # uses: actions/checkout@v4 # - name: ๐Ÿ”‘ Make envfile # uses: SpicyPizza/create-envfile@v2.0 @@ -137,9 +148,9 @@ jobs: # file_name: .env # - name: โŽ” Setup node - # uses: actions/setup-node@v3 + # uses: actions/setup-node@v4 # with: - # node-version: 18 + # node-version: 22 # - name: ๐Ÿ“ฅ Download deps # uses: bahmutov/npm-install@v1 @@ -153,7 +164,7 @@ jobs: # - name: Run Playwright tests # run: npx playwright test - # - uses: actions/upload-artifact@v3 + # - uses: actions/upload-artifact@v4 # if: always() # with: # name: playwright-report From 94b73efc6fd7cf1c748bce52a31595643d65d4e4 Mon Sep 17 00:00:00 2001 From: Donkoko Date: Tue, 28 Jul 2026 13:32:29 +0300 Subject: [PATCH 2/6] ci(react-doctor): run on pull_request and gate the sticky comment Same checkout-guard fix as test.yml. Fork and Dependabot PRs run with a read-only GITHUB_TOKEN, so the sticky comment is skipped for them and findings surface via inline annotations plus a new job summary step. Annotations are emitted as workflow log commands rather than API calls, so they keep working without a writable token. Refs #2385 --- .github/workflows/react-doctor.yml | 48 ++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/.github/workflows/react-doctor.yml b/.github/workflows/react-doctor.yml index 5477a69470..fa1645dabc 100644 --- a/.github/workflows/react-doctor.yml +++ b/.github/workflows/react-doctor.yml @@ -8,7 +8,7 @@ name: ๐Ÿฉบ React Doctor # 2. Fails the check if NEW errors are introduced โ€” warnings stay advisory. on: - pull_request_target: + pull_request: paths: - "apps/webapp/**/*.ts" - "apps/webapp/**/*.tsx" @@ -20,17 +20,12 @@ on: - ".github/workflows/react-doctor.yml" - ".github/scripts/react-doctor-pr-comment.mjs" -jobs: - authorize: - environment: ${{ github.event_name == 'pull_request_target' && - github.event.pull_request.head.repo.full_name != github.repository && - 'external' || '' }} - runs-on: ubuntu-latest - steps: - - run: echo โœ“ +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true +jobs: doctor: - needs: authorize name: ๐Ÿฉบ React Doctor (${{ matrix.app.name }}) runs-on: ubuntu-latest permissions: @@ -47,17 +42,19 @@ jobs: - name: companion dir: apps/companion steps: - - name: ๐Ÿ›‘ Cancel previous runs - uses: styfle/cancel-workflow-action@0.11.0 - - - name: โฌ‡๏ธ Checkout PR head + - name: โฌ‡๏ธ Checkout PR merge ref uses: actions/checkout@v4 with: - ref: ${{ github.event.pull_request.head.sha || github.ref }} + # No `ref:` โ€” the default checkout resolves to the PR merge ref and is + # exempt from actions/checkout's fork-PR guard. fetch-depth: 0 is + # still required so `origin/` exists for the --diff below. fetch-depth: 0 + # Nothing after this step needs git auth, so the token never lands in + # .git/config where fork-controlled code could read it back out. + persist-credentials: false - name: ๐Ÿชข Re-attach HEAD to a named branch - # actions/checkout leaves us on a DETACHED HEAD when given a sha. + # actions/checkout leaves us on a DETACHED HEAD at the PR merge ref. # react-doctor's --diff path calls `git rev-parse --abbrev-ref HEAD` # to find the current branch; in detached state that returns the # literal "HEAD", which the CLI interprets as "no branch" and silently @@ -157,7 +154,20 @@ jobs: cat "$RUNNER_TEMP/comment.md" - name: ๐Ÿ’ฌ Post or update PR comment + # Fork and Dependabot PRs run with a read-only GITHUB_TOKEN, so the + # comment API 403s for them. Those PRs surface findings through the + # inline annotations (emitted as log commands, which need no token) and + # the job summary written below. + # + # Predicate on pull_request.user.login, NOT github.actor: the read-only + # token is keyed to who AUTHORED the PR, but github.actor becomes the + # re-runner's login on a manual re-run โ€” which would wrongly re-enable + # this step on a Dependabot PR re-run. if: always() && steps.changes.outputs.changed == 'true' + && github.event.pull_request.head.repo.full_name == github.repository + && github.event.pull_request.user.login != 'dependabot[bot]' + # Backstop for any read-only-token case not enumerated above. + continue-on-error: true uses: marocchino/sticky-pull-request-comment@v2 with: # Per-app header so the two matrix jobs maintain separate sticky @@ -165,6 +175,12 @@ jobs: header: react-doctor-${{ matrix.app.name }} path: ${{ runner.temp }}/comment.md + - name: ๐Ÿ“‹ Write findings to job summary + # Runs for EVERY PR including forks and Dependabot โ€” this is the only + # aggregated report those PRs get, since the sticky comment is skipped. + if: always() && steps.changes.outputs.changed == 'true' + run: cat "$RUNNER_TEMP/comment.md" >> "$GITHUB_STEP_SUMMARY" + - name: โŒ Fail if errors introduced if: steps.changes.outputs.changed == 'true' && steps.doctor.outputs.exit_code != '0' run: | From 0ccb64b611c7403df700780ce2a5a80bcc6f6fc9 Mon Sep 17 00:00:00 2001 From: Donkoko Date: Tue, 28 Jul 2026 13:32:35 +0300 Subject: [PATCH 3/6] ci(docs): skip Cloudflare deploy on fork PRs and stop interpolating head_ref Fork PRs get no repository secrets, so CLOUDFLARE_API_TOKEN was empty and the deploy always failed. The docs build still runs for them, which is the part that can actually regress. Also routes github.head_ref through an env var instead of interpolating it into the shell command, matching the convention react-doctor.yml already uses for BASE_REF. --- .github/workflows/docs-deploy.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/docs-deploy.yml b/.github/workflows/docs-deploy.yml index 65f69522e1..7e3b10df19 100644 --- a/.github/workflows/docs-deploy.yml +++ b/.github/workflows/docs-deploy.yml @@ -35,11 +35,22 @@ jobs: - name: ๐Ÿ”จ Build docs run: pnpm docs:build + # Fork PRs receive no repository secrets, so CLOUDFLARE_API_TOKEN is empty + # and the deploy is a guaranteed failure. Skip it for them โ€” the docs + # BUILD above still runs, which is the part that can actually regress. + # The `push` arm is required: pushes to main have no pull_request context. - name: ๐Ÿ“ฅ Install Wrangler + if: github.event_name == 'push' || + github.event.pull_request.head.repo.full_name == github.repository run: npm install -g wrangler - name: ๐Ÿš€ Deploy to Cloudflare Pages - run: wrangler pages deploy apps/docs/.vitepress/dist --project-name=shelf-docs --branch=${{ github.event_name == 'push' && 'main' || github.head_ref }} + if: github.event_name == 'push' || + github.event.pull_request.head.repo.full_name == github.repository env: CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + # github.head_ref is attacker-controlled PR input (a branch name), so + # it goes through env instead of being interpolated into the script. + DEPLOY_BRANCH: ${{ github.event_name == 'push' && 'main' || github.head_ref }} + run: wrangler pages deploy apps/docs/.vitepress/dist --project-name=shelf-docs --branch="$DEPLOY_BRANCH" From 0fe790dbb90d5f4d04f511c287064abc2d084786 Mon Sep 17 00:00:00 2001 From: Donkoko Date: Tue, 28 Jul 2026 14:02:22 +0300 Subject: [PATCH 4/6] ci(docs): pin wrangler to a major version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The install step hands CLOUDFLARE_API_TOKEN to whatever it pulls, so resolving "latest" at runtime would let a compromised release reach the credential. Pinned to the major rather than an exact version so patch and minor fixes still land โ€” an exact pin goes stale and breaks the deploy when Cloudflare moves their API forward. Kept as a global install on purpose (acb25d0d1). --- .github/workflows/docs-deploy.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/docs-deploy.yml b/.github/workflows/docs-deploy.yml index 7e3b10df19..70857a941d 100644 --- a/.github/workflows/docs-deploy.yml +++ b/.github/workflows/docs-deploy.yml @@ -42,7 +42,13 @@ jobs: - name: ๐Ÿ“ฅ Install Wrangler if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository - run: npm install -g wrangler + # Pinned to the major: this step hands CLOUDFLARE_API_TOKEN to whatever + # it installs, so resolving "latest" at runtime would let a compromised + # release reach the credential. Major-pinned rather than exact so patch + # and minor fixes still land โ€” an exact pin goes stale and breaks the + # deploy when Cloudflare moves their API forward. + # Installed globally rather than via pnpm on purpose (acb25d0d1). + run: npm install -g wrangler@4 - name: ๐Ÿš€ Deploy to Cloudflare Pages if: github.event_name == 'push' || From 5d4adb5127700cabd4fb24aa89ec5f670e614899 Mon Sep 17 00:00:00 2001 From: Donkoko Date: Tue, 28 Jul 2026 14:25:56 +0300 Subject: [PATCH 5/6] ci(docs): pin wrangler to an exact version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit wrangler@4 still resolved the newest 4.x at runtime, and that package receives CLOUDFLARE_API_TOKEN. Wrangler pins all its own non-optional deps exactly, so an exact pin here yields a deterministic tree. Bumping is manual: Dependabot parses `uses:` refs, not npm names inside `run:` scripts. Accepted trade โ€” a stale wrangler fails loudly at deploy time, a compromised one fails silently. Not made a devDependency of apps/docs: wrangler pulls workerd (~127MB) and CI has no pnpm caching, so all 5 jobs would download it on every PR. --- .github/workflows/docs-deploy.yml | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/.github/workflows/docs-deploy.yml b/.github/workflows/docs-deploy.yml index 70857a941d..44e57829af 100644 --- a/.github/workflows/docs-deploy.yml +++ b/.github/workflows/docs-deploy.yml @@ -42,13 +42,23 @@ jobs: - name: ๐Ÿ“ฅ Install Wrangler if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository - # Pinned to the major: this step hands CLOUDFLARE_API_TOKEN to whatever - # it installs, so resolving "latest" at runtime would let a compromised - # release reach the credential. Major-pinned rather than exact so patch - # and minor fixes still land โ€” an exact pin goes stale and breaks the - # deploy when Cloudflare moves their API forward. - # Installed globally rather than via pnpm on purpose (acb25d0d1). - run: npm install -g wrangler@4 + # Pinned EXACTLY, on purpose. This step hands CLOUDFLARE_API_TOKEN to + # whatever it installs, so a floating range would let a compromised + # release reach the credential. Wrangler pins all of its own non-optional + # deps exactly (esbuild, workerd, miniflare, unenv...), so an exact pin + # here resolves a deterministic tree without needing a lockfile entry. + # + # Bumping is MANUAL: Dependabot's github-actions ecosystem parses `uses:` + # references, not npm package names inside `run:` scripts, so nothing + # updates this automatically. That is the accepted trade โ€” a stale + # wrangler fails loudly at deploy time, a compromised one fails silently. + # + # Not a devDependency of apps/docs: wrangler pulls in workerd (~127MB), + # and with no pnpm caching in CI every job in the monorepo would + # download it on every PR, to upload a static VitePress site. + # Not via pnpm/wrangler-action either โ€” that hits the pnpm + # workspace-root restriction this step was written to avoid (acb25d0d1). + run: npm install -g wrangler@4.114.0 - name: ๐Ÿš€ Deploy to Cloudflare Pages if: github.event_name == 'push' || From 81b592e7fe2844ab33b5b8af237f9baf8b2f4ad5 Mon Sep 17 00:00:00 2001 From: Donkoko Date: Tue, 28 Jul 2026 14:36:17 +0300 Subject: [PATCH 6/6] ci(docs): stop persisting git credentials on checkout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pnpm install and pnpm docs:build run fork-authored code (lifecycle scripts, vitepress config) before the deploy guard, and checkout writes GITHUB_TOKEN into .git/config by default. Nothing here needs git auth after checkout. Completes the sweep โ€” test.yml and react-doctor.yml already had this; docs-deploy.yml was the third fork-reachable workflow and was missed. --- .github/workflows/docs-deploy.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/docs-deploy.yml b/.github/workflows/docs-deploy.yml index 44e57829af..f09697b852 100644 --- a/.github/workflows/docs-deploy.yml +++ b/.github/workflows/docs-deploy.yml @@ -20,6 +20,12 @@ jobs: - name: โฌ‡๏ธ Checkout repo uses: actions/checkout@v4 + with: + # `pnpm install` and `pnpm docs:build` below run fork-authored code + # (lifecycle scripts, vitepress config) BEFORE the deploy guard, and + # checkout persists GITHUB_TOKEN into .git/config by default. Nothing + # here needs git auth after checkout, so don't leave it on disk. + persist-credentials: false - name: ๐Ÿ“ฆ Setup pnpm uses: pnpm/action-setup@v4