Skip to content

fix(ci): harden ingestion-autopilot workflow to avoid service-role key exposure - #1572

Merged
BigSimmo merged 8 commits into
mainfrom
codex/fix-manual-workflow-service-role-key-exposure
Aug 4, 2026
Merged

fix(ci): harden ingestion-autopilot workflow to avoid service-role key exposure#1572
BigSimmo merged 8 commits into
mainfrom
codex/fix-manual-workflow-service-role-key-exposure

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Prevent a critical secret-exposure vulnerability where manually-dispatched runs could execute checked-out branch code while SUPABASE_SERVICE_ROLE_KEY was in job env, allowing same-repo write-capable attackers to exfiltrate or misuse the production service-role key.

Description

  • Remove SUPABASE_SERVICE_ROLE_KEY from the workflow/job-level env so checkout and install steps no longer inherit the production service-role secret.
  • Restrict manual workflow_dispatch usage by adding if: ${{ github.event_name != 'workflow_dispatch' || github.ref_name == github.event.repository.default_branch }} and pinning the checkout to ref: ${{ github.event.repository.default_branch }} so only the trusted default-branch code runs before secrets are introduced.
  • Limit the secret to the privileged steps by injecting SUPABASE_SERVICE_ROLE_KEY only into the preflight check step and the Run autopilot step after dependencies are installed.
  • Add a regression test tests/ingestion-autopilot-workflow.test.ts that asserts the workflow does not expose the service-role key to checkout/install steps and that the autopilot step contains the scoped secret.

Testing

  • Ran npm ci (using Node 24 in PATH) and dependency installation completed successfully.
  • Ran npm run check:github-actions and the workflow pin checks passed.
  • Ran git diff --check and found no whitespace/pin issues.
  • Ran the new regression test with npm run test -- tests/ingestion-autopilot-workflow.test.ts and it passed.
  • Ran the full unit suite npm run test; the newly added workflow test passed but an unrelated pre-existing test (tests/pdf-extraction-budget.test.ts) failed, so the full suite did not finish green in this environment.
  • Provider-backed checks that validate live Supabase or release readiness (npm run check:supabase-project, npm run verify:release, etc.) were intentionally not run because they require explicit confirmation and live credentials.

Codex Task

Summary by CodeRabbit

  • Improvements

    • Improved ingestion recovery authorization through repository dispatch events and an explicit apply flag.
    • Limited sensitive service credentials to the specific workflow steps that require them.
    • Retained the scheduled six-hourly health probe and clarified dry-run messaging.
  • Tests

    • Added automated coverage for dispatch behavior, authorization wiring, and credential handling.
    • Included the new workflow validation in the CI test suite.
  • Documentation

    • Recorded the workflow security and validation updates in the review ledger.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 39 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e1ac6bcf-3fac-48e7-9669-e7a05b04d2de

📥 Commits

Reviewing files that changed from the base of the PR and between 23d0e79 and f3b99dc.

📒 Files selected for processing (2)
  • docs/branch-review-ledger.md
  • tests/ingestion-autopilot-workflow.test.ts
📝 Walkthrough

Walkthrough

The ingestion-autopilot workflow now uses repository dispatch, reads recovery authorization from the event payload, and scopes the service-role secret to two steps. New Vitest coverage validates these rules, and CI runs the test.

Changes

Ingestion autopilot hardening

Layer / File(s) Summary
Dispatch trigger and recovery authorization
.github/workflows/ingestion-autopilot.yml
The workflow uses the ingestion-autopilot repository dispatch event. Recovery reads client_payload.apply and defaults to "false".
Step-scoped service-role secret
.github/workflows/ingestion-autopilot.yml
The Supabase service-role secret is passed only to the preflight and autopilot steps.
Workflow assertions and CI registration
tests/ingestion-autopilot-workflow.test.ts, package.json, docs/branch-review-ledger.md
Tests validate dispatch configuration and secret placement. The CI test command includes the new test. The review ledger records the changes and validation results.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: cursoragent

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers motivation, implementation, and testing, but it omits the required Risk and rollout, Clinical Governance Preflight, and Notes sections. Add the required Risk and rollout and Clinical Governance Preflight sections, including rollback, production effects, and applicable governance confirmations.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: hardening the ingestion-autopilot workflow to prevent service-role key exposure.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@supabase

