diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 6fe2680..33b0732 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -455,7 +455,9 @@ of a functional dependency on Phase 21. 3. `POST /api/phishing/campaigns/{id}/remediate` proceeds only for already-approved actions against a configured, non-destructive-by-default provider path; otherwise it returns `not_implemented`/an explicit failure and never silently succeeds without taking or logging an action 4. Re-running remediation against an already-completed action does not duplicate the destructive effect — proven by a test that calls remediate twice and asserts a single effect/log entry 5. `POST /api/phishing/campaigns/{id}/mark-false-positive` exists, and every state-changing action (classify, approve, remediate, mark-false-positive) writes an `audit_events` row recording actor, event type, and payload -**Plans**: TBD +**Plans**: 2 plans (2 waves) +- [ ] 20-01-PLAN.md — phishing-audit.ts writeAuditEvent + remediation-service.ts approve/remediate/mark-false-positive orchestrators (idempotent, audited, D-04 guard) + vitest suite (REMED-01..06) +- [ ] 20-02-PLAN.md — lib/permissions.ts approve/remediate grant (D-02) + approve/remediate/mark-false-positive routes + classify audit wiring (REMED-02, REMED-03, REMED-04, REMED-05, REMED-06) **UI hint**: no ### Phase 21: Autotask Triage Note @@ -497,7 +499,7 @@ Phases execute in numeric order. v1.0 (Phases 1-9.1) shipped 2026-07-10. v2.0 (P | 17. Mimecast Blast Radius Lookup | v3.0 | 1/1 | Complete | 2026-07-15 | | 18. Campaign Grouping & Phishing Analysis API | v3.0 | 5/5 | Complete | 2026-07-16 | | 19. Classification Engine | v3.0 | 2/2 | Complete | 2026-07-16 | -| 20. Remediation, Approval & Audit Safety | v3.0 | 0/TBD | Not started | - | +| 20. Remediation, Approval & Audit Safety | v3.0 | 0/2 | Planned | - | | 21. Autotask Triage Note | v3.0 | 0/TBD | Not started | - | | 22. Approval UI (LiveLink) | v3.0 | 0/TBD | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 1dcb6ed..d28c7e4 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -4,12 +4,12 @@ milestone: v3.0 milestone_name: Phishing Triage Automation status: executing stopped_at: Phase 20 context gathered -last_updated: "2026-07-16T13:57:04.242Z" -last_activity: 2026-07-16 -- Phase 19 execution started +last_updated: "2026-07-16T14:29:41.665Z" +last_activity: 2026-07-16 -- Phase 20 planning complete progress: total_phases: 8 completed_phases: 5 - total_plans: 14 + total_plans: 16 completed_plans: 14 percent: 63 --- @@ -27,8 +27,8 @@ See: .planning/PROJECT.md (updated 2026-07-14) Phase: 19 (classification-engine) — EXECUTING Plan: 1 of 2 -Status: Executing Phase 19 -Last activity: 2026-07-16 -- Phase 19 execution started +Status: Ready to execute +Last activity: 2026-07-16 -- Phase 20 planning complete Progress: [░░░░░░░░░░] 0% diff --git a/.planning/phases/20-remediation-approval-audit-safety/20-01-PLAN.md b/.planning/phases/20-remediation-approval-audit-safety/20-01-PLAN.md new file mode 100644 index 0000000..e133098 --- /dev/null +++ b/.planning/phases/20-remediation-approval-audit-safety/20-01-PLAN.md @@ -0,0 +1,336 @@ +--- +phase: 20-remediation-approval-audit-safety +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - lib/services/phishing-audit.ts + - lib/services/phishing-audit.test.ts + - lib/services/remediation-service.ts + - lib/services/remediation-service.test.ts +autonomous: true +requirements: [REMED-01, REMED-02, REMED-03, REMED-04, REMED-05, REMED-06] + +must_haves: + truths: + - "Recommended actions exist only as proposals (in the classification's recommended_actions) until an operator explicitly approves them — no service function auto-creates an approved/completed remediation_actions row" + - "approveRemediationActions only materializes action types that appear in the campaign's latest classification recommended_actions, storing the operator-supplied/overridden params per action (D-03); anything else is rejected" + - "remediateApprovedActions applies D-01's simulated internal effect — no real external provider call for any action type — transitioning approved rows to completed, and is idempotent: a second call transitions nothing and writes no second audit row" + - "remediateApprovedActions on a campaign with no approved actions raises an explicit failure, never a silent success" + - "markCampaignFalsePositive is blocked when any approved/completed remediation exists; otherwise it sets campaigns.status='false_positive'" + - "Every state-changing service call writes exactly one audit_events row per state change, atomically with that change" + artifacts: + - path: "lib/services/phishing-audit.ts" + provides: "writeAuditEvent(input, client?) — single audit_events insert path" + exports: ["writeAuditEvent"] + min_lines: 20 + - path: "lib/services/remediation-service.ts" + provides: "approveRemediationActions / remediateApprovedActions / markCampaignFalsePositive + typed error classes" + exports: ["approveRemediationActions", "remediateApprovedActions", "markCampaignFalsePositive", "RemediationValidationError", "RemediationConflictError"] + min_lines: 100 + - path: "lib/services/remediation-service.test.ts" + provides: "idempotency (REMED-04), audit (REMED-06), D-04 guard, recommended-only approval proofs" + min_lines: 80 + key_links: + - from: "lib/services/remediation-service.ts" + to: "audit_events" + via: "writeAuditEvent(client) inside postgresClient.transaction" + pattern: "writeAuditEvent" + - from: "lib/services/remediation-service.ts" + to: "classifications.recommended_actions" + via: "latest-classification read validates approvable action types" + pattern: "recommended_actions" + - from: "lib/services/remediation-service.ts" + to: "remediation_actions.status" + via: "status='approved' FOR UPDATE filter drives idempotent completion" + pattern: "FOR UPDATE" +--- + + +Build the Phase 20 service layer: a single audit-event writer and a remediation +service exposing approve / remediate / mark-false-positive orchestrators. All +state changes are proposed-only until approved (REMED-01), gated by the caller +(routes in Plan 02), idempotent on re-run (REMED-04), and every state change is +audited (REMED-06). + +Purpose: This is the crux of the phase — the correctness properties +(idempotency, audit-atomicity, D-04 conflict guard, recommended-only approval) +live here and are proven by unit tests, independent of the HTTP layer. +Output: `lib/services/phishing-audit.ts`, `lib/services/remediation-service.ts`, +and their vitest suites. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/20-remediation-approval-audit-safety/20-CONTEXT.md +@.planning/phases/20-remediation-approval-audit-safety/20-PATTERNS.md + + + + +remediation_actions table (migration 097): columns +`id UUID`, `campaign_id UUID`, `action_type TEXT`, `status TEXT DEFAULT 'proposed'`, +`params JSONB`, `approved_by TEXT`, `approved_at TIMESTAMPTZ`, `created_at TIMESTAMPTZ`. + +audit_events table (migration 097): columns +`id UUID`, `campaign_id UUID`, `actor TEXT`, `event_type TEXT NOT NULL`, +`payload JSONB`, `created_at TIMESTAMPTZ`. + +campaigns table (migration 097): `status TEXT NOT NULL DEFAULT 'open'` — the +mark-false-positive target column. + +classifications table (migration 097): `recommended_actions JSONB`, +`requires_approval BOOLEAN`, `created_at` — the current verdict is the most +recent row `ORDER BY created_at DESC`. + +Action vocabulary (Phase 19 D-08, lib/services/campaign-classifier.ts): +```typescript +// The 7 action types; DESTRUCTIVE_ACTIONS force requires_approval:true. +export const DESTRUCTIVE_ACTIONS = new Set([ + 'block_sender', 'purge_message', 'reset_password', 'isolate_endpoint', +]); +// mapVerdictToActions returns e.g. ['no_action'] | ['warn_user'] | +// ['block_sender','purge_message', ...] +export interface ClassifyResult { + recommendedActions: string[]; // <- what approve validates against + requiresApproval: boolean; +} +``` + +postgresClient transaction contract (lib/services/postgres-client.ts): +```typescript +// import { postgresClient } from './postgres-client'; +transaction(callback: (client: PoolClient) => Promise): Promise; +// Inside the callback, EVERY query must use `client.query(...)`, never the +// top-level postgresClient.query — so the remediation_actions write and the +// audit_events write commit/rollback atomically. +``` + + + + + + + Task 1: Audit-event writer (phishing-audit.ts) + lib/services/phishing-audit.ts, lib/services/phishing-audit.test.ts + + - lib/services/campaign-classifier.ts (append-only INSERT + RETURNING id::text pattern, lines 489-505; `import { postgresClient } from './postgres-client'`) + - lib/services/campaign-grouping-service.test.ts (vi.mock('./postgres-client') factory-mock discipline, lines 1-13) + - migrations/097_phishing_triage_schema.sql (audit_events columns, lines 150-157) + + + Create `lib/services/phishing-audit.ts` exporting `writeAuditEvent`. + Import `{ postgresClient }` from `'./postgres-client'`. Export + `interface AuditEventInput { campaignId: string; actor: string | null; + eventType: string; payload: Record; }` and a minimal + `interface AuditQueryClient { query: (text: string, params?: unknown[]) => + Promise<{ rows: Array<{ id: string }> }> }`. + Signature: `writeAuditEvent(input: AuditEventInput, client?: AuditQueryClient): + Promise` — when `client` is passed (a transaction client) use it, + otherwise fall back to `postgresClient`. Body performs a single parameterized + INSERT into `audit_events (campaign_id, actor, event_type, payload)` with + `VALUES ($1, $2, $3, $4::jsonb)` binding `[input.campaignId, input.actor, + input.eventType, JSON.stringify(input.payload)]`, `RETURNING id::text AS id`, + and returns `rows[0].id`. No ON CONFLICT (append-only). Document the four + canonical event_type strings this phase emits in a header comment: + `'campaign_classified'`, `'remediation_approved'`, `'remediation_completed'`, + `'campaign_marked_false_positive'`. + Create `phishing-audit.test.ts`: `vi.mock('./postgres-client', ...)` before + import (mirror campaign-grouping-service.test.ts). Assert (a) calling + writeAuditEvent with no client issues one parameterized INSERT into + audit_events binding actor/event_type and a JSON-stringified payload and + returns the RETURNING id; (b) passing an explicit client object routes the + query through that client, not postgresClient. + + + npx vitest run lib/services/phishing-audit.test.ts + + + - `npx vitest run lib/services/phishing-audit.test.ts` passes. + - `grep -n "INSERT INTO audit_events" lib/services/phishing-audit.ts` matches a parameterized statement using `$1`..`$4` (no string interpolation of values). + - `writeAuditEvent` returns the inserted row id (string), sourced from `RETURNING id::text`. + - The optional `client` param, when supplied, is the object `.query` is called on. + + writeAuditEvent exists, is parameterized, supports an injected transaction client, and its test suite passes. + + + + Task 2: approve + remediate orchestrators (idempotent, audited) + lib/services/remediation-service.ts, lib/services/remediation-service.test.ts + + - lib/services/phishing-audit.ts (writeAuditEvent — created in Task 1) + - lib/services/campaign-grouping-service.ts (postgresClient.transaction usage + status short-circuit idiom, lines 160-220) + - lib/services/campaign-grouping-service.test.ts (transactionMock that invokes the callback with a fake client whose query is recorded, lines 55-140) + - lib/services/campaign-classifier.ts (ClassifyResult.recommendedActions, DESTRUCTIVE_ACTIONS, append-only INSERT shape) + - app/api/phishing/campaigns/[id]/route.ts (latest-classification read: `FROM classifications WHERE campaign_id=$1 ORDER BY created_at DESC`, lines 120-124) + + + - approveRemediationActions rejects an action type that is NOT in the latest classification's recommended_actions → throws RemediationValidationError. + - approveRemediationActions with no classification row for the campaign → throws RemediationValidationError. + - approveRemediationActions inserts one remediation_actions row per requested action with status='approved', approved_by=actor, approved_at=NOW(), params=operator-supplied params, AND one audit_events row (event_type 'remediation_approved') — both inside one transaction. + - remediateApprovedActions transitions every status='approved' row for the campaign to 'completed' and writes one 'remediation_completed' audit row per transitioned action. + - remediateApprovedActions called twice: first call transitions N approved rows and writes N audit rows; second call transitions 0 and writes 0 audit rows (idempotent — REMED-04). + - remediateApprovedActions on a campaign with zero remediation_actions rows → throws RemediationValidationError (explicit failure, never silent success — REMED-03). + + + Create `lib/services/remediation-service.ts`. Import `{ postgresClient }` from + `'./postgres-client'` and `{ writeAuditEvent }` from `'./phishing-audit'`. + Export two typed error classes `RemediationValidationError` and + `RemediationConflictError` (both `extends Error`, set `this.name`). + + Export `interface ApproveActionInput { actionType: string; params?: + Record }` and `approveRemediationActions(campaignId: string, + actions: ApproveActionInput[], actor: string | null)`: + inside `postgresClient.transaction(async (client) => { ... })`, first read the + latest classification via `client.query` with + `SELECT recommended_actions FROM classifications WHERE campaign_id=$1 ORDER BY + created_at DESC LIMIT 1`. If no row → throw RemediationValidationError + ('Campaign has no classification to approve against'). Parse + recommended_actions (JSONB → string[]). For each input action, if + `actionType` is not in that set → throw RemediationValidationError + (`Action is not a recommended action for this campaign`). Then INSERT + one row per action into `remediation_actions (campaign_id, action_type, + status, params, approved_by, approved_at)` VALUES `($1,$2,'approved', + $3::jsonb, $4, NOW())` RETURNING `id::text`, collecting the ids. After the + inserts, call `writeAuditEvent({ campaignId, actor, eventType: + 'remediation_approved', payload: { actions, actionIds } }, client)`. Return + the approved rows as camelCase objects. + + Export `remediateApprovedActions(campaignId: string, actor: string | null)`: + inside a transaction, `SELECT id::text, action_type, status FROM + remediation_actions WHERE campaign_id=$1 FOR UPDATE`. If zero rows → throw + RemediationValidationError ('No remediation actions to remediate — nothing + approved') (REMED-03 explicit failure). For each row with status='approved': + `UPDATE remediation_actions SET status='completed' WHERE id=$1` (the D-01 + simulated internal effect — no external provider call for any of the 7 action + types), and `writeAuditEvent({ campaignId, actor, eventType: + 'remediation_completed', payload: { actionId, actionType } }, client)` — one + audit row per transitioned action. Rows already 'completed' are left + untouched and generate NO audit row (idempotency — REMED-04). Return + `{ campaignId, actions: [{ id, actionType, status, alreadyCompleted }] }`. + The `status='approved'` filter + FOR UPDATE is the idempotency mechanism + (planner's-discretion choice per CONTEXT.md): a re-run finds no approved rows + and transitions/audits nothing. Add a header comment noting D-01: this phase + NEVER returns `not_implemented` because the simulated effect always takes and + logs an action; the only explicit-failure branch is the zero-approved case. + + Create `remediation-service.test.ts`: `vi.mock('./postgres-client')` exporting + both `query` and `transaction` mocks, and `vi.mock('./phishing-audit')` + exporting a `writeAuditEvent` spy — both BEFORE importing the module. The + transaction mock invokes its callback with a fake client whose `query` + resolves scripted rows and records calls (copy the fake-client recorder from + campaign-grouping-service.test.ts). Implement the six behavior assertions + above; the REMED-04 idempotency test must call remediateApprovedActions twice + and assert the writeAuditEvent spy was called exactly once per originally- + approved action across both calls (zero additional on the second call). + + + npx vitest run lib/services/remediation-service.test.ts && npx tsc --noEmit --pretty + + + - `npx vitest run lib/services/remediation-service.test.ts` passes all six behavior cases. + - The idempotency test proves the writeAuditEvent spy fires exactly once per action across two remediate calls, and only 'approved' rows transition to 'completed'. + - `grep -n "FOR UPDATE" lib/services/remediation-service.ts` matches inside remediateApprovedActions. + - `grep -n "RemediationValidationError" lib/services/remediation-service.ts` shows it thrown for both the non-recommended-action and zero-approved-actions cases. + - `grep -vn "^\s*//" lib/services/remediation-service.ts | grep -c "not_implemented"` returns 0 (no not_implemented code path, per D-01). + - `npx tsc --noEmit` reports no errors. + + approve and remediate orchestrators exist with recommended-only validation, idempotent completion via status filter, atomic audit writes, and a passing test suite proving REMED-01/02/03/04/06 at the unit level. + + + + Task 3: mark-false-positive orchestrator (D-04 guard, audited) + lib/services/remediation-service.ts, lib/services/remediation-service.test.ts + + - lib/services/remediation-service.ts (the file extended in Task 2 — reuse its transaction + error-class conventions) + - lib/services/phishing-audit.ts (writeAuditEvent) + - migrations/097_phishing_triage_schema.sql (campaigns.status DEFAULT 'open', remediation_actions.status) + - app/api/admin/integrations/route.ts (guard-before-write + actor capture precedent, PATCH lines 58-93) + + + - markCampaignFalsePositive throws RemediationConflictError when any remediation_actions row for the campaign has status IN ('approved','completed') (D-04 — prevents a "remediated AND false-positive" contradiction). + - markCampaignFalsePositive with no approved/completed rows sets campaigns.status='false_positive' and writes one 'campaign_marked_false_positive' audit row, atomically. + - The audit payload records the previous campaign status and the optional reason. + + + Extend `remediation-service.ts` with + `markCampaignFalsePositive(campaignId: string, actor: string | null, reason?: + string)`. Inside `postgresClient.transaction`: first the D-04 guard — + `SELECT id FROM remediation_actions WHERE campaign_id=$1 AND status IN + ('approved','completed') FOR UPDATE LIMIT 1`; if a row is found → throw + RemediationConflictError ('Cannot mark false positive: campaign already has + approved or completed remediation'). Otherwise read the current campaign + status (`SELECT status FROM campaigns WHERE id=$1`; if no row → throw + RemediationValidationError 'Campaign not found'), `UPDATE campaigns SET + status='false_positive', updated_at=NOW() WHERE id=$1`, then + `writeAuditEvent({ campaignId, actor, eventType: + 'campaign_marked_false_positive', payload: { previousStatus, reason: reason ?? + null } }, client)`. Return `{ campaignId, status: 'false_positive', + auditEventId }`. False-positive reversibility is out of scope (CONTEXT.md + Deferred Ideas) — do not add an un-mark path. + Extend `remediation-service.test.ts` with the three behavior assertions: + guard throws RemediationConflictError when an approved/completed row exists; + happy path updates campaigns.status to 'false_positive' and calls + writeAuditEvent once with event_type 'campaign_marked_false_positive'; + audit payload carries previousStatus + reason. + + + npx vitest run lib/services/remediation-service.test.ts && npx tsc --noEmit --pretty + + + - `npx vitest run lib/services/remediation-service.test.ts` passes including the three new mark-false-positive cases. + - `grep -n "status IN ('approved', 'completed')\|status IN ('approved','completed')" lib/services/remediation-service.ts` matches the D-04 guard SELECT. + - `grep -n "false_positive" lib/services/remediation-service.ts` shows the campaigns.status UPDATE. + - RemediationConflictError is thrown (not a silent success) when the guard trips. + - `npx tsc --noEmit` reports no errors. + + markCampaignFalsePositive exists with the D-04 conflict guard, sets campaigns.status='false_positive', writes an atomic audit row, and its tests pass (REMED-05/06). + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| route → remediation-service | Route-supplied campaignId, action list, params, and actor cross into privileged state-change logic | +| service → Postgres | Every state change (approve/remediate/mark-fp) plus its audit write cross into the durable store; must be atomic | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-20-01 | Elevation of Privilege | approveRemediationActions | mitigate | Validate each requested `actionType` against the latest `classifications.recommended_actions`; reject non-recommended (and no-classification) with `RemediationValidationError` → route 400. Approve never invents an action the classifier did not recommend. | +| T-20-02 | Tampering | remediateApprovedActions idempotency | mitigate | Transition only `status='approved'` rows, selected `FOR UPDATE` inside the transaction; already-`completed` rows are not re-selected → a re-run transitions nothing and writes no second audit row. Proven by the double-call test (REMED-04). | +| T-20-03 | Repudiation | audit omission | mitigate | `writeAuditEvent(..., client)` runs inside the SAME `postgresClient.transaction` as every state write; a rollback discards the state change and its audit together — no state change can commit without its audit row (REMED-06). | +| T-20-04 | Tampering | contradictory audit trail | mitigate | D-04 guard in `markCampaignFalsePositive` rejects with `RemediationConflictError` when any approved/completed remediation exists, so a campaign can never be both remediated and false-positive. | +| T-20-05 | Information Disclosure | over-broad remediation | accept | D-01 simulated internal effect makes no external provider call for any of the 7 action types this phase; no real destructive effect can leak. Revisited when REMEDEXEC-01..05 (v2) wires a real provider. | +| T-20-SC | Tampering | npm/pip/cargo installs | n/a | This plan installs no packages — the service depends only on the existing `postgres-client` singleton. No install task; supply-chain checkpoint not required. | + + + +- `npx vitest run lib/services/phishing-audit.test.ts lib/services/remediation-service.test.ts` — all pass. +- `npx tsc --noEmit --pretty` — clean. +- Idempotency, audit-atomicity, recommended-only approval, and the D-04 guard are each asserted by a named test case. + + + +- writeAuditEvent is the single, parameterized, append-only audit_events insert path, usable inside or outside a transaction. +- approveRemediationActions materializes only classifier-recommended actions as status='approved' rows with approver + timestamp + params, plus one audit row. +- remediateApprovedActions is idempotent (status filter + FOR UPDATE), applies the D-01 simulated completion, audits each transition, and fails explicitly when nothing is approved. +- markCampaignFalsePositive enforces the D-04 guard, sets campaigns.status='false_positive', and audits. +- No not_implemented code path and no external-provider call exists in this phase (D-01). + + + +Create `.planning/phases/20-remediation-approval-audit-safety/20-01-SUMMARY.md` when done. + diff --git a/.planning/phases/20-remediation-approval-audit-safety/20-02-PLAN.md b/.planning/phases/20-remediation-approval-audit-safety/20-02-PLAN.md new file mode 100644 index 0000000..66f42c1 --- /dev/null +++ b/.planning/phases/20-remediation-approval-audit-safety/20-02-PLAN.md @@ -0,0 +1,283 @@ +--- +phase: 20-remediation-approval-audit-safety +plan: 02 +type: execute +wave: 2 +depends_on: [20-01] +files_modified: + - lib/permissions.ts + - app/api/phishing/campaigns/[id]/approve/route.ts + - app/api/phishing/campaigns/[id]/remediate/route.ts + - app/api/phishing/campaigns/[id]/mark-false-positive/route.ts + - app/api/phishing/campaigns/[id]/classify/route.ts +autonomous: true +requirements: [REMED-02, REMED-03, REMED-04, REMED-05, REMED-06] + +must_haves: + truths: + - "phishing:approve and phishing:remediate are granted to super-admin and admin only; a plain user (phishing:read) is rejected with 403" + - "POST /approve records approver + timestamp + the exact operator-supplied action params (D-03 — request body specifies which recommended action(s) to approve and may override/narrow their params), gated by phishing:approve" + - "POST /remediate is gated by phishing:remediate and returns the idempotent completion result; a malformed campaign id returns 400" + - "POST /mark-false-positive is gated by phishing:approve and returns 409 when approved/completed remediation already exists" + - "The classify route also writes a 'campaign_classified' audit event, completing REMED-06's four-action audit coverage" + artifacts: + - path: "lib/permissions.ts" + provides: "approve+remediate granted to superAdminRole and adminRole (D-02)" + contains: "phishing" + - path: "app/api/phishing/campaigns/[id]/approve/route.ts" + provides: "POST approve endpoint" + exports: ["POST"] + min_lines: 40 + - path: "app/api/phishing/campaigns/[id]/remediate/route.ts" + provides: "POST remediate endpoint" + exports: ["POST"] + min_lines: 40 + - path: "app/api/phishing/campaigns/[id]/mark-false-positive/route.ts" + provides: "POST mark-false-positive endpoint" + exports: ["POST"] + min_lines: 40 + key_links: + - from: "app/api/phishing/campaigns/[id]/approve/route.ts" + to: "approveRemediationActions" + via: "requirePermission('phishing','approve') → service delegation" + pattern: "requirePermission\\('phishing', ?'approve'\\)" + - from: "app/api/phishing/campaigns/[id]/remediate/route.ts" + to: "remediateApprovedActions" + via: "requirePermission('phishing','remediate') → service delegation" + pattern: "requirePermission\\('phishing', ?'remediate'\\)" + - from: "app/api/phishing/campaigns/[id]/mark-false-positive/route.ts" + to: "markCampaignFalsePositive" + via: "requirePermission('phishing','approve') → service delegation" + pattern: "requirePermission\\('phishing', ?'approve'\\)" + - from: "app/api/phishing/campaigns/[id]/classify/route.ts" + to: "writeAuditEvent" + via: "post-classify audit write" + pattern: "writeAuditEvent" +--- + + +Expose the Phase 20 service layer over HTTP: grant the elevated permissions +(D-02), add the three POST routes (approve / remediate / mark-false-positive) +following the Phase 18/19 route convention, and wire a 'campaign_classified' +audit event into the existing classify route so all four state-changing actions +are audited (REMED-06). + +Purpose: The routes are the security perimeter — they enforce the permission +gate, validate the campaign id, capture the actor from the server session (never +the request body), and translate the service's typed errors into the right HTTP +status. Depends on Plan 01's service functions and writeAuditEvent. +Output: one permissions edit + three new routes + one classify-route edit. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/20-remediation-approval-audit-safety/20-CONTEXT.md +@.planning/phases/20-remediation-approval-audit-safety/20-PATTERNS.md +@.planning/phases/20-remediation-approval-audit-safety/20-01-SUMMARY.md + + + +```typescript +// lib/services/remediation-service.ts +export class RemediationValidationError extends Error {} // -> HTTP 400 +export class RemediationConflictError extends Error {} // -> HTTP 409 +export interface ApproveActionInput { actionType: string; params?: Record; } +export function approveRemediationActions(campaignId: string, actions: ApproveActionInput[], actor: string | null): Promise<...>; +export function remediateApprovedActions(campaignId: string, actor: string | null): Promise<...>; +export function markCampaignFalsePositive(campaignId: string, actor: string | null, reason?: string): Promise<...>; + +// lib/services/phishing-audit.ts +export function writeAuditEvent(input: { campaignId: string; actor: string | null; eventType: string; payload: Record }, client?): Promise; +``` + +Route shell precedent — app/api/phishing/campaigns/[id]/classify/route.ts: +```typescript +import { NextRequest, NextResponse } from 'next/server'; +import { requirePermission } from '@/lib/auth-utils'; +import postgresClient from '@/lib/services/postgres-client'; +const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; +export async function POST(request, { params }: { params: Promise<{ id: string }> }) { ... } +``` + +requirePermission (lib/auth-utils.ts) returns `{ session, error }`; internally +calls requireAuth() (401 if no session) then hasPermission(role, resource, +action) (403 if role lacks it). Actor capture precedent +(app/api/admin/integrations/route.ts line 80): +`const actor = (session?.user as { email?: string } | undefined)?.email ?? null;` + +lib/permissions.ts current state: `statement.phishing` already = +`["read","analyze","approve","remediate"]` (full vocabulary from Phase 18). +`superAdminRole` and `adminRole` currently grant `phishing: ["read","analyze"]`; +`userRole` grants `phishing: ["read"]`. + + + + + + + Task 1: Grant approve + remediate to admin roles (D-02) + lib/permissions.ts + + - lib/permissions.ts (full file — superAdminRole line 51, adminRole line 65, userRole line 79; the statement.phishing vocabulary line 34) + + + Edit `superAdminRole` and `adminRole` so `phishing` becomes + `["read", "analyze", "approve", "remediate"]` (add the `"approve"` and + `"remediate"` array entries — D-02, parity with how `analyze` is granted to + both). Do NOT change the `statement` object (line 34 already has the full + vocabulary). Do NOT change `userRole` — it stays `phishing: ["read"]`, so a + plain user cannot approve/remediate/mark-false-positive. Update the trailing + comment on the superAdminRole line to drop the "ungranted until Phase 20" + note. No new statement key is added (mark-false-positive reuses the + `approve` action per D-04). + + + npx tsc --noEmit --pretty && node -e "const {hasPermission}=require('./lib/permissions.ts')" 2>/dev/null; grep -n "phishing:" lib/permissions.ts + + + - `grep -c "phishing: \[\"read\", \"analyze\", \"approve\", \"remediate\"\]" lib/permissions.ts` returns 2 (superAdminRole + adminRole). + - `grep -n "phishing: \[\"read\"\]" lib/permissions.ts` still matches userRole (unchanged). + - The `statement` object is unchanged (still one `phishing:` line with the four-action vocabulary). + - `npx tsc --noEmit` reports no errors. + + super-admin and admin can approve/remediate; user remains read-only. D-02 satisfied. + + + + Task 2: approve + remediate routes + app/api/phishing/campaigns/[id]/approve/route.ts, app/api/phishing/campaigns/[id]/remediate/route.ts + + - app/api/phishing/campaigns/[id]/classify/route.ts (route shell to copy verbatim: imports, UUID_RE, POST signature, campaign-exists SELECT, try/catch/500) + - app/api/admin/integrations/route.ts (JSON body parse with try/catch → 400, PATCH lines 62-67; actor capture line 80) + - lib/services/remediation-service.ts (approveRemediationActions / remediateApprovedActions signatures + error classes — Plan 01) + - 20-CONTEXT.md D-03 (approve body = which recommended actions to approve, with optional param overrides) + + + Create `approve/route.ts`. Copy the classify route shell. Use + `const { session, error } = await requirePermission('phishing', 'approve'); + if (error) return error;` (destructure `session` — needed for the actor). + Keep the `UUID_RE` guard → 400, and the campaign-exists SELECT → 404. Parse + the JSON body in a try/catch → 400 on parse failure; expected shape per D-03 + is `{ actions: Array<{ actionType: string; params?: Record }> }`. + Validate `actions` is a non-empty array → 400 otherwise. Capture + `const actor = (session?.user as { email?: string } | undefined)?.email ?? null;`. + Delegate to `approveRemediationActions(id, body.actions, actor)`. Wrap in + try/catch: catch `RemediationValidationError` → 400 with its message; catch + `RemediationConflictError` → 409; otherwise log `[PHISHING-APPROVE]` and + return 500. Return the service result as JSON on success. + + Create `remediate/route.ts` with the identical shell but + `requirePermission('phishing', 'remediate')`, no request body needed (the + action set is whatever was previously approved — D-01). Delegate to + `remediateApprovedActions(id, actor)`. Same error mapping: + RemediationValidationError → 400 (the "nothing approved" explicit failure, + REMED-03), RemediationConflictError → 409, else `[PHISHING-REMEDIATE]` 500. + Return the idempotent completion result as JSON. + + + npx tsc --noEmit --pretty && grep -n "requirePermission('phishing', 'approve')" app/api/phishing/campaigns/[id]/approve/route.ts && grep -n "requirePermission('phishing', 'remediate')" app/api/phishing/campaigns/[id]/remediate/route.ts + + + - Both routes export an async `POST` and call `requirePermission` with `'approve'` / `'remediate'` respectively as the first statement, early-returning `error`. + - `grep -n "UUID_RE" app/api/phishing/campaigns/[id]/approve/route.ts app/api/phishing/campaigns/[id]/remediate/route.ts` matches in both (malformed id → 400). + - approve maps `RemediationValidationError`→400 and `RemediationConflictError`→409; grep confirms both catch branches. + - Actor is derived from `session.user.email`, never from the request body (`grep -n "body.actor\|request.*actor" ...` returns nothing). + - `npx tsc --noEmit` reports no errors. + + approve and remediate endpoints exist, permission-gated, UUID-guarded, actor from session, service-delegated with correct status mapping. + + + + Task 3: mark-false-positive route + classify audit wiring + app/api/phishing/campaigns/[id]/mark-false-positive/route.ts, app/api/phishing/campaigns/[id]/classify/route.ts + + - app/api/phishing/campaigns/[id]/approve/route.ts (route shell built in Task 2 — reuse its error-mapping pattern) + - app/api/phishing/campaigns/[id]/classify/route.ts (existing route to extend with the audit write; it currently discards `session`) + - lib/services/remediation-service.ts (markCampaignFalsePositive signature — Plan 01) + - lib/services/phishing-audit.ts (writeAuditEvent — Plan 01) + - 20-CONTEXT.md D-04 (mark-false-positive uses the elevated approve tier; blocked when approved/completed remediation exists) + + + Create `mark-false-positive/route.ts`. Same shell as approve. Gate with + `requirePermission('phishing', 'approve')` (D-04 — same elevated tier as + approve; no separate action key). UUID guard → 400, campaign-exists → 404. + Optionally parse a JSON body for `{ reason?: string }` in a try/catch + (tolerate an empty/absent body — default reason undefined). Capture actor + from session. Delegate to `markCampaignFalsePositive(id, actor, body?.reason)`. + Error mapping: catch `RemediationConflictError` → 409 with its message (the + D-04 block), `RemediationValidationError` → 400, else `[PHISHING-MARK-FP]` + 500. Return the service result as JSON. + + Edit `classify/route.ts` to complete REMED-06's four-action audit coverage: + change `const { error } = await requirePermission('phishing', 'analyze')` to + `const { session, error } = ...` (keep the same `'analyze'` action — do NOT + change the classify permission). Import `writeAuditEvent` from + `'@/lib/services/phishing-audit'`. After the successful `classifyCampaign(id)` + call, capture `actor` from session and call + `await writeAuditEvent({ campaignId: id, actor, eventType: 'campaign_classified', + payload: { verdict: result.verdict, requiresApproval: result.requiresApproval } })` + (no transaction client → uses postgresClient). Keep the existing try/catch + and success response otherwise unchanged; a classification must not fail if + the audit write throws only after the classification already committed — + place the audit write inside the existing try so a failure surfaces as the + 500, but note classifyCampaign has already persisted its row (append-only). + + + npx tsc --noEmit --pretty && grep -n "requirePermission('phishing', 'approve')" app/api/phishing/campaigns/[id]/mark-false-positive/route.ts && grep -n "campaign_classified" app/api/phishing/campaigns/[id]/classify/route.ts + + + - mark-false-positive route exports async `POST`, gates on `requirePermission('phishing', 'approve')`, and maps `RemediationConflictError`→409. + - `grep -n "UUID_RE" app/api/phishing/campaigns/[id]/mark-false-positive/route.ts` matches. + - classify route imports and calls `writeAuditEvent` with `eventType: 'campaign_classified'` after a successful classify. + - classify route destructures `session` from requirePermission and derives actor from `session.user.email`. + - `npx tsc --noEmit` reports no errors. + + mark-false-positive endpoint exists (approve-tier, 409 on D-04 conflict) and classify writes an audit event — all four state-changing actions are now audited (REMED-06). + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| client → API route | Unauthenticated/under-privileged HTTP callers, malformed campaign ids, and attacker-controlled request bodies cross here | +| session → actor | The audited actor identity is derived here — must come from the verified server session, never the request | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-20-06 | Elevation of Privilege | all three POST routes | mitigate | `requirePermission('phishing','approve'\|'remediate')` early-return on every route (401 unauth, 403 unauthorized). D-02 grants approve/remediate to super-admin + admin only; `userRole` stays `["read"]`. mark-false-positive reuses the `approve` tier (D-04) — never a looser check. | +| T-20-07 | Spoofing / Repudiation | actor capture | mitigate | Actor is read from `session.user.email` (server session) and passed to the service for `approved_by` / `audit_events.actor`; it is never sourced from the request body, so a caller cannot forge the audited identity. | +| T-20-08 | Tampering | campaign id path param | mitigate | `UUID_RE` guard rejects malformed ids with 400 before any query; all campaign lookups are parameterized (`$1`), preventing injection and unhandled-500 leakage. | +| T-20-09 | Tampering | false-positive conflict bypass | mitigate | mark-false-positive delegates to the service's D-04 transactional guard; the route surfaces `RemediationConflictError` as 409 rather than silently succeeding, so the state machine cannot be forced into a contradictory state via HTTP. | +| T-20-10 | Repudiation | classify audit gap | mitigate | classify route now emits a `campaign_classified` audit event, closing REMED-06's requirement that ALL four state-changing actions (classify/approve/remediate/mark-false-positive) are audited. | +| T-20-SC | Tampering | npm/pip/cargo installs | n/a | This plan installs no packages — routes import only existing `@/lib/auth-utils`, `@/lib/services/postgres-client`, and the Plan 01 service modules. No install task; supply-chain checkpoint not required. | + + + +- `npx tsc --noEmit --pretty` — clean across all edited/created files. +- Each route greps positive for its `requirePermission` action and the `UUID_RE` guard. +- classify route greps positive for `campaign_classified` / `writeAuditEvent`. +- lib/permissions.ts grants approve+remediate to exactly the two admin roles and leaves userRole read-only. + + + +- POST /api/phishing/campaigns/{id}/approve, /remediate, /mark-false-positive exist, are permission-gated (approve / remediate / approve tiers), UUID-guarded, and delegate to the Plan 01 service with correct 400/404/409/500 mapping. +- The audited actor always originates from the server session. +- Every state-changing action — classify, approve, remediate, mark-false-positive — writes an audit_events row (REMED-06 complete). +- A plain user role is rejected 403 from all three new endpoints. + + + +Create `.planning/phases/20-remediation-approval-audit-safety/20-02-SUMMARY.md` when done. + diff --git a/.planning/phases/20-remediation-approval-audit-safety/20-PATTERNS.md b/.planning/phases/20-remediation-approval-audit-safety/20-PATTERNS.md new file mode 100644 index 0000000..606cc69 --- /dev/null +++ b/.planning/phases/20-remediation-approval-audit-safety/20-PATTERNS.md @@ -0,0 +1,333 @@ +# Phase 20: Remediation, Approval & Audit Safety - Pattern Map + +**Mapped:** 2026-07-16 +**Files analyzed:** 5 (3 new routes, 1 permissions edit, 1 new service — plus test files) +**Analogs found:** 5 / 5 + +## File Classification + +| New/Modified File | Role | Data Flow | Closest Analog | Match Quality | +|-------------------|------|-----------|----------------|---------------| +| `app/api/phishing/campaigns/[id]/approve/route.ts` | route/controller | request-response (CRUD write, multi-row insert) | `app/api/phishing/campaigns/[id]/classify/route.ts` | exact (named precedent in CONTEXT.md) | +| `app/api/phishing/campaigns/[id]/remediate/route.ts` | route/controller | request-response (state transition, idempotent) | `app/api/phishing/campaigns/[id]/classify/route.ts` | exact (same shape) + `campaign-grouping-service.ts` transaction pattern for the state-transition body | +| `app/api/phishing/campaigns/[id]/mark-false-positive/route.ts` | route/controller | request-response (guarded state write) | `app/api/phishing/campaigns/[id]/classify/route.ts` | exact (same shape) + `app/api/admin/integrations/route.ts` PATCH for the "guard before write" + actor-capture pattern | +| `lib/permissions.ts` (edit: add `"approve"`, `"remediate"` to `superAdminRole` and `adminRole`) | config | CRUD (static role literal) | `lib/permissions.ts` itself — edit target, not a new file | exact (in-place edit, no analog needed) | +| `lib/services/remediation-service.ts` (new — service layer for approve/remediate/mark-false-positive + audit writes) | service | CRUD + event-driven (audit trail) | `lib/services/campaign-classifier.ts` (`classifyCampaign` orchestrator + append-only insert) and `lib/services/campaign-grouping-service.ts` (`postgresClient.transaction()` multi-statement write) | exact (classifier) / role-match (grouping service, for transaction shape) | +| `lib/services/remediation-service.test.ts` (new) | test | — | `lib/services/campaign-classifier.test.ts` | exact (mocking discipline: `vi.mock('./postgres-client', ...)` before import) | + +## Pattern Assignments + +### `app/api/phishing/campaigns/[id]/approve/route.ts` (route, request-response) + +**Analog:** `app/api/phishing/campaigns/[id]/classify/route.ts` (full file, 51 lines — read in one pass) + +**Imports pattern** (lines 12-15 of analog): +```typescript +import { NextRequest, NextResponse } from 'next/server'; +import { requirePermission } from '@/lib/auth-utils'; +import postgresClient from '@/lib/services/postgres-client'; +import { classifyCampaign } from '@/lib/services/campaign-classifier'; +``` +For `approve/route.ts`, swap the last import for the new service, e.g. +`import { approveRemediationActions } from '@/lib/services/remediation-service';` + +**UUID guard pattern** (lines 17, 26-31): +```typescript +const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; +... +const { id } = await params; +// V5: validate UUID shape before querying — a malformed id would otherwise +// surface as an unhandled Postgres error -> uncaught 500. +if (!UUID_RE.test(id)) { + return NextResponse.json({ error: 'Invalid campaign id' }, { status: 400 }); +} +``` + +**Auth/permission pattern** (lines 23-24) — use `'approve'` action per D-02: +```typescript +const { error } = await requirePermission('phishing', 'analyze'); +if (error) return error; +``` +becomes, for this route: +```typescript +const { session, error } = await requirePermission('phishing', 'approve'); +if (error) return error; +``` +Note: `classify/route.ts` discards `session` (only needs `error`); `approve`/`mark-false-positive` need `session` too, to capture the actor for `approved_by`/`audit_events.actor` (see Shared Patterns > Actor Capture below) — destructure both. + +**Campaign-exists check** (lines 34-40, adapt to your service call instead of inline query if the service itself validates existence): +```typescript +const campaignRes = await postgresClient.query<{ id: string }>( + `SELECT id FROM campaigns WHERE id = $1`, + [id] +); +if (!campaignRes.rows[0]) { + return NextResponse.json({ error: 'Campaign not found' }, { status: 404 }); +} +``` + +**Request body parsing** — no existing phishing route parses a JSON body; copy the shape from `app/api/admin/integrations/route.ts` PATCH (lines 62-67): +```typescript +let body: { key?: unknown; disabled?: unknown; reason?: unknown }; +try { + body = (await request.json()) as typeof body; +} catch { + return NextResponse.json({ error: 'Invalid JSON body' }, { status: 400 }); +} +``` +For `approve/route.ts`, the body shape is `{ actions: Array<{ actionType: string; params?: Record }> }` per D-03 (operator specifies which recommended actions to approve, with optional param overrides). + +**Delegation + error handling pattern** (lines 33-50, full try/catch): +```typescript +try { + ... + const result = await classifyCampaign(id); + return NextResponse.json(result); +} catch (err) { + console.error('[PHISHING-CLASSIFY] Failed to classify campaign', id, err); + return NextResponse.json( + { error: 'Failed to classify campaign', message: err instanceof Error ? err.message : 'Unknown error' }, + { status: 500 } + ); +} +``` +Copy verbatim, renaming the log tag to `[PHISHING-APPROVE]` and delegating to the new service's `approveRemediationActions(campaignId, actions, actor)`. + +--- + +### `app/api/phishing/campaigns/[id]/remediate/route.ts` (route, request-response, idempotent) + +**Analog:** same as above (`classify/route.ts`) for the route shell; **`lib/services/campaign-grouping-service.ts`** (lines 175-220, `postgresClient.transaction()` usage) for the idempotent-transition body inside the service it delegates to. + +**Idempotency-relevant excerpt** (`campaign-grouping-service.ts` lines 160-173, the "check-then-short-circuit" idiom to mirror for D-01's "calling remediate twice produces exactly one `completed` transition"): +```typescript +export async function groupReportIntoCampaign( + reportId: string, + opts?: { skipIfAlreadyGrouped?: boolean } +): Promise { + try { + if (opts?.skipIfAlreadyGrouped) { + const existing = await postgresClient.query<{ campaign_id: string | null }>( + `SELECT campaign_id::text AS campaign_id FROM reports WHERE id = $1`, + [reportId] + ); + if (existing.rows[0]?.campaign_id) { + return null; // already grouped, short-circuit (D-08) + } + } + return await postgresClient.transaction(async (client) => { + ... + }); + } catch (error) { ... } +} +``` +Apply the same "check current `status` column, short-circuit before the write" idiom in the remediate service function: read `remediation_actions.status` for the targeted row(s); if already `'completed'`, return the same success shape without re-inserting an `audit_events` row. + +Route shell: identical UUID guard + `requirePermission('phishing', 'remediate')` + try/catch/500 as `approve/route.ts` above — delegate to `remediateApprovedActions(campaignId, actor)`. + +--- + +### `app/api/phishing/campaigns/[id]/mark-false-positive/route.ts` (route, request-response, guarded state write) + +**Analog:** `classify/route.ts` for the shell; `app/api/admin/integrations/route.ts` PATCH (lines 58-93) for the "guard condition before mutating, capture actor from session" pattern. + +**Actor capture pattern** (line 80): +```typescript +const actor = (session?.user as { email?: string } | undefined)?.email ?? null; +``` +This is the concrete precedent for D-02's "Claude's Discretion: actor value format — likely session email, matching the `disabled_by` convention" note in CONTEXT.md. Use this exact cast/fallback shape for `audit_events.actor` and `remediation_actions.approved_by`. + +**Guard-before-write pattern** — no existing phishing route has a state guard; the shape to copy is "query current state, branch on a conflicting condition, return 409/400 before mutating" — synthesize from the general `try/catch` + explicit-status-check idiom used throughout (e.g. `campaigns/[id]/route.ts` lines 79-82, `if (!campaign) return NextResponse.json({ error: ... }, { status: 404 })`). For D-04's guard: +```typescript +const blockingRes = await postgresClient.query<{ id: string }>( + `SELECT id FROM remediation_actions WHERE campaign_id = $1 AND status IN ('approved', 'completed') LIMIT 1`, + [id] +); +if (blockingRes.rows[0]) { + return NextResponse.json( + { error: 'Cannot mark false positive: campaign already has approved or completed remediation' }, + { status: 409 } + ); +} +``` +Route shell otherwise identical: UUID guard + `requirePermission('phishing', 'approve')` (D-04 says same elevated tier as approve — no separate `'mark-false-positive'` action was added to the statement) + try/catch/500. + +--- + +### `lib/permissions.ts` (config edit — no new file) + +**Edit target:** lines 51 and 65 (this file, in place — not a new file/analog pair). + +Current state (lines 41-52, `superAdminRole`): +```typescript +export const superAdminRole = ac.newRole({ + ... + phishing: ["read", "analyze"], // approve/remediate ungranted until Phase 20 +}); +``` +Current state (lines 55-66, `adminRole`): +```typescript +export const adminRole = ac.newRole({ + ... + phishing: ["read", "analyze"], +}); +``` +D-02 change: both become `phishing: ["read", "analyze", "approve", "remediate"]`. The `statement` object (line 34) already has the full vocabulary — no change needed there. `userRole` (line 79, `phishing: ["read"]`) is untouched. + +--- + +### `lib/services/remediation-service.ts` (service, CRUD + event-driven audit trail) + +**Analog:** `lib/services/campaign-classifier.ts` (full orchestrator shape — `classifyCampaign`, lines 449-524) for the "single exported orchestrator wrapping try/catch, delegating to helper functions, ending in an append-only INSERT + RETURNING" shape; `lib/services/campaign-grouping-service.ts` (lines 175-220) for the `postgresClient.transaction()` multi-statement write shape needed when approve/remediate write both a `remediation_actions` row AND an `audit_events` row atomically. + +**Imports pattern** (campaign-classifier.ts lines 17-19): +```typescript +import type { AuthResults } from './eml-parser'; +import { postgresClient } from './postgres-client'; +import { getBlastRadius, type BlastRadiusResult } from './mimecast-blast-radius'; +``` +For `remediation-service.ts`: +```typescript +import { postgresClient } from './postgres-client'; +``` +(No external client dependency — D-01 confirms no real provider call this phase.) + +**Orchestrator + append-only insert pattern** (campaign-classifier.ts lines 449-524, condensed): +```typescript +export async function classifyCampaign(campaignId: string): Promise { + try { + const evidence = await gatherCampaignEvidence(campaignId); + ... + // D-02: append-only INSERT — no ON CONFLICT. Each classify call is a new + // history row; "current" verdict is the most recent by created_at. + const insertResult = await postgresClient.query<{ id: string; created_at: string }>( + `INSERT INTO classifications ( + campaign_id, verdict, confidence, summary, reasons, recommended_actions, requires_approval + ) VALUES ($1, $2, $3, $4, $5::jsonb, $6::jsonb, $7) + RETURNING id::text AS id, created_at::text AS created_at`, + [campaignId, verdict, confidence, summary, JSON.stringify(reasons), JSON.stringify(recommendedActions), requiresApproval] + ); + const row = insertResult.rows[0]; + return { id: row.id, campaignId, verdict, ... }; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.error('[CAMPAIGN-CLASSIFIER] Failed to classify campaign', campaignId, message); + throw error; + } +} +``` +Apply the identical shape for `approveRemediationActions(campaignId, actions, actor)`: read the latest `classifications` row (mirrors `app/api/phishing/campaigns/[id]/route.ts` lines 120-124's `ORDER BY created_at DESC` query) to validate the requested action types are in `recommendedActions`, then INSERT one `remediation_actions` row per approved action (`status: 'approved'`, `approved_by: actor`, `approved_at: NOW()`, `params` from the request body) inside a transaction that also writes one `audit_events` row (`event_type: 'remediation_approved'`). + +**Transaction pattern** (campaign-grouping-service.ts lines 175-176, 210-220 style — read-then-write inside one `client`): +```typescript +return await postgresClient.transaction(async (client) => { + const ownReportRes = await client.query( + `SELECT ... FROM reports WHERE id = $1`, + [reportId] + ); + const ownReport = ownReportRes.rows[0]; + if (!ownReport) { + throw new Error(`groupReportIntoCampaign: report ${reportId} not found`); + } + ... // further client.query() calls using the SAME `client`, not postgresClient +}); +``` +Use this exact "all queries go through the callback's `client` param, not the top-level `postgresClient`" discipline for the remediate/approve/mark-false-positive functions so the `remediation_actions` INSERT and the `audit_events` INSERT are atomic. + +**Idempotency check (D-01/REMED-04)** — status-check short-circuit, mirroring `groupReportIntoCampaign`'s `opts?.skipIfAlreadyGrouped` early-return (lines 165-173): +```typescript +if (existing.rows[0]?.campaign_id) { + return null; // already grouped, short-circuit (D-08) +} +``` +For `remediateApprovedActions`: query the `remediation_actions` row's `status` first; if already `'completed'`, return the same result shape without a second `audit_events` INSERT or a second `status` transition. + +**Error handling pattern** (campaign-classifier.ts lines 519-523): +```typescript +} catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.error('[CAMPAIGN-CLASSIFIER] Failed to classify campaign', campaignId, message); + throw error; +} +``` +Rethrow after logging — the route layer's try/catch converts to the 500 JSON response (per `classify/route.ts`'s pattern), so the service should NOT itself return `NextResponse` — keep the layering the same as Phase 19. + +--- + +### `lib/services/remediation-service.test.ts` (test) + +**Analog:** `lib/services/campaign-classifier.test.ts` (lines 1-17, mock setup) + +```typescript +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Mock postgresClient BEFORE importing the module under test — mirrors +// campaign-grouping-service.test.ts's vi.mock() factory-mocking discipline. +const queryMock = vi.fn(); +vi.mock('./postgres-client', () => ({ + postgresClient: { + query: (...args: unknown[]) => queryMock(...args), + }, +})); +``` +If `remediation-service.ts` uses `postgresClient.transaction()`, the mock factory must also export a `transaction` mock (check `campaign-grouping-service.test.ts` for the transaction-mock variant — not read in this pass, but the same file is the analog for that specific mock shape; grep `transaction` in that test file before writing this one). + +## Shared Patterns + +### Auth / Permission Gate +**Source:** `app/api/phishing/campaigns/[id]/classify/route.ts` lines 23-24; `lib/auth-utils.ts` lines 51-74 +**Apply to:** All three new routes (`approve` → action `'approve'`, `remediate` → action `'remediate'`, `mark-false-positive` → action `'approve'` per D-04) +```typescript +const { session, error } = await requirePermission('phishing', 'approve'); +if (error) return error; +``` +`requirePermission` internally calls `requireAuth()` (401 if no session) then `hasPermission(userRole, resource, action)` (403 if role lacks it) — no route-level code needed beyond this one call + early return. + +### UUID Path-Param Guard +**Source:** `app/api/phishing/campaigns/[id]/classify/route.ts` line 17 +**Apply to:** All three new routes (same `[id]` campaign param) +```typescript +const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; +if (!UUID_RE.test(id)) { + return NextResponse.json({ error: 'Invalid campaign id' }, { status: 400 }); +} +``` + +### Actor Capture (for `approved_by` / `audit_events.actor`) +**Source:** `app/api/admin/integrations/route.ts` line 80 +**Apply to:** `remediation-service.ts` functions and/or the route layer before delegating +```typescript +const actor = (session?.user as { email?: string } | undefined)?.email ?? null; +``` + +### Error Handling / Response Shape +**Source:** `app/api/phishing/campaigns/[id]/classify/route.ts` lines 33, 44-49 +**Apply to:** All three new routes +```typescript +try { + ... +} catch (err) { + console.error('[PHISHING-] Failed to campaign', id, err); + return NextResponse.json( + { error: 'Failed to campaign', message: err instanceof Error ? err.message : 'Unknown error' }, + { status: 500 } + ); +} +``` + +### Transactional Multi-Row Write +**Source:** `lib/services/campaign-grouping-service.ts` lines 175 onward +**Apply to:** `remediation-service.ts` — every function that writes both a `remediation_actions` row and an `audit_events` row must do so inside one `postgresClient.transaction(async (client) => { ... })`, using `client.query()` for every statement in that block (never the top-level `postgresClient.query()` inside the callback). + +### Append-Only History Insert (no `ON CONFLICT`) +**Source:** `lib/services/campaign-classifier.ts` lines 489-505 (`classifications` INSERT); confirmed as the established shape for `remediation_actions`/`audit_events` per CONTEXT.md `` ("Established Patterns" section: "Append-only history tables with `campaign_id` FK + `created_at`... `remediation_actions` and `audit_events` should follow the same insert-only shape, no upsert.") + +## No Analog Found + +None — all 5 files/edits have a strong analog. The only genuinely new pattern is the D-04 "guard-before-write" conflict check on `mark-false-positive`, for which no existing phishing route has a direct precedent; it is synthesized from the general "query state, branch, 4xx before mutating" idiom used for 404s elsewhere (`app/api/phishing/campaigns/[id]/route.ts` line 81) plus the actor-capture precedent from `app/api/admin/integrations/route.ts`. + +## Metadata + +**Analog search scope:** `app/api/phishing/**`, `app/api/admin/integrations/route.ts`, `lib/services/campaign-classifier.ts`, `lib/services/campaign-grouping-service.ts`, `lib/services/campaign-classifier.test.ts`, `lib/permissions.ts`, `lib/auth-utils.ts`, `migrations/097_phishing_triage_schema.sql` +**Files scanned:** 9 +**Pattern extraction date:** 2026-07-16