docs(260717-v6c): pre-dispatch plan for Mark as accidental report action
This commit is contained in:
parent
ec6c5d05e6
commit
2b48dc0c10
1 changed files with 238 additions and 0 deletions
|
|
@ -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"
|
||||
---
|
||||
|
||||
<objective>
|
||||
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).
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@$HOME/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/STATE.md
|
||||
@./CLAUDE.md
|
||||
|
||||
<interfaces>
|
||||
<!-- Direct templates to mirror. Executor should copy these patterns verbatim, changing only the accidental-report specifics. Do not re-explore the codebase. -->
|
||||
|
||||
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<MarkFalsePositiveResult> {
|
||||
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<TriageNoteResult> {
|
||||
// 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).
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Add backend service functions (status flip + customer note)</name>
|
||||
<files>lib/services/remediation-service.ts, lib/services/triage-note-service.ts</files>
|
||||
<behavior>
|
||||
- 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).
|
||||
</behavior>
|
||||
<action>
|
||||
In lib/services/triage-note-service.ts, add exported `generateAndPostAccidentalReportNote(campaignId: string): Promise<TriageNoteResult>` 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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npx tsc --noEmit --pretty</automated>
|
||||
</verify>
|
||||
<done>Both functions exported and typed; markCampaignAccidentalReport returns the richer result shape; tsc passes.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Add the mark-accidental-report API route</name>
|
||||
<files>app/api/phishing/campaigns/[id]/mark-accidental-report/route.ts</files>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npx tsc --noEmit --pretty</automated>
|
||||
</verify>
|
||||
<done>Route exports POST, gated by phishing/approve, returns the richer result including notePosted/noteError; tsc passes.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: Add frontend button, dialog, and timeline case</name>
|
||||
<files>components/phishing/action-area-card.tsx, components/phishing/timeline-card.tsx</files>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npx tsc --noEmit --pretty</automated>
|
||||
</verify>
|
||||
<done>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.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 4: Mirror test coverage</name>
|
||||
<files>lib/services/remediation-service.test.ts, lib/services/triage-note-service.test.ts</files>
|
||||
<behavior>
|
||||
- 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.
|
||||
</behavior>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npx tsc --noEmit --pretty && npx vitest run lib/services/remediation-service.test.ts lib/services/triage-note-service.test.ts</automated>
|
||||
</verify>
|
||||
<done>New tests pass; both target vitest files green; tsc passes.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## 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 |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `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.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 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.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/quick/260717-v6c-add-a-mark-as-accidental-report-action-t/260717-v6c-SUMMARY.md` when done.
|
||||
</output>
|
||||
Loading…
Add table
Add a link
Reference in a new issue