From f58856e1034be1f5461883cae7e7c50437b12ae7 Mon Sep 17 00:00:00 2001 From: lorentz Date: Tue, 21 Jul 2026 16:41:14 -0400 Subject: [PATCH 01/53] docs(260721-n49): pre-dispatch plan for fix classifier Mimecast tenant resolution --- .../260721-n49-PLAN.md | 186 ++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 .planning/quick/260721-n49-fix-classifier-mimecast-tenant-scope/260721-n49-PLAN.md diff --git a/.planning/quick/260721-n49-fix-classifier-mimecast-tenant-scope/260721-n49-PLAN.md b/.planning/quick/260721-n49-fix-classifier-mimecast-tenant-scope/260721-n49-PLAN.md new file mode 100644 index 0000000..100463e --- /dev/null +++ b/.planning/quick/260721-n49-fix-classifier-mimecast-tenant-scope/260721-n49-PLAN.md @@ -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" +--- + + +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. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.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 + + + + +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; +// 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; cacheScope: string } | undefined; +if (primaryReport.companyId) { + const tenantRes = await postgresClient.query(/* 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, + }; + } +} +``` + + + + + + + Task 1: Resolve per-company Mimecast tenant in gatherCampaignEvidence() + lib/services/campaign-classifier.ts + + - 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). + + + 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. + + + npx tsc --noEmit --pretty 2>&1 | grep -i campaign-classifier || echo "TSC CLEAN for campaign-classifier" + + + 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. + + + + + Task 2: Extend campaign-classifier tests for tenant resolution vs. global fallback + lib/services/campaign-classifier.test.ts + + 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. + + + npm test -- campaign-classifier 2>&1 | tail -20 + + + 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. + + + + + + +- `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). + + + +- 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. + + + +Create `.planning/quick/260721-n49-fix-classifier-mimecast-tenant-scope/260721-n49-SUMMARY.md` when done. + From 9f75f2160c0e5f846c426d6c1f5574dedc41481a Mon Sep 17 00:00:00 2001 From: lorentz Date: Tue, 21 Jul 2026 16:44:44 -0400 Subject: [PATCH 02/53] fix(260721-n49): resolve per-company Mimecast tenant in gatherCampaignEvidence - Thread company_id through reports query and CampaignReportSummary - Mirror the route's Bug 2 (D-05) tenant-resolution block: query mimecast_tenants for an enabled row, build a tenant-scoped client via getMimecastClientForTenant, and pass { client, cacheScope } to getBlastRadius() when one exists - Preserve global env fallback unchanged when no companyId or no enabled tenant row is present --- lib/services/campaign-classifier.ts | 61 ++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 9 deletions(-) diff --git a/lib/services/campaign-classifier.ts b/lib/services/campaign-classifier.ts index 45c0b7b..3e3e5bb 100644 --- a/lib/services/campaign-classifier.ts +++ b/lib/services/campaign-classifier.ts @@ -17,6 +17,7 @@ import type { AuthResults } from './eml-parser'; import { postgresClient } from './postgres-client'; import { getBlastRadius, type BlastRadiusResult } from './mimecast-blast-radius'; +import { getMimecastClientForTenant } from './mimecast-client'; // ============================================================================= // D-06/D-07: KnowBe4 / Breach Secure Now simulation sender-domain allowlist @@ -213,6 +214,14 @@ interface ReportDbRow { title: string | null; created_at: string; requester_email: string | null; + company_id: string | null; +} + +/** Mirrors app/api/phishing/campaigns/[id]/route.ts's MimecastTenantRow shape verbatim. */ +interface MimecastTenantRow { + client_id: string; + client_secret: string; + base_url: string | null; } interface MessageDbRow { @@ -256,6 +265,7 @@ export interface CampaignReportSummary { title: string | null; createdAt: string; requesterEmail: string | null; + companyId: string | null; } export interface CampaignEvidence { @@ -283,7 +293,7 @@ export async function gatherCampaignEvidence(campaignId: string): Promise( `SELECT r.id::text AS id, r.title, r.created_at::text AS created_at, - c.email_address AS requester_email + c.email_address AS requester_email, r.company_id::text AS company_id FROM reports r LEFT JOIN contacts c ON c.id = r.requester_contact_id WHERE r.campaign_id = $1 @@ -295,6 +305,7 @@ export async function gatherCampaignEvidence(campaignId: string): Promise r.id); @@ -347,15 +358,47 @@ export async function gatherCampaignEvidence(campaignId: string): Promise i.messageId === primaryMessage?.id && i.indicatorType === 'sender' ); const createdAt = new Date(primaryReport.createdAt); - blastRadius = await getBlastRadius({ - sender: senderIndicator?.value ?? primaryMessage?.from.email ?? '', - recipient: primaryReport.requesterEmail ?? '', - subject: primaryMessage?.subject ?? primaryReport.title ?? '', - dateWindow: { - start: new Date(createdAt.getTime() - 24 * 60 * 60 * 1000), - end: new Date(createdAt.getTime() + 24 * 60 * 60 * 1000), + + // Bug 2 (D-05): resolve the reporting company's own registered Mimecast + // tenant, if one exists, and query it directly instead of the global + // env-configured (Wulf) tenant. Falls back to the global client when the + // company has no enabled mimecast_tenants row. Mirrors + // app/api/phishing/campaigns/[id]/route.ts's identical tenant-resolution + // block verbatim — same query, same client build, same options shape. + let tenantOptions: { client: ReturnType; cacheScope: string } | undefined; + if (primaryReport.companyId) { + const tenantRes = await postgresClient.query( + `SELECT client_id, client_secret, base_url + FROM mimecast_tenants + WHERE company_id = $1 AND enabled = true + ORDER BY id LIMIT 1`, + [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, + }; + } + } + + blastRadius = await getBlastRadius( + { + sender: senderIndicator?.value ?? primaryMessage?.from.email ?? '', + recipient: primaryReport.requesterEmail ?? '', + subject: primaryMessage?.subject ?? primaryReport.title ?? '', + dateWindow: { + start: new Date(createdAt.getTime() - 24 * 60 * 60 * 1000), + end: new Date(createdAt.getTime() + 24 * 60 * 60 * 1000), + }, }, - }); + tenantOptions + ); } else { // No report ever linked to this campaign — nothing to look up (research A6). blastRadius = { status: 'unavailable', reason: 'not_configured' }; From 9f12cd610a3199a02dcf44b5fe22142784b8db53 Mon Sep 17 00:00:00 2001 From: lorentz Date: Tue, 21 Jul 2026 16:45:44 -0400 Subject: [PATCH 03/53] test(260721-n49): cover tenant-scoped vs global-fallback getBlastRadius branches - Add mimecast_tenants routing branch to stageQueries and company_id to report fixtures - New test: enabled mimecast_tenants row -> getBlastRadius called with { client, cacheScope: companyId } and the tenant SQL is issued - New parameterized test: null companyId and companyId-with-no-enabled-row -> getBlastRadius called with no second argument (global fallback) --- lib/services/campaign-classifier.test.ts | 112 +++++++++++++++++++++-- 1 file changed, 102 insertions(+), 10 deletions(-) diff --git a/lib/services/campaign-classifier.test.ts b/lib/services/campaign-classifier.test.ts index 7295bb7..d54d4e4 100644 --- a/lib/services/campaign-classifier.test.ts +++ b/lib/services/campaign-classifier.test.ts @@ -247,12 +247,14 @@ interface ReportFixtureRow { title: string | null; created_at: string; requester_email: string | null; + company_id: string | null; } interface StagedRows { reports?: ReportFixtureRow[]; messages?: Array<{ id: string; report_id: string; headers: NormalizedMessage }>; indicators?: Array<{ id: string; message_id: string; indicator_type: string; value: string }>; + mimecastTenants?: Array<{ client_id: string; client_secret: string; base_url: string | null }>; } function stageQueries(rows: StagedRows) { @@ -260,6 +262,9 @@ function stageQueries(rows: StagedRows) { if (sql.includes('INSERT INTO classifications')) { return { rows: [{ id: 'classification-1', created_at: '2026-07-16T00:00:00.000Z' }], rowCount: 1 }; } + if (sql.includes('FROM mimecast_tenants')) { + return { rows: rows.mimecastTenants ?? [], rowCount: rows.mimecastTenants?.length ?? 0 }; + } if (sql.includes('FROM reports')) { return { rows: rows.reports ?? [], rowCount: rows.reports?.length ?? 0 }; } @@ -288,7 +293,7 @@ describe('classifyCampaign', () => { it('returns exactly one verdict with the full payload shape (returns exactly one verdict)', async () => { stageQueries({ reports: [ - { id: 'report-1', title: cleanSpamMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL }, + { id: 'report-1', title: cleanSpamMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL , company_id: null}, ], messages: [toMessageRow('message-1', 'report-1', cleanSpamMessage)], indicators: [], @@ -320,7 +325,7 @@ describe('classifyCampaign', () => { it('inserts exactly one append-only classifications row with no ON CONFLICT', async () => { stageQueries({ reports: [ - { id: 'report-1', title: cleanSpamMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL }, + { id: 'report-1', title: cleanSpamMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL , company_id: null}, ], messages: [toMessageRow('message-1', 'report-1', cleanSpamMessage)], indicators: [], @@ -353,7 +358,7 @@ describe('classifyCampaign', () => { async (_label, fixture) => { stageQueries({ reports: [ - { id: 'report-1', title: fixture.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL }, + { id: 'report-1', title: fixture.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL , company_id: null}, ], messages: [toMessageRow('message-1', 'report-1', fixture)], indicators: [], @@ -383,7 +388,7 @@ describe('classifyCampaign', () => { async (_label, fixture) => { stageQueries({ reports: [ - { id: 'report-1', title: fixture.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL }, + { id: 'report-1', title: fixture.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL , company_id: null}, ], messages: [toMessageRow('message-1', 'report-1', fixture)], indicators: [], @@ -410,7 +415,7 @@ describe('classifyCampaign', () => { it('classifies a real non-simulation signal as THREAT with destructive recommended actions (threat tier)', async () => { stageQueries({ reports: [ - { id: 'report-1', title: threatMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL }, + { id: 'report-1', title: threatMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL , company_id: null}, ], messages: [toMessageRow('message-1', 'report-1', threatMessage)], indicators: [], @@ -442,8 +447,8 @@ describe('classifyCampaign', () => { const sharedUrl = 'http://evil-shared.example.test/payload'; stageQueries({ reports: [ - { id: 'report-1', title: 'Invoice attached', created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL }, - { id: 'report-2', title: 'Invoice attached', created_at: '2026-07-15T02:00:00.000Z', requester_email: 'reporter2@wulfconsulting.test' }, + { id: 'report-1', title: 'Invoice attached', created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL , company_id: null}, + { id: 'report-2', title: 'Invoice attached', created_at: '2026-07-15T02:00:00.000Z', requester_email: 'reporter2@wulfconsulting.test' , company_id: null}, ], messages: [ toMessageRow('message-1', 'report-1', cleanSpamMessage), @@ -473,7 +478,7 @@ describe('classifyCampaign', () => { it('classifies a clean campaign with no indicators and no delivery/click signal as SPAM (spam vs unwanted tier)', async () => { stageQueries({ reports: [ - { id: 'report-1', title: cleanSpamMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL }, + { id: 'report-1', title: cleanSpamMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL , company_id: null}, ], messages: [toMessageRow('message-1', 'report-1', cleanSpamMessage)], indicators: [], @@ -497,7 +502,7 @@ describe('classifyCampaign', () => { it('classifies a suspicious-but-contained campaign (one url indicator, delivery contained to reporter) as UNWANTED (spam vs unwanted tier)', async () => { stageQueries({ reports: [ - { id: 'report-1', title: suspiciousUnwantedMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL }, + { id: 'report-1', title: suspiciousUnwantedMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL , company_id: null}, ], messages: [toMessageRow('message-1', 'report-1', suspiciousUnwantedMessage)], indicators: [ @@ -529,7 +534,7 @@ describe('classifyCampaign', () => { })); stageQueries({ reports: [ - { id: 'report-1', title: cleanSpamMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL }, + { id: 'report-1', title: cleanSpamMessage.subject, created_at: '2026-07-15T00:00:00.000Z', requester_email: REPORTER_EMAIL , company_id: null}, ], messages: [toMessageRow('message-1', 'report-1', cleanSpamMessage)], indicators: manyIndicators, @@ -552,4 +557,91 @@ describe('classifyCampaign', () => { expect(reason.length).toBeLessThan(300); } }); + + // =========================================================================== + // Bug 2 (D-05) parity: per-company Mimecast tenant resolution vs. global + // env fallback — mirrors app/api/phishing/campaigns/[id]/route.ts's + // already-tested tenant-resolution block (260721-n49). + // =========================================================================== + + it('resolves the reporting company\'s own Mimecast tenant and scopes getBlastRadius when an enabled mimecast_tenants row exists (tenant resolution)', async () => { + stageQueries({ + reports: [ + { + id: 'report-1', + title: cleanSpamMessage.subject, + created_at: '2026-07-15T00:00:00.000Z', + requester_email: REPORTER_EMAIL, + company_id: '29683407', + }, + ], + messages: [toMessageRow('message-1', 'report-1', cleanSpamMessage)], + indicators: [], + mimecastTenants: [ + { client_id: 'tenant-client-id', client_secret: 'tenant-client-secret', base_url: 'https://eu-api.mimecast.com' }, + ], + }); + getBlastRadiusMock.mockResolvedValue({ + status: 'ok', + matched: 1, + delivered: 0, + held: 1, + rejected: 0, + clicked: 0, + perRecipient: [], + source: 'fan-out', + }); + + await classifyCampaign('campaign-1'); + + expect(getBlastRadiusMock).toHaveBeenCalledTimes(1); + const [, tenantOptions] = getBlastRadiusMock.mock.calls[0]; + expect(tenantOptions).toBeDefined(); + expect(tenantOptions.cacheScope).toBe('29683407'); + expect(tenantOptions.client).toBeTruthy(); + + const tenantQueryCalls = queryMock.mock.calls.filter( + ([sql]) => typeof sql === 'string' && sql.includes('FROM mimecast_tenants') + ); + expect(tenantQueryCalls).toHaveLength(1); + expect(tenantQueryCalls[0][0]).toMatch(/company_id = \$1 AND enabled = true/); + }); + + it.each([ + ['no companyId on the primary report', null, [] as Array<{ client_id: string; client_secret: string; base_url: string | null }>], + ['companyId set but no enabled tenant row', '29683407', []], + ])( + 'calls getBlastRadius with no tenant scoping when %s (global fallback preserved)', + async (_label, companyId, mimecastTenants) => { + stageQueries({ + reports: [ + { + id: 'report-1', + title: cleanSpamMessage.subject, + created_at: '2026-07-15T00:00:00.000Z', + requester_email: REPORTER_EMAIL, + company_id: companyId, + }, + ], + messages: [toMessageRow('message-1', 'report-1', cleanSpamMessage)], + indicators: [], + mimecastTenants, + }); + getBlastRadiusMock.mockResolvedValue({ + status: 'ok', + matched: 1, + delivered: 0, + held: 1, + rejected: 0, + clicked: 0, + perRecipient: [], + source: 'fan-out', + }); + + await classifyCampaign('campaign-1'); + + expect(getBlastRadiusMock).toHaveBeenCalledTimes(1); + expect(getBlastRadiusMock.mock.calls[0][1]).toBeUndefined(); + } + ); }); From e9478101a36a04502309b071052a4a68c293c0b0 Mon Sep 17 00:00:00 2001 From: lorentz Date: Tue, 21 Jul 2026 16:48:11 -0400 Subject: [PATCH 04/53] docs(quick-260721-n49): fix classifier to resolve per-company Mimecast tenant instead of the global tenant --- .planning/STATE.md | 3 +- .../260721-n49-SUMMARY.md | 102 ++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 .planning/quick/260721-n49-fix-classifier-mimecast-tenant-scope/260721-n49-SUMMARY.md diff --git a/.planning/STATE.md b/.planning/STATE.md index 6d7d0b3..3752204 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -28,7 +28,7 @@ See: .planning/PROJECT.md (updated 2026-07-14) Phase: Milestone v3.0 complete Plan: — Status: Awaiting next milestone -Last activity: 2026-07-21 — Completed quick task 260721-mmf: fix Mimecast blast-radius query scope (tenant-wide fan-out instead of single-recipient) +Last activity: 2026-07-21 — Completed quick task 260721-n49: fix classifier to resolve per-company Mimecast tenant instead of always using the global tenant ## Performance Metrics @@ -132,6 +132,7 @@ None yet. | 260718-9qg | Add self-contained `QBO_INTEGRATION_HANDOFF.md` documenting Pulse's QuickBooks Online OAuth2 flow, token storage/refresh, sandbox/production API base URLs, and gotchas (deletion-diffing, CSRF state gap, NEXTAUTH_URL legacy var) for a new app's team | 2026-07-18 | ea8a36b | [260718-9qg-create-a-quickbooks-online-integration-h](./quick/260718-9qg-create-a-quickbooks-online-integration-h/) | | 260721-fy8 | Fix missing `mimecast-sync`/`qbo` scheduler dispatch branches (both silently fell through to a generic Autotask full sync) and reschedule `mimecast-sync` off the 2am 3-way cron collision with `qbo-sync-2am` and `veeam-full` | 2026-07-21 | db7db98 | [260721-fy8-fix-mimecast-and-qbo-sync-scheduler-disp](./quick/260721-fy8-fix-mimecast-and-qbo-sync-scheduler-disp/) | | 260721-mmf | Fix Mimecast blast-radius query scope — dropped the single-recipient `to`/`recipient` filter from `searchDeliveredMessages`/`getHeldMessages` so the fan-out returns every delivered/held message across the whole tenant for a campaign's sender+subject+date-window, not just whether it reached the original reporter's mailbox | 2026-07-21 | 534eda3 | [260721-mmf-fix-mimecast-blast-radius-scope](./quick/260721-mmf-fix-mimecast-blast-radius-scope/) | +| 260721-n49 | Fix `gatherCampaignEvidence()` (used by auto-classification on ticket creation) to resolve the reporting company's own `mimecast_tenants` row before calling `getBlastRadius()`, mirroring the campaign-detail route's existing per-tenant resolution — previously it always used the global env-configured (Wulf) tenant, silently returning wrong-tenant (often empty) blast-radius data for any company with its own registered Mimecast tenant | 2026-07-21 | 9f12cd6 | [260721-n49-fix-classifier-mimecast-tenant-scope](./quick/260721-n49-fix-classifier-mimecast-tenant-scope/) | ## Deferred Items diff --git a/.planning/quick/260721-n49-fix-classifier-mimecast-tenant-scope/260721-n49-SUMMARY.md b/.planning/quick/260721-n49-fix-classifier-mimecast-tenant-scope/260721-n49-SUMMARY.md new file mode 100644 index 0000000..f157703 --- /dev/null +++ b/.planning/quick/260721-n49-fix-classifier-mimecast-tenant-scope/260721-n49-SUMMARY.md @@ -0,0 +1,102 @@ +--- +phase: quick-260721-n49 +plan: 01 +subsystem: phishing-triage +tags: [mimecast, classifier, multi-tenant, campaign-classifier, blast-radius] + +requires: + - phase: 260721-mmf + provides: "getBlastRadius() tenant-wide fan-out fix (query-scope, unrelated to this tenant-resolution fix)" +provides: + - "gatherCampaignEvidence() resolves the reporting company's own Mimecast tenant before calling getBlastRadius(), mirroring the already-shipped route-level fix" +affects: [phishing-triage, classifications, mimecast] + +tech-stack: + added: [] + patterns: + - "Per-company Mimecast tenant resolution: SELECT client_id/client_secret/base_url FROM mimecast_tenants WHERE company_id = $1 AND enabled = true ORDER BY id LIMIT 1, then getMimecastClientForTenant() + { client, cacheScope: companyId } passed as getBlastRadius()'s second argument. Falls back to no second argument (global env client) when absent." + +key-files: + created: [] + modified: + - lib/services/campaign-classifier.ts + - lib/services/campaign-classifier.test.ts + +key-decisions: + - "Mirrored app/api/phishing/campaigns/[id]/route.ts's 'Bug 2 (D-05)' tenant-resolution block inline in the classifier rather than extracting a shared helper — the plan's judgment note allowed extraction only if it was a clean, behavior-preserving drop-in for both call sites; the route's block was already correct/tested, so leaving it untouched and duplicating the small inline block in the classifier was the safer choice." + +requirements-completed: [FIX-CLASSIFIER-TENANT] + +duration: 12min +completed: 2026-07-21 +--- + +# Quick Task 260721-n49: Fix classifier Mimecast tenant scope Summary + +**Auto-classification (`classifyCampaign`/`gatherCampaignEvidence`) now queries the reporting company's own registered Mimecast tenant instead of always falling back to the global env-configured (Wulf) tenant.** + +## Performance + +- **Duration:** 12 min +- **Tasks:** 2 completed +- **Files modified:** 2 + +## Accomplishments + +- `gatherCampaignEvidence()` now threads `company_id` from the `reports` table through `CampaignReportSummary.companyId`. +- Before calling `getBlastRadius()`, the classifier queries `mimecast_tenants` for an enabled row matching the primary report's `companyId` and, if found, builds a tenant-scoped client via `getMimecastClientForTenant()` and passes `{ client, cacheScope: companyId }` as the second argument — exactly mirroring the already-shipped, already-tested block in `app/api/phishing/campaigns/[id]/route.ts` ("Bug 2 (D-05)"). +- When no `companyId` is present, or no enabled tenant row exists for it, `getBlastRadius()` is still called with no second argument — the global env-configured Mimecast client fallback is unchanged from before this fix. +- Root cause was verified live against ticket 700716 / company 29683407 ("Seubert and Associates"): before this fix, that company's auto-classification was being computed against the wrong (Wulf) tenant's Mimecast data, producing a silent false-"clean" signal. + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Resolve per-company Mimecast tenant in gatherCampaignEvidence()** - `9f75f21` (fix) +2. **Task 2: Extend campaign-classifier tests for tenant resolution vs. global fallback** - `9f12cd6` (test) + +_Note: `git log --oneline -3` from HEAD: `9f12cd6` (test), `9f75f21` (fix), `f58856e` (docs: pre-dispatch plan)._ + +## Files Created/Modified + +- `lib/services/campaign-classifier.ts` - Added `MimecastTenantRow` interface, `company_id` on `ReportDbRow`/`CampaignReportSummary`, and the per-company tenant-resolution block (query + `getMimecastClientForTenant` + `{ client, cacheScope }`) before the existing `getBlastRadius()` call in `gatherCampaignEvidence()`. +- `lib/services/campaign-classifier.test.ts` - Added a `mimecast_tenants` routing branch to `stageQueries`/`StagedRows`, `company_id` on `ReportFixtureRow` (and all existing report fixtures, set to `null` where not under test), plus two new test cases: one asserting tenant-scoped `{ client, cacheScope }` when an enabled tenant row exists, and a parameterized test asserting no second argument (`undefined`) when `companyId` is null or has no enabled tenant row. + +## Decisions Made + +- Kept the tenant-resolution block inline in the classifier (not extracted into a shared helper) per the plan's explicit judgment note — the route's existing block is already correct and tested, and extracting a shared helper would have required editing it, which the plan disallowed unless the extraction was a clean behavior-preserving drop-in for both call sites. + +## Deviations from Plan + +None - plan executed exactly as written. + +## Issues Encountered + +None. + +## User Setup Required + +None - no external service configuration required. + +## Verification + +- `npx vitest run lib/services/campaign-classifier.test.ts` — 49/49 tests passed (including the 2 new tenant-resolution/global-fallback tests; all 47 pre-existing tests remained green). +- `npx tsc --noEmit --pretty` — clean, no errors. +- Manual re-read confirmed the classifier's new tenant-resolution block is a line-for-line behavior mirror of `app/api/phishing/campaigns/[id]/route.ts`'s "Bug 2 (D-05)" block (same query, same `getMimecastClientForTenant` build, same `{ client, cacheScope }` shape); the route file itself was not modified. + +## Next Phase Readiness + +- Fix is self-contained to the evidence-gathering path of the campaign classifier; no follow-up work required. +- Companies with their own `mimecast_tenants` row (e.g. Seubert and Associates / 29683407) will now get correct tenant-scoped blast-radius evidence on the next auto-classification run (webhook-triggered ticket creation, or any future manual `classifyCampaign()` invocation) — no backfill of historical `classifications` rows was in scope for this fix. + +--- +*Phase: quick-260721-n49* +*Completed: 2026-07-21* + +## Self-Check: PASSED + +- FOUND: lib/services/campaign-classifier.ts +- FOUND: lib/services/campaign-classifier.test.ts +- FOUND: .planning/quick/260721-n49-fix-classifier-mimecast-tenant-scope/260721-n49-SUMMARY.md +- FOUND commit: 9f75f21 (fix task) +- FOUND commit: 9f12cd6 (test task) From 5f308f836fb110eb7af7264d965c412c062c992c Mon Sep 17 00:00:00 2001 From: lorentz Date: Wed, 22 Jul 2026 12:34:33 -0400 Subject: [PATCH 05/53] fix(phishing-recipient-seubert): scope campaign grouping Tier 3 to company, not reporting contact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tier 3 is the only tier automatic (webhook-triggered) grouping ever reaches, since grouping runs before message parsing. It was scoped to reports.requester_contact_id, so the same campaign reported by different employees at the same company never consolidated into one campaign — each report's evidence/blast-radius view silently under-reported the campaign's true recipients. Co-Authored-By: Claude Sonnet 5 --- .../campaign-grouping-service.test.ts | 43 ++++++++-- lib/services/campaign-grouping-service.ts | 84 +++++++++++-------- 2 files changed, 85 insertions(+), 42 deletions(-) diff --git a/lib/services/campaign-grouping-service.test.ts b/lib/services/campaign-grouping-service.test.ts index dcb868c..5e007d5 100644 --- a/lib/services/campaign-grouping-service.test.ts +++ b/lib/services/campaign-grouping-service.test.ts @@ -83,7 +83,7 @@ function makeClient(rows: MockRows) { query: vi.fn(async (sql: string, params?: unknown[]) => { clientCalls.push({ sql, params: params ?? [] }); - if (sql.includes('requester_contact_id, company_id, created_at')) { + if (sql.includes('SELECT title, company_id, created_at')) { return { rows: rows.ownReport ?? [], rowCount: rows.ownReport?.length ?? 0 }; } if (sql.includes('FROM messages') && sql.includes('WHERE report_id = $1')) { @@ -101,7 +101,7 @@ function makeClient(rows: MockRows) { if (sql.includes('message_id = ANY')) { return { rows: rows.candidateIndicators ?? [], rowCount: rows.candidateIndicators?.length ?? 0 }; } - if (sql.includes('BETWEEN $4::timestamptz')) { + if (sql.includes('BETWEEN $3::timestamptz') && sql.includes('FROM reports r')) { return { rows: rows.tier3 ?? [], rowCount: rows.tier3?.length ?? 0 }; } if (sql.includes('SELECT campaign_key')) { @@ -140,7 +140,6 @@ describe('groupReportIntoCampaign', () => { const REPORT_ROW = { title: 'Re: Invoice Alert', - requester_contact_id: 5, company_id: 10, created_at: '2026-07-15T10:00:00Z', }; @@ -173,6 +172,36 @@ describe('groupReportIntoCampaign', () => { expect(callsContaining('INSERT INTO campaigns')).toHaveLength(0); }); + it('bug fix (phishing-recipient-seubert): Tier 3 matches a sibling report from a DIFFERENT reporting contact at the same company — company-wide, not contact-scoped', async () => { + // Regression coverage for the fix: Tier 3 previously required + // r.requester_contact_id = $1, which meant two different employees at + // the same company reporting the identical campaign (same normalized + // subject, same company, within 24h) could never be consolidated into + // one campaign. The query itself must not filter or join on any + // contact/requester column, and must scope only by company_id. + stage({ + ownReport: [REPORT_ROW], // this report's own reporter is irrelevant to the match now + ownMessage: [], + tier3: [{ campaign_id: 'shared-campaign', title: 'Invoice Alert' }], + }); + + const result = await groupReportIntoCampaign('report-different-reporter'); + + expect(result).toEqual({ + campaignId: 'shared-campaign', + groupMethod: 'sender_subject_client', + created: false, + }); + + const tier3Calls = clientCalls.filter( + (c) => c.sql.includes('FROM reports r') && c.sql.includes('r.company_id = $1') + ); + expect(tier3Calls).toHaveLength(1); + expect(tier3Calls[0].sql).not.toContain('requester_contact_id'); + expect(tier3Calls[0].sql).not.toContain('JOIN contacts'); + expect(tier3Calls[0].params).toEqual([10, 'report-different-reporter', REPORT_ROW.created_at]); + }); + it('creates exactly one new campaign when no tier matches anything', async () => { stage({ ownReport: [REPORT_ROW], @@ -192,7 +221,7 @@ describe('groupReportIntoCampaign', () => { const insertCalls = callsContaining('INSERT INTO campaigns'); expect(insertCalls).toHaveLength(1); expect(insertCalls[0].params).toEqual([ - 'sender_subject_client:5:invoice alert:10', + 'sender_subject_client:invoice alert:10', 'sender_subject_client', ]); expect(callsContaining('UPDATE campaigns')).toHaveLength(0); @@ -407,7 +436,7 @@ describe('groupReportIntoCampaign', () => { ownMessage: [], tier3: [], ownCampaign: [ - { campaign_key: 'sender_subject_client:5:invoice alert:10', group_method: 'sender_subject_client' }, + { campaign_key: 'sender_subject_client:invoice alert:10', group_method: 'sender_subject_client' }, ], }); @@ -450,7 +479,7 @@ describe('groupReportIntoCampaign', () => { ownMessage: [], tier3: [], ownCampaign: [ - { campaign_key: 'sender_subject_client:5:old subject:10', group_method: 'sender_subject_client' }, + { campaign_key: 'sender_subject_client:old subject:10', group_method: 'sender_subject_client' }, ], insertCampaign: [{ id: 'diverged-new-campaign' }], }); @@ -532,7 +561,7 @@ describe('groupReportIntoCampaign', () => { ownMessage: [], tier3: [], ownCampaign: [ - { campaign_key: 'sender_subject_client:5:old subject:10', group_method: 'sender_subject_client' }, + { campaign_key: 'sender_subject_client:old subject:10', group_method: 'sender_subject_client' }, ], insertCampaign: [{ id: 'diverged-new-campaign-h' }], }); diff --git a/lib/services/campaign-grouping-service.ts b/lib/services/campaign-grouping-service.ts index e11a59d..7b0e205 100644 --- a/lib/services/campaign-grouping-service.ts +++ b/lib/services/campaign-grouping-service.ts @@ -10,14 +10,32 @@ * logic between callers, mirroring `phishing-detector.ts`'s shared-core * architecture. * - * D-07 limitation (load-bearing, stated explicitly): `parseAndStoreMessage` - * (the only writer of `messages`/`indicators` rows — Phase 16) is not wired - * into the automatic webhook/cron path this phase. That means the automatic - * path only ever has `reports`/`contacts` data available, so Tier 1 - * (Message-ID) and Tier 2 (attachment-hash/URL-domain) can only ever match - * for a report that has already been through an explicit `/analyze` call at - * least once. Until then, automatic grouping effectively only reaches - * Tier 3 (sender + normalized subject + client + 24h window). + * D-07 limitation (load-bearing, stated explicitly): `groupReportIntoCampaign` + * always runs BEFORE `parseAndStoreMessage` on the automatic webhook path + * (see `webhook-service.ts`'s `triggerPhishingDetection()` — grouping happens + * first, parsing happens afterward inside `runGatedPhishingStages()`). That + * means at grouping time the CURRENT report never has its own `messages`/ + * `indicators` row yet, so Tier 1 (Message-ID) and Tier 2 (attachment-hash/ + * URL-domain) — both of which require the report's OWN signal to search + * for candidates — can never match on the automatic path's one-and-only + * grouping call (subsequent webhook events short-circuit via + * `skipIfAlreadyGrouped`). Automatic grouping therefore always resolves via + * Tier 3 (normalized subject + company + 24h window). + * + * Bug fix (debug session phishing-recipient-seubert): Tier 3 previously + * scoped its match to `reports.requester_contact_id` — i.e. it only ever + * merged reports filed by the SAME reporting employee. Since Tier 3 is the + * only tier automatic grouping can ever reach (see D-07 above), that meant + * the same phishing campaign sent to and reported by MULTIPLE different + * employees at the same company could never be consolidated into one + * campaign — each recipient's report silently became its own single-report + * campaign, so any single ticket's evidence/blast-radius view under-reported + * the campaign's true recipient list. Tier 3 now scopes to company + subject + * only (no contact/requester restriction), matching its `sender_subject_ + * client` name's original intent of grouping the same external campaign + * across a company, independent of who reported it. (`sender` isn't + * literally available yet at this point — see D-07 — so "client" scoping is + * company-wide, deliberately wider than a single reporter.) */ import type { PoolClient } from 'pg'; @@ -64,7 +82,6 @@ export interface GroupReportResult { interface OwnReportRow { title: string | null; - requester_contact_id: number | null; company_id: number | null; created_at: string; campaign_id: string | null; @@ -109,14 +126,15 @@ function computeTier2Key( return `attachment_or_url:${keyParts.join(',')}:${normalizedSubject}:${senderValue}`; } -/** Builds a `sender_subject_client:...` key from sender + subject + company (Tier 3). */ -function computeTier3Key( - requesterContactId: number | null, - normalizedSubject: string, - companyId: number | null -): string | null { - if (!requesterContactId || !normalizedSubject || !companyId) return null; - return `sender_subject_client:${requesterContactId}:${normalizedSubject}:${companyId}`; +/** + * Builds a `sender_subject_client:...` key from normalized subject + company + * (Tier 3) — deliberately company-wide, NOT scoped to a single reporting + * contact, so the same campaign reported by different employees at the same + * company still consolidates into one campaign (see file-level bug-fix note). + */ +function computeTier3Key(normalizedSubject: string, companyId: number | null): string | null { + if (!normalizedSubject || !companyId) return null; + return `sender_subject_client:${normalizedSubject}:${companyId}`; } /** @@ -174,7 +192,7 @@ export async function groupReportIntoCampaign( return await postgresClient.transaction(async (client) => { const ownReportRes = await client.query( - `SELECT title, requester_contact_id, company_id, created_at, campaign_id::text AS campaign_id + `SELECT title, company_id, created_at, campaign_id::text AS campaign_id FROM reports WHERE id = $1`, [reportId] @@ -311,24 +329,24 @@ export async function groupReportIntoCampaign( } // --------------------------------------------------------------------- - // Tier 3: sender + normalizeSubject(title) + client + 24h window - // (D-02). Joins reports.requester_contact_id -> contacts (Pitfall 5 — - // NOT `contact_id`). Self-exclusion (`r.id != $3`) required for the - // same reason as Tiers 1-2. + // Tier 3: normalizeSubject(title) + company + 24h window (D-02, fixed + // per file-level bug-fix note). Company-wide — deliberately NOT scoped + // to `reports.requester_contact_id` — so the same campaign reported by + // different employees at the same company still consolidates into one + // campaign. Self-exclusion (`r.id != $2`) required for the same reason + // as Tiers 1-2. // --------------------------------------------------------------------- - if (!matchCampaignId && ownReport.requester_contact_id && ownReport.company_id && normalizedSubject) { + if (!matchCampaignId && ownReport.company_id && normalizedSubject) { const tier3 = await client.query<{ campaign_id: string; title: string | null }>( `SELECT r.campaign_id::text AS campaign_id, r.title FROM reports r - JOIN contacts c ON c.id = r.requester_contact_id - WHERE r.requester_contact_id = $1 - AND r.company_id = $2 + WHERE r.company_id = $1 AND r.campaign_id IS NOT NULL - AND r.id != $3 - AND r.created_at BETWEEN $4::timestamptz - INTERVAL '24 hours' - AND $4::timestamptz + INTERVAL '24 hours' + AND r.id != $2 + AND r.created_at BETWEEN $3::timestamptz - INTERVAL '24 hours' + AND $3::timestamptz + INTERVAL '24 hours' ORDER BY r.created_at ASC`, - [ownReport.requester_contact_id, ownReport.company_id, reportId, ownReport.created_at] + [ownReport.company_id, reportId, ownReport.created_at] ); const match = tier3.rows.find((r) => normalizeSubject(r.title) === normalizedSubject); if (match) { @@ -349,11 +367,7 @@ export async function groupReportIntoCampaign( normalizedSubject, ownSenderValue ); - const tier3Key = computeTier3Key( - ownReport.requester_contact_id, - normalizedSubject, - ownReport.company_id - ); + const tier3Key = computeTier3Key(normalizedSubject, ownReport.company_id); const currentKeys = [tier1Key, tier2Key, tier3Key, `report:${reportId}`].filter( (k): k is string => k !== null ); From 0805e387e3847c05a21de24f8c31fba3a66e6252 Mon Sep 17 00:00:00 2001 From: lorentz Date: Wed, 5 Aug 2026 18:31:06 -0400 Subject: [PATCH 06/53] docs(24): capture phase context --- .../24-CONTEXT.md | 214 ++++++++++++++++++ .../24-DISCUSSION-LOG.md | 148 ++++++++++++ 2 files changed, 362 insertions(+) create mode 100644 .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-CONTEXT.md create mode 100644 .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-DISCUSSION-LOG.md diff --git a/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-CONTEXT.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-CONTEXT.md new file mode 100644 index 0000000..e578283 --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-CONTEXT.md @@ -0,0 +1,214 @@ +# Phase 24: AWS Route 53 DNS Sync - Context + +**Gathered:** 2026-08-05 +**Status:** Ready for planning + + +## Phase Boundary + +Sync DNS hosted zones/records from AWS Route 53 into Postgres on a schedule, +support full CRUD back to Route 53 from Pulse for common record types, track +record-level change history over time (both Pulse-initiated and externally +detected drift), log every sync and CRUD operation for audit (including +failures), and integrate into the existing per-system sync section +(scheduler, `/admin/sync` UI, health checks) alongside Autotask/Datto +RMM/Veeam/PAX8. AWS credentials are resolved via BWS (Bitwarden Secrets +Manager), not plaintext env vars. + + + + +## Implementation Decisions + +### CRUD Scope & Guardrails +- **D-01:** Writable record types are the common set only — A, AAAA, CNAME, + MX, TXT, SRV. NS and SOA are excluded from the write path (zone-delegation + records; editing them risks breaking the zone). +- **D-02:** Records only, not zones. Pulse can create/update/delete records + within hosted zones that already exist in Route 53. Hosted zone + creation/deletion (domain onboarding/decommissioning) stays outside Pulse + (AWS console or infra-as-code). +- **D-03:** Destructive record operations (update/delete) execute + immediately — no phishing-style staged/two-step approval gate. Every + change is logged with actor/timestamp/before/after so mistakes are + traceable after the fact, not blocked beforehand. +- **D-04:** CRUD is gated at `requireAdmin()` (admin + super-admin) — the + same bar as other write-capable admin surfaces in Pulse, not a stricter + super-admin-only gate. + +### Change Tracking & Audit Schema +- **D-05:** Dedicated Route 53 tables, not a reuse of the phishing + pipeline's `audit_events` table. New migration introduces + `route53_zones` / `route53_records` / `route53_record_history` / + `route53_audit_log` (naming for planner/researcher to finalize) — mirrors + how Veeam and Datto RMM each own their tables rather than sharing a + cross-domain audit schema. +- **D-06:** Change history is written both for Pulse-initiated CRUD and for + sync-detected drift (a record changed outside Pulse, e.g. directly in the + AWS console). Each history row is tagged with a `source` field: + `pulse_crud` | `sync_detected_drift`, so the query "did someone change + this outside Pulse?" is answerable. +- **D-07:** Failed AWS API attempts (rate-limited, invalid record, AWS-side + error) are also logged in the audit trail — attempted before/after + + error message + `status: failed` — not just successful writes. +- **D-08:** Retention is unbounded — no purge job. Matches existing Pulse + convention; no audit/history table in this codebase currently has an + automatic retention/purge mechanism. + +### Admin UI & Sync Integration +- **D-09:** New tile on `/admin/sync` (same list as Veeam/Datto RMM/PAX8) + plus a dedicated `/admin/sync/route53` detail page for zones, records, + and history — the existing per-integration pattern, not folded into an + existing page. +- **D-10:** `/admin/integrations` disable toggle for `route53` is + display-only (suppresses health-check display; scheduler/sync/CRUD keep + working underneath) — the default behavior per CLAUDE.md. Route 53 is + **not** a second PAX8-style exception that blocks sync/writes when + disabled. +- **D-11:** Sync cadence is incremental + periodic full — more frequent + incremental checks plus a daily full reconciliation, rather than a single + daily full sync. Trade-off (more API calls against Route 53 rate limits + for better real-time drift detection) accepted knowingly. +- **D-12:** The health-check row for Route 53 goes beyond the generic + auth-check + last-sync-age pattern used by other integrations — it also + includes a DNS-specific delegation check: compare each hosted zone's + Route-53-authoritative NS records against a **live public DNS lookup** + (e.g. Node's `dns` module or a DoH resolver) for that domain, flagging a + mismatch as degraded health. No manually-maintained "expected NS" field — + the live lookup is itself the source of truth to diff against. + +### Claude's Discretion +- **Credentials & AWS account scope** — not discussed interactively (user + deliberately skipped this topic, treating it as already settled). Codebase + scouting found uncommitted infrastructure already in place: + `docker-entrypoint.sh` (new, untracked) plus diffs to `Dockerfile` and + `docker-compose.yml` that install the `bws` CLI and wrap the app's start + command as `bws run --project-id "$BWS_PROJECT_ID" -- node server.js` + when `BWS_ACCESS_TOKEN` is set, falling back to a plain `node server.js` + otherwise. **This means Bitwarden secret injection happens at the + container-entrypoint layer, before the Node process starts** — the app + itself never calls a BWS SDK; AWS credentials simply appear as normal + `process.env` values by the time `getRoute53Client()`-style code runs. + Researcher/planner should: (1) follow the exact existing + `lib/services/-factory.ts` + `isConfigured()` pattern used by + every other integration, reading credentials from `process.env`; (2) + confirm the actual env var names with the user (e.g. `AWS_ACCESS_KEY_ID` + / `AWS_SECRET_ACCESS_KEY` / `AWS_REGION`, vs a `ROUTE53_*`-prefixed + variant) before finalizing the factory — this wasn't locked in + discussion; (3) add a `ROUTE53_*` (or `AWS_*`) row to CLAUDE.md's + integration env-prefix table once confirmed. +- **AWS account scope** — not discussed. Default assumption for planning + purposes is a single AWS account holding all client hosted zones (the + common MSP pattern), not per-client AWS accounts/cross-account IAM roles. + Flag during research if this assumption looks wrong once the actual AWS + setup is inspected. +- **Exact record-change diff granularity** (whole-recordset replace vs + individual value diffing) — left to researcher/planner, informed by how + the AWS SDK's `ChangeResourceRecordSets` API actually models a record + update. +- **Table/column naming inside the dedicated Route 53 schema** — D-05 locks + "dedicated tables," not literal names; researcher/planner should follow + existing migration conventions (`snake_case`, audit columns + `created_at`/`updated_at`/`synced_at`/`is_deleted`/`deleted_at`). + + + + +## Canonical References + +**Downstream agents MUST read these before planning or implementing.** + +No external specs, ADRs, or docs reference AWS Route 53 anywhere in this +repo — ROADMAP.md's Phase 24 section has no "Canonical refs:" field, and no +seed file covers this integration. Requirements are fully captured in the +decisions above and in ROADMAP.md's Phase 24 Success Criteria. + +### Closest existing analogs (not canonical docs, but the patterns to follow) +- `lib/services/veeam-factory.ts`, `lib/services/veeam-sync-service.ts` — + factory + sync-service pattern for a full external integration with + scheduler + admin UI + health check +- `lib/services/datto-rmm-factory.ts`, `lib/services/datto-rmm-sync-service.ts` + — second reference implementation of the same pattern +- `lib/services/sync-scheduler.ts` — `sync_type` union, `ScheduleConfig`, + `defaultSchedules` array to extend for `route53-incremental`/`route53-full` +- `lib/services/integration-health.ts` — health-check aggregator to extend +- `app/admin/sync/page.tsx` — integration tile list (`id`/`category`/ + `product`/`description`/`href`/`logo`/`color`) to extend with a `route53` + entry +- `migrations/081_*.sql` — `integration_settings` table backing the + `/admin/integrations` disable toggle (D-10) + + + + +## Existing Code Insights + +### Reusable Assets +- `components/admin/DataTable.tsx` — for the zones/records list on the new + `/admin/sync/route53` page +- `components/admin/DetailModal.tsx` — for record detail/history drill-down + (formatted/raw tab pattern already established) +- `app/admin/sync/page.tsx` tile array — extend with a `route53` entry + following the exact shape used for `veeam`/`datto-rmm`/`pax8` + +### Established Patterns +- `lib/services/-factory.ts` + `isConfigured()` — credential + lazy-load + config-check pattern every integration follows; Route 53 + client should match this exactly (see Claude's Discretion above) +- `lib/services/-sync-service.ts` — incremental/full sync against + `lastTrackedModificationDateTime`-style cursors, batched via + `postgresClient.bulkUpsert()` +- `lib/services/sync-scheduler.ts` — node-cron singleton; new sync types + added to the `sync_type` string union and `defaultSchedules` seed array +- `lib/services/integration-health.ts` — per-integration health check, + cached ~5 minutes, read by both `/admin/integrations` and `/admin/sync` +- `migrations/081_*.sql` `integration_settings` — disable-toggle table with + `disabled_by`/`disabled_at`/`disabled_reason` audit columns; per + CLAUDE.md, PAX8 is currently the only integration where disabling also + blocks sync/writes — Route 53 explicitly does **not** join that list + (D-10) + +### Integration Points +- `lib/services/sync-scheduler.ts` — add `route53-incremental` / + `route53-full` to the `sync_type` union and `defaultSchedules` +- `lib/services/integration-health.ts` — add a `route53` health entry + (auth check + last-sync age + the D-12 NS-delegation check) +- `app/admin/sync/page.tsx` — add `route53` tile; new + `app/admin/sync/route53/page.tsx` detail page +- New `app/api/route53/*` routes for zone/record read + CRUD + history +- New numbered migration (planner/researcher to confirm the current highest + migration number — duplicates exist at 002/004/009, alphabetical apply + order per CLAUDE.md) for the dedicated Route 53 tables (D-05) +- `docker-entrypoint.sh` / `Dockerfile` / `docker-compose.yml` — already + modified (uncommitted) to wire BWS secret injection; see Claude's + Discretion above + + + + +## Specific Ideas + +- Health check must include a **live public DNS lookup** for NS delegation + mismatch (D-12) — not a manually-maintained "expected NS" field. +- Audit trail must capture failed AWS API attempts, not just successful + writes (D-07). +- Change history must distinguish Pulse-initiated changes from + sync-detected external drift via a `source` tag (D-06). + + + + +## Deferred Ideas + +None — discussion stayed within phase scope. The "Credentials & AWS scope" +gray area was deliberately not discussed interactively (user judged it +already settled by the existing BWS/docker-entrypoint infrastructure) — see +Claude's Discretion above, not treated as out-of-scope or deferred to a +future phase. + + + +--- + +*Phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud* +*Context gathered: 2026-08-05* diff --git a/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-DISCUSSION-LOG.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-DISCUSSION-LOG.md new file mode 100644 index 0000000..b6ee4a6 --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-DISCUSSION-LOG.md @@ -0,0 +1,148 @@ +# Phase 24: AWS Route 53 DNS Sync - Discussion Log + +> **Audit trail only.** Do not use as input to planning, research, or execution agents. +> Decisions are captured in CONTEXT.md — this log preserves the alternatives considered. + +**Date:** 2026-08-05 +**Phase:** 24-AWS Route 53 DNS Sync +**Areas discussed:** CRUD scope & guardrails, Change tracking & audit schema, Admin UI & sync integration + +--- + +## CRUD Scope & Guardrails + +| Option | Description | Selected | +|--------|-------------|----------| +| Common types only | A, AAAA, CNAME, MX, TXT, SRV — the record types an MSP actually edits day-to-day. NS/SOA excluded. | ✓ | +| All record types | Includes NS/SOA — full parity with the AWS console, higher risk. | | +| Read-only for now | Ship sync + audit log first; defer writable types to a follow-up. | | + +**User's choice:** Common types only +**Notes:** — + +| Option | Description | Selected | +|--------|-------------|----------| +| Records only | Zones provisioned/decommissioned outside Pulse; Pulse only CRUDs records within existing zones. | ✓ | +| Zones + records | Pulse can also create/delete whole hosted zones. | | + +**User's choice:** Records only +**Notes:** — + +| Option | Description | Selected | +|--------|-------------|----------| +| Immediate, with full audit trail | Executes right away like other admin CRUD; every change logged with actor/before/after. | ✓ | +| Confirmation dialog only | Client-side "Are you sure?" modal, no server-side gate. | | +| Two-step approval gate | Mirrors phishing remediation — staged, second admin approves. | | + +**User's choice:** Immediate, with full audit trail +**Notes:** — + +| Option | Description | Selected | +|--------|-------------|----------| +| Admin + super-admin | Same bar as other write-capable admin surfaces (`requireAdmin()`). | ✓ | +| Super-admin only | Tighter gate given DNS-change blast radius. | | +| Any authenticated user | No role restriction. | | + +**User's choice:** Admin + super-admin +**Notes:** — + +--- + +## Change Tracking & Audit Schema + +| Option | Description | Selected | +|--------|-------------|----------| +| Dedicated Route 53 tables | New `route53_*` tables scoped to this integration, mirrors Veeam/Datto RMM. | ✓ | +| Reuse phishing's audit_events table | Shared cross-domain audit table. | | + +**User's choice:** Dedicated Route 53 tables +**Notes:** — + +| Option | Description | Selected | +|--------|-------------|----------| +| Log both, tagged by source | History rows tagged `pulse_crud` / `sync_detected_drift`. | ✓ | +| Only log Pulse-initiated CRUD | Sync silently overwrites current-state tables, no drift history. | | + +**User's choice:** Log both, tagged by source +**Notes:** — + +| Option | Description | Selected | +|--------|-------------|----------| +| Log failed attempts too | Audit row captures attempted before/after + error + status=failed. | ✓ | +| Only log successful changes | Failed API calls just console.error'd. | | + +**User's choice:** Log failed attempts too +**Notes:** — + +| Option | Description | Selected | +|--------|-------------|----------| +| Unbounded, no purge | Matches existing Pulse convention — no history/audit table currently purges. | ✓ | +| Time-boxed retention | Scheduled purge job for rows older than N months/years. | | + +**User's choice:** Unbounded, no purge +**Notes:** — + +--- + +## Admin UI & Sync Integration + +| Option | Description | Selected | +|--------|-------------|----------| +| New tile + detail page | `/admin/sync` tile + dedicated `/admin/sync/route53` page — existing Veeam/Datto RMM/PAX8 pattern. | ✓ | +| Fold into an existing page | Attach DNS management to an existing admin section. | | + +**User's choice:** New tile + detail page +**Notes:** — + +| Option | Description | Selected | +|--------|-------------|----------| +| Display-only toggle | Disabling suppresses health-check display only; matches CLAUDE.md default for every integration except PAX8. | ✓ | +| Blocks sync + CRUD like PAX8 | Second exception alongside PAX8 — disabling also skips scheduler and 403s writes. | | + +**User's choice:** Display-only toggle +**Notes:** — + +| Option | Description | Selected | +|--------|-------------|----------| +| Daily full sync | One scheduled job/day, matches pax8-daily/engagement-daily cadence. | | +| Incremental + periodic full | More frequent incremental checks plus daily full reconciliation. | ✓ | + +**User's choice:** Incremental + periodic full +**Notes:** Accepted trade-off of more API calls against Route 53 rate limits for better real-time drift detection. + +| Option | Description | Selected | +|--------|-------------|----------| +| Generic pattern | Same integration-health.ts shape — auth check + last-sync age. | | +| Add DNS-specific checks | Also flag degraded health on NS delegation mismatch vs registrar. | ✓ | + +**User's choice:** Add DNS-specific checks +**Notes:** Follow-up clarified how to determine "expected" NS — see next row. + +| Option | Description | Selected | +|--------|-------------|----------| +| Live public DNS lookup | Query a public resolver (DoH or Node `dns` module) for the domain's NS records, diff against Route 53's authoritative set. | ✓ | +| Manual expected-NS field | Admin manually records expected NS per zone; check diffs against stored value. | | + +**User's choice:** Live public DNS lookup +**Notes:** No manually-maintained field — the live lookup is itself the source of truth. + +--- + +## Claude's Discretion + +- **Credentials & AWS account scope** — user deliberately did not select this + topic for discussion (treated as already settled by existing uncommitted + `docker-entrypoint.sh`/`Dockerfile`/`docker-compose.yml` BWS wiring found + during codebase scouting). Left to researcher/planner to confirm exact + env var names and follow the existing factory pattern. See CONTEXT.md + Claude's Discretion section for the full writeup. +- AWS account scope (single account vs per-client) — not discussed; + defaulted to single-account assumption. +- Exact record-change diff granularity — left to researcher/planner, + informed by the AWS SDK's `ChangeResourceRecordSets` shape. +- Table/column naming inside the dedicated Route 53 schema — locked concept + ("dedicated tables"), not literal names. + +## Deferred Ideas + +None — discussion stayed within phase scope. From e289f2d24f4d798ed5034b292d25a453b65d19dd Mon Sep 17 00:00:00 2001 From: lorentz Date: Wed, 5 Aug 2026 18:31:11 -0400 Subject: [PATCH 07/53] docs(state): record phase 24 context session --- .planning/STATE.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/.planning/STATE.md b/.planning/STATE.md index 3752204..0238c5c 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,9 +3,9 @@ gsd_state_version: 1.0 milestone: v3.0 milestone_name: Phishing Triage Automation status: Awaiting next milestone -stopped_at: Phase 23 context gathered -last_updated: "2026-07-18T15:20:36.300Z" -last_activity: "2026-07-18 — Completed quick task 260718-9qg: QBO integration handoff document" +stopped_at: Phase 24 context gathered +last_updated: "2026-08-05T22:31:11.421Z" +last_activity: "2026-07-21 — Completed quick task 260721-n49: fix classifier to resolve per-company Mimecast tenant instead of always using the global tenant" progress: total_phases: 9 completed_phases: 9 @@ -107,6 +107,8 @@ Recent decisions affecting current work: 699415) — the classifier already detects the simulation vendor and skips the THREAT tier, but has no distinct outcome to reflect it. +- Phase 24 edited: edited fields: title, goal, success_criteria (tidied up phase.add output; AWS Route 53 DNS sync via BWS credentials, full CRUD + audit logging, integrated into existing sync infra) + ### Pending Todos None yet. @@ -163,9 +165,9 @@ Items acknowledged and deferred at v3.0 milestone close on 2026-07-17 (pre-fligh ## Session Continuity -Last session: 2026-07-18T15:20:36.295Z -Stopped at: Phase 23 context gathered -Resume file: None +Last session: 2026-08-05T22:31:11.414Z +Stopped at: Phase 24 context gathered +Resume file: .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-CONTEXT.md ## Operator Next Steps From 6ad27fcbb6ac699fdd27484df41766aa9862c531 Mon Sep 17 00:00:00 2001 From: lorentz Date: Wed, 5 Aug 2026 18:46:43 -0400 Subject: [PATCH 08/53] docs(phase-24): add validation strategy --- .../24-VALIDATION.md | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-VALIDATION.md diff --git a/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-VALIDATION.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-VALIDATION.md new file mode 100644 index 0000000..ad2baf0 --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-VALIDATION.md @@ -0,0 +1,80 @@ +--- +phase: 24 +slug: aws-route-53-dns-sync-track-changes-crud-operations-full-aud +status: draft +nyquist_compliant: false +wave_0_complete: false +created: 2026-08-05 +--- + +# Phase 24 — Validation Strategy + +> Per-phase validation contract for feedback sampling during execution. + +--- + +## Test Infrastructure + +| Property | Value | +|----------|-------| +| **Framework** | vitest 4.1.5 | +| **Config file** | `vitest.config.ts` (`include: ['lib/**/*.test.ts']`, `environment: 'node'`) | +| **Quick run command** | `npx vitest run lib/services/route53-sync-service.test.ts` (once created) | +| **Full suite command** | `npm test` | +| **Estimated runtime** | ~10 seconds (small existing suite) | + +--- + +## Sampling Rate + +- **After every task commit:** Run `npx vitest run ` +- **After every plan wave:** Run `npm test` +- **Before `/gsd:verify-work`:** Full suite must be green +- **Max feedback latency:** 15 seconds + +--- + +## Per-Task Verification Map + +| Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status | +|---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------| +| TBD | TBD | TBD | SC-3/SC-4 (audit + history correctness) | — | `route53_record_history` gets a row with correct `source` tag (pulse_crud vs sync_detected_drift) for CRUD vs. drift | unit | `npx vitest run lib/services/route53-sync-service.test.ts` | ❌ W0 | ⬜ pending | +| TBD | TBD | TBD | D-12 (NS delegation health check) | — | NS-list normalization (case, trailing dot) and mismatch detection classify correctly | unit | `npx vitest run lib/services/integration-health.test.ts` | ❌ W0 | ⬜ pending | +| TBD | TBD | TBD | D-01 (write-type allowlist) | T-24-01 | Server-side rejection of NS/SOA record writes with 400, before constructing `ChangeResourceRecordSetsCommand` | unit | `npx vitest run lib/services/route53-sync-service.test.ts` | ❌ W0 | ⬜ pending | +| TBD | TBD | TBD | SC-2 (CRUD write-back auth gating) | T-24-02 | `requireAdmin()` returns 401/403 for non-admin sessions on write routes | manual / smoke | none automated — matches existing project convention (see `22-VERIFICATION.md` precedent) | ❌ — manual by convention | ⬜ pending | + +*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky* + +*Plan/Wave/Task IDs are TBD — the planner has not yet assigned plan numbers. Update this table's Task ID/Plan/Wave columns once `PLAN.md` files exist (or leave for `gsd-plan-checker` to cross-reference against actual plan task IDs).* + +--- + +## Wave 0 Requirements + +- [ ] `lib/services/route53-sync-service.test.ts` — record-set key derivation, before/after diff classification (CREATE/UPSERT/DELETE → history row shape), NS-list normalization +- [ ] `lib/services/integration-health.test.ts` — first test file for any integration's health check; cover the new Route 53 NS-delegation mismatch detection logic (no existing precedent — this is a new test file, not an extension) +- [ ] `lib/services/route53-factory.test.ts` — `isRoute53Configured()` true/false branches (optional, low priority; mirrors that `veeam-factory.ts` has no test file today, but this is the first AWS-credential-shaped factory in the codebase) +- [ ] No framework install needed — vitest is already configured project-wide + +--- + +## Manual-Only Verifications + +| Behavior | Requirement | Why Manual | Test Instructions | +|----------|-------------|------------|--------------------| +| Write-route auth gating (401/403 for non-admin) | SC-2 / D-04 | `requireAdmin()`/`requirePermission()` are Better Auth session-dependent; this codebase has no precedent for testing route auth gating in isolation — historically verified by manual click-through (see `22-VERIFICATION.md`) | Sign in as a `user`-role account, hit `POST /api/route53/records` directly (curl or browser devtools) with a valid record payload, confirm 403. Repeat as `admin` and confirm success. | +| Live AWS write-back round-trip (create/update/delete a real record) | SC-2 | Cannot be safely automated against a real AWS account/hosted zone in CI; requires a real Route 53 zone and live credentials | Using a disposable test record in a real (or sandbox) hosted zone: create via Pulse UI, confirm it appears in the AWS console within the `GetChange` poll window; update it; delete it; confirm `route53_audit_log` has 3 rows with correct before/after values. | +| DNS-egress-in-production assumption (D-12 health check) | D-12 | Whether outbound UDP/53 to public resolvers is permitted from the production container network is unverifiable from the repo (flagged as Open Question 3 in `24-RESEARCH.md`) | Deploy to production/staging, trigger the Route 53 health check, confirm the live NS lookup resolves rather than timing out. If it times out, the health check needs a fallback (DoH) per the research's open question. | + +--- + +## Validation Sign-Off + +- [ ] All tasks have `` verify or Wave 0 dependencies +- [ ] Sampling continuity: no 3 consecutive tasks without automated verify +- [ ] Wave 0 covers all MISSING references +- [ ] No watch-mode flags +- [ ] Feedback latency < 15s +- [ ] `nyquist_compliant: true` set in frontmatter + +**Approval:** pending From 15e52280b51ef7b2c5e21007daf0dd8123487932 Mon Sep 17 00:00:00 2001 From: lorentz Date: Wed, 5 Aug 2026 19:09:15 -0400 Subject: [PATCH 09/53] =?UTF-8?q?docs(24):=20create=20phase=20plan=20?= =?UTF-8?q?=E2=80=94=207=20plans=20in=204=20waves=20for=20AWS=20Route=2053?= =?UTF-8?q?=20DNS=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .planning/ROADMAP.md | 24 ++ .../24-01-PLAN.md | 381 ++++++++++++++++ .../24-02-PLAN.md | 371 ++++++++++++++++ .../24-03-PLAN.md | 295 +++++++++++++ .../24-04-PLAN.md | 300 +++++++++++++ .../24-05-PLAN.md | 407 ++++++++++++++++++ .../24-06-PLAN.md | 254 +++++++++++ .../24-07-PLAN.md | 368 ++++++++++++++++ .../24-VALIDATION.md | 89 ++-- 9 files changed, 2462 insertions(+), 27 deletions(-) create mode 100644 .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-PLAN.md create mode 100644 .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-02-PLAN.md create mode 100644 .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-03-PLAN.md create mode 100644 .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-04-PLAN.md create mode 100644 .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-05-PLAN.md create mode 100644 .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-06-PLAN.md create mode 100644 .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-07-PLAN.md diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 4aaa2fa..a58beb8 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -677,6 +677,30 @@ Phases execute in numeric order. v1.0 (Phases 1-9.1) shipped 2026-07-10. v2.0 (P | 22. Approval UI (LiveLink) | v3.0 | 6/6 | Complete | 2026-07-16 | | 23. Classification Disposition + Per-Client Automation Gate | v3.0 | 6/6 | Complete | 2026-07-17 | +### Phase 24: AWS Route 53 DNS Sync + +**Goal:** Sync DNS zones/records from AWS Route 53 into Postgres, support full CRUD back to Route 53 from Pulse, track record-level changes over time, log every sync and CRUD operation for audit, and integrate into the existing per-system sync section (scheduler, admin UI, health checks) alongside Autotask/Datto RMM/Veeam. AWS credentials are resolved via BWS (Bitwarden Secrets Manager), not plaintext env vars. +**Requirements**: SC-1, SC-2, SC-3, SC-4, SC-5, SC-6 (the numbered Success Criteria below serve as this phase's requirement IDs — this project has no REQUIREMENTS.md) +**Depends on:** Phase 23 +**Plans:** 7 plans in 4 waves + +Plans: +- [ ] 24-01-PLAN.md — Foundation: AWS SDK install, migration 102 (zones/records/history/audit tables), shared types, credential factory, BWS + DNS-egress checkpoint *(wave 1)* +- [ ] 24-02-PLAN.md — Route53SyncService: zone/record mirror sync with pagination, soft-delete, and `sync_detected_drift` change history *(wave 2)* +- [ ] 24-03-PLAN.md — Record validation (D-01 NS/SOA allowlist, AWS error sanitizer) + pending/committed/failed audit lifecycle persistence *(wave 2)* +- [ ] 24-04-PLAN.md — Health check: auth probe + D-12 live NS-delegation comparison, registered in integration-health *(wave 2)* +- [ ] 24-05-PLAN.md — `/api/route53/*` read routes, sync trigger, and CRUD write routes with `requireAdmin()` gating *(wave 3)* +- [ ] 24-06-PLAN.md — Scheduler entries (`route53-incremental`, `route53-full`) + `/admin/sync` tile *(wave 3)* +- [ ] 24-07-PLAN.md — `/admin/sync/route53` detail page, record editor dialog, end-to-end phase verification *(wave 4)* + +**Success Criteria:** +1. Route 53 hosted zones and records sync into Postgres on a schedule, matching AWS as source of truth +2. Create/update/delete operations initiated from Pulse propagate to Route 53 via the AWS API +3. Every sync and CRUD operation is logged with actor, timestamp, and before/after values +4. Record-level change history is queryable (not just current state) +5. AWS credentials are resolved via BWS at runtime — never persisted in plaintext env vars +6. Integration appears in the existing sync admin UI/scheduler alongside other integrations + --- *Roadmap created: 2026-05-03* *v2.0 phases added: 2026-07-10* diff --git a/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-PLAN.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-PLAN.md new file mode 100644 index 0000000..9f5792e --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-PLAN.md @@ -0,0 +1,381 @@ +--- +phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - package.json + - package-lock.json + - migrations/102_route53_tables.sql + - lib/types/route53.ts + - lib/services/route53-factory.ts + - lib/services/route53-factory.test.ts + - CLAUDE.md +autonomous: false +requirements: [SC-3, SC-4, SC-5] +user_setup: + - service: aws-route53 + why: "Route 53 API access for zone/record sync and CRUD write-back" + env_vars: + - name: AWS_ACCESS_KEY_ID + source: "Bitwarden Secrets Manager project referenced by BWS_PROJECT_ID (injected by docker-entrypoint.sh via `bws run`) — NOT the committed .env file" + - name: AWS_SECRET_ACCESS_KEY + source: "Bitwarden Secrets Manager project referenced by BWS_PROJECT_ID" + - name: AWS_REGION + source: "Bitwarden Secrets Manager project, or leave unset to default to us-east-1" + dashboard_config: + - task: "Create/confirm an IAM user or role scoped to route53:ListHostedZones, route53:GetHostedZone, route53:ListResourceRecordSets, route53:ChangeResourceRecordSets, route53:GetChange only (least privilege — T-24-08)" + location: "AWS Console -> IAM -> Users/Roles -> Permissions" + - task: "Confirm the BWS project emits the secrets under the literal key names AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY / AWS_REGION" + location: "Bitwarden Secrets Manager -> project referenced by BWS_PROJECT_ID" + +must_haves: + truths: + - "SC-5: AWS credentials reach the Node process only as env vars injected by `bws run` at the docker entrypoint; no AWS_ACCESS_KEY_ID or AWS_SECRET_ACCESS_KEY value is added to the committed .env file" + - "SC-5: isRoute53Configured() returns false when AWS credential env vars are absent, and getRoute53Client() throws rather than constructing an unauthenticated client" + - "SC-3/SC-4: The dedicated Route 53 schema (zones, records, record history, audit log) exists in Postgres with a source tag column distinguishing pulse_crud from sync_detected_drift (D-06) and a status column supporting pending/committed/failed (D-07)" + - "D-10: An integration_settings row with key='route53' exists so the /admin/integrations toggle is display-only, with no sync/CRUD-blocking behavior anywhere" + - "D-03 ACCEPTED TRADEOFF: destructive record operations execute immediately with no staged approval gate. Malicious or malformed record values (dangling-CNAME / subdomain-takeover, SPF/DKIM TXT tampering) are NOT blocked pre-write. Mitigation is post-hoc traceability only — route53_audit_log captures actor, timestamp, and before/after for every attempt. This is an intentional, documented acceptance, not an oversight." + artifacts: + - path: "migrations/102_route53_tables.sql" + provides: "route53_zones / route53_records / route53_record_history / route53_audit_log + integration_settings seed" + contains: "CREATE TABLE IF NOT EXISTS route53_audit_log" + - path: "lib/services/route53-factory.ts" + provides: "getRoute53Client() + isRoute53Configured() + resetRoute53Client()" + exports: ["getRoute53Client", "isRoute53Configured", "resetRoute53Client"] + - path: "lib/types/route53.ts" + provides: "Route53Zone / Route53Record / Route53RecordHistory / Route53AuditLog / Route53SyncResult types" + - path: "lib/services/route53-factory.test.ts" + provides: "isRoute53Configured() branch coverage" + key_links: + - from: "lib/services/route53-factory.ts" + to: "@aws-sdk/client-route-53" + via: "Route53Client construction with no explicit credentials option" + pattern: "new Route53Client\\(" + - from: "migrations/102_route53_tables.sql" + to: "integration_settings" + via: "seed row for key='route53'" + pattern: "INSERT INTO integration_settings" +--- + + +Lay the Route 53 foundation: install the official AWS SDK client, create the dedicated +Postgres schema (mirror tables + change-history ledger + audit ledger), define shared +TypeScript types, and add the credential factory following the exact +`lib/services/-factory.ts` + `isConfigured()` shape every other Pulse +integration uses. + +Purpose: every downstream plan in this phase (sync service, CRUD routes, health check, +admin UI) imports from these four artifacts. Nothing else can start until they exist. +Output: `@aws-sdk/client-route-53` in package.json, `migrations/102_route53_tables.sql`, +`lib/types/route53.ts`, `lib/services/route53-factory.ts` (+ test), CLAUDE.md env-prefix row. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-CONTEXT.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-RESEARCH.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-PATTERNS.md + + + + +lib/services/route53-factory.ts exports: + isRoute53Configured(): boolean + getRoute53Client(): Route53Client // from '@aws-sdk/client-route-53' + resetRoute53Client(): void + +lib/types/route53.ts exports (camelCase — API-response shape, transformed from snake_case rows): + Route53Zone { id, name, comment, privateZone, recordCount, authoritativeNameServers, syncedAt, isDeleted } + Route53Record { recordKey, zoneId, name, type, setIdentifier, ttl, resourceRecords, aliasTarget, syncedAt, isDeleted } + Route53RecordHistory { id, zoneId, recordKey, recordName, recordType, changeAction, beforeValue, afterValue, source, changedByUserId, changedByEmail, changedAt } + Route53AuditLog { id, operation, zoneId, recordKey, recordName, recordType, beforeValue, afterValue, performedByUserId, performedByEmail, performedAt, completedAt, status, awsChangeId, awsChangeStatus, errorMessage } + Route53RecordValue { value: string } + Route53WritableType 'A' | 'AAAA' | 'CNAME' | 'MX' | 'TXT' | 'SRV' + Route53HistorySource 'pulse_crud' | 'sync_detected_drift' + Route53AuditStatus 'pending' | 'committed' | 'failed' + Route53SyncResult { syncId, syncType, status, startedAt, completedAt, duration, entities, errors } + +Postgres primary keys (used by every downstream query): + route53_zones.id = AWS hosted zone id with the '/hostedzone/' prefix stripped + route53_records.record_key = `${zoneId}:${name}:${type}:${setIdentifier ?? ''}` + + + + + + + Task 1: Install AWS SDK client and create the Route 53 migration + package.json, package-lock.json, migrations/102_route53_tables.sql + + - package.json (confirm no existing @aws-sdk dependency, confirm scripts) + - migrations/091_pax8_tables.sql (mirror-table conventions: raw_payload JSONB, synced_at/is_deleted/deleted_at, per-table idx_*_is_deleted) + - migrations/075_itglue_audit.sql (itglue_writes ledger shape: status CHECK, performed_by_user_id FK to "user"(id) ON DELETE SET NULL, before_value/after_value JSONB, error_message) + - migrations/081_integration_settings.sql (integration_settings columns + ON CONFLICT (key) DO NOTHING seed pattern) + - migrations/001_initial_schema.sql lines 542-555 (sync_history table — entity_type/sync_type/status/records_* columns the sync service will reuse; do NOT recreate it) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-PATTERNS.md (migration section) + + +Run `npm install @aws-sdk/client-route-53`. The package legitimacy audit in 24-RESEARCH.md +already recorded a `[OK]` slopcheck verdict (official `aws/aws-sdk-js-v3` repo, ~1.78M +weekly downloads) — no additional legitimacy gate is required. Do NOT install +`@aws-sdk/credential-provider-node` or any `@smithy/*` package explicitly; they arrive +transitively and the default credential chain is used implicitly. + +Create `migrations/102_route53_tables.sql` (next number after the current highest, +`101_reschedule_mimecast_sync.sql`). Every statement uses `IF NOT EXISTS`. Open with a +header comment block matching `migrations/091_pax8_tables.sql`'s style, stating that this +is the Phase 24 AWS Route 53 schema and that retention is unbounded by design (D-08 — no +purge job, matching existing Pulse convention). + +Table `route53_zones`: +`id TEXT PRIMARY KEY` (AWS hosted zone id, `/hostedzone/` prefix stripped), +`name TEXT NOT NULL`, `comment TEXT`, `private_zone BOOLEAN NOT NULL DEFAULT false`, +`record_count INTEGER NOT NULL DEFAULT 0`, +`authoritative_name_servers JSONB` (the `DelegationSet.NameServers` array — consumed by the +D-12 NS-delegation health check in plan 24-04), `raw_payload JSONB`, +`created_at TIMESTAMPTZ NOT NULL DEFAULT NOW()`, `updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW()`, +`synced_at TIMESTAMPTZ NOT NULL DEFAULT NOW()`, +`is_deleted BOOLEAN NOT NULL DEFAULT false`, `deleted_at TIMESTAMPTZ`. +Indexes: `idx_route53_zones_is_deleted` on `(is_deleted)`, `idx_route53_zones_name` on `(name)`. + +Table `route53_records`: +`record_key TEXT PRIMARY KEY` (composite string `zoneId:name:type:setIdentifier`, empty +string for a null set identifier — Route 53 recordsets are uniquely identified by +zone+name+type+SetIdentifier, there is no AWS-side record id), +`zone_id TEXT NOT NULL REFERENCES route53_zones(id) ON DELETE CASCADE`, +`name TEXT NOT NULL`, `type TEXT NOT NULL`, `set_identifier TEXT`, `ttl INTEGER`, +`resource_records JSONB` (array of `{ "value": "..." }` objects), +`alias_target JSONB` (Route 53 alias records have no TTL/ResourceRecords), `raw_payload JSONB`, +plus the same five audit columns as `route53_zones`. +Indexes: `idx_route53_records_zone` on `(zone_id)`, `idx_route53_records_is_deleted` on +`(is_deleted)`, `idx_route53_records_name_type` on `(zone_id, name, type)`. + +Table `route53_record_history` (D-06 — append-only change ledger, written by BOTH the sync +service on detected drift and the CRUD routes): +`id UUID PRIMARY KEY DEFAULT gen_random_uuid()`, +`zone_id TEXT NOT NULL REFERENCES route53_zones(id) ON DELETE CASCADE`, +`record_key TEXT NOT NULL`, `record_name TEXT NOT NULL`, `record_type TEXT NOT NULL`, +`change_action TEXT NOT NULL CHECK (change_action IN ('create','update','delete'))`, +`before_value JSONB`, `after_value JSONB`, +`source TEXT NOT NULL CHECK (source IN ('pulse_crud','sync_detected_drift'))`, +`changed_by_user_id TEXT REFERENCES "user"(id) ON DELETE SET NULL`, +`changed_by_email TEXT`, `audit_log_id UUID` (soft ref to `route53_audit_log(id)` — no hard +FK, so a history row survives audit-log changes), `changed_at TIMESTAMPTZ NOT NULL DEFAULT NOW()`. +Indexes: `idx_route53_record_history_record` on `(record_key, changed_at DESC)`, +`idx_route53_record_history_source` on `(source)`, +`idx_route53_record_history_zone` on `(zone_id, changed_at DESC)`. + +Table `route53_audit_log` (D-03/D-07 — every attempted operation including failures): +`id UUID PRIMARY KEY DEFAULT gen_random_uuid()`, +`operation TEXT NOT NULL CHECK (operation IN ('create','update','delete','sync'))`, +`zone_id TEXT`, `record_key TEXT`, `record_name TEXT`, `record_type TEXT`, +`before_value JSONB`, `after_value JSONB`, +`performed_by_user_id TEXT REFERENCES "user"(id) ON DELETE SET NULL`, +`performed_by_email TEXT`, `performed_at TIMESTAMPTZ NOT NULL DEFAULT NOW()`, +`completed_at TIMESTAMPTZ`, +`status TEXT NOT NULL CHECK (status IN ('pending','committed','failed'))`, +`aws_change_id TEXT`, `aws_change_status TEXT`, `aws_response JSONB`, `error_message TEXT`. +Indexes: `ix_route53_audit_log_record` on `(zone_id, record_key, performed_at DESC)`, +`ix_route53_audit_log_status` on `(status)`. +`zone_id` is deliberately NOT an FK here — a failed attempt against a zone that was never +synced must still be recordable. + +Finally append the D-10 seed row: +`INSERT INTO integration_settings (key, disabled) VALUES ('route53', false) ON CONFLICT (key) DO NOTHING;` + +Do NOT edit `migrations/081_integration_settings.sql` — it is committed. + +Apply the migration to the running database manually (Postgres init only applies +`migrations/*.sql` on first volume boot — per CLAUDE.md and the MEMORY caveat confirmed by +migration 090): `docker exec -i pulse-postgres psql -U "$POSTGRES_USER" -d "$POSTGRES_DB" < migrations/102_route53_tables.sql`. +If the container is not running, note this in the SUMMARY as a deployment follow-up rather +than skipping the file. + + + grep -c 'CREATE TABLE IF NOT EXISTS route53_' migrations/102_route53_tables.sql | grep -qx 4 && grep -q "source IN ('pulse_crud','sync_detected_drift')" migrations/102_route53_tables.sql && grep -q "status IN ('pending','committed','failed')" migrations/102_route53_tables.sql && grep -q "INSERT INTO integration_settings" migrations/102_route53_tables.sql && node -e "const p=require('./package.json');if(!p.dependencies['@aws-sdk/client-route-53'])process.exit(1)" && echo PASS + + + - `package.json` `dependencies` contains a `@aws-sdk/client-route-53` entry; `package-lock.json` is updated in the same commit + - `migrations/102_route53_tables.sql` exists and contains exactly 4 `CREATE TABLE IF NOT EXISTS route53_*` statements + - `route53_record_history` has a `source TEXT NOT NULL CHECK (source IN ('pulse_crud','sync_detected_drift'))` column (D-06) + - `route53_audit_log` has `status TEXT NOT NULL CHECK (status IN ('pending','committed','failed'))` and an `error_message TEXT` column (D-07) + - `route53_zones` has an `authoritative_name_servers JSONB` column (D-12 input) + - The file ends with `INSERT INTO integration_settings (key, disabled) VALUES ('route53', false) ON CONFLICT (key) DO NOTHING;` (D-10) + - `git diff --name-only` does NOT list `migrations/081_integration_settings.sql` or any other pre-existing migration + - No `AWS_ACCESS_KEY_ID` or `AWS_SECRET_ACCESS_KEY` line appears in the committed `.env` file: `grep -c '^AWS_' .env` returns 0 + + Migration file created with all 4 tables + seed row; AWS SDK installed; no committed migration edited; no AWS secret written to .env. + + + + Task 2: Add Route 53 shared types and the credential factory + lib/types/route53.ts, lib/services/route53-factory.ts, lib/services/route53-factory.test.ts, CLAUDE.md + + - lib/services/veeam-factory.ts (canonical factory shape — singleton, isXConfigured, throw-on-missing, resetXClient) + - lib/services/datto-rmm-factory.ts (multi-var config check variant) + - lib/services/pax8-factory.test.ts (existing factory test conventions in this codebase — env var save/restore pattern) + - lib/types/veeam.ts (domain type barrel conventions) + - migrations/102_route53_tables.sql (column names the types must mirror in camelCase) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-RESEARCH.md (Pattern 1 and Pitfall 1) + + + - `isRoute53Configured()` returns `false` when `AWS_ACCESS_KEY_ID` is unset + - `isRoute53Configured()` returns `false` when `AWS_SECRET_ACCESS_KEY` is unset + - `isRoute53Configured()` returns `false` when both are set to empty strings + - `isRoute53Configured()` returns `true` when both are set to non-empty values + - `getRoute53Client()` throws an Error mentioning `AWS_ACCESS_KEY_ID` when credentials are absent + - `getRoute53Client()` returns the same instance on a second call (singleton), and a different instance after `resetRoute53Client()` + + +Create `lib/types/route53.ts` exporting the camelCase interfaces and string-literal unions +listed in this plan's `` block. These are the API-response shapes — route +handlers transform `snake_case` rows into them manually (no ORM, per CLAUDE.md). Include +`Route53WritableType` as `'A' | 'AAAA' | 'CNAME' | 'MX' | 'TXT' | 'SRV'` (D-01) and +`Route53HistorySource` / `Route53AuditStatus` matching the migration's CHECK constraints +exactly. `Route53SyncResult` mirrors the shape `VeeamSyncResult` uses in +`lib/services/veeam-sync-service.ts` (`syncId`, `syncType`, `status`, `startedAt`, +`completedAt`, `duration`, `entities`, `errors`). + +Create `lib/services/route53-factory.ts` following `lib/services/veeam-factory.ts` line for +line, adapted per 24-PATTERNS.md's "Adaptation for Route 53" snippet: +module-level `let route53ClientInstance: Route53Client | null = null`; +`isRoute53Configured()` returning `!!(process.env.AWS_ACCESS_KEY_ID && process.env.AWS_SECRET_ACCESS_KEY)`; +`getRoute53Client()` that throws with the message +`'AWS credentials missing. Please set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY (and AWS_REGION) environment variables.'` +when unconfigured, otherwise constructs +`new Route53Client({ region: process.env.AWS_REGION || 'us-east-1' })`; +and `resetRoute53Client()` setting the singleton back to null. + +CRITICAL (24-RESEARCH.md Pitfall 1): do NOT pass an explicit `credentials:` option to +`Route53Client`. Omitting it lets `@aws-sdk/credential-provider-node`'s default chain read +`AWS_ACCESS_KEY_ID` / `AWS_SECRET_ACCESS_KEY` / `AWS_SESSION_TOKEN` from `process.env`, +which is exactly how BWS injects them at the `docker-entrypoint.sh` layer. Add an inline +comment stating this so a future reader does not "fix" it by adding explicit credentials. +Do NOT introduce a `ROUTE53_*` env prefix — the SDK hardcodes the `AWS_*` names. + +Create `lib/services/route53-factory.test.ts` covering the `` cases above. Follow +`lib/services/pax8-factory.test.ts`'s env-var save/restore discipline (snapshot +`process.env` values in `beforeEach`, restore in `afterEach`) and call +`resetRoute53Client()` between cases so the singleton does not leak across tests. Import +`describe`/`it`/`expect`/`beforeEach`/`afterEach` explicitly from `vitest` (this project +sets `globals: false` in `vitest.config.ts`). + +Add a row to CLAUDE.md's integration env-prefix table: `| AWS Route 53 | AWS_* (literal +AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY / AWS_REGION — intentional exception to the +per-service prefix convention; the AWS SDK's default credential chain hardcodes these +names. Injected by BWS at the container entrypoint, never in .env) |`. + + + npx vitest run lib/services/route53-factory.test.ts && npx tsc --noEmit --pretty + + + - `npx vitest run lib/services/route53-factory.test.ts` passes with at least 5 assertions covering the `` list + - `npx tsc --noEmit --pretty` exits 0 + - `grep -n "credentials" lib/services/route53-factory.ts` shows only comment lines, never a `credentials:` object literal passed to `Route53Client` + - `grep -c 'ROUTE53_ACCESS\|ROUTE53_SECRET' lib/services/route53-factory.ts` returns 0 + - `lib/types/route53.ts` exports `Route53WritableType` with exactly the six D-01 types and no `NS` or `SOA` member: `grep -q "'SRV'" lib/types/route53.ts && ! grep -q "'NS'" lib/types/route53.ts` + - CLAUDE.md's integration table contains a row matching `grep -c 'AWS Route 53' CLAUDE.md` >= 1 + + Types and factory exist, factory tests green, type-check clean, no explicit credentials wiring, CLAUDE.md documents the AWS_* exception. + + + + Task 3: Confirm BWS credential names and outbound DNS egress + +Pause execution and present the four verification steps below to the developer verbatim. Run any command the developer asks you to run on their behalf, but do not proceed to plan 24-02 until they respond. Record every answer in the SUMMARY — plan 24-04's resolver implementation branches on the DNS-egress result, and plan 24-01 Task 2's factory may need a one-line env var name change if the BWS key names differ. + + + The AWS SDK is installed, the Route 53 schema exists in Postgres, and + `lib/services/route53-factory.ts` reads credentials from the literal `AWS_ACCESS_KEY_ID` + / `AWS_SECRET_ACCESS_KEY` / `AWS_REGION` env var names via the AWS SDK's default + credential provider chain. Three assumptions from 24-RESEARCH.md's Open Questions cannot + be verified from the repository and must be confirmed before plans 24-02 through 24-07 + build on them. + + +1. **BWS secret key names (Open Question 1, Assumption A2).** In Bitwarden Secrets Manager, + open the project referenced by `BWS_PROJECT_ID` and confirm the AWS credentials are + stored under keys named exactly `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, and + (optionally) `AWS_REGION`. `bws run` exports each secret under its own key name, so a + different key name means the factory will not see the credentials. + If the names differ, report the actual names — the factory in Task 2 needs a one-line change. + +2. **Credentials reach the container (Open Question 2).** With the app running, run: + `docker exec pulse-app sh -lc 'echo "id=${AWS_ACCESS_KEY_ID:+SET} secret=${AWS_SECRET_ACCESS_KEY:+SET} region=${AWS_REGION:-unset}"'` + Expected: `id=SET secret=SET region=us-east-1` (or another explicit region). + This prints only presence markers, never the secret values. + +3. **Outbound DNS egress to public resolvers (Open Question 3, Assumption A3).** The D-12 + NS-delegation health check in plan 24-04 depends on reaching 1.1.1.1/8.8.8.8 on UDP/53 + from inside the container. Run: + `docker exec pulse-app node -e "const{Resolver}=require('dns');const r=new Resolver();r.setServers(['1.1.1.1','8.8.8.8']);r.resolveNs('google.com',(e,a)=>console.log(e?'EGRESS-BLOCKED: '+e.code:'EGRESS-OK: '+a.join(',')))"` + Expected: a line starting `EGRESS-OK:`. If it prints `EGRESS-BLOCKED`, plan 24-04 must + use the DoH-over-HTTPS fallback described in 24-RESEARCH.md's Alternatives Considered + instead of Node's `dns` module. + +4. **IAM scope (T-24-08, operational).** Confirm the IAM principal behind these credentials + is scoped to Route 53 actions only (`route53:ListHostedZones`, `route53:GetHostedZone`, + `route53:ListResourceRecordSets`, `route53:ChangeResourceRecordSets`, `route53:GetChange`). + This is an AWS-console concern Pulse cannot enforce; report the answer either way. + + +Reply with the four answers, e.g. "1: names match / 2: SET SET us-east-1 / 3: EGRESS-OK / +4: scoped to route53 only", or describe any deviation. Type "approved" if all four match +expectations. + + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| BWS/Bitwarden cloud → container env | AWS credentials cross into the process at `docker-entrypoint.sh` before Node starts | +| Node process → AWS Route 53 API | SigV4-signed HTTPS calls carrying long-lived IAM credentials | +| npm registry → repo dependency tree | New third-party package added to production runtime | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-24-06 | Information Disclosure | committed `.env` file | mitigate | AWS credentials are NEVER written to `.env` (which is committed to git per CLAUDE.md). Task 1 acceptance criterion asserts `grep -c '^AWS_' .env` returns 0. Credentials arrive only via `bws run` env injection. | +| T-24-08 | Elevation of Privilege | IAM principal behind AWS_ACCESS_KEY_ID | transfer | Least-privilege IAM policy scoped to the five Route 53 actions. Enforced in the AWS console, not in Pulse code — surfaced as checkpoint question 4 and recorded in `user_setup`. | +| T-24-09 | Information Disclosure | checkpoint verification commands | mitigate | Verification steps print only `SET`/`unset` presence markers (`${VAR:+SET}`), never secret values. | +| T-24-SC | Tampering | `npm install @aws-sdk/client-route-53` | mitigate | 24-RESEARCH.md `## Package Legitimacy Audit` records a `[OK]` slopcheck verdict for the single new package (official `aws/aws-sdk-js-v3` repo, ~6 years old, ~1.78M weekly downloads). No `[ASSUMED]`/`[SUS]` packages in this phase, so no blocking legitimacy checkpoint is required. Version is recorded in `package-lock.json`. | +| T-24-05 | Tampering / Spoofing | record values written to live DNS (dangling CNAME, SPF/DKIM TXT tampering) | accept | D-03 explicitly accepts immediate execution with no pre-write approval gate. Compensating control is post-hoc only: `route53_audit_log` captures actor, timestamp, and before/after for every attempt including failures. Recorded verbatim in this plan's `must_haves.truths` as an intentional acceptance. | + + + +- `npx vitest run lib/services/route53-factory.test.ts` green +- `npx tsc --noEmit --pretty` exits 0 +- `npm test` (full suite) still green — no regression from the new dependency +- `psql -c "\d route53_audit_log"` (or `docker exec pulse-postgres psql ... -c '\dt route53_*'`) lists all four tables +- Checkpoint answers recorded in the SUMMARY, including whether DNS egress is available (drives plan 24-04's implementation choice) + + + +- `@aws-sdk/client-route-53` present in `package.json` dependencies +- `migrations/102_route53_tables.sql` defines route53_zones, route53_records, route53_record_history, route53_audit_log and seeds `integration_settings('route53')` +- `lib/types/route53.ts` and `lib/services/route53-factory.ts` export the contracts listed in `` +- Factory tests pass; type-check clean +- No AWS secret value written to `.env` or any committed file +- Checkpoint answered: BWS key names confirmed, credentials present in container, DNS egress result known + + + +Create `.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-SUMMARY.md` when done. +Record in the SUMMARY: the confirmed BWS secret key names, the DNS-egress result +(EGRESS-OK vs EGRESS-BLOCKED — plan 24-04 depends on this), and whether the migration was +applied to the live database or is pending deployment. + diff --git a/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-02-PLAN.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-02-PLAN.md new file mode 100644 index 0000000..d72223b --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-02-PLAN.md @@ -0,0 +1,371 @@ +--- +phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud +plan: 02 +type: execute +wave: 2 +depends_on: ["24-01"] +files_modified: + - lib/services/route53-record-key.ts + - lib/services/route53-record-key.test.ts + - lib/services/route53-sync-service.ts + - lib/services/route53-sync-service.test.ts +autonomous: true +requirements: [SC-1, SC-4] + +must_haves: + truths: + - "SC-1: A scheduled sync pulls every hosted zone and every resource record set from AWS Route 53 into route53_zones / route53_records, with AWS as the source of truth" + - "SC-1: Records that disappear from AWS are soft-deleted in the mirror (is_deleted=true, deleted_at set) rather than left stale" + - "SC-4: D-06 — when a synced record differs from the mirror row, the sync writes a route53_record_history row tagged source='sync_detected_drift', so 'did someone change this outside Pulse?' is answerable from the ledger" + - "SC-4: Drift history rows carry the whole-recordset before/after snapshot (Route 53 models an update as a whole-recordset replace, so per-value diffing is not attempted)" + - "A sync run is bookkept in the existing sync_history table with entity_type='route53', and a catastrophic failure marks that row status='failed' with the error message" + artifacts: + - path: "lib/services/route53-record-key.ts" + provides: "Pure record-key derivation, recordset normalization, and drift classification helpers (unit-testable without AWS)" + exports: ["buildRecordKey", "normalizeRecordSet", "classifyDrift", "recordSetsEqual"] + - path: "lib/services/route53-sync-service.ts" + provides: "Route53SyncService with fullSync/incrementalSync + getRoute53SyncService() singleton" + exports: ["Route53SyncService", "getRoute53SyncService"] + min_lines: 200 + - path: "lib/services/route53-sync-service.test.ts" + provides: "Drift-classification and history-row-shape coverage" + key_links: + - from: "lib/services/route53-sync-service.ts" + to: "lib/services/route53-factory.ts" + via: "getRoute53Client() import" + pattern: "getRoute53Client" + - from: "lib/services/route53-sync-service.ts" + to: "route53_record_history" + via: "INSERT with source='sync_detected_drift'" + pattern: "sync_detected_drift" + - from: "lib/services/route53-sync-service.ts" + to: "sync_history" + via: "INSERT/UPDATE bookkeeping with entity_type='route53'" + pattern: "sync_history" +--- + + +Build the Route 53 → Postgres mirror sync: paginate hosted zones and resource record sets +from AWS, upsert them into the Phase 24 tables, soft-delete anything AWS no longer returns, +and append a `sync_detected_drift` history row for every record whose content changed +outside Pulse (D-06). + +Purpose: SC-1 (scheduled sync with AWS as source of truth) and the drift half of SC-4 +(queryable record-level history, not just current state). +Output: `lib/services/route53-record-key.ts` (pure helpers + tests), +`lib/services/route53-sync-service.ts` (+ tests), exporting `getRoute53SyncService()` for +plans 24-05 and 24-06 to consume. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-RESEARCH.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-PATTERNS.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-SUMMARY.md + + + + +lib/services/route53-factory.ts: + isRoute53Configured(): boolean + getRoute53Client(): Route53Client + resetRoute53Client(): void + +lib/types/route53.ts: + Route53Record { recordKey, zoneId, name, type, setIdentifier, ttl, resourceRecords, aliasTarget, syncedAt, isDeleted } + Route53SyncResult { syncId, syncType, status, startedAt, completedAt, duration, entities, errors } + Route53HistorySource = 'pulse_crud' | 'sync_detected_drift' + +Postgres (migration 102): + route53_zones(id TEXT PK, name, comment, private_zone, record_count, + authoritative_name_servers JSONB, raw_payload JSONB, + created_at, updated_at, synced_at, is_deleted, deleted_at) + route53_records(record_key TEXT PK, zone_id TEXT FK->route53_zones(id) ON DELETE CASCADE, + name, type, set_identifier, ttl, resource_records JSONB, + alias_target JSONB, raw_payload JSONB, + created_at, updated_at, synced_at, is_deleted, deleted_at) + route53_record_history(id UUID PK, zone_id, record_key, record_name, record_type, + change_action CHECK IN ('create','update','delete'), + before_value JSONB, after_value JSONB, + source CHECK IN ('pulse_crud','sync_detected_drift'), + changed_by_user_id, changed_by_email, audit_log_id, changed_at) + +Pre-existing (migration 001, do not alter): + sync_history(id SERIAL PK, entity_type VARCHAR(100), sync_type VARCHAR(50) + CHECK IN ('full','incremental','entity-specific'), + status CHECK IN ('started','in_progress','completed','failed'), + started_at, completed_at, records_added, records_updated, + records_deleted, error_message, triggered_by, entity_details) + +lib/services/postgres-client.ts default export `postgresClient`: + .query(sql, params?) -> { rows: T[] } + .transaction(fn) + + + + + + + Task 1: Pure record-key, normalization, and drift-classification helpers + lib/services/route53-record-key.ts, lib/services/route53-record-key.test.ts + + - lib/types/route53.ts (types created in plan 24-01) + - lib/services/analyzer/link-discovery.ts (the `_INTERNALS` export convention used in this codebase for testing private helpers) + - lib/services/pax8-company-matcher.test.ts (existing pure-helper test style: explicit vitest imports, table-driven cases) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-RESEARCH.md (Anti-Patterns: no per-value diffing; whole-recordset replace) + + + - `buildRecordKey({ zoneId: 'Z123', name: 'www.example.com.', type: 'A', setIdentifier: null })` returns `'Z123:www.example.com.:A:'` + - `buildRecordKey` with a non-null `setIdentifier` appends it after the final colon + - `normalizeRecordSet` lowercases `name`, preserves the trailing dot, uppercases `type`, coerces missing `TTL` to `null`, and maps `ResourceRecords` to a sorted array of `{ value }` objects so ordering differences do not register as drift + - `normalizeRecordSet` on an alias record (no `TTL`, no `ResourceRecords`, has `AliasTarget`) returns `ttl: null`, `resourceRecords: []`, and a populated `aliasTarget` + - `recordSetsEqual(a, a)` is `true`; changing a single `ResourceRecords` value makes it `false`; changing only the order of `ResourceRecords` keeps it `true` + - `recordSetsEqual` returns `false` when TTL differs + - `classifyDrift(null, next)` returns `'create'` + - `classifyDrift(prev, null)` returns `'delete'` + - `classifyDrift(prev, next)` returns `'update'` when the normalized sets differ + - `classifyDrift(prev, next)` returns `null` (no history row) when the normalized sets are equal + + +Create `lib/services/route53-record-key.ts` — a dependency-free module (no `pg`, no AWS SDK +client construction; it may import types from `@aws-sdk/client-route-53` and +`@/lib/types/route53`) so it is unit-testable without mocking anything. + +Export: +- `buildRecordKey(input: { zoneId: string; name: string; type: string; setIdentifier?: string | null }): string` + producing `${zoneId}:${name}:${type}:${setIdentifier ?? ''}`. This is the + `route53_records.record_key` primary key and the `recordId` URL segment used by plan + 24-05's routes. +- `normalizeRecordSet(rs: ResourceRecordSet, zoneId: string): NormalizedRecordSet` where + `NormalizedRecordSet` is `{ recordKey, zoneId, name, type, setIdentifier, ttl, resourceRecords, aliasTarget }`. + Normalization rules: `name` lowercased with its trailing dot preserved; `type` uppercased; + `ttl` is `rs.TTL ?? null`; `resourceRecords` is `(rs.ResourceRecords ?? []).map(r => ({ value: r.Value })).sort((a,b) => a.value.localeCompare(b.value))`; + `aliasTarget` is `rs.AliasTarget ?? null`; `setIdentifier` is `rs.SetIdentifier ?? null`. + Sorting matters: AWS does not guarantee value ordering, and unsorted comparison would + produce phantom drift history rows on every sync. +- `recordSetsEqual(a: NormalizedRecordSet | null, b: NormalizedRecordSet | null): boolean` + comparing `ttl`, the serialized `resourceRecords` array, and the serialized `aliasTarget`. + Both null returns true; one null returns false. +- `classifyDrift(prev: NormalizedRecordSet | null, next: NormalizedRecordSet | null): 'create' | 'update' | 'delete' | null` + mapping to `route53_record_history.change_action`, returning `null` when there is no + material difference so the sync does not write a no-op history row. +- `toHistoryPayload(ns: NormalizedRecordSet | null): unknown` returning the JSONB shape + stored in `before_value`/`after_value`: `null` for a null input, otherwise + `{ name, type, setIdentifier, ttl, resourceRecords, aliasTarget }` — the whole-recordset + snapshot (24-RESEARCH.md Anti-Patterns: Route 53 has no partial-value primitive, so the + history unit of change is the whole recordset). + +Do not implement per-value diffing anywhere. + +Create `lib/services/route53-record-key.test.ts` covering every `` case above. +Import `describe`/`it`/`expect` explicitly from `vitest` (`globals: false` in +`vitest.config.ts`). + + + npx vitest run lib/services/route53-record-key.test.ts && npx tsc --noEmit --pretty + + + - `npx vitest run lib/services/route53-record-key.test.ts` passes with at least 10 assertions covering every `` bullet + - `grep -c "from 'pg'\|postgres-client\|Route53Client" lib/services/route53-record-key.ts` returns 0 — the module has no runtime dependency on a DB pool or an AWS client + - `classifyDrift` returns `null` for equal recordsets (asserted in the test file), preventing no-op history rows + - Re-ordering `resourceRecords` does not change `recordSetsEqual`'s result (asserted in the test file) + - `npx tsc --noEmit --pretty` exits 0 + + Pure helpers exist with full unit coverage; no phantom-drift ordering bug; type-check clean. + + + + Task 2: Route53SyncService — zones and records mirror sync + lib/services/route53-sync-service.ts + + - lib/services/veeam-sync-service.ts (class shape, isSyncInProgress, executeSync, sync_history bookkeeping at lines ~78-90 and ~139-165, step-loop with per-step error isolation at lines ~94-120, upsert-with-FK-safety-set at lines ~219-247) + - lib/services/pax8-sync-service.ts (getPax8SyncService() singleton export shape at line ~689) + - lib/services/route53-record-key.ts (created in Task 1) + - lib/services/route53-factory.ts (created in plan 24-01) + - lib/services/postgres-client.ts (query/transaction signatures) + - migrations/102_route53_tables.sql (exact column names) + + +Create `lib/services/route53-sync-service.ts` following `lib/services/veeam-sync-service.ts`'s +class shape exactly: + +- `export class Route53SyncService` with a private `client: Route53Client` (constructor + takes an optional client, defaulting to `getRoute53Client()`) and a private + `isSyncing = false` flag. +- `isSyncInProgress(): boolean` +- `fullSync(triggeredBy = 'system'): Promise` → `executeSync('full', triggeredBy)` +- `incrementalSync(triggeredBy = 'system'): Promise` → `executeSync('incremental', triggeredBy)` +- private `executeSync(syncType, triggeredBy)` that throws + `'A Route 53 sync operation is already in progress'` when `isSyncing`, sets the flag, + builds `syncId = 'route53-' + Date.now()`, inserts the `sync_history` row + (`entity_type='route53'`, `sync_type` = the literal `'full'`/`'incremental'` — the table's + CHECK constraint only allows `full`/`incremental`/`entity-specific`, so do NOT write + `route53-full` there), runs the step loop, updates `sync_history` on completion, and + resets `isSyncing` in a `finally` block. +- Step array in order: `{ name: 'zones', fn: () => this.syncZones() }` then + `{ name: 'records', fn: () => this.syncRecords() }`. Zones must run first because + `route53_records.zone_id` has an FK to `route53_zones(id)`. +- Wrap each step in its own try/catch so one failing step does not abort the other, pushing + `{ entity, success, recordsUpserted, duration, error }` into `entityResults` — same shape + as `VeeamSyncService`. +- Catastrophic-failure catch block updates `sync_history` to `status='failed'` with + `error_message`, matching the Veeam analog's lines ~152-173. Log with a `[ROUTE53-SYNC]` + prefix. Log `error.message` only — never the full AWS SDK error object, which can carry + request headers (T-24-03). + +`syncZones()`: paginate `ListHostedZonesCommand` using `Marker` / `IsTruncated` / +`NextMarker`. For each zone strip the `/hostedzone/` prefix from `Id`. To populate +`authoritative_name_servers` (needed by plan 24-04's D-12 check), call +`GetHostedZoneCommand({ Id: zoneId })` per zone and store `DelegationSet?.NameServers ?? []` +as JSONB. Upsert with `INSERT INTO route53_zones (...) VALUES (...) ON CONFLICT (id) DO UPDATE SET ... synced_at = NOW(), updated_at = NOW(), is_deleted = false, deleted_at = NULL`. +After the loop, soft-delete zones no longer returned by AWS: +`UPDATE route53_zones SET is_deleted = true, deleted_at = NOW(), updated_at = NOW() WHERE is_deleted = false AND id <> ALL($1)` +using the collected id array. Return the upserted count. + +`syncRecords()`: load the live zone id set with +`SELECT id FROM route53_zones WHERE is_deleted = false` (the FK-safety-set pattern from +`veeam-sync-service.ts` `syncBackupServers()`). For each zone, paginate +`ListResourceRecordSetsCommand` using `StartRecordName` / `StartRecordType` / +`StartRecordIdentifier` from `NextRecordName` / `NextRecordType` / `NextRecordIdentifier` +while `IsTruncated`. Normalize each recordset with `normalizeRecordSet()` and derive its key +with `buildRecordKey()`. Upsert into `route53_records` on `ON CONFLICT (record_key) DO UPDATE`, +resetting `is_deleted = false, deleted_at = NULL, synced_at = NOW(), updated_at = NOW()`. +After each zone's pagination completes, soft-delete that zone's records no longer present: +`UPDATE route53_records SET is_deleted = true, deleted_at = NOW(), updated_at = NOW() WHERE zone_id = $1 AND is_deleted = false AND record_key <> ALL($2)`. +Do NOT hard-delete — the history ledger references `record_key`. + +Both `fullSync` and `incrementalSync` run the same two steps. Per D-11 the difference is +cadence, not scope: Route 53's list APIs expose no modification cursor, so an "incremental" +run is the same full read against AWS with the same diff, just scheduled more frequently. +Add a comment in `executeSync` stating this explicitly so a future reader does not assume a +missing incremental optimization is a bug. + +Export a `getRoute53SyncService(): Route53SyncService` module-level singleton following +`pax8-sync-service.ts`'s pattern (lazy `let instance` + accessor). Plans 24-05 and 24-06 +import this. + +Do NOT add any `integration_settings.disabled` check in this file — per D-10 the Route 53 +disable toggle is display-only and must never gate sync. + + + npx tsc --noEmit --pretty && grep -q "getRoute53SyncService" lib/services/route53-sync-service.ts && grep -qv "integration_settings" lib/services/route53-sync-service.ts && echo PASS + + + - `lib/services/route53-sync-service.ts` exports `Route53SyncService` and `getRoute53SyncService` + - `npx tsc --noEmit --pretty` exits 0 + - `grep -c 'integration_settings' lib/services/route53-sync-service.ts` returns 0 (D-10 — disable never gates sync) + - `grep -c "entity_type" lib/services/route53-sync-service.ts` >= 1 and the inserted `sync_type` value is the literal `'full'` or `'incremental'`, satisfying `sync_history`'s CHECK constraint + - Both `ListHostedZonesCommand` and `ListResourceRecordSetsCommand` pagination loops are present: `grep -c 'IsTruncated' lib/services/route53-sync-service.ts` >= 2 + - Soft-delete statements exist for both tables: `grep -c 'is_deleted = true' lib/services/route53-sync-service.ts` >= 2 + - No `console.error` call passes a raw error object: every logging site uses `error instanceof Error ? error.message : String(error)` (T-24-03) + + Sync service mirrors zones and records with pagination, soft-delete, and sync_history bookkeeping; no disable gating; type-check clean. + + + + Task 3: Drift detection writes sync_detected_drift history rows + lib/services/route53-sync-service.ts, lib/services/route53-sync-service.test.ts + + - lib/services/route53-sync-service.ts (as written in Task 2) + - lib/services/route53-record-key.ts (classifyDrift, toHistoryPayload) + - migrations/102_route53_tables.sql (route53_record_history columns and CHECK constraints) + - lib/services/pax8-sync-service.test.ts (existing sync-service test style — how this codebase mocks postgresClient and external clients) + + + - Given a mirror row and an AWS recordset with a changed TTL, the drift step produces one history row with `change_action='update'`, `source='sync_detected_drift'`, `changed_by_user_id=null` + - Given a mirror row with no matching AWS recordset, the drift step produces a history row with `change_action='delete'` and a non-null `before_value`, null `after_value` + - Given an AWS recordset with no matching mirror row, the drift step produces a history row with `change_action='create'`, null `before_value`, non-null `after_value` + - Given identical mirror and AWS recordsets, the drift step produces zero history rows + - `before_value`/`after_value` payloads contain the whole recordset (`name`, `type`, `setIdentifier`, `ttl`, `resourceRecords`, `aliasTarget`), not a per-field delta + - A record changed by Pulse CRUD within the same window is still tagged `sync_detected_drift` by the sync (the sync has no way to know), and the `pulse_crud` row written by the CRUD route is the authoritative one — both rows coexist in the ledger + + +Extract the drift logic into an exported, injectable pure function in +`lib/services/route53-sync-service.ts` so it is unit-testable without a database: +`export function buildDriftHistoryRows(prevByKey: Map, nextByKey: Map, zoneId: string): DriftHistoryRow[]` +where `DriftHistoryRow` is +`{ zoneId, recordKey, recordName, recordType, changeAction, beforeValue, afterValue }`. +Implementation: union the two key sets, call `classifyDrift(prev, next)` per key, skip keys +returning `null`, and build the row using `toHistoryPayload()` for both values. + +Wire it into `syncRecords()`: before upserting a zone's recordsets, load that zone's current +mirror rows (`SELECT record_key, name, type, set_identifier, ttl, resource_records, alias_target FROM route53_records WHERE zone_id = $1 AND is_deleted = false`) +and shape them into `NormalizedRecordSet`s. Compute `buildDriftHistoryRows(...)` BEFORE the +upsert (after the upsert the previous state is gone). Then upsert, then insert the drift +rows with +`INSERT INTO route53_record_history (zone_id, record_key, record_name, record_type, change_action, before_value, after_value, source, changed_by_user_id, changed_by_email) VALUES (..., 'sync_detected_drift', NULL, NULL)`. +Insert drift rows in a batch (one multi-row INSERT or a loop inside +`postgresClient.transaction()`), and count them into the step's result so the sync summary +reports drift volume. + +Guard the very first sync: if the mirror had zero rows for a zone, do NOT emit `create` +history rows for the entire zone (that would produce thousands of meaningless rows on +initial import). Detect this with a zone-level check — if `prevByKey.size === 0`, skip +history generation for that zone entirely and log +`[ROUTE53-SYNC] Initial import for zone — skipping drift history` once. + +Create `lib/services/route53-sync-service.test.ts` covering every `` case by +calling `buildDriftHistoryRows` directly with hand-constructed maps. No AWS or Postgres +mocking is required for these cases. Import vitest primitives explicitly. + + + npx vitest run lib/services/route53-sync-service.test.ts && npx tsc --noEmit --pretty && npm test + + + - `npx vitest run lib/services/route53-sync-service.test.ts` passes with cases for update, delete, create, and no-change + - `buildDriftHistoryRows` is exported from `lib/services/route53-sync-service.ts` and takes no database handle + - `grep -c "'sync_detected_drift'" lib/services/route53-sync-service.ts` >= 1 + - The insert statement sets `changed_by_user_id` to NULL for drift rows (drift has no Pulse actor) + - Initial-import guard present: `grep -q "prevByKey.size === 0" lib/services/route53-sync-service.ts` + - Drift rows are computed before the upsert — the `buildDriftHistoryRows` call appears earlier in `syncRecords()` than the `INSERT INTO route53_records` statement + - `npm test` (full suite) exits 0 + + Drift produces correctly tagged history rows, initial import does not flood the ledger, full suite green. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| AWS Route 53 API → Pulse sync service | Untrusted-shape external payloads (zone/record data) enter Postgres | +| Sync service → application logs | AWS SDK errors may carry request metadata | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-24-03 | Information Disclosure | `console.error` in `Route53SyncService.executeSync` and step catch blocks | mitigate | Log `error instanceof Error ? error.message : String(error)` only; never pass the AWS SDK error object (which can include `$metadata` and request headers) to a logger. Asserted in Task 2 acceptance criteria. | +| T-24-04 | Repudiation | record changes made outside Pulse (AWS console, IaC) | mitigate | D-06 drift detection writes a `route53_record_history` row with `source='sync_detected_drift'` and whole-recordset before/after for every externally-changed record, making external mutation attributable-in-time even when the actor is unknown to Pulse. | +| T-24-10 | Denial of Service | initial import emitting one history row per record across all zones | mitigate | Zone-level initial-import guard (`prevByKey.size === 0` → skip history) prevents an unbounded first-run write amplification into the append-only, never-purged ledger (D-08). | +| T-24-11 | Tampering | AWS-supplied record values written directly into Postgres JSONB | accept | Values are stored as data and rendered as text by the admin UI (plan 24-07 renders through React's default escaping, no `dangerouslySetInnerHTML`). Route 53 is itself the authoritative source; validating its own output against itself provides no security benefit. | +| T-24-05 | Tampering / Spoofing | live DNS record content | accept | Carried forward from plan 24-01 — D-03 accepts immediate execution with post-hoc audit only. | + + + +- `npx vitest run lib/services/route53-record-key.test.ts lib/services/route53-sync-service.test.ts` green +- `npm test` full suite green +- `npx tsc --noEmit --pretty` exits 0 +- Manual smoke (if AWS credentials are live): `node -e "require('ts-node')"` is not available — instead trigger via plan 24-05's `/api/route53/sync` route once it exists, or confirm at the plan 24-07 checkpoint + + + +- Zones and records mirror into Postgres with pagination and soft-delete +- Drift produces `route53_record_history` rows tagged `sync_detected_drift` with whole-recordset before/after +- Equal recordsets produce zero history rows; initial import produces zero history rows +- `getRoute53SyncService()` exported for plans 24-05 and 24-06 +- No `integration_settings` gating anywhere in this file (D-10) + + + +Create `.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-02-SUMMARY.md` when done. + diff --git a/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-03-PLAN.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-03-PLAN.md new file mode 100644 index 0000000..7f6b289 --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-03-PLAN.md @@ -0,0 +1,295 @@ +--- +phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud +plan: 03 +type: execute +wave: 2 +depends_on: ["24-01"] +files_modified: + - lib/services/route53-record-validation.ts + - lib/services/route53-record-validation.test.ts + - lib/services/route53-write-persistence.ts + - lib/services/route53-write-persistence.test.ts +autonomous: true +requirements: [SC-3] + +must_haves: + truths: + - "D-01: A server-side validator rejects any write targeting record type NS or SOA, and accepts only A, AAAA, CNAME, MX, TXT, SRV — the rejection happens in library code the routes call before any AWS command is constructed, not in the UI" + - "SC-3/D-07: An audit row is created with status='pending' BEFORE any AWS call is made, and is transitioned to 'committed' or 'failed' after — no write to Route 53 can occur without an audit row already in flight" + - "SC-3/D-07: A failed AWS attempt leaves a route53_audit_log row with status='failed', a sanitized error_message, and the attempted before/after values — failures are as auditable as successes" + - "T-24-03: Error messages persisted and returned are sanitized — capped in length and stripped of AWS account ARNs, request ids, and access key ids before storage or client return" + artifacts: + - path: "lib/services/route53-record-validation.ts" + provides: "D-01 writable-type allowlist + record-shape validation, unit-testable without a request" + exports: ["WRITABLE_RECORD_TYPES", "validateRecordWrite", "sanitizeAwsError"] + - path: "lib/services/route53-write-persistence.ts" + provides: "pending/committed/failed audit lifecycle + pulse_crud history rows + mirror refresh" + exports: ["createPendingAuditLog", "markAuditCommitted", "markAuditFailed", "insertPulseCrudHistory", "upsertMirrorRecord", "softDeleteMirrorRecord"] + - path: "lib/services/route53-record-validation.test.ts" + provides: "allowlist and sanitizer coverage" + key_links: + - from: "lib/services/route53-record-validation.ts" + to: "lib/types/route53.ts" + via: "Route53WritableType import" + pattern: "Route53WritableType" + - from: "lib/services/route53-write-persistence.ts" + to: "route53_audit_log" + via: "INSERT ... status='pending' / UPDATE ... status='committed'|'failed'" + pattern: "route53_audit_log" + - from: "lib/services/route53-write-persistence.ts" + to: "route53_record_history" + via: "INSERT with source='pulse_crud'" + pattern: "pulse_crud" +--- + + +Build the two library modules the CRUD routes in plan 24-05 depend on: a server-side record +validator enforcing D-01's writable-type allowlist, and a write-persistence module +implementing the pending → committed/failed audit lifecycle (the single most important +pattern in this phase, lifted from the IT Glue write-back precedent). + +Purpose: SC-3 (every CRUD operation logged with actor, timestamp, before/after — including +failures per D-07) and the enforcement half of D-01. These live in `lib/services/` rather +than in route handlers specifically so they are unit-testable — `vitest.config.ts` only +includes `lib/**/*.test.ts`, so validation logic embedded in `app/api/**` route files cannot +be covered by an automated test. +Output: `lib/services/route53-record-validation.ts` and +`lib/services/route53-write-persistence.ts`, both with test files. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-RESEARCH.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-PATTERNS.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-SUMMARY.md + + + + +lib/types/route53.ts: + Route53WritableType = 'A' | 'AAAA' | 'CNAME' | 'MX' | 'TXT' | 'SRV' + Route53AuditStatus = 'pending' | 'committed' | 'failed' + Route53HistorySource = 'pulse_crud' | 'sync_detected_drift' + Route53RecordValue = { value: string } + +Postgres (migration 102): + route53_audit_log(id UUID PK DEFAULT gen_random_uuid(), + operation CHECK IN ('create','update','delete','sync'), + zone_id, record_key, record_name, record_type, + before_value JSONB, after_value JSONB, + performed_by_user_id TEXT FK->"user"(id) ON DELETE SET NULL, + performed_by_email, performed_at, completed_at, + status CHECK IN ('pending','committed','failed'), + aws_change_id, aws_change_status, aws_response JSONB, error_message) + route53_record_history(id UUID PK, zone_id, record_key, record_name, record_type, + change_action CHECK IN ('create','update','delete'), + before_value JSONB, after_value JSONB, + source CHECK IN ('pulse_crud','sync_detected_drift'), + changed_by_user_id, changed_by_email, audit_log_id, changed_at) + route53_records(record_key PK, zone_id, name, type, set_identifier, ttl, + resource_records JSONB, alias_target JSONB, raw_payload JSONB, + created_at, updated_at, synced_at, is_deleted, deleted_at) + +lib/services/postgres-client.ts default export `postgresClient`: + .query(sql, params?) -> { rows: T[] } + + + + + + + Task 1: Server-side record validation and AWS error sanitizer + lib/services/route53-record-validation.ts, lib/services/route53-record-validation.test.ts + + - lib/types/route53.ts (Route53WritableType — created in plan 24-01) + - app/api/analyzer/itglue/applications/[id]/apply/route.ts lines 86-94 (the credential-field blocklist — the exact "belt over the UI's braces" defensive-check pattern to mirror) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-RESEARCH.md (Security Domain: ASVS V5 note, Known Threat Patterns table) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-CONTEXT.md (D-01) + + + - `validateRecordWrite({ type: 'NS', ... })` returns `{ ok: false, status: 400, reason: }` + - `validateRecordWrite({ type: 'SOA', ... })` returns `{ ok: false, status: 400 }` + - `validateRecordWrite({ type: 'ns', ... })` (lowercase) is also rejected — the check is case-insensitive + - Each of `A`, `AAAA`, `CNAME`, `MX`, `TXT`, `SRV` with a well-formed payload returns `{ ok: true }` + - An unknown type such as `CAA` or `DS` returns `{ ok: false, status: 400 }` — the allowlist is closed, not a blocklist + - Missing or empty `name` returns `{ ok: false, status: 400 }` + - `ttl` outside 0..2147483647, or non-integer, returns `{ ok: false, status: 400 }` + - An empty `resourceRecords` array returns `{ ok: false, status: 400 }` (Route 53 rejects an empty value set) + - A `resourceRecords` entry with an empty-string value returns `{ ok: false, status: 400 }` + - `sanitizeAwsError` strips anything matching an AWS access key id pattern (`AKIA` followed by 16 alphanumerics), any `arn:aws:` substring through the following whitespace, and truncates the result to 500 characters + - `sanitizeAwsError` on a non-Error input returns a string, never throws + + +Create `lib/services/route53-record-validation.ts`. + +Export `WRITABLE_RECORD_TYPES` as a frozen array of exactly the six D-01 types: +`['A', 'AAAA', 'CNAME', 'MX', 'TXT', 'SRV']`. `NS` and `SOA` must not appear anywhere in +this array. This is a closed allowlist, deliberately not a blocklist — an unrecognized type +is rejected rather than passed through. + +Export `validateRecordWrite(input: { name: unknown; type: unknown; ttl?: unknown; resourceRecords?: unknown }): { ok: true; value: ValidatedRecordWrite } | { ok: false; status: 400; reason: string }` +where `ValidatedRecordWrite` is `{ name: string; type: Route53WritableType; ttl: number; resourceRecords: Route53RecordValue[] }`. +Rules, in order, each returning a distinct `reason` string: +1. `name` must be a non-empty string after trimming; normalize by lowercasing and appending + a trailing dot if absent (Route 53's canonical form). +2. `type` must be a string; uppercase it, then it must be a member of + `WRITABLE_RECORD_TYPES`. When the uppercased type is `NS` or `SOA`, use an explicit + reason naming zone delegation, e.g. + `'Record type NS is not writable from Pulse — NS and SOA are zone-delegation records (D-01)'`, + so an operator understands the refusal rather than seeing a generic type error. +3. `ttl` must be an integer between 0 and 2147483647 inclusive; default to 300 when omitted. +4. `resourceRecords` must be a non-empty array whose entries each have a non-empty string + `value`. Cap the array at 100 entries. +Do not introduce Zod — CLAUDE.md says route handlers do not use it and this module is +plain validation. Do not construct any AWS command here; this module has no AWS or DB +dependency. + +Export `sanitizeAwsError(err: unknown): string` (T-24-03). Take +`err instanceof Error ? err.message : String(err)`, then redact with regex replacements: +`/AKIA[0-9A-Z]{16}/g` → `'[redacted-key-id]'`, `/arn:aws:[^\s"']+/g` → `'[redacted-arn]'`, +and `/\b[0-9]{12}\b/g` → `'[redacted-account-id]'` (12-digit AWS account ids). Then truncate +to 500 characters with a trailing ellipsis. This is the only string that may be written to +`route53_audit_log.error_message` or returned in an API response body. + +Create `lib/services/route53-record-validation.test.ts` covering every `` case. +Import vitest primitives explicitly (`globals: false`). + + + npx vitest run lib/services/route53-record-validation.test.ts && npx tsc --noEmit --pretty + + + - `npx vitest run lib/services/route53-record-validation.test.ts` passes with at least 11 assertions covering every `` bullet + - `grep -c "'NS'\|'SOA'" lib/services/route53-record-validation.ts` shows NS/SOA appearing only inside the rejection branch and its reason message, never inside `WRITABLE_RECORD_TYPES` + - The test file asserts rejection for both `'NS'` and `'ns'` (case-insensitivity) and for an unlisted type such as `'CAA'` + - `sanitizeAwsError` test asserts an input containing `AKIAIOSFODNN7EXAMPLE` and `arn:aws:route53:::hostedzone/Z123` produces a string containing neither substring + - `grep -c "@aws-sdk\|postgres-client" lib/services/route53-record-validation.ts` returns 0 — no AWS or DB dependency + - `npx tsc --noEmit --pretty` exits 0 + + D-01 allowlist is enforced by tested library code with a closed allowlist; AWS errors have a tested sanitizer. + + + + Task 2: Audit lifecycle and pulse_crud history persistence + lib/services/route53-write-persistence.ts, lib/services/route53-write-persistence.test.ts + + - lib/services/analyzer/asset-audit/persistence.ts lines 321-379 (createPendingWrite / markWriteCommitted / markWriteFailed — the exact three-function shape to mirror) + - migrations/075_itglue_audit.sql lines 60-88 (itglue_writes precedent) + - migrations/102_route53_tables.sql (route53_audit_log and route53_record_history columns) + - lib/services/route53-record-validation.ts (sanitizeAwsError — created in Task 1) + - lib/services/postgres-client.ts (query signature and parameter binding style) + - lib/services/route53-record-key.ts (buildRecordKey — created in plan 24-02 Task 1; if plan 24-02 has not landed, import path is still `@/lib/services/route53-record-key`) + + +Create `lib/services/route53-write-persistence.ts` following +`lib/services/analyzer/asset-audit/persistence.ts`'s three-function shape exactly. + +`createPendingAuditLog(input: { operation: 'create'|'update'|'delete'; zoneId: string; recordKey: string; recordName: string; recordType: string; beforeValue: unknown; afterValue: unknown; performedByUserId: string | null; performedByEmail: string | null }): Promise<{ id: string }>` +— `INSERT INTO route53_audit_log (operation, zone_id, record_key, record_name, record_type, before_value, after_value, performed_by_user_id, performed_by_email, status) VALUES ($1,...,$6::jsonb,$7::jsonb,...,'pending') RETURNING id::text AS id`. +This must be callable and complete BEFORE any `ChangeResourceRecordSetsCommand` is +constructed — that discipline is the whole point of the pattern (24-RESEARCH.md Pattern 3: +"never write to the external system without an audit row already in flight"). + +`markAuditCommitted(id: string, awsChangeId: string | null, awsChangeStatus: string | null, awsResponse: unknown): Promise` +— `UPDATE route53_audit_log SET status = 'committed', completed_at = NOW(), aws_change_id = $2, aws_change_status = $3, aws_response = $4::jsonb WHERE id = $1`. + +`markAuditFailed(id: string, err: unknown): Promise` +— `UPDATE route53_audit_log SET status = 'failed', completed_at = NOW(), error_message = $2 WHERE id = $1`, passing `sanitizeAwsError(err)` as `$2` (D-07 + T-24-03). Never pass a raw +error object or `JSON.stringify(err)`. + +`insertPulseCrudHistory(input: { zoneId: string; recordKey: string; recordName: string; recordType: string; changeAction: 'create'|'update'|'delete'; beforeValue: unknown; afterValue: unknown; changedByUserId: string | null; changedByEmail: string | null; auditLogId: string }): Promise` +— `INSERT INTO route53_record_history (...) VALUES (..., 'pulse_crud', ...)`. Callers must +invoke this ONLY after `markAuditCommitted` — a failed AWS call changed nothing on AWS's +side, so it gets an audit row but no history row (24-RESEARCH.md Pattern 3, explicit). +Document that rule in a comment above the function. + +`upsertMirrorRecord(input: { recordKey, zoneId, name, type, setIdentifier, ttl, resourceRecords, aliasTarget, rawPayload }): Promise` +— best-effort refresh of `route53_records` after a committed write so the admin UI reflects +the change before the next scheduled sync. `INSERT ... ON CONFLICT (record_key) DO UPDATE SET ... synced_at = NOW(), updated_at = NOW(), is_deleted = false, deleted_at = NULL`. + +`softDeleteMirrorRecord(recordKey: string): Promise` +— `UPDATE route53_records SET is_deleted = true, deleted_at = NOW(), updated_at = NOW() WHERE record_key = $1`. Never hard-delete: `route53_record_history` references `record_key` +and the ledger is unbounded by design (D-08). + +`loadMirrorRecord(recordKey: string): Promise` +— `SELECT record_key, zone_id, name, type, set_identifier, ttl, resource_records, alias_target FROM route53_records WHERE record_key = $1 AND is_deleted = false`, returning a camelCase +object (manual snake_case→camelCase transform per CLAUDE.md, no ORM). This supplies the +`before_value` and, critically, the exact TTL and value set that a Route 53 `DELETE` action +requires to match (24-RESEARCH.md Pitfall 3 — a DELETE with a mismatched TTL or value set +fails or targets the wrong thing). + +Wrap the mirror-refresh helpers so a failure there is logged (`[ROUTE53-WRITE]` prefix, +`sanitizeAwsError`) but does not throw — the AWS write already succeeded and the next +incremental sync reconciles the mirror regardless. The audit/history writes must NOT be +best-effort; let them throw. + +Create `lib/services/route53-write-persistence.test.ts` verifying the SQL contract with a +mocked `postgresClient`: use `vi.mock('@/lib/services/postgres-client', ...)` (follow +whichever mocking style `lib/services/pax8-sync-service.test.ts` already uses in this repo) +and assert (a) `createPendingAuditLog` issues an INSERT whose SQL contains `'pending'`, +(b) `markAuditFailed` passes a sanitized string (an input containing `AKIAIOSFODNN7EXAMPLE` +does not appear in the bound parameters), (c) `insertPulseCrudHistory` binds the literal +`'pulse_crud'`, (d) `softDeleteMirrorRecord` issues an UPDATE and never a `DELETE FROM`. + + + npx vitest run lib/services/route53-write-persistence.test.ts && npx tsc --noEmit --pretty && npm test + + + - `lib/services/route53-write-persistence.ts` exports `createPendingAuditLog`, `markAuditCommitted`, `markAuditFailed`, `insertPulseCrudHistory`, `upsertMirrorRecord`, `softDeleteMirrorRecord`, `loadMirrorRecord` + - `npx vitest run lib/services/route53-write-persistence.test.ts` passes with the four assertions listed in the action + - `grep -c 'DELETE FROM route53_records' lib/services/route53-write-persistence.ts` returns 0 (soft-delete only, D-08) + - `markAuditFailed` calls `sanitizeAwsError`: `grep -q 'sanitizeAwsError' lib/services/route53-write-persistence.ts` + - `grep -c "'pulse_crud'" lib/services/route53-write-persistence.ts` >= 1 + - A comment above `insertPulseCrudHistory` states that it must not be called on a failed AWS write + - `npm test` full suite exits 0 + - `npx tsc --noEmit --pretty` exits 0 + + Audit lifecycle and history persistence exist with tested SQL contracts; failures are sanitized; no hard deletes. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| HTTP request body → validation module | Untrusted operator-supplied record payload crosses into logic that will mutate live DNS | +| AWS SDK error → Postgres / HTTP response | Error text may carry account ids, ARNs, or key ids | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-24-01 | Tampering | record `type` field in a write request | mitigate | `validateRecordWrite` enforces a closed allowlist of the six D-01 types in `lib/services/route53-record-validation.ts`, rejecting `NS`/`SOA` (case-insensitively) with status 400. Enforced in library code the routes call before constructing any AWS command — never relying on the UI hiding the option. Unit-tested. | +| T-24-03 | Information Disclosure | `route53_audit_log.error_message` and API error responses | mitigate | `sanitizeAwsError` redacts AKIA-prefixed key ids, `arn:aws:*` strings, and 12-digit account ids, then truncates to 500 chars. It is the only permitted source of `error_message` values, asserted by a unit test. | +| T-24-04 | Repudiation | Pulse-initiated record writes | mitigate | `createPendingAuditLog` runs before any AWS call, capturing actor (`performed_by_user_id`/`performed_by_email`), timestamp, and before/after. A crashed process leaves a `pending` row, which is itself evidence an attempt occurred (SC-3). | +| T-24-07 | Tampering | audit/history rows treated as best-effort | mitigate | Audit and history writes intentionally throw on failure; only mirror-refresh helpers are best-effort. A DB failure must fail the request rather than silently produce an unlogged DNS mutation. | +| T-24-12 | Denial of Service | oversized `resourceRecords` array in a write request | mitigate | Validation caps `resourceRecords` at 100 entries and rejects empty-string values before any AWS call. | +| T-24-05 | Tampering / Spoofing | semantic content of record values (dangling CNAME, SPF/DKIM TXT) | accept | Carried forward from plan 24-01 — D-03 accepts immediate execution with no pre-write approval gate; `route53_audit_log` before/after + actor is the compensating post-hoc control. Shape validation here explicitly does NOT attempt semantic threat detection. | + + + +- `npx vitest run lib/services/route53-record-validation.test.ts lib/services/route53-write-persistence.test.ts` green +- `npm test` full suite green +- `npx tsc --noEmit --pretty` exits 0 +- `grep -rn "NS\b" lib/services/route53-record-validation.ts` confirms NS appears only in the rejection path + + + +- D-01 allowlist enforced by tested library code, closed (unknown types rejected) +- AWS errors sanitized before storage or client return +- pending → committed/failed lifecycle implemented with the audit row created before any AWS call +- `pulse_crud` history rows written only after a committed write +- Mirror updates are soft-delete only + + + +Create `.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-03-SUMMARY.md` when done. + diff --git a/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-04-PLAN.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-04-PLAN.md new file mode 100644 index 0000000..1d6982e --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-04-PLAN.md @@ -0,0 +1,300 @@ +--- +phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud +plan: 04 +type: execute +wave: 2 +depends_on: ["24-01"] +files_modified: + - lib/services/route53-dns-delegation.ts + - lib/services/route53-dns-delegation.test.ts + - lib/services/integration-health.ts +autonomous: true +requirements: [SC-6] + +must_haves: + truths: + - "SC-6: Route 53 appears in the integration health list under key 'route53', with the same not_configured / ok / auth_failed / unreachable status vocabulary every other integration uses" + - "D-12: The health check compares each hosted zone's Route-53-authoritative NS records against a LIVE public DNS lookup for that domain, and a mismatch degrades the reported health — there is no manually-maintained 'expected NS' field anywhere" + - "D-12: The live lookup uses a dedicated dns.Resolver() instance with setServers(['1.1.1.1','8.8.8.8']); the process-global dns.setServers() is never called, so internal service hostname resolution is unaffected" + - "D-10: The 'route53' health result flows through the existing applyDisableOverlay(), so disabling Route 53 in /admin/integrations suppresses the health display only — no sync or CRUD path consults integration_settings" + artifacts: + - path: "lib/services/route53-dns-delegation.ts" + provides: "NS normalization + mismatch detection + live resolver lookup, split so the pure half is unit-testable" + exports: ["normalizeNsList", "compareNsDelegation", "resolveLiveNs", "checkAllZoneDelegations"] + - path: "lib/services/route53-dns-delegation.test.ts" + provides: "NS normalization and mismatch-detection coverage" + - path: "lib/services/integration-health.ts" + provides: "checkRoute53() registered in checkIntegrationHealth()'s Promise.all" + contains: "checkRoute53" + key_links: + - from: "lib/services/integration-health.ts" + to: "lib/services/route53-factory.ts" + via: "isRoute53Configured() gate + ListHostedZonesCommand auth probe" + pattern: "isRoute53Configured" + - from: "lib/services/integration-health.ts" + to: "lib/services/route53-dns-delegation.ts" + via: "checkAllZoneDelegations() call inside checkRoute53()" + pattern: "checkAllZoneDelegations" + - from: "lib/services/route53-dns-delegation.ts" + to: "node:dns" + via: "dedicated Resolver instance with setServers" + pattern: "new Resolver\\(" +--- + + +Add Route 53 to the integration health system with the D-12 DNS-specific delegation check: +beyond the standard auth probe and last-sync age, compare each hosted zone's +Route-53-authoritative name servers against a live public DNS lookup and flag mismatches as +degraded health. + +Purpose: SC-6 (integration appears in the existing health/admin surface alongside the +others) plus D-12's DNS-specific extension. +Output: `lib/services/route53-dns-delegation.ts` (+ tests) and a `checkRoute53()` function +registered in `lib/services/integration-health.ts`. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-RESEARCH.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-PATTERNS.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-SUMMARY.md + + + + +export type HealthStatus = /* union defined at line 18 — includes 'ok', 'not_configured', + 'auth_failed', 'unreachable', 'unknown', 'disabled'; read the live union before writing */ + +export interface IntegrationHealth { + key: string; + name: string; + category: 'psa' | 'rmm' | 'docs' | 'security' | 'backup' | 'network' | 'identity' + | 'mdm' | 'mail' | 'finance' | 'productivity' | 'llm'; + status: HealthStatus; + configured: boolean; + latencyMs?: number; + error?: string | null; + tokenExpiry?: TokenExpiry | null; + checkedAt: string; +} + +export async function checkIntegrationHealth(opts?: { skipCache?: boolean }): Promise + // line ~325: results = await Promise.all([ checkAutotask(), checkDattoRmm(), checkItglue(), checkS1(), ...checkConfigOnly wrappers ]) + // line ~354: const overlaid = await applyDisableOverlay(results); <- D-10 disable overlay, already generic by key + + +lib/services/route53-factory.ts: + isRoute53Configured(): boolean + getRoute53Client(): Route53Client + +Postgres (migration 102): + route53_zones(id, name, authoritative_name_servers JSONB, is_deleted, synced_at, ...) + + + + + + + Task 1: NS normalization and delegation-comparison module + lib/services/route53-dns-delegation.ts, lib/services/route53-dns-delegation.test.ts + + - lib/services/pipeline-steps/ping-flap-suppress.ts line 6 (the existing `import { promises as dns } from 'dns'` precedent in this codebase) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-RESEARCH.md (Code Examples: checkNsDelegation; Pitfall 5: never call global dns.setServers) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-SUMMARY.md (the recorded DNS-egress result — EGRESS-OK or EGRESS-BLOCKED — decides the resolver strategy in Task 2) + - migrations/102_route53_tables.sql (route53_zones.authoritative_name_servers) + + + - `normalizeNsList(['NS-123.AWSDNS-45.com.', 'ns-999.awsdns-01.org'])` returns `['ns-123.awsdns-45.com', 'ns-999.awsdns-01.org']` — lowercased, trailing dot stripped + - `normalizeNsList` returns `[]` for `null`, `undefined`, and a non-array input + - `normalizeNsList` de-duplicates and sorts, so ordering differences never register as a mismatch + - `compareNsDelegation(authoritative, live)` with identical sets returns `{ mismatch: false, missingFromLive: [], extraInLive: [] }` + - `compareNsDelegation` returns `mismatch: true` with a populated `missingFromLive` when an authoritative NS is absent from the live answer + - `compareNsDelegation` returns `mismatch: true` with a populated `extraInLive` when the live answer contains an NS Route 53 does not consider authoritative + - `compareNsDelegation(authoritative, [])` returns `mismatch: true` (a domain with no live NS answer is a delegation problem, not a pass) + - `compareNsDelegation([], live)` returns `mismatch: false` — a zone with no recorded authoritative NS cannot be judged, so it must not produce a false alarm + - Comparison is case-insensitive and trailing-dot-insensitive on both sides + + +Create `lib/services/route53-dns-delegation.ts` split into a pure half and an I/O half so the +comparison logic is unit-testable without network access. + +Pure exports: +- `normalizeNsList(input: unknown): string[]` — returns `[]` for non-arrays; otherwise maps + each entry through `String(x).trim().toLowerCase().replace(/\.$/, '')`, drops empty + strings, de-duplicates via a `Set`, and sorts. +- `compareNsDelegation(authoritative: unknown, live: unknown): { mismatch: boolean; authoritative: string[]; live: string[]; missingFromLive: string[]; extraInLive: string[] }` + — normalizes both sides, then computes set differences. Returns `mismatch: false` when the + normalized authoritative list is empty (unjudgeable, not a failure). Returns + `mismatch: true` when the normalized live list is empty but the authoritative list is not. + Otherwise `mismatch` is `missingFromLive.length > 0 || extraInLive.length > 0`. + +I/O exports: +- `resolveLiveNs(domain: string, timeoutMs = 5000): Promise<{ ok: true; nameServers: string[] } | { ok: false; error: string }>` + — construct `new Resolver()` from `node:dns` (import `Resolver` from `'dns'`, matching the + existing codebase precedent in `ping-flap-suppress.ts`), call + `resolver.setServers(['1.1.1.1', '8.8.8.8'])` on that instance, then `resolveNs` + (promisified via `util.promisify(resolver.resolveNs.bind(resolver))` or the + `resolver.resolveNs` callback wrapped in a `Promise`). Race it against a timeout that + calls `resolver.cancel()` and resolves `{ ok: false, error: 'DNS lookup timed out after Nms' }`. + Strip the trailing dot from `domain` before lookup. + CRITICAL (24-RESEARCH.md Pitfall 5): never call the module-level `dns.setServers()` — that + would repoint DNS resolution for the entire Node process, including Postgres and Redis + hostname resolution. Add an inline comment stating this. +- `checkAllZoneDelegations(zones: Array<{ id: string; name: string; authoritativeNameServers: unknown }>, opts?: { concurrency?: number }): Promise>` + — resolve each zone's live NS and compare. Run at most `concurrency` (default 5) lookups in + parallel so a large zone list does not open hundreds of concurrent UDP sockets. A lookup + error yields `{ mismatch: false, error: }` — an unreachable resolver is an + infrastructure problem, not evidence of delegation drift, and must not be reported as a + mismatch. Skip zones whose `authoritativeNameServers` normalizes to an empty list. + +Create `lib/services/route53-dns-delegation.test.ts` covering every pure-half `` +case. Do not test `resolveLiveNs` against a live resolver — network calls in unit tests are +flaky; that path is covered by the manual verification in 24-VALIDATION.md. +Import vitest primitives explicitly (`globals: false`). + + + npx vitest run lib/services/route53-dns-delegation.test.ts && npx tsc --noEmit --pretty + + + - `npx vitest run lib/services/route53-dns-delegation.test.ts` passes with at least 9 assertions covering every `` bullet + - `grep -c 'dns.setServers\|setServers(\[.*\])' lib/services/route53-dns-delegation.ts` shows `setServers` called only on a `Resolver` instance variable, never on the imported `dns` module namespace + - `grep -q 'new Resolver(' lib/services/route53-dns-delegation.ts` + - `compareNsDelegation([], ['ns1.example.com'])` returns `mismatch: false` — asserted in the test file (no false alarm on unjudgeable zones) + - `compareNsDelegation(['ns1.example.com'], [])` returns `mismatch: true` — asserted in the test file + - The test file contains no network call: `grep -c 'resolveLiveNs' lib/services/route53-dns-delegation.test.ts` returns 0 + - `npx tsc --noEmit --pretty` exits 0 + + Pure NS comparison logic fully unit-tested; live resolver isolated to a dedicated instance; global DNS untouched. + + + + Task 2: Register checkRoute53() in the integration health aggregator + lib/services/integration-health.ts + + - lib/services/integration-health.ts (read in full — HealthStatus union at line ~18, IntegrationHealth interface at line ~33, checkDattoRmm() at lines 145-191 for the custom-body live-check pattern, checkItglue() at lines 193-209, checkConfigOnly() at lines 238-252, applyDisableOverlay() at lines ~310-319, checkIntegrationHealth() Promise.all at lines 325-352) + - lib/services/route53-dns-delegation.ts (created in Task 1) + - lib/services/route53-factory.ts (created in plan 24-01) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-SUMMARY.md (DNS-egress result) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-PATTERNS.md (integration-health section) + + +Modify `lib/services/integration-health.ts` only — do not create a parallel health module. + +Extend the `IntegrationHealth` interface with two optional fields (optional so no existing +check function needs changing): +`nsDelegationMismatches?: string[] | null` (zone names with a delegation mismatch) and +`nsDelegationErrors?: string[] | null` (zone names whose live lookup failed). Do NOT overload +the existing `error` field for this — 24-PATTERNS.md calls this out explicitly. + +Add `async function checkRoute53(): Promise` placed next to +`checkDattoRmm()`, using `key: 'route53'`, `name: 'AWS Route 53'`, `category: 'network'` +(an existing member of the category union — do not add a new category value). Behavior: + +1. Config gate, mirroring `checkDattoRmm()`'s early return: if `isRoute53Configured()` is + false, return `status: 'not_configured', configured: false` with `checkedAt` set. Do not + construct the client. +2. Auth probe: `getRoute53Client().send(new ListHostedZonesCommand({ MaxItems: '1' }))` + wrapped in try/catch, timing it for `latencyMs`. On an AWS SDK error whose `name` or + `$metadata.httpStatusCode` indicates a credential/authorization problem + (`InvalidClientTokenId`, `SignatureDoesNotMatch`, `AccessDenied`, `UnrecognizedClientException`, + or HTTP 401/403), return `status: 'auth_failed'`. On any other error return + `status: 'unreachable'`. In both branches set `error` to the sanitized message from + `sanitizeAwsError` in `lib/services/route53-record-validation.ts` (T-24-03) — never the + raw AWS error object, which carries `$metadata` including request ids. +3. D-12 delegation check: query + `SELECT id, name, authoritative_name_servers FROM route53_zones WHERE is_deleted = false` + and pass the rows (transformed to the `{ id, name, authoritativeNameServers }` camelCase + shape) to `checkAllZoneDelegations()`. Collect zone names where `mismatch === true` into + `nsDelegationMismatches` and zone names with an `error` into `nsDelegationErrors`. + If `nsDelegationMismatches` is non-empty, downgrade the returned `status` from `'ok'` to + the existing degraded-status member of the `HealthStatus` union — read the union at + line ~18 and use the member that already represents "reachable but not healthy"; if the + union has no such member, add `'degraded'` to it and confirm every consumer that + switches on `HealthStatus` (grep for `status ===` across `app/` and `components/`) + renders an unknown value without crashing. + Set `error` to a summary such as + `'NS delegation mismatch for N zone(s): example.com, other.com'` when mismatches exist. +4. Bound the total cost: if the zone list exceeds 50 zones, check only the first 50 by name + order and note the truncation in `error`. The health check runs behind a 5-minute cache + and must not become the slowest call in the aggregate. +5. Wrap the whole delegation step in try/catch — a Postgres failure or a blocked resolver + must degrade to `nsDelegationErrors` and leave the auth-probe status intact, never throw + out of `checkIntegrationHealth()`'s `Promise.all`. + +If plan 24-01's SUMMARY recorded `EGRESS-BLOCKED` for the DNS smoke test, implement +`resolveLiveNs`'s fallback path instead: a DoH GET to +`https://cloudflare-dns.com/dns-query?name=&type=NS` with header +`Accept: application/dns-json`, parsing `Answer[].data` — same normalized output shape, no +new npm dependency (uses `fetch`). Note which path was taken in the SUMMARY. + +Register the check in `checkIntegrationHealth()`'s `Promise.all` array (line ~325) as a bare +`checkRoute53(),` call alongside `checkAutotask()` / `checkDattoRmm()` — not wrapped in +`Promise.resolve()`, which is only used for the synchronous `checkConfigOnly()` helpers. + +Do NOT add any Route 53 branch to `applyDisableOverlay()` — it already keys off +`item.key`, so the `'route53'` result is covered automatically (D-10, display-only). + + + npx tsc --noEmit --pretty && grep -q "checkRoute53()," lib/services/integration-health.ts && grep -q "key: 'route53'" lib/services/integration-health.ts && npm test + + + - `lib/services/integration-health.ts` contains an `async function checkRoute53()` returning `key: 'route53'`, `name: 'AWS Route 53'`, `category: 'network'` + - `checkRoute53(),` appears inside `checkIntegrationHealth()`'s `Promise.all([...])` array, unwrapped + - `IntegrationHealth` gained `nsDelegationMismatches?` and `nsDelegationErrors?` as optional fields; `npx tsc --noEmit --pretty` exits 0 with no changes required in any other check function + - `grep -c 'integration_settings' lib/services/integration-health.ts` is unchanged from before this task (the disable overlay already existed; no new Route-53-specific disable logic added — D-10) + - The auth-probe catch branch passes its error through `sanitizeAwsError`: `grep -q 'sanitizeAwsError' lib/services/integration-health.ts` + - `curl -s localhost:3100/api/admin/integration-health` (or whichever route already serves `checkIntegrationHealth`, found by `grep -rl checkIntegrationHealth app/api`) returns a JSON array containing an object with `"key":"route53"` + - `npm test` full suite exits 0 + + Route 53 appears in the health aggregate with an auth probe plus D-12 delegation check; disable overlay works via the existing generic path; no other integration's check regressed. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| Pulse container → public DNS resolvers (1.1.1.1 / 8.8.8.8, UDP/53 or DoH/443) | Outbound network call to a third party whose answer influences a health verdict | +| AWS Route 53 API → health check | Auth probe error text may carry request metadata | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-24-13 | Denial of Service | process-global DNS resolver configuration | mitigate | `resolveLiveNs` calls `setServers` on a dedicated `new Resolver()` instance only. The process-global `dns.setServers()` is never invoked, so Postgres/Redis/AWS hostname resolution inside the container is unaffected. Asserted in Task 1 acceptance criteria. | +| T-24-14 | Denial of Service | unbounded parallel NS lookups across a large zone list | mitigate | `checkAllZoneDelegations` runs at most 5 concurrent lookups, each with a 5s timeout and `resolver.cancel()`, and `checkRoute53` caps the checked zone list at 50. The whole check sits behind the existing 5-minute health cache. | +| T-24-03 | Information Disclosure | auth-probe error surfaced in the health API response (readable by any authenticated user) | mitigate | Auth-probe errors pass through `sanitizeAwsError` before being placed in `IntegrationHealth.error`, redacting key ids, ARNs, and account ids. | +| T-24-15 | Spoofing | a third-party public resolver returning a forged NS answer | accept | The check is advisory health signalling, not an enforcement gate — a false mismatch degrades a status badge and triggers human investigation; it cannot cause a DNS mutation. Two independent resolvers (1.1.1.1 and 8.8.8.8) are configured, and lookup failures are reported as `nsDelegationErrors` rather than mismatches so an unreachable/hostile resolver cannot manufacture a false-positive drift alarm. | +| T-24-16 | Denial of Service | an exception in the delegation step aborting `Promise.all` and blanking every integration's health | mitigate | The entire delegation step is wrapped in try/catch inside `checkRoute53`; failures degrade to `nsDelegationErrors` while preserving the auth-probe status. | + + + +- `npx vitest run lib/services/route53-dns-delegation.test.ts` green +- `npm test` full suite green +- `npx tsc --noEmit --pretty` exits 0 +- The health endpoint returns a `route53` entry (curl assertion in Task 2 acceptance criteria) +- Toggling `route53` off at `/admin/integrations` flips its status to `disabled` within the 5-minute cache while a manual `POST /api/route53/sync` still works (D-10) — confirmed at the plan 24-07 checkpoint + + + +- `route53` present in the integration health list with the standard status vocabulary +- D-12 live NS comparison implemented against a dedicated resolver instance +- Delegation mismatches degrade the reported status and are enumerated in `nsDelegationMismatches` +- Lookup failures are reported separately and never counted as mismatches +- No process-global DNS mutation; no Route-53-specific disable gating + + + +Create `.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-04-SUMMARY.md` when done. +Record whether the Node `dns` path or the DoH fallback was used, and the exact +`HealthStatus` union member chosen for the degraded state. + diff --git a/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-05-PLAN.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-05-PLAN.md new file mode 100644 index 0000000..c9f9136 --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-05-PLAN.md @@ -0,0 +1,407 @@ +--- +phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud +plan: 05 +type: execute +wave: 3 +depends_on: ["24-02", "24-03"] +files_modified: + - lib/services/route53-change-submit.ts + - lib/services/route53-change-submit.test.ts + - app/api/route53/sync/route.ts + - app/api/route53/zones/route.ts + - app/api/route53/zones/[zoneId]/records/route.ts + - app/api/route53/zones/[zoneId]/records/[recordId]/route.ts + - app/api/route53/zones/[zoneId]/records/[recordId]/history/route.ts +autonomous: true +requirements: [SC-2, SC-3, SC-4] + +must_haves: + truths: + - "SC-2: Creating, updating, or deleting a record in Pulse submits a ChangeResourceRecordSetsCommand to AWS Route 53 and the change is accepted by AWS (the response carries a ChangeInfo.Id)" + - "SC-2/D-03: Update and delete execute immediately on request — there is no staged approval, second confirmation endpoint, or pending-approval state anywhere in the write path" + - "SC-3: Every write attempt creates a route53_audit_log row with status='pending' before the AWS call, transitioned to 'committed' or 'failed' after, carrying actor email, timestamp, and before/after values" + - "SC-4: Committed writes append a route53_record_history row tagged source='pulse_crud'; failed writes append an audit row but no history row" + - "D-01: A request with record type NS or SOA is rejected with HTTP 400 before any AWS command is constructed" + - "D-04: Every write route is gated by requireAdmin(); every read route is gated by at least requireAuth()" + - "A DELETE submits the exact current recordset (name, type, TTL, full value set) read from the mirror, because Route 53 rejects or mis-targets a DELETE that does not match exactly" + artifacts: + - path: "lib/services/route53-change-submit.ts" + provides: "ChangeResourceRecordSets construction + bounded GetChange poll, testable without a route" + exports: ["buildChangeBatch", "submitRecordChange", "pollChangeStatus"] + - path: "app/api/route53/zones/[zoneId]/records/[recordId]/route.ts" + provides: "PATCH (update) and DELETE handlers with requireAdmin gating" + exports: ["PATCH", "DELETE"] + - path: "app/api/route53/zones/[zoneId]/records/route.ts" + provides: "GET (list records in zone) and POST (create record)" + exports: ["GET", "POST"] + - path: "app/api/route53/zones/[zoneId]/records/[recordId]/history/route.ts" + provides: "GET record change history (SC-4 queryable ledger)" + exports: ["GET"] + - path: "app/api/route53/sync/route.ts" + provides: "POST manual sync trigger + GET sync status" + exports: ["GET", "POST"] + key_links: + - from: "app/api/route53/zones/[zoneId]/records/[recordId]/route.ts" + to: "lib/services/route53-record-validation.ts" + via: "validateRecordWrite() call before any AWS command construction" + pattern: "validateRecordWrite" + - from: "app/api/route53/zones/[zoneId]/records/[recordId]/route.ts" + to: "lib/services/route53-write-persistence.ts" + via: "createPendingAuditLog before the AWS call, markAuditCommitted/markAuditFailed after" + pattern: "createPendingAuditLog" + - from: "app/api/route53/zones/[zoneId]/records/[recordId]/route.ts" + to: "lib/auth-utils.ts" + via: "requireAdmin() gate" + pattern: "requireAdmin" + - from: "app/api/route53/sync/route.ts" + to: "lib/services/route53-sync-service.ts" + via: "getRoute53SyncService().fullSync() fire-and-forget" + pattern: "getRoute53SyncService" +--- + + +Build the `/api/route53/*` surface: read routes for zones, records, and change history; a +manual sync trigger; and the CRUD write routes that propagate creates, updates, and deletes +to AWS Route 53 through the pending → committed/failed audit lifecycle. + +Purpose: SC-2 (CRUD propagates to Route 53), SC-3 (every operation logged with actor, +timestamp, before/after), SC-4 (history queryable). +Output: one library module (`route53-change-submit.ts`, so the AWS-command construction is +unit-testable — `vitest.config.ts` only includes `lib/**`) plus five route files. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-RESEARCH.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-PATTERNS.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-02-SUMMARY.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-03-SUMMARY.md + + + + +lib/services/route53-factory.ts: + isRoute53Configured(): boolean + getRoute53Client(): Route53Client + +lib/services/route53-record-key.ts (plan 24-02): + buildRecordKey({ zoneId, name, type, setIdentifier }): string // `${zoneId}:${name}:${type}:${setIdentifier ?? ''}` + normalizeRecordSet(rs, zoneId): NormalizedRecordSet + toHistoryPayload(ns): unknown + +lib/services/route53-sync-service.ts (plan 24-02): + getRoute53SyncService(): Route53SyncService + Route53SyncService#isSyncInProgress(): boolean + Route53SyncService#fullSync(triggeredBy?): Promise + Route53SyncService#incrementalSync(triggeredBy?): Promise + +lib/services/route53-record-validation.ts (plan 24-03): + WRITABLE_RECORD_TYPES: readonly ['A','AAAA','CNAME','MX','TXT','SRV'] + validateRecordWrite(input): { ok: true; value: ValidatedRecordWrite } | { ok: false; status: 400; reason: string } + sanitizeAwsError(err: unknown): string + +lib/services/route53-write-persistence.ts (plan 24-03): + createPendingAuditLog(input): Promise<{ id: string }> + markAuditCommitted(id, awsChangeId, awsChangeStatus, awsResponse): Promise + markAuditFailed(id, err): Promise + insertPulseCrudHistory(input): Promise + upsertMirrorRecord(input): Promise + softDeleteMirrorRecord(recordKey): Promise + loadMirrorRecord(recordKey): Promise + +lib/auth-utils.ts: + requireAuth(): Promise<{ session, error: NextResponse|null }> // 401 when unauthenticated + requireAdmin(): Promise<{ session, error: NextResponse|null }> // 403 unless role is admin|super-admin + // session.user has { id, email, role } + +Postgres (migration 102): route53_zones, route53_records, route53_record_history, route53_audit_log +Pre-existing: sync_history(entity_type='route53', sync_type IN ('full','incremental'), ...) + + + + + + + Task 1: Change-batch construction and bounded propagation poll + lib/services/route53-change-submit.ts, lib/services/route53-change-submit.test.ts + + - lib/services/route53-record-key.ts (buildRecordKey, normalizeRecordSet — plan 24-02) + - lib/services/route53-record-validation.ts (ValidatedRecordWrite shape, sanitizeAwsError — plan 24-03) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-RESEARCH.md (Code Examples: short-interval GetChange poll; Anti-Patterns: never use waitUntilResourceRecordSetsChanged in a request handler; Pitfall 3: exact-match DELETE; Pitfall 4: PriorRequestNotComplete is retryable; Pitfall 6: do not block on propagation) + + + - `buildChangeBatch('UPSERT', { name: 'www.example.com.', type: 'A', ttl: 300, resourceRecords: [{ value: '1.2.3.4' }] })` produces `{ Changes: [{ Action: 'UPSERT', ResourceRecordSet: { Name, Type, TTL, ResourceRecords: [{ Value: '1.2.3.4' }] } }] }` + - `buildChangeBatch('CREATE', ...)` sets `Action: 'CREATE'`; `buildChangeBatch('DELETE', ...)` sets `Action: 'DELETE'` + - A `setIdentifier` present on the input is emitted as `SetIdentifier` on the `ResourceRecordSet`; absent/null omits the key entirely rather than emitting `undefined` + - `buildChangeBatch` for a DELETE emits the full `TTL` and complete `ResourceRecords` array from the supplied current state (Route 53 requires an exact match) + - `buildChangeBatch` throws when given a record type of `NS` or `SOA` — a defence-in-depth backstop independent of `validateRecordWrite` + - `isRetryableAwsError` returns `true` for an error named `ThrottlingException`, `PriorRequestNotComplete`, or `Throttling`, and `false` for `InvalidChangeBatch` + - `pollChangeStatus` returns `'INSYNC'` as soon as a supplied client reports `ChangeInfo.Status === 'INSYNC'` + - `pollChangeStatus` returns `'PENDING'` once its timeout budget elapses without an INSYNC answer, and makes no further calls after returning + + +Create `lib/services/route53-change-submit.ts` so AWS-command construction and the polling +loop live under `lib/**` where vitest can reach them (route files under `app/api/**` are +outside `vitest.config.ts`'s `include` glob). + +Export `buildChangeBatch(action: 'CREATE' | 'UPSERT' | 'DELETE', recordSet: { name: string; type: string; ttl: number; resourceRecords: Array<{ value: string }>; setIdentifier?: string | null }): ChangeBatch` +producing the `@aws-sdk/client-route-53` `ChangeBatch` shape. Omit `SetIdentifier` from the +emitted object when null/undefined rather than setting it to `undefined`. Throw an `Error` +naming the type when `type.toUpperCase()` is `NS` or `SOA` — a second, independent D-01 +enforcement point so no future caller can bypass `validateRecordWrite` (T-24-01, +defence in depth). + +Export `isRetryableAwsError(err: unknown): boolean` returning true for AWS error `name` +values `ThrottlingException`, `Throttling`, `PriorRequestNotComplete`, and +`ServiceUnavailable`. Per 24-RESEARCH.md Pitfall 4, `PriorRequestNotComplete` is a per-zone +serialization constraint (two writes to the same hosted zone close together), not a hard +failure. + +Export `submitRecordChange(input: { zoneId: string; action: 'CREATE'|'UPSERT'|'DELETE'; recordSet: ...; client?: Route53Client }): Promise<{ changeId: string | null; awsResponse: unknown }>` +— constructs `ChangeResourceRecordSetsCommand({ HostedZoneId: zoneId, ChangeBatch: buildChangeBatch(...) })` +and sends it. On an error where `isRetryableAwsError` is true, retry up to 2 additional +times with 750ms then 1500ms backoff; any other error rethrows immediately. Do not add a +general backoff wrapper around every AWS call — the SDK's built-in retry strategy already +handles transport-level retries (24-RESEARCH.md "Don't Hand-Roll"). + +Export `pollChangeStatus(changeId: string, opts?: { client?: Route53Client; timeoutMs?: number; intervalMs?: number }): Promise<'INSYNC' | 'PENDING'>` +— default `timeoutMs: 15000`, `intervalMs: 2000`. Loop sending `GetChangeCommand({ Id: changeId })` +until `ChangeInfo.Status === 'INSYNC'` or the budget elapses, then return `'PENDING'`. +Swallow per-attempt errors (a transient GetChange failure is not a write failure — the write +was already accepted) and keep polling until the budget elapses. +CRITICAL (24-RESEARCH.md Anti-Patterns): do NOT use the SDK's +`waitUntilResourceRecordSetsChanged` waiter — its default config is a 30-second interval +with 60 attempts, i.e. up to 30 minutes inside an HTTP request handler. + +Create `lib/services/route53-change-submit.test.ts` covering every `` case. Test +`pollChangeStatus` with a hand-rolled fake client object exposing a `send()` that returns +canned `ChangeInfo` values and counts invocations — no AWS mocking library, no network. Use +a short `timeoutMs`/`intervalMs` (e.g. 50/10) so the timeout case runs in milliseconds. +Import vitest primitives explicitly (`globals: false`). + + + npx vitest run lib/services/route53-change-submit.test.ts && npx tsc --noEmit --pretty + + + - `npx vitest run lib/services/route53-change-submit.test.ts` passes with at least 8 assertions covering every `` bullet, completing in under 5 seconds + - `grep -c 'waitUntilResourceRecordSetsChanged' lib/services/route53-change-submit.ts` returns 0 + - `buildChangeBatch` throws for `NS` and for `SOA` — asserted in the test file + - `pollChangeStatus` timeout case is asserted to return `'PENDING'` and to have stopped calling `send()` after returning (invocation counter does not increase afterwards) + - `npx tsc --noEmit --pretty` exits 0 + + Change construction, retry classification, and bounded polling are tested library code with no 30-minute waiter. + + + + Task 2: Read routes — zones, records, history, and sync status + app/api/route53/sync/route.ts, app/api/route53/zones/route.ts, app/api/route53/zones/[zoneId]/records/route.ts, app/api/route53/zones/[zoneId]/records/[recordId]/history/route.ts + + - app/api/pax8/sync/route.ts (full file — fire-and-forget POST + status GET shape to copy; note it has NO auth gate, an existing gap this plan must not replicate) + - app/api/pax8/companies/route.ts (list-read route shape, snake_case to camelCase transform) + - app/api/analyzer/itglue/applications/[id]/audit/route.ts (append-only ledger read route shape) + - lib/auth-utils.ts lines 31-45 and 79-103 (requireAuth / requireAdmin return shape) + - lib/services/route53-sync-service.ts (getRoute53SyncService, isSyncInProgress — plan 24-02) + - migrations/102_route53_tables.sql (exact column names for the SELECT statements) + - middleware.ts (confirm /api/route53/* is NOT added to the public-route list) + + +Create four read surfaces. Every handler follows CLAUDE.md's route conventions: `try/catch`, +`NextResponse.json({ error, message }, { status })`, manual snake_case→camelCase transform, +no Zod, no `'use server'`. + +`app/api/route53/sync/route.ts` — modelled on `app/api/pax8/sync/route.ts` but WITH auth: +- `POST`: `const { session, error } = await requireAdmin(); if (error) return error;` then + read `{ syncType }` from the body (default `'full'`). If `!isRoute53Configured()` return + 503 with a message naming the missing AWS env vars. If + `getRoute53SyncService().isSyncInProgress()` return 409 `{ error: 'Sync already in progress' }`. + Otherwise fire-and-forget `fullSync(session.user.email)` or `incrementalSync(...)`, catching + in a `.catch(err => console.error('[ROUTE53-SYNC] Background sync error:', sanitizeAwsError(err)))`, + and return `{ ok: true, message: 'Route 53 sync started' }` immediately. +- `GET`: `requireAuth()` gate. Return `{ inProgress, counts, history }` where `counts` comes + from a single query selecting `(SELECT COUNT(*) FROM route53_zones WHERE is_deleted = false) AS zones`, + the equivalent for `route53_records`, and `(SELECT COUNT(*) FROM route53_record_history) AS historyRows`; + and `history` from + `SELECT id, sync_type, status, started_at, completed_at, records_added, records_updated, records_deleted, error_message, triggered_by FROM sync_history WHERE entity_type = 'route53' ORDER BY started_at DESC LIMIT 10`. +Do NOT gate either handler on `integration_settings.disabled` — that is the PAX8-only +exception and D-10 explicitly excludes Route 53 from it. + +`app/api/route53/zones/route.ts` — `GET` with `requireAuth()`. Return +`SELECT id, name, comment, private_zone, record_count, authoritative_name_servers, synced_at FROM route53_zones WHERE is_deleted = false ORDER BY name` +transformed to the `Route53Zone` camelCase shape from `lib/types/route53.ts`. + +`app/api/route53/zones/[zoneId]/records/route.ts` — `GET` with `requireAuth()`. Params are a +Promise in Next 16 (`{ params }: { params: Promise<{ zoneId: string }> }`, awaited). Return +`SELECT record_key, zone_id, name, type, set_identifier, ttl, resource_records, alias_target, synced_at FROM route53_records WHERE zone_id = $1 AND is_deleted = false ORDER BY name, type` +transformed to `Route53Record`. Support optional `?type=` and `?search=` query params applied +as parameterized SQL predicates — never string-interpolated into the SQL. (The `POST` create +handler is added in Task 3 in this same file.) + +`app/api/route53/zones/[zoneId]/records/[recordId]/history/route.ts` — `GET` with +`requireAuth()`. `recordId` is the URL-encoded `record_key`; decode it with +`decodeURIComponent`. Return +`SELECT id, zone_id, record_key, record_name, record_type, change_action, before_value, after_value, source, changed_by_user_id, changed_by_email, changed_at FROM route53_record_history WHERE record_key = $1 ORDER BY changed_at DESC LIMIT $2` +with `limit` from `?limit=` clamped to 1..200, default 50. Transformed to +`Route53RecordHistory`. This route is SC-4's "history is queryable, not just current state" +proof. + +Confirm `middleware.ts` does not list `/api/route53` among its public routes — these +endpoints must stay behind the session-cookie check, with role enforcement in the handlers. + + + npx tsc --noEmit --pretty && test $(grep -rl "requireAuth\|requireAdmin" app/api/route53 | wc -l) -eq 4 && ! grep -rq "integration_settings" app/api/route53 && echo PASS + + + - All four read route files exist and every exported handler begins with a `requireAuth()` or `requireAdmin()` call whose `error` is returned early + - `grep -rc 'integration_settings' app/api/route53/` returns 0 across all files (D-10) + - `POST /api/route53/sync` uses `requireAdmin()`; `GET /api/route53/sync` uses `requireAuth()` + - `grep -rn 'route53' middleware.ts` returns no match (routes stay non-public) + - No SQL string interpolation of user input: `grep -rn '\${' app/api/route53/*/route.ts app/api/route53/**/route.ts` shows no template literal inside a SQL string containing a request-derived value + - `npx tsc --noEmit --pretty` exits 0 + - `curl -s -o /dev/null -w '%{http_code}' localhost:3100/api/route53/zones` returns 401 when unauthenticated + + Four read surfaces exist, all auth-gated, all parameterized, none gated on the disable toggle. + + + + Task 3: CRUD write routes — create, update, delete with the audit lifecycle + app/api/route53/zones/[zoneId]/records/route.ts, app/api/route53/zones/[zoneId]/records/[recordId]/route.ts + + - app/api/analyzer/itglue/applications/[id]/apply/route.ts (full file — the canonical pending-row-before-external-call pattern, the pre-write guardrail at lines 86-94, and the 502 failure-response convention at lines 188-200) + - lib/services/route53-write-persistence.ts (plan 24-03 — exact function signatures) + - lib/services/route53-record-validation.ts (plan 24-03 — validateRecordWrite, sanitizeAwsError) + - lib/services/route53-change-submit.ts (Task 1 of this plan) + - lib/services/route53-record-key.ts (buildRecordKey, toHistoryPayload — plan 24-02) + - app/api/route53/zones/[zoneId]/records/route.ts (the GET handler written in Task 2 — POST goes in this same file) + - lib/auth-utils.ts lines 79-103 (requireAdmin) + + +Add `POST` to `app/api/route53/zones/[zoneId]/records/route.ts` and create +`app/api/route53/zones/[zoneId]/records/[recordId]/route.ts` exporting `PATCH` and `DELETE`. +All three follow the identical eight-step sequence — factor the shared body into a local +helper in the `[recordId]` file only if it does not obscure the flow; duplication across two +files is acceptable here. + +Sequence for every write handler: +1. `const { session, error } = await requireAdmin(); if (error) return error;` (D-04). Never + rely on the UI hiding a control (T-24-02). +2. `if (!isRoute53Configured()) return NextResponse.json({ error: 'Route 53 not configured', message: '...' }, { status: 503 });` +3. Await `params`, parse the JSON body with `.catch(() => ({}))`. +4. Call `validateRecordWrite(...)`. On `{ ok: false }` return + `NextResponse.json({ error: 'Invalid record', message: result.reason }, { status: 400 })`. + This runs BEFORE any `@aws-sdk/client-route-53` command object is constructed (T-24-01). + For DELETE, validate the type of the record being deleted the same way — an NS/SOA delete + is as destructive as an NS/SOA write. +5. Establish `beforeValue`: + - POST (create): `loadMirrorRecord(recordKey)` must return null; if a record already + exists return 409 `{ error: 'Record already exists' }`. `beforeValue` is `null`. + - PATCH (update) / DELETE: `loadMirrorRecord(decodeURIComponent(recordId))`; a null result + returns 404. For DELETE, the loaded row's exact `name`, `type`, `ttl`, and full + `resourceRecords` set are what gets submitted to AWS — Route 53 rejects or mis-targets + a DELETE whose recordset does not match exactly (24-RESEARCH.md Pitfall 3). Never build + a DELETE from only `{ name, type }` supplied by the client. +6. `const audit = await createPendingAuditLog({ operation, zoneId, recordKey, recordName, recordType, beforeValue, afterValue, performedByUserId: session.user.id, performedByEmail: session.user.email })`. + This must complete before step 7. Never call AWS without a pending audit row in flight. +7. In a `try`: `const { changeId, awsResponse } = await submitRecordChange({ zoneId, action, recordSet })` + with `action` = `'CREATE'` for POST, `'UPSERT'` for PATCH, `'DELETE'` for DELETE. Then + `const propagationStatus = changeId ? await pollChangeStatus(changeId) : 'PENDING';` + Then `await markAuditCommitted(audit.id, changeId, propagationStatus, awsResponse);` + Then `await insertPulseCrudHistory({ ..., changeAction: 'create'|'update'|'delete', beforeValue, afterValue, changedByUserId: session.user.id, changedByEmail: session.user.email, auditLogId: audit.id });` + Then refresh the mirror: `upsertMirrorRecord(...)` for POST/PATCH, + `softDeleteMirrorRecord(recordKey)` for DELETE. + Return `NextResponse.json({ auditId: audit.id, status: 'committed', propagationStatus, record })` + with HTTP 200 (or 201 for POST). +8. In the `catch`: `const message = sanitizeAwsError(err); await markAuditFailed(audit.id, err);` + then `return NextResponse.json({ auditId: audit.id, status: 'failed', error: 'Route 53 write failed', message }, { status: 502 });` + Use 502 for AWS-side failures, matching the IT Glue write route's existing convention for + "upstream integration rejected the write" (24-PATTERNS.md), not 500. Do NOT call + `insertPulseCrudHistory` on this path — nothing changed on AWS's side (24-RESEARCH.md + Pattern 3). + +D-03 compliance: these handlers execute the mutation on the first request. Do not add a +`confirm` body flag, a two-phase endpoint, a `pending_approval` status, or any gate that +requires a second call. The audit trail is the control, not a pre-write block. + +`recordKey` derivation: for POST, compute it with +`buildRecordKey({ zoneId, name: validated.name, type: validated.type, setIdentifier })`. For +PATCH/DELETE it is `decodeURIComponent(recordId)`; verify the decoded key's `zoneId` prefix +matches the `zoneId` path param and return 400 on mismatch (prevents a caller from mutating +a record in a different zone through a mismatched path — T-24-17). + +Log with a `[ROUTE53-WRITE]` prefix and `sanitizeAwsError(err)` only. Never +`console.error(err)` with the raw AWS error object. + + + npx tsc --noEmit --pretty && npm test && test $(grep -rc "requireAdmin" app/api/route53/zones/\[zoneId\]/records/route.ts app/api/route53/zones/\[zoneId\]/records/\[recordId\]/route.ts | awk -F: '{s+=$2} END {print s}') -ge 3 && echo PASS + + + - `app/api/route53/zones/[zoneId]/records/route.ts` exports `GET` and `POST`; `app/api/route53/zones/[zoneId]/records/[recordId]/route.ts` exports `PATCH` and `DELETE` + - All three write handlers call `requireAdmin()` as their first statement and return `error` early (D-04) + - In each write handler, the `validateRecordWrite` call appears at a lower line number than any `submitRecordChange` / `ChangeResourceRecordSetsCommand` reference (D-01 enforced before command construction) + - In each write handler, `createPendingAuditLog` appears at a lower line number than `submitRecordChange` (audit row in flight before the AWS call) + - `insertPulseCrudHistory` appears only inside a `try` success path, never inside a `catch`: `grep -A20 'catch' | grep -c insertPulseCrudHistory` returns 0 + - Failure responses use status 502 and a `sanitizeAwsError` message: `grep -c 'status: 502' app/api/route53/zones/\[zoneId\]/records/\[recordId\]/route.ts` >= 2 + - `grep -rc 'pending_approval\|requiresConfirmation\|confirmToken' app/api/route53/` returns 0 (D-03 — no staged approval) + - DELETE builds its recordset from `loadMirrorRecord` output, not from the request body: `grep -B5 -A5 "'DELETE'" app/api/route53/zones/\[zoneId\]/records/\[recordId\]/route.ts` shows the mirror row's ttl/resourceRecords being passed + - `npm test` full suite exits 0; `npx tsc --noEmit --pretty` exits 0 + + Create/update/delete propagate to Route 53 with the audit row created first, history written only on success, failures logged with sanitized messages and returned as 502. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| Browser / any HTTP client → `/api/route53/*` | Untrusted request bodies and path params reach code that mutates live DNS | +| Pulse API route → AWS Route 53 | Authenticated mutation of a production DNS zone | +| AWS error → HTTP response body | Upstream error text returned to an authenticated caller | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-24-01 | Tampering | record `type` in POST/PATCH/DELETE bodies | mitigate | `validateRecordWrite` (closed six-type allowlist) runs before any AWS command is constructed and returns 400 for `NS`/`SOA`; `buildChangeBatch` throws on the same types as an independent second gate. Line-order asserted in acceptance criteria. | +| T-24-02 | Elevation of Privilege | a `user`-role session calling a write route directly, bypassing UI gating | mitigate | `requireAdmin()` is the first statement of every write handler (D-04), returning 403. Read routes use `requireAuth()` (401). `middleware.ts` is not modified — `/api/route53/*` stays outside the public-route list. Verified manually per 24-VALIDATION.md's Manual-Only table. | +| T-24-03 | Information Disclosure | AWS error text in the 502 response body and in `route53_audit_log.error_message` | mitigate | Every error path returns `sanitizeAwsError(err)`, which redacts AKIA key ids, `arn:aws:*` strings, and 12-digit account ids and truncates to 500 chars. Raw error objects are never logged. | +| T-24-04 | Repudiation | a DNS mutation with no attributable actor | mitigate | `createPendingAuditLog` runs before the AWS call with `performed_by_user_id` and `performed_by_email` from the Better Auth session; `insertPulseCrudHistory` records the same actor on success. A crashed request leaves a `pending` row as evidence of the attempt. | +| T-24-17 | Tampering | `recordId` path param decoding to a record in a different hosted zone than `zoneId` | mitigate | The decoded `record_key`'s zone prefix is compared against the `zoneId` path param; a mismatch returns 400 before any audit row or AWS call. | +| T-24-18 | Tampering | SQL injection via `?type=` / `?search=` / `recordId` query and path params | mitigate | Every SELECT uses `postgresClient.query(sql, params)` parameter binding; no request-derived value is interpolated into a SQL template literal. Asserted by grep in Task 2 acceptance criteria. | +| T-24-19 | Denial of Service | an HTTP handler blocked for up to 30 minutes on DNS propagation | mitigate | `pollChangeStatus` is bounded at 15s / 2s intervals and returns `propagationStatus: 'PENDING'` on timeout; the SDK's 30s/60-attempt `waitUntilResourceRecordSetsChanged` waiter is explicitly not used (grep-asserted). The next incremental sync reconciles final state. | +| T-24-20 | Denial of Service | concurrent writes to one hosted zone producing `PriorRequestNotComplete` | mitigate | `isRetryableAwsError` classifies `PriorRequestNotComplete` and throttling as retryable with bounded backoff (2 retries); plan 24-07's UI disables the save control while a request for that zone is in flight. | +| T-24-05 | Tampering / Spoofing | semantically malicious record values (dangling CNAME → subdomain takeover, SPF/DKIM TXT tampering) | accept | D-03 locks immediate execution with no pre-write approval gate. No semantic threat analysis is performed on record values. The compensating control is entirely post-hoc: `route53_audit_log` records actor, timestamp, and before/after for every attempt including failures, and `route53_record_history` makes the change queryable. Documented as an intentional acceptance in plan 24-01's `must_haves`. | + + + +- `npx vitest run lib/services/route53-change-submit.test.ts` green +- `npm test` full suite green; `npx tsc --noEmit --pretty` exits 0 +- Unauthenticated `curl localhost:3100/api/route53/zones` returns 401 +- Manual (per 24-VALIDATION.md): signed in as a `user`-role account, + `curl -X POST localhost:3100/api/route53/zones//records` returns 403; as `admin` it succeeds +- Manual (per 24-VALIDATION.md): a live create/update/delete round-trip against a disposable + test record produces 3 `route53_audit_log` rows with correct before/after and 3 + `route53_record_history` rows tagged `pulse_crud` +- Manual: `curl -X POST .../records -d '{"name":"x.example.com","type":"NS",...}'` as admin returns 400 + + + +- POST/PATCH/DELETE propagate to Route 53 and return the AWS change id plus a propagation status +- Every write attempt produces exactly one `route53_audit_log` row, transitioned to committed or failed +- Committed writes produce exactly one `route53_record_history` row tagged `pulse_crud`; failed writes produce none +- NS/SOA writes are rejected with 400 before any AWS command is constructed +- All write routes gated by `requireAdmin()`, all read routes by at least `requireAuth()` +- History is queryable via `GET /api/route53/zones/{zoneId}/records/{recordId}/history` +- No staged-approval mechanism anywhere in the write path (D-03) + + + +Create `.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-05-SUMMARY.md` when done. + diff --git a/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-06-PLAN.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-06-PLAN.md new file mode 100644 index 0000000..a32b9a2 --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-06-PLAN.md @@ -0,0 +1,254 @@ +--- +phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud +plan: 06 +type: execute +wave: 3 +depends_on: ["24-02"] +files_modified: + - lib/services/sync-scheduler.ts + - app/admin/sync/page.tsx + - public/logos/route53.svg +autonomous: true +requirements: [SC-1, SC-6] + +must_haves: + truths: + - "SC-1/D-11: Two cron schedules exist — route53-incremental (every 15 minutes) and route53-full (daily) — both seeded into sync_schedules and editable from /admin, matching how every other integration's schedules are managed" + - "SC-6: A Route 53 tile appears in the /admin/sync integration list with the same shape as the Veeam / Datto RMM / PAX8 tiles, linking to /admin/sync/route53" + - "D-10: The scheduler dispatch branch for Route 53 checks isRoute53Configured() only — it does NOT consult integration_settings.disabled, so disabling Route 53 in /admin/integrations suppresses health display without stopping sync (PAX8 remains the sole blocking exception)" + - "New schedules are seeded is_enabled: false, matching every other newly-introduced integration in this file, so nothing starts hitting AWS before an operator enables it" + artifacts: + - path: "lib/services/sync-scheduler.ts" + provides: "route53-incremental and route53-full in the sync_type union, defaultSchedules, and the dispatch chain" + contains: "route53-incremental" + - path: "app/admin/sync/page.tsx" + provides: "route53 entry in the INTEGRATIONS tile array" + contains: "id: 'route53'" + - path: "public/logos/route53.svg" + provides: "Tile logo asset" + key_links: + - from: "lib/services/sync-scheduler.ts" + to: "lib/services/route53-sync-service.ts" + via: "dynamic import of getRoute53SyncService inside the dispatch branch" + pattern: "getRoute53SyncService" + - from: "app/admin/sync/page.tsx" + to: "/admin/sync/route53" + via: "tile href" + pattern: "/admin/sync/route53" +--- + + +Wire Route 53 into the two existing operator surfaces it must appear in: the node-cron sync +scheduler (D-11's incremental + daily-full cadence) and the `/admin/sync` integration tile +list. + +Purpose: SC-1 (sync runs on a schedule) and SC-6 (integration appears in the existing sync +admin UI/scheduler alongside the others). +Output: modified `lib/services/sync-scheduler.ts` and `app/admin/sync/page.tsx`, plus a +`public/logos/route53.svg` tile asset. + + + +@$HOME/.claude/get-shit-done/workflows/execute-plan.md +@$HOME/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-PATTERNS.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-CONTEXT.md +@.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-02-SUMMARY.md + + + + +lib/services/sync-scheduler.ts: + line 25: sync_type: 'incremental' | 'full' | 'veeam-incremental' | ... | 'phishing-sweep'; + line 147: sync_schedules table DDL — sync_type VARCHAR(30) NOT NULL (no CHECK constraint; + 'route53-incremental' is 19 chars and fits) + line 180: const defaultSchedules = [ ... ] // entries: { id, name, description, cron_expression, sync_type, is_enabled } + line 312: for (const schedule of defaultSchedules) { INSERT INTO sync_schedules ... ON CONFLICT DO NOTHING } + line 413+: dispatch chain — if (config.sync_type === 'veeam-incremental') { ... } else if (...) + line 478-493: pax8-daily branch — the ONE branch that also checks integration_settings.disabled. + Route 53 must NOT copy that gate (D-10). + line 494-504: mimecast-sync branch — config-check-only pattern; this is the shape to copy. + +app/admin/sync/page.tsx: + lines 9-16: interface IntegrationCard { id, category, product, description, href, logo, color } + lines 20-29: const INTEGRATIONS: IntegrationCard[] = [ ... 'pax8' entry is last ] + lines 32-39: COLOR_MAP — available keys: red, green, blue, orange, purple, gray (no others) + line 266: const colors = COLOR_MAP[intg.color]; // an unmapped color yields undefined + + +lib/services/route53-sync-service.ts: + getRoute53SyncService(): Route53SyncService + #fullSync(triggeredBy?): Promise + #incrementalSync(triggeredBy?): Promise + + +lib/services/route53-factory.ts: + isRoute53Configured(): boolean + + + + + + + Task 1: Add route53 sync types, schedules, and dispatch branches to the scheduler + lib/services/sync-scheduler.ts + + - lib/services/sync-scheduler.ts (read the full file — the sync_type union at line 25, the sync_schedules DDL at line 147, defaultSchedules at lines 180-311, the seed loop at line 312, and the whole dispatch chain from line 413 onward) + - lib/services/route53-sync-service.ts (getRoute53SyncService — plan 24-02) + - lib/services/route53-factory.ts (isRoute53Configured — plan 24-01) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-PATTERNS.md (sync-scheduler section — the exact branch shape, and the explicit instruction NOT to copy PAX8's disable gate) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-CONTEXT.md (D-10, D-11) + + +Modify `lib/services/sync-scheduler.ts` in three places. + +1. Line 25 `sync_type` union: append `| 'route53-incremental' | 'route53-full'` to the + existing pipe-delimited string union. Do not reformat the rest of the line. + +2. `defaultSchedules` array (starts line 180): append two entries matching the exact object + shape of the surrounding `veeam-incremental` / `veeam-full` entries at lines 198-221: + - id `route53-incremental`, name `Route 53 Incremental Sync`, description + `Syncs AWS Route 53 hosted zones and records every 15 minutes`, cron_expression + `*/15 * * * *`, sync_type `route53-incremental`, is_enabled `false`. + - id `route53-full`, name `Route 53 Full Sync`, description + `Full AWS Route 53 zone and record reconciliation daily at 4:00 AM`, cron_expression + `0 4 * * *`, sync_type `route53-full`, is_enabled `false`. + `is_enabled: false` matches every newly-introduced integration in this file — an operator + enables them from `/admin` after confirming credentials. The `0 4 * * *` slot avoids the + known collisions at 2:00 AM (`veeam-full`, `qbo-sync-2am`, `mimecast-sync`) and 3:00 AM + (`weekly-full`'s `0 3 * * 0`). Enumerate every `cron_expression` in the live array before + committing and pick the next free hour if 4:00 AM is now occupied. + The seed loop at line 312 uses `ON CONFLICT DO NOTHING`, so existing deployments pick + these up without overwriting operator-modified rows. + +3. Dispatch chain (line 413 onward): add two `else if` branches following the + `mimecast-sync` config-check-only shape at lines 494-504, NOT the `pax8-daily` shape at + lines 478-493: + - `else if (config.sync_type === 'route53-incremental')` — dynamically + `await import('@/lib/services/route53-factory')` for `isRoute53Configured`; if false, + log `[SCHEDULER] Skipping route53-incremental — Route 53 not configured` and return; + otherwise dynamically `await import('@/lib/services/route53-sync-service')` and call + `getRoute53SyncService().incrementalSync('scheduled')`. + - `else if (config.sync_type === 'route53-full')` — same shape, calling + `fullSync('scheduled')`. + Use dynamic `import()` in both branches (as every other branch does) so the sync service + module is not eager-loaded at scheduler-import time. The scheduler self-initializes as a + side effect of its first server-side import, and a top-level import here would pull the + AWS SDK into every server module graph. + +CRITICAL (D-10): do NOT add an `integration_settings.disabled` query to either branch. PAX8 +is the codebase's sole exception where disabling also blocks sync; CONTEXT.md D-10 states +Route 53 explicitly does not join that list. Add a short comment above the first Route 53 +branch recording this, so a future reader does not "fix" the apparent inconsistency with the +PAX8 branch sitting a few lines above. + +Confirm `sync_schedules.sync_type` at line 147 is `VARCHAR(30)` with no CHECK constraint +before relying on the new values fitting — `route53-incremental` is 19 characters. + + + npx tsc --noEmit --pretty && test $(grep -c "route53-incremental" lib/services/sync-scheduler.ts) -ge 3 && test $(grep -c "route53-full" lib/services/sync-scheduler.ts) -ge 3 && test $(grep -A12 "config.sync_type === 'route53" lib/services/sync-scheduler.ts | grep -c integration_settings) -eq 0 && echo PASS + + + - `route53-incremental` and `route53-full` each appear at least 3 times in `lib/services/sync-scheduler.ts` (union, defaultSchedules, dispatch) + - Both new `defaultSchedules` entries have `is_enabled: false` + - Neither Route 53 dispatch branch body contains `integration_settings` (D-10) — grep over the 12 lines following each branch head returns 0 + - Both dispatch branches use dynamic `await import(...)`; no top-level route53 import exists: `grep -c "^import.*route53" lib/services/sync-scheduler.ts` returns 0 + - The `route53-full` cron hour differs from every other daily `cron_expression` hour in `defaultSchedules` — verified by enumerating the array + - A comment above the Route 53 branches records why the PAX8 disable gate is deliberately absent + - `npx tsc --noEmit --pretty` exits 0 + + Scheduler knows both Route 53 sync types, seeds them disabled, dispatches via dynamic import, and never consults the disable toggle. + + + + Task 2: Add the Route 53 tile to /admin/sync + app/admin/sync/page.tsx, public/logos/route53.svg + + - app/admin/sync/page.tsx (read the full file — the IntegrationCard interface at lines 9-16, the INTEGRATIONS array at lines 20-29, COLOR_MAP at lines 32-39, and the tile render at line ~266 to confirm how `logo` is consumed) + - public/logos/ directory listing (confirm the existing asset naming convention — every current asset is `.ico`) + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-PATTERNS.md (admin sync page section) + + +Create `public/logos/route53.svg` — a small hand-authored SVG with `viewBox="0 0 32 32"`, +`xmlns="http://www.w3.org/2000/svg"`, using the AWS orange `#FF9900`, containing a simple +globe-or-DNS-node glyph built from primitive shapes only. It must contain no `