fix(adhoc-sweep-fixes): CU-86akdypw4 16 review findings across 15 files

[flamingo]

Closes 16 review findings across 15 files.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

Warning

This PR edits CI-executable files (workflows, build/manifest definitions). A same-repo PR can run a modified workflow with a write-scoped token as soon as it opens — review those hunks FIRST, before anything else in this PR.

#	Fix confidence	Finding	Location
1	🟢 90 high	Ingress default catch-all rule's YAML indentation is broken and inconsistent, risking a malformed manifest	manifests/tenant/templates/openframe-gateway/ingress.yaml:23
2	🟡 85 medium	ngrok Ingress hardcodes numeric port 8100 while localhost Ingress references the named port "http"	manifests/tenant/templates/openframe-gateway/ingress.yaml:56
3	🔴 55 low — review closely	Hardcoded plaintext NATS credentials committed in nats-credentials Secret	manifests/datasources/nats/templates/secret.yaml:1
4	🟢 95 high	Trailing stray single-quote breaks openframe-external-api.yml autoconfigure exclude list	configs/base/openframe-external-api.yml:20
5	🔴 55 low — review closely	Base64-encoded but effectively plaintext MongoDB root credentials committed to git	manifests/datasources/mongodb/templates/secret.yaml:8
6	🔴 55 low — review closely	Fleet MDM admin password and MySQL password hardcoded as plaintext defaults in secret templates	manifests/tenant/templates/fleet/secret.yaml:7
7	🟡 65 medium	MongoDB root credentials and keyfile hardcoded as plaintext in tenant secret template	manifests/tenant/templates/meshcentral/secret.yaml:24
8	🟢 95 high	mongo-init.sh checks for wrong marker file before writing, so first-time-init guard is broken	manifests/datasources/mongodb/scripts/mongo-init.sh:15
9	🔴 55 low — review closely	NATS DEVICE_USER account has an empty password, effectively allowing unauthenticated login	manifests/datasources/nats/templates/configmap.yaml:29
10	🟢 95 high	Stray debug key 'test: test' left in fleet-server ConfigMap	manifests/tenant/templates/fleet/configmap.yaml:12
11	🔴 45 low — review closely	redis-cluster node count is duplicated between two charts with only a comment linking them	manifests/datasources/redis-cluster/values.yaml:16
12	🟡 75 medium	release.yml release job silently discards artifact-download failures with continue-on-error, risking an empty release	.github/workflows/release.yml:285
13	🔴 55 low — review closely	openframe-gateway response-timeout of 20s will prematurely close long-lived SSE streams, contradicting the file's own comment	configs/base/openframe-gateway.yml:53
14	🔴 55 low — review closely	init-privileges Job embeds MySQL root password in shell -c script, visible in process args and Job logs	manifests/datasources/mysql-fleetmdm/templates/init-privileges-job.yaml:34
15	🔴 40 low — review closely	openframe-gateway deployment omits wait-for-management-server init container present in its siblings	manifests/tenant/templates/openframe-gateway/deployment.yaml:32
16	🟢 98 high	Cyrillic character 'С' used instead of Latin 'C' in Chart.yaml description	manifests/datasources/cassandra/Chart.yaml:3

What 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: c1688cba-c9bc-481b-b917-2b7f45403ab4

Merging 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-86akdypw4 Ad hoc sweep fixes across services (14 PRs)

[flamingo]

🦩 What this fix changed, finding by finding

16 finding(s) fixed in this draft — 16 explained inline on the diff; 8 low-confidence hunk(s) need close review before merging.

[flamingo]

🦩 🔴 Ingress default catch-all rule's YAML indentation is broken and inconsistent, risking a malformed manifest

Fixed indentation of the second rules entry (catch-all block) in the localhost Ingress spec: aligned the # add catch-all for IP access / other hosts comment to 4 spaces and re-indented paths/- path/pathType/backend/service/port to 8/10/12 spaces respectively, matching the structure and depth of the first rules entry above it, producing valid, consistent YAML list-item nesting.

