chore: merge executor worktree (worktree-agent-a5d0fe500dc50fe1e)

This commit is contained in:
lorentz 2026-07-16 19:40:47 -04:00
commit 9aadbcdeea
10 changed files with 373 additions and 6 deletions

View file

@ -0,0 +1,116 @@
---
phase: 23-classification-disposition-per-client-automation-gate
plan: 01
subsystem: phishing-classifier-remediation
tags: [classifier, verdict, remediation, autotask, triage-note]
dependency-graph:
requires: []
provides:
- "USER_AWARENESS verdict + acknowledge_user action (campaign-classifier.ts)"
- "generateAndPostAcknowledgment customer-visible note writer (triage-note-service.ts)"
- "acknowledge_user real-effect post-commit wiring (remediation-service.ts)"
affects:
- "Phase 23 Plan 02 (per-client automation gate) — will call generateAndPostAcknowledgment automatically for USER_AWARENESS campaigns"
- "Phase 23 Plan 05 — references the locked USER_AWARENESS verdict string and acknowledge_user action"
- "components/phishing/classification-card.tsx, components/phishing/action-area-card.tsx — will need USER_AWARENESS/acknowledge_user UI labels (not in this plan's scope)"
tech-stack:
added: []
patterns:
- "Post-commit side effect outside postgresClient.transaction() for network I/O that must not hold a DB transaction open"
- "Per-ticket try/catch-in-loop error isolation for Autotask TicketNotes writes (reused from Phase 21)"
key-files:
created: []
modified:
- lib/services/campaign-classifier.ts
- lib/services/campaign-classifier.test.ts
- lib/services/remediation-default-params.ts
- lib/services/remediation-default-params.test.ts
- lib/services/triage-note-service.ts
- lib/services/triage-note-service.test.ts
- lib/services/triage-note-format.ts
- lib/services/remediation-service.ts
- lib/services/remediation-service.test.ts
decisions:
- "USER_AWARENESS verdict string locked exactly as specified in CONTEXT.md D-01 (Plans 02/05 reference this literal)"
- "acknowledge_user deliberately excluded from DESTRUCTIVE_ACTIONS so requires_approval computes false for it, per D-04"
- "acknowledge_user posts via noteType:18 (Client Portal Note) with publish:1 unchanged, per the live-verified Autotask field semantics in D-03"
- "The acknowledge_user real-effect Autotask call in remediation-service.ts runs strictly after the DB transaction commits and never propagates its own failure (D-04 carve-out is the only real provider call in this file)"
metrics:
duration: "~35 minutes"
completed: "2026-07-16"
---
# Phase 23 Plan 01: Classification Disposition (USER_AWARENESS) + Acknowledgment Note Writer Summary
Adds a 4th classifier verdict (`USER_AWARENESS`) for confirmed phishing-simulation-vendor
campaigns, maps it to a new non-destructive `acknowledge_user` action, writes a
customer-visible thank-you note via Autotask `noteType: 18`, and wires that note post
into the manual approve->remediate path as the sole real-effect carve-out in
`remediation-service.ts`.
## What Was Built
### Task 1: USER_AWARENESS verdict + acknowledge_user action mapping
- Extended `Verdict` union in `lib/services/campaign-classifier.ts` to
`'SPAM' | 'UNWANTED' | 'THREAT' | 'USER_AWARENESS'`.
- `mapVerdictToActions('USER_AWARENESS', ...)` returns exactly `['acknowledge_user']`.
- `acknowledge_user` is deliberately NOT added to `DESTRUCTIVE_ACTIONS``computeRequiresApproval(['acknowledge_user'])` is `false`.
- `classifyCampaign`'s `isSimulation` branch now assigns `verdict = 'USER_AWARENESS'` directly (previously fell through to `evaluateSpamVsUnwanted`, landing in `SPAM`/`UNWANTED`).
- `deriveDefaultParams('acknowledge_user', evidence)` returns `{}` (no operator-editable params, same as `no_action`).
- `TriageNoteEvidence.verdict` in `triage-note-format.ts` widened to admit `'USER_AWARENESS'` — pure type change, no formatting logic touched (verdict is only interpolated into note text there).
### Task 2: generateAndPostAcknowledgment customer-visible note writer
- New exported function `generateAndPostAcknowledgment(campaignId)` in `lib/services/triage-note-service.ts`.
- Posts one `TicketNotes` entry per linked report/ticket with `noteType: 18` ("Client Portal Note" — the actual client-visibility field, verified live against this tenant's field metadata per D-03) and `publish: 1` unchanged.
- Note body is a fixed, genuinely appreciative thank-you template with zero evidence/URL/classification interpolation (T-23-01) — deliberately NOT the evidence-dump `formatTriageNote()` template.
- Mirrors `generateAndPostTriageNote`'s per-ticket try/catch-in-loop error isolation and `{ noteText, tickets }` return shape.
### Task 3: acknowledge_user manual-path real note post wiring
- `remediateApprovedActions` in `lib/services/remediation-service.ts` now captures the transaction's `RemediateResult`, then — strictly AFTER the transaction commits — checks whether an approved `acknowledge_user` row was transitioned this pass (`alreadyCompleted === false`).
- When true, calls `generateAndPostAcknowledgment(campaignId)` exactly once inside its own `try/catch` that logs and swallows failure (the DB transition has already committed; `generateAndPostAcknowledgment` also isolates per-ticket failures internally).
- All 7 pre-existing action types (block_sender, purge_message, warn_user, reset_password, isolate_endpoint, disable_forwarding_rule, quarantine) remain simulated status-only transitions — no provider call added for any of them.
- Top-of-file D-01 doc comment updated to record this narrow D-04 carve-out.
## Verification
```
npx vitest run lib/services/campaign-classifier.test.ts lib/services/remediation-default-params.test.ts lib/services/triage-note-service.test.ts lib/services/remediation-service.test.ts
# Test Files 4 passed (4)
# Tests 77 passed (77)
npx tsc --noEmit --pretty
# (no output — passes)
```
All acceptance criteria from the plan's per-task `<acceptance_criteria>` blocks were checked directly via grep and are satisfied (USER_AWARENESS present >=2x in campaign-classifier.ts, `acknowledge_user` case in remediation-default-params.ts, `acknowledge_user` absent from DESTRUCTIVE_ACTIONS, `noteType: 18` present, `generateAndPostAcknowledgment` imported from `./triage-note-service` and guarded by `actionType === 'acknowledge_user' && alreadyCompleted === false`).
## Deviations from Plan
### Process deviation (not a functional deviation)
**1. Combined RED+GREEN into single `feat` commits for Tasks 1 and 3 (both `tdd="true"`)**
- The plan's per-task `tdd="true"` attribute calls for separate `test(...)` (RED) then `feat(...)` (GREEN) commits per the standard `<tdd_execution>` flow. I did follow the RED discipline in substance — wrote the failing tests first and ran `npx vitest run` to confirm they failed for the expected reason (undefined/wrong-verdict assertions) before writing the implementation — but committed the test+source changes together in one `feat` commit per task instead of splitting into a `test` commit followed by a `feat` commit.
- This plan's frontmatter is `type: execute` (not `type: tdd`), so the mandatory "Plan-Level TDD Gate Enforcement" gate-sequence check (which requires a `test(...)` commit before a `feat(...)` commit in git log) does not apply here — that section is explicitly scoped to plans with `type: tdd` in frontmatter. No functional risk: all RED failures were verified live in the terminal before any GREEN code was written.
- Files/commits affected: `14ed8ca` (Task 1), `6224e44` (Task 3).
No other deviations. Plan executed as written; no auto-fixes, no architectural questions, no auth gates.
## Known Stubs
None. No hardcoded empty values, placeholder text, or unwired data sources were introduced by this plan.
## Threat Flags
None beyond what the plan's own `<threat_model>` already covers (T-23-01, T-23-02) — both threats are mitigated exactly as the plan specified: the acknowledgment note body has zero evidence/URL/secret interpolation, and `acknowledge_user` is excluded from `DESTRUCTIVE_ACTIONS` with its real-effect carve-out narrowly guarded by an exact action-type string match.
## Self-Check: PASSED
- FOUND: lib/services/campaign-classifier.ts (USER_AWARENESS present)
- FOUND: lib/services/remediation-default-params.ts (acknowledge_user case present)
- FOUND: lib/services/triage-note-service.ts (generateAndPostAcknowledgment exported, noteType: 18 present)
- FOUND: lib/services/triage-note-format.ts (USER_AWARENESS in verdict union)
- FOUND: lib/services/remediation-service.ts (generateAndPostAcknowledgment imported + guarded call)
- FOUND commit 14ed8ca (feat(23-01): add USER_AWARENESS verdict + acknowledge_user action mapping)
- FOUND commit 50e2415 (feat(23-01): add generateAndPostAcknowledgment customer-visible note writer)
- FOUND commit 6224e44 (feat(23-01): wire acknowledge_user manual-path real note post into remediateApprovedActions)
- All 4 target test files pass (77/77); `npx tsc --noEmit` exits 0

View file

@ -186,6 +186,10 @@ describe('mapVerdictToActions', () => {
expect(actions).toContain('isolate_endpoint');
expect(actions).toContain('disable_forwarding_rule');
});
it('maps USER_AWARENESS to exactly acknowledge_user', () => {
expect(mapVerdictToActions('USER_AWARENESS', { clicked: 0 })).toEqual(['acknowledge_user']);
});
});
describe('computeRequiresApproval', () => {
@ -208,6 +212,10 @@ describe('computeRequiresApproval', () => {
expect(computeRequiresApproval(['no_action'])).toBe(false);
expect(computeRequiresApproval(['warn_user'])).toBe(false);
});
it('is false for acknowledge_user (USER_AWARENESS is never destructive)', () => {
expect(computeRequiresApproval(['acknowledge_user'])).toBe(false);
});
});
// =============================================================================
@ -353,6 +361,38 @@ describe('classifyCampaign', () => {
}
);
it.each([
['knowbe4 (From match)', knowbe4SimMessage],
['breach-secure-now (Return-Path match)', bsnSimMessage],
])(
'classifies a known simulation sender as USER_AWARENESS with acknowledge_user recommended and requiresApproval false (%s)',
async (_label, fixture) => {
stageQueries({
reports: [
{ id: 'report-1', title: fixture.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL },
],
messages: [toMessageRow('message-1', 'report-1', fixture)],
indicators: [],
});
getBlastRadiusMock.mockResolvedValue({
status: 'ok',
matched: 1,
delivered: 1,
held: 0,
rejected: 0,
clicked: 0,
perRecipient: [{ recipient: REPORTER_EMAIL, status: 'delivered' }],
source: 'fan-out',
});
const result = await classifyCampaign('campaign-1');
expect(result.verdict).toBe('USER_AWARENESS');
expect(result.recommendedActions).toEqual(['acknowledge_user']);
expect(result.requiresApproval).toBe(false);
}
);
it('classifies a real non-simulation signal as THREAT with destructive recommended actions (threat tier)', async () => {
stageQueries({
reports: [

View file

@ -147,7 +147,7 @@ export function computeConfidence(evidence: ConfidenceEvidenceFlags): Confidence
// D-08: Recommended-actions vocabulary + requires_approval invariant
// =============================================================================
export type Verdict = 'SPAM' | 'UNWANTED' | 'THREAT';
export type Verdict = 'SPAM' | 'UNWANTED' | 'THREAT' | 'USER_AWARENESS';
/** Always force requires_approval:true when recommended (CLASSIFY-02). */
export const DESTRUCTIVE_ACTIONS = new Set([
@ -182,6 +182,8 @@ export function mapVerdictToActions(verdict: Verdict, evidence: ActionEvidence):
}
return actions;
}
case 'USER_AWARENESS':
return ['acknowledge_user'];
}
}
@ -458,7 +460,7 @@ export async function classifyCampaign(campaignId: string): Promise<ClassifyResu
let verdict: Verdict;
const reasons: string[] = [];
if (isSimulation) {
verdict = evaluateSpamVsUnwanted(evidence);
verdict = 'USER_AWARENESS';
reasons.push(
'Sender domain matches a known phishing-simulation vendor allowlist (KnowBe4/Breach Secure Now) — THREAT tier skipped'
);

View file

@ -58,6 +58,10 @@ describe('deriveDefaultParams', () => {
});
});
it('returns {} for acknowledge_user', () => {
expect(deriveDefaultParams('acknowledge_user', filledEvidence)).toEqual({});
});
it('returns {} for an unknown/future action type', () => {
expect(deriveDefaultParams('unknown_future_type', filledEvidence)).toEqual({});
});

View file

@ -36,6 +36,8 @@ export function deriveDefaultParams(actionType: string, evidence: DefaultParamEv
return { deviceId: '' };
case 'disable_forwarding_rule':
return { userPrincipalName: evidence.requesterEmail ?? '', ruleName: '' };
case 'acknowledge_user':
return {};
default:
return {};
}

View file

@ -15,6 +15,11 @@ vi.mock('./phishing-audit', () => ({
writeAuditEvent: (...args: unknown[]) => writeAuditEventMock(...args),
}));
const generateAndPostAcknowledgmentMock = vi.fn();
vi.mock('./triage-note-service', () => ({
generateAndPostAcknowledgment: (...args: unknown[]) => generateAndPostAcknowledgmentMock(...args),
}));
// eslint-disable-next-line import/first -- imported after vi.mock hoisting
import {
approveRemediationActions,
@ -82,6 +87,8 @@ beforeEach(() => {
transactionMock.mockReset();
writeAuditEventMock.mockReset();
writeAuditEventMock.mockResolvedValue('audit-id');
generateAndPostAcknowledgmentMock.mockReset();
generateAndPostAcknowledgmentMock.mockResolvedValue({ noteText: 'thanks', tickets: [] });
clientCalls = [];
});
@ -208,6 +215,51 @@ describe('remediateApprovedActions', () => {
);
expect(writeAuditEventMock).not.toHaveBeenCalled();
});
it('calls generateAndPostAcknowledgment exactly once with the campaignId when an approved acknowledge_user row is remediated', async () => {
stage({
remediationRows: [{ id: 'action-1', action_type: 'acknowledge_user', status: 'approved' }],
});
await remediateApprovedActions('campaign-1', 'operator@example.com');
expect(generateAndPostAcknowledgmentMock).toHaveBeenCalledTimes(1);
expect(generateAndPostAcknowledgmentMock).toHaveBeenCalledWith('campaign-1');
});
it('does NOT call generateAndPostAcknowledgment for a block_sender/warn_user-only remediation', async () => {
stage({
remediationRows: [
{ id: 'action-1', action_type: 'block_sender', status: 'approved' },
{ id: 'action-2', action_type: 'warn_user', status: 'approved' },
],
});
await remediateApprovedActions('campaign-1', 'operator@example.com');
expect(generateAndPostAcknowledgmentMock).not.toHaveBeenCalled();
});
it('does NOT call generateAndPostAcknowledgment when the acknowledge_user row staged is already completed (idempotent re-run)', async () => {
stage({
remediationRows: [{ id: 'action-1', action_type: 'acknowledge_user', status: 'completed' }],
});
await remediateApprovedActions('campaign-1', 'operator@example.com');
expect(generateAndPostAcknowledgmentMock).not.toHaveBeenCalled();
});
it('does not propagate a generateAndPostAcknowledgment rejection out of remediateApprovedActions (already-committed transition)', async () => {
stage({
remediationRows: [{ id: 'action-1', action_type: 'acknowledge_user', status: 'approved' }],
});
generateAndPostAcknowledgmentMock.mockRejectedValueOnce(new Error('Autotask unavailable'));
const result = await remediateApprovedActions('campaign-1', 'operator@example.com');
expect(result.actions[0]).toMatchObject({ actionType: 'acknowledge_user', status: 'completed' });
});
});
// =============================================================================

