diff --git a/.planning/phases/20-remediation-approval-audit-safety/20-01-SUMMARY.md b/.planning/phases/20-remediation-approval-audit-safety/20-01-SUMMARY.md new file mode 100644 index 0000000..98df8d2 --- /dev/null +++ b/.planning/phases/20-remediation-approval-audit-safety/20-01-SUMMARY.md @@ -0,0 +1,107 @@ +--- +phase: 20-remediation-approval-audit-safety +plan: 01 +subsystem: api +tags: [postgres, transactions, phishing-triage, audit-log, vitest] + +# Dependency graph +requires: + - phase: 19-classification-engine + provides: classifications table + classifyCampaign's recommended_actions/requires_approval vocabulary that approveRemediationActions validates against + - phase: 18-campaign-grouping-and-storage + provides: campaigns table (status column) that markCampaignFalsePositive transitions +provides: + - writeAuditEvent — single parameterized append-only audit_events insert path, usable standalone or inside a transaction + - approveRemediationActions — recommended-only validated approval, one remediation_actions row + one audit row per action, atomic + - remediateApprovedActions — idempotent (status='approved' FOR UPDATE filter) simulated completion, explicit failure on zero rows + - markCampaignFalsePositive — D-04 conflict-guarded false-positive transition with atomic audit row +affects: [20-02, 21-autotask-triage-note, 22-approval-ui-livelink] + +# Tech tracking +tech-stack: + added: [] + patterns: + - "Single audit-event writer (writeAuditEvent) accepting an optional transaction client, called from inside the same postgresClient.transaction as every state-changing write — guarantees no state change can commit without its audit row" + - "Recommended-only validation: approve reads the latest classifications row and rejects any actionType not present in recommended_actions before writing anything" + - "Idempotency via status-filtered FOR UPDATE (status='approved') rather than a separate idempotency-key column — a re-run naturally finds nothing left to transition" + +key-files: + created: + - lib/services/phishing-audit.ts + - lib/services/phishing-audit.test.ts + - lib/services/remediation-service.ts + - lib/services/remediation-service.test.ts + modified: [] + +key-decisions: + - "Reworded a header comment that literally contained the string 'not_implemented' inside a /** */ JSDoc block — the plan's grep-based acceptance check only excludes // line comments, so the literal string tripped the D-01 no-not_implemented check even though it was descriptive prose, not code" + - "remediateApprovedActions treats any row whose status is not 'approved' (including already-'completed') as alreadyCompleted:true in its return value and skips both the UPDATE and the audit write for it — this is the sole idempotency mechanism, no separate dedupe table" + +patterns-established: + - "Every phishing-triage state-changing service function returns from inside postgresClient.transaction(async (client) => {...}), passing `client` through to writeAuditEvent(...) as its final argument" + +requirements-completed: [REMED-01, REMED-02, REMED-03, REMED-04, REMED-05, REMED-06] + +# Metrics +duration: 12min +completed: 2026-07-16 +--- + +# Phase 20 Plan 01: Remediation Service Layer Summary + +**Transactional approve/remediate/mark-false-positive service layer with a single append-only audit writer, proving idempotency (REMED-04), atomic audit writes (REMED-06), and the D-04 remediated-vs-false-positive conflict guard via 11 unit tests.** + +## Performance + +- **Duration:** ~12 min +- **Started:** 2026-07-16T14:26:00Z (approx, per orchestrator dispatch) +- **Completed:** 2026-07-16T14:38:52Z +- **Tasks:** 3 completed +- **Files modified:** 4 (all new) + +## Accomplishments +- `writeAuditEvent` — the single, parameterized, append-only `audit_events` insert path, usable standalone or with an injected transaction client +- `approveRemediationActions` — validates every requested action against the campaign's latest `classifications.recommended_actions`, materializes only recommended actions as `status='approved'` rows with approver/timestamp/params, and writes exactly one `remediation_approved` audit row per call, all inside one transaction +- `remediateApprovedActions` — transitions `status='approved'` rows to `'completed'` (D-01 simulated internal effect, no external provider call for any of the 7 action types), proven idempotent by a double-call test (zero additional transitions/audits on the second call), and throws `RemediationValidationError` explicitly when nothing is approved (never a silent success) +- `markCampaignFalsePositive` — D-04 guard (`RemediationConflictError`) blocks marking a campaign false-positive when any approved/completed remediation exists; otherwise sets `campaigns.status='false_positive'` and writes one atomic audit row recording `previousStatus` + optional `reason` + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Audit-event writer (phishing-audit.ts)** - `98d3e92` (feat) +2. **Task 2: approve + remediate orchestrators (idempotent, audited)** - `2937fe7` (test, RED) → `3d63fab` (feat, GREEN) +3. **Task 3: mark-false-positive orchestrator (D-04 guard, audited)** - `b1b66f9` (feat, extends remediation-service.ts + its test suite) + +**Plan metadata:** (this commit) + +_Note: Task 2/3 shared a single test file authored up front (all 9 assertions) at the RED commit; Task 2's GREEN commit implements approve+remediate (6 assertions passing, 3 pending), Task 3's commit implements markCampaignFalsePositive (all 9 assertions passing) plus a one-line test-mock SQL-substring fix needed to correctly route the D-04 guard query in the fake transaction client._ + +## Files Created/Modified +- `lib/services/phishing-audit.ts` - `writeAuditEvent(input, client?)`, the single append-only `audit_events` insert path +- `lib/services/phishing-audit.test.ts` - proves the parameterized INSERT shape and the injected-client routing +- `lib/services/remediation-service.ts` - `approveRemediationActions`, `remediateApprovedActions`, `markCampaignFalsePositive`, `RemediationValidationError`, `RemediationConflictError` +- `lib/services/remediation-service.test.ts` - 9 assertions covering recommended-only validation, idempotency (REMED-04), zero-approved explicit failure (REMED-03), and the D-04 conflict guard + +## Decisions Made +- Reworded the D-01 header comment to avoid the literal substring `not_implemented` inside a `/** */` block, since the plan's acceptance-check grep (`grep -vn "^\s*//"`) only strips `//` line comments, not JSDoc blocks — the original prose ("this file NEVER returns a `not_implemented` code path") would have failed the automated check despite being correct, non-code documentation. +- Kept `remediateApprovedActions`'s per-row branching (`status === 'approved'` vs. everything else) as the sole idempotency mechanism rather than adding a separate "already processed" tracking column — matches the plan's explicit interface contract (`status='approved' FOR UPDATE filter drives idempotent completion`). + +## Deviations from Plan + +None affecting behavior or scope — one wording-only fix (see Decisions Made) to satisfy an acceptance-check grep pattern, and one test-mock SQL-substring correction (the D-04 guard query's SQL is multi-line, so the mock's original single-line substring match needed loosening to `sql.includes('FROM remediation_actions') && sql.includes('status IN')`). + +## Issues Encountered +None beyond the two items above (both resolved inline, no re-scoping required). + +## User Setup Required + +None - no external service configuration required. No package installs. + +## Next Phase Readiness +- Plan 02 (routes) can now import `approveRemediationActions`, `remediateApprovedActions`, `markCampaignFalsePositive`, and their typed error classes to wire `/api/phishing/campaigns/[id]/{approve,remediate,mark-false-positive}` routes, mapping `RemediationValidationError` → 400 and `RemediationConflictError` → 409 (or equivalent per Plan 02's own design). +- No blockers. `npx vitest run lib/services/phishing-audit.test.ts lib/services/remediation-service.test.ts` (11 tests) and `npx tsc --noEmit --pretty` are both clean at HEAD. + +--- +*Phase: 20-remediation-approval-audit-safety* +*Completed: 2026-07-16*