🤖 Prompt for AI agents

In manifests/tenant/templates/openframe-gateway/ingress.yaml around line 23, review and complete this code-review fix: Ingress default catch-all rule's YAML indentation is broken and inconsistent, risking a malformed manifest.
What the draft fix changed: Fixed indentation of the second `rules` entry (catch-all block) in the localhost Ingress spec: aligned the `# add catch-all for IP access / other hosts` comment to 4 spaces and re-indented `paths`/`- path`/`pathType`/`backend`/`service`/`port` to 8/10/12 spaces respectively, matching the structure and depth of the first `rules` entry above it, producing valid, consistent YAML list-item nesting.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 ngrok Ingress hardcodes numeric port 8100 while localhost Ingress references the named port "http"

Changed the ngrok Ingress backend's port field for openframe-gateway from number: 8100 to name: http, matching the localhost Ingress's named-port reference so both Ingress definitions rely on the same named port rather than a hardcoded number; assumes the openframe-gateway Service exposes a port explicitly named http (confirmed by its use in the localhost Ingress in this same file), so no further changes should be needed.

🤖 Prompt for AI agents

In manifests/tenant/templates/openframe-gateway/ingress.yaml around line 56, review and complete this code-review fix: ngrok Ingress hardcodes numeric port 8100 while localhost Ingress references the named port "http".
What the draft fix changed: Changed the ngrok Ingress backend's `port` field for `openframe-gateway` from `number: 8100` to `name: http`, matching the localhost Ingress's named-port reference so both Ingress definitions rely on the same named port rather than a hardcoded number; assumes the `openframe-gateway` Service exposes a port explicitly named `http` (confirmed by its use in the localhost Ingress in this same file), so no further changes should be needed.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 Hardcoded plaintext NATS credentials committed in nats-credentials Secret

In manifests/datasources/nats/templates/secret.yaml, replaced the hardcoded plaintext ADMIN_PASS: "admin_password" and SERVICE_PASS: "service_pass" values with Helm template expressions ({{ required ... .Values.nats.adminPassword }} / .Values.nats.servicePassword) so no plaintext credential remains committed in this file, and chart rendering fails loudly if the values aren't supplied externally. This is a mechanical stopgap, not the full suggested fix: it assumes this file is rendered via Helm (the .tpl-style templating and .Values usage must actually be supported by the surrounding chart, which I cannot verify from this single file) and does not implement the recommended ExternalSecret/external-secret-store integration, since that spans additional files (values.yaml, an ExternalSecret CRD template, secret-store configuration) outside this file's scope. A complete fix additionally requires: (a) rotating the now-exposed admin_password/service_pass credentials in the real NATS deployment immediately, since they remain in git history regardless of this edit, and (b) wiring an actual ExternalSecret resource or secrets-manager integration per the suggested fix.
FILE>>>

🤖 Prompt for AI agents

In manifests/datasources/nats/templates/secret.yaml around line 1, review and complete this code-review fix: Hardcoded plaintext NATS credentials committed in nats-credentials Secret.
What the draft fix changed: In `manifests/datasources/nats/templates/secret.yaml`, replaced the hardcoded plaintext `ADMIN_PASS: "admin_password"` and `SERVICE_PASS: "service_pass"` values with Helm template expressions (`{{ required ... .Values.nats.adminPassword }}` / `.Values.nats.servicePassword`) so no plaintext credential remains committed in this file, and chart rendering fails loudly if the values aren't supplied externally. This is a mechanical stopgap, not the full suggested fix: it assumes this file is rendered via Helm (the `.tpl`-style templating and `.Values` usage must actually be supported by the surrounding chart, which I cannot verify from this single file) and does not implement the recommended ExternalSecret/external-secret-store integration, since that spans additional files (values.yaml, an ExternalSecret CRD template, secret-store configuration) outside this file's scope. A complete fix additionally requires: (a) rotating the now-exposed `admin_password`/`service_pass` credentials in the real NATS deployment immediately, since they remain in git history regardless of this edit, and (b) wiring an actual ExternalSecret resource or secrets-manager integration per the suggested fix.
FILE>>>
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

