Skip to content

fix(OPENFRAM-006-4): CU-86akbhg46 3 review findings in statefulset.yaml - #2317

Draft
flamingo[bot] wants to merge 1 commit into
mainfrom
ai-fix/openfram-006-4-795988ae-c1688cba
Draft

flamingo[bot] wants to merge 1 commit into
mainfrom
ai-fix/openfram-006-4-795988ae-c1688cba

Conversation

@flamingo

@flamingo flamingo Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Closes 3 review findings in manifests/datasources/mongodb-meshcentral/templates/statefulset.yaml.

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

# Fix confidence Finding Location
1 🟡 75 medium meshcentral-mongodb init containers run as root without dropped capabilities, and no readOnlyRootFilesystem guard for /data/db writes manifests/datasources/mongodb-meshcentral/templates/statefulset.yaml:28
2 🟡 60 medium meshcentral-mongodb StatefulSet does not set restrictive container-level securityContext or readOnlyRootFilesystem manifests/datasources/mongodb-meshcentral/templates/statefulset.yaml:59
3 🟢 95 high meshcentral-mongodb StatefulSet revisionHistoryLimit (10) diverges from org-wide standard of 2 used elsewhere manifests/datasources/mongodb-meshcentral/templates/statefulset.yaml:8

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-86akbhg46 OpenFrame Kubernetes manifest hardening (9 PRs)

@flamingo flamingo Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 What this fix changed, finding by finding

3 finding(s) fixed in this draft — 3 explained inline on the diff.

Comment on lines 29 to 37
image: {{ .Values.mongodb.initImage.registry }}/{{ .Values.mongodb.initImage.repository }}:{{ .Values.mongodb.initImage.tag }}
securityContext:
runAsUser: 0
allowPrivilegeEscalation: false
capabilities:
drop: ["ALL"]
command:
- /bin/sh
- -c

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔴 meshcentral-mongodb init containers run as root without dropped capabilities, and no readOnlyRootFilesystem guard for /data/db writes

Added allowPrivilegeEscalation: false and capabilities.drop: ["ALL"] to the securityContext of both the keyfile-prepare and fix-perms init containers (lines under initContainers:). runAsUser: 0 is left in place since both containers perform chown/chmod on files owned by other uids, which requires root privileges (CAP_CHOWN/CAP_FOWNER) and cannot be safely dropped without breaking the permission-fixing logic; readOnlyRootFilesystem was intentionally not added since these containers write to /work and /data/db, and forcing that flag would require additional emptyDir mounts for temp/writable paths not verified against the init image's needs — flagged as a partial fix.

🤖 Prompt for AI agents
In manifests/datasources/mongodb-meshcentral/templates/statefulset.yaml around line 28, review and complete this code-review fix: meshcentral-mongodb init containers run as root without dropped capabilities, and no readOnlyRootFilesystem guard for /data/db writes.
What the draft fix changed: Added `allowPrivilegeEscalation: false` and `capabilities.drop: ["ALL"]` to the securityContext of both the `keyfile-prepare` and `fix-perms` init containers (lines under `initContainers:`). `runAsUser: 0` is left in place since both containers perform `chown`/`chmod` on files owned by other uids, which requires root privileges (CAP_CHOWN/CAP_FOWNER) and cannot be safely dropped without breaking the permission-fixing logic; `readOnlyRootFilesystem` was intentionally not added since these containers write to `/work` and `/data/db`, and forcing that flag would require additional emptyDir mounts for temp/writable paths not verified against the init image's needs — flagged as a partial fix.
Verify the change is correct and complete; do not refactor unrelated code.

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

Comment on lines 66 to 75
- name: mongodb
image: {{ .Values.mongodb.image.registry }}/{{ .Values.mongodb.image.repository }}:{{ .Values.mongodb.image.tag }}
command: ["/usr/local/bin/mongodb-entrypoint.sh"]
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop: ["ALL"]
ports:
- name: mongodb
containerPort: 27017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 meshcentral-mongodb StatefulSet does not set restrictive container-level securityContext or readOnlyRootFilesystem

Added container-level securityContext blocks with allowPrivilegeEscalation: false and capabilities.drop: ["ALL"] to the mongodb and mongodb-exporter containers. Did not set readOnlyRootFilesystem: true or explicit privileged: false because the mongodb container writes to /data/db, /tmp, and possibly other paths at runtime that aren't fully mapped in this file, and mongodb-exporter's filesystem needs are unverified; setting readOnlyRootFilesystem: true without confirming/adding the necessary writable volume mounts risks breaking the container at runtime, so this is a partial fix pending verification of runtime write paths.

🤖 Prompt for AI agents
In manifests/datasources/mongodb-meshcentral/templates/statefulset.yaml around line 59, review and complete this code-review fix: meshcentral-mongodb StatefulSet does not set restrictive container-level securityContext or readOnlyRootFilesystem.
What the draft fix changed: Added container-level `securityContext` blocks with `allowPrivilegeEscalation: false` and `capabilities.drop: ["ALL"]` to the `mongodb` and `mongodb-exporter` containers. Did not set `readOnlyRootFilesystem: true` or explicit `privileged: false` because the mongodb container writes to `/data/db`, `/tmp`, and possibly other paths at runtime that aren't fully mapped in this file, and mongodb-exporter's filesystem needs are unverified; setting `readOnlyRootFilesystem: true` without confirming/adding the necessary writable volume mounts risks breaking the container at runtime, so this is a partial fix pending verification of runtime write paths.
Verify the change is correct and complete; do not refactor unrelated code.

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

spec:
serviceName: meshcentral-mongodb
replicas: 1
revisionHistoryLimit: 10

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔵 meshcentral-mongodb StatefulSet revisionHistoryLimit (10) diverges from org-wide standard of 2 used elsewhere

Changed revisionHistoryLimit: 10 to revisionHistoryLimit: 2 at the top-level spec to match the org-wide convention used by other openframe-saas Deployments.

🤖 Prompt for AI agents
In manifests/datasources/mongodb-meshcentral/templates/statefulset.yaml around line 8, review and complete this code-review fix: meshcentral-mongodb StatefulSet revisionHistoryLimit (10) diverges from org-wide standard of 2 used elsewhere.
What the draft fix changed: Changed `revisionHistoryLimit: 10` to `revisionHistoryLimit: 2` at the top-level `spec` to match the org-wide convention used by other openframe-saas Deployments.
Verify the change is correct and complete; do not refactor unrelated code.

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

@flamingo flamingo Bot changed the title fix(OPENFRAM-006-4): 3 review findings in statefulset.yaml fix(OPENFRAM-006-4): CU-86akbhg46 3 review findings in statefulset.yaml Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants