diff --git a/.planning/quick/260721-mmf-fix-mimecast-blast-radius-scope/260721-mmf-PLAN.md b/.planning/quick/260721-mmf-fix-mimecast-blast-radius-scope/260721-mmf-PLAN.md new file mode 100644 index 0000000..880a71b --- /dev/null +++ b/.planning/quick/260721-mmf-fix-mimecast-blast-radius-scope/260721-mmf-PLAN.md @@ -0,0 +1,170 @@ +--- +phase: quick-260721-mmf +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - lib/services/mimecast-blast-radius.ts + - lib/services/mimecast-blast-radius.test.ts +autonomous: true +requirements: [QUICK-MMF-01] +must_haves: + truths: + - "searchDeliveredMessages is called with from+subject+start+end only — never scoped by a single recipient (to)" + - "getHeldMessages is called with start+end only — never scoped by a single recipient" + - "A delivered-message result set containing multiple distinct `to` addresses yields multiple distinct perRecipient entries, all counted toward matched/delivered" + - "The domainsMatch(h.from, input.sender) post-filter still guards held rows against unrelated tenant traffic" + - "BlastRadiusInput.recipient remains a required field and the reporter is still represented in perRecipient" + artifacts: + - path: "lib/services/mimecast-blast-radius.ts" + provides: "Tenant-wide fan-out query scoped by sender+subject+date-window" + contains: "searchDeliveredMessages" + - path: "lib/services/mimecast-blast-radius.test.ts" + provides: "Coverage asserting no to/recipient params and multi-recipient fan-out" + key_links: + - from: "lib/services/mimecast-blast-radius.ts" + to: "mimecast-client.searchDeliveredMessages" + via: "fan-out call without `to`" + pattern: "searchDeliveredMessages" +--- + + +Fix the Mimecast blast-radius fan-out so it reflects the true blast radius across ALL +recipients of a reported phish, not just the single reporter's mailbox. + +Purpose: `getBlastRadius()` currently forwards `input.recipient` into both Mimecast +search calls (`to:` on searchDeliveredMessages, `recipient:` on getHeldMessages), +scoping the whole fan-out to one address. The counts (matched/delivered/held) and +`perRecipient` therefore only ever describe the reporter — dramatically understating +blast radius. Both params are OPTIONAL on the client; dropping them lets Mimecast +return every message matching sender+subject+date-window across the tenant. + +Output: Corrected fan-out query in `mimecast-blast-radius.ts` plus updated/extended +tests. No UI or type-shape changes; `recipient` stays a required input. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/STATE.md +@lib/services/mimecast-blast-radius.ts +@lib/services/mimecast-blast-radius.test.ts + + + +searchDeliveredMessages({ to?, from?, subject?, start?, end?, ... }): requires at least + one of to/from/subject/senderIP/url. sender+subject+date-window satisfies this. +getHeldMessages({ recipient?, start?, end?, ... }): recipient optional; no server-side + sender filter — relies on caller-side domainsMatch() post-filter. + + + + + + + + + Task 1: Broaden the fan-out query to the whole tenant + lib/services/mimecast-blast-radius.ts + + - searchDeliveredMessages is invoked with { from, subject, start, end } and NO `to` key. + - getHeldMessages is invoked with { start, end } and NO `recipient` key. + - Delivered rows carrying several distinct `to` values each become a distinct + perRecipient entry with the correct status; matched = non-rejected delivered + relevant held. + - domainsMatch(h.from, input.sender) still filters held rows before they count. + - input.recipient is still added to perRecipient as 'unknown' when no row matched it. + + + In `getBlastRadius()`, edit the `Promise.all` fan-out block (currently lines ~142-152). + Remove `to: input.recipient` from the `client.searchDeliveredMessages({...})` argument + so it passes only `from: input.sender`, `subject: input.subject`, `start: startStr`, + `end: endStr`. Remove `recipient: input.recipient` from the `client.getHeldMessages({...})` + argument so it passes only `start: startStr`, `end: endStr`. Leave everything else intact: + the `deliveredResult.error` guard, the `relevantHeldRows` domainsMatch post-filter (now + the sole scoping mechanism for held rows), the per-recipient merge Map, and the + `if (!perRecipientMap.has(input.recipient))` fallback that keeps the reporter represented. + Do NOT change `BlastRadiusInput` — `recipient` stays required per T-17-01 and is still + used to label the reporter in the merge. Update the inline comment on the searchDeliveredMessages + call (and the "Per-recipient merge" comment block) to note the query is now tenant-wide + across all recipients, scoped by sender+subject+date-window. Confirm the merge logic makes + no single-recipient assumption — the Map already keys on `row.to`, so multiple distinct + `to` values produce multiple entries with no drop/overwrite bug (held-over-delivered + override per same recipient is intended and stays). + + + npx tsc --noEmit --pretty 2>&1 | grep -i "mimecast-blast-radius" || echo "no type errors in target file" + + searchDeliveredMessages called without `to`; getHeldMessages called without `recipient`; type-check clean; merge logic unchanged and multi-recipient-safe. + + + + Task 2: Update tests for the broadened fan-out + lib/services/mimecast-blast-radius.test.ts + + - The existing "calls getHeldMessages with the SAME start/end window" test asserts + getHeldMessages receives { start, end } with NO recipient key, and asserts + searchDeliveredMessages receives NO `to` key. + - A new test drives searchDeliveredMessages with 3+ distinct `to` addresses and asserts + all appear in perRecipient and count toward matched/delivered. + + + In `mimecast-blast-radius.test.ts`, update the test titled "calls getHeldMessages with + the SAME start/end window passed to searchDeliveredMessages" (lines ~287-301): change the + getHeldMessages assertion from `toHaveBeenCalledWith({ recipient: ..., start, end })` to + assert it is called WITHOUT recipient — use `expect(getHeldMessagesMock).toHaveBeenCalledWith({ start: expectedStart, end: expectedEnd })` + (exact object, no recipient). For searchDeliveredMessages, add an assertion that the call + argument does NOT contain `to` — e.g. capture `searchDeliveredMessagesMock.mock.calls[0][0]` + and `expect(arg).not.toHaveProperty('to')`, while keeping the existing start/end/from/subject + objectContaining check. Then add a NEW test, e.g. "returns every distinct recipient from a + multi-recipient delivered result (true blast radius)": mock searchDeliveredMessages with three + delivered rows sharing BASE_INPUT.sender/subject but distinct `to` addresses + (reporter@..., coworker-a@..., coworker-b@wulfconsulting.test), all status 'Delivered'; assert + the result has delivered === 3, matched === 3, and perRecipient (via arrayContaining) includes + all three with status 'delivered'. Reuse the fixture shape from the existing merge test. + Leave the other tests (unavailable, cache hit, injected client, Bug 1 guard, unrelated-sender + held filter) unchanged. + + + npm test -- mimecast-blast-radius + + All getBlastRadius tests pass, including the updated no-recipient/no-to assertions and the new multi-recipient fan-out test. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| Pulse → Mimecast tenant API | Search query params determine which tenant messages are returned | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-17-01 | Information Disclosure | getBlastRadius fan-out | mitigate | Query still requires sender+subject+date-window (never bare date-range). Dropping `to`/`recipient` broadens scope within one campaign's sender+subject, not to unrelated traffic. `recipient` stays a required input to preserve the validation contract. | +| T-17-02 | Information Disclosure | error logging | accept | Existing catch logs `err.message` only, never response bodies — unchanged by this fix. | +| quick-mmf-SC | Tampering | npm/pip installs | accept | No new dependencies added. | + + + +- `npm test -- mimecast-blast-radius` passes. +- `npx tsc --noEmit --pretty` reports no new errors. +- `grep -n "to: input.recipient\|recipient: input.recipient" lib/services/mimecast-blast-radius.ts` returns nothing (both scoping params removed from API calls). + + + +- searchDeliveredMessages and getHeldMessages are no longer scoped to a single recipient. +- A multi-recipient delivered result produces multiple perRecipient entries and correct counts. +- No changes to BlastRadiusInput, campaign-classifier, or evidence-card.tsx. + + + +Create `.planning/quick/260721-mmf-fix-mimecast-blast-radius-scope/260721-mmf-SUMMARY.md` when done. +