Skip to content

Redact Secret values from Helm diffs for AI callers and bind radar:ai for background Diagnose - #1949

Merged
nadaverell merged 9 commits into
skyhook-io:mainfrom
arlenvasconcelos:arlen/rad-657-diagnose-permissions-every-diagnose-run-reads-as-radarai
Oct 5, 2026
Merged

nadaverell merged 9 commits into
skyhook-io:mainfrom
arlenvasconcelos:arlen/rad-657-diagnose-permissions-every-diagnose-run-reads-as-radarai

Conversation

@arlenvasconcelos

@arlenvasconcelos arlenvasconcelos commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Linear: RAD-657

Why

Radar Cloud's AI Diagnose is blind on clusters whose role bindings are off (cloud.defaultRbac.create=false, IdP groups decide access), because it reads as radar:viewer, which is unbound there. After this release:

  • Manual Diagnose runs read as the person who started them (skyhook-dev/radar-hub#292), so the AI never exceeds that person's access. A person may be able to read Secrets, so Helm diffs handed to the model must not carry Secret values.
  • Background (alert-triggered) runs read as one view-only identity, the radar:ai group (skyhook-dev/radar-hub#288), which this chart binds.

MCP clients are not affected: they keep reading with the user's own identity.

What changes

1. Secret redaction in get_helm_release (every MCP caller)

  • include=diff (the manifest diff) and include=notes_diff were returned unredacted; only values_diff went through RedactSecrets.
  • Each kind: Secret document now has its data / stringData values replaced by [REDACTED] before diffing. The keys stay, so added and removed keys still show. Secrets inside kind: List items are covered, the last-applied-configuration annotation (a full copy of the object) is dropped, and a document that doesn't parse is withheld whole. Other documents are left byte-for-byte untouched.
  • RedactSecrets then runs over both diffs as a second layer.
  • include=values and include=values_diff now use key-aware redaction built for Helm values (aicontext.RedactHelmValues). Chart authors name credentials freely (dbPassword, auth.postgresPassword), so any key ending in password, token, apiKey, secretKey, clientSecret, credentials and similar is masked, numbers included. Secret references (existingSecret, *SecretName, *Ref) keep their values. The values diff masks both revisions before diffing. CRD specs keep the existing exact-key matching.
  • The REST/UI Helm diff is unchanged: it is the user's own view.

2. radar:ai chart binding

  • New templates/cloud-rbac-ai.yaml: binds radar:ai to view. No Secret role, no writes.
  • radar:ai is added to the cluster-read and integration-read add-ons.
  • New value cloud.aiRbac, default true, independent of cloud.defaultRbac. As with cloud.systemRbac, an absent value means off, so a --reuse-values upgrade never gains the binding silently.
  • Docs: the chart README and docs/authentication.md.

The binding does nothing until Radar Cloud sends the group. Background runs send radar:ai with no tier group, so the AI's access is exactly this binding: cloud.aiRbac=false really turns it off, and a widened viewer role doesn't widen the AI. Clusters on older charts get no background Diagnose until they upgrade. Radar Cloud refuses manual runs on a Radar older than this release, rather than reading with less protection.

Tests

  • Go unit tests for Helm values redaction: nested keys, numeric credentials, references kept under credentials, both revisions of a values diff.
  • Go unit tests for the manifest-diff redaction: Secret values gone, keys and non-Secret changes kept, last-applied copies, Secrets inside Lists, and unparseable documents. Plus a check that resource_diff masks Secret values.
  • TestIntegrationReadBindings expects the radar:ai bindings alongside radar:system, with cases for aiRbac=false.
  • New tests/cloud_rbac_ai_test.yaml: renders with defaultRbac.create=false, absent or false means off, bound to view only, add-ons present whatever the tier settings.
  • scripts/test-chart.sh: new render cases; the "no orphan role" cases now also set cloud.aiRbac=false.
  • helm unittest: the service_test.yaml failures also fail on main locally; CI's Helm chart job passes.

Live test on EKS (proxy auth, simulated Diagnose identities)

  • Background identity (radar:ai,radar:org:<org>) with the chart's radar:ai binding: pods in every namespace, no Secrets.
  • Manual run as a contractor whose IdP group has view in one namespace: pods in that namespace only; listing all namespaces returns only that one.
  • Helm release with a Secret, a token in its notes, and dbPassword / apiKey values, read through get_helm_release by a user who can read Secrets: no sentinel value in values, diff, notes_diff or values_diff. The REST diff (the user's own view in the UI) still shows them, as intended.

End to end on kind

  • Role bindings off, aiRbac=true: the AI identity reads pods, events and nodes; Secrets are forbidden.
  • aiRbac=false: it reads nothing.
  • get_helm_release include=diff,notes_diff, called by an owner who can read Secrets: no Secret values in the output. The Let Kubernetes RBAC, not the Cloud role, decide user-run operations #1899 binary returns the plaintext values and a ghp_ token from the notes.

Note

Medium Risk
Changes cluster RBAC defaults and how credentials appear in MCP Helm diffs; misconfiguration could block background Diagnose or leak secrets if redaction regresses, but grants stay read-only and UI paths are unchanged.

Overview
Adds default Kubernetes RBAC for automatic Radar Cloud Diagnose via new cloud.aiRbac (default on): chart-owned aggregated view role for radar:ai, plus cluster-read and integration-read add-ons—no Secret access. Refactors radar:system the same way (owned aggregated view role instead of binding directly to view), with Secret read unchanged for alerts/timeline. Docs and Helm tests cover independence from cloud.defaultRbac, --reuse-values absent-key behavior, and installer escalate requirements.

Hardens Helm MCP output for AI callers while leaving REST/UI diffs as the user’s own view: manifest diffs strip Secret data/stringData (and unparseable Secret docs) before diffing; values diffs use new RedactHelmValues (suffix/key-aware credentials, preserved secret references); notes diffs get pattern redaction. get_helm_release include=diff, values_diff, and notes_diff route through these paths.

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

arlenvasconcelos and others added 3 commits October 5, 2026 00:50
The manifest diff and the notes diff reached MCP callers unredacted; only
values_diff went through RedactSecrets. Secret documents now have their
data and stringData values replaced by [REDACTED] before diffing, keys
kept so additions and removals still show, and RedactSecrets runs over
both diffs as a second layer. The REST/UI diff is unchanged: it is the
user's own view.
Every AI Diagnose run, manual or background, will read the cluster as the
radar:ai group. cloud.aiRbac (default true) binds it to view plus the
cluster-read and integration-read add-ons, with no Secret role, and stays
on when cloud.defaultRbac.create=false so Diagnose works when IdP groups
decide cluster access. An absent value means off, as with
cloud.systemRbac, so a --reuse-values upgrade never gains it silently.

The binding does nothing until the hub sends the group.
…ts, and expect radar:ai bindings in the chart test
@nadaverell
nadaverell force-pushed the arlen/rad-657-diagnose-permissions-every-diagnose-run-reads-as-radarai branch from 22fd1f5 to 03a4e19 Compare October 4, 2026 23:12
@nadaverell nadaverell changed the title Redact Secret values from Helm diffs and bind radar:ai for AI Diagnose Redact Secret values from Helm diffs for AI callers and bind radar:ai for background Diagnose Oct 4, 2026
@nadaverell
nadaverell marked this pull request as ready for review October 5, 2026 09:54
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Redact Helm secrets for MCP and bind background Diagnose to radar:ai

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Redact Secret manifests, notes, and credential-bearing Helm values before returning MCP diffs;
 preserve REST/UI diffs.
• Bind radar:ai to view-only roles independently of default Cloud tier bindings.
• Test redaction and RBAC rendering, including disabled bindings and upgrades with reused values.
Diagram

graph TD
  C["Chart AI bindings"] --> B["Kubernetes RBAC"] --> A["Cloud Diagnose"]
  D["MCP caller"] --> E["Helm MCP tool"] --> F["Helm client"] --> G["AI redaction"] --> H["MCP response"]
Loading
High-Level Assessment

Keep the separate AI binding and MCP-only redacted methods. Reusing the viewer tier would fail when default tier bindings are disabled, while redacting only completed diffs would not reliably remove arbitrary Secret values or values from both revisions. The dedicated methods also preserve the user's REST/UI view.

Files changed (22) +1007 / -41

Enhancement (2) +86 / -4
cloud-rbac-ai.yamlBind radar:ai to the built-in view role +30/-0

Bind radar:ai to the built-in view role

• Adds a Cloud-mode ClusterRoleBinding for 'radar:ai' when chart-managed RBAC and AI RBAC are enabled. It grants the built-in 'view' role without a Secret or write role.

deploy/helm/radar/templates/cloud-rbac-ai.yaml

redact.goAdd Helm-specific credential-key redaction +56/-4

Add Helm-specific credential-key redaction

• Adds suffix-aware redaction for free-form Helm keys and non-string credential scalars while retaining Secret references. Existing CRD inline redaction keeps its exact-key behavior.

pkg/ai/context/redact.go

Bug fix (2) +156 / -12
client.goAdd AI-safe Helm manifest and values diff methods +151/-8

Add AI-safe Helm manifest and values diff methods

• Adds redacted diff variants while retaining existing unredacted methods for REST/UI callers. Secret documents are sanitized before manifest diffing, malformed documents are withheld, and both values revisions are redacted before comparison.

internal/helm/client.go

tools_helm.goUse sanitized Helm results for MCP callers +5/-4

Use sanitized Helm results for MCP callers

• Routes manifest and values diffs through the new redacted client methods, redacts notes diffs, and applies Helm-specific key-aware redaction to included values.

internal/mcp/tools_helm.go

Tests (9) +693 / -16
cloud_rbac_ai_test.yamlTest AI role bindings and disablement +180/-0

Test AI role bindings and disablement

• Checks the view-only binding, both read add-ons, independence from tier bindings, and suppression when AI RBAC, Cloud mode, or chart-managed RBAC is off.

deploy/helm/radar/tests/cloud_rbac_ai_test.yaml

cloud_rbac_integration_read_test.yamlIsolate tier integration-read test cases +3/-2

Isolate tier integration-read test cases

• Disables the default AI binding in tests intended to check tier and system integration-read behavior.

deploy/helm/radar/tests/cloud_rbac_integration_read_test.yaml

cloud_rbac_system_test.yamlKeep the system-only RBAC fixture isolated +1/-0

Keep the system-only RBAC fixture isolated

• Explicitly disables AI RBAC in a system-binding test so the new default does not affect its expected resources.

deploy/helm/radar/tests/cloud_rbac_system_test.yaml

manifest_redaction_test.goTest manifest Secret redaction and resource-diff safety +153/-0

Test manifest Secret redaction and resource-diff safety

• Covers Secret keys and values, unchanged non-Secret documents, last-applied copies, Lists, and malformed YAML. Also checks that structured resource diffs do not expose Secret values.

internal/helm/manifest_redaction_test.go

values_redaction_test.goTest redaction before values diffing +112/-0

Test redaction before values diffing

• Checks user-supplied and computed values across both revisions, including nested and non-string credentials, while preserving ordinary changes and Secret references.

internal/helm/values_redaction_test.go

integration_read_baseline_test.goExpect radar:ai integration-read bindings +14/-10

Expect radar:ai integration-read bindings

• Extends rendered-chart assertions to cover AI bindings alongside system bindings, including disabled-AI cases and the absence of a legacy 'cloud:ai' subject.

internal/k8s/integration_read_baseline_test.go

tools_helm_test.goTest MCP Helm values masking without mutating inputs +41/-0

Test MCP Helm values masking without mutating inputs

• Adds coverage for nested, numeric, and Boolean credentials, preserved Secret references, and unchanged source values.

internal/mcp/tools_helm_test.go

redact_helm_test.goTest Helm key sensitivity and reference exceptions +168/-0

Test Helm key sensitivity and reference exceptions

• Exercises credential suffixes, nested structures, scalar types, inherited sensitivity, references, high-confidence token patterns, and unchanged CRD redaction rules.

pkg/ai/context/redact_helm_test.go

test-chart.shAdd chart-render checks for AI RBAC +21/-4

Add chart-render checks for AI RBAC

• Checks AI bindings with tier bindings off, the absence of Secret-role grants, and explicit or absent AI RBAC disablement. Updates no-orphan-role cases to disable AI RBAC as well.

scripts/test-chart.sh

Documentation (4) +24 / -6
README.mdDocument the radar:ai binding and upgrade behavior +17/-0

Document the radar:ai binding and upgrade behavior

• Explains the view-only AI binding, its independence from default tier RBAC, how to replace it, and why upgrades with reused values leave an absent setting off.

deploy/helm/radar/README.md

cloud-rbac-system.yamlClarify the separate AI and system identities +4/-4

Clarify the separate AI and system identities

• Updates the template commentary to identify 'radar:ai' as the Diagnose identity rather than 'radar:viewer'.

deploy/helm/radar/templates/cloud-rbac-system.yaml

authentication.mdDescribe radar:ai Cloud authorization +1/-0

Describe radar:ai Cloud authorization

• Documents the AI group's view and read-add-on grants, the independent configuration switch, and MCP clients' continued use of their own permissions.

docs/authentication.md

helm.mdDistinguish UI and MCP Helm values visibility +2/-2

Distinguish UI and MCP Helm values visibility

• Clarifies that the UI retains the user's values while MCP applies key-aware redaction, including to both values-diff revisions.

docs/helm.md

Other (5) +48 / -3
_helpers.tplAdd an explicit opt-in check for AI RBAC rendering +14/-0

Add an explicit opt-in check for AI RBAC rendering

• Defines a cloud-mode helper that enables the AI binding only when 'cloud.aiRbac' is explicitly true, including on upgrades with reused values.

deploy/helm/radar/templates/_helpers.tpl

cloud-rbac-cluster-read.yamlExtend cluster-read access to radar:ai +19/-1

Extend cluster-read access to radar:ai

• Renders the existing cluster-read role for AI access independently of tier bindings and adds a dedicated 'radar:ai' binding.

deploy/helm/radar/templates/cloud-rbac-cluster-read.yaml

cloud-rbac-integration-read.yamlExtend integration-read add-ons to radar:ai +6/-2

Extend integration-read add-ons to radar:ai

• Includes 'radar:ai' in namespaced and cluster integration-read bindings even when tier bindings are disabled. Unlike tier groups, it receives no legacy 'cloud:*' subject.

deploy/helm/radar/templates/cloud-rbac-integration-read.yaml

values.schema.jsonDeclare the cloud.aiRbac chart value +1/-0

Declare the cloud.aiRbac chart value

• Adds Boolean schema validation for the AI RBAC switch.

deploy/helm/radar/values.schema.json

values.yamlEnable AI RBAC by default on fresh chart values +8/-0

Enable AI RBAC by default on fresh chart values

• Introduces 'cloud.aiRbac: true' and documents its view-only scope, independence from default tier RBAC, and reused-values behavior.

deploy/helm/radar/values.yaml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (1) 📜 Skill insights (0)

Grey Divider


Action required

1. Secret comments reach the model ✓ Resolved
Description
redactSecretDocument copies every leading comment back onto a redacted Secret, although only the
Helm source comment needs preserving. When a changed Secret has a leading comment such as `# token:
short-secret-123`, the manifest diff retains that value and the pattern-based second pass does not
mask it.
Code

internal/helm/client.go[R889-895]

+	for _, line := range strings.Split(doc, "\n") {
+		if !strings.HasPrefix(strings.TrimSpace(line), "#") {
+			break
+		}
+		comments.WriteString(line + "\n")
+	}
+	return comments.String() + strings.TrimSuffix(string(b), "\n")
Evidence
The redactor masks parsed Secret fields but then restores leading comments verbatim. The resulting
text is diffed for the MCP response; the fallback redactor only recognizes specified patterns and
does not match an ordinary short token in a comment.

internal/helm/client.go[844-846]
internal/helm/client.go[887-895]
pkg/ai/context/redact.go[9-19]
internal/mcp/tools_helm.go[173-180]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The manifest redactor restores leading comments from Secret documents, allowing credential text in those comments into the MCP diff.
## Fix Focus Areas
- internal/helm/client.go[887-895]
- internal/helm/manifest_redaction_test.go[56-75]
## Recommended Fix
Preserve only a validated Helm `# Source:` line, or discard comments from rewritten Secret documents. Add a test with a short credential in a leading comment and verify it is absent from the resulting diff.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Release notes expose short credentials 🐞 Bug ⛨ Security
Description
notes_diff applies only RedactSecrets to free-form release notes, whose patterns do not
recognize an ordinary short value under a key such as apiKey. If that value changes between
revisions, both versions can appear in the diff returned to the MCP caller.
Code

internal/mcp/tools_helm.go[208]

+				diff.Diff = aicontext.RedactSecrets(diff.Diff)
Evidence
The notes path diffs raw release notes before applying the pattern-based redactor. Its listed
patterns cover specific token formats, password assignments and long base64-like strings, but not a
short arbitrary apiKey value.

internal/helm/client.go[815-837]
internal/mcp/tools_helm.go[201-210]
pkg/ai/context/redact.go[9-19]
pkg/ai/context/redact.go[35-37]
pkg/ai/context/redact.go[108-115]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new notes-diff redaction misses ordinary credential values that do not match its token patterns.
## Fix Focus Areas
- internal/mcp/tools_helm.go[201-210]
- pkg/ai/context/redact.go[9-19]
## Recommended Fix
Apply explicit redaction to credential-labelled assignments in notes. Where free-form notes cannot be sanitized reliably, withhold the notes diff from AI responses; test short `apiKey` and `token` values in both revisions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Numeric credentials reach Helm responses ✓ Resolved
Description
redactHelmNode drops inherited sensitivity when it recurses into an array, then masks a non-string
element only when its own key is sensitive. For values such as credentials: [12345678], that
number survives both the new values redactor and the pre-diff redaction of values_diff.
Code

pkg/ai/context/redact.go[R192-203]

+	case []any:
+		for i, item := range v {
+			v[i] = redactHelmNode(item, keySensitive, false)
+		}
+		return v
+	case string:
+		return redactNode(v, keySensitive, isSensitiveKey)
+	default:
+		if ownKeySensitive && node != nil {
+			return "[REDACTED]"
+		}
+		return node
Evidence
credentials is classified as sensitive, but array recursion sets ownKeySensitive to false. The
non-string branch therefore returns the number unchanged; the MCP values path uses this walker, and
the values-diff path uses it before serialization.

pkg/ai/context/redact.go[173-180]
pkg/ai/context/redact.go[184-204]
internal/mcp/tools_helm.go[149-158]
internal/mcp/tools_helm.go[284-293]
internal/helm/client.go[1063-1072]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Numeric entries in a sensitive Helm values array remain visible because array recursion clears the flag used to mask non-string scalars.
## Fix Focus Areas
- pkg/ai/context/redact.go[184-204]
- pkg/ai/context/redact_helm_test.go[86-108]
- internal/helm/values_redaction_test.go[62-110]
## Recommended Fix
Carry inherited sensitivity through array elements when deciding whether to mask non-string scalars, while retaining the existing behavior for unrelated numeric fields in maps. Test numeric entries under `credentials` through both values and values-diff paths.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View high (1)
4. Staging background Diagnose loses access 🔗 Cross-repo conflict ≡ Correctness
Description
The new radar:ai bindings exist only in Radar's chart, while skyhook-dev/deployment runs the
dev agent image with a vendored chart that has no binding for that group. When the Hub sends
radar:ai for a background Diagnose run, Kubernetes denies its impersonated reads in staging until
the vendored chart is updated.
Code

deploy/helm/radar/templates/cloud-rbac-ai.yaml[R25-28]

+  name: view
+subjects:
+  - kind: Group
+    name: radar:ai
Evidence
The PR adds a binding specifically for radar:ai; staging instead renders a local older chart
alongside the development image, and its existing cluster-read bindings cover only the three tier
groups.

radar -> deployment
deploy/helm/radar/templates/cloud-rbac-ai.yaml[15-29]
deploy/helm/radar/templates/cloud-rbac-cluster-read.yaml[195-210]
deploy/helm/radar/templates/cloud-rbac-integration-read.yaml[20-22]
External repo: skyhook-dev/deployment, argocd/addons/radar-staging/radar-staging/skh-nonprod/radar-staging-appset.yaml [29-35]
External repo: skyhook-dev/deployment, argocd/addons/radar-staging/radar-staging/skh-nonprod/Chart.yaml [5-13]
External repo: skyhook-dev/deployment, argocd/addons/radar-staging/radar-staging/skh-nonprod/values.yaml [4-19]
External repo: skyhook-dev/deployment, argocd/addons/radar-staging/radar-staging/skh-nonprod/charts/radar/templates/cloud-rbac-cluster-read.yaml [88-115]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Staging runs the Radar dev image but renders an older vendored chart without the new `radar:ai` bindings. Background Diagnose requests using that group cannot read the staging cluster.
## Fix Focus Areas
- deploy/helm/radar/templates/cloud-rbac-ai.yaml[15-29]
- argocd/addons/radar-staging/radar-staging/skh-nonprod/Chart.yaml[5-13]
- argocd/addons/radar-staging/radar-staging/skh-nonprod/values.yaml[4-19]
## Recommended Fix
Update the vendored Radar chart in skyhook-dev/deployment to include the AI view, cluster-read, and integration-read bindings, and deploy it before the staging Hub begins sending `radar:ai`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

5. A test comment invokes past behavior ✓ Resolved
Description
The inline comment non-Secret documents diff as before describes the assertion by comparison with
unspecified earlier behavior. It accompanies a replicas diff expectation, leaving a later reader
to guess what before refers to.
Code

internal/helm/manifest_redaction_test.go[67]

+		"-  replicas: \"1\"",     // non-Secret documents diff as before
Evidence
The added inline comment uses as before to refer to prior behavior, which the checklist disallows
in code comments.

Rule 3036538: Disallow references to tickets or PR history in code comments
internal/helm/manifest_redaction_test.go[67-67]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An added test comment describes the expectation by referring to unspecified past behavior.
## Fix Focus Areas
- internal/helm/manifest_redaction_test.go[67-67]
## Recommended Fix
Remove `as before`, or replace the comment with a present-tense constraint if one needs explanation.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Two diff comments repeat the assertions ✓ Resolved
Description
The inline comments on the dbUrl and password expectations restate what the asserted diff lines
show. Neither adds a reason for the expectation or a constraint that would help when the test
changes.
Code

internal/helm/manifest_redaction_test.go[R65-66]

+		"+  dbUrl: '[REDACTED]'", // a key added to the Secret still shows
+		"  password: ",           // an unchanged key stays as context
Evidence
Each added comment describes only the observable meaning of its immediately adjacent expected diff
string.

Rule 3036542: Avoid explanatory comments that restate obvious code behavior
internal/helm/manifest_redaction_test.go[65-66]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Two added comments merely translate their adjacent diff expectations into prose.
## Fix Focus Areas
- internal/helm/manifest_redaction_test.go[65-66]
## Recommended Fix
Remove the two inline comments; retain the assertions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Chart docs misstate who Diagnose reads as ✓ Resolved
Description
The new README section, values.yaml comment, cloud-rbac-ai.yaml header and
docs/authentication.md all say every Diagnose run, manual or background, reads as radar:ai
"whoever started it". They also say that with cloud.aiRbac=false Diagnose falls back to what
radar:viewer sees. The PR's own design is different: manual runs read as the person who started
them, and background runs send only radar:ai with no tier group, so aiRbac=false leaves the AI
with no access at all. Operators reading these docs will expect aiRbac or a custom radar:ai
ClusterRole to limit manual Diagnose, which it does not, and will get the wrong idea of what turning
it off does.
Code

deploy/helm/radar/README.md[R187-197]

+Every Radar Cloud AI Diagnose run, manual or background, reads the cluster as
+the `radar:ai` group, whoever started it. `cloud.aiRbac` (default `true`) binds
+that group to `view` plus the cluster-read and integration-read add-ons above.
+No Secrets, no writes. MCP clients (Claude Desktop, Cursor) are not affected:
+they read with the user's own permissions.
+
+It is independent of `cloud.defaultRbac`, so Diagnose works when the role
+bindings are off. To narrow what the AI reads, set `cloud.aiRbac=false` and bind
+your own ClusterRole to `radar:ai`. With `cloud.aiRbac=false` and no binding of
+your own, Diagnose sees what `radar:viewer` sees, which is nothing on clusters
+without the role bindings.
Evidence
The PR description says: "Manual Diagnose runs read as the person who started them... Background
runs send radar:ai with no tier group, so the AI's access is exactly this binding:
cloud.aiRbac=false really turns it off". The added README lines 187-188 say "Every Radar Cloud AI
Diagnose run, manual or background, reads the cluster as the radar:ai group, whoever started it",
and lines 195-197 say "With cloud.aiRbac=false ... Diagnose sees what radar:viewer sees". The same
wording appears in values.yaml lines 539-541, cloud-rbac-ai.yaml lines 4-5 and
docs/authentication.md line 236.

deploy/helm/radar/README.md[187-197]
deploy/helm/radar/values.yaml[539-546]
deploy/helm/radar/templates/cloud-rbac-ai.yaml[4-13]
docs/authentication.md[236-236]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new docs say every Diagnose run (manual and background) reads as radar:ai, and that aiRbac=false falls back to radar:viewer. By design, manual runs read as the person who started them and background runs read only as radar:ai, so aiRbac=false gives background runs no access.
## Fix Focus Areas
- deploy/helm/radar/README.md[187-197]
- deploy/helm/radar/values.yaml[539-546]
- deploy/helm/radar/templates/cloud-rbac-ai.yaml[1-14]
- docs/authentication.md[236-236]
- deploy/helm/radar/templates/cloud-rbac-system.yaml[5-9]
## Recommended Fix
Rewrite the text to say that background (alert-triggered) Diagnose runs read as radar:ai and manual runs read with the starting user's own permissions. State that with cloud.aiRbac=false and no custom binding, background Diagnose has no cluster access. It does not fall back to radar:viewer.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread internal/helm/manifest_redaction_test.go Outdated
Comment thread internal/helm/manifest_redaction_test.go Outdated
Comment thread internal/helm/client.go
Comment thread internal/mcp/tools_helm.go
Comment thread pkg/ai/context/redact.go
Comment thread deploy/helm/radar/README.md Outdated
Comment thread deploy/helm/radar/templates/cloud-rbac-ai.yaml Outdated
@nadaverell
nadaverell merged commit 2a4ffb1 into skyhook-io:main Oct 5, 2026
10 checks passed
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.

2 participants