diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 14ded89..f9fcfed 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -654,7 +654,7 @@ Phases execute in numeric order. v1.0 (Phases 1-9.1) shipped 2026-07-10. v2.0 (P **Goal:** Add a dedicated "User Awareness" verdict for confirmed phishing-simulation-vendor (KnowBe4/Breach Secure Now) reports — today forced into the generic UNWANTED bucket despite the classifier already detecting the simulation vendor and explicitly skipping the THREAT tier — and add an admin UI gate page letting an admin choose, per Autotask company, whether the phishing pipeline's parse/classify/report-to-ticket stages run automatically (now that the previously-dead Autotask webhook is fixed) or require the existing manual Analyze/Classify/triage-note triggers. **Requirements**: CLASSDISP-01, CLASSDISP-02, CLASSDISP-03, AUTOGATE-01, AUTOGATE-02, AUTOGATE-03 **Depends on:** Phase 17, Phase 18, Phase 19, Phase 20, Phase 21, Phase 22 -**Plans:** 5/5 plans complete +**Plans:** 6 plans (5 complete, 1 gap-closure pending) Plans: **Wave 1** @@ -668,6 +668,10 @@ Plans: - [x] 23-04-PLAN.md — /admin/phishing-automation page (3-toggle company table) + admin index tile (AUTOGATE-02) - [x] 23-05-PLAN.md — Gate reader + gated parse->classify->acknowledge webhook chain (D-04 carve-out) (AUTOGATE-03) +**Gap closure** *(from 23-VERIFICATION.md, Truth #18 / CR-01)* + +- [ ] 23-06-PLAN.md — Idempotent + audited auto-post: autoPostAcknowledgment prevents duplicate customer-visible notes on repeat campaign webhooks (AUTOGATE-03) + --- *Roadmap created: 2026-05-03* *v2.0 phases added: 2026-07-10* diff --git a/.planning/STATE.md b/.planning/STATE.md index c20761d..8e40d82 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -4,14 +4,14 @@ milestone: v3.0 milestone_name: Phishing Triage Automation status: executing stopped_at: Phase 23 context gathered -last_updated: "2026-07-16T23:33:45.377Z" -last_activity: 2026-07-16 -- Phase 23 execution started +last_updated: "2026-07-17T02:53:52.042Z" +last_activity: 2026-07-17 -- Phase 23 planning complete progress: total_phases: 9 completed_phases: 8 - total_plans: 29 - completed_plans: 24 - percent: 83 + total_plans: 30 + completed_plans: 29 + percent: 89 --- # Project State @@ -27,8 +27,8 @@ See: .planning/PROJECT.md (updated 2026-07-14) Phase: 23 (classification-disposition-per-client-automation-gate) — EXECUTING Plan: 1 of 5 -Status: Executing Phase 23 -Last activity: 2026-07-16 -- Phase 23 execution started +Status: Ready to execute +Last activity: 2026-07-17 -- Phase 23 planning complete Progress: [░░░░░░░░░░] 0% @@ -118,6 +118,8 @@ None yet. - 2026-07-16 — Phase 23 planning: decision-coverage gate flagged D-01/D-02/D-03/D-04/D-05/D-06/D-08 (7 of 8 decisions) as not literally cited by `D-NN:` prefix in any plan's `must_haves`/`truths`. Overridden and proceeded — spot-checked plan 23-01's `must_haves.truths` directly and confirmed it substantively describes D-01 through D-04's content (USER_AWARENESS verdict, acknowledge_user action, noteType 18 customer-visible note, manual-path real posting) without the literal citation prefix; the plan-checker's second-pass review (after the blocker-revision cycle) independently confirmed via live source reads that all 8 decisions map to implementing tasks with no contradictions. Same citation-format gap as the Phase 18 precedent above, not an implementation gap. +- 2026-07-16 — Phase 23 gap-closure planning (`--gaps`, plan 23-06): re-running the decision-coverage gate against the phase directory re-surfaced the same 7 pre-existing citation-format misses from the initial planning override above (D-01/D-02/D-03/D-04/D-05/D-06/D-08 — all already implemented and VERIFIED per 23-VERIFICATION.md's Observable Truths #1-15). Not a new gap and not introduced by 23-06 (which only touches AUTOGATE-03/webhook idempotency and cites no CONTEXT.md decisions of its own). Overridden and proceeded for the same reason as the original override — retroactively reformatting 5 already-executed, already-shipped plans to add literal `D-NN:` citations under designated headings is out of scope for a narrow gap-closure run targeting Truth #18 only. + ### Quick Tasks Completed | # | Description | Date | Commit | Directory | diff --git a/.planning/phases/23-classification-disposition-per-client-automation-gate/23-06-PLAN.md b/.planning/phases/23-classification-disposition-per-client-automation-gate/23-06-PLAN.md new file mode 100644 index 0000000..63df647 --- /dev/null +++ b/.planning/phases/23-classification-disposition-per-client-automation-gate/23-06-PLAN.md @@ -0,0 +1,233 @@ +--- +phase: 23-classification-disposition-per-client-automation-gate +plan: 06 +type: execute +wave: 3 +depends_on: [23-05] +files_modified: + - lib/services/remediation-service.ts + - lib/services/remediation-service.test.ts + - lib/services/webhook-service.ts +autonomous: true +gap_closure: true +requirements: [AUTOGATE-03] +must_haves: + truths: + - "A repeat ticket-create webhook joining an already-acknowledged phishing campaign does NOT re-post the acknowledge_user customer-visible note (closes Truth #18 / AUTOGATE-03 to fully SATISFIED)" + - "The first auto_report pass for a USER_AWARENESS campaign still posts the acknowledge_user note exactly once" + - "The auto-post path persists a remediation_actions row (status='completed', approved_by='system:auto_report') and a remediation_completed audit_events row, so the idempotency check has a record to read AND the Action Area / Timeline UI reflect the auto-sent note (closes WR-01)" + artifacts: + - path: "lib/services/remediation-service.ts" + provides: "autoPostAcknowledgment(campaignId, actor) — idempotent, persisting auto-post orchestrator" + exports: ["autoPostAcknowledgment"] + - path: "lib/services/remediation-service.test.ts" + provides: "idempotency + persistence tests for autoPostAcknowledgment" + contains: "autoPostAcknowledgment" + - path: "lib/services/webhook-service.ts" + provides: "auto_report branch calls autoPostAcknowledgment instead of generateAndPostAcknowledgment directly" + contains: "autoPostAcknowledgment" + key_links: + - from: "lib/services/webhook-service.ts" + to: "autoPostAcknowledgment" + via: "runGatedPhishingStages auto_report branch" + pattern: "autoPostAcknowledgment\\(campaignId" + - from: "lib/services/remediation-service.ts autoPostAcknowledgment" + to: "generateAndPostAcknowledgment" + via: "post-commit call, gated by prior remediation_actions acknowledge_user existence check" + pattern: "generateAndPostAcknowledgment\\(campaignId" +--- + + +Close the single unresolved Phase 23 gap: the auto_report webhook path +(`runGatedPhishingStages` in `lib/services/webhook-service.ts`) calls +`generateAndPostAcknowledgment(campaignId)` on every ticket-create webhook that +lands in a USER_AWARENESS campaign, with no idempotency guard and no persisted +record. Because a phishing "campaign" is normally multiple employees reporting +the same email — and `groupReportIntoCampaign` returns the same `campaignId` for +merged-into-existing cases — a company with `auto_classify`+`auto_report` on +re-sends the customer-visible "thank you for reporting" note to every +already-notified ticket on every additional report. This is CR-01 (23-REVIEW.md), +confirmed unresolved by direct code inspection in 23-VERIFICATION.md (Truth #18). + +Fix strategy (mirrors the already-VERIFIED, idempotent manual path in +`remediateApprovedActions`): introduce a new idempotent, audit-persisting +`autoPostAcknowledgment(campaignId, actor)` in `remediation-service.ts` (which +has a test file and already owns the acknowledge-post + audit pattern), then wire +`runGatedPhishingStages` to call it instead of posting directly. The persisted +`remediation_actions` row is BOTH the idempotency record the next pass reads AND +the row the Action Area / Timeline UI already render from (closes WR-01, the root +cause of CR-01). + +Purpose: A company that opts into auto_report gets the acknowledgment note posted +exactly once per campaign, with a full audit/remediation trail — matching every +other phishing state-change in this codebase. +Output: `autoPostAcknowledgment` export + unit tests + rewired webhook branch. +No new migration (reuses the existing `remediation_actions` / `audit_events` +tables — deliberately avoids the "migrations don't re-run on live volumes" +caveat). + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/23-classification-disposition-per-client-automation-gate/23-VERIFICATION.md +@.planning/phases/23-classification-disposition-per-client-automation-gate/23-REVIEW.md + + + + +From lib/services/phishing-audit.ts (the ONLY audit_events writer — reuse it, do not INSERT into audit_events directly): +```typescript +export interface AuditEventInput { + campaignId: string; + actor: string | null; + eventType: string; // canonical strings include 'remediation_completed' + payload: Record; +} +// Pass the transaction client to make the audit row commit/rollback atomically with the state write. +export async function writeAuditEvent(input: AuditEventInput, client?: AuditQueryClient): Promise; +``` + +From lib/services/triage-note-service.ts (posts the customer-visible note; unchanged by this plan): +```typescript +export async function generateAndPostAcknowledgment(campaignId: string): Promise; +``` + +remediation_actions table (migrations/097_phishing_triage_schema.sql) — reuse, no schema change: +```sql +-- id UUID PK default gen_random_uuid(), campaign_id UUID FK, action_type TEXT, +-- status TEXT NOT NULL DEFAULT 'proposed', params JSONB, approved_by TEXT, +-- approved_at TIMESTAMPTZ, created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +``` + +Existing idempotent analog to replicate — remediateApprovedActions (lib/services/remediation-service.ts:182-236): +- Runs state write inside postgresClient.transaction(...) +- Writes exactly one writeAuditEvent(..., client) per real transition, INSIDE the transaction +- Calls generateAndPostAcknowledgment(campaignId) AFTER the transaction commits, in try/catch, only when a transition actually happened (never on the idempotent re-run), never propagating the note-post error + + + + + + + Task 1: Add idempotent, audit-persisting autoPostAcknowledgment to remediation-service.ts + lib/services/remediation-service.ts, lib/services/remediation-service.test.ts + + - lib/services/remediation-service.ts (read fully — replicate the exact shape of `remediateApprovedActions`, lines 182-236: transaction for state, writeAuditEvent inside it, generateAndPostAcknowledgment AFTER commit in try/catch, error caught not propagated. This is the closest existing idempotency analog — the manual path is already VERIFIED and must NOT be touched, only mirrored.) + - lib/services/phishing-audit.ts (writeAuditEvent signature + AuditEventInput; this is the ONLY place that inserts into audit_events) + - lib/services/remediation-service.test.ts lines 1-100 (mock setup: queryMock/transactionMock, writeAuditEventMock, generateAndPostAcknowledgmentMock, makeClient() SQL-branch dispatcher, MockRows interface, stage() helper — extend these, match the style exactly) + - migrations/097_phishing_triage_schema.sql lines 133-157 (remediation_actions + audit_events columns — confirm no schema change is needed) + + + - Test A (first pass posts once): no existing acknowledge_user remediation_actions row for the campaign → autoPostAcknowledgment INSERTs exactly one remediation_actions row (action_type='acknowledge_user', status='completed', approved_by='system:auto_report'), calls writeAuditEvent exactly once with eventType 'remediation_completed', calls generateAndPostAcknowledgment exactly once, returns { posted: true }. + - Test B (idempotency — THE CR-01 FIX): an acknowledge_user remediation_actions row already exists for the campaign → autoPostAcknowledgment does NOT INSERT a remediation_actions row, does NOT call writeAuditEvent, does NOT call generateAndPostAcknowledgment, returns { posted: false }. + - Test C (note-post failure is non-fatal): generateAndPostAcknowledgment rejects → autoPostAcknowledgment still resolves (does not throw) and returns { posted: true } (the DB state/audit already committed; mirrors remediateApprovedActions' post-commit try/catch). + + + Add a new exported async function `autoPostAcknowledgment(campaignId: string, actor: string | null): Promise<{ posted: boolean }>` to lib/services/remediation-service.ts. Reuse the existing imports (`postgresClient`, `writeAuditEvent`, `generateAndPostAcknowledgment`) — add no new imports. + + Implementation, mirroring remediateApprovedActions: + 1. Open `postgresClient.transaction(async (client) => { ... })`. + 2. Serialize concurrent webhooks for the same campaign: `SELECT id FROM campaigns WHERE id = $1 FOR UPDATE` with `[campaignId]`. (Locks the campaign row so two simultaneous ticket-create webhooks for the same campaign cannot both pass the existence check.) + 3. Idempotency check: `SELECT id FROM remediation_actions WHERE campaign_id = $1 AND action_type = 'acknowledge_user' LIMIT 1`. If `rows.length > 0`, return a sentinel from the transaction indicating already-posted (e.g. `{ inserted: false }`) — do NOT insert, do NOT write audit. + 4. Otherwise INSERT the completed row: `INSERT INTO remediation_actions (campaign_id, action_type, status, approved_by, approved_at) VALUES ($1, 'acknowledge_user', 'completed', $2, NOW())` with `[campaignId, actor]`, then `await writeAuditEvent({ campaignId, actor, eventType: 'remediation_completed', payload: { actionType: 'acknowledge_user', auto: true } }, client)` INSIDE the transaction. Return `{ inserted: true }`. + 5. AFTER the transaction commits: if `inserted === true`, `try { await generateAndPostAcknowledgment(campaignId); } catch (err) { console.error('[AUTO-REMEDIATE] acknowledge_user note post failed', campaignId, err); }` — never rethrow (the persisted record already committed; the manual path accepts the identical trade-off). + 6. Return `{ posted: inserted }`. + + Do NOT modify remediateApprovedActions, approveRemediationActions, or markCampaignFalsePositive — they are VERIFIED (truths #5, #16 etc.) and out of scope. + + In remediation-service.test.ts: import `autoPostAcknowledgment`, add an `existingAckRows?: unknown[]` field to the MockRows interface, and extend makeClient()'s query dispatcher with two new branches (place them BEFORE the existing generic `FROM remediation_actions` / `SELECT status FROM campaigns` branches so they match first): + - `sql.includes('SELECT id FROM campaigns') && sql.includes('FOR UPDATE')` → return `{ rows: [{ id: 'campaign-1' }], rowCount: 1 }` + - `sql.includes("action_type = 'acknowledge_user'")` → return `{ rows: rows.existingAckRows ?? [], rowCount: rows.existingAckRows?.length ?? 0 }` + The existing `INSERT INTO remediation_actions` branch already returns an id and is reused as-is. Add a `describe('autoPostAcknowledgment', ...)` block with Tests A, B, C from the behavior section, using the existing stage()/makeClient()/callsContaining() helpers and asserting on writeAuditEventMock / generateAndPostAcknowledgmentMock call counts. + + + npx vitest run lib/services/remediation-service.test.ts + + + - `lib/services/remediation-service.ts` exports `autoPostAcknowledgment` (grep: `grep -c "export async function autoPostAcknowledgment" lib/services/remediation-service.ts` returns 1). + - The idempotency SELECT is present: `grep -c "action_type = 'acknowledge_user'" lib/services/remediation-service.ts` returns >= 1. + - The campaign lock is present: `grep -c "FROM campaigns WHERE id = \$1 FOR UPDATE" lib/services/remediation-service.ts` returns >= 1. + - The auto path writes an audit row: source contains a writeAuditEvent call with eventType 'remediation_completed' inside autoPostAcknowledgment. + - `npx vitest run lib/services/remediation-service.test.ts` passes, and the new autoPostAcknowledgment describe block contains the idempotency test (Test B) asserting generateAndPostAcknowledgmentMock was NOT called when an acknowledge_user row already exists. + - `npx tsc --noEmit --pretty` produces no output. + - remediateApprovedActions / approveRemediationActions / markCampaignFalsePositive bodies are byte-for-byte unchanged (git diff shows additions only, no edits to the existing functions). + + autoPostAcknowledgment exists, is idempotent per campaign, persists a remediation_actions + remediation_completed audit_events row on first post, and is covered by a passing test proving the second call does not re-post. + + + + Task 2: Wire runGatedPhishingStages auto_report branch to autoPostAcknowledgment + lib/services/webhook-service.ts + + - lib/services/webhook-service.ts lines 1-23 (import block) and lines 509-568 (runGatedPhishingStages — the auto_report branch at lines 550-566 that currently calls generateAndPostAcknowledgment(campaignId) with no guard; this is the exact defect site from Truth #18) + - lib/services/remediation-service.ts (confirm the autoPostAcknowledgment signature added in Task 1) + + + In lib/services/webhook-service.ts: + 1. Replace the import `import { generateAndPostAcknowledgment } from './triage-note-service';` (line 22) with `import { autoPostAcknowledgment } from './remediation-service';`. Confirm no other reference to `generateAndPostAcknowledgment` remains in this file (it was only used in the auto_report branch); if none remain, the import removal leaves no dangling reference. + 2. In the auto_report branch, replace `await generateAndPostAcknowledgment(campaignId);` (line ~561) with `await autoPostAcknowledgment(campaignId, 'system:auto_report');`. Leave the surrounding `if (verdict === 'USER_AWARENESS')` guard, the verdict-backfill query, and the stage's try/catch exactly as they are — only the single post call changes. + + The `'system:auto_report'` actor sentinel is what stamps `remediation_actions.approved_by` and `audit_events.actor`, distinguishing the auto path from a human approver in the Action Area / Timeline UI (WR-01). + + Do NOT change detectPhishingTicket, groupReportIntoCampaign, the gate lookup, the auto_parse or auto_classify stages, or any other webhook behavior — truth #17 (gated pipeline wiring) is VERIFIED and must remain intact. + + + grep -c "autoPostAcknowledgment(campaignId, 'system:auto_report')" lib/services/webhook-service.ts + + + - The auto_report branch calls the new function: `grep -c "autoPostAcknowledgment(campaignId, 'system:auto_report')" lib/services/webhook-service.ts` returns 1. + - The direct unguarded post is gone: `grep -c "generateAndPostAcknowledgment" lib/services/webhook-service.ts` returns 0. + - The new import is present: `grep -c "import { autoPostAcknowledgment } from './remediation-service'" lib/services/webhook-service.ts` returns 1. + - The `if (verdict === 'USER_AWARENESS')` guard is still present (grep: `grep -c "verdict === 'USER_AWARENESS'" lib/services/webhook-service.ts` returns >= 1). + - `npx tsc --noEmit --pretty` produces no output. + - `npx vitest run lib/services/remediation-service.test.ts lib/services/phishing-automation-gate.test.ts` passes (no regression in the phase's tested surfaces). + + The webhook auto_report path posts the acknowledge_user note via the idempotent autoPostAcknowledgment; a repeat webhook for the same campaign finds the persisted remediation_actions row and skips the re-post. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| Autotask webhook -> pipeline | Untrusted external trigger; fires once per ticket-create, repeatable/redeliverable | +| pipeline -> Autotask TicketNotes (noteType 18) | Customer-visible external side effect — the asset being over-triggered | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-23-06-01 | Denial of Service / Repudiation | `runGatedPhishingStages` auto_report -> `autoPostAcknowledgment` | mitigate | Idempotency via a persisted `remediation_actions` acknowledge_user row (checked under `SELECT ... campaigns FOR UPDATE`); every auto-post writes exactly one `remediation_completed` audit_events row stamped `actor='system:auto_report'` — no un-audited customer-visible side effect, no N-fold re-post (closes CR-01/WR-01) | +| T-23-06-02 | Tampering (race) | concurrent same-campaign webhooks | mitigate | `SELECT id FROM campaigns WHERE id = $1 FOR UPDATE` serializes the check-then-insert so two simultaneous webhooks cannot both pass the existence check and double-post | +| T-23-06-03 | (accepted) redelivered Autotask webhook | `logWebhookEvent` dedup gap (IN/CR pre-existing) | accept | Out of this gap's scope (webhook-event dedup is a separate pre-existing concern, IN in 23-REVIEW.md); the per-campaign acknowledge idempotency added here already prevents the customer-visible duplicate even on redelivery | + +No package-manager installs in this plan — no package legitimacy gate required. + + + +- `npx tsc --noEmit --pretty` — clean (no output). +- `npx vitest run lib/services/remediation-service.test.ts` — passes, including the new autoPostAcknowledgment idempotency test (Test B) proving the second call does not re-post. +- `grep -c "generateAndPostAcknowledgment" lib/services/webhook-service.ts` — returns 0 (the unguarded direct call is removed). +- `grep -c "autoPostAcknowledgment(campaignId, 'system:auto_report')" lib/services/webhook-service.ts` — returns 1. +- Manual path untouched: `git diff` shows no edits to remediateApprovedActions / approveRemediationActions / markCampaignFalsePositive bodies. + + + +Truth #18 / AUTOGATE-03 moves from PARTIALLY SATISFIED to fully SATISFIED: +- First auto_report pass for a USER_AWARENESS campaign posts the acknowledge_user note exactly once and persists a remediation_actions (status='completed', approved_by='system:auto_report') + remediation_completed audit_events row. +- Every subsequent ticket-create webhook joining the same campaign reads the persisted acknowledge_user row and skips the post — zero duplicate customer-visible notes. +- No VERIFIED truth (#1-17) or its files/behavior is modified. + + + +Create `.planning/phases/23-classification-disposition-per-client-automation-gate/23-06-SUMMARY.md` when done. +