[flamingo]

🦩 🔴 Trailing stray single-quote breaks openframe-external-api.yml autoconfigure exclude list

Removed the stray trailing single-quote character after CassandraReactiveDataAutoConfiguration in the spring.autoconfigure.exclude list (line 20), so the entry now matches the actual fully-qualified class name exactly, ensuring the exclusion takes effect as intended.

🤖 Prompt for AI agents

In configs/base/openframe-external-api.yml around line 20, review and complete this code-review fix: Trailing stray single-quote breaks openframe-external-api.yml autoconfigure exclude list.
What the draft fix changed: Removed the stray trailing single-quote character after `CassandraReactiveDataAutoConfiguration` in the `spring.autoconfigure.exclude` list (line 20), so the entry now matches the actual fully-qualified class name exactly, ensuring the exclusion takes effect as intended.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 Base64-encoded but effectively plaintext MongoDB root credentials committed to git

In manifests/datasources/mongodb/templates/secret.yaml, replaced the hardcoded base64-encoded literals (with plaintext-revealing comments) for MONGO_INITDB_ROOT_USERNAME, MONGO_INITDB_ROOT_PASSWORD, and MONGO_APP_PASSWORD with Helm template expressions that pull values from .Values.mongodb.* and b64enc them at render time, using required to fail fast if unset. This removes the committed plaintext/base64 secret material from the file itself. UNVERIFIED/RISK: this assumes the surrounding chart is Helm-templated (the file lives under a templates/ directory, consistent with Helm) — if it is actually rendered by plain kubectl apply or Kustomize without a templating engine, this change will break deployment entirely since {{ }} syntax won't be processed. A complete fix would also require adding corresponding entries to values.yaml (or documenting that these must be supplied via -f secrets.yaml/--set or an External Secrets Operator/Vault integration as suggested), and ideally migrating to an actual ExternalSecret resource type instead of a raw Secret, which was not done here because it would require knowledge of the specific secret backend (Vault, AWS Secrets Manager, etc.) used by this repo, which is not visible from this single file.

🤖 Prompt for AI agents

In manifests/datasources/mongodb/templates/secret.yaml around line 8, review and complete this code-review fix: Base64-encoded but effectively plaintext MongoDB root credentials committed to git.
What the draft fix changed: In `manifests/datasources/mongodb/templates/secret.yaml`, replaced the hardcoded base64-encoded literals (with plaintext-revealing comments) for `MONGO_INITDB_ROOT_USERNAME`, `MONGO_INITDB_ROOT_PASSWORD`, and `MONGO_APP_PASSWORD` with Helm template expressions that pull values from `.Values.mongodb.*` and `b64enc` them at render time, using `required` to fail fast if unset. This removes the committed plaintext/base64 secret material from the file itself. UNVERIFIED/RISK: this assumes the surrounding chart is Helm-templated (the file lives under a `templates/` directory, consistent with Helm) — if it is actually rendered by plain `kubectl apply` or Kustomize without a templating engine, this change will break deployment entirely since `{{ }}` syntax won't be processed. A complete fix would also require adding corresponding entries to `values.yaml` (or documenting that these must be supplied via `-f secrets.yaml`/`--set` or an External Secrets Operator/Vault integration as suggested), and ideally migrating to an actual `ExternalSecret` resource type instead of a raw `Secret`, which was not done here because it would require knowledge of the specific secret backend (Vault, AWS Secrets Manager, etc.) used by this repo, which is not visible from this single file.
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

[flamingo]

🦩 🟠 release.yml release job silently discards artifact-download failures with continue-on-error, risking an empty release

