[INFRA-422] - chore(plane-enterprise): placeholder defaults for signing secrets (PI/LIVE/HMAC)#254
Open
akshat5302 wants to merge 1 commit into
Open
[INFRA-422] - chore(plane-enterprise): placeholder defaults for signing secrets (PI/LIVE/HMAC)#254akshat5302 wants to merge 1 commit into
akshat5302 wants to merge 1 commit into
Conversation
Replace the shipped baked-in defaults for PI_INTERNAL_SECRET, LIVE_SERVER_SECRET_KEY and SILO_HMAC_SECRET_KEY with the placeholder "change-to-top-secret" so installs don't silently share identical signing keys. Updated across templates, values.yaml, questions.yml and README. AES_SECRET_KEY and Django SECRET_KEY are intentionally left unchanged - rotating those is a breaking change for already-encrypted/hashed data. Chart bumped to 2.6.3.
Contributor
WalkthroughThe chart version is bumped from ChangesSecret Placeholder Cleanup and Chart Version Bump
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
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 |
|
Linked to Plane Work Item(s) References This comment was auto-generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
charts/plane-enterprise/README.md (1)
456-456: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMake these placeholders explicitly “must override” in docs.
Line 456 and Line 568 now show a known placeholder secret, but the table does not clearly communicate that this value is unsafe for real deployments. Please mark them as required when corresponding services are enabled (or add a prominent warning in the description).
Also applies to: 568-568
🤖 Prompt for 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. In `@charts/plane-enterprise/README.md` at line 456, The documentation table contains placeholder secret values like "change-to-top-secret" for parameters such as env.live_server_secret_key (and the corresponding parameter at line 568) that are not clearly marked as requiring override for real deployments. Update the description column in the table for these secret parameters to explicitly indicate they are placeholders that must be changed for production use, either by adding "REQUIRED - must override" language or by including a prominent warning that these default values are unsafe and must be replaced in real deployments.
🤖 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 `@charts/plane-enterprise/templates/config-secrets/app-env.yaml`:
- Around line 14-15: Add fail-fast validation guards to prevent Helm chart
rendering when placeholder signing secrets are still in use. For
LIVE_SERVER_SECRET_KEY and PI_INTERNAL_SECRET in app-env.yaml (lines 14-15),
apply a guard that detects when the default "change-to-top-secret" value is
being used and fails the deployment. Apply the identical guard pattern to
LIVE_SERVER_SECRET_KEY in
charts/plane-enterprise/templates/config-secrets/live-env.yaml (line 9),
PI_INTERNAL_SECRET in
charts/plane-enterprise/templates/config-secrets/pi-api-env.yaml (line 66), and
SILO_SECRET_KEY in charts/plane-enterprise/templates/config-secrets/silo.yaml
(line 10) to ensure consistency across all secret configuration files and
prevent accidental deployments with known placeholder values.
---
Nitpick comments:
In `@charts/plane-enterprise/README.md`:
- Line 456: The documentation table contains placeholder secret values like
"change-to-top-secret" for parameters such as env.live_server_secret_key (and
the corresponding parameter at line 568) that are not clearly marked as
requiring override for real deployments. Update the description column in the
table for these secret parameters to explicitly indicate they are placeholders
that must be changed for production use, either by adding "REQUIRED - must
override" language or by including a prominent warning that these default values
are unsafe and must be replaced in real deployments.
🪄 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: 91ab2451-e345-45c8-b20e-af6f8bfed368
📒 Files selected for processing (8)
charts/plane-enterprise/Chart.yamlcharts/plane-enterprise/README.mdcharts/plane-enterprise/questions.ymlcharts/plane-enterprise/templates/config-secrets/app-env.yamlcharts/plane-enterprise/templates/config-secrets/live-env.yamlcharts/plane-enterprise/templates/config-secrets/pi-api-env.yamlcharts/plane-enterprise/templates/config-secrets/silo.yamlcharts/plane-enterprise/values.yaml
Comment on lines
+14
to
+15
| LIVE_SERVER_SECRET_KEY: {{ .Values.env.live_server_secret_key | default "change-to-top-secret" | quote }} | ||
| PI_INTERNAL_SECRET: {{ .Values.env.pi_envs.internal_secret | default "change-to-top-secret" | quote }} |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Block installs that still use placeholder signing secrets.
Line 14 and Line 15 still allow rendering a known shared secret (change-to-top-secret) when operators forget to override values. That keeps token/signature secrets predictable at deploy time.
Suggested fail-fast guard
+{{- $liveSecret := required "env.live_server_secret_key is required and must not use the placeholder" .Values.env.live_server_secret_key -}}
+{{- if eq $liveSecret "change-to-top-secret" -}}
+{{- fail "env.live_server_secret_key must be changed from the placeholder" -}}
+{{- end -}}
+{{- $piInternal := required "env.pi_envs.internal_secret is required and must not use the placeholder" .Values.env.pi_envs.internal_secret -}}
+{{- if eq $piInternal "change-to-top-secret" -}}
+{{- fail "env.pi_envs.internal_secret must be changed from the placeholder" -}}
+{{- end -}}
- LIVE_SERVER_SECRET_KEY: {{ .Values.env.live_server_secret_key | default "change-to-top-secret" | quote }}
- PI_INTERNAL_SECRET: {{ .Values.env.pi_envs.internal_secret | default "change-to-top-secret" | quote }}
+ LIVE_SERVER_SECRET_KEY: {{ $liveSecret | quote }}
+ PI_INTERNAL_SECRET: {{ $piInternal | quote }}Apply the same guard pattern to charts/plane-enterprise/templates/config-secrets/live-env.yaml (Line 9), charts/plane-enterprise/templates/config-secrets/pi-api-env.yaml (Line 66), and charts/plane-enterprise/templates/config-secrets/silo.yaml (Line 10) for consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| LIVE_SERVER_SECRET_KEY: {{ .Values.env.live_server_secret_key | default "change-to-top-secret" | quote }} | |
| PI_INTERNAL_SECRET: {{ .Values.env.pi_envs.internal_secret | default "change-to-top-secret" | quote }} | |
| {{- $liveSecret := required "env.live_server_secret_key is required and must not use the placeholder" .Values.env.live_server_secret_key -}} | |
| {{- if eq $liveSecret "change-to-top-secret" -}} | |
| {{- fail "env.live_server_secret_key must be changed from the placeholder" -}} | |
| {{- end -}} | |
| {{- $piInternal := required "env.pi_envs.internal_secret is required and must not use the placeholder" .Values.env.pi_envs.internal_secret -}} | |
| {{- if eq $piInternal "change-to-top-secret" -}} | |
| {{- fail "env.pi_envs.internal_secret must be changed from the placeholder" -}} | |
| {{- end -}} | |
| LIVE_SERVER_SECRET_KEY: {{ $liveSecret | quote }} | |
| PI_INTERNAL_SECRET: {{ $piInternal | quote }} |
🤖 Prompt for 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.
In `@charts/plane-enterprise/templates/config-secrets/app-env.yaml` around lines
14 - 15, Add fail-fast validation guards to prevent Helm chart rendering when
placeholder signing secrets are still in use. For LIVE_SERVER_SECRET_KEY and
PI_INTERNAL_SECRET in app-env.yaml (lines 14-15), apply a guard that detects
when the default "change-to-top-secret" value is being used and fails the
deployment. Apply the identical guard pattern to LIVE_SERVER_SECRET_KEY in
charts/plane-enterprise/templates/config-secrets/live-env.yaml (line 9),
PI_INTERNAL_SECRET in
charts/plane-enterprise/templates/config-secrets/pi-api-env.yaml (line 66), and
SILO_SECRET_KEY in charts/plane-enterprise/templates/config-secrets/silo.yaml
(line 10) to ensure consistency across all secret configuration files and
prevent accidental deployments with known placeholder values.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Follow-up to #253 (INFRA-421), which merged only the
PI_INTERNAL_SECRETConfigMap→Secret move. The chart still ships identical baked-in defaults for several signing secrets, so every install shares the same keys unless overridden.This replaces those defaults with an obvious "change me" placeholder.
PI_INTERNAL_SECRETtyfvfqv…change-to-top-secretLIVE_SERVER_SECRET_KEYhtbqv…change-to-top-secretSILO_HMAC_SECRET_KEYgzb7…change-to-top-secretIntentionally left unchanged
AES_SECRET_KEY— rotating the AES key breaks already-encrypted data, so it must not change silently via a chart default.SECRET_KEY— same rationale (hashing/encryption of existing data).Changes
templates/config-secrets/{app-env,live-env,silo,pi-api-env}.yaml— placeholder defaults for the three signing secrets.values.yaml,questions.yml— matching defaults.README.md— placeholder values reflected in the values reference.Chart.yaml—2.6.2→2.6.3.Notes
app/silo/pistay in sync.Testing
helm lint— passes.change-to-top-secretwhileAES_SECRET_KEYandSECRET_KEYkeep their original defaults.Ready, HTTP 200).Summary by CodeRabbit
New Features
Documentation
Chores