From 1e384d5c9e72ebf61d1e873c3439912871794704 Mon Sep 17 00:00:00 2001 From: lorentz Date: Thu, 16 Jul 2026 16:48:29 -0400 Subject: [PATCH] docs(260716-n46): add plan SUMMARY and deferred-items log Documents the getBlastRadius per-tenant injection/swallowed-error fixes, the campaign detail route's date-window clamp + tenant resolution, the in-scope test-hygiene deviation, and the pre-existing out-of-scope itglue-search.test.ts failure. --- .../260716-n46-SUMMARY.md | 115 ++++++++++++++++++ .../deferred-items.md | 5 + 2 files changed, 120 insertions(+) create mode 100644 .planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/260716-n46-SUMMARY.md create mode 100644 .planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/deferred-items.md diff --git a/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/260716-n46-SUMMARY.md b/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/260716-n46-SUMMARY.md new file mode 100644 index 0000000..93410b7 --- /dev/null +++ b/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/260716-n46-SUMMARY.md @@ -0,0 +1,115 @@ +--- +phase: quick-260716-n46 +plan: 01 +subsystem: api +tags: [mimecast, blast-radius, phishing, multi-tenant, ticket-699308] + +requires: + - phase: 17-mimecast-blast-radius-lookup + provides: getBlastRadius() fan-out + getMimecastClientForTenant() factory + - phase: 22-approval-ui-livelink-addressable-campaign-review-and-approve + provides: app/api/phishing/campaigns/[id]/route.ts call site +provides: + - getBlastRadius(input, options?) with optional per-tenant client injection and cache-scope namespacing + - Swallowed searchDeliveredMessages() error now surfaces as unavailable/lookup_failed instead of a false-clean ok + - Campaign detail route clamps dateWindow.end to Date.now() and resolves reports.company_id -> mimecast_tenants -> tenant-scoped client +affects: [phishing-campaign-review, mimecast-integration] + +tech-stack: + added: [] + patterns: + - "Optional client-injection param on a service function so a caller can override a module's default singleton client while preserving the no-injection code path unchanged" + - "Cache-key namespacing by an opaque cacheScope string to prevent cross-tenant collisions on shared cache infra" + +key-files: + created: [] + 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 + +key-decisions: + - "An injected tenant client bypasses the global isMimecastConfigured() gate entirely -- it carries its own credentials and is self-sufficient, matching the plan's explicit behavior spec" + - "Swallowed deliveredResult.error is converted to a thrown Error so the existing outer try/catch (which already correctly logs err.message only, never response bodies) handles it -- no new logging path introduced" + - "Tenant resolution query is scoped to enabled = true and ORDER BY id LIMIT 1, so a disabled or duplicate mimecast_tenants row for a company falls back to the global client rather than picking an unintended row" + +patterns-established: + - "Pattern: optional second options arg ({ client?, cacheScope? }) added additively to an existing exported function signature to avoid breaking BlastRadiusInput/BlastRadiusResult contract" + +requirements-completed: [BUG-1-future-end-date, BUG-2-multi-tenant-gap] + +duration: 5min +completed: 2026-07-16 +--- + +# Quick Task 260716-n46: Fix Mimecast Blast-Radius Date-Window + Multi-Tenant Bugs Summary + +**getBlastRadius now accepts an injected per-tenant MimecastClient and surfaces previously-swallowed search errors instead of a false "clean" zero-count; the campaign detail route clamps its date window to now and resolves each report's own company-specific Mimecast tenant before calling it.** + +## Performance + +- **Duration:** 5 min +- **Started:** 2026-07-16T16:42:23-04:00 +- **Completed:** 2026-07-16T16:47:19-04:00 +- **Tasks:** 3 +- **Files modified:** 4 + +## Accomplishments +- Bug 1 (false clean on fresh campaigns): `getBlastRadius` now rethrows a swallowed `searchDeliveredMessages()` error (e.g. Mimecast's `err_track_and_trace_invalid_end_date`) as `{ status: 'unavailable', reason: 'lookup_failed', error }` instead of a confident zero-count `ok` — and the route-level fix (`Math.min(createdAt + 24h, Date.now())`) prevents the future end-date from ever being sent in the first place. +- Bug 2 (multi-tenant gap, D-05): the campaign detail route now looks up `reports.company_id -> mimecast_tenants` (enabled = true) and, when a row exists, builds a tenant-scoped client via the already-implemented `getMimecastClientForTenant()` and threads it into `getBlastRadius` via a new optional `options` param — falling back to the global env-configured client when no company-specific tenant is registered. +- Cache keys are now namespaced by `cacheScope` (company_id, or `'global'`) so two tenants querying the same synthetic message-id/composite key can never read each other's cached result (T-N46-02 mitigation). + +## Task Commits + +Each task was committed atomically (TDD tasks 1-2 follow red/green discipline): + +1. **Task 1: Add optional per-tenant client injection + swallowed-error surfacing to getBlastRadius** + - `4d54aba` test(260716-n46): add failing tests for tenant client injection + swallowed-error surfacing (RED) + - `7c724cc` feat(260716-n46): support per-tenant client injection + surface swallowed delivered-search errors (GREEN) +2. **Task 2: Add getMimecastClientForTenant coverage** + - `12250c1` test(260716-n46): add coverage for getMimecastClientForTenant (coverage-only — function was already implemented per the plan's interfaces block, so tests passed immediately; no GREEN implementation step needed) +3. **Task 3: Clamp date window (Bug 1) and resolve per-company tenant (Bug 2) in the campaign detail route** + - `9951e53` fix(260716-n46): clamp blast-radius date window and resolve per-company Mimecast tenant + +**Plan metadata:** (this SUMMARY.md commit, made by the executor per worktree-mode protocol) + +## Files Created/Modified +- `lib/services/mimecast-blast-radius.ts` — `getBlastRadius(input, options?: { client?; cacheScope? })`; resolves client as `options?.client ?? (isMimecastConfigured() ? getMimecastClient() : null)`; cache key prefixed by scope; throws on `deliveredResult.error` so the outer catch converts it to `unavailable/lookup_failed`; module doc-comment (c) updated to describe per-tenant resolution as supported (no longer "known limitation"). +- `lib/services/mimecast-blast-radius.test.ts` — 3 new tests: injected tenant client bypasses `getMimecastClient()`; injected client runs fan-out even when `isMimecastConfigured()` is false; swallowed `error` field degrades to `unavailable/lookup_failed` with `setCachedData` not called. Also added `getMimecastClientMock.mockClear()` to `beforeEach` (pre-existing gap in mock hygiene that the new "not.toHaveBeenCalled()" assertions exposed). +- `lib/services/mimecast-client.test.ts` — new `describe('getMimecastClientForTenant', ...)` block: returns a client exposing the fan-out methods, builds a new instance per call, doesn't affect the cached global singleton, defaults `base_url` when omitted. Fake credentials only (`tid`/`tsecret`). +- `app/api/phishing/campaigns/[id]/route.ts` — added `company_id` to the reports SELECT/`ReportRow`/mapped-`reports` shape; added `MimecastTenantRow` interface; inside the `primaryReport` branch, queries `mimecast_tenants` by `company_id` (`enabled = true`, `ORDER BY id LIMIT 1`) and builds a tenant client via `getMimecastClientForTenant()` when a row is found, passed to `getBlastRadius` as `{ client, cacheScope: companyId }`; `dateWindow.end` now `new Date(Math.min(createdAt.getTime() + 24h, Date.now()))`. + +## Decisions Made +- Followed the plan's explicit instruction that an injected tenant client is self-sufficient and bypasses the global `isMimecastConfigured()` gate (tenant carries its own credentials). +- Did not add a redundant reports query for company_id — reused the existing `reportsRes` query per the plan's scope guard. +- Did not touch other Mimecast call sites (e.g. `mimecast-sync-service.ts`) — scope guard honored. + +## Deviations from Plan + +**1. [Test hygiene, in-scope] Added `getMimecastClientMock.mockClear()` to the existing `beforeEach` in `mimecast-blast-radius.test.ts`** +- **Found during:** Task 1 (writing the "injected client bypasses getMimecastClient" test) +- **Issue:** The pre-existing `beforeEach` never reset `getMimecastClientMock`'s call count, so a `not.toHaveBeenCalled()` assertion on it failed due to accumulated calls from earlier tests in the same file, not from the code under test. +- **Fix:** Added `getMimecastClientMock.mockClear()` alongside the other five mock resets already in that `beforeEach`. +- **Files modified:** `lib/services/mimecast-blast-radius.test.ts` +- **Verification:** All 9 tests in the file pass; existing tests' behavior/assertions unchanged. +- **Committed in:** `4d54aba` (Task 1 RED commit) + +--- + +**Total deviations:** 1 (test-infrastructure hygiene fix required to make the plan's own specified assertion pass correctly; no production-code scope creep). +**Impact on plan:** None on shipped behavior — test-only change needed for the plan's own required test assertions to be meaningful. + +## Issues Encountered +- An intermediate `git stash -u` was run in error while diagnosing an unrelated pre-existing test failure (see below) — this is a prohibited operation in worktree mode. It was caught immediately via the system's post-command file-state reminder. Recovery used only read-only `git show stash@{0}:` (never `git stash pop/apply/drop`) to retrieve the stashed file content, which was then written back with the Write tool and verified byte-identical via `diff` before proceeding. The stash entry (`stash@{0}`) was left untouched/undropped in the stash list to avoid any further stash-subcommand risk; it is a stale duplicate of already-committed work and can be safely ignored or cleaned up by the repository owner outside this workflow. +- `npx vitest run` (full suite) shows 2 pre-existing failures in `lib/services/analyzer/itglue-search.test.ts` unrelated to this plan's files — confirmed present both before and after this plan's edits, and unrelated to any file this plan touches. Logged to `deferred-items.md` in this directory per the executor's scope-boundary rule; left unfixed. + +## User Setup Required + +None — no external service configuration required. + +## Next Phase Readiness + +- Both bugs are fixed within the existing `BlastRadiusInput`/`BlastRadiusResult` contract; no consumers of `getBlastRadius` besides the campaign detail route need updates. +- The stray `stash@{0}` entry left in this worktree's shared stash list (see Issues Encountered) is a duplicate of already-committed work in this branch and is safe to drop later; not cleaned up here to avoid any `git stash` subcommand risk during this session. +- `deferred-items.md` in this directory documents the pre-existing, out-of-scope `itglue-search.test.ts` failure for future triage. diff --git a/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/deferred-items.md b/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/deferred-items.md new file mode 100644 index 0000000..431142a --- /dev/null +++ b/.planning/quick/260716-n46-fix-mimecast-blast-radius-date-window-fu/deferred-items.md @@ -0,0 +1,5 @@ +# Deferred Items — 260716-n46 + +## Pre-existing, out-of-scope test failure + +`lib/services/analyzer/itglue-search.test.ts > itglueSearch > tolerates per-call failures (configurations errors, flex still returns)` fails on a clean checkout of this branch, before any of this plan's changes. Confirmed present in the full `npx vitest run` both before and after Task 1-3 edits; none of this plan's files (`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`) touch `lib/services/analyzer/itglue-search.ts` or its test. Left unfixed per the executor scope boundary (only auto-fix issues directly caused by the current task's changes).