From 1726cf47b460d2a9dd5c0a4bb61a69b6e99b78ee Mon Sep 17 00:00:00 2001 From: rcourtman Date: Tue, 12 May 2026 12:06:27 +0100 Subject: [PATCH] Harden Patrol and Assistant action boundaries --- .../v6/internal/subsystems/agent-lifecycle.md | 8 + .../v6/internal/subsystems/ai-runtime.md | 9 + .../v6/internal/subsystems/api-contracts.md | 19 +- .../subsystems/patrol-intelligence.md | 18 +- .../internal/subsystems/storage-recovery.md | 7 + .../src/api/__tests__/patrol.test.ts | 65 +++ frontend-modern/src/api/patrol.ts | 62 ++- .../src/components/AI/FindingsPanel.tsx | 498 +++++++++--------- .../AI/__tests__/FindingsPanel.test.ts | 45 +- .../stores/__tests__/aiIntelligence.test.ts | 16 + frontend-modern/src/stores/aiIntelligence.ts | 5 +- internal/api/agent_capabilities.go | 4 +- internal/api/agent_command_redaction.go | 100 ++++ internal/api/agent_events.go | 73 +-- internal/api/agent_events_test.go | 71 +++ internal/api/agent_resource_context.go | 62 ++- internal/api/agent_resource_context_test.go | 82 +++ internal/api/ai_handlers.go | 47 +- ...handlers_patrol_actions_additional_test.go | 44 ++ internal/api/ai_handlers_test.go | 21 + internal/api/contract_test.go | 24 +- pkg/reporting/engine.go | 21 +- pkg/reporting/engine_narrative_test.go | 99 ++++ 23 files changed, 1042 insertions(+), 358 deletions(-) create mode 100644 internal/api/agent_command_redaction.go diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index d2ee53771..0d2d798de 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -144,6 +144,14 @@ the API-owned action audit records `executing` before dispatch and the terminal execution result afterward. Dry-run-only plans remain planning evidence only; lifecycle surfaces must not present them as executable, dispatch them through agent-local command paths, or bypass the API fail-closed execution gate. +Agent lifecycle consumers of `/api/agent/events` and +`/api/agent/resource-context/{id}` must also honor the shared API command +payload boundary: API tokens with monitoring/read scope receive +`commandRedacted:true` instead of raw approval, action, or verification command +text unless they also hold action execution scope. Lifecycle UI and agents may +use those redacted events as doorbells or status summaries, but they must fetch +governed detail through the approval/action surfaces and must not treat a +monitoring-readable event stream as command disclosure or execution authority. The node setup modal boundary must keep guided setup and manual credential submission separate. For new PVE/PBS setup, API Inventory and Host Telemetry diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index a753f4aac..a20385ece 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -66,6 +66,15 @@ runtime cost control, and shared AI transport surfaces. logged server-side or attached as redacted internal Patrol evidence where governed, but they must not be returned through the browser provider-test contract. + Patrol findings history transport must stay bounded when resolved findings + are included: `/api/ai/patrol/findings?include_resolved=1` defaults to a + 200-finding limit and caps explicit limits at 500, and the frontend Patrol + client/store must send the same bounded history request once the Resolved or + All view has made expanded history sticky. Per-finding suppression creation + is similarly narrow by default: the browser helper must require a concrete + resource ID and category, while backend broad/wildcard suppression scopes + require an explicit `allow_broad_scope` request from a dedicated rule + management surface. 5. Add or change AI usage/cost dashboard presentation through `frontend-modern/src/components/AI/AICostDashboard.tsx` and `frontend-modern/src/utils/aiCostPresentation.ts` 6. Add or change AI provider, control-level, chat/session, or explore-state presentation through `frontend-modern/src/components/AI/Chat/`, `frontend-modern/src/utils/aiProviderPresentation.ts`, `frontend-modern/src/utils/aiProviderHealthPresentation.ts`, `frontend-modern/src/utils/aiControlLevelPresentation.ts`, `frontend-modern/src/utils/aiChatPresentation.ts`, `frontend-modern/src/utils/aiSessionDiffPresentation.ts`, and `frontend-modern/src/utils/aiExplorePresentation.ts` 7. Keep AI chat presentation helpers aligned through `frontend-modern/src/components/AI/Chat/` and the shared `frontend-modern/src/utils/textPresentation.ts` diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index e98151885..557c7df5e 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -1361,9 +1361,12 @@ notifications: `finding.created` when a new finding is raised (suppressed when the finding was auto-dismissed by operator-state), `approval.pending` when a remediation request enters `StatusPending` and is waiting on operator decision (carries -`approvalId`, `resourceId`, target tuple, `command`, `riskLevel`, -`requestedBy`, `requestedAt`, `expiresAt` — full detail stays -behind `/api/approvals/{id}`, the event is a doorbell), +`approvalId`, `resourceId`, target tuple, `riskLevel`, +`requestedBy`, `requestedAt`, `expiresAt`, plus `command` only for +session callers or API tokens that also carry `ai:execute`; plain +`monitoring:read` tokens receive `commandRedacted:true` — full +detail stays behind `/api/approvals/{id}`, the event is a +doorbell), `action.completed` when an action audit reaches a terminal state — Completed, runtime-Failed, or refused-before-dispatch (refusals carry the stable error-token prefix `plan_drift:` or @@ -1373,6 +1376,8 @@ dispatches carry a `verification` block — the agent-stable projection of the broker's read-after-write probe — with `ran`, `success`, `command`, `note`, `ranAt` so agents close the "did it actually work?" loop without a follow-up audit fetch; +raw action and verification commands follow the same `ai:execute` +redaction rule on the stream; refused dispatches omit `verification` because the probe never runs) — and `heartbeat` every 15 seconds so an idle connection can confirm the stream is alive. Each event carries a monotonic ID so agents @@ -1550,13 +1555,17 @@ scoped to this resource as the `AgentResourceApprovalSummary` projection (id, command, riskLevel, requestedBy, requestedAt, expiresAt — same vocabulary as `approval.pending` SSE events so "what's pending right now" and "what just became pending" agree -on shape), and recent action audits including refused dispatches +on shape, with `commandRedacted:true` replacing raw command text +for plain `monitoring:read` API tokens), and recent action audits +including refused dispatches with their stable token prefixes (`resource_remediation_locked:`, `plan_drift:`) preserved verbatim and the same agent-stable `verification` block the SSE `action.completed` payload carries (shared `AgentResourceActionVerification` projection, shared `projectAgentResourceVerification` helper) so the bundle's depth -view and the doorbell speak the same vocabulary on probe outcomes. +view and the doorbell speak the same vocabulary on probe outcomes; +action and verification command text follows the same `ai:execute` +redaction rule. The shape is intentionally narrower than the full internal types so agents see a stable agent-paradigm contract, decoupled from internal type evolution. diff --git a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md index ab9f41bd2..c2328f5fe 100644 --- a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md +++ b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md @@ -292,8 +292,22 @@ Patrol-specific presentation helpers. reassuringly emphasized, medium is neutral, low is a soft amber. Findings without an investigation record (or without a recorded confidence) must show no confidence badge rather than defaulting to - one, mirroring the impact rule that absent metadata is not fabricated. The - TS API client mirrors (`UnifiedFindingRecord.impact`, + one, mirroring the impact rule that absent metadata is not fabricated. + Expanded Patrol finding cards must keep action density product-grade: one + primary Assistant intent button is visible inline (`Investigate`, `Verify + fix`, or `Explain` based on current finding state), while secondary + Assistant intents and operator management controls live behind compact + in-flow `Assistant` and `Manage` menus. Those menus must render inside the + expanded finding detail rather than as clipped floating panels, and the + per-finding "Create rule from this" action must remain disabled unless the + finding has both a concrete resource and category for a scoped suppression + rule. + The Patrol store owns the sticky expanded-history state for Resolved/All + views and must keep that data bounded: once resolved findings are requested, + subsequent Patrol finding loads continue to request include-resolved history + with the 200-item history limit instead of unbounded historical payloads or + a transient active-only refresh. + The TS API client mirrors (`UnifiedFindingRecord.impact`, `Finding.impact`) and the store normalizers (`normalizeUnifiedFindingRecord`, `normalizePatrolFindingRecord`) must carry impact through alongside description and recommendation rather diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index ea803785b..2d7c5d03d 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -236,6 +236,13 @@ bypass the API fail-closed execution gate. `internal/api/router.go` Finding to UnifiedFinding conversion: storage and recovery surfaces may render them as adjacent finding context but must not reinterpret them as backup, restore, or storage remediation authority. + Shared agent event and resource-context transport follows the same adjacent + context boundary: monitoring/read API tokens receive redacted approval, + action, and verification command payloads (`commandRedacted:true`) unless + they also hold action execution scope. Storage and recovery consumers may + display those redacted records as status or evidence, but must not derive + backup, restore, storage remediation, or execution authority from the event + stream or resource-context bundle. The `previous_resolved_fix_summary` operational-memory field carried on findings across regressions follows the same scope: storage and recovery surfaces may render it as adjacent finding context but must not diff --git a/frontend-modern/src/api/__tests__/patrol.test.ts b/frontend-modern/src/api/__tests__/patrol.test.ts index 9f17d790b..7163acb89 100644 --- a/frontend-modern/src/api/__tests__/patrol.test.ts +++ b/frontend-modern/src/api/__tests__/patrol.test.ts @@ -11,6 +11,7 @@ import { getPatrolRunHistory, getPatrolRunHistoryWithToolCalls, getPatrolRunWithToolCalls, + createSuppressionRuleFromFinding, resolveFinding, type Finding as PatrolFinding, } from '@/api/patrol'; @@ -224,6 +225,23 @@ describe('patrol api', () => { }); }); + it('bounds include-resolved Patrol finding queries', async () => { + await getPatrolFindings({ includeResolved: true }); + expect(apiFetchJSONMock).toHaveBeenCalledWith( + '/api/ai/patrol/findings?include_resolved=1&limit=200', + ); + + await getPatrolFindings({ includeResolved: true, limit: 25.9 }); + expect(apiFetchJSONMock).toHaveBeenCalledWith( + '/api/ai/patrol/findings?include_resolved=1&limit=25', + ); + + await getPatrolFindings({ includeResolved: true, limit: 9999 }); + expect(apiFetchJSONMock).toHaveBeenCalledWith( + '/api/ai/patrol/findings?include_resolved=1&limit=500', + ); + }); + it('normalizes single patrol run payloads', async () => { apiFetchJSONMock.mockResolvedValueOnce({ id: 'run-2', @@ -328,6 +346,53 @@ describe('patrol api', () => { }); }); + it('refuses to create broad suppression rules from a finding shortcut', async () => { + await expect( + createSuppressionRuleFromFinding({ + resourceId: '', + resourceName: 'Any resource', + category: 'capacity', + description: 'Known pattern', + }), + ).rejects.toThrow('resource and category'); + expect(apiFetchJSONMock).not.toHaveBeenCalled(); + + await expect( + createSuppressionRuleFromFinding({ + resourceId: 'resource-1', + resourceName: 'resource-1', + category: '', + description: 'Known pattern', + }), + ).rejects.toThrow('resource and category'); + expect(apiFetchJSONMock).not.toHaveBeenCalled(); + }); + + it('trims scoped suppression rules created from findings', async () => { + apiFetchJSONMock.mockResolvedValueOnce({ + success: true, + message: 'ok', + rule: { id: 'rule-1' }, + } as any); + + await createSuppressionRuleFromFinding({ + resourceId: ' resource-1 ', + resourceName: ' node-1 ', + category: ' backup ', + description: ' Known backup exception ', + }); + + expect(apiFetchJSONMock).toHaveBeenCalledWith('/api/ai/patrol/suppressions', { + method: 'POST', + body: JSON.stringify({ + resource_id: 'resource-1', + resource_name: 'node-1', + category: 'backup', + description: 'Known backup exception', + }), + }); + }); + it('round-trips remind_at on dismissed-as-will_fix_later patrol findings', async () => { // The backend treats will_fix_later as an operator commitment with a // wake-up deadline (Finding.RemindAt, default 7 days). The TS API client diff --git a/frontend-modern/src/api/patrol.ts b/frontend-modern/src/api/patrol.ts index bc9c15606..bd7def50f 100644 --- a/frontend-modern/src/api/patrol.ts +++ b/frontend-modern/src/api/patrol.ts @@ -236,12 +236,36 @@ export async function getPatrolStatus(): Promise { return apiFetchJSON('/api/ai/patrol/status'); } -export async function getPatrolFindings( - options?: { includeResolved?: boolean }, -): Promise { - const path = options?.includeResolved - ? '/api/ai/patrol/findings?include_resolved=1' - : '/api/ai/patrol/findings'; +const DEFAULT_PATROL_FINDINGS_HISTORY_LIMIT = 200; +const MAX_PATROL_FINDINGS_HISTORY_LIMIT = 500; + +function normalizePatrolFindingsLimit(limit: number | undefined): number { + if (limit === undefined || !Number.isFinite(limit)) { + return DEFAULT_PATROL_FINDINGS_HISTORY_LIMIT; + } + const normalized = Math.floor(limit); + if (normalized < 1) { + return DEFAULT_PATROL_FINDINGS_HISTORY_LIMIT; + } + if (normalized > MAX_PATROL_FINDINGS_HISTORY_LIMIT) { + return MAX_PATROL_FINDINGS_HISTORY_LIMIT; + } + return normalized; +} + +export async function getPatrolFindings(options?: { + includeResolved?: boolean; + limit?: number; +}): Promise { + const search = new URLSearchParams(); + if (options?.includeResolved) { + search.set('include_resolved', '1'); + search.set('limit', String(normalizePatrolFindingsLimit(options.limit))); + } else if (options?.limit !== undefined) { + search.set('limit', String(normalizePatrolFindingsLimit(options.limit))); + } + const query = search.toString(); + const path = query ? `/api/ai/patrol/findings?${query}` : '/api/ai/patrol/findings'; const findings = await apiFetchJSON(path); return arrayOrEmpty(findings).map((finding) => promoteLegacyAlertIdentifier(finding as Finding & { alert_identifier?: string }), @@ -326,9 +350,10 @@ export async function dismissFinding( * * The description is required by the backend — it's the operator's * stated reason, surfaced when the rule is later listed or audited. - * Pass an empty resourceId or category to broaden the rule's scope - * (e.g. all categories on this resource, or this category on any - * resource). + * This per-finding helper is intentionally narrow: it refuses empty + * resource/category values so a noisy finding cannot accidentally create + * a wildcard suppression rule. Broader rules must come from an explicit + * rule-management surface. */ export async function createSuppressionRuleFromFinding(input: { resourceId: string; @@ -336,13 +361,24 @@ export async function createSuppressionRuleFromFinding(input: { category: string; description: string; }): Promise<{ success: boolean; message: string; rule: { id: string } }> { + const resourceId = input.resourceId.trim(); + const resourceName = input.resourceName.trim() || resourceId; + const category = input.category.trim(); + const description = input.description.trim(); + if (!resourceId || !category) { + throw new Error('Suppression rules created from a finding require a resource and category'); + } + if (!description) { + throw new Error('Suppression rules require a description'); + } + return apiFetchJSON('/api/ai/patrol/suppressions', { method: 'POST', body: JSON.stringify({ - resource_id: input.resourceId, - resource_name: input.resourceName, - category: input.category, - description: input.description, + resource_id: resourceId, + resource_name: resourceName, + category, + description, }), }); } diff --git a/frontend-modern/src/components/AI/FindingsPanel.tsx b/frontend-modern/src/components/AI/FindingsPanel.tsx index a3c75dc19..bbc07bd86 100644 --- a/frontend-modern/src/components/AI/FindingsPanel.tsx +++ b/frontend-modern/src/components/AI/FindingsPanel.tsx @@ -103,6 +103,45 @@ interface FindingsPanelProps { >; } +type FindingAssistantIntent = 'discuss' | 'explain' | 'investigate' | 'why' | 'verify_fix'; + +function getPrimaryAssistantFindingAction(finding: UnifiedFinding): { + intent: FindingAssistantIntent; + label: string; + title: string; +} { + if (findingHasAppliedFix(finding) && finding.status === 'active') { + return { + intent: 'verify_fix', + label: 'Verify fix', + title: 'Ask Pulse Assistant to verify whether the applied fix cleared the condition', + }; + } + if (finding.status === 'active') { + return { + intent: 'investigate', + label: 'Investigate', + title: 'Ask Pulse Assistant to investigate this finding with the attached evidence', + }; + } + return { + intent: 'explain', + label: 'Explain', + title: 'Ask Pulse Assistant to explain what happened and why it matters', + }; +} + +function getFindingSuppressionRuleScope(finding: UnifiedFinding) { + const resourceId = finding.resourceId.trim(); + const category = finding.category.trim(); + return { + resourceId, + resourceName: finding.resourceName.trim() || resourceId || 'this resource', + category, + canCreate: Boolean(resourceId && category), + }; +} + export const FindingsPanel: Component = (props) => { const location = useLocation(); const { get: getResource } = useResources(); @@ -534,7 +573,7 @@ export const FindingsPanel: Component = (props) => { const openFindingInAssistant = async ( finding: UnifiedFinding, - intent: 'discuss' | 'explain' | 'investigate' | 'why' | 'verify_fix', + intent: FindingAssistantIntent, ) => { await aiIntelligenceStore.loadPendingApprovals(); const subject = getFindingSubjectPresentation(finding).label; @@ -649,6 +688,13 @@ export const FindingsPanel: Component = (props) => { const handleStartCreateRule = (finding: UnifiedFinding, e: Event) => { e.stopPropagation(); + const scope = getFindingSuppressionRuleScope(finding); + if (!scope.canCreate) { + notificationStore.error( + 'This finding is missing the resource or category needed for a scoped rule', + ); + return; + } setCreatingRuleForId(finding.id); setCreateRuleDescription(finding.userNote || ''); setExpandedId(finding.id); @@ -666,16 +712,23 @@ export const FindingsPanel: Component = (props) => { notificationStore.error('A reason for the rule is required'); return; } + const scope = getFindingSuppressionRuleScope(finding); + if (!scope.canCreate) { + notificationStore.error( + 'This finding is missing the resource or category needed for a scoped rule', + ); + return; + } setActionLoading(finding.id); try { await createSuppressionRuleFromFinding({ - resourceId: finding.resourceId, - resourceName: finding.resourceName, - category: finding.category || '', + resourceId: scope.resourceId, + resourceName: scope.resourceName, + category: scope.category, description, }); notificationStore.success( - `Rule created: future ${finding.category || 'matching'} findings on ${finding.resourceName} will auto-dismiss`, + `Rule created: future ${scope.category} findings on ${scope.resourceName} will auto-dismiss`, ); setCreatingRuleForId(null); // Refresh so the operator sees the finding update (typically the @@ -967,11 +1020,15 @@ export const FindingsPanel: Component = (props) => { class="ml-2 text-blue-600 dark:text-blue-400" title="Auto-suppressed by Pulse based on operator-set state for this resource." > - {' · '}auto: {formatOperatorStateDismissCauseLabel(getOperatorStateDismissCause(finding))} + {' · '}auto:{' '} + {formatOperatorStateDismissCauseLabel(getOperatorStateDismissCause(finding))} - + {' · '}Reminding {formatTime(finding.remindAt!)} @@ -1259,243 +1316,192 @@ export const FindingsPanel: Component = (props) => { - {/* Action area, grouped by intent. Each cluster sits in its own - flex container so the eye reads them as semantically distinct: - - Ask Pulse Assistant: action-style AI handoffs (Explain, - Investigate, Why, optional Verify fix) plus the - open-ended Discuss entry. - - Operator memory + share: Add Note and Copy summary. - The two clusters are separated by a faint vertical divider - (border-l) so the row reads as two intents rather than - ten flat buttons. */} -
-
- - - - - - - - - -
-
- - - - -
-
+
e.stopPropagation()} + > + - - {/* Lifecycle decisions, grouped by consequence: - - Decide: Acknowledge (mark aware), Mark resolved (close out) - - Delay: Snooze 1h / 24h / 7d (re-surface later) - - Dismiss/promote: Not an issue / Remember as expected / - Later / Create rule (permanent pattern) - Visual dividers (border-l) separate the clusters so the - operator reads three distinct intents rather than one - undifferentiated row of 9. */} -
- -
- +
e.stopPropagation()}> + + Assistant + +
+ + + + + + + + + +
+
+ +
e.stopPropagation()}> + + Manage + +
+ + + + + + - + -
- - -
- - - -
- - -
- - - - -
- -
- + + + + + + + + + + + + +
+ +
{/* Inline create-rule confirmation. Confirms scope (resource + category) and requires a reason so the persisted rule has audit context. Mirrors the dismiss-confirmation panel @@ -1506,17 +1512,16 @@ export const FindingsPanel: Component = (props) => {
Create suppression rule for{' '} - {finding.resourceName} - - {' '}({finding.category}) - + + {getFindingSuppressionRuleScope(finding).resourceName} + {' '} + ({getFindingSuppressionRuleScope(finding).category})

- Future findings matching this resource and category will be - auto-dismissed by Patrol without surfacing as new findings. - You can list or remove rules later from the suppressions - management surface. + Future findings matching this resource and category will be auto-dismissed by Patrol + without surfacing as new findings. You can list or remove rules later from the + suppressions management surface.