docs(260721-n49): pre-dispatch plan for fix classifier Mimecast tenant resolution
This commit is contained in:
parent
3e9c7633d1
commit
f58856e103
1 changed files with 186 additions and 0 deletions
|
|
@ -0,0 +1,186 @@
|
|||
---
|
||||
phase: quick-260721-n49
|
||||
plan: 01
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- lib/services/campaign-classifier.ts
|
||||
- lib/services/campaign-classifier.test.ts
|
||||
autonomous: true
|
||||
requirements: [FIX-CLASSIFIER-TENANT]
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "gatherCampaignEvidence() resolves the reporting company's own Mimecast tenant before calling getBlastRadius() when an enabled mimecast_tenants row exists for that company"
|
||||
- "When the reporting company has no enabled mimecast_tenants row (or no companyId), getBlastRadius() is still called with no second argument — global env fallback preserved, unchanged from today"
|
||||
- "getBlastRadius receives a tenant-scoped client + cacheScope matching the companyId for companies that have their own tenant row"
|
||||
artifacts:
|
||||
- path: "lib/services/campaign-classifier.ts"
|
||||
provides: "Per-company Mimecast tenant resolution in the auto-classification evidence path"
|
||||
contains: "getMimecastClientForTenant"
|
||||
- path: "lib/services/campaign-classifier.test.ts"
|
||||
provides: "Coverage for both tenant-scoped and global-fallback blast-radius calls"
|
||||
contains: "mimecast_tenants"
|
||||
key_links:
|
||||
- from: "lib/services/campaign-classifier.ts gatherCampaignEvidence()"
|
||||
to: "mimecast_tenants table"
|
||||
via: "postgresClient.query WHERE company_id = $1 AND enabled = true"
|
||||
pattern: "mimecast_tenants[\\s\\S]*company_id = \\$1 AND enabled = true"
|
||||
- from: "lib/services/campaign-classifier.ts gatherCampaignEvidence()"
|
||||
to: "getBlastRadius second argument"
|
||||
via: "{ client, cacheScope } tenant options"
|
||||
pattern: "getBlastRadius\\([\\s\\S]*cacheScope"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Fix `gatherCampaignEvidence()` in `lib/services/campaign-classifier.ts` so the automatic campaign classifier queries the reporting company's OWN Mimecast tenant (from `mimecast_tenants`) before calling `getBlastRadius()`, instead of always falling back to the global env-configured (Wulf) Mimecast client.
|
||||
|
||||
Purpose: `classifyCampaign()` runs automatically on ticket creation/webhook and persists verdict+confidence to `classifications`. Today, any company with its own `mimecast_tenants` row has its auto-classification computed against the WRONG tenant's Mimecast data — a silent, systemic false-"clean" signal (verified live against ticket 700716 / company 29683407 "Seubert and Associates"). The correct, already-shipped, already-tested pattern lives in `app/api/phishing/campaigns/[id]/route.ts` (marked "Bug 2 (D-05)") — mirror it, do not invent a new one.
|
||||
|
||||
Output: `gatherCampaignEvidence()` threads `company_id` through and resolves a per-company tenant client exactly as the route does; tests cover both the tenant-scoped and global-fallback branches.
|
||||
</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
|
||||
|
||||
The file to fix (evidence-gathering path only):
|
||||
@lib/services/campaign-classifier.ts
|
||||
|
||||
The ALREADY-CORRECT reference to mirror (tenant-resolution block, roughly lines 256-313, "Bug 2 (D-05)"):
|
||||
@app/api/phishing/campaigns/[id]/route.ts
|
||||
|
||||
<interfaces>
|
||||
<!-- Contracts the executor needs — extracted from the codebase. No exploration required. -->
|
||||
|
||||
From lib/services/mimecast-client.ts:
|
||||
```typescript
|
||||
export function getMimecastClientForTenant(tenant: {
|
||||
client_id: string;
|
||||
client_secret: string;
|
||||
base_url?: string;
|
||||
}): MimecastClient;
|
||||
```
|
||||
|
||||
From lib/services/mimecast-blast-radius.ts:
|
||||
```typescript
|
||||
export async function getBlastRadius(
|
||||
input: { sender: string; recipient: string; subject: string; dateWindow: { start: Date; end: Date } },
|
||||
options?: { client?: MimecastClient; cacheScope?: string }
|
||||
): Promise<BlastRadiusResult>;
|
||||
// When options is omitted, falls back to the global env-configured getMimecastClient()
|
||||
// (gated by isMimecastConfigured()). This is the current classifier behavior for ALL companies.
|
||||
```
|
||||
|
||||
The exact tenant row shape + query used by the route (mirror verbatim):
|
||||
```typescript
|
||||
interface MimecastTenantRow {
|
||||
client_id: string;
|
||||
client_secret: string;
|
||||
base_url: string | null;
|
||||
}
|
||||
// SELECT client_id, client_secret, base_url
|
||||
// FROM mimecast_tenants
|
||||
// WHERE company_id = $1 AND enabled = true
|
||||
// ORDER BY id LIMIT 1
|
||||
```
|
||||
|
||||
Route's tenantOptions build (mirror verbatim in the classifier):
|
||||
```typescript
|
||||
let tenantOptions: { client: ReturnType<typeof getMimecastClientForTenant>; cacheScope: string } | undefined;
|
||||
if (primaryReport.companyId) {
|
||||
const tenantRes = await postgresClient.query<MimecastTenantRow>(/* query above */, [primaryReport.companyId]);
|
||||
const tenantRow = tenantRes.rows[0];
|
||||
if (tenantRow) {
|
||||
tenantOptions = {
|
||||
client: getMimecastClientForTenant({
|
||||
client_id: tenantRow.client_id,
|
||||
client_secret: tenantRow.client_secret,
|
||||
base_url: tenantRow.base_url ?? undefined,
|
||||
}),
|
||||
cacheScope: primaryReport.companyId,
|
||||
};
|
||||
}
|
||||
}
|
||||
```
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: Resolve per-company Mimecast tenant in gatherCampaignEvidence()</name>
|
||||
<files>lib/services/campaign-classifier.ts</files>
|
||||
<behavior>
|
||||
- When primaryReport.companyId is set AND an enabled mimecast_tenants row exists for it: getBlastRadius is called with a second argument { client, cacheScope: companyId } where client was built via getMimecastClientForTenant using that row's client_id/client_secret/base_url.
|
||||
- When primaryReport.companyId is null/empty: getBlastRadius is called with NO second argument (global fallback — unchanged from today).
|
||||
- When primaryReport.companyId is set but no enabled tenant row exists: getBlastRadius is called with NO second argument (global fallback — unchanged from today).
|
||||
- When the campaign has no linked reports: blastRadius is still synthesized as { status: 'unavailable', reason: 'not_configured' } without any Mimecast or tenant lookup (unchanged).
|
||||
</behavior>
|
||||
<action>
|
||||
Mirror the route's "Bug 2 (D-05)" tenant-resolution block. Do NOT invent a new pattern and do NOT touch computeConfidence() semantics or the blast-radius query-scope (the recipient param is already handled by task 260721-mmf — leave it).
|
||||
|
||||
1. Add `import { getMimecastClientForTenant } from './mimecast-client';` (relative sibling import — this file already imports getBlastRadius from './mimecast-blast-radius' the same way).
|
||||
2. Add a local `MimecastTenantRow` interface matching the route's shape ({ client_id: string; client_secret: string; base_url: string | null }).
|
||||
3. Extend the `ReportDbRow` interface with `company_id: string | null` and add `r.company_id::text AS company_id` to the reports SELECT list in gatherCampaignEvidence().
|
||||
4. Extend `CampaignReportSummary` with `companyId: string | null` and set it in the `reportsRes.rows.map(...)` mapping (`companyId: r.company_id`).
|
||||
5. Inside the `if (primaryReport) { ... }` branch, BEFORE the `getBlastRadius(...)` call, build `tenantOptions` exactly as the route does: if `primaryReport.companyId` is truthy, query `mimecast_tenants` (`WHERE company_id = $1 AND enabled = true ORDER BY id LIMIT 1`), and if a row exists, build the client via `getMimecastClientForTenant({ client_id, client_secret, base_url: tenantRow.base_url ?? undefined })` with `cacheScope: primaryReport.companyId`.
|
||||
6. Pass `tenantOptions` as the second argument to `getBlastRadius(...)`. Keep the existing input object (sender/recipient/subject/dateWindow) untouched. When tenantOptions is undefined, passing it as the second arg is equivalent to today's no-arg call — getBlastRadius already treats `options?.client` and `options?.cacheScope` as optional; the global fallback is preserved.
|
||||
|
||||
Judgment note on the optional shared helper (`resolveMimecastTenantOptions(companyId)`): default to an equivalent inline block in the classifier — it is the safe drop-in and matches the route's own inline style. Only extract a shared helper if it is a clean, behavior-preserving drop-in for BOTH call sites; if extracting would require editing the route's already-correct/already-tested block in any behavior-affecting way, do NOT extract — leave the route alone and keep the classifier's block inline.
|
||||
|
||||
Add a short inline comment tagging this as the same "Bug 2 (D-05)" per-company tenant resolution mirrored from the campaigns/[id] route, so the parity is discoverable.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npx tsc --noEmit --pretty 2>&1 | grep -i campaign-classifier || echo "TSC CLEAN for campaign-classifier"</automated>
|
||||
</verify>
|
||||
<done>
|
||||
gatherCampaignEvidence() selects company_id, threads it to primaryReport.companyId, and passes a tenant-scoped { client, cacheScope } to getBlastRadius() when an enabled mimecast_tenants row exists — otherwise calls with no scoping (global fallback). tsc has no new errors in this file.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Extend campaign-classifier tests for tenant resolution vs. global fallback</name>
|
||||
<files>lib/services/campaign-classifier.test.ts</files>
|
||||
<action>
|
||||
Extend the existing vitest suite (which mocks `postgresClient` via `queryMock` and `getBlastRadius` via `getBlastRadiusMock`) to cover the two new branches. Match the existing `stageQueries` / SQL-substring-routing discipline — do NOT rewrite the mocking approach.
|
||||
|
||||
1. In `stageQueries`, add a routing branch for the tenant lookup: `if (sql.includes('FROM mimecast_tenants'))` returns a staged tenant row array (default empty). Add an optional `mimecastTenants?: Array<{ client_id: string; client_secret: string; base_url: string | null }>` field to the `StagedRows` interface and route it through. Order the branch so it does not collide with the existing `FROM reports` / `FROM messages` / `FROM indicators` checks (the mimecast_tenants SQL contains none of those substrings, so any position before the final throw works).
|
||||
2. Add `company_id` to the `ReportFixtureRow` interface (`company_id: string | null`) and include it in the report fixtures used by the two new tests.
|
||||
3. New test A (tenant-scoped): stage a report with `company_id: '29683407'` and an enabled `mimecastTenants` row (`client_id`, `client_secret`, `base_url`). After `classifyCampaign('campaign-1')`, assert `getBlastRadiusMock` was called with a second argument whose `cacheScope === '29683407'` and whose `client` is defined (truthy). Assert the tenant SQL was actually issued (a queryMock call whose SQL includes `FROM mimecast_tenants` and `company_id = $1 AND enabled = true`).
|
||||
4. New test B (global fallback preserved): stage a report whose company has NO enabled tenant row — cover both sub-cases in one or two tests: (b1) `company_id: null`, and (b2) `company_id` set but `mimecastTenants: []`. After classify, assert `getBlastRadiusMock` was called with either exactly one argument or a second argument of `undefined` (i.e. no tenant scoping) — inspect `getBlastRadiusMock.mock.calls[0]` and assert `calls[0][1]` is `undefined`.
|
||||
5. Keep existing tests green — the current report fixtures omit `company_id`; ensure the added interface field is optional-compatible or updated in-place so existing `stageQueries` calls still typecheck (prefer making `company_id` present on fixtures, or `string | null` with existing rows getting `company_id: null`).
|
||||
|
||||
Do not use real Mimecast/Postgres — everything stays mocked, same as the existing file.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>npm test -- campaign-classifier 2>&1 | tail -20</automated>
|
||||
</verify>
|
||||
<done>
|
||||
New tests assert: (a) a company WITH an enabled mimecast_tenants row → getBlastRadius called with { client, cacheScope: companyId }; (b) a company with NO enabled tenant row (null companyId AND empty-tenant cases) → getBlastRadius called with no second argument. All existing campaign-classifier tests still pass.
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
- `npx tsc --noEmit --pretty` reports no new errors.
|
||||
- `npm test -- campaign-classifier` passes, including the two new branch tests.
|
||||
- Manual re-read confirms the classifier's tenant-resolution block is a behavior-parity mirror of `app/api/phishing/campaigns/[id]/route.ts` (same query, same `getMimecastClientForTenant` build, same `{ client, cacheScope }` shape), and that the route file was not changed in a behavior-affecting way (only touched if a clean shared-helper extraction was possible).
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- Auto-classification (`classifyCampaign` → `gatherCampaignEvidence`) queries the reporting company's own Mimecast tenant when one exists, matching the live blast-radius display path.
|
||||
- Global env fallback is preserved unchanged for companies without an enabled `mimecast_tenants` row and for reports without a companyId.
|
||||
- No change to confidence semantics or the already-shipped query-scope fix (260721-mmf).
|
||||
- Tests prove both branches.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/quick/260721-n49-fix-classifier-mimecast-tenant-scope/260721-n49-SUMMARY.md` when done.
|
||||
</output>
|
||||
Loading…
Add table
Add a link
Reference in a new issue