fix(OPENFRAM-006-8): CU-86akdypzw 2 review findings across 2 files - #2348
flamingo[bot] wants to merge 2 commits into
Conversation
| name: fleet-server | ||
| type: Opaque | ||
| stringData: | ||
| FLEET_SETUP_ADMIN_PASSWORD: "openframe123!" | ||
| FLEET_SETUP_ADMIN_PASSWORD: {{ required "fleet.setupAdminPassword is required (inject via ExternalSecret/sealed-secret, do not hardcode)" .Values.fleet.setupAdminPassword | quote }} | ||
|
|
||
| --- | ||
|
|
There was a problem hiding this comment.
🦩 🔴 Hardcoded default fleet/MySQL credentials committed to secret template
In manifests/tenant/templates/fleet/secret.yaml, replaced the hardcoded plaintext defaults "openframe123!" and "fleet-mdm-password-1234" with required Helm template calls that pull from .Values.fleet.setupAdminPassword and .Values.fleet.mysqlPassword, failing the render if not supplied. This removes the literal secret from version control and forces callers to inject values via --set, a values file sourced from a secret manager, or an ExternalSecret-populated values override. It does not itself wire up an ExternalSecret/sealed-secret resource (no such CRD/controller config was visible in this file), so a complete fix would additionally require defining those values in a non-committed source (e.g., CI secret injection or a SealedSecret/ExternalSecret manifest referencing the same key names) — that wiring is outside this file and not verifiable here.
🤖 Prompt for AI agents
In manifests/tenant/templates/fleet/secret.yaml around line 2, review and complete this code-review fix: Hardcoded default fleet/MySQL credentials committed to secret template.
What the draft fix changed: In `manifests/tenant/templates/fleet/secret.yaml`, replaced the hardcoded plaintext defaults `"openframe123!"` and `"fleet-mdm-password-1234"` with `required` Helm template calls that pull from `.Values.fleet.setupAdminPassword` and `.Values.fleet.mysqlPassword`, failing the render if not supplied. This removes the literal secret from version control and forces callers to inject values via `--set`, a values file sourced from a secret manager, or an ExternalSecret-populated values override. It does not itself wire up an ExternalSecret/sealed-secret resource (no such CRD/controller config was visible in this file), so a complete fix would additionally require defining those values in a non-committed source (e.g., CI secret injection or a SealedSecret/ExternalSecret manifest referencing the same key names) — that wiring is outside this file and not verifiable here.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.
fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer
| @@ -7,9 +7,9 @@ metadata: | |||
| name: meshcentral | |||
| type: Opaque | |||
| stringData: | |||
There was a problem hiding this comment.
🦩 🔴 Hardcoded default MeshCentral and MongoDB credentials committed to templates
Removed hardcoded default credentials (meshpass@1234, K7vMzYZmkntu, and the plaintext defaults for MESHCENTRAL_USER/MESHCENTRAL_CONFIG_KEY/MC_MONGO_USER) in manifests/tenant/templates/meshcentral/secret.yaml. Replaced each | default "<value>" with Helm's required function so the chart now fails to render with a clear error message unless .Values.meshcentral.credentials.user/password/configKey and .Values.meshcentral.mongodb.user/password are explicitly supplied by the caller (e.g. via values file or --set), eliminating any silent fallback to committed plaintext secrets. This is a behavior change (chart will error without values set) rather than a values-schema/documentation update, so a complete fix should also ensure the corresponding values.yaml/values-schema and any environment-specific values files are updated to provide these secrets via a secret manager or user-supplied overrides — that is out of scope for this single file.
🤖 Prompt for AI agents
In manifests/tenant/templates/meshcentral/secret.yaml around line 9, review and complete this code-review fix: Hardcoded default MeshCentral and MongoDB credentials committed to templates.
What the draft fix changed: Removed hardcoded default credentials (`meshpass@1234`, `K7vMzYZmkntu`, and the plaintext defaults for `MESHCENTRAL_USER`/`MESHCENTRAL_CONFIG_KEY`/`MC_MONGO_USER`) in `manifests/tenant/templates/meshcentral/secret.yaml`. Replaced each `| default "<value>"` with Helm's `required` function so the chart now fails to render with a clear error message unless `.Values.meshcentral.credentials.user/password/configKey` and `.Values.meshcentral.mongodb.user/password` are explicitly supplied by the caller (e.g. via values file or `--set`), eliminating any silent fallback to committed plaintext secrets. This is a behavior change (chart will error without values set) rather than a values-schema/documentation update, so a complete fix should also ensure the corresponding values.yaml/values-schema and any environment-specific values files are updated to provide these secrets via a secret manager or user-supplied overrides — that is out of scope for this single file.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 65 medium — react 👍/👎 to teach the reviewer
Closes 2 review findings across 2 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
manifests/tenant/templates/fleet/secret.yaml:2manifests/tenant/templates/meshcentral/secret.yaml:9What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
16dc4bdf-608c-4c65-84b9-63df756e4657Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.
ClickUp task: CU-86akdypzw OpenFrame Kubernetes manifest hardening secondary (12 PRs)