Clarify review app security guidance#350
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 (2)
WalkthroughThis PR updates two documentation files to enhance guidance around review-app deployments in public repositories. It establishes security constraints for fork PRs, revises secret token requirements, adds a comprehensive review-app security section, and clarifies safe secret storage using Control Plane references and scoped credentials. ChangesReview App Security for Public Repositories
Sequence Diagram(s)(Diagrams are embedded in the hidden review stack artifact above.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
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)
⚔️ Resolve merge conflicts
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 |
coderabbitai
Bot
added
documentation
Greptile SummaryThis PR adds security guidance for review apps in public repositories, explaining why fork PR heads are skipped during deploy, clarifying that trusted
Confidence Score: 4/5Documentation-only changes with accurate security guidance; safe to merge. All changes are documentation. The security claims are accurate and internally consistent, with one bullet point in the new security section using an "unless" clause that reads differently from the clearer prose elsewhere in the file — a reader could interpret it as an automatic workflow check rather than a manual workaround. The one bullet at line 209 of Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PR opened or push] --> B{Is PR head from\nbase repository?}
B -- Yes\n(same-repo branch) --> C[Review app deploy eligible]
B -- No\n(fork head) --> D[Skip deploy\nPost help comment only]
C --> E{Trigger: push or\n+review-app-deploy comment?}
E -- Push --> F{Is review app\nalready active?}
F -- Yes --> G[Auto-redeploy]
F -- No --> H[Skip until first comment]
E -- Comment --> I{Is commenter\nOWNER / MEMBER /\nCOLLABORATOR?}
I -- No --> J[Ignore comment]
I -- Yes --> K[Create or update review app]
K --> L{Are secrets disposable?\nCPLN_TOKEN_STAGING\nnot prod-scoped?}
L -- Yes --> M[Safe review-app deploy]
L -- No --> N[⚠️ Credential exposure risk\nRotate and scope secrets]
D --> O[Maintainer workaround:\nmove change to base-repo branch\nthen open new PR]
O --> C
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}
| - same-repository pull requests can create or update review apps; | ||
| - fork pull requests are skipped for review-app deploys; | ||
| - `+review-app-deploy` and `+review-app-delete` are accepted only from trusted commenters; | ||
| - trusted comments still cannot deploy fork heads unless the branch exists in the base repository; |
There was a problem hiding this comment.
The bullet reads "trusted comments still cannot deploy fork heads unless the branch exists in the base repository," which can be parsed as an automatic condition check — as if the workflow detects the branch name in the base repo and switches behavior. The clearer phrasing used a few lines later in the same file ("move the change to a branch in the base repository if it needs a review app") makes it explicit that this is a manual workaround. Aligning the bullet with the prose avoids the implication that the workflow itself checks whether the branch exists in both repos.
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!
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09f553d9a9
ℹ️ 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".
| - for open-source repositories, use a dedicated review-app org or a tightly scoped staging token so review-app builds | ||
| cannot reveal production or long-lived staging secrets |
There was a problem hiding this comment.
When a public repo follows this recommendation and provisions a dedicated review-app org/token, the generated workflows do not have separate review-app org settings: review deploys use vars.CPLN_ORG_STAGING/secrets.CPLN_TOKEN_STAGING (.github/workflows/cpflow-deploy-review-app.yml lines 29 and 201-202), and staging deploy/promotion require the same staging token/org (.github/workflows/cpflow-deploy-staging.yml lines 105-106; .github/workflows/cpflow-promote-staging-to-production.yml lines 72-82). A review-only token would therefore break staging/promotion or, if the staging token is kept, review apps still deploy into the staging org, so this doc should either say the generated flow shares the staging org or call out the required workflow customization.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Summary
+review-app-deploycomments still require a branch in the base repositorycpln://secretruntime exposureVerification
git diff --checkscript/check_cpln_linksscript/check_command_docsNote
Low Risk
Documentation-only; no runtime, workflow, or secret configuration changes.
Overview
Documentation-only updates to
docs/ci-automation.mdanddocs/secrets-and-env-values.mdthat spell out review-app security for public and open-source repos—no workflow or generator changes.ci-automation.mdnow states upfront that review deploys only run for base-repo branches (fork PRs get help but not deploys because builds use repo secrets), and that maintainers must land fork work on a trusted branch for a review app. It expandsCPLN_TOKEN_STAGINGguidance (prefer a review/staging token that cannot touch production),DOCKER_BUILD_SSH_KEYcautions (minimal read-only deploy keys), and a new Review App Security For Public Repositories section covering trusted commenters, fork skips, disposable credentials, scoped tokens, separate data stores, rotation, and cleanup. Workflow descriptions and the demo rollout checklist add matching bullets (including verifying staging tokens cannot reach production).secrets-and-env-values.mdadds that review apps execute PR code, socpln://secret/...does not protect values once mounted, and recommends dummy/disposable/revocable review credentials plus separate secret dictionaries from production or long-lived staging when orgs are shared.Reviewed by Cursor Bugbot for commit 09f553d. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit