Repository navigation
Authorize audit Secret and Crossplane subjects before evaluation - #1753
Merged
Merged
Conversation
PR Summary by QodoAuthorize audit subjects before evaluation
AI Description
Diagram
High-Level Assessment
Files changed (18)
|
Code Review by Qodo
1. Cluster-only readers lose audit results
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
nadaverell
force-pushed
the
fix/audit-subject-authorization
branch
from
September 14, 2026 13:50
3936ba8 to
b8b766f
Compare
|
â Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing |
nadaverell
added a commit
that referenced
this pull request
Sep 15, 2026
## Why A ConfigMap or Secret can look unused because Radar cannot see its consumer. For example, a workload can consume a Reflector mirror in another namespace while the audit is displaying only the source namespace. Suggesting deletion from that incomplete view is misleading. This PR makes the unused ConfigMap/Secret check distinguish observed use, supported evidence of no use, and unknown evidence. It builds on the subject authorization and incomplete-scan presentation merged in #1753, and reuses the Reflector metadata parser from #1740. It has no remaining prerequisite on the deferred discovery work in #1750. ## Behavior | Available evidence | Result | |---|---| | An observed supported reference uses the object | Count it as evaluated and passing, even if other consumer inventories are unavailable | | No reference, with adequate readable and initially synced consumer inventories | Report the object as unused | | No reference, with a required inventory unavailable or incomplete for the subject namespace | Exclude it from both findings and passing/evaluated counts; disclose incomplete reference evidence | The namespace picker selects subjects. For Reflector-annotated subjects, the collector can inspect authorized consumers of remote mirrors. Use propagates through observed same-kind links, including chains and cycles. Unused automatic mirrors with visible sources are exempt, but their existence does not establish source use. Typed workload, ServiceAccount and Ingress inventories use the existing per-kind readiness and namespace-scope APIs. The existing dynamic reference handlers share a registry with their ConfigMap/Secret and cross-namespace requirements, so discovery and collection agree about which installed integrations matter. An installed but unsynced consumer kind is unknown, not empty. Partial discovery of a relevant group prevents an absence claim; unrelated discovery failures do not. Cert-manager Issuer credential references cover ACME, CA, Vault and Venafi fields. ClusterIssuer credentials use the namespace established by an observed controller's explicit flag, including a literal or downward-API environment value. Hidden controllers, unresolved values or configuration files without an explicit namespace flag do not produce a guessed namespace. Explicit flags take precedence over configuration files. An authorized, synced, empty ClusterIssuer inventory needs no controller namespace. ## Scope and tradeoffs The runtime change stays in the audit collector and orphan check. It adds no shared discovery lifecycle, informer failure latch, permission grants, new watches, scan-cache redesign or coverage response schema. Existing `missingInputs` carries `configmap-references` and `secret-references` through REST, Home, AI context and MCP. Tests in the HTTP/MCP adapters keep their authorization assertions while explicitly supplying the core-only inventory they depend on. Full-access users can also lose unsupported orphan findings while required inventories are still syncing. Restricted users can see fewer orphan evaluations when an installed integration can reference Secrets across namespaces. This is deliberate: an omitted unknown evaluation is preferable to an unsupported deletion suggestion. Known use still counts. The check covers supported reference fields, not arbitrary controller behavior. Existing Istio gateway workload-namespace and Contour delegated TLS resolution limitations are outside this change. Readiness here means initial synchronization, not continuous watch freshness. The existing scan memo can lag evidence changes by up to five seconds. The per-resource audit endpoint retains its findings-array shape and has no completeness metadata; the scan and AI resource context provide that distinction. Package callers that omit `ConfigReferenceEvidence` can establish observed use but cannot establish absence. ## Validation - `make test`, `make tsc`, and `make build` passed. - Race tests passed for `pkg/audit`, `pkg/configrefs`, and the affected audit/server/MCP scenarios. - Live tests used the existing GitOps demo cluster with an explicit kubectl context. Local and authenticated full-access results matched exactly; Secret-denied identities disclosed missing inputs without Secret findings. Alternating and concurrent identities retained separate results. - A temporary ConfigMap-only Reflector fixture exercised the central case. Complete visibility evaluated two subjects, with the remotely consumed source passing and the unused ConfigMap reported. A source-namespace-only cache evaluated only the unused ConfigMap and reported `configmap-references` for the unknown source. Configured REST, dashboard, MCP and the rendered Checks page preserved that warning. - Live test fixtures and processes were removed. No Kubernetes grants or current-context changes. One actual embedded-UI screenshot verified the warning; no frontend source changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Radar can cache more resources than an authenticated viewer may read. Previously, audit could evaluate cached Secrets and cluster-scoped Crossplane resources without checking that viewerâs grants, exposing findings and evaluation counts about hidden subjects. This PR applies those checks before evaluation and carries incomplete-input warnings through the audit surfaces.
Authorization and behavior
Secret subjects require
list secretsin their namespace. Namespace-local grants remain valid when cluster-wide listing is denied. Cluster-scoped Crossplane subjects requireliston their exact API group/resource. Other namespaced subjects retain Radarâs existing namespace-visibility policy.HTTP audit (including raw and per-resource reads), Home, AI context, MCP audit, and MCP resource summaries use the same subject scope. The existing five-second HTTP memo includes authorization scope and typed/dynamic cache identity, preventing one callerâs results from satisfying another callerâs differently scoped request.
Local and fully authorized scans retain their findings and evaluation counts. Restricted callers lose unauthorized subjects and their counts. Existing
missingInputsmetadata now reaches Home, Checks, MCP and AI/resource context, including when no findings remain. Partial Secret grants are disclosed; namespace-local grants covering the whole ready namespace inventory do not produce a false warning.Home and Checks show neutral empty states when nothing was evaluated, and qualify results when inputs are missing. Fully evaluated clean scans retain their positive state. These presentation changes also affect full-access users when their scan evaluates nothing. The shared UI gains optional coverage props; downstream hosts need to supply them to display missing-input warnings. Per-resource REST audit responses remain arrays.
Scope and risk
This is the subject-authorization prerequisite for #1741. It does not solve whether orphan checks have enough consumer evidence to assert that a resource is unused. That broader coverage work remains separate. No discovery, informer lifecycle, Kubernetes grant, or cluster-write behavior changes.
Risk is medium and concentrated in audit reads: incorrect scoping could hide valid findings, disclose hidden subjects through counts, or reuse another callerâs results. Tests cover these boundaries and live requests verify identity isolation. Full-access backend results are preserved. Cold permission checks still fan out across candidate namespaces and watched resource types with bounded concurrency; the existing single-slot memo can recompute across differing grant profiles. Large-cluster cold latency is not benchmarked here. Existing permission-cache invalidation timing and other kindsâ namespace-based authorization policy remain unchanged.
Coverage warnings remain scan-wide: namespaces later excluded by local settings can still trigger them, and a terminating namespace can briefly trigger one after its grants disappear. The absence of a warning does not certify that Radarâs own cache covers every namespace; that existing limitation belongs to the separate coverage work in #1741.
Validation
All live checks were read-only against the cluster. The current kubeconfig context was preserved, temporary Radar/proxy processes were stopped, and no grants or fixtures were changed.