From 990bccf191a843e8d7869cd852709a5b1be62fdc Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 24 Jul 2026 22:25:46 +0100 Subject: [PATCH] Redact PBS probe failures in the diagnostics export 4815a5c80 split PBSDiagnostic into canonical fields plus a nested probe object, moving probe failure text to pbs.probe.error and adding pbs.stateReason. sanitizeDiagnosticsData spreads the PBS entry and redacts only host, name, id and the top-level error, so transport strings such as "dial tcp 192.168.1.20:8007: connect: connection refused" stopped being IP-redacted. That text used to live in pbs.error and was scrubbed; it now rode unredacted into the sanitized bundle operators attach to support requests. The new fields are spread conditionally so an entry without them keeps its exact previous shape, which leaves the existing strict-equality coverage untouched. Both proofs assert on the serialized bundle rather than on the sanitizer's source, because the failure mode is a payload field added later that the sanitizer never learned about. Verified to fail with the redaction removed, reporting the raw address inside the exported JSON. --- .../subsystems/frontend-primitives.md | 15 +++++++++ .../__tests__/diagnosticsModel.test.ts | 32 +++++++++++++++++++ .../__tests__/settingsArchitecture.test.ts | 30 +++++++++++++++++ .../components/Settings/diagnosticsModel.ts | 17 ++++++++++ 4 files changed, 94 insertions(+) diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 4ff36c13e5..30a0fe8018 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -2426,6 +2426,21 @@ verification. the pool discriminator` in `frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerTrueNASModel.test.ts`. +43. The diagnostics export sanitizer owns the redaction boundary for the + whole diagnostics payload, not just the fields that existed when it + was written. Any provider failure string reaching the export must be + passed through the IP redactor, including nested probe results and + state reasons. Proofs assert on the serialized bundle rather than on + the sanitizer's source, because the failure mode is a payload field + added later that the sanitizer never learned about. Regression + coverage: `redacts PBS probe failures and state reasons in the + exported bundle` in + `frontend-modern/src/components/Settings/__tests__/diagnosticsModel.test.ts` + and `keeps every PBS diagnostic failure string inside the export + redaction boundary` in + `frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts`. + + ## Current State ### System member rows are source-type aware diff --git a/frontend-modern/src/components/Settings/__tests__/diagnosticsModel.test.ts b/frontend-modern/src/components/Settings/__tests__/diagnosticsModel.test.ts index 2fa1b62c20..6e4b7a7747 100644 --- a/frontend-modern/src/components/Settings/__tests__/diagnosticsModel.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/diagnosticsModel.test.ts @@ -181,4 +181,36 @@ describe('diagnosticsModel', () => { 'pulse-diagnostics-sanitized-2026-04-22.json', ); }); + + it('redacts PBS probe failures and state reasons in the exported bundle', () => { + // 4815a5c80 moved probe failure text onto pbs.probe and added + // pbs.stateReason, both of which carry raw transport strings. The + // sanitizer only redacted the top-level pbs.error, so addresses that used + // to be scrubbed escaped into the support bundle people email. + const raw = createDiagnosticsData(); + raw.pbs[0].stateReason = 'unreachable since dial tcp 192.168.1.20:8007 failed'; + raw.pbs[0].probe = { + connected: false, + error: 'dial tcp 192.168.1.20:8007: connect: connection refused', + errorKind: 'network', + troubleshooting: 'Check that 192.168.1.20 is reachable on port 8007.', + }; + + const sanitized = sanitizeDiagnosticsData(raw); + const serialized = JSON.stringify(sanitized); + + expect(serialized).not.toContain('192.168.1.20'); + expect(sanitized.pbs[0].probe?.error).toBe( + 'dial tcp [REDACTED_IP]:8007: connect: connection refused', + ); + expect(sanitized.pbs[0].probe?.troubleshooting).toBe( + 'Check that [REDACTED_IP] is reachable on port 8007.', + ); + expect(sanitized.pbs[0].stateReason).toBe( + 'unreachable since dial tcp [REDACTED_IP]:8007 failed', + ); + // Non-sensitive probe fields survive the copy. + expect(sanitized.pbs[0].probe?.errorKind).toBe('network'); + expect(sanitized.pbs[0].probe?.connected).toBe(false); + }); }); diff --git a/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts b/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts index 31465a18ea..2f77f9cb96 100644 --- a/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it } from 'vitest'; +import { sanitizeDiagnosticsData } from '../diagnosticsModel'; +import type { DiagnosticsData } from '../diagnosticsModel'; import settingsSource from '../Settings.tsx?raw'; import { EN_MESSAGES } from '@/i18n/messages'; import settingsDialogsSource from '../SettingsDialogs.tsx?raw'; @@ -1737,4 +1739,32 @@ describe('settings architecture guardrails', () => { expect(diagnosticsResultsPanelSource).not.toContain('const StatusBadge'); expect(diagnosticsResultsPanelSource).not.toContain('getStatusIndicatorBadgeToneClasses'); }); + + it('keeps every PBS diagnostic failure string inside the export redaction boundary', () => { + // Behavioural guard, not a source scan: the redaction gap that shipped was + // a field added to the payload after the sanitizer was written, which no + // amount of reading the sanitizer's own source would reveal. Assert on the + // serialized bundle so any future PBS failure string has to be redacted to + // pass. + const sanitized = sanitizeDiagnosticsData({ + pbs: [ + { + id: 'pbs-1', + name: 'backup', + host: '192.168.1.20', + connected: false, + state: 'unreachable', + stateReason: 'dial tcp 192.168.1.20:8007 failed', + error: 'Get https://192.168.1.20:8007: EOF', + probe: { + connected: false, + error: 'dial tcp 192.168.1.20:8007: connect: connection refused', + troubleshooting: 'Confirm 192.168.1.20 accepts connections on 8007.', + }, + }, + ], + } as unknown as DiagnosticsData); + + expect(JSON.stringify(sanitized)).not.toContain('192.168.1.20'); + }); }); diff --git a/frontend-modern/src/components/Settings/diagnosticsModel.ts b/frontend-modern/src/components/Settings/diagnosticsModel.ts index b8c61ef4da..1b4d1b950a 100644 --- a/frontend-modern/src/components/Settings/diagnosticsModel.ts +++ b/frontend-modern/src/components/Settings/diagnosticsModel.ts @@ -191,6 +191,23 @@ export function sanitizeDiagnosticsData(raw: DiagnosticsData): DiagnosticsData { name: `pbs-${index + 1}`, id: `pbs-${index + 1}`, error: pbs.error ? redactString(pbs.error) : undefined, + // Probe failures and state reasons carry raw transport text such as + // "dial tcp 192.168.1.20:8007: connect: connection refused". They were + // added after this sanitizer was written and are exported in support + // bundles, so they need the same redaction the top-level error gets. + // Spread conditionally so an entry without them keeps its exact shape. + ...(pbs.stateReason ? { stateReason: redactString(pbs.stateReason) } : {}), + ...(pbs.probe + ? { + probe: { + ...pbs.probe, + ...(pbs.probe.error ? { error: redactString(pbs.probe.error) } : {}), + ...(pbs.probe.troubleshooting + ? { troubleshooting: redactString(pbs.probe.troubleshooting) } + : {}), + }, + } + : {}), })); }