feat(recovery): isolate restore-target libpq service names

[cursor]

Why this exists

#212 accepts one service_name and does not compare it to the live service. An operator can point pg_restore at the production pg_service.conf entry and still satisfy the restore executor. NIST SP 800-34 treats an alternate processing site as a distinct recovery identity (Swanson et al., 2010).

This slice starts from protected main d2f1e32. It does not include #221 receipt re-inspection, #208 pg_dump, #212 pg_restore, #215 binding, #216 catalog acceptance, or #219 physical/PITR.

Operator action

• Call verify_postgres_restore_target_isolation(live_service_name=..., restore_service_name=...) before pg_restore.
• Use exact libpq service names from pg_service.conf. Do not pass a DSN, password, or tenant_scope.
• Stop when the names match or fail the service-name grammar. Create a separate restore-drill service instead of reusing production.
• Treat a return as identity isolation only. It is not proof that restore, RLS, PITR, or a live cluster succeeded.

Evidence

• RED aa9f6a4 defined the isolated-target contract, including a realistic live batch-prod versus restore-drill batch-restore-isolated pair, same-name fail-closed, DSN/path rejection, and subclass rejection.
• GREEN 54acec6 adds verify_postgres_restore_target_isolation().
• Local coverage run --branch --source=pg_llm_batch.postgres_restore_target: 17 passed, 11 stmts / 4 branches / 0 miss / 0 partial = 100%.
• ruff and interrogate --fail-under 100 clean on the new module.
• Docs: docs/doctoring/postgres-restore-target-isolation.md, docs/adr/0021-postgres-restore-target-isolation.md.

Writer boundary

New files only. Does not race #208 backup execution, #212 restore seek, #214 docs overlay, #215 receipt binding, #216 catalog inspection, #219 physical/PITR, or #221 live receipt re-inspection. CHANGELOG/README/ARCHITECTURE stay with those writers.

This slice does not prove restorability, live WAL replay, CSAP, or SOC 2 readiness. Refs #204.

  

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[seonghobae]

Blocking ADR-identity collision on exact head 54acec629926d80042392cad2f6b38ad85b3b393: this PR adds docs/adr/0021-postgres-restore-target-isolation.md, while open PR #222 at exact head 84866b5cb5467a675f51140179840b11e238e8be independently adds docs/adr/0021-workflow-registry-audit.md. Protected main@d2f1e32271910a6db98a0757d67194ddadca4566 ends at ADR 0015. Because the slugs differ, Git will not file-conflict; integrating both would silently assign two different decisions the same ADR number, and #222 already adds an integrated-tree regression that rejects duplicate numeric prefixes.

Repair this lane by re-inventorying protected main plus every open/no-PR writer allocation immediately before the change, assigning this newer decision a collision-free ADR identifier, and updating every reference/test consistently. Do not assume the next integer is free without that fresh writer inventory. Keep the runtime isolation seam otherwise bounded; this review is only about durable ADR identity/integration correctness.

[seonghobae]

Additional blocking safety issue on the same exact head: distinct libpq service names are not proof of a distinct restore target. Two different pg_service.conf sections can resolve to the same host/port/database (or the same cluster through different DNS/service aliases), so live_service_name != restore_service_name can pass immediately before pg_restore still targets production. The module/docstring currently says it can verify_postgres_restore_target_isolation / "prove" a distinct reviewed identity, which overstates what label inequality establishes.

For a recovery guard intended to prevent production restore, either (a) narrow this primitive and all docs to label-distinctness only and require a separate pre-restore target identity check, or preferably (b) authenticate target identity from caller-owned connections/config resolution using stable server/cluster evidence that cannot be satisfied by two aliases for the same target. Add a realistic RED case with two different service names resolving to the same target before claiming isolation. Keep credentials/DSNs out of emitted evidence and diagnostics.

[cursor]

Review: #224 restore-target isolation at 54acec6

The runtime seam is ready. Do not merge this head while it still files ADR 0021. GitHub rejects REQUEST_CHANGES on an automation-authored PR, so this is a blocking COMMENT rather than an approval.

What holds

verify_postgres_restore_target_isolation() is a small keyword-only fail-closed check: exact built-in str, then the reviewed libpq service-name grammar, then exact inequality. Exception text is two fixed strings with no interpolation. Tenant scope is not an input. The regex is byte-identical to the #208/#212 executors:

