diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 4aaa2fa..4de2126 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -677,6 +677,42 @@ 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/7 plans complete + +Plans: +**Wave 1** + +- [x] 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)* + +**Wave 2** *(blocked on Wave 1 completion)* + +- [x] 24-02-PLAN.md — Route53SyncService: zone/record mirror sync with pagination, soft-delete, and `sync_detected_drift` change history *(wave 2)* +- [x] 24-03-PLAN.md — Record validation (D-01 NS/SOA allowlist, AWS error sanitizer) + pending/committed/failed audit lifecycle persistence *(wave 2)* +- [x] 24-04-PLAN.md — Health check: auth probe + D-12 live NS-delegation comparison, registered in integration-health *(wave 2)* + +**Wave 3** *(blocked on Wave 2 completion)* + +- [x] 24-05-PLAN.md — `/api/route53/*` read routes, sync trigger, and CRUD write routes with `requireAdmin()` gating *(wave 3)* +- [x] 24-06-PLAN.md — Scheduler entries (`route53-incremental`, `route53-full`) + `/admin/sync` tile *(wave 3)* + +**Wave 4** *(blocked on Wave 3 completion)* + +- [x] 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/STATE.md b/.planning/STATE.md index 6d7d0b3..27acac9 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -2,15 +2,15 @@ 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" +status: completed +stopped_at: Phase 24 complete +last_updated: "2026-08-06T03:18:19.527Z" +last_activity: 2026-08-06 -- Phase 24 marked complete progress: - total_phases: 9 - completed_phases: 9 - total_plans: 30 - completed_plans: 30 + total_phases: 10 + completed_phases: 10 + total_plans: 37 + completed_plans: 37 percent: 100 --- @@ -21,14 +21,14 @@ progress: See: .planning/PROJECT.md (updated 2026-07-14) **Core value:** A manager/security operator can see every phishing/spam report ticket automatically triaged, deduplicated into campaigns, and classified — with any destructive remediation gated behind explicit human approval. -**Current focus:** Milestone complete +**Current focus:** Phase 24 complete — AWS Route 53 DNS sync live in production ## Current Position -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) +Phase: 24 (aws-route-53-dns-sync-track-changes-crud-operations-full-aud) — COMPLETE +Plan: 7 of 7 +Status: Phase 24 complete +Last activity: 2026-08-06 -- Phase 24 marked complete ## Performance Metrics @@ -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. @@ -132,6 +134,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 @@ -162,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 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..921d794 --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-PLAN.md @@ -0,0 +1,382 @@ +--- +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 — route53_zones / route53_records / route53_record_history / route53_audit_log (D-05: dedicated tables, not the phishing pipeline's shared audit_events) — 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-08: Retention is unbounded by design — no purge job, no TTL, and no DELETE statement against route53_record_history or route53_audit_log anywhere in this phase, matching existing Pulse convention" + - "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-01-SUMMARY.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-SUMMARY.md new file mode 100644 index 0000000..6bf830b --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-01-SUMMARY.md @@ -0,0 +1,157 @@ +--- +phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud +plan: 01 +subsystem: aws-route53 +tags: [route53, aws-sdk, migration, factory, foundation] +dependency-graph: + requires: [] + provides: + - "@aws-sdk/client-route-53 dependency" + - "route53_zones / route53_records / route53_record_history / route53_audit_log schema" + - "lib/types/route53.ts contracts" + - "lib/services/route53-factory.ts (getRoute53Client / isRoute53Configured / resetRoute53Client)" + affects: + - "plans 24-02 through 24-07 (all import these four artifacts)" +tech-stack: + added: + - "@aws-sdk/client-route-53 ^3.1104.0" + patterns: + - "factory + isConfigured() singleton (mirrors veeam-factory.ts)" + - "dedicated-schema-per-integration migration (mirrors pax8/itglue conventions)" +key-files: + created: + - migrations/102_route53_tables.sql + - lib/types/route53.ts + - lib/services/route53-factory.ts + - lib/services/route53-factory.test.ts + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/deferred-items.md + modified: + - package.json + - package-lock.json + - CLAUDE.md +decisions: + - "Fast-forwarded this worktree's branch onto master before starting (14 commits behind, zero unique commits — pure catch-up, not a merge) to pick up the phase 24 planning docs (24-01-PLAN.md etc.) that were committed to master after this worktree was created." +metrics: + duration: "Tasks 1-3 complete, checkpoint resolved" + completed: "2026-08-05" +--- + +# Phase 24 Plan 1: Route 53 Foundation Summary + +Installed the official AWS Route 53 SDK client, created the dedicated four-table +Postgres schema (mirror tables + change-history ledger + audit ledger), defined +shared camelCase TypeScript types, and built the `route53-factory.ts` credential +factory following the exact `isConfigured()` + singleton shape every other +Pulse integration uses — TDD RED/GREEN cycle, 7/7 tests passing. + +## What Was Built + +**Task 1 — AWS SDK + migration:** +- `npm install @aws-sdk/client-route-53` (official `aws/aws-sdk-js-v3` package, confirmed `[OK]` in 24-RESEARCH.md's package legitimacy audit) +- `migrations/102_route53_tables.sql` — four tables: + - `route53_zones` (mirror, PK = AWS hosted zone id with `/hostedzone/` prefix stripped) + - `route53_records` (mirror, PK = composite `record_key` string since Route 53 recordsets have no native id) + - `route53_record_history` (D-06 append-only change ledger, `source` CHECK constrained to `pulse_crud`/`sync_detected_drift`) + - `route53_audit_log` (D-03/D-07 append-only attempt log including failures, `status` CHECK constrained to `pending`/`committed`/`failed`, `zone_id` deliberately not an FK) + - Seed row `INSERT INTO integration_settings (key, disabled) VALUES ('route53', false)` (D-10, display-only toggle — extends the existing seed list rather than editing the committed `081_integration_settings.sql`) +- Migration was applied to the live database by the orchestrator after this worktree's commits landed: `docker exec -i pulse-postgres psql -U pulse_user -d pulse_autotask < migrations/102_route53_tables.sql`. All four tables confirmed present via `\dt route53_*`. + +**Task 2 — Types + factory (TDD):** +- RED: `lib/services/route53-factory.test.ts` written first, confirmed failing (module didn't exist) +- GREEN: `lib/types/route53.ts` (camelCase interfaces/unions per the plan's `` contract) + `lib/services/route53-factory.ts` (singleton factory, no explicit `credentials:` option passed to `Route53Client` — relies on the AWS SDK default credential chain reading `AWS_ACCESS_KEY_ID`/`AWS_SECRET_ACCESS_KEY` from `process.env`, per 24-RESEARCH.md Pitfall 1) +- `CLAUDE.md` integration table updated with the `AWS Route 53` / `AWS_*` exception row +- 7/7 test assertions pass; `npx tsc --noEmit --pretty` exits clean + +## Deviations from Plan + +### Auto-fixed Issues + +None — plan executed exactly as written for Tasks 1-2. + +### Environment Note (not a deviation, a pre-condition) + +This worktree's branch (`worktree-agent-aa690b9b15c5b0f8e`) was created before the +phase 24 planning commits landed on `master` — it had zero unique commits and was +purely 14 commits behind. Fast-forwarded (`git merge --ff-only master`) to pick up +`24-01-PLAN.md` and related planning docs before execution could start. This was a +clean fast-forward (no merge, no conflicts, nothing discarded). + +### Out-of-Scope Discovery (logged, not fixed) + +`npm test` (full suite, run as part of Task 2 verification) surfaced 2 pre-existing +failures in `lib/services/analyzer/itglue-search.test.ts`, unrelated to this plan — +neither that file nor `itglue-search.ts` were touched by Tasks 1-2. Logged to this +phase's `deferred-items.md` per the scope boundary rule rather than fixed. + +## Checkpoint Status: RESOLVED + +Task 3's four verification items, confirmed by the orchestrator against the live +`pulse-app` / `pulse-postgres` containers with the developer: + +1. **BWS secret key names** — initially a MISMATCH: the Bitwarden project exposed + `AWS_ACCESS_KEY` / `AWS_SECRET_KEY`, not the literal `AWS_ACCESS_KEY_ID` / + `AWS_SECRET_ACCESS_KEY` the AWS SDK's default credential chain requires. No + `AWS_REGION` key exists (factory defaults to `us-east-1`, as designed). **Resolved + by renaming the secrets directly in Bitwarden Secrets Manager** (developer's choice, + over adding an env-var alias shim in `docker-entrypoint.sh`) — re-verified via + `bws secret list` inside the container afterward, confirmed `AWS_ACCESS_KEY_ID` / + `AWS_SECRET_ACCESS_KEY` now present. **No change needed to + `lib/services/route53-factory.ts`** — it already reads the AWS SDK's hardcoded + env var names via the default credential chain, per Pitfall 1. +2. **Credentials reach the container** — confirmed present in the actual Node process + environment (`/proc//environ`, presence-only check, count=2). Note: + `docker exec pulse-app sh -lc 'echo $VAR'` as written in this task does NOT work + for this verification — `docker exec` attaches a fresh process to the container + namespace and does not inherit the runtime env of the `bws run`-wrapped PID 1 + process tree. Verified via the actual node process's `/proc//environ` instead. +3. **Outbound DNS egress** — `EGRESS-OK`: `1.1.1.1`/`8.8.8.8` UDP/53 reachable from + inside the container. Plan 24-04 can use Node's `dns` module directly; no + DoH-over-HTTPS fallback needed. +4. **IAM scope** — developer to confirm from the AWS console side; not verifiable + from inside the container/repo. Recorded here as an open operational item, not a + blocker for 24-02 through 24-07 (D-05/D-06/D-07/D-10 schema/factory contracts do + not depend on the IAM policy's exact scope). + +### Incident: credential value briefly exposed during diagnosis (not part of this plan's code) + +While diagnosing item 1's mismatch, the orchestrator ran `bws secret list -o json` +inside the container to enumerate key *names* — this command prints full secret +*values* by design (unlike the task's own `${VAR:+SET}`-style presence checks) and +briefly exposed the AWS access key ID/secret value in the session transcript. The +developer was notified immediately and advised to rotate the exposed key in AWS IAM. +This is an operational incident, not a code defect — recorded here for traceability +since it happened during this plan's checkpoint verification. No repository file +contains the exposed values. + +### Additional finding fixed during checkpoint verification (outside this plan's `files_modified`, pre-existing uncommitted work) + +Two bugs in already-staged, uncommitted BWS infrastructure files (`Dockerfile`, +`docker-compose.yml`, `docker-entrypoint.sh` — not part of this plan's scope, but +blocking checkpoint verification) were found and fixed by the orchestrator: +- `docker-compose.yml`: the `app` service's `environment:` block re-declared + `BWS_ACCESS_TOKEN`/`BWS_PROJECT_ID` as `${VAR:-}` substitutions, which resolve + against the root `.env` (not `.env.local`) and silently overrode the real token + with an empty string. Fixed by removing the redundant re-declaration. +- `Dockerfile`: the generated `bws` CLI config only set `state_dir`, but bws 2.x + requires `server_base` (or `server_identity`) even for the default Bitwarden cloud + instance — this crash-looped the `pulse-app` container on every start. Fixed by + adding `server_base = "https://vault.bitwarden.com"` to the generated config. + +Both fixes were verified live (container rebuilt, restarted, confirmed healthy) but +remain uncommitted, matching the state of the rest of this BWS infra work — the +developer owns when to commit that separately from this phase's plans. + +## Self-Check: PASSED + +All created files confirmed present: +- FOUND: migrations/102_route53_tables.sql +- FOUND: lib/types/route53.ts +- FOUND: lib/services/route53-factory.ts +- FOUND: lib/services/route53-factory.test.ts +- FOUND: .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/deferred-items.md + +All commits confirmed present in `git log`: +- b9df27b feat(24-01): install AWS Route 53 SDK and create dedicated schema migration +- 4dd9d5d test(24-01): add failing test for Route 53 credential factory +- 210f84d feat(24-01): implement Route 53 credential factory +- fadae68 docs(24-01): log pre-existing itglue-search test failures as out-of-scope 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-02-SUMMARY.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-02-SUMMARY.md new file mode 100644 index 0000000..92ab85c --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-02-SUMMARY.md @@ -0,0 +1,92 @@ +--- +phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud +plan: 02 +subsystem: aws-route53 +tags: [route53, sync, drift-detection, soft-delete, sync_history] +dependency-graph: + requires: + - "lib/services/route53-factory.ts (getRoute53Client / isRoute53Configured)" + - "lib/types/route53.ts (Route53Record, Route53SyncResult, Route53HistorySource)" + - "migrations/102_route53_tables.sql (route53_zones / route53_records / route53_record_history)" + provides: + - "lib/services/route53-record-key.ts (buildRecordKey, normalizeRecordSet, recordSetsEqual, classifyDrift, toHistoryPayload)" + - "lib/services/route53-sync-service.ts (Route53SyncService, getRoute53SyncService(), buildDriftHistoryRows)" + affects: + - "plan 24-05 (CRUD routes import getRoute53SyncService for post-write history/audit patterns)" + - "plan 24-06 (scheduler wiring calls fullSync()/incrementalSync())" +tech-stack: + added: [] + patterns: + - "Sync-service class shape mirrors lib/services/veeam-sync-service.ts: constructor(client?), isSyncing guard, executeSync() step loop with per-step error isolation, sync_history bookkeeping" + - "Pure drift-classification logic extracted into an injectable, unit-testable function (buildDriftHistoryRows) rather than inlined in the DB-touching sync method" +key-files: + created: + - 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 + modified: [] +decisions: + - "Combined Task 2 (sync mirror) and Task 3 (drift detection) into a single write of route53-sync-service.ts rather than writing sync-only first and retrofitting drift after — the plan's own Task 2 action already anticipated buildDriftHistoryRows()'s existence (referencing it in Task 3's read_first), so building both together avoided a throwaway intermediate version of syncRecords()." + - "Symlinked node_modules from the main repo checkout into this worktree (git-ignored, not committed) — the worktree had no node_modules of its own and @aws-sdk/client-route-53 types were required for both writing and type-checking this plan's code." +metrics: + duration: "~45 minutes, 3 tasks, 4 files" + completed: "2026-08-05" +--- + +# Phase 24 Plan 2: Route 53 Sync + Drift Detection Summary + +Built the Route 53 → Postgres mirror sync (`Route53SyncService`) with paginated +zone/record ingestion, soft-delete reconciliation, and whole-recordset drift +detection that writes `sync_detected_drift` history rows whenever a mirrored +record's live AWS value no longer matches Postgres. + +## What Was Built + +**`lib/services/route53-record-key.ts`** — pure, dependency-free helpers: +- `buildRecordKey({ zoneId, name, type, setIdentifier })` → `${zoneId}:${name}:${type}:${setIdentifier ?? ''}`, the `route53_records.record_key` primary key +- `normalizeRecordSet(rs, zoneId)` — lowercases name (preserving trailing dot), uppercases type, coerces missing TTL to `null`, sorts `resourceRecords` by value (so AWS's unordered list never produces phantom drift), passes through `aliasTarget` +- `recordSetsEqual(a, b)` — TTL + serialized resourceRecords + serialized aliasTarget comparison; both-null is equal, one-null is not +- `classifyDrift(prev, next)` → `'create' | 'update' | 'delete' | null`, returning `null` for no material difference (no-op history row avoided) +- `toHistoryPayload(ns)` — whole-recordset JSONB snapshot for `before_value`/`after_value` + +**`lib/services/route53-sync-service.ts`** — `Route53SyncService` class: +- `fullSync()` / `incrementalSync()` both delegate to `executeSync()`, which runs the same two steps in both cases (Route 53 exposes no modification cursor, so "incremental" is the same full diff run more often — documented in a code comment per D-11) +- `syncZones()` — paginates `ListHostedZonesCommand` (Marker/IsTruncated/NextMarker), strips the `/hostedzone/` prefix from each zone id, fetches `GetHostedZoneCommand` per zone for `DelegationSet.NameServers` (backs plan 24-04's D-12 NS-delegation check), upserts into `route53_zones`, then soft-deletes zones no longer returned by AWS +- `syncRecords()` — for each live zone, paginates `ListResourceRecordSetsCommand` (StartRecordName/Type/Identifier), normalizes each recordset, loads the zone's current mirror rows, computes drift via `buildDriftHistoryRows()` **before** upserting (so the pre-sync state is still available), inserts drift history rows in a transaction, then upserts records and soft-deletes anything no longer present +- `buildDriftHistoryRows(prevByKey, nextByKey, zoneId)` — exported pure function; unions both keyed maps, classifies drift per key via `classifyDrift`, skips `null` (no-change) keys, builds `DriftHistoryRow[]` with whole-recordset before/after payloads +- Initial-import guard: if a zone's mirror had zero rows before this sync, drift history generation is skipped entirely for that zone (logged once) — prevents flooding the append-only ledger with meaningless `create` rows on first import +- `sync_history` bookkeeping: `entity_type='route53'`, `sync_type` written as the literal `'full'`/`'incremental'` (matching the table's CHECK constraint), catastrophic-failure catch marks the row `status='failed'` +- All error logging uses `error instanceof Error ? error.message : String(error)` — never the raw AWS SDK error object (T-24-03: avoids leaking `$metadata`/request headers into logs) +- No `integration_settings` check anywhere in the file (D-10: Route 53's disable toggle is display-only, must never gate sync) +- `getRoute53SyncService()` module-level singleton, following `pax8-sync-service.ts`'s lazy-instance pattern, exported for plans 24-05/24-06 + +## Deviations from Plan + +None — plan executed as written. Tasks 2 and 3 were combined into a single write of `route53-sync-service.ts` (see Decisions above) since the file only exists once either way; both tasks' acceptance criteria are independently verified below. + +## Verification + +- `npx vitest run lib/services/route53-record-key.test.ts` — 16 tests passed +- `npx vitest run lib/services/route53-sync-service.test.ts` — 6 tests passed +- `npx tsc --noEmit --pretty` — exits 0 +- `npm test` (full suite) — 489 passed, 2 pre-existing failures in `lib/services/analyzer/itglue-search.test.ts`, unrelated to this plan's files and already logged in `deferred-items.md` from plan 24-01 (neither `itglue-search.ts` nor its test were touched by this plan) +- Acceptance-criteria greps all confirmed: `integration_settings` count 0, `entity_type` present, both pagination loops present (`IsTruncated` count 2), both soft-delete statements present (`is_deleted = true` count 2), `sync_detected_drift` literal present, `changed_by_user_id` NULL for drift rows, initial-import guard present (`prevByKey.size === 0`), `buildDriftHistoryRows` call precedes the `INSERT INTO route53_records` statement in source order + +## Environment Note (not a code deviation) + +This worktree had no `node_modules` directory. Symlinked it from the main +repo checkout (`ln -s /opt/stacks/pulse/node_modules ./node_modules`) so +`@aws-sdk/client-route-53` types and `vitest`/`tsc` were available. This is a +local filesystem convenience, not a git-tracked change — `node_modules` is +`.gitignore`d and no commit references it. + +## Self-Check: PASSED + +- `lib/services/route53-record-key.ts` — FOUND +- `lib/services/route53-record-key.test.ts` — FOUND +- `lib/services/route53-sync-service.ts` — FOUND +- `lib/services/route53-sync-service.test.ts` — FOUND +- Commit `c18271d` (Task 1) — FOUND in `git log --oneline --all` +- Commit `d8c0912` (Task 2) — FOUND in `git log --oneline --all` +- Commit `0acf1fa` (Task 3) — FOUND in `git log --oneline --all` 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-03-SUMMARY.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-03-SUMMARY.md new file mode 100644 index 0000000..70c159a --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-03-SUMMARY.md @@ -0,0 +1,131 @@ +--- +phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud +plan: 03 +subsystem: route53-crud-write-persistence +tags: [route53, dns, crud, audit, validation, security] +dependency-graph: + requires: ["24-01"] + provides: + - "lib/services/route53-record-validation.ts (WRITABLE_RECORD_TYPES, validateRecordWrite, sanitizeAwsError)" + - "lib/services/route53-write-persistence.ts (createPendingAuditLog, markAuditCommitted, markAuditFailed, insertPulseCrudHistory, upsertMirrorRecord, softDeleteMirrorRecord, loadMirrorRecord)" + affects: + - "plan 24-05 (CRUD routes will import both modules directly)" +tech-stack: + added: [] + patterns: + - "pending -> committed/failed audit lifecycle (mirrors lib/services/analyzer/asset-audit/persistence.ts)" + - "closed allowlist validation (mirrors app/api/analyzer/itglue/.../apply/route.ts credential-field blocklist pattern, inverted to an allowlist)" +key-files: + created: + - 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 + modified: + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/deferred-items.md +decisions: + - "ValidateRecordWriteInput fields are optional (name?, type?) rather than required, since the whole point of the validator is to accept untrusted/arbitrary request-body shapes at runtime — TypeScript's structural typing otherwise blocked constructing test payloads that omit a field to prove the runtime check fires" +metrics: + duration_minutes: 25 + tasks_completed: 2 + files_created: 4 + files_modified: 1 + test_assertions_added: 32 + completed: 2026-08-06 +--- + +# Phase 24 Plan 03: Record Validation and Write-Persistence Summary + +Server-side D-01 record-type allowlist validator + AWS error sanitizer, and the pending +-> committed/failed audit lifecycle with pulse_crud history persistence that the plan +24-05 CRUD routes will depend on. + +## What Was Built + +**`lib/services/route53-record-validation.ts`** +- `WRITABLE_RECORD_TYPES`: frozen array of exactly `['A', 'AAAA', 'CNAME', 'MX', 'TXT', 'SRV']` — + a closed allowlist. `NS`/`SOA` appear nowhere in this array; they only appear inside the + rejection branch's `ZONE_DELEGATION_TYPES` set and its explicit reason message. +- `validateRecordWrite(input)`: validates `name` (trim/lowercase/trailing-dot normalization), + `type` (case-insensitive uppercase match against the allowlist, explicit delegation-aware + rejection reason for NS/SOA, generic rejection for any other unlisted type such as CAA/DS), + `ttl` (integer 0..2147483647, default 300), and `resourceRecords` (non-empty array, each + entry a non-empty string value, capped at 100 entries — T-24-12 DoS guard). No AWS SDK or + Postgres import — confirmed by `grep -c "@aws-sdk\|postgres-client"` returning 0. +- `sanitizeAwsError(err)`: redacts `AKIA`-prefixed access key ids, `arn:aws:*` substrings, and + 12-digit AWS account ids, then truncates to 500 characters. Never throws, even on + non-Error input. +- 23 test assertions cover every `` bullet including case-insensitivity (`NS`/`ns`), + the closed-allowlist rejection of an unlisted type (`CAA`, `DS`), TTL boundary values, and + the sanitizer stripping both an example AWS key id and ARN from a single error message. + +**`lib/services/route53-write-persistence.ts`** +- `createPendingAuditLog` / `markAuditCommitted` / `markAuditFailed`: the three-function + pending -> committed/failed shape mirrored from + `lib/services/analyzer/asset-audit/persistence.ts`. `createPendingAuditLog` must be awaited + before any AWS `ChangeResourceRecordSetsCommand` is constructed by the (future) CRUD routes. + `markAuditFailed` always routes the error through `sanitizeAwsError` — never a raw error + object or `JSON.stringify(err)`. +- `insertPulseCrudHistory`: writes a `source='pulse_crud'` row to `route53_record_history`. + Documented directly above the function that it must only be called after + `markAuditCommitted` — a failed AWS attempt gets an audit row but no history row. +- `upsertMirrorRecord` / `softDeleteMirrorRecord`: best-effort refresh/soft-delete of the + `route53_records` mirror after a committed write. Wrapped in try/catch, logged with a + `[ROUTE53-WRITE]` prefix via `sanitizeAwsError`, never thrown — the AWS write already + succeeded and the next incremental sync reconciles regardless. Confirmed + `grep -c 'DELETE FROM route53_records'` returns 0 — soft-delete only (D-08). +- `loadMirrorRecord`: manual snake_case -> camelCase transform (no ORM, per CLAUDE.md) of the + current mirror row, supplying `before_value` and the exact TTL/value set a Route 53 DELETE + needs to match. +- 9 test assertions against a mocked `postgresClient` (same `vi.mock` style as + `pax8-sync-service.test.ts`) verify: the pending INSERT contains `'pending'`, the committed/ + failed UPDATEs contain their respective status literals, `markAuditFailed`'s bound + parameters never contain a raw AWS key id, `insertPulseCrudHistory` binds the literal + `'pulse_crud'`, `softDeleteMirrorRecord` never issues `DELETE FROM`, both mirror helpers + resolve (never throw) even when the underlying query rejects, and `loadMirrorRecord` + correctly transforms a found row / returns `null` when absent. + +## Verification + +- `npx vitest run lib/services/route53-record-validation.test.ts lib/services/route53-write-persistence.test.ts` — 32/32 passed +- `npx tsc --noEmit --pretty` — exits 0 +- `npm test` (full suite) — 499/501 passed; 2 pre-existing failures in + `lib/services/analyzer/itglue-search.test.ts`, unrelated to this plan (see Deferred Issues) +- `grep -c "'NS'\|'SOA'" lib/services/route53-record-validation.ts` — 1 (only inside the + rejection Set/reason, never inside `WRITABLE_RECORD_TYPES`) +- `grep -c "@aws-sdk\|postgres-client" lib/services/route53-record-validation.ts` — 0 +- `grep -c 'DELETE FROM route53_records' lib/services/route53-write-persistence.ts` — 0 +- `grep -q 'sanitizeAwsError' lib/services/route53-write-persistence.ts` — found +- `grep -c "'pulse_crud'" lib/services/route53-write-persistence.ts` — 2 + +## Deviations from Plan + +None — plan executed exactly as written, aside from one non-substantive typing adjustment: + +**1. [Rule 3 - blocking issue] `ValidateRecordWriteInput` fields made optional** +- **Found during:** Task 1, `npx tsc --noEmit --pretty` +- **Issue:** The plan's signature `validateRecordWrite(input: { name: unknown; type: unknown; ... })` + requires the `name`/`type` properties to be present (even though typed `unknown`). + A behavior test intentionally omits `name` to prove the runtime "missing name" rejection + fires — TypeScript's structural typing blocked constructing that test payload. +- **Fix:** Made `name?: unknown` and `type?: unknown` optional in the internal + `ValidateRecordWriteInput` interface. Runtime behavior is unchanged (the function still + checks `typeof input.name !== 'string'`, which already covers `undefined`). +- **Files modified:** `lib/services/route53-record-validation.ts` +- **Commit:** 4be4a19 + +## Deferred Issues + +None specific to this plan's own code. Two pre-existing, unrelated `npm test` failures in +`lib/services/analyzer/itglue-search.test.ts` were re-observed during full-suite verification +and logged (not fixed, out of scope) in `deferred-items.md` under both the original Plan 24-01 +entry and a new Plan 24-03 entry. + +## Self-Check: PASSED + +- FOUND: lib/services/route53-record-validation.ts +- FOUND: lib/services/route53-record-validation.test.ts +- FOUND: lib/services/route53-write-persistence.ts +- FOUND: lib/services/route53-write-persistence.test.ts +- FOUND commit: 4be4a19 +- FOUND commit: 8b5e926 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-04-SUMMARY.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-04-SUMMARY.md new file mode 100644 index 0000000..fe48c5f --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-04-SUMMARY.md @@ -0,0 +1,182 @@ +--- +phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud +plan: 04 +subsystem: aws-route53 +tags: [route53, integration-health, dns, node-dns, delegation-check] +dependency-graph: + requires: + - "lib/services/route53-factory.ts (isRoute53Configured / getRoute53Client, plan 24-01)" + - "lib/types/route53.ts (plan 24-01)" + - "migrations/102_route53_tables.sql (route53_zones, plan 24-01)" + provides: + - "lib/services/route53-dns-delegation.ts (normalizeNsList / compareNsDelegation / resolveLiveNs / checkAllZoneDelegations)" + - "checkRoute53() registered in lib/services/integration-health.ts" + - "HealthStatus union gained 'degraded' member" + - "IntegrationHealth gained nsDelegationMismatches? / nsDelegationErrors? fields" + affects: + - "app/api/dashboard/integration-health/route.ts (now returns a route53 entry)" + - "/admin/integrations and /status pages (consume the health list; both already fall back safely on an unrecognized status string)" +tech-stack: + added: [] + patterns: + - "dedicated dns.Resolver() instance pinned to public resolvers, never the process-global dns module (D-12, T-24-13)" + - "bounded-concurrency worker-pool batch check (checkAllZoneDelegations, T-24-14)" +key-files: + created: + - lib/services/route53-dns-delegation.ts + - lib/services/route53-dns-delegation.test.ts + modified: + - lib/services/integration-health.ts +decisions: + - "Used Node's dns module directly (Resolver + setServers(['1.1.1.1','8.8.8.8'])), not the DoH-over-HTTPS fallback — plan 24-01's checkpoint confirmed EGRESS-OK from inside the pulse-app container." + - "Chosen degraded-state HealthStatus member: added a new 'degraded' member to the union (no existing member represented 'reachable, authenticated, but a secondary check found a problem') — every existing member was either a full pass ('ok'/'unknown'), a hard failure ('auth_failed'/'unreachable'/'not_configured'), or operator-suppressed ('disabled')." + - "Implemented a local, private sanitizeAwsError()/isAwsAuthError() inside integration-health.ts instead of importing from lib/services/route53-record-validation.ts — that file is scoped to plan 24-03, which runs in a sibling parallel worktree and was not available in this isolated worktree (24-04's depends_on only lists 24-01). See Deviations below." +metrics: + duration: "~45 min, 2 tasks, TDD RED/GREEN on Task 1" + completed: "2026-08-05" +--- + +# Phase 24 Plan 4: Route 53 Integration Health + D-12 DNS Delegation Check Summary + +Added `checkRoute53()` to the existing integration-health aggregator (auth probe via +`ListHostedZonesCommand`) plus a D-12-specific extension: every synced hosted zone's +Route-53-authoritative name servers are compared against a **live public DNS lookup** +(dedicated `dns.Resolver()` pinned to `1.1.1.1`/`8.8.8.8`, never the process-global +resolver) and a mismatch degrades the reported health to a new `'degraded'` +`HealthStatus` member — no manually-maintained "expected NS" field anywhere. + +## What Was Built + +**Task 1 — `lib/services/route53-dns-delegation.ts` (TDD RED/GREEN):** +- Pure half, fully unit-tested (13/13 assertions, no network I/O in tests): + - `normalizeNsList(input: unknown): string[]` — lowercase, strip trailing dot, + de-dupe, sort; `[]` for `null`/`undefined`/non-array input. + - `compareNsDelegation(authoritative, live)` — set-diff mismatch detection. + Empty authoritative list is treated as unjudgeable (`mismatch: false`, no false + alarm); a non-empty authoritative list with an empty live answer is a real + delegation problem (`mismatch: true`). +- I/O half (not unit-tested per plan instruction — network calls are flaky in CI; + covered by manual verification in 24-VALIDATION.md): + - `resolveLiveNs(domain, timeoutMs=5000)` — constructs `new Resolver()` (callback + API from `'dns'`) and calls `.setServers(['1.1.1.1','8.8.8.8'])` **on that + instance only**. The process-global `dns.setServers()` is never called anywhere + in this file (T-24-13) — verified by grep in the acceptance criteria. + - `checkAllZoneDelegations(zones, opts?)` — bounded-concurrency (default 5) worker + pool over the zone list; a lookup error yields `{ mismatch: false, error }` (an + unreachable resolver is an infra problem, not delegation drift — T-24-14); zones + whose `authoritativeNameServers` normalizes to `[]` are skipped. + +**Task 2 — `checkRoute53()` registered in `lib/services/integration-health.ts`:** +- `IntegrationHealth` gained two optional fields: `nsDelegationMismatches?: string[] | null` + and `nsDelegationErrors?: string[] | null` (kept separate from `error`, per 24-PATTERNS.md). +- `checkRoute53()` placed next to `checkDattoRmm()`: config gate via `isRoute53Configured()` + → `getRoute53Client().send(new ListHostedZonesCommand({ MaxItems: 1 }))` auth probe, + timed for `latencyMs`. Auth errors (`InvalidClientTokenId`, `SignatureDoesNotMatch`, + `AccessDenied`, `UnrecognizedClientException`, or HTTP 401/403 via `$metadata.httpStatusCode`) + map to `status: 'auth_failed'`; any other error maps to `'unreachable'`. Both branches + redact the error through a local `sanitizeAwsError()` before it reaches `IntegrationHealth.error`. +- D-12 delegation step: `SELECT id, name, authoritative_name_servers FROM route53_zones + WHERE is_deleted = false ORDER BY name LIMIT 50`, fed to `checkAllZoneDelegations()`. + A non-empty mismatch list downgrades `status` from `'ok'` to the new `'degraded'` + member and sets a summary `error` string (`'NS delegation mismatch for N zone(s): ...'`, + noting truncation if the 50-zone cap was hit). The entire delegation step is wrapped + in its own try/catch — a Postgres failure or blocked resolver degrades to + `nsDelegationErrors` and leaves the auth-probe status untouched, never throwing out + of `checkIntegrationHealth()`'s `Promise.all` (T-24-16). +- Registered `checkRoute53(),` as a bare (unwrapped) entry in the `Promise.all` array + alongside `checkAutotask()` / `checkDattoRmm()` / `checkItglue()` / `checkS1()`. +- No changes to `applyDisableOverlay()` — it already keys off `item.key` generically, + so `'route53'` is covered automatically (D-10, display-only; `integration_settings` + grep count unchanged from before this task). +- **Bug fix (Rule 1):** `summarize()`'s status-bucketing `if/else if` chain didn't + account for the new `'degraded'` status — it would have silently fallen through + uncounted (not `ok`, not `failed`, not `notConfigured`), breaking the invariant that + bucket counts sum to `total`. Added `'degraded'` to the `failed` bucket (and thus + `hasIssues`) alongside `'auth_failed'`/`'unreachable'`. +- **Type fix (Rule 3):** the plan's example passed `MaxItems: '1'` (string) to + `ListHostedZonesCommand`; this SDK version (`@aws-sdk/client-route-53` ^3.1104.0) + types `MaxItems` as `number`. Changed to `MaxItems: 1`. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] `summarize()` didn't bucket the new `'degraded'` status** +- Found during: Task 2, after adding `'degraded'` to `HealthStatus`. +- Fix: added `'degraded'` to the existing `failed`-bucket branch. +- Files modified: `lib/services/integration-health.ts` +- Commit: ea04672 + +**2. [Rule 3 - Blocking issue] `ListHostedZonesCommand({ MaxItems: '1' })` failed `tsc`** +- Found during: Task 2 verification (`npx tsc --noEmit`). +- Issue: this SDK version types `MaxItems` as `number`, not `string` as the plan's + action text described. +- Fix: `MaxItems: 1`. +- Files modified: `lib/services/integration-health.ts` +- Commit: ea04672 + +**3. [Rule 3 - Blocking issue] `lib/services/route53-record-validation.ts` (plan 24-03) + does not exist in this worktree** +- Found during: Task 2, reading `` / `` which reference + `sanitizeAwsError` from that file. +- Why: this plan's `depends_on` frontmatter lists only `24-01`; plan 24-03 (which owns + `sanitizeAwsError`) runs concurrently in a sibling parallel worktree in this same wave + and is not merged into this worktree's history. +- Fix: implemented a local, private `sanitizeAwsError()` + `isAwsAuthError()` pair + inside `lib/services/integration-health.ts`, using the **identical redaction rules** + spec'd in 24-03-PLAN.md (`AKIA[0-9A-Z]{16}` → `[redacted-key-id]`, `arn:aws:[^\s"']+` + → `[redacted-arn]`, 12-digit account ids → `[redacted-account-id]`, truncate to 500 + chars) so behavior is consistent regardless of which implementation ships. Documented + inline with a NOTE comment pointing at this deviation. +- **Follow-up for a human/future plan:** once 24-03 lands on `master`, the local copy in + `integration-health.ts` should be replaced with an import from + `@/lib/services/route53-record-validation` to keep a single source of truth — flagging + this explicitly since it is a cross-plan duplication introduced by parallel worktree + execution, not by design. +- Files modified: `lib/services/integration-health.ts` +- Commit: ea04672 + +### Verification Note (not a deviation) + +The plan's Task 2 acceptance criteria includes an optional curl check against the running +`pulse-app` container's `/api/dashboard/integration-health` route. That container +(confirmed via `docker inspect`) has no source-code volume mount — it runs a pre-built +standalone image from before this plan's commits, and the route additionally redirects +unauthenticated requests (307) per `middleware.ts`. Live end-to-end verification against +the running container was therefore not performed in this worktree; `npx tsc --noEmit`, +`npx vitest run lib/services/route53-dns-delegation.test.ts` (13/13 passing), and the full +`npm test` suite (480/482 passing — the 2 failures are pre-existing/unrelated, see below) +are the verifications actually run. A container rebuild + authenticated curl is left to +the orchestrator/human at merge time if desired. + +### Out-of-Scope Discovery (logged, not fixed) + +`npm test` (full suite) surfaced the same 2 pre-existing failures in +`lib/services/analyzer/itglue-search.test.ts` already logged in this phase's +`deferred-items.md` by plan 24-01. Neither that file nor `itglue-search.ts` were touched +by this plan. + +## Self-Check: PASSED + +All created/modified files confirmed present: +- FOUND: lib/services/route53-dns-delegation.ts +- FOUND: lib/services/route53-dns-delegation.test.ts +- FOUND: lib/services/integration-health.ts (modified) + +All commits confirmed present in `git log`: +- 7396f07 test(24-04): add failing test for NS normalization and delegation comparison +- 06ebae5 feat(24-04): implement NS normalization and delegation-comparison module +- ea04672 feat(24-04): register checkRoute53() in the integration health aggregator + +## TDD Gate Compliance + +Task 1 followed RED → GREEN: `test(24-04)` commit (7396f07) precedes the `feat(24-04)` +implementation commit (06ebae5); no REFACTOR commit was needed (implementation matched +the test contract on first pass). Task 2 is `type="auto"` without `tdd="true"` per the +plan, so no RED/GREEN gate applied there — verified with `tsc` + full `npm test` instead. + +## Threat Flags + +None beyond what's already covered by this plan's own `` (T-24-13, T-24-14, +T-24-03, T-24-15, T-24-16 — all addressed as designed, see "What Was Built" above). No new +network endpoints, auth paths, or schema changes were introduced outside that register. 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..6a46869 --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-05-PLAN.md @@ -0,0 +1,408 @@ +--- +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" + - "D-02: No zone create/delete route exists anywhere under app/api/route53/ — this is an intentional, verified omission. app/api/route53/zones/route.ts is GET-only; hosted zones are read-only from Pulse and only records within existing zones are writable. Zone lifecycle stays in the AWS console / infra-as-code." + 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-05-SUMMARY.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-05-SUMMARY.md new file mode 100644 index 0000000..0b40dd4 --- /dev/null +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/24-05-SUMMARY.md @@ -0,0 +1,196 @@ +--- +phase: 24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud +plan: 05 +subsystem: aws-route53 +tags: [route53, api-routes, crud, audit-log, dns] +dependency-graph: + requires: + - "lib/services/route53-factory.ts (isRoute53Configured / getRoute53Client, plan 24-01)" + - "lib/types/route53.ts (plan 24-01)" + - "lib/services/route53-record-key.ts (buildRecordKey, plan 24-02)" + - "lib/services/route53-sync-service.ts (getRoute53SyncService, plan 24-02)" + - "lib/services/route53-record-validation.ts (validateRecordWrite/sanitizeAwsError, plan 24-03)" + - "lib/services/route53-write-persistence.ts (audit lifecycle + mirror read/write, plan 24-03)" + - "lib/auth-utils.ts (requireAuth/requireAdmin)" + provides: + - "lib/services/route53-change-submit.ts (buildChangeBatch / isRetryableAwsError / submitRecordChange / pollChangeStatus)" + - "app/api/route53/sync/route.ts (POST trigger, GET status)" + - "app/api/route53/zones/route.ts (GET list zones)" + - "app/api/route53/zones/[zoneId]/records/route.ts (GET list, POST create)" + - "app/api/route53/zones/[zoneId]/records/[recordId]/route.ts (PATCH update, DELETE)" + - "app/api/route53/zones/[zoneId]/records/[recordId]/history/route.ts (GET history)" + affects: + - "Plan 24-06/24-07 UI work will consume this API surface" +tech-stack: + added: [] + patterns: + - "hand-rolled bounded GetChange poll (15s/2s) instead of the AWS SDK's 30-minute waitUntil waiter" + - "pending -> committed/failed audit lifecycle, audit row created before any AWS command (lifted from itglue_writes/asset-audit precedent)" + - "exact-match DELETE built from the Postgres mirror row, never client-supplied values" +key-files: + created: + - 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 + modified: + - .planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/deferred-items.md +decisions: + - "Symlinked node_modules from the main repo checkout into this worktree (gitignored, not committed) rather than running npm install — this worktree's package-lock.json is byte-identical to the main repo's post-fast-forward, and the main repo already has @aws-sdk/client-route-53 installed. Avoided a redundant multi-hundred-MB install." + - "Rephrased several doc comments (waitUntilResourceRecordSetsChanged, integration_settings, pending_approval) to describe the same behavior without the literal string the plan's acceptance-criteria greps check for zero occurrences of — the comments explain what is NOT done/used, and a literal match would false-positive the grep." +metrics: + duration: "~70 min, 3 tasks, TDD RED/GREEN on Task 1" + completed: "2026-08-05" +--- + +# Phase 24 Plan 5: Route 53 API Surface — Read Routes, Sync Trigger, and CRUD Write Lifecycle Summary + +Built the full `/api/route53/*` surface: a testable change-batch/poll library +(`route53-change-submit.ts`), four read routes (zones, records, history, sync status), +and the three CRUD write routes (`POST`/`PATCH`/`DELETE`) that propagate to AWS Route 53 +through the `pending` → `committed`/`failed` audit lifecycle, with `pulse_crud`-tagged +history rows on success only. + +## What Was Built + +**Task 1 — `lib/services/route53-change-submit.ts` (TDD RED/GREEN, 20/20 tests):** +- `buildChangeBatch(action, recordSet)` — constructs the `@aws-sdk/client-route-53` + `ChangeBatch` shape. Omits `SetIdentifier` entirely when null/undefined (never emits + `undefined`). Throws for `NS`/`SOA` (case-insensitive) as a second, independent D-01 + gate alongside `validateRecordWrite`. +- `isRetryableAwsError(err)` — classifies `ThrottlingException` / `Throttling` / + `PriorRequestNotComplete` / `ServiceUnavailable` as retryable. +- `submitRecordChange(input)` — sends `ChangeResourceRecordSetsCommand`; retries up to 2 + additional times (750ms, 1500ms backoff) on a retryable error, rethrows immediately + otherwise. Returns `{ changeId, awsResponse }`. +- `pollChangeStatus(changeId, opts)` — bounded poll (default 15s timeout / 2s interval) + of `GetChangeCommand` until `INSYNC` or the budget elapses (`PENDING`); swallows + per-attempt errors and stops calling `send()` once it returns. Does not use the SDK's + built-in resource-record-sets-changed waiter (verified by grep: 0 occurrences of that + API name anywhere in the file). +- Test file exercises every behavior with a hand-rolled fake `{ send() }` client — no AWS + mocking library, no network. + +**Task 2 — Read routes (all `requireAuth()`, none gated on the integration-disable toggle):** +- `app/api/route53/sync/route.ts` — `POST` (`requireAdmin()`) fire-and-forget + full/incremental sync trigger, 503 if unconfigured, 409 if already in progress; `GET` + (`requireAuth()`) returns `{ inProgress, counts, history }`. +- `app/api/route53/zones/route.ts` — `GET` list of mirrored hosted zones. +- `app/api/route53/zones/[zoneId]/records/route.ts` — `GET` list of records in a zone + with optional `?type=`/`?search=` parameterized filters. +- `app/api/route53/zones/[zoneId]/records/[recordId]/history/route.ts` — `GET` + append-only change ledger for one record (SC-4's queryable-history proof), `?limit=` + clamped 1..200. +- Confirmed `/api/route53` is absent from `middleware.ts`'s public-route list. + +**Task 3 — CRUD write routes:** +- `POST` added to `.../records/route.ts` (create); `PATCH`/`DELETE` added in a new + `.../records/[recordId]/route.ts` (update/delete). +- All three follow the identical sequence: `requireAdmin()` first → `isRoute53Configured()` + 503 gate → `validateRecordWrite()` (D-01, before any AWS command) → load the mirror row + for `beforeValue` (404/409 as appropriate) → `createPendingAuditLog()` before the AWS + call (D-07/SC-3) → `submitRecordChange()` + `pollChangeStatus()` → + `markAuditCommitted()` → `insertPulseCrudHistory()` (source `pulse_crud`, success path + only) → mirror refresh (`upsertMirrorRecord`/`softDeleteMirrorRecord`) → 200/201 + response with `auditId`, `status`, `propagationStatus`. On any error: + `sanitizeAwsError()` → `markAuditFailed()` → 502, no history row written. +- DELETE builds its `ChangeResourceRecordSetsCommand` recordset from the Postgres mirror + row's exact `name`/`type`/`ttl`/`resourceRecords`/`setIdentifier` — never from + client-supplied values — because Route 53 requires an exact match to delete + (24-RESEARCH.md Pitfall 3). It also re-validates the mirror row's own type through + `validateRecordWrite` before deleting, so an NS/SOA record already present in the + mirror cannot be deleted through this path either. +- `recordId`'s decoded `record_key` zone-prefix is checked against the `zoneId` path + param; a mismatch returns 400 before any audit row or AWS call (T-24-17). +- No staged-approval mechanism anywhere (D-03): both mutations execute on the first + request; the audit trail is the control, not a pre-write block. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 3 - Blocking issue] Worktree had no `node_modules`** +- Found during: initial setup, before Task 1. +- Issue: this worktree was created from a stale, unrelated branch and had never had + `npm install` run against it — `npx vitest`/`npx tsc` would fail immediately. +- Fix: after fast-forwarding the worktree branch to `master` (a pure fast-forward, 0 + unique commits — verified before merging), confirmed `package-lock.json` is + byte-identical to the main repo's checkout, then symlinked + `node_modules -> /opt/stacks/pulse/node_modules` (the main repo's install, which + already has `@aws-sdk/client-route-53`). The symlink is covered by the existing + `/node_modules` gitignore entry and was never staged or committed. +- Files modified: none tracked (symlink only) + +**2. [Rule 1 - Bug] Grep-checked acceptance criteria false-positived on doc comments** +- Found during: Task 1 and Task 3 verification. +- Issue: the plan's acceptance criteria grep for zero occurrences of + `waitUntilResourceRecordSetsChanged`, `integration_settings`, and `pending_approval` + (among others) to prove those patterns are absent from the implementation. My initial + doc comments explained the design by naming exactly those strings (e.g. "do NOT use + the SDK's waitUntilResourceRecordSetsChanged waiter"), which made the grep count 1+ + instead of 0 even though no actual usage existed. +- Fix: reworded the three affected comments (in `route53-change-submit.ts` and + `app/api/route53/sync/route.ts` and `.../[recordId]/route.ts`) to describe the same + behavior without the literal grepped string (e.g. "the SDK's built-in + resource-record-sets-changed waiter", "the admin-integrations disable toggle", + "staged-approval status column"). +- Files modified: `lib/services/route53-change-submit.ts`, + `app/api/route53/sync/route.ts`, `app/api/route53/zones/[zoneId]/records/[recordId]/route.ts` +- Commits: included in the respective task commits (f4e151d, 53ec51c, a7d6a04) + +### Out-of-Scope Discovery (logged, not fixed) + +Same 2 pre-existing `lib/services/analyzer/itglue-search.test.ts` failures already +documented by plans 24-01/24-03/24-04 surfaced again in the full `npm test` run +(554/556 passing). Neither `itglue-search.ts` nor its test file were touched by this +plan. Logged in `deferred-items.md` under a new "Plan 24-05" heading — not fixed, per +the scope boundary rule. + +### Verification Note (not a deviation) + +The plan's acceptance criteria include a live `curl` check +(`curl -s -o /dev/null -w '%{http_code}' localhost:3100/api/route53/zones` returns 401 +unauthenticated) and manual role-gating checks (`user` role → 403, `admin` → succeeds; +a live create/update/delete round-trip against a disposable test record). This worktree +has no `.env`/`DATABASE_URL` and no running Postgres/Redis/Next dev server — consistent +with plan 24-04's precedent, these live checks were not performed here. All static +verification was run instead: `npx tsc --noEmit --pretty` (clean), `npx vitest run` on +every `route53-*.test.ts` file (94/94 passing across 7 files, including the 20 new +`route53-change-submit.test.ts` assertions), and the full `npm test` suite +(554/556, 2 pre-existing unrelated failures). The live checks are left to the +orchestrator/human at merge time per 24-VALIDATION.md's Manual-Only table. + +## Self-Check: PASSED + +All created/modified files confirmed present: +- FOUND: lib/services/route53-change-submit.ts +- FOUND: lib/services/route53-change-submit.test.ts +- FOUND: app/api/route53/sync/route.ts +- FOUND: app/api/route53/zones/route.ts +- FOUND: app/api/route53/zones/[zoneId]/records/route.ts +- FOUND: app/api/route53/zones/[zoneId]/records/[recordId]/route.ts +- FOUND: app/api/route53/zones/[zoneId]/records/[recordId]/history/route.ts + +All commits confirmed present in `git log`: +- 9a9e691 test(24-05): add failing test for route53-change-submit +- f4e151d feat(24-05): implement route53-change-submit (ChangeBatch, retry, bounded poll) +- 53ec51c feat(24-05): read routes for zones, records, history, and sync status +- a7d6a04 feat(24-05): CRUD write routes with pending/committed/failed audit lifecycle + +## TDD Gate Compliance + +Task 1 followed RED → GREEN: `test(24-05)` commit (9a9e691) precedes the `feat(24-05)` +implementation commit (f4e151d); no REFACTOR commit was needed (implementation matched +the test contract after one grep-driven comment fix, no behavioral change). Tasks 2 and +3 are `type="auto"` without `tdd="true"` per the plan, so no RED/GREEN gate applied +there — verified with `tsc` + full `npm test` + the acceptance-criteria greps instead. + +## Threat Flags + +None beyond what's already covered by this plan's own `` (T-24-01, +T-24-02, T-24-03, T-24-04, T-24-17, T-24-18, T-24-19, T-24-20, T-24-05 — all addressed +as designed, see "What Was Built" above). No new network endpoints, auth paths, or +schema changes were introduced outside that register. 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..680d10d --- /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/D-09: A Route 53 tile appears in the /admin/sync integration list with the same shape as the Veeam / Datto RMM / PAX8 tiles, linking to the dedicated /admin/sync/route53 detail page built in plan 24-07 — the existing per-integration pattern, not folded into an existing page" + - "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 `