radar: let rbac.secrets=false actually deny Secrets access

[ksc98]

Supersedes #38, which I opened with a different approach (a rbac.secretsAutoEnable opt-out) and closed. Bugbot correctly flagged that the new key would be absent under helm upgrade --reuse-values and silently drop the grant. This version adds no value and has that failure mode by construction.

The issue

rbac.secrets: false does not deny Secrets access. Enabling auth grants the Radar ServiceAccount get/list/watch on Secrets cluster-wide regardless, because the auth mode is ORed in alongside the explicit opt-ins:

{{- if or .Values.rbac.secrets .Values.rbac.helm (ne .Values.auth.mode "none") .Values.cloud.enabled }}

Turning on SSO is what widens the grant, which is a surprising direction, and there is no way to decline it. On our production cluster Radar holds an open watch on every Secret while rbac.secrets is set to false.

The fix

{{- if or .Values.rbac.secrets .Values.rbac.helm }}

This also makes the rule consistent with the rest of the chart. Both Helm add-on ClusterRoles already require rbac.helm, treating auth mode as an additional condition rather than a substitute for it:

{{- if and .Values.rbac.create .Values.rbac.helm (or (ne .Values.auth.mode "none") .Values.cloud.enabled) -}}

Only the Secrets rule let auth mode stand in on its own.

Who this affects on upgrade

Anyone matching rbac.secrets: false and rbac.helm: false and auth enabled. Both flags default to false, so that is every authenticated install that has not set one of them — not an edge case.

They lose Helm release listing, since Helm stores releases as Secrets. It is silent: no error, the Helm view just renders empty after a routine helm upgrade. Setting rbac.helm: true (already required for every other Helm capability) or rbac.secrets: true restores it.

That is the real cost of this change and it is your call whether it is worth it. If you would rather not break authenticated defaults, I am happy to rework this as a default-true opt-out using the chart's existing absence-means-true pattern, which is what Bugbot pointed at on #38 — say the word.

Test plan

• Three cases added to the existing upgrade_impact_rbac_test.yaml. The key one, keeps Secrets access opt-in when auth is enabled, fails on unmodified upstream/main and passes on this branch — verified by running it against a clean worktree checkout of main, so it is a real regression test rather than a tautology.

• Full suite on this branch: 25 pass, 2 fail. Those 2 (service.yaml annotation merging) fail identically on unmodified upstream/main — pre-existing, unrelated to this change.

• helm lint charts/radar passes.

• Rendered ClusterRole, unscoped Secrets rule:

  config	before	after
  defaults	absent	absent
  auth.mode=oidc	["get","list","watch"]	absent
  auth.mode=oidc + rbac.secrets=true	granted	granted
  auth.mode=oidc + rbac.helm=true	granted	granted
  rbac.secrets=true, no auth	granted	granted
  rbac.helm=true, no auth	granted	granted

  The narrowly-scoped hubble-relay-client-certs rule is unaffected throughout, and the tests assert that.

No chart version bump — charts/radar/Chart.yaml looks maintainer-owned (Update radar chart to vX bot commits), and per CLAUDE.md anything landing in charts/ triggers packaging. Happy to add one if you would prefer.

---

Note

Medium Risk
Security-sensitive RBAC tightening with a behavior change on upgrade for common authenticated defaults (Helm UI may go empty until rbac.helm or rbac.secrets is enabled).

Overview
rbac.secrets: false now actually withholds cluster-wide Secret get/list/watch on the Radar ServiceAccount. The ClusterRole no longer auto-grants that rule when auth.mode is not none or cloud.enabled is on—only rbac.secrets or rbac.helm enable it (Helm still documents that release storage needs Secrets read).

Comments in values.yaml and clusterrole.yaml clarify that list/watch cannot be scoped by Secret type, so the grant is all Secrets and should stay explicit opt-in.

Helm chart tests add cases that OIDC without those flags omits the broad Secrets rule (while the scoped hubble-relay-client-certs rule is unchanged) and that rbac.secrets or rbac.helm still grant access with auth on.

Upgrade note: authenticated installs that left both rbac.secrets and rbac.helm at default false will lose Helm release listing after upgrade unless one of those flags is set—no error, just an empty Helm view.

^{Reviewed by Cursor Bugbot for commit df8701f. Bugbot is set up for automated code reviews on this repo. Configure here.}