Repository navigation
ci: skip the preview deploy on fork PRs too - #517
Conversation
The deploy job was guarded on `github.actor != 'dependabot[bot]'`, which covers the case #398 reported but not the other one that lands here with no credentials. Fork PRs lose the secrets for the same reason and by design — this workflow is plain `pull_request`, so a fork head never sees them — but the actor on those runs is the contributor, not dependabot, so the job starts and fails exactly as the dependabot runs used to: wrangler aborts on an empty CLOUDFLARE_API_TOKEN, and Comment Preview URL then fails too, since GitHub downgrades the job's write permission to read on a fork run whatever the permissions block asks for. Adding the head-repo guard the other same-repo-only workflows already use (ai-scan, ai-triage, claude-review, story-screenshots) turns that red X into a skip. Theirs carry an extra `pull_request == null` branch because they also run on issues events; this workflow only runs on pull_request, so the PR object is always there. Both clauses only ever narrow what runs. The trigger is untouched and stays plain `pull_request` — the fork run has no secrets to reach in the first place, which is the property this preserves rather than works around. Unvalidatable until an outside PR arrives, same caveat #398 carried: a secret-less run cannot be simulated from a same-repo branch, where both clauses are true and the preview must still deploy as before. Closes #398
|
Warning Review limit reached
Next review available in: 49 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe deployment job now excludes Dependabot pull requests and pull requests from fork repositories. Workflow comments document the related secret and permission limitations. ChangesDeployment guards
Estimated code review effort: 2 (Simple) | ~5 minutes Merge Risk: ⚪ Minimal · up to The workflow now avoids starting preview deployments that cannot access required secrets on fork pull requests while preserving previews for same-repository changes. No actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Prevents fork PRs from attempting credentialed preview deployments while preserving build validation.
Changes:
- Adds a same-repository guard to the deployment job.
- Documents fork security behavior.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # `permissions:` block below says. Same idiom as ai-scan.yml, | ||
| # ai-triage.yml, claude-review.yml and story-screenshots.yml; they carry an | ||
| # extra `pull_request == null` branch only because they also run on issues. |
There was a problem hiding this comment.
Correct, and fixed in 3d87464. Verified each of the four: ai-scan.yml (issues + pull_request) and ai-triage.yml (pull_request + issues) carry the null branch; claude-review.yml (pull_request only) and story-screenshots.yml (workflow_dispatch + pull_request) use the bare head-repo check, which is the form this job copies.
The comment now names the two groups separately. Worth fixing at any size, since a security comment that misdescribes its own mechanism is what stops the next reader verifying it.
Preview DeploymentPreview URL: https://4f513156.bestax.pages.dev |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — 0 blocking · 1 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | Robustness | The in-file comment says all four cited workflows "carry an extra pull_request == null branch only because they also run on issues" — but only ai-scan.yml/ai-triage.yml do; claude-review.yml and story-screenshots.yml neither run on issues nor carry that branch (the same idiom this PR copies from claude-review.yml directly). |
.github/workflows/test-deploy.yml:96-98 |
Overall: The change is sound and low-risk. It adds a single &&-clause head-repo guard (github.event.pull_request.head.repo.full_name == github.repository) to the test-deploy job's if:, which only ever narrows what runs — a fork PR that today reproduces #398's failure (wrangler on an empty CLOUDFLARE_API_TOKEN, then a Comment Preview URL step that can't write on a fork run) now skips instead. The idiom is verified byte-for-byte identical to claude-review.yml:50, the trigger stays plain pull_request (rule 7 preserved — no pull_request_target, no permissions:/allowlist/SHA change), and prettier --check passes (parsing the YAML as a side effect). The riskiest thing here is nothing structural — it's the fork path staying empirically unvalidated until an outside PR lands, exactly the caveat #398 already carried. The human should focus first on confirming this PR's own run (same-repo, human actor -> both clauses true) still deploys and posts a preview URL; that is the regression test that matters.
Residual risk:
- Other secret-less
pull_requestruns slipping through — GitHub scopes secrets away in only two cases: Dependabot's separate secret store (covered by the pre-existinggithub.actor != 'dependabot[bot]'clause) and fork heads (covered by the new guard). No third secret-less path exists on plainpull_request; other same-repo bot/human actors get the Actions store and deploy correctly, which is intended. - Deleted-fork edge (
head.reponull) — if the fork repo is gone,github.event.pull_request.head.repo.full_nameresolves to null, which!= github.repository, so the job skips — the safe direction. - Skip vs. required-check — this job is not in the
mainruleset's required set (Build and Test, React 18/19, Dependency Review), so a fork PR skipping it does not wedge merge;buildstill runs and catches build breakage. The comment already flags the "move the gate to steps if ever promoted to required" follow-up.
🏄 Chill little one-line patch, dude — it just tells the deploy job to stay on the beach when there's no secrets in the water, same wave every other same-repo workflow is already riding. Totally good to go; only ripple is a comment that name-drops two workflows that don't actually carry the branch it says they do. No worries, ship it.
Copilot and the deep review both caught the same overstatement. The comment said the four cited workflows carry an extra `pull_request == null` branch because they also run on issues, but only ai-scan and ai-triage do — claude-review and story-screenshots run on pull_request alone and use the bare head-repo check, which is the form this job copies. Small as it is, a wrong claim in a security comment is the failure the review checklist ends on: the next reader stops verifying once a comment has misdescribed its own mechanism.
Review round 1 addressed — 3d87464Deep review advisory 1 / Copilot inline ( The comment claimed all four cited workflows carry an extra
The comment now names the two groups separately and says which form this job copies (the bare one, from Small, but worth a commit rather than a shrug: the review checklist in No other findings. The deep review reported 0 blocking, CodeRabbit generated no actionable comments, and its three residual-risk notes are observations I agree with rather than requests — in particular that a deleted fork head resolves The regression test that mattered already passed on the previous head: same-repo, human actor, so both clauses are true and Test and Preview Deployment ran and deployed (22s), posting the preview URL comment above. This push re-runs it. |
Preview DeploymentPreview URL: https://0145d1a6.bestax.pages.dev |
|
🎉 This PR is included in version 5.11.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 4.1.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What #398 asked for, and what is left of it
#398 reported that Test and Preview Deployment went red on every dependabot PR: those runs are scoped to the Dependabot secrets store, so
secrets.CLOUDFLARE_API_TOKENinterpolates empty andwrangler-actionaborts. The build passed first, so the red X was pure noise that reads as a merge blocker while triaging.That symptom already shipped fixed, by a different mechanism than the issue proposed:
github.actor != 'dependabot[bot]'.Confirmed live on the three dependabot PRs opened since — #511, #510, #469:
Build preview sitepasses,Test and Preview Deploymentreports skipping, and each run concludes success. The job comment notes the skipped-not-green tradeoff is fine while the check is not required; still true, themainruleset requires only Build and Test, React 18 compatibility, React 19 compatibility, and Dependency Review.The case the actor test does not reach
Fork PRs arrive with no secrets too, for the same reason and by design — this workflow is plain
pull_request, so a fork head never sees them. But the actor on a fork run is the contributor rather than dependabot, so the job starts and reproduces #398's failure exactly: wrangler on an empty token, thenComment Preview URL, whose write permission GitHub downgrades to read on a fork run whatever thepermissions:block asks for.This PR adds the head-repo guard the other same-repo-only workflows already use (
ai-scan.yml,ai-triage.yml,claude-review.yml,story-screenshots.yml). Theirs carry an extrapull_request == nullbranch only because they also run onissuesevents; this one runs onpull_requestalone, so the PR object is always present.Security notes
pull_requestonmain, nopull_request_target. Rule 7 in.github/CLAUDE.mdis what makes the fork run secret-less in the first place, and this preserves that rather than working around it.permissions:change, no allowlist change, no action SHA change, no new repository variable, no verdict or budget path.Verification
pnpm exec prettier --checkpasses on the file, which parses the YAML as a side effect. The repo has no actionlint.Closes #398
Summary by CodeRabbit