docs(20): create phase plan
This commit is contained in:
parent
377ea3853d
commit
948a218d86
5 changed files with 961 additions and 7 deletions
|
|
@ -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 | - |
|
||||
|
||||
|
|
|
|||
|
|
@ -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%
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
---
|
||||
|
||||
<objective>
|
||||
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.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@$HOME/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.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
|
||||
|
||||
<interfaces>
|
||||
<!-- Contracts the executor must build against. Do not re-derive from the codebase. -->
|
||||
|
||||
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<T>(callback: (client: PoolClient) => Promise<T>): Promise<T>;
|
||||
// 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.
|
||||
```
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Audit-event writer (phishing-audit.ts)</name>
|
||||
<files>lib/services/phishing-audit.ts, lib/services/phishing-audit.test.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<action>
|
||||
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<string, unknown>; }` and a minimal
|
||||
`interface AuditQueryClient { query: (text: string, params?: unknown[]) =>
|
||||
Promise<{ rows: Array<{ id: string }> }> }`.
|
||||
Signature: `writeAuditEvent(input: AuditEventInput, client?: AuditQueryClient):
|
||||
Promise<string>` — 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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npx vitest run lib/services/phishing-audit.test.ts</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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.
|
||||
</acceptance_criteria>
|
||||
<done>writeAuditEvent exists, is parameterized, supports an injected transaction client, and its test suite passes.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: approve + remediate orchestrators (idempotent, audited)</name>
|
||||
<files>lib/services/remediation-service.ts, lib/services/remediation-service.test.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- 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).
|
||||
</behavior>
|
||||
<action>
|
||||
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<string, unknown> }` 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 <type> 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).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npx vitest run lib/services/remediation-service.test.ts && npx tsc --noEmit --pretty</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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.
|
||||
</acceptance_criteria>
|
||||
<done>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.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 3: mark-false-positive orchestrator (D-04 guard, audited)</name>
|
||||
<files>lib/services/remediation-service.ts, lib/services/remediation-service.test.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<behavior>
|
||||
- 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.
|
||||
</behavior>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npx vitest run lib/services/remediation-service.test.ts && npx tsc --noEmit --pretty</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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.
|
||||
</acceptance_criteria>
|
||||
<done>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).</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## 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. |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `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.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 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).
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/20-remediation-approval-audit-safety/20-01-SUMMARY.md` when done.
|
||||
</output>
|
||||
|
|
@ -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"
|
||||
---
|
||||
|
||||
<objective>
|
||||
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.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@$HOME/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.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
|
||||
|
||||
<interfaces>
|
||||
<!-- Service surface delivered by Plan 01 — call these directly. -->
|
||||
```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<string, unknown>; }
|
||||
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<string, unknown> }, client?): Promise<string>;
|
||||
```
|
||||
|
||||
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"]`.
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Grant approve + remediate to admin roles (D-02)</name>
|
||||
<files>lib/permissions.ts</files>
|
||||
<read_first>
|
||||
- lib/permissions.ts (full file — superAdminRole line 51, adminRole line 65, userRole line 79; the statement.phishing vocabulary line 34)
|
||||
</read_first>
|
||||
<action>
|
||||
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).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npx tsc --noEmit --pretty && node -e "const {hasPermission}=require('./lib/permissions.ts')" 2>/dev/null; grep -n "phishing:" lib/permissions.ts</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- `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.
|
||||
</acceptance_criteria>
|
||||
<done>super-admin and admin can approve/remediate; user remains read-only. D-02 satisfied.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: approve + remediate routes</name>
|
||||
<files>app/api/phishing/campaigns/[id]/approve/route.ts, app/api/phishing/campaigns/[id]/remediate/route.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<action>
|
||||
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<string, unknown> }> }`.
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>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</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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.
|
||||
</acceptance_criteria>
|
||||
<done>approve and remediate endpoints exist, permission-gated, UUID-guarded, actor from session, service-delegated with correct status mapping.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: mark-false-positive route + classify audit wiring</name>
|
||||
<files>app/api/phishing/campaigns/[id]/mark-false-positive/route.ts, app/api/phishing/campaigns/[id]/classify/route.ts</files>
|
||||
<read_first>
|
||||
- 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)
|
||||
</read_first>
|
||||
<action>
|
||||
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).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>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</automated>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- 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.
|
||||
</acceptance_criteria>
|
||||
<done>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).</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## 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. |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `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.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 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.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/20-remediation-approval-audit-safety/20-02-SUMMARY.md` when done.
|
||||
</output>
|
||||
|
|
@ -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<string, unknown> }> }` 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<GroupReportResult | null> {
|
||||
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<ClassifyResult> {
|
||||
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<OwnReportRow>(
|
||||
`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-<ACTION>] Failed to <verb> campaign', id, err);
|
||||
return NextResponse.json(
|
||||
{ error: 'Failed to <verb> 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 `<code_context>` ("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
|
||||
Loading…
Add table
Add a link
Reference in a new issue