fix(mcp): don't stamp no_recent_changes when RBAC hid correlated evidence - #1322
Merged
Conversation
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.
Problem
Found while auditing the #1300/#1302 RBAC work for drop-vs-degrade behavior. Per-issue change correlation (
internal/mcp/issue_correlation.go) filters a workload issue's correlated changes by per-kind RBAC — a workload subject's candidates include its consumed ConfigMaps, which the caller may not be able to read (#1300). But when that filter empties the list, the code fell through to stamping the issue with an affirmativeno_recent_changesmarker.The marker's own contract says otherwise — fetch errors and saturated fetches deliberately omit it ("marker omitted = unknown, never a false 'no changes'"). RBAC-hidden is the same epistemic state: evidence exists; this caller can't see it. The failure mode is concrete: a user with workload access but no ConfigMap access gets their crashing Deployment certified "no recent changes in the window — chronic issue" when its consumed ConfigMap rotated two minutes ago, and MCP consumers (including AI diagnosis) explicitly weigh that marker.
Fix
applyCorrelationVisibilityFiltersapplies the category filter first, then the RBAC filter, and reports whether RBAC removed a relevant row. When the visible set is empty,rbacHiddenjoinssaturatedin omitting the marker — unknown, not "no changes". Ordering matters: a status-churn row dropped by the category filter must not suppress the marker for a genuinely quiet subject.CorrelatedChangesbehavior is unchanged (still only rows the caller may read); auth-off is unchanged (no user → RBAC filter is a passthrough →rbacHiddenis always false).Tests
TestApplyCorrelationVisibilityFilters_RBACHiddenIsNotNoChanges: unreadable ConfigMap as the only relevant change → empty + hidden (marker suppressed); readable Deployment + status-churn row → visible + not hidden (quiet subjects still earn the marker); auth-off passthrough.go build,go vet, fullgo test ./...green.Note
Medium Risk
Changes triage marker semantics for MCP/issue consumers (including diagnosis), but scope is limited to the correlation visibility path and aligns with existing “omit = unknown” behavior for errors and saturation.
Overview
Fixes a false no_recent_changes stamp when per-kind RBAC removes correlated evidence (e.g. a consumed ConfigMap the caller cannot list) but the issue subject is still readable.
Correlation now runs
applyCorrelationVisibilityFilters, which applies spec/lifecycle filtering first, then RBAC, and setsrbacHiddenwhen relevant rows were dropped for permissions. If the visible set is empty,rbacHiddenis treated like a saturated fetch: the marker is omitted (unknown), not an affirmative “nothing changed.” Status-only churn still does not count as hidden, so genuinely quiet subjects still get the marker.Adds
TestApplyCorrelationVisibilityFilters_RBACHiddenIsNotNoChangesfor hidden ConfigMap, readable Deployment + status churn, and auth-off passthrough.Reviewed by Cursor Bugbot for commit cd705f9. Bugbot is set up for automated code reviews on this repo. Configure here.