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