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/deferred-items.md b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/deferred-items.md index 364f6c7..4c5f52c 100644 --- a/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/deferred-items.md +++ b/.planning/phases/24-aws-route-53-dns-sync-track-changes-crud-operations-full-aud/deferred-items.md @@ -14,3 +14,11 @@ changes). confirms zero changes to either path from Task 1 or Task 2). Unrelated to the Route 53 factory/schema work in this plan — not fixed, logged here for visibility. + +## Plan 24-03 + +- Same 2 pre-existing `lib/services/analyzer/itglue-search.test.ts` failures + re-surfaced by `npm test` (full suite) while verifying Task 2's + `route53-write-persistence.ts`. Neither `itglue-search.ts` nor its test + file were touched by this plan. Out of scope per the scope boundary rule — + not fixed. diff --git a/lib/services/route53-record-validation.test.ts b/lib/services/route53-record-validation.test.ts new file mode 100644 index 0000000..1008bfb --- /dev/null +++ b/lib/services/route53-record-validation.test.ts @@ -0,0 +1,163 @@ +/** + * lib/services/route53-record-validation.ts unit tests — D-01 allowlist + * enforcement and AWS error sanitization (T-24-03). Pure logic, no AWS SDK + * or Postgres dependency to mock. + */ + +import { describe, it, expect } from 'vitest'; +import { + WRITABLE_RECORD_TYPES, + validateRecordWrite, + sanitizeAwsError, +} from './route53-record-validation'; + +describe('WRITABLE_RECORD_TYPES', () => { + it('is exactly the six D-01 writable types, never NS or SOA', () => { + expect([...WRITABLE_RECORD_TYPES].sort()).toEqual( + ['A', 'AAAA', 'CNAME', 'MX', 'SRV', 'TXT'].sort() + ); + expect(WRITABLE_RECORD_TYPES).not.toContain('NS'); + expect(WRITABLE_RECORD_TYPES).not.toContain('SOA'); + }); +}); + +describe('validateRecordWrite', () => { + const basePayload = { + name: 'www.example.com', + resourceRecords: [{ value: '1.2.3.4' }], + }; + + it('rejects NS with a reason naming it as a zone-delegation record', () => { + const result = validateRecordWrite({ ...basePayload, type: 'NS' }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.status).toBe(400); + expect(result.reason).toMatch(/NS/); + expect(result.reason).toMatch(/delegation/i); + } + }); + + it('rejects SOA', () => { + const result = validateRecordWrite({ ...basePayload, type: 'SOA' }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.status).toBe(400); + }); + + it('rejects lowercase "ns" case-insensitively', () => { + const result = validateRecordWrite({ ...basePayload, type: 'ns' }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.status).toBe(400); + expect(result.reason).toMatch(/delegation/i); + } + }); + + it.each(['A', 'AAAA', 'CNAME', 'MX', 'TXT', 'SRV'])( + 'accepts well-formed %s payload', + (type) => { + const result = validateRecordWrite({ ...basePayload, type }); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.value.type).toBe(type); + expect(result.value.name).toBe('www.example.com.'); + expect(result.value.ttl).toBe(300); + } + } + ); + + it('rejects an unknown type such as CAA (closed allowlist, not a blocklist)', () => { + const result = validateRecordWrite({ ...basePayload, type: 'CAA' }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.status).toBe(400); + }); + + it('rejects an unknown type such as DS', () => { + const result = validateRecordWrite({ ...basePayload, type: 'DS' }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.status).toBe(400); + }); + + it('rejects a missing name', () => { + const result = validateRecordWrite({ type: 'A', resourceRecords: [{ value: '1.2.3.4' }] }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.status).toBe(400); + }); + + it('rejects an empty-string name', () => { + const result = validateRecordWrite({ ...basePayload, name: ' ', type: 'A' }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.status).toBe(400); + }); + + it('rejects a non-integer ttl', () => { + const result = validateRecordWrite({ ...basePayload, type: 'A', ttl: 3.5 }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.status).toBe(400); + }); + + it('rejects a ttl outside 0..2147483647', () => { + const tooHigh = validateRecordWrite({ ...basePayload, type: 'A', ttl: 2147483648 }); + expect(tooHigh.ok).toBe(false); + const tooLow = validateRecordWrite({ ...basePayload, type: 'A', ttl: -1 }); + expect(tooLow.ok).toBe(false); + }); + + it('accepts ttl of exactly 0 and exactly 2147483647', () => { + const min = validateRecordWrite({ ...basePayload, type: 'A', ttl: 0 }); + expect(min.ok).toBe(true); + const max = validateRecordWrite({ ...basePayload, type: 'A', ttl: 2147483647 }); + expect(max.ok).toBe(true); + }); + + it('rejects an empty resourceRecords array (Route 53 rejects an empty value set)', () => { + const result = validateRecordWrite({ name: 'www.example.com', type: 'A', resourceRecords: [] }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.status).toBe(400); + }); + + it('rejects a resourceRecords entry with an empty-string value', () => { + const result = validateRecordWrite({ + name: 'www.example.com', + type: 'A', + resourceRecords: [{ value: '' }], + }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.status).toBe(400); + }); + + it('caps resourceRecords at 100 entries', () => { + const tooMany = Array.from({ length: 101 }, (_, i) => ({ value: `10.0.0.${i % 256}` })); + const result = validateRecordWrite({ name: 'www.example.com', type: 'A', resourceRecords: tooMany }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.status).toBe(400); + }); +}); + +describe('sanitizeAwsError', () => { + it('strips AWS access key ids, ARNs, and account ids, then truncates to 500 chars', () => { + const err = new Error( + 'AccessDenied for AKIAIOSFODNN7EXAMPLE on arn:aws:route53:::hostedzone/Z123 account 123456789012' + ); + const sanitized = sanitizeAwsError(err); + expect(sanitized).not.toContain('AKIAIOSFODNN7EXAMPLE'); + expect(sanitized).not.toContain('arn:aws:route53:::hostedzone/Z123'); + expect(sanitized).not.toContain('123456789012'); + expect(sanitized.length).toBeLessThanOrEqual(504); + }); + + it('truncates messages longer than 500 characters', () => { + const longMessage = 'x'.repeat(1000); + const sanitized = sanitizeAwsError(new Error(longMessage)); + expect(sanitized.length).toBeLessThanOrEqual(504); + expect(sanitized.endsWith('...')).toBe(true); + }); + + it('never throws on a non-Error input and returns a string', () => { + expect(() => sanitizeAwsError('a plain string error')).not.toThrow(); + expect(typeof sanitizeAwsError('a plain string error')).toBe('string'); + expect(() => sanitizeAwsError(undefined)).not.toThrow(); + expect(typeof sanitizeAwsError(undefined)).toBe('string'); + expect(() => sanitizeAwsError({ weird: 'object' })).not.toThrow(); + expect(typeof sanitizeAwsError(null)).toBe('string'); + }); +}); diff --git a/lib/services/route53-record-validation.ts b/lib/services/route53-record-validation.ts new file mode 100644 index 0000000..30fb74e --- /dev/null +++ b/lib/services/route53-record-validation.ts @@ -0,0 +1,171 @@ +/** + * AWS Route 53 DNS sync — server-side write validation + AWS error sanitizer. + * + * D-01: A closed allowlist of writable record types. NS and SOA are + * zone-delegation records and must never be writable from Pulse — this check + * runs in library code the CRUD routes call BEFORE any AWS command is + * constructed, mirroring the credential-field blocklist pattern in + * app/api/analyzer/itglue/applications/[id]/apply/route.ts (belt over the + * UI's braces: never rely on the UI simply not offering the option). + * + * T-24-03: `sanitizeAwsError` is the only permitted source of a persisted or + * client-returned AWS error string — it strips access key ids, ARNs, and AWS + * account ids before the message is written to `route53_audit_log.error_message` + * or returned in an API response body. + * + * No AWS SDK or Postgres dependency here by design — this module is plain, + * synchronous, unit-testable validation logic. + */ + +import type { Route53RecordValue, Route53WritableType } from '@/lib/types/route53'; + +/** + * D-01: closed allowlist. NS and SOA are deliberately absent — an + * unrecognized type is rejected, never passed through. + */ +export const WRITABLE_RECORD_TYPES: readonly Route53WritableType[] = Object.freeze([ + 'A', + 'AAAA', + 'CNAME', + 'MX', + 'TXT', + 'SRV', +]); + +const MIN_TTL = 0; +const MAX_TTL = 2147483647; +const MAX_RESOURCE_RECORDS = 100; +const ZONE_DELEGATION_TYPES = new Set(['NS', 'SOA']); + +export interface ValidatedRecordWrite { + name: string; + type: Route53WritableType; + ttl: number; + resourceRecords: Route53RecordValue[]; +} + +export type ValidateRecordWriteResult = + | { ok: true; value: ValidatedRecordWrite } + | { ok: false; status: 400; reason: string }; + +interface ValidateRecordWriteInput { + name?: unknown; + type?: unknown; + ttl?: unknown; + resourceRecords?: unknown; +} + +function fail(reason: string): ValidateRecordWriteResult { + return { ok: false, status: 400, reason }; +} + +/** + * Validate a record write payload against D-01's closed allowlist and basic + * shape rules. Does not construct any AWS command and has no DB dependency — + * callers invoke this first and only proceed to build a + * ChangeResourceRecordSetsCommand when `ok` is true. + */ +export function validateRecordWrite(input: ValidateRecordWriteInput): ValidateRecordWriteResult { + // 1. name: non-empty string after trimming, normalized to Route 53's + // canonical lowercase + trailing-dot form. + if (typeof input.name !== 'string' || input.name.trim().length === 0) { + return fail('Record name is required and must be a non-empty string'); + } + let name = input.name.trim().toLowerCase(); + if (!name.endsWith('.')) { + name = `${name}.`; + } + + // 2. type: string, uppercased, must be a member of the closed allowlist. + if (typeof input.type !== 'string' || input.type.trim().length === 0) { + return fail('Record type is required and must be a string'); + } + const type = input.type.trim().toUpperCase(); + if (ZONE_DELEGATION_TYPES.has(type)) { + return fail( + `Record type ${type} is not writable from Pulse — NS and SOA are zone-delegation records (D-01)` + ); + } + if (!WRITABLE_RECORD_TYPES.includes(type as Route53WritableType)) { + return fail( + `Record type ${type} is not a supported writable type — must be one of ${WRITABLE_RECORD_TYPES.join(', ')}` + ); + } + + // 3. ttl: integer 0..2147483647 inclusive, default 300 when omitted. + let ttl: number; + if (input.ttl === undefined || input.ttl === null) { + ttl = 300; + } else { + if (typeof input.ttl !== 'number' || !Number.isInteger(input.ttl)) { + return fail('ttl must be an integer'); + } + if (input.ttl < MIN_TTL || input.ttl > MAX_TTL) { + return fail(`ttl must be between ${MIN_TTL} and ${MAX_TTL}`); + } + ttl = input.ttl; + } + + // 4. resourceRecords: non-empty array, each entry a non-empty string value, + // capped at 100 entries (Route 53 rejects an empty value set; unbounded + // arrays are a DoS surface — T-24-12). + if (!Array.isArray(input.resourceRecords) || input.resourceRecords.length === 0) { + return fail('resourceRecords must be a non-empty array'); + } + if (input.resourceRecords.length > MAX_RESOURCE_RECORDS) { + return fail(`resourceRecords cannot exceed ${MAX_RESOURCE_RECORDS} entries`); + } + const resourceRecords: Route53RecordValue[] = []; + for (const entry of input.resourceRecords) { + if ( + typeof entry !== 'object' || + entry === null || + typeof (entry as { value?: unknown }).value !== 'string' || + (entry as { value: string }).value.trim().length === 0 + ) { + return fail('Each resourceRecords entry must have a non-empty string value'); + } + resourceRecords.push({ value: (entry as { value: string }).value }); + } + + return { + ok: true, + value: { + name, + type: type as Route53WritableType, + ttl, + resourceRecords, + }, + }; +} + +const AWS_ACCESS_KEY_ID_PATTERN = /AKIA[0-9A-Z]{16}/g; +const AWS_ARN_PATTERN = /arn:aws:[^\s"']+/g; +const AWS_ACCOUNT_ID_PATTERN = /\b[0-9]{12}\b/g; +const MAX_SANITIZED_LENGTH = 500; + +/** + * Redact anything that looks like an AWS access key id, ARN, or 12-digit + * account id from an AWS SDK error, then truncate. This is the only string + * that may be written to `route53_audit_log.error_message` or returned in an + * API response body (T-24-03). Never throws. + */ +export function sanitizeAwsError(err: unknown): string { + let message: string; + try { + message = err instanceof Error ? err.message : String(err); + } catch { + message = 'Unknown error'; + } + + let sanitized = message + .replace(AWS_ACCESS_KEY_ID_PATTERN, '[redacted-key-id]') + .replace(AWS_ARN_PATTERN, '[redacted-arn]') + .replace(AWS_ACCOUNT_ID_PATTERN, '[redacted-account-id]'); + + if (sanitized.length > MAX_SANITIZED_LENGTH) { + sanitized = `${sanitized.slice(0, MAX_SANITIZED_LENGTH)}...`; + } + + return sanitized; +} diff --git a/lib/services/route53-write-persistence.test.ts b/lib/services/route53-write-persistence.test.ts new file mode 100644 index 0000000..e442673 --- /dev/null +++ b/lib/services/route53-write-persistence.test.ts @@ -0,0 +1,180 @@ +/** + * lib/services/route53-write-persistence.ts unit tests — SQL contract checks + * against a mocked postgresClient. No real Postgres connection is made. + * Follows the mocking discipline in lib/services/pax8-sync-service.test.ts. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Mock postgresClient BEFORE importing the module under test. +const queryMock = vi.fn(); +vi.mock('@/lib/services/postgres-client', () => ({ + default: { + query: (...args: unknown[]) => queryMock(...args), + }, +})); + +// Import AFTER the mock is declared so vi.mock hoisting takes effect. +import { + createPendingAuditLog, + markAuditCommitted, + markAuditFailed, + insertPulseCrudHistory, + upsertMirrorRecord, + softDeleteMirrorRecord, + loadMirrorRecord, +} from './route53-write-persistence'; + +interface MockCall { + sql: string; + params: unknown[]; +} + +function calls(): MockCall[] { + return queryMock.mock.calls.map(([sql, params]) => ({ + sql: String(sql), + params: (params as unknown[]) ?? [], + })); +} + +beforeEach(() => { + queryMock.mockReset(); +}); + +describe('createPendingAuditLog', () => { + it("issues an INSERT into route53_audit_log whose SQL contains 'pending'", async () => { + queryMock.mockResolvedValueOnce({ rows: [{ id: 'audit-1' }] }); + const result = await createPendingAuditLog({ + operation: 'create', + zoneId: 'Z123', + recordKey: 'Z123:www.example.com.:A:', + recordName: 'www.example.com.', + recordType: 'A', + beforeValue: null, + afterValue: { ttl: 300, resourceRecords: [{ value: '1.2.3.4' }] }, + performedByUserId: 'user-1', + performedByEmail: 'lorentz@wulfconsulting.com', + }); + expect(result).toEqual({ id: 'audit-1' }); + expect(calls()).toHaveLength(1); + expect(calls()[0].sql).toContain('INSERT INTO route53_audit_log'); + expect(calls()[0].sql).toContain("'pending'"); + }); +}); + +describe('markAuditCommitted', () => { + it("issues an UPDATE setting status = 'committed'", async () => { + queryMock.mockResolvedValueOnce({ rows: [] }); + await markAuditCommitted('audit-1', 'change-1', 'INSYNC', { ResponseMetadata: {} }); + expect(calls()[0].sql).toContain('UPDATE route53_audit_log'); + expect(calls()[0].sql).toContain("'committed'"); + }); +}); + +describe('markAuditFailed', () => { + it('passes a sanitized error string — a raw AWS key id never reaches the bound parameters', async () => { + queryMock.mockResolvedValueOnce({ rows: [] }); + const rawError = new Error('AccessDenied for AKIAIOSFODNN7EXAMPLE'); + await markAuditFailed('audit-1', rawError); + + expect(calls()).toHaveLength(1); + expect(calls()[0].sql).toContain("'failed'"); + const boundParams = calls()[0].params; + expect(boundParams).not.toContain(rawError); + for (const param of boundParams) { + if (typeof param === 'string') { + expect(param).not.toContain('AKIAIOSFODNN7EXAMPLE'); + } + } + }); +}); + +describe('insertPulseCrudHistory', () => { + it("binds the literal 'pulse_crud' as the history source", async () => { + queryMock.mockResolvedValueOnce({ rows: [] }); + await insertPulseCrudHistory({ + zoneId: 'Z123', + recordKey: 'Z123:www.example.com.:A:', + recordName: 'www.example.com.', + recordType: 'A', + changeAction: 'create', + beforeValue: null, + afterValue: { ttl: 300 }, + changedByUserId: 'user-1', + changedByEmail: 'lorentz@wulfconsulting.com', + auditLogId: 'audit-1', + }); + expect(calls()[0].sql).toContain('INSERT INTO route53_record_history'); + expect(calls()[0].sql).toContain("'pulse_crud'"); + }); +}); + +describe('upsertMirrorRecord', () => { + it('issues an INSERT ... ON CONFLICT upsert and never throws even if the query rejects', async () => { + queryMock.mockRejectedValueOnce(new Error('connection reset')); + await expect( + upsertMirrorRecord({ + recordKey: 'Z123:www.example.com.:A:', + zoneId: 'Z123', + name: 'www.example.com.', + type: 'A', + setIdentifier: null, + ttl: 300, + resourceRecords: [{ value: '1.2.3.4' }], + aliasTarget: null, + rawPayload: null, + }) + ).resolves.toBeUndefined(); + }); +}); + +describe('softDeleteMirrorRecord', () => { + it('issues an UPDATE and never a DELETE FROM', async () => { + queryMock.mockResolvedValueOnce({ rows: [] }); + await softDeleteMirrorRecord('Z123:www.example.com.:A:'); + expect(calls()[0].sql).toContain('UPDATE route53_records'); + expect(calls()[0].sql).not.toContain('DELETE FROM'); + }); + + it('never throws even if the query rejects (best-effort)', async () => { + queryMock.mockRejectedValueOnce(new Error('connection reset')); + await expect(softDeleteMirrorRecord('Z123:www.example.com.:A:')).resolves.toBeUndefined(); + }); +}); + +describe('loadMirrorRecord', () => { + it('returns a camelCase object when found', async () => { + queryMock.mockResolvedValueOnce({ + rowCount: 1, + rows: [ + { + record_key: 'Z123:www.example.com.:A:', + zone_id: 'Z123', + name: 'www.example.com.', + type: 'A', + set_identifier: null, + ttl: 300, + resource_records: [{ value: '1.2.3.4' }], + alias_target: null, + }, + ], + }); + const result = await loadMirrorRecord('Z123:www.example.com.:A:'); + expect(result).toEqual({ + recordKey: 'Z123:www.example.com.:A:', + zoneId: 'Z123', + name: 'www.example.com.', + type: 'A', + setIdentifier: null, + ttl: 300, + resourceRecords: [{ value: '1.2.3.4' }], + aliasTarget: null, + }); + }); + + it('returns null when not found', async () => { + queryMock.mockResolvedValueOnce({ rowCount: 0, rows: [] }); + const result = await loadMirrorRecord('missing-key'); + expect(result).toBeNull(); + }); +}); diff --git a/lib/services/route53-write-persistence.ts b/lib/services/route53-write-persistence.ts new file mode 100644 index 0000000..1f0b83f --- /dev/null +++ b/lib/services/route53-write-persistence.ts @@ -0,0 +1,297 @@ +/** + * AWS Route 53 DNS sync — CRUD write-back persistence. + * + * Implements the pending -> committed/failed audit lifecycle, the single + * most important pattern in this phase (lifted from + * lib/services/analyzer/asset-audit/persistence.ts's createPendingWrite / + * markWriteCommitted / markWriteFailed shape, itself following + * migrations/075_itglue_audit.sql's itglue_writes precedent). + * + * Discipline (24-RESEARCH.md Pattern 3, D-07): an audit row is created with + * status='pending' BEFORE any AWS command is constructed. It is then + * transitioned to 'committed' or 'failed' after the AWS call resolves. + * Callers must never construct a ChangeResourceRecordSetsCommand without an + * audit row already in flight. + */ + +import postgresClient from '@/lib/services/postgres-client'; +import { sanitizeAwsError } from '@/lib/services/route53-record-validation'; +import type { Route53RecordValue } from '@/lib/types/route53'; + +const LOG_PREFIX = '[ROUTE53-WRITE]'; + +// ============================================================================ +// Audit lifecycle (route53_audit_log) +// ============================================================================ + +export interface CreatePendingAuditLogInput { + operation: 'create' | 'update' | 'delete'; + zoneId: string; + recordKey: string; + recordName: string; + recordType: string; + beforeValue: unknown; + afterValue: unknown; + performedByUserId: string | null; + performedByEmail: string | null; +} + +/** + * Insert a 'pending' audit row. Must be called and awaited BEFORE any AWS + * ChangeResourceRecordSetsCommand is constructed — that ordering is the + * whole point of this pattern: no write to Route 53 can occur without an + * audit row already in flight (D-07, SC-3). This write is not best-effort — + * let it throw so a DB failure fails the request rather than silently + * allowing an unlogged DNS mutation (T-24-07). + */ +export async function createPendingAuditLog( + input: CreatePendingAuditLogInput +): Promise<{ id: string }> { + const res = await postgresClient.query<{ 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, $2, $3, $4, $5, + $6::jsonb, $7::jsonb, + $8, $9, 'pending') + RETURNING id::text AS id`, + [ + input.operation, + input.zoneId, + input.recordKey, + input.recordName, + input.recordType, + JSON.stringify(input.beforeValue ?? null), + JSON.stringify(input.afterValue ?? null), + input.performedByUserId, + input.performedByEmail, + ] + ); + return { id: res.rows[0].id }; +} + +/** + * Transition an audit row to 'committed' after a successful AWS call. Not + * best-effort — let it throw. + */ +export async function markAuditCommitted( + id: string, + awsChangeId: string | null, + awsChangeStatus: string | null, + awsResponse: unknown +): Promise { + await postgresClient.query( + `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`, + [id, awsChangeId, awsChangeStatus, JSON.stringify(awsResponse ?? null)] + ); +} + +/** + * Transition an audit row to 'failed' after an unsuccessful AWS call. + * D-07 + T-24-03: the error is always passed through sanitizeAwsError first + * — never pass a raw error object or JSON.stringify(err) into error_message. + * Not best-effort — let it throw. + */ +export async function markAuditFailed(id: string, err: unknown): Promise { + await postgresClient.query( + `UPDATE route53_audit_log + SET status = 'failed', + completed_at = NOW(), + error_message = $2 + WHERE id = $1`, + [id, sanitizeAwsError(err)] + ); +} + +// ============================================================================ +// pulse_crud history (route53_record_history) +// ============================================================================ + +export interface InsertPulseCrudHistoryInput { + zoneId: string; + recordKey: string; + recordName: string; + recordType: string; + changeAction: 'create' | 'update' | 'delete'; + beforeValue: unknown; + afterValue: unknown; + changedByUserId: string | null; + changedByEmail: string | null; + auditLogId: string; +} + +/** + * Insert a 'pulse_crud' history row. + * + * IMPORTANT: callers must invoke this ONLY after markAuditCommitted. A + * failed AWS call changed nothing on AWS's side, so it gets an audit row + * (for traceability of the attempt) but must NOT get a history row (which is + * a timeline of resolved, actually-applied changes). See 24-RESEARCH.md + * Pattern 3. + */ +export async function insertPulseCrudHistory( + input: InsertPulseCrudHistoryInput +): Promise { + await postgresClient.query( + `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, audit_log_id) + VALUES ($1, $2, $3, $4, + $5, $6::jsonb, $7::jsonb, + 'pulse_crud', $8, $9, $10)`, + [ + input.zoneId, + input.recordKey, + input.recordName, + input.recordType, + input.changeAction, + JSON.stringify(input.beforeValue ?? null), + JSON.stringify(input.afterValue ?? null), + input.changedByUserId, + input.changedByEmail, + input.auditLogId, + ] + ); +} + +// ============================================================================ +// Mirror refresh (route53_records) — best-effort +// ============================================================================ + +export interface UpsertMirrorRecordInput { + recordKey: string; + zoneId: string; + name: string; + type: string; + setIdentifier: string | null; + ttl: number | null; + resourceRecords: Route53RecordValue[] | null; + aliasTarget: Record | null; + rawPayload: unknown; +} + +/** + * Best-effort refresh of route53_records after a committed write so the + * admin UI reflects the change before the next scheduled sync. A failure + * here is logged but never thrown — the AWS write already succeeded and the + * next incremental sync reconciles the mirror regardless. + */ +export async function upsertMirrorRecord(input: UpsertMirrorRecordInput): Promise { + try { + await postgresClient.query( + `INSERT INTO route53_records + (record_key, zone_id, name, type, set_identifier, ttl, + resource_records, alias_target, raw_payload, + synced_at, updated_at, is_deleted, deleted_at) + VALUES ($1, $2, $3, $4, $5, $6, + $7::jsonb, $8::jsonb, $9::jsonb, + NOW(), NOW(), false, NULL) + ON CONFLICT (record_key) DO UPDATE SET + zone_id = EXCLUDED.zone_id, + name = EXCLUDED.name, + type = EXCLUDED.type, + set_identifier = EXCLUDED.set_identifier, + ttl = EXCLUDED.ttl, + resource_records = EXCLUDED.resource_records, + alias_target = EXCLUDED.alias_target, + raw_payload = EXCLUDED.raw_payload, + synced_at = NOW(), + updated_at = NOW(), + is_deleted = false, + deleted_at = NULL`, + [ + input.recordKey, + input.zoneId, + input.name, + input.type, + input.setIdentifier, + input.ttl, + JSON.stringify(input.resourceRecords ?? null), + JSON.stringify(input.aliasTarget ?? null), + JSON.stringify(input.rawPayload ?? null), + ] + ); + } catch (err) { + console.error(`${LOG_PREFIX} upsertMirrorRecord failed (best-effort):`, sanitizeAwsError(err)); + } +} + +/** + * Soft-delete a mirror record. Never hard-delete: route53_record_history + * references record_key and the ledger is unbounded by design (D-08). Best + * effort — logged, never thrown. + */ +export async function softDeleteMirrorRecord(recordKey: string): Promise { + try { + await postgresClient.query( + `UPDATE route53_records + SET is_deleted = true, deleted_at = NOW(), updated_at = NOW() + WHERE record_key = $1`, + [recordKey] + ); + } catch (err) { + console.error(`${LOG_PREFIX} softDeleteMirrorRecord failed (best-effort):`, sanitizeAwsError(err)); + } +} + +// ============================================================================ +// Mirror read (route53_records) +// ============================================================================ + +export interface MirrorRecordRow { + recordKey: string; + zoneId: string; + name: string; + type: string; + setIdentifier: string | null; + ttl: number | null; + resourceRecords: Route53RecordValue[] | null; + aliasTarget: Record | null; +} + +interface RawMirrorRecordRow { + record_key: string; + zone_id: string; + name: string; + type: string; + set_identifier: string | null; + ttl: number | null; + resource_records: Route53RecordValue[] | null; + alias_target: Record | null; +} + +/** + * Load the current mirror row for a record key, if present and not + * soft-deleted. Supplies the before_value for updates/deletes and, + * critically, the exact TTL and value set a Route 53 DELETE action requires + * to match — a DELETE with a mismatched TTL or value set fails or targets + * the wrong recordset (24-RESEARCH.md Pitfall 3). + */ +export async function loadMirrorRecord(recordKey: string): Promise { + const res = await postgresClient.query( + `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`, + [recordKey] + ); + if (res.rowCount === 0) return null; + const row = res.rows[0]; + return { + recordKey: row.record_key, + zoneId: row.zone_id, + name: row.name, + type: row.type, + setIdentifier: row.set_identifier, + ttl: row.ttl, + resourceRecords: row.resource_records, + aliasTarget: row.alias_target, + }; +}