In the release job, removed continue-on-error: true from the "Download all artifacts" step so a genuine download failure now fails the job instead of being silently swallowed, and added a new "Verify release artifacts are present" step (after "Prepare release artifacts") that checks final-artifacts/clients contains at least one .tar.gz or .zip file, exiting with an error before the release is created if none are found. This addresses both the silent-failure risk and the missing non-empty check called out in the finding. Residual risk: removing continue-on-error changes behavior for any currently-tolerated partial-download scenarios (e.g., if some but not all artifact names are expected to be absent by design); if that was intentional, the reviewer may want a narrower guard instead of an outright job failure.

🤖 Prompt for AI agents

In .github/workflows/release.yml around line 285, review and complete this code-review fix: release.yml release job silently discards artifact-download failures with continue-on-error, risking an empty release.
What the draft fix changed: In the `release` job, removed `continue-on-error: true` from the "Download all artifacts" step so a genuine download failure now fails the job instead of being silently swallowed, and added a new "Verify release artifacts are present" step (after "Prepare release artifacts") that checks `final-artifacts/clients` contains at least one `.tar.gz` or `.zip` file, exiting with an error before the release is created if none are found. This addresses both the silent-failure risk and the missing non-empty check called out in the finding. Residual risk: removing `continue-on-error` changes behavior for any currently-tolerated partial-download scenarios (e.g., if some but not all artifact names are expected to be absent by design); if that was intentional, the reviewer may want a narrower guard instead of an outright job failure.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 75 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 openframe-gateway response-timeout of 20s will prematurely close long-lived SSE streams, contradicting the file's own comment

Changed response-timeout from 20s back to 5m under spring.cloud.gateway.httpclient (line ~53) and updated the adjoining comment to explain the restoration, keeping the existing TODO. This directly resolves the finding's concern that a 20s gateway-wide timeout would prematurely close long-lived SSE streams (e.g. AI chat streaming). Risk/unverified: this reintroduces the original tradeoff the 20s change was presumably made to address (e.g. holding connections open longer for non-SSE routes, potentially masking slow/hung backends) — the proper fix (per-route SSE-specific timeout configuration) is not implemented here since it would require gateway route-level filter changes beyond a one-line config edit; a human should confirm reverting to 5m is acceptable or should instead scope this timeout per-route.

🤖 Prompt for AI agents

In configs/base/openframe-gateway.yml around line 53, review and complete this code-review fix: openframe-gateway response-timeout of 20s will prematurely close long-lived SSE streams, contradicting the file's own comment.
What the draft fix changed: Changed `response-timeout` from `20s` back to `5m` under `spring.cloud.gateway.httpclient` (line ~53) and updated the adjoining comment to explain the restoration, keeping the existing TODO. This directly resolves the finding's concern that a 20s gateway-wide timeout would prematurely close long-lived SSE streams (e.g. AI chat streaming). Risk/unverified: this reintroduces the original tradeoff the 20s change was presumably made to address (e.g. holding connections open longer for non-SSE routes, potentially masking slow/hung backends) — the proper fix (per-route SSE-specific timeout configuration) is not implemented here since it would require gateway route-level filter changes beyond a one-line config edit; a human should confirm reverting to 5m is acceptable or should instead scope this timeout per-route.
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

[flamingo]

🦩 🟠 init-privileges Job embeds MySQL root password in shell -c script, visible in process args and Job logs

In the command shell script (container init-privileges), replaced all -p${MYSQL_ROOT_PASSWORD} inline password arguments with an exported MYSQL_PWD environment variable set once at the top of the script, removing the password from process arguments//proc/cmdline. Also redirected the GRANT and SHOW GRANTS command outputs to /dev/null (and the readiness check output to /dev/null too) so grant/connection details no longer appear in Job pod logs via kubectl logs. Note: MYSQL_PWD is a known-but-still-visible-in-env approach (readable via /proc/<pid>/environ by same-container processes) and is a common accepted mitigation for CLI password exposure, but it does not fully eliminate secret exposure to anyone with pod exec/debug access; a complete fix would use a --defaults-extra-file with a mounted secret or a mysql config file instead.

