-
Notifications
You must be signed in to change notification settings - Fork 373
Clarify review app security guidance #752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -34,6 +34,12 @@ For review apps, GitHub needs one repository secret: | |||||||||||||||||||||||||||||
| | --- | --- | | ||||||||||||||||||||||||||||||
| | `CPLN_TOKEN_STAGING` | Service-account token for `shakacode-open-source-examples-staging`. | | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Use a staging/review token that cannot access production Control Plane | ||||||||||||||||||||||||||||||
| resources. In public repositories, generated review-app deploys skip fork PR | ||||||||||||||||||||||||||||||
| heads because Docker builds use repository secrets; if a forked change needs a | ||||||||||||||||||||||||||||||
| review app, first move the reviewed change to a trusted branch in this | ||||||||||||||||||||||||||||||
| repository. | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| No review-app repository variables are required for the standard path. The | ||||||||||||||||||||||||||||||
| workflow infers `qa-react-webpack-rails-tutorial` and | ||||||||||||||||||||||||||||||
| `shakacode-open-source-examples-staging` from `.controlplane/controlplane.yml`, | ||||||||||||||||||||||||||||||
|
|
@@ -111,6 +117,13 @@ Generate `SECRET_KEY_BASE` with `openssl rand -hex 64` and | |||||||||||||||||||||||||||||
| managed Postgres and Redis services and update `DATABASE_URL` and `REDIS_URL` | ||||||||||||||||||||||||||||||
| accordingly. | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Review apps run pull request code, so anything mounted through | ||||||||||||||||||||||||||||||
| `cpln://secret/...` can be read by that code after it starts. Keep the | ||||||||||||||||||||||||||||||
| `qa-react-webpack-rails-tutorial-secrets` dictionary limited to review-safe | ||||||||||||||||||||||||||||||
| values: disposable databases, review-only renderer credentials, and a Pro | ||||||||||||||||||||||||||||||
| license value that is acceptable for review-app exposure. Do not reuse | ||||||||||||||||||||||||||||||
| production or long-lived staging secret dictionaries for review apps. | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ### Advanced Overrides | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Most repos should leave these unset. They exist so forks and clones can test | ||||||||||||||||||||||||||||||
|
|
@@ -483,6 +496,10 @@ this automated process. When an approved collaborator comments exactly | |||||||||||||||||||||||||||||
| After the review app exists, new pushes to the PR redeploy it automatically. | ||||||||||||||||||||||||||||||
| Use `+review-app-delete` to delete it manually; closing the PR deletes it | ||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||
|
Comment on lines
498
to
503
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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! |
||||||||||||||||||||||||||||||
| from the `cpflow-promote-staging-to-production` workflow. | ||||||||||||||||||||||||||||||
| If staging moves off `master`, update both the `STAGING_APP_BRANCH` repository | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.