lorentz/wulf-pulse
1
0
Fork 0
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity Actions

fix(24): address code-review findings — PATCH identity guard, empty-array tombstone, health-check timeout, record-key normalization

Browse source 
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>
This commit is contained in:
lorentz 2026-08-05 23:15:08 -04:00
parent 52afdca8a1
commit e057255f4f
4 changed files with 66 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

36
lib/services/route53-sync-service.ts
View file

@ -279,12 +279,19 @@ export class Route53SyncService {
);
}
// Soft-delete zones no longer returned by AWS.
await postgresClient.query(
`UPDATE route53_zones SET is_deleted = true, deleted_at = NOW(), updated_at = NOW()
WHERE is_deleted = false AND id <> ALL($1)`,
[seenIds]
);
// Soft-delete zones no longer returned by AWS. Guard against an empty
// seenIds — `id <> ALL('{}')` is vacuously true for every row, so a
// successful-but-empty AWS response (transient API quirk, not "the
// account has zero zones") would otherwise soft-delete every previously
// synced zone in one shot. Same bug class already fixed in
// pax8-sync-service.ts.
if (seenIds.length > 0) {
await postgresClient.query(
`UPDATE route53_zones SET is_deleted = true, deleted_at = NOW(), updated_at = NOW()
WHERE is_deleted = false AND id <> ALL($1)`,
[seenIds]
);
}
return seenIds.length;
}
@ -396,12 +403,17 @@ export class Route53SyncService {
}
// Soft-delete this zone's records no longer present in AWS. Never
// hard-delete — the history ledger references record_key.
await postgresClient.query(
`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)`,
[zoneId, seenKeys]
);
// hard-delete — the history ledger references record_key. Guard
// against an empty seenKeys for the same reason as syncZones() above —
// every real zone has at least apex NS/SOA records, so an empty page
// here is a transient AWS API quirk, not "this zone has zero records."
if (seenKeys.length > 0) {
await postgresClient.query(
`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)`,
[zoneId, seenKeys]
);
}
}
return totalUpserted;