Skip to content

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

Closed
ksc98 wants to merge 1 commit into
skyhook-io:mainfrom
ksc98:rbac-secrets-auto-enable-optout
Closed

radar: let rbac.secrets=false actually deny Secrets access#38
ksc98 wants to merge 1 commit into
skyhook-io:mainfrom
ksc98:rbac-secrets-auto-enable-optout

Conversation

@ksc98

@ksc98 ksc98 commented Aug 18, 2026

Copy link
Copy Markdown

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 }}

So turning on SSO is what widens the grant, and there is no way to decline it. On our production cluster Radar ends up holding an open watch on every Secret while rbac.secrets is set to false.

The fix

Drop the auth/cloud clauses so the rule follows rbac.secrets / rbac.helm only:

{{- 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 the 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.

Behaviour change: an install with auth on and both flags at their false defaults no longer gets cluster-wide Secrets read, so Helm release listing and the certificate-expiry check go empty. Setting rbac.helm: true (already required for every other Helm capability) or rbac.secrets: true restores it.

Test plan

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 ["get","list","watch"] ["get","list","watch"]
auth.mode=oidc + rbac.helm=true ["get","list","watch"] ["get","list","watch"]
rbac.secrets=true, no auth ["get","list","watch"] ["get","list","watch"]
rbac.helm=true, no auth ["get","list","watch"] ["get","list","watch"]

The narrowly-scoped hubble-relay-client-certs rule is unaffected in every case.

No chart version bump — charts/radar/Chart.yaml looks maintainer-owned (Update radar chart to vX bot commits). Happy to add one if you'd rather.


Note

Medium Risk
Changes Radar’s cluster RBAC around Secrets (credential exposure surface); misconfiguration could remove Helm/cert features for auth-only users while defaults are intended to stay equivalent.

Overview
Cluster-wide Secrets read access for the Radar ServiceAccount is no longer tied to auth.mode or cloud.enabled. The unscoped secrets rule in clusterrole.yaml is emitted only when rbac.secrets or rbac.helm is enabled.

Comments on that block are updated to state that list/watch cannot be scoped by Secret type, so the grant must stay behind explicit opt-in (rbac.secrets / rbac.helm). The narrowly scoped hubble-relay-client-certs rule is unchanged.

Per the PR description, rbac.secretsAutoEnable (default true) is meant to preserve today’s auth/cloud auto-grant when left at default; with it set to false, operators can avoid Radar holding cluster-wide Secret credentials while still opting in via rbac.secrets or rbac.helm. Trade-off when declining the auto-grant: Helm release visibility and certificate-expiry checks may be empty for users without their own Secrets permissions.

Reviewed by Cursor Bugbot for commit 85ac25f. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a08125e. Configure here.

Comment thread charts/radar/templates/clusterrole.yaml Outdated
Enabling auth (auth.mode != none) granted the ServiceAccount get/list/watch on
Secrets cluster-wide even with rbac.secrets=false, because the gate ORed the
auth mode in alongside the explicit opt-ins.

Drop the auth/cloud clauses so the rule follows rbac.secrets / rbac.helm only.
This matches the Helm add-on ClusterRoles, which already require rbac.helm and
treat auth mode as an additional condition rather than a substitute.
@ksc98
ksc98 force-pushed the rbac-secrets-auto-enable-optout branch from a08125e to 85ac25f Compare August 18, 2026 03:37
@ksc98 ksc98 changed the title radar: allow declining the auth-mode Secrets auto-grant radar: let rbac.secrets=false actually deny Secrets access Aug 18, 2026
@ksc98 ksc98 closed this Aug 18, 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.

1 participant