From ad84885daa77a6f18e7b469d034ecc5c474c5c73 Mon Sep 17 00:00:00 2001 From: lorentz Date: Thu, 16 Jul 2026 16:42:23 -0400 Subject: [PATCH] docs(260716-n46): pre-dispatch plan for Mimecast blast-radius fixes --- .../260716-n46-PLAN.md | 213 ++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 .planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/260716-n46-PLAN.md diff --git a/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/260716-n46-PLAN.md b/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/260716-n46-PLAN.md new file mode 100644 index 0000000..da9e8c0 --- /dev/null +++ b/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/260716-n46-PLAN.md @@ -0,0 +1,213 @@ +--- +phase: quick-260716-n46 +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - lib/services/mimecast-blast-radius.ts + - lib/services/mimecast-blast-radius.test.ts + - lib/services/mimecast-client.test.ts + - app/api/phishing/campaigns/[id]/route.ts +autonomous: true +requirements: [BUG-1-future-end-date, BUG-2-multi-tenant-gap] + +must_haves: + truths: + - "A freshly-detected campaign (primary report created <24h ago) no longer returns a false clean blast-radius reading — the future end-date is clamped to now before the Mimecast search." + - "When the underlying delivered-messages search fails silently (returns an error field instead of throwing), getBlastRadius surfaces status: 'unavailable', reason: 'lookup_failed' instead of a confident zero-count 'ok'." + - "A campaign whose reporting company has its own enabled mimecast_tenants row is queried against that company's own tenant credentials, not the global Wulf tenant." + - "A campaign whose company has no registered tenant still falls back to the global env-configured client (existing behavior preserved)." + - "Tenant client_secret / client_id are never written to logs." + artifacts: + - path: "lib/services/mimecast-blast-radius.ts" + provides: "getBlastRadius with optional per-tenant client injection + swallowed-error detection" + contains: "options?" + - path: "app/api/phishing/campaigns/[id]/route.ts" + provides: "clamped date window + per-company mimecast_tenants resolution threaded into getBlastRadius" + contains: "mimecast_tenants" + - path: "lib/services/mimecast-blast-radius.test.ts" + provides: "coverage for injected-client path + swallowed-error-degrades-to-unavailable" + - path: "lib/services/mimecast-client.test.ts" + provides: "coverage for getMimecastClientForTenant building an independent tenant-scoped instance" + key_links: + - from: "app/api/phishing/campaigns/[id]/route.ts" + to: "mimecast_tenants table" + via: "SELECT by reports.company_id where enabled = true" + pattern: "mimecast_tenants.*company_id" + - from: "app/api/phishing/campaigns/[id]/route.ts" + to: "getMimecastClientForTenant" + via: "resolved tenant client passed as getBlastRadius option" + pattern: "getMimecastClientForTenant" + - from: "app/api/phishing/campaigns/[id]/route.ts" + to: "getBlastRadius dateWindow.end" + via: "Math.min clamp against Date.now()" + pattern: "Math\\.min" +--- + + +Fix two blast-radius bugs found while live-testing ticket 699308 (Seubert & Associates) against the real Mimecast API. + +**Bug 1 (false clean on fresh campaigns):** `app/api/phishing/campaigns/[id]/route.ts` builds `dateWindow.end = createdAt + 24h`. For any campaign whose primary report is <24h old — every freshly-detected campaign, exactly when an operator opens LiveLink review — `end` is in the future. Mimecast's real `/api/message-finder/search` rejects a future `end` with `err_track_and_trace_invalid_end_date`. `searchDeliveredMessages()` swallows that error internally (`return { messages: [], error }`), so `getBlastRadius()`'s outer try/catch never fires and it returns a confident zero-count `status: 'ok'` — a false "nothing happened." + +**Bug 2 (multi-tenant gap, documented as D-05):** `getBlastRadius()` always uses the global env-configured `getMimecastClient()` (Wulf's tenant `CUSA13A95`). For a company with its own registered `mimecast_tenants` row (e.g. Seubert, `CUSA96A181`), querying the global tenant returns zero across delivered/held/threat searches; querying the company's own credentials finds the real message immediately. + +Purpose: make the freshest, most operationally-relevant campaigns return the real answer. +Output: clamped date window, swallowed-error surfacing, and per-company tenant resolution — all within the existing `BlastRadiusInput`/`BlastRadiusResult` contract. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/STATE.md +@./CLAUDE.md + + + + +From lib/services/mimecast-client.ts: +```typescript +// Per-tenant factory — ALREADY IMPLEMENTED and exported. Builds a fresh, +// independent MimecastClient (NOT the cached global). accountCode is '' — +// no x-mc-account header is sent when authenticating directly as the tenant. +export function getMimecastClientForTenant(tenant: { + client_id: string; + client_secret: string; + base_url?: string; +}): MimecastClient; + +export function getMimecastClient(): MimecastClient; // global env-configured, cached +export function isMimecastConfigured(): boolean; // checks global env vars only + +// The swallow point Bug 1 exploits — returns error field, never throws: +async searchDeliveredMessages(options): Promise<{ messages: MimecastDeliveredMessage[]; error?: string }>; +``` + +From lib/services/mimecast-blast-radius.ts (current signature to extend): +```typescript +export async function getBlastRadius(input: BlastRadiusInput): Promise; +// Internally hardcodes: const client = getMimecastClient(); +// Internally: const deliveredResult = await client.searchDeliveredMessages(...); +// -> deliveredResult.error is currently IGNORED (Bug 1 swallow). +``` + +mimecast_tenants schema (migration 062 — snake_case columns): +``` +id SERIAL, company_id BIGINT, account_code VARCHAR, account_name VARCHAR, +client_id VARCHAR NOT NULL, client_secret VARCHAR NOT NULL, +base_url VARCHAR DEFAULT 'https://api.services.mimecast.com', +enabled BOOLEAN DEFAULT true, notes TEXT, created_at, updated_at +``` + +reports table (migration 097): has `company_id BIGINT` and `company_name VARCHAR`. +The route's reports SELECT currently does NOT include company_id — Task 3 adds it. + + + + + + + Task 1: Add optional per-tenant client injection + swallowed-error surfacing to getBlastRadius + lib/services/mimecast-blast-radius.ts, lib/services/mimecast-blast-radius.test.ts + + - Given a fake tenant client passed via a new options arg, getBlastRadius calls THAT client's methods and never calls getMimecastClient(). + - Given a tenant client is provided but isMimecastConfigured() returns false (global env unset), getBlastRadius still runs the fan-out (does NOT short-circuit to not_configured) — a per-tenant client is self-sufficient. + - Given NO tenant client and isMimecastConfigured() false, still returns { status: 'unavailable', reason: 'not_configured' } (unchanged). + - Given searchDeliveredMessages resolves { messages: [], error: 'err_track_and_trace_invalid_end_date' }, getBlastRadius returns { status: 'unavailable', reason: 'lookup_failed', error: 'err_track_and_trace_invalid_end_date' } — NOT a false-clean ok (Bug 1 defense-in-depth). + - Given a cacheScope option, the cache key is namespaced by it so two tenants querying the same messageId do not collide. + - All existing getBlastRadius tests still pass unchanged (existing mocks return no `error` field, so the swallow-detection path is inert for them). + + + Extend getBlastRadius to accept an optional second parameter: `getBlastRadius(input, options?: { client?: MimecastClient; cacheScope?: string })`. Import the `MimecastClient` type from './mimecast-client'. + + Rework the configuration gate: resolve the client as `options?.client ?? (isMimecastConfigured() ? getMimecastClient() : null)`. If the resolved client is null, return `{ status: 'unavailable', reason: 'not_configured' }`. This means an injected tenant client bypasses the global env check (the tenant carries its own credentials), while the no-injection path preserves today's exact behavior. + + Namespace the cache key with the scope: prefix the existing cache key with `options?.cacheScope ?? 'global'` (e.g. `mimecast:blast-radius:${scope}:msgid:...` / `...:composite:...`). This prevents cross-tenant cache collisions on the same messageId/composite key. + + After the Promise.all fan-out, before computing counts, honor the swallowed delivered-search error: if `deliveredResult.error` is truthy, throw a new Error(deliveredResult.error) so the existing outer catch converts it to `{ status: 'unavailable', reason: 'lookup_failed', error }`. This is the Bug 1 defense-in-depth: even if a future end-date (or any other Mimecast rejection) slips through, the operator sees "unavailable" not a false zero. Do NOT log the error separately here — the existing outer catch already logs err.message only (T-17-02); do not widen logging to response bodies or tenant secrets. + + Update the module doc-comment item (c) "KNOWN LIMITATION — MULTI-TENANT GAP (D-05)": note that per-tenant resolution is now supported via the optional client injection, resolved by the caller (campaign detail route). + + Add tests to mimecast-blast-radius.test.ts following the existing vi.mock() discipline: (1) a fake tenant client object (same shape as getMimecastClientMock's return: getMessageInfo/searchDeliveredMessages/getHeldMessages/getThreatEvents) passed via options.client — assert it is used and getMimecastClientMock is NOT called; (2) tenant client provided while isMimecastConfiguredMock returns false — assert fan-out still runs (result.status === 'ok'); (3) searchDeliveredMessagesMock resolves { messages: [], error: 'err_track_and_trace_invalid_end_date' } — assert result is unavailable/lookup_failed with that error string and setCachedData NOT called. Use only mocked/fake tenant data — never real credentials. + + + npx vitest run lib/services/mimecast-blast-radius.test.ts + + All new and existing getBlastRadius tests pass; injected tenant client is exercised; swallowed delivered-search error degrades to unavailable/lookup_failed. + + + + Task 2: Add getMimecastClientForTenant coverage + lib/services/mimecast-client.test.ts + + - getMimecastClientForTenant returns a MimecastClient instance. + - The returned instance is independent of the cached global (calling it does not populate/replace the getMimecastClient() singleton; a subsequent getMimecastClient() with env set still builds its own). + - base_url defaults to the Mimecast API host when omitted from the tenant arg. + + + Add a `describe('getMimecastClientForTenant', ...)` block to the existing mimecast-client.test.ts, following that file's beforeEach env-delete + _resetMimecastClient() pattern. Import getMimecastClientForTenant alongside the existing imports. Assert: passing a fake tenant object `{ client_id: 'tid', client_secret: 'tsecret', base_url: 'https://tenant.example' }` returns a MimecastClient (instanceof, or a truthy object exposing searchDeliveredMessages). Assert it returns a NEW instance each call (two calls are not `===`), confirming it is not the cached global. Assert omitting base_url does not throw. Use only fake credentials — never real mimecast_tenants values. + + + npx vitest run lib/services/mimecast-client.test.ts + + getMimecastClientForTenant tests pass, confirming an independent tenant-scoped instance. + + + + Task 3: Clamp date window (Bug 1) and resolve per-company tenant (Bug 2) in the campaign detail route + app/api/phishing/campaigns/[id]/route.ts + + Bug 1 — clamp the end date: in the getBlastRadius call site (~line 262-265), change `end: new Date(createdAt.getTime() + 24 * 60 * 60 * 1000)` to clamp against now: `end: new Date(Math.min(createdAt.getTime() + 24 * 60 * 60 * 1000, Date.now()))`. Leave `start` unchanged. This is the primary Bug 1 fix (Task 1's swallow-detection is the backstop). + + Bug 2 — per-company tenant resolution: + 1. Add `company_id` to the reports SELECT: add `r.company_id::text` to the column list in the reportsRes query, add `company_id: string | null` to the ReportRow interface, and map it to `companyId` in the `reports.rows.map(...)` block. + 2. Inside the `if (primaryReport)` branch, before calling getBlastRadius, resolve the reporting company's tenant: if `primaryReport.companyId` is non-null, query `SELECT client_id, client_secret, base_url FROM mimecast_tenants WHERE company_id = $1 AND enabled = true ORDER BY id LIMIT 1` with `[primaryReport.companyId]`. If a row is returned, build a tenant client via `getMimecastClientForTenant({ client_id, client_secret, base_url })` (import it from '@/lib/services/mimecast-client'; note snake_case columns map directly to that function's snake_case param shape) and pass it as the getBlastRadius second arg: `getBlastRadius({...}, { client: tenantClient, cacheScope: primaryReport.companyId })`. If no enabled tenant row exists, call getBlastRadius with no options (global fallback, cacheScope defaults to 'global'). + 3. Never log client_id / client_secret. Do not add a redundant reports query — reuse the company_id added to the existing reportsRes. + + Scope guard: touch only this route and its getBlastRadius call — do not refactor other Mimecast call sites (mimecast-sync-service.ts, etc.). + + + npx tsc --noEmit --pretty + + Type check passes; dateWindow.end is clamped via Math.min against Date.now(); route resolves an enabled mimecast_tenants row by reports.company_id and threads a per-tenant client (via getMimecastClientForTenant) into getBlastRadius, falling back to the global client when none exists. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| Postgres mimecast_tenants → app | Reads per-company OAuth client_id/client_secret into process memory | +| app → Mimecast API | Authenticates as a specific tenant; credentials must not leak to logs/responses | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-N46-01 | Information Disclosure | tenant client_id/client_secret from mimecast_tenants | mitigate | Never console.log/echo tenant credentials; existing outer catch logs err.message only (T-17-02 unchanged); tests use fake credentials only | +| T-N46-02 | Information Disclosure | cache key collision across tenants | mitigate | Namespace blast-radius cache key by cacheScope (company_id) so one company cannot read another's cached result | +| T-N46-03 | Spoofing/Tampering | wrong-tenant query returning false clean | mitigate | Resolve tenant strictly by reports.company_id with enabled = true; fall back to global only when no per-company row exists | + + + +- `npx vitest run lib/services/mimecast-blast-radius.test.ts lib/services/mimecast-client.test.ts` — all pass. +- `npx tsc --noEmit --pretty` — clean (route.ts safety net, per project convention; vitest does not cover app/**). +- Manual reasoning check: a report created 1h ago now produces `end = Date.now()` (not +24h future); a company with an enabled tenant row is queried against its own credentials. + + + +- Fresh campaigns (<24h primary report) no longer produce false-clean blast-radius readings — end date clamped, and any swallowed search error surfaces as unavailable/lookup_failed. +- Companies with their own enabled mimecast_tenants row are queried against their own tenant; companies without one fall back to the global client. +- Public BlastRadiusInput/BlastRadiusResult contract unchanged (only an additive optional second param to getBlastRadius). +- No tenant credentials logged; cache keys are tenant-scoped. +- All lib tests green; tsc clean. + + + +Create `.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/260716-n46-SUMMARY.md` when done. +