B3 PR-B: approval-gated Apply Fix UI over the privileged core#1033
Merged
Conversation
Exposes PR-A's privileged remediation executor through a gated Dashboard UI: the operator reviews a PLAN_REGRESSION finding's force-plan remediation and applies it behind a confirm modal. Pure presentation + orchestration over the already-reviewed core; no change to PR-A's mutation/audit/gate logic. Six security properties (each enforced in code): 1. ONLY PATH is the gated flow. The UI reaches the executor solely via RemediationApplyService.ApplyAsync -> read-only preflight (display) -> confirm modal -> handler.ApplyAsync (which re-gates on one connection, PR-A). Apply runs only from the operator's button click; no auto-apply, no apply-on-load, no batch-without-per-action-confirm. 2. No-caller guard -> reachability-with-gate. PR-A's NoCaller_ForceUnforce_OnlyReferencedInRemediationCore is REPLACED (not deleted) by: CoreMachinery_OnlyReferencedInRemediationCore (the machinery types + force/unforce stay core-only), GatedEntry_ReferencedOnlyBySanctioned UiPath (RemediationApplyService is referenced only by the 4 sanctioned UI files), and behavioural Gate_ConfirmFalse/True tests (the service cannot reach handler.ApplyAsync unless confirm() == true). 3. Fail-closed server resolution (M3). ServerId restored onto AlertHistoryDisplayItem + mapped. Resolution: exact GUID -> unique ServerName -> ambiguous/unresolved/int-id-fallback => Apply HARD-DISABLED with a tooltip. Never silently picks a server. 4. Confirm modal (M2 + five W's). Shows the exact SQL preview, server + per- target database, executing identity (SUSER_SNAME) + operator, the original regression_factor, and an explicit "freshness != still-better; verify against current data" caveat (surfaces FactAdvice.cs:393-394, not just the preview comment). 5. Result surfacing incl. LOW-2. Per-target success / skip / blocked (audit-absent -> "upgrade to 2.12.0") / permission-denied / applied-but- unlogged. A PERMANENT audit-INSERT failure (login lacks INSERT on config.remediation_action_log) is classified by a read-only probe and surfaced LOUDLY (bold/error) vs a transient warning. 6. Never automatic. The confirm gate lives inside RemediationApplyService.RunAsync, before the only call to the privileged handler. No elevation/credential prompt (existing monitoring connection only, per PR-A). New: Services/Remediation/RemediationApplyService.cs (gated facade + M3 resolution), RemediationApplyModels.cs (UI DTOs), AuditWritabilityProbe.cs (LOW-2 classifier); RemediationConfirmWindow.xaml(.cs) (confirm modal). Modified: AlertDetailWindow (Apply/Un-apply + results), AlertsHistoryContent (ServerId + wiring), MainWindow (construct + inject service). Tests: 84 pass (was 68; replaced 1 no-caller test with 2 reachability tests + 15 PR-B tests). Real-server (sql2022, 2.12.0 schema) via the full production gated path: Apply forced plan + force|success audit row; Un-apply unforced + unforce|success row; confirm=false -> NotConfirmed, no mutation; absent-table (monitoring conn at tempdb) -> Blocked with the 2.12.0 message, no mutation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
PR-B of B3 "Apply Fix": the gated Dashboard UI over the privileged force-plan executor merged in PR-A (#1032). A Dashboard operator reviews a
PLAN_REGRESSIONfinding's remediation in the alert detail dialog and clicks Apply Fix… behind a confirm modal. This PR is pure presentation + orchestration over the already-reviewed core — no change to PR-A's mutation/audit/gate logic.Six security properties (and how each is enforced)
1. The ONLY path to the executor is the gated flow.
The UI reaches the executor solely through
RemediationApplyService.ApplyAsync→ read-only preflight (display) → confirm modal →handler.ApplyAsync(which re-gates on one connection per PR-A's R2-MOD-1).ApplyAsync/UnapplyAsyncrun only from the operator's button click — there is no auto-apply, no apply-on-load, and no batch-without-per-action-confirm. The confirm gate lives insideRunAsync, immediately before the single call site of the privileged handler.2. No-caller guard → reachability-with-gate (replaced, not deleted).
PR-A's
NoCaller_ForceUnforce_OnlyReferencedInRemediationCorenecessarily fails now (PR-B adds a legitimate caller). It is replaced with three assertions that prove reachable only through the gate:CoreMachinery_OnlyReferencedInRemediationCore— the machinery types +ForcePlanAsync/UnforcePlanAsyncare still referenced only insideServices/Remediation/; the UI never touches them directly.GatedEntry_ReferencedOnlyBySanctionedUiPath— the single gated facadeRemediationApplyServiceis referenced outside the core only by the 4 sanctioned UI files (MainWindow,AlertsHistoryContent,AlertDetailWindow,RemediationConfirmWindow). A new reference fails the build.Gate_ConfirmFalse_NeverReachesHandler/Gate_ConfirmTrue_ReachesHandlerExactlyOnce— behavioural proof the service cannot reachhandler.ApplyAsyncunlessconfirm()returnstrue.3. Fail-closed server resolution (M3).
ServerIdis restored ontoAlertHistoryDisplayItemand mapped fromAlertLogEntry.ServerId. Resolution: exact GUID match → unique ServerName match → ambiguous / unresolved / int-id-fallback ⇒ Apply HARD-DISABLED with a tooltip. Never silently picks a server. Unit-tested incl. the int-id-fallback case fromMainWindow.xaml.cs:148-150.4. Confirm modal (M2 + the five W's).
RemediationConfirmWindowshows the exact SQL preview, server + per-target database, the executing identity (SUSER_SNAME()from preflight) + the operator, the originalregression_factor, and an explicit caveat: freshness proves plan-present/forceable — NOT that the forced plan is still better; verify against current data. This surfacesFactAdvice.cs:393-394in the modal rather than relying on the operator reading the preview comment.5. Result surfacing incl. LOW-2.
Per-target outcomes: success / skipped / blocked (audit-table-absent → "upgrade this server to 2.12.0") / permission-denied (fail-closed grant guidance, no elevation prompt) / applied-but-unlogged. LOW-2: a permanent audit-INSERT failure (login lacks
INSERTon a presentconfig.remediation_action_log) is distinguished from a transient one by a read-onlyAuditWritabilityProbeand surfaced loudly (bold/error: "every Apply will go unlogged until fixed") vs a soft transient warning — so a force-succeeds-nothing-ever-logged misconfiguration is visible, not buried.6. Never automatic.
No path applies without the operator's explicit per-action confirm. No elevation/credential prompt — the existing per-server monitoring connection only (PR-A's model). B4 (fully automatic) stays out.
Files
New —
Dashboard/Services/Remediation/RemediationApplyService.cs(gated facade + M3 resolution),RemediationApplyModels.cs(UI DTOs),AuditWritabilityProbe.cs(LOW-2 read-only classifier);Dashboard/RemediationConfirmWindow.xaml(.cs)(confirm modal).Modified —
AlertDetailWindow.xaml(.cs)(Apply/Un-apply buttons, results, server resolution),Controls/AlertsHistoryContent.xaml.cs(ServerId+ service threading),MainWindow.xaml.cs(construct + inject the service).The UI references only the gated facade + result DTOs — never the handler/registry/executor types or the force/unforce methods.
Verification
Build: Dashboard + test projects clean (
CS4014is WarningsAsErrors; 0 errors). The one build warning is PR-A's pre-existingFakeExecutor.UnforceFunc.Tests:
dotnet test84 passed / 0 failed (baseline 68 → replaced 1 no-caller test with 2 reachability tests + 15 new PR-B tests: gate, M3 resolution, applied-but-unlogged permanent-vs-transient,CanApplyview-model gating).Real-server (sql2022, 2.12.0 schema present) via the full production gated path (
RemediationApplyService→ForcePlanHandler→DatabaseServiceRemediationExecutor→DatabaseService.Remediation→ live DB), targetStatsAsymTestquery_id 1 / plan_id 1:is_forced_plan0→1, audit rowforce | success | sa | StatsAsymTestunforce | success | sa | StatsAsymTestNotConfirmed, 0 targets, plan unchanged (gate held)Blockedwith the 2.12.0 upgrade message, no mutationis_forced_plan=0).The literal mouse-driven click-through of the WPF dialog remains a manual spot-check for a human; the underlying gated path the dialog invokes is verified end-to-end above.
🤖 Generated with Claude Code