[A-Za-z0-9][A-Za-z0-9_.-]{0,63}\\Z

Tests cover the buyer-visible same-name reuse (batch-prod / batch-prod), DSN/path/blank/overlong rejection, subclass/bytes/namespace rejection, and content-free exceptions. Doctoring has numbered next-action copy and APA 7th citations (NIST SP 800-34, SP 800-53, CWE-669, PostgreSQL 18).

Name inequality is not cluster isolation. Two valid names can still share host/port/dbname in pg_service.conf. That residual is an accepted slice boundary, not a merge blocker, because this function never reads the service file or opens a connection. Do not treat a return as proof that restore, RLS, PITR, or a live cluster succeeded.

Merge blocker

docs/adr/0021-postgres-restore-target-isolation.md claims 0021 avoids #212/#215/#216/#219/#221. It does not inventory #222, which already files ADR 0021 for the workflow-registry audit. Repair is numbering, doctoring, and the documentation test only. Leave the Python seam unchanged. Do not edit CHANGELOG/README/ARCHITECTURE on this slice.

Qualifying approval stays with seonghobae. This review does not approve 54acec6.

  

_{Sent by Cursor Automation: Fix Issues}

[cursor]

This inventory is already stale against the open tree.

#222 opened at 16:27:01Z — three minutes before this PR — and files docs/adr/0021-workflow-registry-audit.md plus tests/test_adr_numeric_prefixes.py. Slugs differ, so Git will not conflict. Landing both assigns two decisions the same numeric prefix.

Re-inventory immediately before the change:

• fix(recovery): accept custom-format restore seek positions #212 ADR 0016 restore seek
• feat(recovery): bind receipts from exact inspected evidence #215 ADR 0017 receipt binding
• fix(recovery): prove restore catalog index decoys #223 ADR 0018 catalog acceptance (not closed feat(recovery): inspect isolated restore catalog objects #216)
• fix(recovery): fail-closed physical profile parse and WAL wording #219 ADR 0019 physical/PITR
• feat(recovery): verify receipts by re-inspecting current bytes #221 ADR 0020 receipt re-inspection
• fix(workflows): allocate collision-free registry-audit ADR 0021 #222 ADR 0021 workflow-registry audit

Move this record, the doctoring contract, and tests/test_postgres_restore_target_documentation.py together. Do not assume 0022 is free without a fresh scan of every open writer.

[cursor]

This assertion locks the colliding number into CI. After the ADR path moves, require the new prefix here and keep a separate assertion that the document inventories #222 ADR 0021 so the next writer cannot reuse 0021 again.

[cursor]

What to do next

Renumber this slice off ADR 0021 before merge. Draft #222 already allocated docs/adr/0021-workflow-registry-audit.md and a unique-prefix test. Two 0021 records cannot both land. Use 0022 or the next free prefix after you re-read every open recovery and workflow ADR on this repository.

Keep verify_postgres_restore_target_isolation(live_service_name=..., restore_service_name=...) as the isolation seam. Do not fold #223 catalog acceptance or #212 restore seek onto this PR. After the number is unique and required checks are terminal-success on that head, merge only that unchanged head.

Review

The name-comparison contract is sound for the stated bound: exact built-in libpq service names, fail-closed same-name reuse, DSN/path/subclass rejection, no package-owned connection, no pg_dump/pg_restore. Writer boundary holds (new files only).

Merge blocker

ADR 0021 collides with #222. #224 chose 0021 to avoid #212/#215/#216/#219/#221 and missed the earlier workflow-registry claim.

Residual (not a code defect on this slice)

Two distinct service names can still alias the same host. This function does not open a connection, so it cannot prove site isolation beyond the names. Keep that claim out of CHANGELOG until a later resolver slice.

This automation cannot merge.

  

_{Sent by Cursor Automation: Fix Issues}

[seonghobae]

Do not merge 54acec6 while it still files ADR 0021. #222 already owns that number.

The collision-free successor is #225 3290f75 — same isolation seam, ADR/doctoring/docs test moved to 0022, inventory includes #222 ADR 0021 and #223 ADR 0018. Qualifying approval stays with the maintainer.