🤖 Prompt for AI agents

In manifests/datasources/mysql-fleetmdm/templates/init-privileges-job.yaml around line 34, review and complete this code-review fix: init-privileges Job embeds MySQL root password in shell -c script, visible in process args and Job logs.
What the draft fix changed: In the `command` shell script (container `init-privileges`), replaced all `-p${MYSQL_ROOT_PASSWORD}` inline password arguments with an exported `MYSQL_PWD` environment variable set once at the top of the script, removing the password from process arguments/`/proc/cmdline`. Also redirected the GRANT and SHOW GRANTS command outputs to `/dev/null` (and the readiness check output to `/dev/null` too) so grant/connection details no longer appear in Job pod logs via `kubectl logs`. Note: `MYSQL_PWD` is a known-but-still-visible-in-env approach (readable via `/proc/<pid>/environ` by same-container processes) and is a common accepted mitigation for CLI password exposure, but it does not fully eliminate secret exposure to anyone with pod exec/debug access; a complete fix would use a `--defaults-extra-file` with a mounted secret or a mysql config file instead.
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

[flamingo]

🦩 🟠 openframe-gateway deployment omits wait-for-management-server init container present in its siblings

Added a wait-for-management-server init container to initContainers in manifests/tenant/templates/openframe-gateway/deployment.yaml, after the existing wait-for-config-server container, mirroring the pattern used in sibling charts (curling the management service's health endpoint via initImage and looping with sleep 1 until success). This makes use of the previously-unused $mgmt variable via $mgmt.image.tag in the health path. Risk/unverified: the exact service hostname (openframe-management), port (8101), and health path format (/management/{tag}/health) were inferred from convention used elsewhere in the gateway deployment (e.g. the management port and /management/{{ tag }}/health/... probe paths) but were not directly observable in the sibling files, since only this file was provided. A reviewer should confirm the actual service name and port used by other siblings' wait-for-management-server containers to ensure exact consistency.

🤖 Prompt for AI agents

In manifests/tenant/templates/openframe-gateway/deployment.yaml around line 32, review and complete this code-review fix: openframe-gateway deployment omits wait-for-management-server init container present in its siblings.
What the draft fix changed: Added a `wait-for-management-server` init container to `initContainers` in `manifests/tenant/templates/openframe-gateway/deployment.yaml`, after the existing `wait-for-config-server` container, mirroring the pattern used in sibling charts (curling the management service's health endpoint via `initImage` and looping with `sleep 1` until success). This makes use of the previously-unused `$mgmt` variable via `$mgmt.image.tag` in the health path. Risk/unverified: the exact service hostname (`openframe-management`), port (`8101`), and health path format (`/management/{tag}/health`) were inferred from convention used elsewhere in the gateway deployment (e.g. the `management` port and `/management/{{ tag }}/health/...` probe paths) but were not directly observable in the sibling files, since only this file was provided. A reviewer should confirm the actual service name and port used by other siblings' `wait-for-management-server` containers to ensure exact consistency.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 40 low — review closely — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔵 Cyrillic character 'С' used instead of Latin 'C' in Chart.yaml description

Replaced the Cyrillic 'С' (U+0421) with the Latin 'C' in the description field on line 3, changing "Wrapper chart for Сassandra" to "Wrapper chart for Cassandra" as suggested.

🤖 Prompt for AI agents

In manifests/datasources/cassandra/Chart.yaml around line 3, review and complete this code-review fix: Cyrillic character 'С' used instead of Latin 'C' in Chart.yaml description.
What the draft fix changed: Replaced the Cyrillic 'С' (U+0421) with the Latin 'C' in the description field on line 3, changing "Wrapper chart for Сassandra" to "Wrapper chart for Cassandra" as suggested.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 98 high — react 👍/👎 to teach the reviewer