supabase Bot commented Aug 2, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

CI triage

CI failed on this PR. Automated classification of the 3 failed job(s):

  • Unit coverageneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • Static PR checksneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #7797 (cancelled).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

@BigSimmo

BigSimmo commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

@cursoragent Please review PR and fix all issues in this draft PR

@cursor

cursor Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Reviewed and fixed PR #1572. Tip is now ace81d84.

Findings

  • P1 (CI): New tests/ingestion-autopilot-workflow.test.ts wasn’t listed in test:ci-workflows → Static PR checks / Unit coverage failed.
  • P1 (CI): Branch was ~1144 commits behind main, so Gitleaks couldn’t find scripts/run-gitleaks-pinned.mjs.
  • P2 (CI): Prettier needed to reformat the new test (autofix.ci then failed on push auth).
  • P2 (test gap): Original assertion only blocked workflow-level env; job-level exposure could slip through.
  • Security change: Looks correct — no remaining P0/P1. Secret is step-scoped; dispatch limited to default branch; checkout pinned to default branch.

Fixes

  • Merged origin/main and kept hardening on the current autopilot workflow (v7 action pins, npm ci --include=dev, --alert-on-stuck).
  • Registered the suite in test:ci-workflows.
  • Strengthened secret-scoping assertions + Prettier.
  • Recorded the review in docs/branch-review-ledger.md.

Verification

  • npm run test:ci-workflows206/206
  • Focused workflow/cache tests — 21/21
  • check:github-actions, Prettier, git diff --check — pass
  • No OpenAI/Supabase/provider calls

Residual risk (intentional): workflow_dispatch from non-default branches can’t dry-run this workflow until it lands on main.

Open in Web Open in Cursor 

Add the new workflow-reading regression suite to test:ci-workflows so
Static PR checks pass, tighten the secret-scoping assertions, and apply
Prettier so autofix.ci does not need to rewrite the file.
Capture the reviewed HEAD, CI fixes, and verification after syncing the
draft PR with main and registering the workflow secret contract.
@BigSimmo
BigSimmo marked this pull request as ready for review August 4, 2026 07:25

@devin-ai-integration devin-ai-integration Bot left a comment

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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ace81d8494

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/ingestion-autopilot.yml Outdated
@BigSimmo
BigSimmo enabled auto-merge (squash) August 4, 2026 08:36

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/branch-review-ledger.md`:
- Line 591: Update the ledger entry for PR `#1572` to include the decisive output
lines from both check:github-actions and prettier --check, preserving the
existing gate names and recording their actual results. Regenerate the entry
through the ledger workflow rather than editing it manually.

In `@tests/ingestion-autopilot-workflow.test.ts`:
- Around line 22-37: Update the workflow assertions around the Checkout and
Preflight required secrets blocks to validate each block independently: assert
Checkout excludes SUPABASE_SERVICE_ROLE_KEY, and assert Preflight required
secrets contains the expected mapping. Do not rely on the broader
install-to-autopilot slice, which can pass if the mapping moves into Checkout.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4aae4379-badc-44f1-9a1c-5335844ded55

📥 Commits

Reviewing files that changed from the base of the PR and between 67f0eb8 and 23d0e79.

📒 Files selected for processing (4)
  • .github/workflows/ingestion-autopilot.yml
  • docs/branch-review-ledger.md
  • package.json
  • tests/ingestion-autopilot-workflow.test.ts

Comment thread docs/branch-review-ledger.md
Comment thread tests/ingestion-autopilot-workflow.test.ts
@BigSimmo
BigSimmo disabled auto-merge August 4, 2026 08:54

BigSimmo commented Aug 4, 2026

Copy link
Copy Markdown
Owner Author

Resolved both fresh review findings: Checkout and Preflight are now asserted independently in the workflow regression test (focused test passes), and the historical ledger row is superseded through ledger:append with explicit PASS outputs for each named gate.

@BigSimmo
BigSimmo enabled auto-merge (squash) August 4, 2026 08:56
@BigSimmo
BigSimmo merged commit 0e8ba7e into main Aug 4, 2026
23 checks passed
@BigSimmo
BigSimmo deleted the codex/fix-manual-workflow-service-role-key-exposure branch August 4, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants