Skip to content

feat(security): opt velero into the baseline security-context mutation - #3482

Merged
devantler merged 1 commit into
mainfrom
claude/excluded-ns-baseline-context-3239
Aug 30, 2026
Merged

feat(security): opt velero into the baseline security-context mutation#3482
devantler merged 1 commit into
mainfrom
claude/excluded-ns-baseline-context-3239

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Motivation

Twelve namespaces are excluded from our security-context defaults because their workloads genuinely need elevated privilege. That exclusion is right about privilege — but it also suppressed two hardening controls that have nothing to do with privilege, so those namespaces have been failing a CIS check they could always have passed.

The mutation that supplies just those two fields already shipped, deliberately switched off, so that namespaces could be turned on one at a time rather than all twelve at once. Nothing has been turned on yet, so the capability currently does nothing.

Description

Turns it on for the first namespace, velero, and adds a guard so the list of switched-on namespaces can only change deliberately rather than as an unreviewed one-line edit.

Verified against velero's real production workloads: with the switch on, both fields appear on every pod; with it off, neither does. No user, group, or privilege setting changes in either state — which is what makes this safe for a namespace that is privileged by design.

Part of #3239

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

User evaluation — exercised against real production pod specs

Not a fixture run. I pulled the live velero pod specs from prod (oidc@prod, read-only) and ran them through the policy in both flag states with the Kyverno CLI.

Live baseline (before): all 56 pods in velero — 2 distinct pod-level shapes — carry no fsGroupChangePolicy and no container seLinuxOptions.

real pod state fsGroupChangePolicy container seLinuxOptions.level fsGroup / runAsUser / runAsGroup
velero-…-j57sm (Deployment) on OnRootMismatch s0 65532 / 65532 / 65532 — unchanged
node-agent-… (DaemonSet) on OnRootMismatch s0 absent / 0 / absent — unchanged
…-kopia-maintain-… (Job) on OnRootMismatch s0 65532 / 65532 / 65532 — unchanged
all three off absent absent identical to live

The guardrail this issue calls out is satisfied by construction. The recorded failure mode is a pod-level runAsGroup with no runAsUser, which yields the OCI user string ":1000" and makes the sandbox fail to build. The opt-in rules write no user, group, or fsGroup field at all — the third column above is byte-identical in both states — so that coupling cannot arise. On node-agent the injected fsGroupChangePolicy is inert (no fsGroup is set, and none is added); it satisfies the control without changing volume behaviour.

Render controls: the rendered hetzner controllers tree carries the label on velero (positive) and on no other namespace (negative). velero is prod-only — the docker provider has it commented out — so no local-cluster surface changes.

Guard ablations, both firing for the right reason:

  • label removed → expected 'velero', got ''
  • unreviewed widening (second namespace labelled) → expected 'velero', got 'kubescape,velero'
  • restored → green both times

Suite: bash scripts/tests/test-add-baseline-context.sh green; shellcheck clean. The one shfmt -i 2 -ci nit in that file is pre-existing on main and left alone as out of scope.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Relationship to #3465 — disclosed rather than left for the reviewer to find

#3465 is an open, undecided question about the constant seLinuxOptions.level: s0 these rules assign. This PR switches that field on for a namespace, so it belongs on the record here.

#3465 states the position directly: this is "a property of the pre-existing rules, not of any one recent change", and extending the pattern "without introducing a new risk class". This activation is that same extension, so it does not create the concern — but it does enlarge the population the eventual decision applies to, and that is worth being explicit about.

Premise re-verified live today, independently of #3465's 2026-08-29 measurement:

fact measured now
node OS / kernel (all nodes) Talos v1.13.9, 6.18.44-talos
pods carrying pod-level seLinuxOptions 0
containers carrying seLinuxOptions fleet-wide 119 (#3465 measured 116)
containers this PR adds to that population 56 in velero

So the "SELinux is not the active LSM" premise still holds, and runc continues to ignore the label — the field is inert on this cluster and changes declared posture only.

Why I did not block on it: #3465 owns the decision and lists three options, none of which is "stop opting namespaces in". Whichever it lands on is a change to the rules, applied once to every namespace already opted in — velero included. Holding this PR would not make that decision arrive sooner, and would leave a CIS control unmet in the meantime for no isolation gain on a cluster where the field does nothing.

If a reviewer disagrees and would rather #3465 resolve first, this is one label to revert.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

@devantler I will review the changes in #3482.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: eded9e4f-b32c-4031-b304-14cb4d405c43

📥 Commits

Reviewing files that changed from the base of the PR and between 378ef3a and 483935d.

📒 Files selected for processing (2)
  • k8s/bases/infrastructure/controllers/velero/namespace.yaml
  • scripts/tests/test-add-baseline-context.sh

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (1)
Never run a cluster

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • k8s/bases/infrastructure/controllers/velero/namespace.yaml
🧠 Learnings (1)
📚 Learning: 2026-08-10T13:01:12.782Z
Learnt from: devantler
Repo: devantler-tech/platform PR: 3057
File: .github/workflows/ci.yaml:622-659
Timestamp: 2026-08-10T13:01:12.782Z
Learning: Repository shell tests and scripts must remain compatible with macOS Bash 3.2. Do not use Bash 4+ features such as `mapfile`; use portable constructs, such as a `while IFS= read -r` loop, instead.

Applied to files:

  • scripts/tests/test-add-baseline-context.sh
🔇 Additional comments (2)
k8s/bases/infrastructure/controllers/velero/namespace.yaml (1)

10-19: LGTM!

scripts/tests/test-add-baseline-context.sh (1)

147-147: 🎯 Functional Correctness

Keep the .yaml-only inventory filter.

The repository contains no .yml files, and its Kubernetes convention covers k8s/**/*.yaml. The filter does not currently exclude any repository manifest.


📝 Walkthrough

Walkthrough

The Velero Namespace now has the pod-security.devantler.tech/baseline-context: enabled label. The rollout test scans Kubernetes YAML files, identifies enabled Namespace labels, and compares the sorted inventory with the expected value velero. The test sets a failure status and reports an error when the inventories differ.

Merge Risk: 🔵 Low · up to 48393

This enables two hardening fields for Velero pods without changing their user, group, or privilege settings. The rollout is limited to one namespace, but rollback and failure behavior for already-running workloads should remain an explicit owner consideration before merge.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: enabling the baseline security-context mutation for Velero.
Description check ✅ Passed The description is directly related to the changeset. It explains the Velero opt-in, the namespace-list guard, the security fields, and the validation performed.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.)


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devantler
devantler marked this pull request as ready for review August 30, 2026 20:01
@devantler
devantler added this pull request to the merge queue Aug 30, 2026
Merged via the queue into main with commit 06e6db9 Aug 30, 2026
26 checks passed
@devantler
devantler deleted the claude/excluded-ns-baseline-context-3239 branch August 30, 2026 20:12
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant