Clarify review app security guidance#752
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughDocumentation enhancements clarify review-app deployment security across Control Plane configuration and GitHub workflow guidance: staging/review tokens must not access production resources, fork PR deployments are restricted due to Docker build secrets, and review-app secret dictionaries must use only disposable/review-safe credentials, never reusing production values. ChangesReview-App Deployment Safety and Credential Guidance
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🚀 Quick Review App CommandsWelcome! Here are the commands you can use in this PR:
|
Greptile SummaryThis documentation-only PR adds security guidance across five Control Plane and GitHub Actions markdown files, clarifying how review-app secrets are scoped and why public fork PR heads are skipped during deploys.
Confidence Score: 4/5Documentation-only change with accurate, well-scoped guidance; safe to merge after fixing the missing blank line in readme.md. All five files contain only comment and documentation updates — no workflow logic, template values, or secrets are modified. The guidance added is internally consistent and matches best practices for isolating review-app credentials. The only defect is a missing blank line in readme.md that causes two distinct paragraphs to merge when rendered. .controlplane/readme.md — the new fork-PR paragraph runs directly into the next paragraph without a blank line separator, which collapses them in rendered Markdown. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PR pushed] --> B{Fork PR?}
B -- Yes --> C[Skip deploy\nrepo secrets unavailable]
B -- No --> D{Trusted branch?}
D -- Yes --> E[Build Docker image\nusing repo secrets]
D -- No / Untrusted --> C
E --> F[Deploy review app]
F --> G[Mount cpln://secret/...\nvalues into workload]
G --> H{Secrets review-safe?}
H -- Yes\ndisposable DB, review token --> I[PR code runs safely]
H -- No\nprod secrets reused --> J[⚠️ Secrets exposed\nto PR code]
Reviews (1): Last reviewed commit: "docs: clarify review app security" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| automatically. Use `+review-app-help` for the review-app command reference. | ||
| Fork PR heads are skipped for deploys because the workflow builds Docker images | ||
| with repository secrets. A trusted comment on a fork PR still should not deploy | ||
| the fork head; move the reviewed change to a branch in this repository when a | ||
| review app is needed. | ||
| Pushes to the staging branch deploy staging, and production promotion is manual |
There was a problem hiding this comment.
| automatically. Use `+review-app-help` for the review-app command reference. | |
| Fork PR heads are skipped for deploys because the workflow builds Docker images | |
| with repository secrets. A trusted comment on a fork PR still should not deploy | |
| the fork head; move the reviewed change to a branch in this repository when a | |
| review app is needed. | |
| Pushes to the staging branch deploy staging, and production promotion is manual | |
| automatically. Use `+review-app-help` for the review-app command reference. | |
| Fork PR heads are skipped for deploys because the workflow builds Docker images | |
| with repository secrets. A trusted comment on a fork PR still should not deploy | |
| the fork head; move the reviewed change to a branch in this repository when a | |
| review app is needed. | |
| Pushes to the staging branch deploy staging, and production promotion is manual |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| Fork PR heads are skipped for deploys because the workflow builds Docker images | ||
| with repository secrets. A trusted comment on a fork PR still should not deploy | ||
| the fork head; move the reviewed change to a branch in this repository when a | ||
| review app is needed. |
There was a problem hiding this comment.
Missing blank line between this new paragraph and the following "Pushes to the staging branch…" paragraph. Without it, Markdown renders them as a single run-on paragraph.
| review app is needed. | |
| review app is needed. | |
Code Review: Clarify review-app security guidanceSummary: Documentation-only PR adding security guidance around review-app secrets, fork PR deploy restrictions, and SSH key best practices. No application, workflow, or infrastructure code is changed. What the PR does well
IssuesFormatting bug — missing blank line (inline comment posted)In Ambiguous language: "still should not deploy"
"Should not" reads like a recommendation, but the intent appears to be that the workflow enforces this — fork heads are actively skipped, not merely discouraged. Consider:
This avoids implying that someone could override the behaviour if they chose to.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
✅ Review App DeletedReview app for PR #752 is deleted |
1 participant
Summary
Verification
git diff --checkNote
Low Risk
Markdown and template comment changes only; no application, workflow, or infrastructure behavior is modified.
Overview
Documentation-only updates across Control Plane and cpflow help docs to spell out review-app security expectations.
Fork PRs and deploys: Public fork PR heads do not get review-app deploys because Docker builds use repository secrets; contributors should land changes on a trusted branch in this repo when a review app is needed. Comment-triggered deploys on fork PRs still should not run untrusted fork code.
Secrets and tokens: Staging/review
CPLN_TOKEN_STAGINGmust not reach production Control Plane orgs or secret dictionaries. Review apps execute PR code, so values mounted viacpln://secret/...(sharedqa-*dictionaries, renderer credentials, DB URLs, Pro license) must be review-safe and must not reuse production or long-lived staging secrets. Template comments inapp.ymlandorg.ymlreinforce the same rules.Build keys:
DOCKER_BUILD_SSH_KEYguidance now calls for a read-only, revocable deploy key instead of a personal SSH key.Reviewed by Cursor Bugbot for commit 5945332. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit