wulf-pulse/lib/services/route53-record-key.ts
lorentz e057255f4f fix(24): address code-review findings — PATCH identity guard, empty-array tombstone, health-check timeout, record-key normalization
Two critical issues from the post-phase code review:

- PATCH /api/route53/zones/[zoneId]/records/[recordId] never verified the
  request body's name/type/setIdentifier matched the record identified by
  the URL. A mismatch would silently UPSERT a brand-new AWS recordset
  (leaving the original live and untouched) while corrupting the mirror's
  record_key invariant. Now rejects with 400 if any of those three fields
  differ from the existing record — renaming/retyping is delete-plus-create,
  not an update.

- route53-sync-service.ts's syncZones()/syncRecords() tombstone queries used
  "id <> ALL(seenIds)" style queries with no empty-array guard — a
  successful-but-empty AWS response would soft-delete every previously
  synced zone/record in one shot. Same bug class already fixed in
  pax8-sync-service.ts; now guarded the same way here.

Two smaller fixes:

- checkRoute53()'s AWS auth probe had no timeout, unlike every other
  integration's liveCheck() (8s AbortController). Added the same bound via
  the SDK's abortSignal option.
- buildRecordKey() relied on every caller to pre-normalize name/type case
  before calling it. Now normalizes internally (lowercase name, uppercase
  type) so the record_key invariant holds regardless of caller discipline.

Full REVIEW.md findings in 24-REVIEW.md. Two remaining Warnings (alias
records un-editable/undeletable, no admin-UI surface for route53_audit_log)
deliberately left as backlog items for a follow-up phase — out of scope for
a post-execution fix pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-05 23:15:27 -04:00

124 lines
4.4 KiB
TypeScript

/**
* Pure, dependency-free helpers for Route 53 record-key derivation,
* recordset normalization, and drift classification.
*
* No `pg` import, no AWS SDK client construction — only types are imported
* from `@aws-sdk/client-route-53`. This keeps the module unit-testable
* without mocking anything (see 24-RESEARCH.md Anti-Patterns: no per-value
* diffing; Route 53 models an update as a whole-recordset replace).
*/
import type { ResourceRecordSet } from '@aws-sdk/client-route-53';
export interface NormalizedRecordSet {
recordKey: string;
zoneId: string;
name: string;
type: string;
setIdentifier: string | null;
ttl: number | null;
resourceRecords: Array<{ value: string }>;
aliasTarget: Record<string, unknown> | null;
}
/**
* Build the `route53_records.record_key` primary key / `recordId` URL
* segment: `${zoneId}:${name}:${type}:${setIdentifier ?? ''}`.
*
* Normalizes `name` (lowercase) and `type` (uppercase) itself rather than
* trusting every caller to pre-normalize — every current call site happens
* to normalize before calling this, but that's a fragile invariant with no
* guard at the point where it actually matters: a mismatched-case call would
* silently produce a different `record_key` than the canonical one, splitting
* a single AWS record across two mirror rows.
*/
export function buildRecordKey(input: {
zoneId: string;
name: string;
type: string;
setIdentifier?: string | null;
}): string {
return `${input.zoneId}:${input.name.toLowerCase()}:${input.type.toUpperCase()}:${input.setIdentifier ?? ''}`;
}
/**
* Normalize a raw AWS `ResourceRecordSet` into a comparable, stable shape.
*
* - `name` is lowercased with its trailing dot preserved
* - `type` is uppercased
* - `ttl` is `rs.TTL ?? null` (alias records have no TTL)
* - `resourceRecords` is sorted by value so AWS's unordered list doesn't
* register as drift on every sync
* - `aliasTarget` is `rs.AliasTarget ?? null`
*/
export function normalizeRecordSet(rs: ResourceRecordSet, zoneId: string): NormalizedRecordSet {
const name = (rs.Name ?? '').toLowerCase();
const type = (rs.Type ?? '').toUpperCase();
const setIdentifier = rs.SetIdentifier ?? null;
const ttl = rs.TTL ?? null;
const resourceRecords = (rs.ResourceRecords ?? [])
.map((r) => ({ value: r.Value ?? '' }))
.sort((a, b) => a.value.localeCompare(b.value));
const aliasTarget = (rs.AliasTarget as unknown as Record<string, unknown> | undefined) ?? null;
return {
recordKey: buildRecordKey({ zoneId, name, type, setIdentifier }),
zoneId,
name,
type,
setIdentifier,
ttl,
resourceRecords,
aliasTarget,
};
}
/**
* Compare two normalized recordsets for material equality: TTL, the
* serialized (already-sorted) resourceRecords array, and the serialized
* aliasTarget. Both null returns true; exactly one null returns false.
*/
export function recordSetsEqual(a: NormalizedRecordSet | null, b: NormalizedRecordSet | null): boolean {
if (a === null && b === null) return true;
if (a === null || b === null) return false;
if (a.ttl !== b.ttl) return false;
if (JSON.stringify(a.resourceRecords) !== JSON.stringify(b.resourceRecords)) return false;
if (JSON.stringify(a.aliasTarget) !== JSON.stringify(b.aliasTarget)) return false;
return true;
}
/**
* Classify the drift between a previous (mirror) and next (AWS-live)
* normalized recordset, mapping to `route53_record_history.change_action`.
* Returns `null` when there is no material difference, so the sync does not
* write a no-op history row.
*/
export function classifyDrift(
prev: NormalizedRecordSet | null,
next: NormalizedRecordSet | null
): 'create' | 'update' | 'delete' | null {
if (prev === null && next === null) return null;
if (prev === null) return 'create';
if (next === null) return 'delete';
return recordSetsEqual(prev, next) ? null : 'update';
}
/**
* Build the JSONB payload stored in `before_value`/`after_value`: the whole
* recordset snapshot. Route 53 has no partial-value primitive, so the unit
* of change recorded in history is always the whole recordset, never a
* per-field delta.
*/
export function toHistoryPayload(ns: NormalizedRecordSet | null): unknown {
if (ns === null) return null;
return {
name: ns.name,
type: ns.type,
setIdentifier: ns.setIdentifier,
ttl: ns.ttl,
resourceRecords: ns.resourceRecords,
aliasTarget: ns.aliasTarget,
};
}