diff --git a/.planning/phases/20-remediation-approval-audit-safety/20-02-SUMMARY.md b/.planning/phases/20-remediation-approval-audit-safety/20-02-SUMMARY.md new file mode 100644 index 0000000..33212bf --- /dev/null +++ b/.planning/phases/20-remediation-approval-audit-safety/20-02-SUMMARY.md @@ -0,0 +1,110 @@ +--- +phase: 20-remediation-approval-audit-safety +plan: 02 +subsystem: api +tags: [nextjs, api-routes, permissions, better-auth, phishing-triage, audit-log] + +# Dependency graph +requires: + - phase: 20-remediation-approval-audit-safety + provides: "Plan 01's transactional service layer (approveRemediationActions, remediateApprovedActions, markCampaignFalsePositive, writeAuditEvent) and its typed error classes" +provides: + - "lib/permissions.ts grants phishing:approve and phishing:remediate to super-admin and admin roles only (D-02); userRole stays read-only" + - "POST /api/phishing/campaigns/[id]/approve — permission-gated, D-03 action-list body, delegates to approveRemediationActions" + - "POST /api/phishing/campaigns/[id]/remediate — permission-gated, delegates to remediateApprovedActions (idempotent completion)" + - "POST /api/phishing/campaigns/[id]/mark-false-positive — approve-tier gated (D-04), delegates to markCampaignFalsePositive" + - "classify route now writes a 'campaign_classified' audit event, completing REMED-06's four-action audit coverage" +affects: [21-autotask-triage-note, 22-approval-ui-livelink] + +# Tech tracking +tech-stack: + added: [] + patterns: + - "Route shell precedent (from classify route) reused verbatim across all three new routes: requirePermission early-return, UUID_RE guard -> 400, campaign-exists SELECT -> 404, try/catch mapping typed service errors to HTTP status, actor derived from session.user.email never the request body" + +key-files: + created: + - "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" + modified: + - "lib/permissions.ts" + - "app/api/phishing/campaigns/[id]/classify/route.ts" + +key-decisions: + - "mark-false-positive body parsing reads request.text() first and only JSON.parse()s if non-empty, so an absent/empty body is tolerated (no request.json() try/catch swallowing the 'no body sent' case as valid JSON parse failure)" + - "classify route's audit write happens inside the existing try/catch after classifyCampaign() succeeds; a write failure surfaces as a 500 even though the classification row has already committed (append-only, not rolled back) — documented inline" + +patterns-established: + - "Every state-changing phishing route destructures `session` (not just `error`) from requirePermission so the actor can be captured for the audit trail" + +requirements-completed: [REMED-02, REMED-03, REMED-04, REMED-05, REMED-06] + +# Metrics +duration: 10min +completed: 2026-07-16 +--- + +# Phase 20 Plan 02: Remediation Approval Routes Summary + +**Three new permission-gated POST routes (approve/remediate/mark-false-positive) wired to Plan 01's service layer, plus a 'campaign_classified' audit event added to the existing classify route, completing all four state-changing actions' audit coverage.** + +## Performance + +- **Duration:** ~10 min +- **Started:** 2026-07-16 (per orchestrator dispatch, wave 2) +- **Completed:** 2026-07-16 +- **Tasks:** 3 completed +- **Files modified:** 5 (3 created, 2 modified) + +## Accomplishments +- `lib/permissions.ts` — `superAdminRole` and `adminRole` now grant `phishing: ["read", "analyze", "approve", "remediate"]` (D-02); `userRole` unchanged at `["read"]`, so a plain user is rejected 403 from all three new endpoints +- `POST /api/phishing/campaigns/[id]/approve` — gated by `phishing:approve`, validates `{ actions: [{actionType, params?}] }` is a non-empty array (D-03), delegates to `approveRemediationActions`, maps `RemediationValidationError`→400 and `RemediationConflictError`→409 +- `POST /api/phishing/campaigns/[id]/remediate` — gated by `phishing:remediate`, no request body (D-01 — remediates whatever was previously approved), delegates to `remediateApprovedActions`, same error mapping +- `POST /api/phishing/campaigns/[id]/mark-false-positive` — gated by `phishing:approve` (D-04 elevated tier, no separate action key), optional `{ reason? }` body, delegates to `markCampaignFalsePositive`, maps the D-04 conflict guard to 409 +- `classify/route.ts` extended to destructure `session` and write a `campaign_classified` audit event (via `writeAuditEvent`) after a successful classification — REMED-06's four-action audit coverage (classify/approve/remediate/mark-false-positive) is now complete + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Grant approve + remediate to admin roles (D-02)** - `80e7129` (feat) +2. **Task 2: approve + remediate routes** - `65c4253` (feat) +3. **Task 3: mark-false-positive route + classify audit wiring** - `1a12607` (feat) + +**Plan metadata:** (this commit) + +## Files Created/Modified +- `lib/permissions.ts` - superAdminRole/adminRole grant phishing approve+remediate; userRole and the statement vocabulary unchanged +- `app/api/phishing/campaigns/[id]/approve/route.ts` - POST approve endpoint (D-03 action-list body) +- `app/api/phishing/campaigns/[id]/remediate/route.ts` - POST remediate endpoint (idempotent completion) +- `app/api/phishing/campaigns/[id]/mark-false-positive/route.ts` - POST mark-false-positive endpoint (D-04 approve-tier gate) +- `app/api/phishing/campaigns/[id]/classify/route.ts` - now writes a `campaign_classified` audit event after successful classification + +## Decisions Made +- Body parsing in mark-false-positive uses `request.text()` + conditional `JSON.parse` rather than `request.json()` in a try/catch, so a genuinely empty body (no bytes sent) is treated as "no reason provided" rather than a parse failure — matches the plan's "tolerate an empty/absent body" instruction more precisely than a blanket try/catch around `request.json()` would (an empty body passed to `request.json()` throws, which would have incorrectly produced a 400 for the common case of "no reason given"). +- Kept the classify route's audit write inside the existing try/catch (as instructed) rather than isolating it in its own try/catch — a failure there surfaces as the route's existing 500 path, and the classification row itself is unaffected since it was already committed by `classifyCampaign` (append-only, no transaction spanning both). + +## Deviations from Plan + +None - plan executed exactly as written. All acceptance criteria (grep patterns for `requirePermission`, `UUID_RE`, `campaign_classified`, actor-from-session, and the two role permission-array greps) verified directly against the edited/created files. + +## Issues Encountered +None. + +## User Setup Required + +None - no external service configuration required. No package installs. + +## Next Phase Readiness +- All four phishing-triage state-changing actions (classify, approve, remediate, mark-false-positive) are now exposed over HTTP, permission-gated, and fully audited. +- Plan 21 (Autotask triage note) and Plan 22 (Approval UI) can call these four routes directly; the JSON response shapes match the Plan 01 service return types (`ApprovedRemediationAction[]`, `RemediateResult`, `MarkFalsePositiveResult`, `ClassifyResult`). +- No blockers. `npx tsc --noEmit --pretty` is clean at HEAD across all edited/created files. + +## Self-Check: PASSED + +All created files and commit hashes verified present on disk / in git log. + +--- +*Phase: 20-remediation-approval-audit-safety* +*Completed: 2026-07-16*