View file

@ -13,13 +13,22 @@
* D-01: this file never has an unimplemented/no-op code path for the
* remediate step. Its "external effect" is a simulated internal transition
* (status='approved' -> 'completed') no real provider call for any of the
* 7 action types this milestone. The only explicit-failure branch is the
* 7 original action types. The only explicit-failure branch is the
* zero-approved-actions case (REMED-03) remediating with nothing approved
* always throws, never silently no-ops as a success.
*
* Phase 23 carve-out (D-04): the ONE exception is the post-commit
* `acknowledge_user` customer-note post below a non-destructive thank-you
* message, not a security action. It runs AFTER the transaction commits
* (never inside it the Autotask write is network I/O and must not hold a
* DB transaction open or risk a post-then-rollback), and its failure is
* caught/logged, never propagated (the DB transition already succeeded).
* Every other action type remains a simulated status-only transition.
*/
import { postgresClient } from './postgres-client';
import { writeAuditEvent } from './phishing-audit';
import { generateAndPostAcknowledgment } from './triage-note-service';
export class RemediationValidationError extends Error {
constructor(message: string) {
@ -171,7 +180,7 @@ export interface RemediateResult {
* success for "nothing to do".
*/
export async function remediateApprovedActions(campaignId: string, actor: string | null): Promise<RemediateResult> {
return postgresClient.transaction(async (client) => {
const result = await postgresClient.transaction(async (client) => {
const rowsRes = await client.query<RemediationActionRow>(
`SELECT id::text, action_type, status FROM remediation_actions WHERE campaign_id = $1 FOR UPDATE`,
[campaignId]
@ -204,6 +213,26 @@ export async function remediateApprovedActions(campaignId: string, actor: string
return { campaignId, actions };
});
// Phase 23 D-04 carve-out: post the real customer-visible acknowledgment
// note AFTER the transaction has committed — only when an approved
// acknowledge_user row was actually transitioned this pass (never on the
// already-completed idempotent re-run). Never held inside the DB
// transaction (network I/O), and never allowed to propagate (the DB
// transition already succeeded; generateAndPostAcknowledgment already
// isolates per-ticket failures internally).
const shouldPostAcknowledgment = result.actions.some(
(action) => action.actionType === 'acknowledge_user' && action.alreadyCompleted === false
);
if (shouldPostAcknowledgment) {
try {
await generateAndPostAcknowledgment(campaignId);
} catch (err) {
console.error('[REMEDIATE] acknowledge_user note post failed', campaignId, err);
}
}
return result;
}
// =============================================================================

View file

@ -22,7 +22,7 @@ export interface TriageNoteEvidence {
reportCount: number;
companyName?: string | null;
subject?: string | null;
verdict: 'SPAM' | 'UNWANTED' | 'THREAT' | null;
verdict: 'SPAM' | 'UNWANTED' | 'THREAT' | 'USER_AWARENESS' | null;
confidence: number | null;
summary: string | null;
reasons: string[];

View file

@ -36,7 +36,7 @@ vi.mock('./triage-note-format', async (importOriginal) => {
});
// eslint-disable-next-line import/first -- imported after vi.mock hoisting
import { generateAndPostTriageNote } from './triage-note-service';
import { generateAndPostTriageNote, generateAndPostAcknowledgment } from './triage-note-service';
interface MockRows {
reports?: unknown[];
@ -257,3 +257,69 @@ describe('generateAndPostTriageNote', () => {
expect(result.noteText).toContain('not yet classified');
});
});
describe('generateAndPostAcknowledgment', () => {
it('posts a customer-visible (noteType 18, publish 1) thank-you note to every linked ticket', async () => {
stage({
reports: [
report({ id: 'r1', ticket_id: '1001' }),
report({ id: 'r2', ticket_id: '1002' }),
],
});
const result = await generateAndPostAcknowledgment('campaign-1');
expect(createEntityMock).toHaveBeenCalledTimes(2);
for (const [entityName, data] of createEntityMock.mock.calls) {
expect(entityName).toBe('TicketNotes');
expect(data).toMatchObject({
description: result.noteText,
noteType: 18,
publish: 1,
});
expect(typeof (data as { ticketID: unknown }).ticketID).toBe('number');
}
expect(createEntityMock.mock.calls.map((c) => (c[1] as { ticketID: number }).ticketID)).toEqual([1001, 1002]);
expect(result.tickets).toEqual([
{ ticketId: '1001', posted: true },
{ ticketId: '1002', posted: true },
]);
expect(result.noteText.length).toBeGreaterThan(0);
// Appreciative, non-evidence-dump body — no evidence/URL/secret interpolation.
expect(result.noteText).not.toContain('Blast Radius');
expect(result.noteText).not.toContain('Recommended Actions');
});
it('isolates a single ticket write failure without aborting the remaining writes', async () => {
stage({
reports: [
report({ id: 'r1', ticket_id: '1001' }),
report({ id: 'r2', ticket_id: '1002' }),
report({ id: 'r3', ticket_id: '1003' }),
],
});
createEntityMock
.mockResolvedValueOnce({ id: 1 })
.mockRejectedValueOnce(new Error('Autotask API unavailable'))
.mockResolvedValueOnce({ id: 3 });
const result = await generateAndPostAcknowledgment('campaign-1');
expect(createEntityMock).toHaveBeenCalledTimes(3);
expect(result.tickets[0]).toEqual({ ticketId: '1001', posted: true });
expect(result.tickets[1]).toMatchObject({ ticketId: '1002', posted: false });
expect(result.tickets[1].error).toBe('Autotask API unavailable');
expect(result.tickets[2]).toEqual({ ticketId: '1003', posted: true });
});
it('resolves { noteText, tickets: [] } for a campaign with zero linked reports, without throwing', async () => {
stage({ reports: [] });
const result = await generateAndPostAcknowledgment('campaign-empty');
expect(result.tickets).toEqual([]);
expect(typeof result.noteText).toBe('string');
expect(result.noteText.length).toBeGreaterThan(0);
expect(createEntityMock).not.toHaveBeenCalled();
});
});

View file

@ -193,3 +193,59 @@ export async function generateAndPostTriageNote(campaignId: string): Promise<Tri
return { noteText, tickets };
}
/**
* Phase 23 D-02/D-03: posts a short, genuinely appreciative thank-you note
* to every ticket linked to a USER_AWARENESS campaign the `acknowledge_user`
* delivery action. Unlike `generateAndPostTriageNote` (an internal evidence
* dump), this note is customer-visible: `noteType: 18` ("Client Portal
* Note", verified live against this tenant's TicketNotes field metadata)
* with `publish: 1` left unchanged (an internal-staff-tier field, orthogonal
* to client visibility see triage-note-format.ts / 23-PATTERNS.md watch-out
* flag #1). The body is a FIXED template with zero evidence/URL/classification
* interpolation (T-23-01) nothing from the parsed email or classification
* reasons is ever placed in this note.
*
* Mirrors `generateAndPostTriageNote`'s per-ticket try/catch-in-loop error
* isolation (D-05) and `{ noteText, tickets }` return shape.
*/
export async function generateAndPostAcknowledgment(campaignId: string): Promise<TriageNoteResult> {
const reportsRes = await postgresClient.query<Pick<ReportRow, 'id' | 'ticket_id'>>(
`SELECT id::text, ticket_id::text AS ticket_id
FROM reports WHERE campaign_id = $1 ORDER BY created_at ASC`,
[campaignId]
);
const reports = reportsRes.rows;
const noteText = [
'Thank you for reporting this email as suspicious!',
'',
'Your quick action in flagging this message is exactly the kind of vigilance that helps keep our organization secure. We really appreciate you taking the time to report it — please keep it up.',
].join('\n');
const client = getAutotaskClient();
const tickets: TriageNotePostResult[] = [];
for (const report of reports) {
// Per-ticket try/catch is INSIDE the loop (not around it) so one
// ticket's write failure never aborts the remaining writes (D-05).
try {
await client.createEntity('TicketNotes', {
ticketID: Number(report.ticket_id),
title: 'Thank You — Suspicious Email Reported',
description: noteText,
noteType: 18, // Client Portal Note — customer-visible (D-03)
publish: 1,
});
tickets.push({ ticketId: report.ticket_id, posted: true });
} catch (err) {
console.error('[PHISHING-ACKNOWLEDGMENT] Failed to post note to ticket', report.ticket_id, err);
tickets.push({
ticketId: report.ticket_id,
posted: false,
error: err instanceof Error ? err.message : 'Unknown error',
});
}
}
return { noteText, tickets };
}