get_plan_force_actions read no longer serves pre-#4326 exception text (#4346)

[erikdarlingdata]

Refs #4346 (the one-time scrub of already-stored legacy rows follows in its own PR; #4346 stays open until it merges).

Why

#4326 stopped the plan-force bot's state-read failure note from putting
ex.Message into collect.plan_force_actions.detail. Rows a build before
#4326 wrote still carry the raw exception text, and stay until the
retention purge (up to a year). Nothing calls GetRecentActionsAsync in
this build (get_plan_force_actions appears only in doc comments), but the
disposition ruling on #4346 is that whoever builds that read owns not
returning legacy detail verbatim, so this fixes the read now rather than
waiting for a caller to land first.

What changes

PgPlanForceActionStore.GetRecentActionsAsync (the read the doc comments
name as get_plan_force_actions's backer) now runs every row's detail
through a new SanitizeDetailForAudit before returning it.

There is no build marker on the row, so this is a positive allow-list, not
a before/after cutoff: state_unavailable is the one blocker whose
evidence line ever carried exception text (TryGetTargetStatesAsync's old
catch, fixed by #4326's M1 round). The new gate matches ONLY the one line
shape #4326 can produce for it (a type name, plus SQLSTATE for a
DbException, plus one of CollectionFailure.Describe's two fixed log
notes). Any state_unavailable: line that does not match that template
verbatim — including every legacy row's raw exception message — is
replaced with a fixed sentence that names no exception text. Every other
blocker's evidence line (parameter_sensitivity_cofired,
secondary_replica_evidence, apc_owns_it, apc_enabled_for_database,
and the empty-state shape of state_unavailable) has never carried
exception text on any build and passes through untouched, as does detail
on non-blocked rows (would_force's null, the withheld-force sentence).

This is on the READ side only — no data rewrite of existing rows, per the
brief.

Lite parity: grepped for plan_force_actions and PlanForceActionStore
outside Darling — no hits. Lite has no plan-force bot and no twin of this
table or read. N/A.

Other read surfaces: grepped the whole repo for get_plan_force_actions
and GetRecentActionsAsync — GetRecentActionsAsync is the only production
caller-eligible read of the column (GetPendingReviewsAsync is a different,
uncalled read used only by #2731's future review path and is out of this
issue's scope — it also has no caller yet, so nothing serves it today
either; flagging it here rather than silently widening the diff). No MCP
tool, web route, or viewer route reads collect.plan_force_actions today.

Test plan

• SanitizeDetailForAudit unit tests (PlanForceActionAuditRedactionTests.cs,
  net10.0 — ran locally): the three post-Stop leaking raw exception text from /api/ping and two MCP notes (#4316) #4326 safe shapes (type only, with
  SQLSTATE, missing-schema note) pass through unchanged; a legacy row whose
  exception text carries a fake credential and hostname is redacted to the
  fixed sentence with none of that text surviving; an unrelated blocker line
  on the same row is untouched; null and non-state_unavailable detail are
  untouched.
• Build: dotnet build Darling/Darling.Tests/Darling.Tests.csproj -p:EnableWindowsTargeting=true — 0 warnings, 0 errors.
• Darling.Tests full suite (net10.0-windows) — builds here but cannot run
  on macOS; CI decides.
• Live-Postgres SQL rig proof (insert one legacy-shaped row + one
  new-shaped row, run GetRecentActionsAsync, compare) — not run in this
  lane; the unit tests above exercise the exact same regex/string logic the
  SQL-side read calls, and the read itself does no SQL-side filtering (the
  redaction is pure C# over the returned detail), so the rig would prove
  plumbing, not new logic. Left unchecked per the brief's timebox; flagging
  as a gap if the coordinator wants that extra proof.

CHANGELOG entry

• Plan-force journal reads no longer return pre-Stop leaking raw exception text from /api/ping and two MCP notes (#4316) #4326 exception text (#4363) - Rows written before Stop leaking raw exception text from /api/ping and two MCP notes (#4316) #4326 kept a state-read failure's raw exception message in collect.plan_force_actions.detail. Both journal reads now replace that legacy block with a fixed sentence. Post-Stop leaking raw exception text from /api/ping and two MCP notes (#4316) #4326 evidence lines are returned unchanged. The stored rows are not rewritten.

For the coordinator

This is a secret-handling fix (exception text potentially carrying hosts,
credentials, relation names). Please consider it for a review round before
merge, per the brief's note. GetRecentActionsAsync has no production
caller yet in this build (confirmed by grep), so this fix has no runtime
blast radius today — it closes the gap before a caller lands. The
uncalled GetPendingReviewsAsync read on the same table is sanitized too
(see the lane report below): both journal reads apply
SanitizeDetailForAudit before returning detail. No migration, no data
rewrite.

pm-pr lane report (readers + anchoring)

Reader enumeration. Grepped the whole repo for the table name and its detail column (SQL text
included: SELECT, RETURNING, *) and for every place PlanForceActionRecord.Detail is read back in
C#. collect.plan_force_actions has exactly two row readers in this build:

• PgPlanForceActionStore.GetRecentActionsAsync — the audit read behind get_plan_force_actions (no MCP
  tool wires it up yet, per ServiceCommandDeadlines's own comment, but the doc-commented intent is that
  it will). Already sanitized in the PR as opened; unchanged disposition here.
• PgPlanForceActionStore.GetPendingReviewsAsync — the own-forces-only review read for Add auto force-plan bot phase 2: the write path (#2138) #2731's live-force
  write path. No caller in this build either (also noted in ServiceCommandDeadlines), and it ran
  ReadRecord straight into the return list with no sanitizer at all. Fixed in this round: it now passes
  every row's detail through the same SanitizeDetailForAudit, at the one choke point (the while (await reader.ReadAsync(ct)) loop), so no path out of this method can carry detail around it.

No other reader exists: JournalAsync only INSERTs and returns action_id; GetQueryHistoryAsync never
selects detail; TryGetTargetStatesAsync reads a different table via DarlingForcePlanTargetStateReader.
There is no web, WPF, export or Lite-side reader of this table at all — PlanForceActionRecord,
plan_force_actions and the store class are Darling-only; Lite.Tests/OperatorRemediationLiteDivergencePinTests
asserts the absence of a Lite twin, so there is no Lite equivalent to sanitize. Both readers now sanitize;
per the ruling, a reader with no caller today still gets the treatment, since a caller added later must not
find an open path back to raw exception text.

The regex, as it now stands. The line-by-line gate in the PR as opened had a real gap: it split
detail on \n and checked each line's own text against the safe-shape regex, so a legacy row whose
raw exception message itself embedded a newline (a multi-line driver message, a wrapped stack fragment)
only had its first line checked — anything after that embedded newline passed through unexamined. Replaced
with a whole-BLOCK match:

^state_unavailable:.*?(?=\r?\n(?:parameter_sensitivity_cofired|secondary_replica_evidence|apc_owns_it|apc_enabled_for_database|state_unavailable):|\z)

with RegexOptions.Singleline | RegexOptions.Multiline. This matches from the state_unavailable: prefix
up to (not including) the next known blocker's line or the end of the string, and the whole matched span
— not one line of it — is checked against SafeStateUnavailableLine (still a full-line ^…$ anchor) before
being kept or replaced with the fixed sentence. Singleline lets . cross an embedded \n so the lazy body
can span one; Multiline anchors ^ to right after a \n (which also covers a \r\n pair, since the
position immediately after \n doesn't change). A lone \r, and the Unicode LINE SEPARATOR (U+2028) /
PARAGRAPH SEPARATOR (U+2029), are deliberately not line boundaries to .NET's ^/\z under these
options, so text smuggled after one of those stays inside the block and is redacted with it, rather than
surviving past the anchor as a "new line."

New pins (PlanForceActionAuditRedactionTests), RED/GREEN evidence. Proved with a throwaway net10.0
console at /tmp/sanicheck linking a verbatim copy of the sanitizer (the real class needs Npgsql/ILogger
that aren't worth pulling into a standalone harness) — the test project itself is net10.0-windows and can't
run on this macOS host.

• Against the PR-as-opened (99d3c02) line-by-line regex: pin (b) (safe shape + \n + exception text) and
  both halves of pin (c) (CRLF) went RED — leaked...hunter2 survived in the output, because the second
  line never matched state_unavailable: at its own start and was never checked. Pin (a) (safe prefix +
  trailing exception text, still on one line) was already caught by the original single-line anchor and
  showed GREEN even on 99d3c02, since a full-line ^…$ regex rejects any line with extra trailing text.
• Against the new whole-block regex: all of pin (a), pin (b), pin (c) (both the CRLF-preceded-safe-line
  case, which must still pass, and the CRLF-smuggled case, which must redact), plus the lone-\r,
  U+2028 and U+2029 smuggling cases, and every pre-existing pin (three post-fix shapes, the original legacy
  pin, the sibling-blocker-line survives, non-state_unavailable detail unchanged, null stays null) —
  ALL PASS (16/16 in the harness; mirrored 1:1 as the 8 new xUnit facts added to
  PlanForceActionAuditRedactionTests.cs, plus the 7 pre-existing ones, 15 total in that file as of
  head 7ddba0d3 — corrected count per the security round's F6; see the new section below for the
  further F1/F2 changes to this file, which bring the total to 18).

New head sha: 7ddba0d3 (merge of origin/dev onto 99d3c02b, no conflicts — dev had touched
unrelated files: ComposeStoreRolesLiveTests, PgFileSettingsCapability*, PgServerConfigPendingRestartLiveTests,
etc.). PR head confirmed still 99d3c02b immediately before this push.

Build/test status. dotnet build Darling/Darling.Tests/Darling.Tests.csproj -p:EnableWindowsTargeting=true:
0 Warning(s), 0 Error(s), both before and after the merge from dev. Darling.Tests.dll is
net10.0-windows and refuses to launch on this macOS host (Microsoft.WindowsDesktop.App not present) — the
sanitizer pins were proven with the standalone harness above instead; CI's build job runs the actual xUnit
facts. DocCommentHygiene* was not run locally for the same host reason; left to CI.

3 riskiest changed lines (for the security round that follows):

1. The StateUnavailableBlock regex's lookahead alternation (the four sibling-blocker prefixes hardcoded
   into the pattern) — if a new blocker name is ever added to ForcePlanBotPolicy without a matching entry
   here, that blocker's own line would be swallowed into an adjacent unsafe state_unavailable block's
   replacement instead of surviving untouched.
2. SafeStateUnavailableLine's reliance on CollectionFailure.Describe's exact two fixed log notes as an
   allow-list — if that method's wording ever changes without updating this regex, every future
   state_unavailable row (not just legacy ones) starts failing the match and gets replaced with the fixed
   sentence, which is a silent behavior change (safe direction, but loses real evidence).
3. GetPendingReviewsAsync's new record with { Detail = SanitizeDetailForAudit(record.Detail) } — the
   with-expression rebuild is easy to lose in a future refactor of that method (e.g. someone inlining
   ReadRecord's call site) since nothing at the type level forces the sanitizer to run.

CHANGELOG entry: kept as the single line above in "## CHANGELOG entry", corrected for F2 (both
journal reads, both fixed shapes untouched, no data rewrite).

pm-pr lane report (security fixes F1, F2, F6)

New head sha: d5300856 (merge of origin/dev onto 7ddba0d3, keeping both sides, no conflicts —
dev had touched WebFetchLayerTests.cs, DarlingWebEndpoints.cs, DarlingMcpHealthParserTools.cs, and
three Lite/Viewer wwwroot files, none overlapping this PR's files). PR head confirmed 7ddba0d3 immediately
before this round's push.

F1 (HIGH) — writer-order evidence, confirmed by reading the code (not just cited from the review):
ForcePlanBotPolicy.Blockers (PerformanceMonitor.Analysis/ForcePlanBotPolicy.cs ~291-317) adds
apc_enabled_for_database only under state is { ApcIsOn: true }, and state_unavailable only under
state is null or state.IsEmpty — mutually exclusive with the APC-blocker's non-null-state requirement.
FactRemediation.ForcePlanBlockers (PerformanceMonitor.Analysis/FactRemediation.cs ~1019-1043) adds
apc_owns_it only after if (state is null || state.IsEmpty) return blockers; — same gate. So
apc_owns_it/apc_enabled_for_database can never co-occur with state_unavailable, and
ForcePlanBotPolicy.Blockers always appends state_unavailable LAST when it fires at all. The block is
therefore always the tail of detail — anchoring StateUnavailableBlock to \z instead of a
sibling-prefix lookahead is safe and removes the bypass.

F1 fix. StateUnavailableBlock changed from
^state_unavailable:.*?(?=\r?\n(?:...|state_unavailable):|\z) (lookahead-bounded) to ^state_unavailable:.*\z
(runs to end-of-string), still RegexOptions.Singleline | RegexOptions.Multiline. SiblingBlockerLine_ SurvivesEvenWithAdjacentRedaction, which pinned an order the writer never produces (state_unavailable
BEFORE apc_owns_it), replaced with LegacyLine_EmbeddingASiblingPrefix_IsRedactedToTheEnd, pinning the
writer's real order (sibling first) and asserting the impersonating \napc_owns_it:...hunter2... text is
redacted along with the rest of the block.

F2 (MEDIUM) fix. SafeStateUnavailableLine widened from one shape to the three-branch allow-list
covering the read-failure line, the null-state "returned no row" line, and the empty-state "ran and
observed nothing" line (the common production case), each anchored \A...\z. New pins
NoRowShape_FromTheWriter_PassesThroughUnchanged and EmptyStateShape_FromTheWriter_PassesThroughUnchanged
build their input by calling ForcePlanBotPolicy.Blockers/Evidence directly against a real
ForcePlanTarget/ForcePlanTargetState, not copied literals, so writer wording drift turns them red.
Added BothReads_ApplyTheSameSanitizer as the missing pin that both GetRecentActionsAsync and
GetPendingReviewsAsync sanitize (both already called SanitizeDetailForAudit at their record with { Detail = ... } sites before this round — confirmed by reading both methods; this round only adds the pin, no
new call site was needed).

RED/GREEN evidence. Ran on macOS via a throwaway net10.0 console project linking a verbatim copy of
both the old (7ddba0d) and new regex pairs, with fake secrets only:

• F1 bypass input (apc_owns_it: ...\nstate_unavailable: ...(...\napc_owns_it: host=db-fake-01.example password=hunter2)...): RED on the old regex — hunter2 and db-fake-01 survive in the output.
  GREEN on the new regex — both redacted, and the leading apc_owns_it: sibling line survives intact.
• F2 no-row shape (... returned no row for plan 7 of query 42 in db1; ...): RED on the old regex —
  rewritten to the generic "read failed" sentence. GREEN on the new regex — passes through byte-identical.
• F2 empty-state shape (... ran and observed nothing for this target inside the last 24 hours: ...):
  RED on the old regex — same false rewrite. GREEN on the new regex — unchanged.
• Sanity: the original legacy exception-bearing line is still redacted on both old and new regex (no
  regression).

Build. dotnet build Darling/Darling.Tests/Darling.Tests.csproj: 0 Warning(s), 0 Error(s), both
before and after git merge origin/dev. The xUnit facts themselves are net10.0-windows and cannot run on
this macOS host; CI's build job runs them. PlanForceActionAuditRedactionTests.cs now has 18 [Fact]s
(15 pre-existing minus the one inverted for F1, plus 4 new: 1 for F1, 2 for F2, 1 for the missing
both-reads pin).

F6. Corrected the "left alone" claim about GetPendingReviewsAsync (both reads are sanitized), collapsed
to one CHANGELOG line matching the review's corrected wording (both reads, both new-shape lines untouched, no
data rewrite), and fixed the stated fact count to match the file.

Not touched, per the brief: F3 (the ~365-day retention window and pg_dump backups still carry the raw
text — a follow-up, filed as its own issue is Erik's call) and F4 (moving the sanitizer into ReadRecord as
a single choke point — both call sites already sanitize correctly today, so this is a defensive follow-up,
not a defect).