From cbe35198490e96f42c2b42726f3910c2e6fcf4d4 Mon Sep 17 00:00:00 2001 From: justin Date: Tue, 8 Sep 2026 21:39:36 -0600 Subject: [PATCH 1/4] Fix secure fork PR workflow boundaries --- .../workflows/neon-before-after-validate.yml | 15 ++-- .../workflows/neon-integration-pg-matrix.yml | 12 ++- .github/workflows/nixos-local-test.yml | 20 ++--- .github/workflows/pr-safe-checks.yml | 39 +++++++++ .github/workflows/release-drafter.yml | 2 +- .../2026-09-09-secure-fork-pr-ci-plan.md | 74 +++++++++++++++++ .../2026-09-09-secure-fork-pr-ci-design.md | 53 ++++++++++++ testing/test_workflow_security.py | 81 +++++++++++++++++++ workflows/neon-before-after-validate.yml | 9 ++- 9 files changed, 283 insertions(+), 22 deletions(-) create mode 100644 .github/workflows/pr-safe-checks.yml create mode 100644 docs/superpowers/plans/2026-09-09-secure-fork-pr-ci-plan.md create mode 100644 docs/superpowers/specs/2026-09-09-secure-fork-pr-ci-design.md create mode 100644 testing/test_workflow_security.py diff --git a/.github/workflows/neon-before-after-validate.yml b/.github/workflows/neon-before-after-validate.yml index 069b0ad..1650098 100644 --- a/.github/workflows/neon-before-after-validate.yml +++ b/.github/workflows/neon-before-after-validate.yml @@ -39,15 +39,14 @@ # (b) leave it blank and apply changes outside this workflow # (you only need the pre- and post- comparison jobs). # -# Note: pull_request from forks gets no repo secrets. Fork PRs fail at -# "Create Neon branch". That is the intended safe failure mode. +# Note: fork pull requests skip the privileged job before runner assignment. # Maintainers can validate a fork PR by running workflow_dispatch manually, # or by pushing the change to a same-repo branch first. name: Neon Before/After Validation on: - pull_request: + pull_request_target: paths: - 'migrations/**' - 'pgFirstAid.sql' @@ -75,6 +74,9 @@ env: jobs: before-after-validation: name: Before/After Health Validation + if: >- + github.event_name != 'pull_request_target' || + github.event.pull_request.head.repo.full_name == github.repository runs-on: [self-hosted, nix, nixos, x86_64-linux] timeout-minutes: 20 permissions: @@ -90,7 +92,8 @@ jobs: uses: actions/checkout@v4 with: fetch-depth: 1 - ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: false + ref: ${{ github.event.pull_request.head.sha || github.ref }} - name: Add Nix profile paths run: | @@ -174,7 +177,7 @@ jobs: echo " Pre-change baseline captured above." echo "" echo " The change being tested:" - echo " ${{ github.event_name == 'pull_request' && format('PR #{0}', github.event.number) || 'manual dispatch' }}" + echo " ${{ github.event_name == 'pull_request_target' && format('PR #{0}', github.event.number) || 'manual dispatch' }}" echo "" echo " Now applying SQL changes..." echo "" @@ -251,7 +254,7 @@ jobs: fi - name: Post PR comment - if: ${{ github.event_name == 'pull_request' }} + if: ${{ github.event_name == 'pull_request_target' }} uses: actions/github-script@v7 with: script: | diff --git a/.github/workflows/neon-integration-pg-matrix.yml b/.github/workflows/neon-integration-pg-matrix.yml index d8100d8..1fd296d 100644 --- a/.github/workflows/neon-integration-pg-matrix.yml +++ b/.github/workflows/neon-integration-pg-matrix.yml @@ -11,15 +11,15 @@ # PG17_HOST, PG17_PORT, PG17_USER, PG17_PASSWORD, PG17_DATABASE # PG18_HOST, PG18_PORT, PG18_USER, PG18_PASSWORD, PG18_DATABASE # -# Note: pull_request from forks gets no repo secrets. Fork PRs fail at -# "Validate required PG env vars". Maintainers can validate by pushing -# the change to a same-repo branch first. +# Note: fork pull requests skip the privileged job before runner assignment. +# Maintainers can validate a fork PR by running workflow_dispatch manually, +# or by pushing the change to a same-repo branch first. name: Neon Integration (PG15-PG18) on: workflow_dispatch: - pull_request: + pull_request_target: types: [opened, synchronize] paths: - pgFirstAid.sql @@ -35,6 +35,9 @@ concurrency: jobs: integration: + if: >- + github.event_name != 'pull_request_target' || + github.event.pull_request.head.repo.full_name == github.repository runs-on: [self-hosted, nix, nixos, x86_64-linux] permissions: contents: read @@ -109,6 +112,7 @@ jobs: uses: actions/checkout@v4 with: fetch-depth: 1 + persist-credentials: false ref: ${{ github.event.pull_request.head.sha || github.ref }} - name: Add Nix profile paths diff --git a/.github/workflows/nixos-local-test.yml b/.github/workflows/nixos-local-test.yml index f8532ab..ef29eaf 100644 --- a/.github/workflows/nixos-local-test.yml +++ b/.github/workflows/nixos-local-test.yml @@ -5,8 +5,8 @@ # instances your secrets point at (Neon / DigitalOcean / etc.). # # Triggers: -# - pull_request [opened, synchronize]: full matrix on PG15-18. Fork PRs -# get no secrets, so they fail at env validation. That's the point. +# - pull_request_target [opened, synchronize]: full matrix on PG15-18. +# Fork PRs skip the privileged job before runner assignment. # - workflow_dispatch: pick a PG version and cloud_provider label. # # The NixOS runner already has psql, uv, and network access to the test PG @@ -41,7 +41,7 @@ on: - gcp - azure default: 'direct' - pull_request: + pull_request_target: types: [opened, synchronize] paths: - pgFirstAid.sql @@ -57,14 +57,13 @@ concurrency: jobs: local-test: + if: >- + github.event_name != 'pull_request_target' || + github.event.pull_request.head.repo.full_name == github.repository runs-on: [self-hosted, nix, nixos, x86_64-linux] - # No job-level `if:`. The original conditional referenced - # `matrix.postgres_version` which is only valid in the strategy scope - # (after matrix evaluation), not at the job level. GitHub Actions - # rejects the workflow with "Unrecognized named-value: 'matrix'." - # The PR validation pass runs the full matrix unconditionally on - # pull_request; the workflow_dispatch path's per-version selection - # is handled by the strategy below. + # Skip fork pull requests before assigning a persistent self-hosted + # runner. The PR validation pass runs the full matrix for trusted PRs; + # the workflow_dispatch path's per-version selection is handled below. permissions: contents: read concurrency: @@ -126,6 +125,7 @@ jobs: uses: actions/checkout@v4 with: fetch-depth: 1 + persist-credentials: false ref: ${{ github.event.pull_request.head.sha || github.ref }} - name: Add Nix profile paths diff --git a/.github/workflows/pr-safe-checks.yml b/.github/workflows/pr-safe-checks.yml new file mode 100644 index 0000000..a3ff659 --- /dev/null +++ b/.github/workflows/pr-safe-checks.yml @@ -0,0 +1,39 @@ +name: PR Safe Checks + +on: + pull_request: + types: [opened, synchronize, reopened] + +permissions: + contents: read + +jobs: + safe-checks: + name: PR Safe Checks + runs-on: ubuntu-latest + steps: + - name: Checkout PR head + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 + with: + python-version: "3.11" + + - name: Install uv + run: | + curl -LsSf https://astral.sh/uv/install.sh | sh + echo "$HOME/.local/bin" >> "$GITHUB_PATH" + + - name: Sync dependencies + run: uv sync --frozen + + - name: Run pure contract tests + run: | + uv run pytest -q \ + testing/test_workflow_security.py \ + testing/integration/tests/integration/test_pgtap_suite.py::test_every_health_check_has_pgtap_coverage \ + testing/integration/tests/integration/test_pgtap_suite.py::test_both_view_sql_files_cover_all_health_checks diff --git a/.github/workflows/release-drafter.yml b/.github/workflows/release-drafter.yml index a6c8e6b..12a8486 100644 --- a/.github/workflows/release-drafter.yml +++ b/.github/workflows/release-drafter.yml @@ -15,7 +15,7 @@ jobs: permissions: contents: write pull-requests: write - runs-on: [self-hosted, nix, nixos, x86_64-linux] + runs-on: ubuntu-latest steps: - uses: release-drafter/release-drafter@v5 env: diff --git a/docs/superpowers/plans/2026-09-09-secure-fork-pr-ci-plan.md b/docs/superpowers/plans/2026-09-09-secure-fork-pr-ci-plan.md new file mode 100644 index 0000000..a8dbf44 --- /dev/null +++ b/docs/superpowers/plans/2026-09-09-secure-fork-pr-ci-plan.md @@ -0,0 +1,74 @@ +# Secure Fork PR Validation Implementation Plan + +**Goal:** Guard privileged pull-request workflows before runner assignment while retaining owner-operated validation and an always-on safe check for fork pull requests. + +**Spec:** `docs/superpowers/specs/2026-09-09-secure-fork-pr-ci-design.md` + +## Scope and constraints + +- Change only workflow YAML, workflow-security regression coverage, and this security design/plan documentation. +- Do not edit SQL files, pgTAP files, seed expectations, or `docs/health-checks/README.md`. +- Keep PR #44's `Unread Large Constraint-Backing Index`, pgTAP, seed, and health-check documentation work in PR #44 or a later follow-up. +- Do not change repository settings, rulesets, or dispatch behavior. +- Keep `workflow_dispatch` on all privileged database workflows. + +## Implementation steps + +### 1. Update regression coverage first + +Update `testing/test_workflow_security.py` to verify: + +- The three live secret-backed workflows use `pull_request_target` and `workflow_dispatch`. +- Each privileged job has the exact base-controlled repository guard before `runs-on`: + + ```yaml + if: >- + github.event_name != 'pull_request_target' || + github.event.pull_request.head.repo.full_name == github.repository + ``` + +- Privileged checkouts use the reviewed head SHA with a `github.ref` fallback and `persist-credentials: false`. +- The distributed Neon template has the same trigger, guard, checkout behavior, and no fork execution path. +- `PR Safe Checks` remains plain `pull_request`, hosted, secret-free, and runs only the workflow-security test and the two pure SQL coverage tests. +- `release-drafter.yml` uses `ubuntu-latest` and not the persistent self-hosted runner. + +Run the workflow-security test at this point and record its expected red result before implementation. + +### 2. Guard the three live privileged workflows + +In: + +- `.github/workflows/neon-before-after-validate.yml` +- `.github/workflows/neon-integration-pg-matrix.yml` +- `.github/workflows/nixos-local-test.yml` + +Change the automatic trigger to `pull_request_target`, preserve `workflow_dispatch`, and put the exact guard under the job before `runs-on`. Retain reviewed-head checkout behavior, add the `github.ref` fallback where needed, and keep `persist-credentials: false`. Replace stale comments about empty-secret failures with the statement that fork PRs skip the privileged job before runner assignment. + +### 3. Guard the distributed Neon template + +In `workflows/neon-before-after-validate.yml`, apply the same guarded `pull_request_target` and `workflow_dispatch` pattern. Use the reviewed head SHA with a dispatch fallback and `persist-credentials: false`. Keep the Neon secret only for trusted execution; the base-workflow guard must be false for fork heads before the job can run SQL or receive the secret. + +### 4. Make safe and release workflows hosted appropriately + +- Change `.github/workflows/release-drafter.yml` to `runs-on: ubuntu-latest`. +- Remove `testing/test_seed_and_validate.py::test_expected_check_groups_cover_all_defined_checks` from `.github/workflows/pr-safe-checks.yml` and from the workflow-security test's command assertions. +- Keep the workflow-security test and the two file-only SQL coverage tests in the safe workflow. + +### 5. Update design documentation + +Update the spec and this plan to describe guarded `pull_request_target`, the base-controlled fork boundary, the hosted release-drafter runner, and the reduced safe-check command. Explicitly state that PR #44's constraint-index, pgTAP, seed, and health-check documentation items are out of scope and belong to PR #44 or a later follow-up. + +### 6. Validate and commit + +Run: + +```bash +uv run pytest -q testing/test_workflow_security.py +uv run pytest -q \ + testing/integration/tests/integration/test_pgtap_suite.py::test_every_health_check_has_pgtap_coverage \ + testing/integration/tests/integration/test_pgtap_suite.py::test_both_view_sql_files_cover_all_health_checks +git diff --check +git status --short --branch +``` + +Review the diff to confirm no SQL, pgTAP, seed, health-check README, repository settings, or PR #44 files changed. Commit the completed fix round locally on `fix/secure-fork-pr-ci`. diff --git a/docs/superpowers/specs/2026-09-09-secure-fork-pr-ci-design.md b/docs/superpowers/specs/2026-09-09-secure-fork-pr-ci-design.md new file mode 100644 index 0000000..b3a14b1 --- /dev/null +++ b/docs/superpowers/specs/2026-09-09-secure-fork-pr-ci-design.md @@ -0,0 +1,53 @@ +# Secure Fork PR Validation Design + +## Goal + +Keep pull-request validation available to fork contributors without allowing fork-controlled code to access repository database credentials, `NEON_API_KEY`, or persistent self-hosted runners. + +## Current problem + +The live secret-backed database workflows run on persistent self-hosted runners and previously used `pull_request` with empty-secret validation inside the job. A fork job could therefore be assigned a persistent runner before that validation failed. The distributed Neon template also needs the same explicit trust boundary. + +## Design + +1. The three live secret-backed workflows and the distributed Neon template use `pull_request_target` plus `workflow_dispatch`. +2. Each privileged job has this base-controlled guard before `runs-on`: + + ```yaml + if: >- + github.event_name != 'pull_request_target' || + github.event.pull_request.head.repo.full_name == github.repository + ``` + + Fork pull requests therefore skip the job before runner assignment. Manual dispatch remains available for owner-operated validation, and same-repository pull requests remain eligible. +3. Privileged checkouts use the reviewed pull-request head SHA, with `github.ref` as the dispatch fallback, and set `persist-credentials: false`. The guarded jobs are the only workflows that consume database or Neon secrets. +4. `PR Safe Checks` remains a secret-free plain `pull_request` workflow on `ubuntu-latest`. It checks out the PR head without persisted credentials and runs workflow-security plus the two pure SQL coverage contracts. +5. `release-drafter.yml` runs on `ubuntu-latest`; its pull-request event does not require Nix or a persistent runner. + +## Trust boundaries + +A fork pull request can run only the secret-free hosted checks. Its privileged jobs evaluate false in the base workflow before runner selection, so fork SQL is not checked out or executed and `NEON_API_KEY` is not exposed to fork execution. Trusted database validation runs only for same-repository pull requests or an explicit owner-triggered dispatch. + +The distributed Neon template retains its Neon secret references for trusted execution, but its base-workflow guard is false for fork heads. No workflow uses `workflow_run` to check out a fork commit with secrets. + +## Status checks + +The safe workflow reports a stable advisory check without changing repository rulesets. Its test command includes the workflow-security test and the two file-only SQL coverage tests. The pre-existing failing static seed expectation test is intentionally not part of this security PR's safe-check command; it remains outside this branch's scope. + +## Out of scope + +PR #44's `Unread Large Constraint-Backing Index` check, pgTAP assertions, seed expectations, and health-check documentation are not part of this separate security PR. Those items belong to PR #44 or a later follow-up. This branch does not edit SQL, pgTAP, seed expectations, or `docs/health-checks/README.md`. + +## Validation + +- `testing/test_workflow_security.py` verifies guarded `pull_request_target` jobs, reviewed-head checkouts, the secret-free plain `pull_request` workflow, and the hosted release-drafter runner. +- The two pure SQL coverage tests run without a database. +- `git diff --check` and workflow inspection verify whitespace and scope. + +## Rejected alternatives + +- Do not use an unguarded `pull_request_target` job for privileged workflows. +- Do not retain empty-secret validation as the fork safety boundary. +- Do not run fork-controlled code on persistent self-hosted runners. +- Do not add a `workflow_run` follower that checks out a fork commit and runs it with secrets. +- Do not alter repository settings or mix PR #44's health-check contract changes into this security PR. diff --git a/testing/test_workflow_security.py b/testing/test_workflow_security.py new file mode 100644 index 0000000..469d45f --- /dev/null +++ b/testing/test_workflow_security.py @@ -0,0 +1,81 @@ +import re +from pathlib import Path + + +REPO_ROOT = Path(__file__).parent.parent +WORKFLOW_DIR = REPO_ROOT / ".github" / "workflows" + + +PRIVILEGED_WORKFLOWS = ( + "neon-before-after-validate.yml", + "neon-integration-pg-matrix.yml", + "nixos-local-test.yml", +) +GUARDED_JOB = re.compile( + r"(?ms)^ [A-Za-z0-9_-]+:\n" + r"(?:(?!^ [A-Za-z0-9_-]+:).)*?" + r"^ if: >-\n" + r" github\.event_name != 'pull_request_target' \|\|\n" + r" github\.event\.pull_request\.head\.repo\.full_name == github\.repository\n" + r"(?:(?!^ [A-Za-z0-9_-]+:).)*?" + r"^ runs-on:", +) + + +def test_secret_backed_pr_jobs_skip_forks_before_runner_selection(): + for workflow_name in PRIVILEGED_WORKFLOWS: + workflow = (WORKFLOW_DIR / workflow_name).read_text() + assert re.search(r"(?m)^ pull_request_target:\s*$", workflow) + assert re.search(r"(?m)^ workflow_dispatch:\s*$", workflow) + assert GUARDED_JOB.search(workflow), ( + f"{workflow_name} must guard its job before runs-on" + ) + assert "persist-credentials: false" in workflow + assert "github.event.pull_request.head.sha || github.ref" in workflow + + +def test_distributed_neon_template_uses_guarded_pull_request_target(): + template = (REPO_ROOT / "workflows" / "neon-before-after-validate.yml").read_text() + assert re.search(r"(?m)^ pull_request_target:\s*$", template) + assert re.search(r"(?m)^ workflow_dispatch:\s*$", template) + assert GUARDED_JOB.search(template) + assert "github.event.pull_request.head.sha || github.ref" in template + assert "persist-credentials: false" in template + assert "api_key: ${{ secrets.NEON_API_KEY }}" in template + + +def test_pr_safe_checks_is_hosted_and_secret_free(): + workflow = (WORKFLOW_DIR / "pr-safe-checks.yml").read_text() + + assert "name: PR Safe Checks" in workflow + assert "pull_request:" in workflow + assert "types: [opened, synchronize, reopened]" in workflow + assert "paths:" not in workflow + assert "runs-on: ubuntu-latest" in workflow + assert "permissions:\n contents: read" in workflow + assert "write" not in workflow + assert "self-hosted" not in workflow + assert "secrets." not in workflow + assert "NEON_API_KEY" not in workflow + assert "pull_request_target" not in workflow + assert "workflow_run" not in workflow + assert "persist-credentials: false" in workflow + assert "ref: ${{ github.event.pull_request.head.sha }}" in workflow + assert ( + "actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3" + in workflow + ) + assert "python-version: \"3.11\"" in workflow + assert "curl -LsSf https://astral.sh/uv/install.sh | sh" in workflow + assert 'echo "$HOME/.local/bin" >> "$GITHUB_PATH"' in workflow + assert "uv sync --frozen" in workflow + assert "testing/test_workflow_security.py" in workflow + assert "test_every_health_check_has_pgtap_coverage" in workflow + assert "test_both_view_sql_files_cover_all_health_checks" in workflow + assert "test_expected_check_groups_cover_all_defined_checks" not in workflow + + +def test_release_drafter_uses_hosted_runner(): + workflow = (WORKFLOW_DIR / "release-drafter.yml").read_text() + assert re.search(r"(?m)^ runs-on: ubuntu-latest\s*$", workflow) + assert "self-hosted" not in workflow diff --git a/workflows/neon-before-after-validate.yml b/workflows/neon-before-after-validate.yml index adbd875..43c589b 100644 --- a/workflows/neon-before-after-validate.yml +++ b/workflows/neon-before-after-validate.yml @@ -38,6 +38,9 @@ # (a) pass your SQL files via the `change_files` input, or # (b) leave it blank and apply changes outside this workflow # (you only need the pre- and post- comparison jobs). +# +# Note: fork pull requests skip the privileged job before runner assignment. +# The base workflow guard prevents fork SQL from running with NEON_API_KEY. name: Neon Before/After Validation @@ -70,6 +73,9 @@ env: jobs: before-after-validation: name: Before/After Health Validation + if: >- + github.event_name != 'pull_request_target' || + github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest timeout-minutes: 20 permissions: @@ -81,7 +87,8 @@ jobs: uses: actions/checkout@v4 with: fetch-depth: 1 - ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: false + ref: ${{ github.event.pull_request.head.sha || github.ref }} - name: Install PostgreSQL client run: | From 0c0bdb78e52207e081e1d0cf3df951b67a1ff5cf Mon Sep 17 00:00:00 2001 From: justin Date: Tue, 8 Sep 2026 21:46:40 -0600 Subject: [PATCH 2/4] Fix secure fork PR workflow boundaries --- .github/workflows/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 8f7eeab..cbdfa53 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -8,6 +8,11 @@ self-hosted NixOS runner. Installs `pg_stat_statements`, then `pgFirstAid.sql` + `view_pgFirstAid_managed.sql`, runs the pytest integration suite, and finishes with `seed_and_validate.py --managed`. +Fork pull requests skip the privileged job before runner assignment +because the job-level guard evaluates in the base workflow file; only +same-repository pull requests and owner-triggered `workflow_dispatch` +runs reach the self-hosted runner and the Neon secrets. + Required secrets (one set per PG version): `PG{15,16,17,18}_{HOST,PORT,USER,PASSWORD,DATABASE}`. From f60d830067f563d4e38341b51cdbe771c7f19d25 Mon Sep 17 00:00:00 2001 From: justin Date: Wed, 9 Sep 2026 19:28:33 -0600 Subject: [PATCH 3/4] Ignore and untrack local superpowers docs --- .gitignore | 2 +- .../2026-09-09-secure-fork-pr-ci-plan.md | 74 ------------------- .../2026-09-09-secure-fork-pr-ci-design.md | 53 ------------- 3 files changed, 1 insertion(+), 128 deletions(-) delete mode 100644 docs/superpowers/plans/2026-09-09-secure-fork-pr-ci-plan.md delete mode 100644 docs/superpowers/specs/2026-09-09-secure-fork-pr-ci-design.md diff --git a/.gitignore b/.gitignore index 790f5b5..c2d18a2 100644 --- a/.gitignore +++ b/.gitignore @@ -34,7 +34,7 @@ AGENTS.md CLAUDE.md # Local docs -docs/superpowers/plans/ +docs/superpowers/ # Python __pycache__/ diff --git a/docs/superpowers/plans/2026-09-09-secure-fork-pr-ci-plan.md b/docs/superpowers/plans/2026-09-09-secure-fork-pr-ci-plan.md deleted file mode 100644 index a8dbf44..0000000 --- a/docs/superpowers/plans/2026-09-09-secure-fork-pr-ci-plan.md +++ /dev/null @@ -1,74 +0,0 @@ -# Secure Fork PR Validation Implementation Plan - -**Goal:** Guard privileged pull-request workflows before runner assignment while retaining owner-operated validation and an always-on safe check for fork pull requests. - -**Spec:** `docs/superpowers/specs/2026-09-09-secure-fork-pr-ci-design.md` - -## Scope and constraints - -- Change only workflow YAML, workflow-security regression coverage, and this security design/plan documentation. -- Do not edit SQL files, pgTAP files, seed expectations, or `docs/health-checks/README.md`. -- Keep PR #44's `Unread Large Constraint-Backing Index`, pgTAP, seed, and health-check documentation work in PR #44 or a later follow-up. -- Do not change repository settings, rulesets, or dispatch behavior. -- Keep `workflow_dispatch` on all privileged database workflows. - -## Implementation steps - -### 1. Update regression coverage first - -Update `testing/test_workflow_security.py` to verify: - -- The three live secret-backed workflows use `pull_request_target` and `workflow_dispatch`. -- Each privileged job has the exact base-controlled repository guard before `runs-on`: - - ```yaml - if: >- - github.event_name != 'pull_request_target' || - github.event.pull_request.head.repo.full_name == github.repository - ``` - -- Privileged checkouts use the reviewed head SHA with a `github.ref` fallback and `persist-credentials: false`. -- The distributed Neon template has the same trigger, guard, checkout behavior, and no fork execution path. -- `PR Safe Checks` remains plain `pull_request`, hosted, secret-free, and runs only the workflow-security test and the two pure SQL coverage tests. -- `release-drafter.yml` uses `ubuntu-latest` and not the persistent self-hosted runner. - -Run the workflow-security test at this point and record its expected red result before implementation. - -### 2. Guard the three live privileged workflows - -In: - -- `.github/workflows/neon-before-after-validate.yml` -- `.github/workflows/neon-integration-pg-matrix.yml` -- `.github/workflows/nixos-local-test.yml` - -Change the automatic trigger to `pull_request_target`, preserve `workflow_dispatch`, and put the exact guard under the job before `runs-on`. Retain reviewed-head checkout behavior, add the `github.ref` fallback where needed, and keep `persist-credentials: false`. Replace stale comments about empty-secret failures with the statement that fork PRs skip the privileged job before runner assignment. - -### 3. Guard the distributed Neon template - -In `workflows/neon-before-after-validate.yml`, apply the same guarded `pull_request_target` and `workflow_dispatch` pattern. Use the reviewed head SHA with a dispatch fallback and `persist-credentials: false`. Keep the Neon secret only for trusted execution; the base-workflow guard must be false for fork heads before the job can run SQL or receive the secret. - -### 4. Make safe and release workflows hosted appropriately - -- Change `.github/workflows/release-drafter.yml` to `runs-on: ubuntu-latest`. -- Remove `testing/test_seed_and_validate.py::test_expected_check_groups_cover_all_defined_checks` from `.github/workflows/pr-safe-checks.yml` and from the workflow-security test's command assertions. -- Keep the workflow-security test and the two file-only SQL coverage tests in the safe workflow. - -### 5. Update design documentation - -Update the spec and this plan to describe guarded `pull_request_target`, the base-controlled fork boundary, the hosted release-drafter runner, and the reduced safe-check command. Explicitly state that PR #44's constraint-index, pgTAP, seed, and health-check documentation items are out of scope and belong to PR #44 or a later follow-up. - -### 6. Validate and commit - -Run: - -```bash -uv run pytest -q testing/test_workflow_security.py -uv run pytest -q \ - testing/integration/tests/integration/test_pgtap_suite.py::test_every_health_check_has_pgtap_coverage \ - testing/integration/tests/integration/test_pgtap_suite.py::test_both_view_sql_files_cover_all_health_checks -git diff --check -git status --short --branch -``` - -Review the diff to confirm no SQL, pgTAP, seed, health-check README, repository settings, or PR #44 files changed. Commit the completed fix round locally on `fix/secure-fork-pr-ci`. diff --git a/docs/superpowers/specs/2026-09-09-secure-fork-pr-ci-design.md b/docs/superpowers/specs/2026-09-09-secure-fork-pr-ci-design.md deleted file mode 100644 index b3a14b1..0000000 --- a/docs/superpowers/specs/2026-09-09-secure-fork-pr-ci-design.md +++ /dev/null @@ -1,53 +0,0 @@ -# Secure Fork PR Validation Design - -## Goal - -Keep pull-request validation available to fork contributors without allowing fork-controlled code to access repository database credentials, `NEON_API_KEY`, or persistent self-hosted runners. - -## Current problem - -The live secret-backed database workflows run on persistent self-hosted runners and previously used `pull_request` with empty-secret validation inside the job. A fork job could therefore be assigned a persistent runner before that validation failed. The distributed Neon template also needs the same explicit trust boundary. - -## Design - -1. The three live secret-backed workflows and the distributed Neon template use `pull_request_target` plus `workflow_dispatch`. -2. Each privileged job has this base-controlled guard before `runs-on`: - - ```yaml - if: >- - github.event_name != 'pull_request_target' || - github.event.pull_request.head.repo.full_name == github.repository - ``` - - Fork pull requests therefore skip the job before runner assignment. Manual dispatch remains available for owner-operated validation, and same-repository pull requests remain eligible. -3. Privileged checkouts use the reviewed pull-request head SHA, with `github.ref` as the dispatch fallback, and set `persist-credentials: false`. The guarded jobs are the only workflows that consume database or Neon secrets. -4. `PR Safe Checks` remains a secret-free plain `pull_request` workflow on `ubuntu-latest`. It checks out the PR head without persisted credentials and runs workflow-security plus the two pure SQL coverage contracts. -5. `release-drafter.yml` runs on `ubuntu-latest`; its pull-request event does not require Nix or a persistent runner. - -## Trust boundaries - -A fork pull request can run only the secret-free hosted checks. Its privileged jobs evaluate false in the base workflow before runner selection, so fork SQL is not checked out or executed and `NEON_API_KEY` is not exposed to fork execution. Trusted database validation runs only for same-repository pull requests or an explicit owner-triggered dispatch. - -The distributed Neon template retains its Neon secret references for trusted execution, but its base-workflow guard is false for fork heads. No workflow uses `workflow_run` to check out a fork commit with secrets. - -## Status checks - -The safe workflow reports a stable advisory check without changing repository rulesets. Its test command includes the workflow-security test and the two file-only SQL coverage tests. The pre-existing failing static seed expectation test is intentionally not part of this security PR's safe-check command; it remains outside this branch's scope. - -## Out of scope - -PR #44's `Unread Large Constraint-Backing Index` check, pgTAP assertions, seed expectations, and health-check documentation are not part of this separate security PR. Those items belong to PR #44 or a later follow-up. This branch does not edit SQL, pgTAP, seed expectations, or `docs/health-checks/README.md`. - -## Validation - -- `testing/test_workflow_security.py` verifies guarded `pull_request_target` jobs, reviewed-head checkouts, the secret-free plain `pull_request` workflow, and the hosted release-drafter runner. -- The two pure SQL coverage tests run without a database. -- `git diff --check` and workflow inspection verify whitespace and scope. - -## Rejected alternatives - -- Do not use an unguarded `pull_request_target` job for privileged workflows. -- Do not retain empty-secret validation as the fork safety boundary. -- Do not run fork-controlled code on persistent self-hosted runners. -- Do not add a `workflow_run` follower that checks out a fork commit and runs it with secrets. -- Do not alter repository settings or mix PR #44's health-check contract changes into this security PR. From bdad54e769a9f0eeacb8c4f6a4e1c1cc523994f0 Mon Sep 17 00:00:00 2001 From: justin Date: Wed, 9 Sep 2026 21:05:25 -0600 Subject: [PATCH 4/4] fix(ci): add trusted pr-workflow-guard and pin pr-safe-checks uv --- .github/workflows/pr-safe-checks.yml | 13 +++-- .github/workflows/pr-workflow-guard.yml | 66 ++++++++++++++++++++++++ testing/test_workflow_security.py | 68 ++++++++++++++++++++++++- 3 files changed, 140 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/pr-workflow-guard.yml diff --git a/.github/workflows/pr-safe-checks.yml b/.github/workflows/pr-safe-checks.yml index a3ff659..06b5456 100644 --- a/.github/workflows/pr-safe-checks.yml +++ b/.github/workflows/pr-safe-checks.yml @@ -11,22 +11,25 @@ jobs: safe-checks: name: PR Safe Checks runs-on: ubuntu-latest + timeout-minutes: 10 steps: - name: Checkout PR head - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ github.event.pull_request.head.sha }} persist-credentials: false - name: Set up Python - uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 + uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0 with: python-version: "3.11" - name: Install uv - run: | - curl -LsSf https://astral.sh/uv/install.sh | sh - echo "$HOME/.local/bin" >> "$GITHUB_PATH" + uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + with: + version: "0.12.9" + checksum: "ec7a99cd05e0cd7f80243f135ce1361c76835cb0ee60055d14d20eba8eba1460" + enable-cache: true - name: Sync dependencies run: uv sync --frozen diff --git a/.github/workflows/pr-workflow-guard.yml b/.github/workflows/pr-workflow-guard.yml new file mode 100644 index 0000000..096a842 --- /dev/null +++ b/.github/workflows/pr-workflow-guard.yml @@ -0,0 +1,66 @@ +name: Workflow Security Guard + +on: + pull_request_target: + types: [opened, synchronize, reopened] + +permissions: + contents: read + +jobs: + workflow-security: + name: Workflow Security Guard + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout base + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0 + with: + python-version: "3.11" + + - name: Install uv + uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + with: + version: "0.12.9" + checksum: "ec7a99cd05e0cd7f80243f135ce1361c76835cb0ee60055d14d20eba8eba1460" + enable-cache: true + + - name: Sync dependencies + run: uv sync --frozen + + - name: Fetch PR workflow files as data + env: + GH_TOKEN: ${{ github.token }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + mkdir -p /tmp/pr-workflows /tmp/pr-distributed + for f in \ + pr-safe-checks.yml \ + pr-workflow-guard.yml \ + neon-before-after-validate.yml \ + neon-integration-pg-matrix.yml \ + nixos-local-test.yml; do + gh api \ + "repos/${HEAD_REPO}/contents/.github/workflows/${f}?ref=${HEAD_SHA}" \ + --jq .content | base64 -d > "/tmp/pr-workflows/${f}" + done + gh api \ + "repos/${HEAD_REPO}/contents/workflows/neon-before-after-validate.yml?ref=${HEAD_SHA}" \ + --jq .content | base64 -d > /tmp/pr-distributed/neon-before-after-validate.yml + + - name: Overlay PR YAML for verifier + run: | + set -euo pipefail + cp /tmp/pr-workflows/*.yml .github/workflows/ + mkdir -p workflows + cp /tmp/pr-distributed/neon-before-after-validate.yml workflows/neon-before-after-validate.yml + + - name: Run trusted verifier + run: uv run pytest -q testing/test_workflow_security.py diff --git a/testing/test_workflow_security.py b/testing/test_workflow_security.py index 469d45f..f2c4c2f 100644 --- a/testing/test_workflow_security.py +++ b/testing/test_workflow_security.py @@ -66,8 +66,6 @@ def test_pr_safe_checks_is_hosted_and_secret_free(): in workflow ) assert "python-version: \"3.11\"" in workflow - assert "curl -LsSf https://astral.sh/uv/install.sh | sh" in workflow - assert 'echo "$HOME/.local/bin" >> "$GITHUB_PATH"' in workflow assert "uv sync --frozen" in workflow assert "testing/test_workflow_security.py" in workflow assert "test_every_health_check_has_pgtap_coverage" in workflow @@ -79,3 +77,69 @@ def test_release_drafter_uses_hosted_runner(): workflow = (WORKFLOW_DIR / "release-drafter.yml").read_text() assert re.search(r"(?m)^ runs-on: ubuntu-latest\s*$", workflow) assert "self-hosted" not in workflow + + +def test_pr_workflow_guard_is_trusted_base_only(): + workflow = (WORKFLOW_DIR / "pr-workflow-guard.yml").read_text() + + assert "name: Workflow Security Guard" in workflow + assert re.search(r"(?m)^ pull_request_target:\s*$", workflow) + assert re.search(r"(?m)^ types: \[opened, synchronize, reopened\]\s*$", workflow) + assert "permissions:\n contents: read" in workflow + assert "runs-on: ubuntu-latest" in workflow + assert re.search( + r"(?m)^ name: Workflow Security Guard\s*$", workflow + ) + assert "self-hosted" not in workflow + assert "secrets." not in workflow + assert "workflow_run" not in workflow + assert "astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9" in workflow + assert "version: \"0.12.9\"" in workflow + assert ( + "checksum: \"ec7a99cd05e0cd7f80243f135ce1361c76835cb0ee60055d14d20eba8eba1460\"" + in workflow + ) + assert "enable-cache: true" in workflow + assert "persist-credentials: false" in workflow + # Guard must checkout base, not the PR head. + assert "ref: ${{ github.event.pull_request.head.sha }}" not in workflow + # Guard must fetch PR files via the API as data, not by checking out PR head. + assert "gh api" in workflow + assert "repos/${HEAD_REPO}/contents/" in workflow + # Guard must run the trusted verifier, not PR-supplied code. + assert "uv run pytest -q testing/test_workflow_security.py" in workflow + # Guard must never copy PR Python or other test sources from the fetch result. + assert "cp /tmp/pr-workflows/*.py" not in workflow + assert "cp -r /tmp/pr-workflows/." not in workflow + assert "rsync -a /tmp/pr-workflows" not in workflow + + +def test_pr_workflow_guard_overlay_replaces_yaml_only(): + workflow = (WORKFLOW_DIR / "pr-workflow-guard.yml").read_text() + # The overlay step must copy YAML only. No .py, no testing/*, no pgTAP, no seed. + overlay_run = workflow.split("Overlay PR YAML for verifier", 1)[1].split( + "run: |\n", 1 + )[1].split("\n - name:", 1)[0] + assert ".py" not in overlay_run + assert ".sql" not in overlay_run + assert "testing/" not in overlay_run + assert "pgTAP" not in overlay_run + assert "seed" not in overlay_run + assert "cp /tmp/pr-workflows/*.yml .github/workflows/" in workflow + assert ( + "cp /tmp/pr-distributed/neon-before-after-validate.yml workflows/neon-before-after-validate.yml" + in workflow + ) + + +def test_pr_safe_checks_no_longer_pipes_curl_to_sh(): + workflow = (WORKFLOW_DIR / "pr-safe-checks.yml").read_text() + assert "curl -LsSf https://astral.sh/uv/install.sh | sh" not in workflow + assert 'echo "$HOME/.local/bin" >> "$GITHUB_PATH"' not in workflow + assert "astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9" in workflow + assert "version: \"0.12.9\"" in workflow + assert ( + "checksum: \"ec7a99cd05e0cd7f80243f135ce1361c76835cb0ee60055d14d20eba8eba1460\"" + in workflow + ) + assert "enable-cache: true" in workflow