diff --git a/.planning/quick/260717-v6c-add-a-mark-as-accidental-report-action-t/260717-v6c-PLAN.md b/.planning/quick/260717-v6c-add-a-mark-as-accidental-report-action-t/260717-v6c-PLAN.md new file mode 100644 index 0000000..dda442b --- /dev/null +++ b/.planning/quick/260717-v6c-add-a-mark-as-accidental-report-action-t/260717-v6c-PLAN.md @@ -0,0 +1,238 @@ +--- +phase: quick-260717-v6c +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - lib/services/remediation-service.ts + - lib/services/triage-note-service.ts + - app/api/phishing/campaigns/[id]/mark-accidental-report/route.ts + - components/phishing/action-area-card.tsx + - components/phishing/timeline-card.tsx + - lib/services/remediation-service.test.ts + - lib/services/triage-note-service.test.ts +autonomous: true +requirements: [] + +must_haves: + truths: + - "A reviewer can click 'Mark as accidental report' on a phishing campaign and the campaign closes out" + - "Marking a campaign as accidental report posts the fixed reviewed-report note to every reporting employee's ticket" + - "The status change persists even when the customer note fails to post; the reviewer is told the note failed" + - "The accidental-report action is blocked when approved/completed remediation already exists (D-04 guard)" + - "The timeline shows a distinct, less-alarming 'Marked as accidental report' entry" + artifacts: + - path: "lib/services/remediation-service.ts" + provides: "markCampaignAccidentalReport (status flip + audit + post-commit note post)" + contains: "markCampaignAccidentalReport" + - path: "lib/services/triage-note-service.ts" + provides: "generateAndPostAccidentalReportNote (fixed-template customer note)" + contains: "generateAndPostAccidentalReportNote" + - path: "app/api/phishing/campaigns/[id]/mark-accidental-report/route.ts" + provides: "POST route gated by phishing/approve" + exports: ["POST"] + - path: "components/phishing/action-area-card.tsx" + provides: "Mark as accidental report button + confirm dialog" + contains: "accidental" + - path: "components/phishing/timeline-card.tsx" + provides: "campaign_marked_accidental_report timeline case" + contains: "campaign_marked_accidental_report" + key_links: + - from: "app/api/phishing/campaigns/[id]/mark-accidental-report/route.ts" + to: "markCampaignAccidentalReport" + via: "service import + call" + pattern: "markCampaignAccidentalReport" + - from: "lib/services/remediation-service.ts" + to: "generateAndPostAccidentalReportNote" + via: "post-commit call inside try/catch" + pattern: "generateAndPostAccidentalReportNote" + - from: "components/phishing/action-area-card.tsx" + to: "/api/phishing/campaigns/[id]/mark-accidental-report" + via: "fetch in confirm handler" + pattern: "mark-accidental-report" +--- + + +Add a "Mark as accidental report" action to the phishing campaign review Action Area. When an employee accidentally reports a legitimate email, a reviewer can close out the campaign AND post a customer-facing note letting the reporter know the report was reviewed and needs no further action. + +Purpose: Today the false-positive action is silent — it never notifies the reporting employee. This adds a parallel action that both flips campaign status and posts an appreciative "no action needed" note. + +Output: New backend service functions, a new API route, two updated frontend components, and mirrored test coverage. Every piece mirrors an existing verbatim pattern in the codebase (`markCampaignFalsePositive`, `generateAndPostAcknowledgment`, the mark-false-positive route, the false-positive button/dialog, the `campaign_marked_false_positive` timeline case). + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/STATE.md +@./CLAUDE.md + + + + +markCampaignFalsePositive (lib/services/remediation-service.ts ~line 276) — the exact template: +``` +export interface MarkFalsePositiveResult { campaignId: string; status: 'false_positive'; auditEventId: string; } +export async function markCampaignFalsePositive(campaignId, actor, reason?): Promise { + return postgresClient.transaction(async (client) => { + // FOR UPDATE guard: reject if remediation_actions has any approved/completed row -> RemediationConflictError + // SELECT status FROM campaigns WHERE id=$1 -> RemediationValidationError('Campaign not found') if missing + // UPDATE campaigns SET status='false_positive', updated_at=NOW() + // writeAuditEvent({ campaignId, actor, eventType:'campaign_marked_false_positive', payload:{ previousStatus, reason: reason ?? null } }, client) + return { campaignId, status:'false_positive', auditEventId }; + }); +} +``` +Note the post-commit note-post pattern already used by `remediateApprovedActions` (~line 242): AFTER the transaction, `try { await generateAndPostAcknowledgment(campaignId) } catch (err) { console.error(...) }` — external Autotask call must be OUTSIDE the FOR UPDATE-locked transaction. + +generateAndPostAcknowledgment (lib/services/triage-note-service.ts ~line 212) — the exact template: +``` +export async function generateAndPostAcknowledgment(campaignId): Promise { + // SELECT id::text, ticket_id::text AS ticket_id FROM reports WHERE campaign_id=$1 ORDER BY created_at ASC + const noteText = [ ...lines ].join('\n'); + const client = getAutotaskClient(); + const tickets: TriageNotePostResult[] = []; + for (const report of reports) { + try { + await client.createEntity('TicketNotes', { ticketID: Number(report.ticket_id), title, description: noteText, noteType: 18, publish: 1 }); + tickets.push({ ticketId: report.ticket_id, posted: true }); + } catch (err) { console.error(...); tickets.push({ ticketId: report.ticket_id, posted:false, error: err instanceof Error ? err.message : 'Unknown error' }); } + } + return { noteText, tickets }; +} +``` +TriageNoteResult = { noteText: string; tickets: TriageNotePostResult[] }; TriageNotePostResult = { ticketId: string; posted: boolean; error?: string }. + +Route template (app/api/phishing/campaigns/[id]/mark-false-positive/route.ts): requirePermission('phishing','approve'), UUID_RE validation (400), optional JSON body reason parse (400 on invalid JSON), campaign-existence SELECT (404), call service, RemediationConflictError->409 / RemediationValidationError->400 / else 500. + +Frontend false-positive block (components/phishing/action-area-card.tsx): +- state: isMarkingFalsePositive, falsePositiveDialogOpen, falsePositiveReason +- resolved = campaignStatus === 'false_positive' || completedAction != null (line 412) +- resolvedTooltipCopy() (line 414) — completedAction branch + false-positive branch +- hasBlockingRemediation = remediationActions.some(a => approved/completed) (line 425) +- markFalsePositiveDisabledReason = !canApprove ? ... : resolved ? resolvedTooltipCopy() : hasBlockingRemediation ? ... : undefined (line 450) +- GatedButton (line 550) + AlertDialog (line 588) with Textarea reason + handleMarkFalsePositiveConfirm (line 475) + +Timeline case (components/phishing/timeline-card.tsx line 90): +``` +case 'campaign_marked_false_positive': + return { label:'Marked as false positive', icon: XCircle, dotClass:'bg-slate-500', textClass:'text-slate-600' }; +``` +remediation_approved (line 79) uses icon CheckCircle2, dotClass 'bg-blue-500', textClass 'text-blue-600' — reuse this blue/CheckCircle2 tint for the accidental-report case (distinct from slate/XCircle false-positive and from THREAT tint). + + + + + + + Task 1: Add backend service functions (status flip + customer note) + lib/services/remediation-service.ts, lib/services/triage-note-service.ts + + - generateAndPostAccidentalReportNote: returns { noteText, tickets } with the exact locked copy; posts noteType 18 / publish 1 to every ticket from the reports lookup; one ticket's createEntity failure is caught inside the loop and recorded as { posted:false, error } without aborting the rest. + - markCampaignAccidentalReport: rejects with RemediationConflictError when an approved/completed remediation_actions row exists; on success flips campaigns.status to 'accidental_report', writes a 'campaign_marked_accidental_report' audit event with payload { previousStatus, reason: reason ?? null }, and returns { campaignId, status:'accidental_report', auditEventId, notePosted, noteError? }. + - When the post-commit note call throws, markCampaignAccidentalReport still returns the committed status change with notePosted:false and noteError set (never throws past the function for a note failure). + + +In lib/services/triage-note-service.ts, add exported `generateAndPostAccidentalReportNote(campaignId: string): Promise` immediately after `generateAndPostAcknowledgment` (~line 253). Copy `generateAndPostAcknowledgment` verbatim in structure — same reports lookup query, same per-ticket try/catch INSIDE the loop (D-05 isolation), same noteType 18 / publish 1, same `{ noteText, tickets }` return. Change ONLY: the title to `Thank You — Report Reviewed` and the body to the locked copy, built as two paragraphs joined by `'\n\n'`: +Paragraph 1: `Thanks for flagging this — after review, this turned out to be a legitimate email that was reported by mistake, not a phishing attempt.` +Paragraph 2: `No action is needed on your part, and this report has been closed out. If anything ever looks off in the future, please keep reporting it — that's exactly the right move.` +Zero interpolation of any evidence/URL/classification data (T-23-01 invariant). Use a distinct log prefix e.g. `[PHISHING-ACCIDENTAL-REPORT]`. + +In lib/services/remediation-service.ts, add exported `markCampaignAccidentalReport(campaignId: string, actor: string | null, reason?: string)` immediately after `markCampaignFalsePositive` (~line 322). Add a `MarkAccidentalReportResult` interface: `{ campaignId: string; status: 'accidental_report'; auditEventId: string; notePosted: boolean; noteError?: string }`. Copy the `markCampaignFalsePositive` transaction body verbatim (identical FOR UPDATE guard query and RemediationConflictError, identical campaign-existence check and RemediationValidationError, identical previousStatus capture). Change ONLY: `UPDATE campaigns SET status = 'accidental_report'` and `eventType: 'campaign_marked_accidental_report'`. AFTER the transaction resolves (not inside it — the note post is an external Autotask call that must not run inside the FOR UPDATE lock), call `generateAndPostAccidentalReportNote(campaignId)` inside a try/catch mirroring the post-commit pattern at ~line 242: on success set `notePosted = true`; on catch, `console.error(...)` and set `notePosted = false, noteError = err instanceof Error ? err.message : 'Unknown error'`. Return the transaction result merged with `{ notePosted, noteError }`. Import `generateAndPostAccidentalReportNote` from the triage-note-service (match how `generateAndPostAcknowledgment` is already imported). `campaigns.status` is unconstrained TEXT (migrations/097) — no migration needed for the new value. + + + npx tsc --noEmit --pretty + + Both functions exported and typed; markCampaignAccidentalReport returns the richer result shape; tsc passes. + + + + Task 2: Add the mark-accidental-report API route + app/api/phishing/campaigns/[id]/mark-accidental-report/route.ts + +Create the route by copying app/api/phishing/campaigns/[id]/mark-false-positive/route.ts verbatim, changing ONLY: import and call `markCampaignAccidentalReport` instead of `markCampaignFalsePositive`; keep the same `requirePermission('phishing', 'approve')` gate, the same `UUID_RE` validation (400), the same optional `{ reason?: string }` body parse (400 on invalid JSON), and the same campaign-existence SELECT (404 if missing). Return the full result object (which now includes `notePosted`/`noteError`) as JSON so the frontend can distinguish full success from status-changed-but-note-failed. Keep identical error handling: `RemediationConflictError` -> 409, `RemediationValidationError` -> 400, else 500 with a `[PHISHING-MARK-ACCIDENTAL]` log prefix. Update the file header comment to describe the accidental-report action. + + + npx tsc --noEmit --pretty + + Route exports POST, gated by phishing/approve, returns the richer result including notePosted/noteError; tsc passes. + + + + Task 3: Add frontend button, dialog, and timeline case + components/phishing/action-area-card.tsx, components/phishing/timeline-card.tsx + +In components/phishing/action-area-card.tsx, mirror the false-positive block: +- Add state: `isMarkingAccidentalReport`, `accidentalReportDialogOpen`, `accidentalReportReason` (alongside the false-positive state ~line 315-318). +- Update `resolved` (line 412) to also treat `campaignStatus === 'accidental_report'` as resolved: `campaignStatus === 'false_positive' || campaignStatus === 'accidental_report' || completedAction != null`. +- Update `resolvedTooltipCopy()` (line 414) to add a distinct branch for the accidental-report case, e.g. `Marked as an accidental report` (with the campaign date if available), kept separate from the existing false-positive/completed messages. +- Add `markAccidentalReportDisabledReason` mirroring `markFalsePositiveDisabledReason` (line 450) exactly: `!canApprove` -> permission message, else `resolved` -> `resolvedTooltipCopy()`, else `hasBlockingRemediation` -> blocking message, else undefined. +- Add a `handleMarkAccidentalReportConfirm` handler mirroring `handleMarkFalsePositiveConfirm` (line 475): POST to `/api/phishing/campaigns/${campaignId}/mark-accidental-report` with body `accidentalReportReason ? { reason: accidentalReportReason } : {}`. On success, inspect the response: if `notePosted === true` toast success "Marked as accidental report, reporter notified"; if `notePosted === false` toast a warning that surfaces `noteError` so an admin knows to follow up manually (exact wording at executor discretion). Close the dialog and reset the reason on success; refresh the same way the false-positive handler does. +- Add a `GatedButton` (mirror line 550) labeled "Mark as accidental report" next to the false-positive button, wired to open `accidentalReportDialogOpen`, disabled by `markAccidentalReportDisabledReason`, loading on `isMarkingAccidentalReport`. +- Add an `AlertDialog` (mirror line 588) with an optional reason `Textarea` (bound to `accidentalReportReason`). Confirm-dialog copy must mention it ALSO posts the reviewed-report note to the employee, e.g.: "Mark this campaign as an accidental report? This posts a note to the reporting employee explaining no action is needed, and closes out the campaign. This cannot be undone." + +In components/phishing/timeline-card.tsx, add `case 'campaign_marked_accidental_report':` alongside `case 'campaign_marked_false_positive':` (line 90). Return label "Marked as accidental report — reporter notified" with a visually distinct, less-alarming tint than the slate/XCircle false-positive entry and distinct from THREAT-toned entries — use the existing blue/CheckCircle2 pairing (`icon: CheckCircle2, dotClass: 'bg-blue-500', textClass: 'text-blue-600'`, matching `remediation_approved`). CheckCircle2 is already imported. + + + npx tsc --noEmit --pretty + + New button + dialog render and post to the new route; success/partial-failure toasts differ; timeline shows a distinct accidental-report entry; resolved/tooltip logic covers accidental_report; tsc passes. + + + + Task 4: Mirror test coverage + lib/services/remediation-service.test.ts, lib/services/triage-note-service.test.ts + + - remediation-service.test.ts: markCampaignAccidentalReport rejects with RemediationConflictError when an approved/completed remediation exists (D-04 guard); on success flips status to 'accidental_report' and writes a 'campaign_marked_accidental_report' audit event; a note-post failure still returns the committed status change with notePosted:false and noteError set. + - triage-note-service.test.ts: generateAndPostAccidentalReportNote posts a non-empty noteText with noteType 18 / publish 1 to every reporting ticket; one ticket's createEntity rejection is isolated (posted:false + error) without aborting posts to the other tickets. + + +Add tests to lib/services/remediation-service.test.ts and lib/services/triage-note-service.test.ts by mirroring the existing false-positive and acknowledgment test blocks (same mocking of postgresClient / getAutotaskClient, same describe/it structure, same fixtures). Cover: D-04 guard rejection, successful status flip + correct audit event type, note-posting success (noteType 18 / publish 1, non-empty noteText, correct title/body), per-ticket try/catch isolation, and the note-post-failure-still-commits path for markCampaignAccidentalReport. Do not duplicate the false-positive tests — add new cases scoped to the accidental-report functions. + + + npx tsc --noEmit --pretty && npx vitest run lib/services/remediation-service.test.ts lib/services/triage-note-service.test.ts + + New tests pass; both target vitest files green; tsc passes. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| client -> API route | Untrusted campaign id + optional reason cross here | +| API -> Autotask (external) | Customer-visible note post leaves the trust boundary to the PSA | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-v6c-01 | Elevation of Privilege | mark-accidental-report route | mitigate | requirePermission('phishing','approve') gate mirrors the false-positive route — no lower tier | +| T-v6c-02 | Tampering | campaign id path param | mitigate | UUID_RE validation before any DB query (rejects malformed ids with 400) | +| T-v6c-03 | Information Disclosure | customer-visible Autotask note | mitigate | Fixed template with zero evidence/URL/classification interpolation (T-23-01 invariant) — nothing from parsed email reaches the note | +| T-v6c-04 | Denial of Service | note post inside DB transaction | mitigate | Autotask call runs AFTER the FOR UPDATE-locked transaction commits; note failure is caught and never rolls back or throws | +| T-v6c-SC | Tampering | npm/pip/cargo installs | accept | No new dependencies introduced by this plan | + + + +- `npx tsc --noEmit --pretty` passes. +- `npx vitest run lib/services/remediation-service.test.ts lib/services/triage-note-service.test.ts` passes. +- Manual sanity: on a campaign with no blocking remediation, the "Mark as accidental report" button posts the note and closes the campaign; on a campaign with approved/completed remediation the button is disabled with the blocking tooltip; the route returns 409 if called directly in that state. + + + +- markCampaignAccidentalReport and generateAndPostAccidentalReportNote exist, exported, mirroring their false-positive/acknowledgment templates. +- New route POST /api/phishing/campaigns/[id]/mark-accidental-report gated by phishing/approve, returns notePosted/noteError. +- Action Area shows the new button + confirm dialog; resolved + tooltip logic covers accidental_report; toasts distinguish full success from note-failed. +- Timeline shows a distinct, less-alarming accidental-report entry. +- All tests + tsc green. + + + +Create `.planning/quick/260717-v6c-add-a-mark-as-accidental-report-action-t/260717-v6c-SUMMARY.md` when done. +