Skip to content

B3 Phase 3 PR-A: RCSI destructive-consent privileged core (unregistered)#1040

Merged
erikdarlingdata merged 1 commit into
devfrom
feature/b3-phase3-rcsi
Jun 1, 2026
Merged

B3 Phase 3 PR-A: RCSI destructive-consent privileged core (unregistered)#1040
erikdarlingdata merged 1 commit into
devfrom
feature/b3-phase3-rcsi

Conversation

@erikdarlingdata
Copy link
Copy Markdown
Owner

What ships (PR-A — dead-code-safe, unregistered)

The privileged core for enabling READ_COMMITTED_SNAPSHOT (RCSI) behind an informed-consent destructive gate, kept UNREACHABLE exactly like the Phase-1/2 PR-A pattern: RcsiHandler is NOT registered and the "Enable RCSI" detail item is NOT emitted. The acknowledge-each-risk dialog rendering, registry registration, second-detail-item emission, and cross-surface disclosure are PR-B.

  • Risk model + builder (PerformanceMonitor.Analysis): RiskItem/RiskDisclosure records; FactRiskDisclosure.GetForAction(action, finding) builds the two-sided, honest-both-directions content — risks of changing (fixed prose + validated identifier) and risks of NOT changing (fixed framing filled with the server's monitoring figures, with the writer/writer "RCSI won't resolve this" branch and the no-data weak-case baseline).
  • Drill-down enrichment in BOTH collectors: rcsi_blocking_events (int), rcsi_deadlocks (int), rcsi_reader_writer_pct (int 0–100, nullable), emitted ONLY for RCSI-off databases with identical JSON names + types. Dashboard sources counts from the pre-aggregated collect.blocking_deadlock_stats (O-P3-F) and the reader/writer split from a separate pass over collect.blocking_BlockedProcessReport classified against the FULL lock_mode vocabulary (M-1). Lite emits 0/0/null (M-2).
  • Executor RCSI enum arm: DbConfigSetting.ReadCommittedSnapshotOn wired through all four switch sites (m-1) + the SettingTitle arm (m-2); reuses the Phase-2 SetDatabaseOptionAsync machinery unchanged.
  • RcsiHandler : IRemediationHandler: FactKey => "RCSI", IsDestructive => true (first true in the codebase), SupportsUnapply => false, UnapplyAsync throws. Same audit-absent-hard-block / single-connection-gate / one-row-per-attempt skeleton as DbConfigHandler. NOT registered.
  • FactRemediation.BuildRcsiAction (parallel to the unchanged BuildAction): emits the RCSI action only on rcsi == false (RCSI OFF — M2-1 polarity) + enrichment present.
  • RemediationConfirmRequest += RequiresInformedConsent + Risks; BuildConfirmRequest signature change to take the resolved handler (+ finding) (B-1) + the RunAsync call-site update.
  • consent_acknowledged: idempotent COL_LENGTH-guarded ALTER appended after the create block (B-3, NOT a create-body amend) + the four-file plumbing (M-3): RemediationAuditRecord.ConsentAcknowledged, a real bound @consent_acknowledged Bit param in the writer, RcsiHandler BuildRecord => true, DbConfig + force-plan => false.
  • CoreMachineryMarkers += "RcsiHandler"; all headless §9 unit tests.

Security invariants enforced (file:line)

  • Identifier safety — only non-constant token is the validated+bracketed DB identifier; SET clause is the hardcoded literal SET READ_COMMITTED_SNAPSHOT ON (Dashboard/Services/DatabaseService.Remediation.cs SetClauseFor arm ~270, BuildAlterStatement ~279); existence via parameterized sys.databases check (ReadDbConfigGateAsync ~441-452); same-string ValidatedName (~465). Executor builds its own statement — preview never executed.
  • Single-connection self-gating (R2-MOD-1) — gate (existence + fail-closed ISNULL(HAS_PERMS_BY_NAME(@db,'DATABASE','ALTER'),0) + is_read_committed_snapshot_on freshness) and the ALTER on ONE monitoring connection (SetDatabaseOptionAsync ~314-388); desired state = on → Skip (new gate arm ~433-440); GateSpid/ExecSpid emitted.
  • Fail-closed permsRcsiHandler clones the DbConfigHandler PermissionDenied path; no elevation, no InitialCatalog retarget, no credential handling.
  • Audit — audit-absent hard block before any mutation (RcsiHandler.RunApplyAsync); one audit row per attempt; consent_acknowledged = true only on a gated RCSI apply/skip (RcsiHandler.BuildRecord), false for DbConfig (DbConfigHandler.cs) and force-plan (ForcePlanHandler.cs).
  • Dead-code-safeRcsiHandler unregistered (RemediationApplyService.cs:55 unchanged); RCSI second detail item un-emitted; asserted by Rcsi_Handler_IsUnregistered_DeadCodeSafe + CoreMachinery_OnlyReferencedInRemediationCore.
  • Boolean polarity (M2-1)BuildRcsiAction emits on JsonValueKind.False only (FactRemediation.cs), mirroring the existing rcsiOffDatabases scan.
  • Lock-mode classifier (M-1) — reader = S/IS/RangeS-%; writer = X/IX/U/UIX/RangeX-%/RangeI-%; Sch-S/Sch-M/BU excluded; SQL CASE in the Dashboard collector mirrors RcsiLockModeClassifier, golden-tested over the full vocabulary.
  • Idempotent ALTER (B-3)IF COL_LENGTH(...) IS NULL ALTER TABLE ... ADD consent_acknowledged bit NOT NULL CONSTRAINT df_... DEFAULT (0) appended after the guarded create block; correct on both fresh and pre-existing tables.

Test results (real)

  • dotnet build Dashboard/Dashboard.csproj -c Debug — clean (0 warnings, 0 errors).
  • Shared libs (Analysis, Notifications) — clean.
  • Dashboard.Tests: Passed 175, Failed 0, Skipped 0.
  • Lite.Tests: Passed 312, Failed 0, Skipped 0.

Maintainer real-server pre-merge gate (sql2022, §9 — NOT run here)

  1. Scratch DB RCSI OFF + induced reader/writer blocking → RCSI affordance shows real counts + high reader-writer %; all boxes → Apply → is_read_committed_snapshot_on = 1, audit action set_rcsi_on, prior_value='OFF', consent_acknowledged=1. Re-apply → freshness-skip.
  2. Scratch DB RCSI OFF, predominantly writer/writer → disclosure shows low reader-writer % + "RCSI won't resolve this" (honest-both-directions).
  3. db_owner-of-PerformanceMonitor-only login → PermissionDenied, no ALTER.
  4. Hold an open txn in the target DB, attempt apply → ALTER waits then errors on timeout; per-target Error, no partial state.
  5. Audit-absent + DB-not-found behave as Phase 2.

Do NOT merge / register the handler / do PR-B. Re-run the tests, read the privileged core, and run a security-reviewer pass on the diff before anything lands.

🤖 Generated with Claude Code

Implements the privileged core for enabling READ_COMMITTED_SNAPSHOT (RCSI) behind
an informed-consent destructive gate, kept UNREACHABLE (dead-code-safe) exactly like
the Phase-1/2 PR-A pattern: RcsiHandler is NOT registered and the "Enable RCSI"
detail item is NOT emitted. The acknowledge-each-risk dialog rendering, the registry
registration, and the cross-surface disclosure are deferred to PR-B.

What ships (PR-A):
- Shared Analysis lib: RiskItem / RiskDisclosure records; FactRiskDisclosure.
  GetForAction(action, finding) builds the two-sided, honest-both-directions risk
  content (risks of changing = fixed prose + validated identifier; risks of NOT
  changing = fixed framing filled with this server's monitoring figures, with the
  writer/writer "RCSI won't resolve this" branch and the no-data weak-case baseline).
- Drill-down enrichment in BOTH collectors: rcsi_blocking_events (int),
  rcsi_deadlocks (int), rcsi_reader_writer_pct (int 0-100, nullable), emitted ONLY
  for RCSI-off databases with identical JSON names + types. Dashboard sources counts
  from the pre-aggregated collect.blocking_deadlock_stats (O-P3-F) and the
  reader/writer split from a separate pass over collect.blocking_BlockedProcessReport
  classified against the FULL lock_mode vocabulary (M-1). Lite emits 0/0/null (M-2).
- Executor RCSI arm: DbConfigSetting.ReadCommittedSnapshotOn wired through all four
  switch sites (m-1: executor SetClauseFor, executor gate ReadDbConfigGateAsync
  is_read_committed_snapshot_on freshness, service SetClauseFor, FactRemediation
  .StatementFor) plus the SettingTitle arm (m-2). Reuses the Phase-2
  SetDatabaseOptionAsync machinery UNCHANGED.
- RcsiHandler : IRemediationHandler (FactKey "RCSI", IsDestructive => true — the
  first true in the codebase, SupportsUnapply => false, UnapplyAsync throws). Same
  audit-absent-hard-block / single-connection-gate / one-row-per-attempt skeleton as
  DbConfigHandler. NOT registered.
- FactRemediation.BuildRcsiAction (parallel to the unchanged BuildAction); emits the
  RCSI action only on rcsi == false (RCSI OFF — M2-1 polarity) + enrichment present.
- RemediationConfirmRequest += RequiresInformedConsent + Risks; BuildConfirmRequest
  signature change to take the resolved handler (+ finding) (B-1) and the RunAsync
  call-site update.
- consent_acknowledged: idempotent COL_LENGTH-guarded ALTER appended after the create
  block (B-3, NOT a create-body amend) + the four-file plumbing (M-3):
  RemediationAuditRecord.ConsentAcknowledged, a real bound @consent_acknowledged Bit
  param in the writer, RcsiHandler BuildRecord => true, DbConfig + force-plan => false.
- CoreMachineryMarkers += "RcsiHandler"; all headless §9 unit tests.

Security invariants enforced (all reused verbatim from Phase 2 + extended to RCSI):
- Identifier safety: the only non-constant token in the RCSI ALTER is the
  validated+bracketed DB identifier; SET clause is the hardcoded literal
  "SET READ_COMMITTED_SNAPSHOT ON"; existence validated by a parameterized
  sys.databases check; same-string ValidatedName bracketed; executor builds its own
  statement (never the rendered preview).
- Single-connection self-gating (R2-MOD-1): gate (existence + fail-closed
  HAS_PERMS_BY_NAME(@db,'DATABASE','ALTER') + is_read_committed_snapshot_on freshness)
  and the ALTER on ONE connection; desired state = on => Skip.
- Audit-absent hard block before any mutation; one audit row per attempt; per-target
  independence; no elevation, no InitialCatalog retarget, no credential handling.

Tests: Dashboard.Tests 175/175 pass, Lite.Tests 312/312 pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@erikdarlingdata erikdarlingdata merged commit b82ed38 into dev Jun 1, 2026
2 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.

1 participant