docs(260721-mmf): pre-dispatch plan for fix Mimecast blast-radius query scope
This commit is contained in:
parent
6eafaae40a
commit
f50b8a3a9b
1 changed files with 170 additions and 0 deletions
|
|
@ -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"
|
||||
---
|
||||
|
||||
<objective>
|
||||
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.
|
||||
</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
|
||||
@lib/services/mimecast-blast-radius.ts
|
||||
@lib/services/mimecast-blast-radius.test.ts
|
||||
|
||||
<interfaces>
|
||||
<!-- Confirmed contracts from mimecast-client.ts — both scoping params are OPTIONAL. -->
|
||||
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.
|
||||
|
||||
<!-- Call site in campaign-classifier.ts (line ~350) is UNCHANGED — it still passes
|
||||
recipient: primaryReport.requesterEmail. recipient stays a required input. -->
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Broaden the fan-out query to the whole tenant</name>
|
||||
<files>lib/services/mimecast-blast-radius.ts</files>
|
||||
<behavior>
|
||||
- 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.
|
||||
</behavior>
|
||||
<action>
|
||||
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).
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npx tsc --noEmit --pretty 2>&1 | grep -i "mimecast-blast-radius" || echo "no type errors in target file"</automated>
|
||||
</verify>
|
||||
<done>searchDeliveredMessages called without `to`; getHeldMessages called without `recipient`; type-check clean; merge logic unchanged and multi-recipient-safe.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: Update tests for the broadened fan-out</name>
|
||||
<files>lib/services/mimecast-blast-radius.test.ts</files>
|
||||
<behavior>
|
||||
- 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.
|
||||
</behavior>
|
||||
<action>
|
||||
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.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npm test -- mimecast-blast-radius</automated>
|
||||
</verify>
|
||||
<done>All getBlastRadius tests pass, including the updated no-recipient/no-to assertions and the new multi-recipient fan-out test.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## 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. |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
- `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).
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- 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.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/quick/260721-mmf-fix-mimecast-blast-radius-scope/260721-mmf-SUMMARY.md` when done.
|
||||
</output>
|
||||
Loading…
Add table
Add a link
Reference in a new issue