From 7d56da8171eae6f28cb898c970139877ccbd4284 Mon Sep 17 00:00:00 2001 From: Andrew Harris Date: Wed, 22 Jul 2026 14:35:00 +0100 Subject: [PATCH 1/5] Validate fork PR plugins via pull_request_target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CI workflow logged in with --apiKey before validating, but GitHub Actions withholds repository secrets from pull_request runs triggered by a fork PR. The API key resolved to an empty string, and the CLI silently fell through to the OAuth device flow, hanging for its full 5-minute timeout before failing instead of erroring immediately. Switched the trigger to pull_request_target, which runs with the base repo's secrets regardless of where the PR originates, and pinned checkout to the PR's head commit so a later push can't swap in different code after review. The workflow file itself is always read from the base branch under pull_request_target, so a fork editing pr-run.yaml has no effect — but the checked-out plugin files are still attacker-controlled data. Three spots spliced ${{ steps.detect.outputs.plugin_paths }} directly into shell scripts, a script-injection hole that was harmless without secrets but not with them; that data now flows through an env var instead. The CLI install also moved outside the checkout, since npm reads a project .npmrc from the working directory even for global installs, and a fork PR could otherwise redirect the install to a package it controls in the same step the API key is in scope. Deploy still only runs for same-repo PRs — validating fork-submitted plugin code is one thing, but auto-deploying it to the live tenant needs a maintainer to decide that deliberately. Also logs the installed CLI version, since not knowing which version had actually run was part of what made this issue hard to diagnose. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/pr-run.yaml | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/.github/workflows/pr-run.yaml b/.github/workflows/pr-run.yaml index bdbf94a4..cb31ff64 100644 --- a/.github/workflows/pr-run.yaml +++ b/.github/workflows/pr-run.yaml @@ -1,7 +1,7 @@ name: Validate & Deploy Plugins on: - pull_request: + pull_request_target: jobs: validate-and-deploy: @@ -11,6 +11,7 @@ jobs: steps: - uses: actions/checkout@v6 with: + ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 0 - name: Detect modified plugins @@ -37,14 +38,18 @@ jobs: env: SQUAREDUP_API_KEY: ${{ secrets.SQUAREDUP_API_KEY }} run: | - echo "Installing SquaredUp CLI..." - npm install -g @squaredup/cli - echo "Configuring SquaredUp CLI with API key..." - squaredup login --apiKey "$SQUAREDUP_API_KEY" + echo "Installing SquaredUp CLI..." + cd "$RUNNER_TEMP" + npm install -g @squaredup/cli + echo "SquaredUp CLI version: $(squaredup -v)" + echo "Configuring SquaredUp CLI with API key..." + squaredup login --apiKey "$SQUAREDUP_API_KEY" - name: Validate modified plugins id: validate if: steps.detect.outputs.plugins_modified == 'true' + env: + PLUGIN_PATHS: ${{ steps.detect.outputs.plugin_paths }} run: | validation_failed=false @@ -66,7 +71,7 @@ jobs: validation_failed=true fi echo "" - done <<< "${{ steps.detect.outputs.plugin_paths }}" + done <<< "$PLUGIN_PATHS" if [ "$validation_failed" = "true" ]; then echo "One or more plugins failed validation." @@ -75,17 +80,23 @@ jobs: - name: Deploy modified plugins id: deploy - if: steps.detect.outputs.plugins_modified == 'true' + # Fork PRs only validate; deploying arbitrary fork-submitted plugin code (incl. + # scripts) to the live tenant needs a maintainer to merge/trigger it deliberately. + if: steps.detect.outputs.plugins_modified == 'true' && github.event.pull_request.head.repo.full_name == github.repository + env: + PLUGIN_PATHS: ${{ steps.detect.outputs.plugin_paths }} run: | while IFS= read -r plugin_path; do echo "Deploying ${plugin_path}..." squaredup deploy "${plugin_path}" --suffix "${{ github.event.pull_request.number }}" --force echo "Deployed ${plugin_path} successfully." echo "" - done <<< "${{ steps.detect.outputs.plugin_paths }}" + done <<< "$PLUGIN_PATHS" - name: Summary if: always() + env: + PLUGIN_PATHS: ${{ steps.detect.outputs.plugin_paths }} run: | OUT=/tmp/summary.md @@ -101,7 +112,7 @@ jobs: echo "### 📦 Modified Plugins" >> $OUT while IFS= read -r plugin_path; do echo "- \`${plugin_path}\`" >> $OUT - done <<< "${{ steps.detect.outputs.plugin_paths }}" + done <<< "$PLUGIN_PATHS" echo "" >> $OUT echo "### 📋 Results" >> $OUT From aa2e33585b0b5fc1c2aadb2850048e7442405909 Mon Sep 17 00:00:00 2001 From: Andrew Harris Date: Thu, 23 Jul 2026 10:47:19 +0100 Subject: [PATCH 2/5] Allow manually re-running the plugin CI workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pull_request_target locks execution to the workflow file on the PR's base ref, which is exactly what makes it safe against a fork editing pr-run.yaml — but it also means a PR can never be used to test changes to this file itself; the old version keeps running until the change lands on main. Adding workflow_dispatch closes that gap: it lets anyone with write access run this workflow directly against a branch (gh workflow run "Validate & Deploy Plugins" --ref ), executing that branch's own copy of the file. Two steps assumed a pull_request payload that doesn't exist for a manual run — checkout's ref now falls back to the dispatched commit, and the PR-comment step (which has nothing to comment on) is skipped outside pull_request_target. workflow_dispatch has a different safety model than pull_request_target though: it runs whatever copy of the workflow lives on the ref you select, not always the default branch's. A fork can't trigger it at all (dispatching requires write access to this repo), but a maintainer manually dispatching against a PR/fork ref rather than a real branch would run that PR's own version of the workflow with the real secret in scope. Left an explicit warning against doing that next to the trigger. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/pr-run.yaml | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-run.yaml b/.github/workflows/pr-run.yaml index cb31ff64..1ca9ca80 100644 --- a/.github/workflows/pr-run.yaml +++ b/.github/workflows/pr-run.yaml @@ -2,6 +2,16 @@ name: Validate & Deploy Plugins on: pull_request_target: + # Lets you run this workflow directly against any branch — pull_request_target always + # resolves the workflow file from the PR's base ref, so a PR can never exercise changes + # made to this file on its own branch. Test with: + # gh workflow run "Validate & Deploy Plugins" --ref + # + # Only ever dispatch against a real branch of this repo. Unlike pull_request_target, + # workflow_dispatch runs whatever copy of this file lives on the ref you pick — so + # dispatching against a PR/fork ref (e.g. refs/pull/89/head) would run that PR's own + # version of this workflow, with the real secret in scope. + workflow_dispatch: jobs: validate-and-deploy: @@ -11,7 +21,8 @@ jobs: steps: - uses: actions/checkout@v6 with: - ref: ${{ github.event.pull_request.head.sha }} + # workflow_dispatch has no pull_request payload — fall back to the dispatched ref. + ref: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.sha }} fetch-depth: 0 - name: Detect modified plugins @@ -152,7 +163,8 @@ jobs: cat $OUT >> $GITHUB_STEP_SUMMARY - name: Post PR comment - if: always() + # No PR/issue to comment on for a manual workflow_dispatch run. + if: always() && github.event_name == 'pull_request_target' uses: actions/github-script@v7 with: script: | From 8a17b72728e5dd99d0ac53da58ea9420b3d52375 Mon Sep 17 00:00:00 2001 From: Andrew Harris Date: Thu, 23 Jul 2026 11:13:30 +0100 Subject: [PATCH 3/5] Temporarily keep pull_request so this PR's own check runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validate-and-deploy is a required status check on main, and this PR can't satisfy it against itself: its own branch removed pull_request (the only trigger that ever evaluates a PR's own workflow content), and pull_request_target always resolves from main, which doesn't have that trigger yet. Neither one fires, so the required check never reports anything and the merge stays blocked — confirmed via `gh run list`, which shows no pull_request/pull_request_target run has ever fired on this branch, and the repo's ruleset has no bypass actors, so there's no way around it either. Restoring pull_request here just long enough for this PR to report its own required check. Once merged, main has pull_request_target and every subsequent PR is covered by it, so pull_request comes back out in an immediate follow-up. Both the checkout ref fallback and the PR-comment gate were already written in terms of what data is present (a pull_request payload, or not workflow_dispatch) rather than a specific event name, so they keep working through this transition without further changes, including once pull_request is removed again. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/pr-run.yaml | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/.github/workflows/pr-run.yaml b/.github/workflows/pr-run.yaml index 1ca9ca80..34673b0a 100644 --- a/.github/workflows/pr-run.yaml +++ b/.github/workflows/pr-run.yaml @@ -1,10 +1,21 @@ name: Validate & Deploy Plugins on: + # TEMPORARY, remove in a follow-up PR once this is merged: validate-and-deploy is a + # required status check, and a required check can never be satisfied by a PR that + # removes its own only trigger — pull_request_target (see below) always resolves from + # main, which doesn't have it yet, so it can't fire for *this* PR either. Keeping + # pull_request alive here just long enough for this PR to report its own required + # check; once merged, main has pull_request_target and every future PR is covered by + # it, so pull_request can come out again. + pull_request: + # GitHub always resolves the workflow file (and GITHUB_REF/GITHUB_SHA) for + # pull_request_target from the repo's actual default branch, never the PR's own + # branch — so a PR editing this file (fork or not) can't change what runs against it. pull_request_target: - # Lets you run this workflow directly against any branch — pull_request_target always - # resolves the workflow file from the PR's base ref, so a PR can never exercise changes - # made to this file on its own branch. Test with: + # Lets you run this workflow directly against any branch — since pull_request_target + # always runs main's copy, a PR can't exercise changes made to this file on its own + # branch any other way. Test with: # gh workflow run "Validate & Deploy Plugins" --ref # # Only ever dispatch against a real branch of this repo. Unlike pull_request_target, @@ -22,7 +33,7 @@ jobs: - uses: actions/checkout@v6 with: # workflow_dispatch has no pull_request payload — fall back to the dispatched ref. - ref: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.sha }} + ref: ${{ github.event.pull_request.head.sha || github.sha }} fetch-depth: 0 - name: Detect modified plugins @@ -164,7 +175,7 @@ jobs: - name: Post PR comment # No PR/issue to comment on for a manual workflow_dispatch run. - if: always() && github.event_name == 'pull_request_target' + if: always() && github.event_name != 'workflow_dispatch' uses: actions/github-script@v7 with: script: | From 80f646e2cbb74f07dc97f762afd3ef127e7ac11c Mon Sep 17 00:00:00 2001 From: Andrew Harris Date: Thu, 23 Jul 2026 11:17:30 +0100 Subject: [PATCH 4/5] Clarify who the workflow_dispatch ref warning is for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior wording didn't say who could actually act on it, which read as if a fork PR author had some way to leak the secret directly. They don't — dispatching requires write access to this repo, so only a maintainer can trigger it at all. The real risk is a maintainer being asked (or absent-mindedly choosing) to dispatch against a PR/fork ref rather than a branch, which is a social-engineering risk against a trusted human, not a bypass available to the fork author. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/pr-run.yaml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr-run.yaml b/.github/workflows/pr-run.yaml index 34673b0a..eb6fbc28 100644 --- a/.github/workflows/pr-run.yaml +++ b/.github/workflows/pr-run.yaml @@ -18,10 +18,12 @@ on: # branch any other way. Test with: # gh workflow run "Validate & Deploy Plugins" --ref # - # Only ever dispatch against a real branch of this repo. Unlike pull_request_target, - # workflow_dispatch runs whatever copy of this file lives on the ref you pick — so - # dispatching against a PR/fork ref (e.g. refs/pull/89/head) would run that PR's own - # version of this workflow, with the real secret in scope. + # Dispatching requires write access to this repo, so a fork PR author can't trigger + # this themselves. But if YOU dispatch it, only ever pick a real branch of this repo: + # unlike pull_request_target, workflow_dispatch runs whatever copy of this file lives + # on the ref you choose — dispatching against a PR/fork ref (e.g. refs/pull/89/head) + # instead of a branch would run that PR's own version of this workflow, with the real + # secret in scope. Never dispatch this against a ref someone else asked you to use. workflow_dispatch: jobs: From e6c148980ffee5f78374297c0ff202d04dec28fc Mon Sep 17 00:00:00 2001 From: Andrew Harris Date: Thu, 23 Jul 2026 13:24:04 +0100 Subject: [PATCH 5/5] Prevent duplicate/racing deploys, drop workflow_dispatch for now MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real issues from CodeRabbit's review, both against the current transitional dual-trigger setup: with pull_request and pull_request_target both active, a same-repo PR touching plugin files would run deploy twice per push (once per trigger), racing on the same --suffix. Restricted deploy to pull_request_target specifically to fix that at the root, and added a concurrency group per PR number so a rapid follow-up push can't independently overlap and finish out of order either — both were live gaps, not just theoretical. Also dropping workflow_dispatch. It was added so future changes to this file could be tested pre-merge, but it also puts the real SQUAREDUP_API_KEY in scope for a maintainer-triggered run, and we don't have an actual need for it yet — easier to add back once we do than to carry the extra secret-bearing trigger on spec. Checkout's ref and the PR-comment step's condition were only written the way they were to account for workflow_dispatch's missing pull_request payload, so both simplify back now that it's gone. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/pr-run.yaml | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/.github/workflows/pr-run.yaml b/.github/workflows/pr-run.yaml index eb6fbc28..1edf4b34 100644 --- a/.github/workflows/pr-run.yaml +++ b/.github/workflows/pr-run.yaml @@ -13,18 +13,13 @@ on: # pull_request_target from the repo's actual default branch, never the PR's own # branch — so a PR editing this file (fork or not) can't change what runs against it. pull_request_target: - # Lets you run this workflow directly against any branch — since pull_request_target - # always runs main's copy, a PR can't exercise changes made to this file on its own - # branch any other way. Test with: - # gh workflow run "Validate & Deploy Plugins" --ref - # - # Dispatching requires write access to this repo, so a fork PR author can't trigger - # this themselves. But if YOU dispatch it, only ever pick a real branch of this repo: - # unlike pull_request_target, workflow_dispatch runs whatever copy of this file lives - # on the ref you choose — dispatching against a PR/fork ref (e.g. refs/pull/89/head) - # instead of a branch would run that PR's own version of this workflow, with the real - # secret in scope. Never dispatch this against a ref someone else asked you to use. - workflow_dispatch: + +# Serialize runs per PR so an older run (from a rapid follow-up push, or the transitional +# pull_request/pull_request_target overlap above) can't finish last and overwrite a newer +# deploy that reuses the same --suffix. +concurrency: + group: pr-run-${{ github.event.pull_request.number }} + cancel-in-progress: true jobs: validate-and-deploy: @@ -34,8 +29,7 @@ jobs: steps: - uses: actions/checkout@v6 with: - # workflow_dispatch has no pull_request payload — fall back to the dispatched ref. - ref: ${{ github.event.pull_request.head.sha || github.sha }} + ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 0 - name: Detect modified plugins @@ -106,7 +100,10 @@ jobs: id: deploy # Fork PRs only validate; deploying arbitrary fork-submitted plugin code (incl. # scripts) to the live tenant needs a maintainer to merge/trigger it deliberately. - if: steps.detect.outputs.plugins_modified == 'true' && github.event.pull_request.head.repo.full_name == github.repository + # Restricted to pull_request_target specifically while both triggers coexist + # (see TEMPORARY note above) — otherwise a same-repo PR would deploy twice per + # push, racing on the same --suffix. + if: steps.detect.outputs.plugins_modified == 'true' && github.event.pull_request.head.repo.full_name == github.repository && github.event_name == 'pull_request_target' env: PLUGIN_PATHS: ${{ steps.detect.outputs.plugin_paths }} run: | @@ -176,8 +173,7 @@ jobs: cat $OUT >> $GITHUB_STEP_SUMMARY - name: Post PR comment - # No PR/issue to comment on for a manual workflow_dispatch run. - if: always() && github.event_name != 'workflow_dispatch' + if: always() uses: actions/github-script@v7 with: script: |