From 64fb19d489aa6f1bbbe745a6de259ef4076a0073 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 11:20:09 -0400 Subject: [PATCH 01/11] =?UTF-8?q?docs(dns):=20design=20+=20ADR=200047=20?= =?UTF-8?q?=E2=80=94=20delegation=20in=20portfolio=20via=20Snapshot.Author?= =?UTF-8?q?ity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Capture registrar NS delegation alongside hosted records in the canonical portfolio (both layers, per user req). 3-repo cascade: hover EnumerateAll delegation + workflow FromResourceStates -> Authority + gocodealone-dns import. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...0047-dns-delegation-portfolio-authority.md | 26 ++++++ ...6-06-02-dns-delegation-portfolio-design.md | 87 +++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 decisions/0047-dns-delegation-portfolio-authority.md create mode 100644 docs/plans/2026-06-02-dns-delegation-portfolio-design.md diff --git a/decisions/0047-dns-delegation-portfolio-authority.md b/decisions/0047-dns-delegation-portfolio-authority.md new file mode 100644 index 00000000..a516fe23 --- /dev/null +++ b/decisions/0047-dns-delegation-portfolio-authority.md @@ -0,0 +1,26 @@ +# 0047. Represent DNS delegation in the portfolio via Snapshot.Authority + +**Status:** Accepted +**Date:** 2026-06-02 +**Decision-makers:** codingsloth@pm.me (directed), Claude (Opus 4.8) +**Related:** docs/plans/2026-06-02-dns-delegation-portfolio-design.md + +## Context + +The DNS catalog (gocodealone-dns import-dns.yml) imports `--type infra.dns` only — hosted records. For domains whose NS delegate elsewhere, those are parking/placeholder records, and the registrar-level NS delegation (which provider actually serves each domain) is never captured. The canonical portfolio schema `workflow.dns-portfolio.export.v1` already has an unused `Snapshot.Authority map[string]any` field. User requires BOTH layers in the catalog — delegation AND hosted records — so that records staged at a provider ahead of an NS cutover stay visible (live-only would hide them). + +## Decision + +Populate `Snapshot.Authority` with the registrar NS delegation, merged by `(provider, domain)` with hosted records, so one snapshot carries both layers. `record.FromResourceStates` gains an `infra.dns_delegation` branch (`Outputs["nameservers"]` → `Authority["nameservers"]`); `workflow-plugin-hover` `EnumerateAll` gains an `infra.dns_delegation` case (the per-domain `GetDomainDelegation` already exists). + +Alternatives rejected: +- **Side-file (`--format state`)** — no engine change, but splits the catalog into two inconsistent formats; the user wants both layers in the canonical portfolio. +- **NS-as-records** — conflates registrar delegation with in-zone NS records (different semantics); `Authority` is the correct home. + +## Consequences + +- 3-repo cascade (workflow engine + hover plugin + gocodealone-dns) + 2 releases (wfctl + hover v0.5.1). Engine change is ~15 lines + tests. +- Additive schema use — `Authority` was always present (`omitempty`); no schema break, existing portfolio consumers unaffected. +- A snapshot now distinguishes live (NS point here) from staging/placeholder (NS elsewhere) records. +- Revertible per-repo by version pins; reverting canonicalize returns portfolios to records-only. +- DO delegation not captured (DO NS are self-referential; the registrar Hover holds the real delegation) — future-optional, not built. diff --git a/docs/plans/2026-06-02-dns-delegation-portfolio-design.md b/docs/plans/2026-06-02-dns-delegation-portfolio-design.md new file mode 100644 index 00000000..03c298ed --- /dev/null +++ b/docs/plans/2026-06-02-dns-delegation-portfolio-design.md @@ -0,0 +1,87 @@ +# DNS catalog: capture delegation (NS) alongside hosted records — Design + +**Date:** 2026-06-02 +**Status:** Design (autonomous pipeline; user-directed "design and build it, both layers") +**Repos:** workflow (engine/wfctl) · workflow-plugin-hover · gocodealone-dns +**Guidance:** no `docs/design-guidance.md`; Q&A captured inline (user direction below) + +## Problem + +The gocodealone-dns DNS catalog import (`import-dns.yml`) imports `--type infra.dns` only → each provider's **hosted records**. For a domain whose **NS delegate elsewhere**, the Hover-hosted records are **parking/placeholders** (e.g. `blackorchid-tributeband.com` → `A * → 216.40.34.41` = Hover park IP, default Hover MX; portfolio `authority: null`, 0 NS). The catalog never captures the **registrar-level NS delegation** — the map of *which provider actually serves each domain*. Result: the catalog shows placeholder records with no indication they're not live. + +**User requirement (verbatim intent):** capture BOTH layers — (1) delegation (registrar NS = where DNS is live) and (2) hosted records per provider — *"regardless of hover or another provider, because if only track the live dns, we won't have knowledge of records being added to prep for an NS switch. So both layers seem a minimum."* I.e. staging records (provisioned at a provider before an NS cutover) must remain visible. + +## Decision + +Populate the **already-existing** `Snapshot.Authority` field (canonical schema `workflow.dns-portfolio.export.v1`, `dns/record/record.go`) with the registrar NS delegation, merged by domain with the hosted records, so one portfolio snapshot carries BOTH layers. + +``` +Snapshot{ provider, domain, + authority: {nameservers:[...]}, // infra.dns_delegation — where DNS is LIVE + records: [...] } // infra.dns — hosted records (live OR staging) +``` + +A domain's snapshot now answers: *where is DNS authoritatively served* (authority NS) AND *what records exist at this provider* (records — authoritative if NS point here, staging/placeholder otherwise). + +### 3-repo cascade + +1. **workflow-plugin-hover** — `HoverProvider.EnumerateAll` currently rejects all but `infra.dns`. Add `infra.dns_delegation`: `ListDomains` → per-domain `GetDomainDelegation` → emit `ResourceOutput{ProviderID: domain, Type: "infra.dns_delegation", Outputs: {nameservers: [...], domain_id}}`. (The delegation driver + `GetDomainDelegation` already exist; only the enumerate loop is missing.) Release v0.5.1. +2. **workflow** — `record.FromResourceStates` (`dns/record/canonicalize.go`) currently `continue`s on every non-`infra.dns` state. Change to merge by `(provider, domain)`: `infra.dns` → `Records`; `infra.dns_delegation` → `Authority = {"nameservers": [...]}` (read from `st.Outputs["nameservers"]`). Domains with only one type get that layer; the other stays empty/omitted. Release (next wfctl minor). +3. **gocodealone-dns** — `import-dns.yml`: add a Hover `--type infra.dns_delegation` import into the **same** Hover state dir (so the portfolio merge sees both), bump the wfctl pin (engine change) + hover pin (v0.5.1). The Hover delegation = the registrar master map (all 30 domains are Hover-registered). + +### Data flow + +`import-all --provider hover --type infra.dns` + `--type infra.dns_delegation` → both resource types land in `.state/hover/` → `--format portfolio` → `FromResourceStates` merges by domain → `hover.portfolio.json` with `authority` + `records` per snapshot. DO unchanged (records only; DO NS are self-referential — registrar truth comes from Hover). + +## Approaches considered + +- **A — unified portfolio via `Authority` (chosen).** Engine change populates the schema's existing `Authority` field. One canonical catalog, both layers per snapshot. Cost: 3-repo cascade + 2 releases (engine change is ~15 lines + tests). Matches the schema author's intent (the field exists, unused). +- **B — side-file (`--format state`).** Import delegation as a raw state file (no engine change). Lighter (2 repos, 1 release) but produces a NON-canonical side artifact split from the portfolio; the catalog becomes two inconsistent formats. Rejected: the user wants both layers in the catalog, and the portfolio schema already models authority. +- **C — NS-as-records.** Emit delegation as `NS` records in the existing `records[]`. Rejected: conflates registrar delegation (where the zone is served) with in-zone NS records (subdomain delegation) — different semantics; `Authority` is the correct home. + +## Global Design Guidance + +| guidance | response | +|---|---| +| Primary Go, stdlib-first | engine change is pure Go map-merge; no new deps | +| Canonical portfolio schema | reuses existing `Authority`/`Snapshot` — no schema break (additive; `authority,omitempty`) | +| Plugin contract stability | hover gains an enumerate case; no gRPC/driver contract change | +| e2e via real consumer | validated by re-running gocodealone-dns import-dns.yml on the self-hosted runner | + +## Security Review + +- No new secrets/flows. Delegation read uses the same authenticated Hover session as records. NS are non-sensitive (public DNS). Portfolio `Sanitize` path unaffected (NS are not secret). +- `Authority map[string]any` is free-form; restrict written keys to `nameservers` (+ optional `domain_id`) to avoid leaking internal state into the catalog. + +## Infrastructure Impact + +- Two releases (workflow wfctl + hover v0.5.1) + gocodealone-dns pin bumps. import-dns.yml gains one provider-import step (read-only; another browser-auth Hover login per run). No new cloud resources. +- Rollback class: version pins + engine behavior + plugin behavior → see Rollback. + +## Multi-Component Validation + +Re-run gocodealone-dns `import-dns.yml` on the self-hosted runner after both releases + pin bumps: assert `hover.portfolio.json` snapshots carry non-null `authority.nameservers` AND `records`, and that at least one domain shows NS ≠ hover (delegated-away → records are staging/placeholder). Engine change unit-tested in `dns/record`; hover enumerate unit-tested + the live probe path. + +## Assumptions + +1. **`GetDomainDelegation` works for all 30 domains live** (the browser-auth session reads `/api/control_panel/domains/domain-` NS). Most fragile — validated by the live import. +2. **Registrar NS at Hover is the authoritative delegation source** for these domains (all Hover-registered). True for the current portfolio. +3. **Merging by `(provider, domain)` is correct** — a domain appears once per provider state; the delegation + records for the same domain share provider=hover. Holds because both imports run against the same `.state/hover/`. +4. **`Snapshot.Authority` consumers tolerate population** (it was always in the schema, `omitempty`; scenario-88 fixtures + `Validate()` don't reject it). + +## Rollback + +- Engine: revert the canonicalize change → `Authority` returns to never-populated; portfolios drop to records-only (prior behavior). No schema break. +- hover: revert to v0.5.0 (delegation enumerate returns "not supported" — prior behavior). +- gocodealone-dns: revert pins + drop the delegation import step. Catalog returns to records-only. +- Additive + behind version pins; each repo independently revertible. + +## Self-challenge — top doubts + +1. **3-repo cascade for a ~15-line engine change feels heavy.** But the canonical-catalog requirement forces it: a side-file (B) splits the catalog. The engine change is small + the schema was built for it. +2. **Assumption #1 (live delegation read for all 30 domains)** — if `GetDomainDelegation` fails for some domains (rate-limit, shape drift), enumerate must continue + emit partial (skip-with-warning), not abort the whole import. +3. **DO delegation not captured** — DO NS are self-referential (ns1.digitalocean.com); the registrar (Hover) holds the real delegation. Capturing DO delegation is YAGNI for this portfolio; the design notes it as future-optional, not built. + +## ADR + +Will record an ADR for the unified-Authority decision (chosen over side-file / NS-as-records) in the workflow repo. From 027d3062d692d31a7035f32f998db63bc4c8d66f Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 11:28:00 -0400 Subject: [PATCH 02/11] =?UTF-8?q?docs(dns):=20design=20rev2=20=E2=80=94=20?= =?UTF-8?q?fold=20adversarial=20findings=20(registrar+live=20NS,=20read=20?= =?UTF-8?q?model,=20fail-gate)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I-1: capture registrar_nameservers (GetDomainDelegation, authoritative) + live_nameservers (public DNS, propagation); gap = NS-switch signal; source from registrar not the live-first Read (drift unchanged). I-2: consumer read model. I-3: import-dns fail-gate includes delegation step. C-1: enumerate emit shape + skip-with-warning. m-1: delegation-only snapshot. m-2: shared browser profile. m-3: Sanitize Authority key allow-list. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...0047-dns-delegation-portfolio-authority.md | 13 +++- ...6-06-02-dns-delegation-portfolio-design.md | 74 ++++++++++--------- 2 files changed, 50 insertions(+), 37 deletions(-) diff --git a/decisions/0047-dns-delegation-portfolio-authority.md b/decisions/0047-dns-delegation-portfolio-authority.md index a516fe23..960e43ff 100644 --- a/decisions/0047-dns-delegation-portfolio-authority.md +++ b/decisions/0047-dns-delegation-portfolio-authority.md @@ -11,11 +11,18 @@ The DNS catalog (gocodealone-dns import-dns.yml) imports `--type infra.dns` only ## Decision -Populate `Snapshot.Authority` with the registrar NS delegation, merged by `(provider, domain)` with hosted records, so one snapshot carries both layers. `record.FromResourceStates` gains an `infra.dns_delegation` branch (`Outputs["nameservers"]` → `Authority["nameservers"]`); `workflow-plugin-hover` `EnumerateAll` gains an `infra.dns_delegation` case (the per-domain `GetDomainDelegation` already exists). +Populate `Snapshot.Authority` with delegation NS, merged by `(provider, domain)` with hosted records, so one snapshot carries both layers. `record.FromResourceStates` groups states by `(provider, domain)`: `infra.dns` → `Records`; `infra.dns_delegation` → `Authority`. `workflow-plugin-hover` `EnumerateAll` gains an `infra.dns_delegation` case. + +**Capture BOTH registrar and live NS** (`authority.registrar_nameservers` from `GetDomainDelegation` = registrar intent/authoritative; `authority.live_nameservers` from public DNS = propagation). The registrar-vs-live gap IS the NS-switch-staging signal the user requires. Critically: the catalog must source `registrar_nameservers` from `GetDomainDelegation` explicitly — NOT from `DelegationDriver.Read`, which returns the live public lookup first (so a naive import would capture stale live NS during a cutover). `Read`/drift behavior is left unchanged (no DNS-provider drift blast radius); the delegation `EnumerateAll`/`Import` path sources registrar+live directly. + +**Consumer read model:** `authority` attaches to the registrar's snapshot (provider=hover). To find where a domain is live, match `registrar_nameservers` to a provider; a Hover snapshot whose `registrar_nameservers` point elsewhere carries staging/placeholder records. Alternatives rejected: -- **Side-file (`--format state`)** — no engine change, but splits the catalog into two inconsistent formats; the user wants both layers in the canonical portfolio. -- **NS-as-records** — conflates registrar delegation with in-zone NS records (different semantics); `Authority` is the correct home. +- **Side-file (`--format state`)** — splits the catalog into two inconsistent formats. +- **NS-as-records** — conflates registrar delegation with in-zone NS records. +- **Live-NS-only** — captures the wrong NS during a cutover (defeats the staging-visibility requirement). +- **Change `DelegationDriver.Read` to registrar-primary** — would fix the source but changes drift semantics across the DNS-provider ecosystem; isolated to the catalog path instead. +- **Single EnumerateAll pass emitting both types** — overloads the `--type` filter contract; shared browser profile mitigates the double-login cost instead. ## Consequences diff --git a/docs/plans/2026-06-02-dns-delegation-portfolio-design.md b/docs/plans/2026-06-02-dns-delegation-portfolio-design.md index 03c298ed..d9872fdb 100644 --- a/docs/plans/2026-06-02-dns-delegation-portfolio-design.md +++ b/docs/plans/2026-06-02-dns-delegation-portfolio-design.md @@ -1,87 +1,93 @@ # DNS catalog: capture delegation (NS) alongside hosted records — Design **Date:** 2026-06-02 -**Status:** Design (autonomous pipeline; user-directed "design and build it, both layers") +**Status:** Design (autonomous pipeline; user-directed "design and build it, both layers"). Rev 2 — incorporates adversarial-design-review findings. **Repos:** workflow (engine/wfctl) · workflow-plugin-hover · gocodealone-dns **Guidance:** no `docs/design-guidance.md`; Q&A captured inline (user direction below) ## Problem -The gocodealone-dns DNS catalog import (`import-dns.yml`) imports `--type infra.dns` only → each provider's **hosted records**. For a domain whose **NS delegate elsewhere**, the Hover-hosted records are **parking/placeholders** (e.g. `blackorchid-tributeband.com` → `A * → 216.40.34.41` = Hover park IP, default Hover MX; portfolio `authority: null`, 0 NS). The catalog never captures the **registrar-level NS delegation** — the map of *which provider actually serves each domain*. Result: the catalog shows placeholder records with no indication they're not live. +The gocodealone-dns DNS catalog import (`import-dns.yml`) imports `--type infra.dns` only → each provider's **hosted records**. For a domain whose **NS delegate elsewhere**, the Hover-hosted records are **parking/placeholders** (`blackorchid-tributeband.com` → `A * → 216.40.34.41` = Hover park IP, default Hover MX; portfolio `authority: null`, 0 NS). The catalog never captures the **registrar-level NS delegation** — the map of which provider actually serves each domain. -**User requirement (verbatim intent):** capture BOTH layers — (1) delegation (registrar NS = where DNS is live) and (2) hosted records per provider — *"regardless of hover or another provider, because if only track the live dns, we won't have knowledge of records being added to prep for an NS switch. So both layers seem a minimum."* I.e. staging records (provisioned at a provider before an NS cutover) must remain visible. +**User requirement (verbatim intent):** capture BOTH layers — (1) delegation (registrar NS = where DNS is intended to be served) and (2) hosted records per provider — *"regardless of hover or another provider, because if only track the live dns, we won't have knowledge of records being added to prep for an NS switch. So both layers seem a minimum."* Records staged at a provider before an NS cutover must stay visible. ## Decision -Populate the **already-existing** `Snapshot.Authority` field (canonical schema `workflow.dns-portfolio.export.v1`, `dns/record/record.go`) with the registrar NS delegation, merged by domain with the hosted records, so one portfolio snapshot carries BOTH layers. +Populate the **already-existing** `Snapshot.Authority` field (canonical schema `workflow.dns-portfolio.export.v1`, `dns/record/record.go`) with delegation NS, merged by domain with hosted records, so one snapshot carries BOTH layers: ``` Snapshot{ provider, domain, - authority: {nameservers:[...]}, // infra.dns_delegation — where DNS is LIVE - records: [...] } // infra.dns — hosted records (live OR staging) + authority: { registrar_nameservers:[...], // GetDomainDelegation — registrar INTENT (authoritative) + live_nameservers:[...] }, // public DNS — current PROPAGATION + records: [...] } // infra.dns — hosted records (live OR staging) } ``` -A domain's snapshot now answers: *where is DNS authoritatively served* (authority NS) AND *what records exist at this provider* (records — authoritative if NS point here, staging/placeholder otherwise). +**The registrar-vs-live gap is the NS-switch-staging signal** (adversarial finding I-1, Option E): during a cutover the registrar holds the new NS while live DNS (TTL-cached) still shows the old. Capturing only live NS would hide exactly the prep state the user needs; capturing only registrar NS would hide propagation status. So capture both. + +### Critical source-of-truth detail (finding I-1) + +`DelegationDriver.Read` (drift/apply path) returns `lookupPublicNameservers` **first**, `GetDomainDelegation` only on fallback. `import-all` re-imports via `provider.Import` → that Read → so a naive delegation import captures **live** NS, not registrar intent. **Fix:** the catalog delegation path MUST source `registrar_nameservers` from `GetDomainDelegation` (registrar) explicitly, and `live_nameservers` from the public lookup — independent of the live-first `Read`. The existing `Read` (drift) stays **unchanged** (no DNS-provider-ecosystem drift blast radius). Implementation: `EnumerateAll("infra.dns_delegation")` fetches both per domain and emits them in `Outputs`; the hover `Import("infra.dns_delegation", domain)` path returns the same registrar+live `Outputs` (so the re-import in `runInfraImportAllWithDeps` persists registrar truth, not the live-first Read). ### 3-repo cascade -1. **workflow-plugin-hover** — `HoverProvider.EnumerateAll` currently rejects all but `infra.dns`. Add `infra.dns_delegation`: `ListDomains` → per-domain `GetDomainDelegation` → emit `ResourceOutput{ProviderID: domain, Type: "infra.dns_delegation", Outputs: {nameservers: [...], domain_id}}`. (The delegation driver + `GetDomainDelegation` already exist; only the enumerate loop is missing.) Release v0.5.1. -2. **workflow** — `record.FromResourceStates` (`dns/record/canonicalize.go`) currently `continue`s on every non-`infra.dns` state. Change to merge by `(provider, domain)`: `infra.dns` → `Records`; `infra.dns_delegation` → `Authority = {"nameservers": [...]}` (read from `st.Outputs["nameservers"]`). Domains with only one type get that layer; the other stays empty/omitted. Release (next wfctl minor). -3. **gocodealone-dns** — `import-dns.yml`: add a Hover `--type infra.dns_delegation` import into the **same** Hover state dir (so the portfolio merge sees both), bump the wfctl pin (engine change) + hover pin (v0.5.1). The Hover delegation = the registrar master map (all 30 domains are Hover-registered). +1. **workflow-plugin-hover** — `HoverProvider.EnumerateAll` currently `!= "infra.dns"` → hard error. Add an `infra.dns_delegation` case: `ListDomains` → per domain: `GetDomainDelegation` (registrar) + `lookupPublicNameservers` (live) → emit `ResourceOutput{ProviderID: domain, Type: "infra.dns_delegation", Outputs: {registrar_nameservers, live_nameservers, domain_id}}`. **Per-domain failures skip-with-warning + continue** (do not abort the 30-domain enumerate; matches import-all's per-zone isolation). The delegation `Import` path returns the same dual `Outputs`. `Read`/drift unchanged. Release v0.5.1. +2. **workflow** — `record.FromResourceStates` (`dns/record/canonicalize.go`) currently `continue`s on every non-`infra.dns` state, one Snapshot per state. **Restructure to group by `(provider, domain)`** (control-flow change, not a one-liner): build `map[provider+domain]*Snapshot`; `infra.dns` → `Records`; `infra.dns_delegation` → `Authority = {registrar_nameservers, live_nameservers}` (read from `st.Outputs`). A domain with delegation but no `infra.dns` state → authority-only snapshot (`records: []`). Deterministic emit order (sort by provider,domain). Release (next wfctl minor). +3. **gocodealone-dns** — `import-dns.yml`: add a Hover `--type infra.dns_delegation` import into the **same** `.state/hover/` (so the portfolio merge sees both); **add the new step's outcome to the final fail-gate** (currently hardcoded to `import-do`/`import-hover` only — finding I-3). Bump wfctl pin (engine change) + hover pin (v0.5.1). The two Hover imports **share `browser_profile_dir`** → the second reuses Imperva clearance (cookie reuse, no second full login → no extra lockout risk; finding m-2). -### Data flow +### Consumer read semantics (finding I-2) -`import-all --provider hover --type infra.dns` + `--type infra.dns_delegation` → both resource types land in `.state/hover/` → `--format portfolio` → `FromResourceStates` merges by domain → `hover.portfolio.json` with `authority` + `records` per snapshot. DO unchanged (records only; DO NS are self-referential — registrar truth comes from Hover). +Delegation is a **registrar fact**, so `authority` attaches to the **registrar's** snapshot (provider=hover). To find where a domain is **live**: read `authority.registrar_nameservers` and match it to a provider (e.g. `ns1.digitalocean.com` → provider=digitalocean) — the records in *that* provider's snapshot are authoritative. A Hover snapshot whose `registrar_nameservers` point elsewhere carries **staging/placeholder** records. This read model is documented in the ADR consequences. ## Approaches considered -- **A — unified portfolio via `Authority` (chosen).** Engine change populates the schema's existing `Authority` field. One canonical catalog, both layers per snapshot. Cost: 3-repo cascade + 2 releases (engine change is ~15 lines + tests). Matches the schema author's intent (the field exists, unused). -- **B — side-file (`--format state`).** Import delegation as a raw state file (no engine change). Lighter (2 repos, 1 release) but produces a NON-canonical side artifact split from the portfolio; the catalog becomes two inconsistent formats. Rejected: the user wants both layers in the catalog, and the portfolio schema already models authority. -- **C — NS-as-records.** Emit delegation as `NS` records in the existing `records[]`. Rejected: conflates registrar delegation (where the zone is served) with in-zone NS records (subdomain delegation) — different semantics; `Authority` is the correct home. +- **A — unified portfolio via `Authority` (chosen).** Engine populates the existing `Authority` field; one canonical catalog, both layers per snapshot. Cost: 3-repo cascade + 2 releases. +- **B — side-file (`--format state`).** No engine change but splits the catalog into two inconsistent formats. Rejected (user wants both layers in the canonical portfolio). +- **C — NS-as-records.** Conflates registrar delegation with in-zone NS records. Rejected (`Authority` is the correct home). +- **D — single EnumerateAll pass emitting both `infra.dns` + `infra.dns_delegation`** (one browser session). Considered (finding m-2 / reviewer Option D); rejected for now because it overloads `EnumerateAll("infra.dns")` to emit a different type, breaking the `--type` filter contract. Shared `browser_profile_dir` already mitigates the double-login cost. ## Global Design Guidance | guidance | response | |---|---| -| Primary Go, stdlib-first | engine change is pure Go map-merge; no new deps | -| Canonical portfolio schema | reuses existing `Authority`/`Snapshot` — no schema break (additive; `authority,omitempty`) | -| Plugin contract stability | hover gains an enumerate case; no gRPC/driver contract change | +| Primary Go, stdlib-first | engine change is pure Go map-merge; delegation reuses existing `net.LookupNS` + `GetDomainDelegation` | +| Canonical portfolio schema | reuses existing `Authority`/`Snapshot` — additive (`authority,omitempty`), no schema break | +| Plugin contract stability | hover gains an enumerate case + dual-NS Import outputs; gRPC/driver contract unchanged; `Read`/drift unchanged | | e2e via real consumer | validated by re-running gocodealone-dns import-dns.yml on the self-hosted runner | ## Security Review -- No new secrets/flows. Delegation read uses the same authenticated Hover session as records. NS are non-sensitive (public DNS). Portfolio `Sanitize` path unaffected (NS are not secret). -- `Authority map[string]any` is free-form; restrict written keys to `nameservers` (+ optional `domain_id`) to avoid leaking internal state into the catalog. +- No new secrets/flows. Delegation reads use the existing authenticated Hover session; NS are public/non-sensitive. +- `Authority map[string]any` is free-form → `Sanitize` (`dns/record/sanitize.go`) only touches `Records`. **Restrict written Authority keys to `{registrar_nameservers, live_nameservers, domain_id}`** and have `Sanitize` drop any key outside that allow-list when `--sanitize` is set (finding m-3), so future callers can't leak internal data via Authority. ## Infrastructure Impact -- Two releases (workflow wfctl + hover v0.5.1) + gocodealone-dns pin bumps. import-dns.yml gains one provider-import step (read-only; another browser-auth Hover login per run). No new cloud resources. +- Two releases (workflow wfctl + hover v0.5.1) + gocodealone-dns pin bumps. import-dns.yml gains one read-only provider-import step (one extra Hover login per run, cookie-reused via shared profile). No new cloud resources. - Rollback class: version pins + engine behavior + plugin behavior → see Rollback. ## Multi-Component Validation -Re-run gocodealone-dns `import-dns.yml` on the self-hosted runner after both releases + pin bumps: assert `hover.portfolio.json` snapshots carry non-null `authority.nameservers` AND `records`, and that at least one domain shows NS ≠ hover (delegated-away → records are staging/placeholder). Engine change unit-tested in `dns/record`; hover enumerate unit-tested + the live probe path. +Re-run gocodealone-dns `import-dns.yml` on the self-hosted runner after both releases + pin bumps. Assert: (a) `hover.portfolio.json` snapshots carry `authority.registrar_nameservers` AND `records`; (b) at least one domain shows `registrar_nameservers` ≠ hover (delegated-away → its Hover records are staging/placeholder); (c) the merge produces ONE snapshot per (provider,domain) — no duplicates; (d) a delegation-only domain (if any) yields an authority-only snapshot. Engine merge-by-domain unit-tested in `canonicalize_test.go` (incl. delegation-only + both-layers + records-only cases); hover enumerate + dual-NS Import unit-tested; live probe path exercised. ## Assumptions -1. **`GetDomainDelegation` works for all 30 domains live** (the browser-auth session reads `/api/control_panel/domains/domain-` NS). Most fragile — validated by the live import. -2. **Registrar NS at Hover is the authoritative delegation source** for these domains (all Hover-registered). True for the current portfolio. -3. **Merging by `(provider, domain)` is correct** — a domain appears once per provider state; the delegation + records for the same domain share provider=hover. Holds because both imports run against the same `.state/hover/`. -4. **`Snapshot.Authority` consumers tolerate population** (it was always in the schema, `omitempty`; scenario-88 fixtures + `Validate()` don't reject it). +1. **`GetDomainDelegation` works live for the 30 domains** (browser-auth session reads `/api/control_panel/domains/domain-` NS). Most fragile — validated by the live import; per-domain failures skip-with-warning, not abort. +2. **Registrar NS at Hover is the authoritative delegation source** (all 30 are Hover-registered). True for the current portfolio. +3. **Merge by `(provider, domain)` is correct** — both Hover imports run against the same `.state/hover/`, so a domain's records + delegation share provider=hover. +4. **`Snapshot.Authority` consumers tolerate population** — verified: `record.Validate()` ignores Authority; scenario-88 fixtures don't use it; `omitempty`. ## Rollback -- Engine: revert the canonicalize change → `Authority` returns to never-populated; portfolios drop to records-only (prior behavior). No schema break. -- hover: revert to v0.5.0 (delegation enumerate returns "not supported" — prior behavior). -- gocodealone-dns: revert pins + drop the delegation import step. Catalog returns to records-only. -- Additive + behind version pins; each repo independently revertible. +- Engine: revert canonicalize → `Authority` never populated; portfolios drop to records-only (prior behavior). No schema break. +- hover: revert to v0.5.0 (delegation enumerate → "not supported"). `Read`/drift never changed, so nothing to undo there. +- gocodealone-dns: revert pins + drop the delegation import step + fail-gate entry. Catalog returns to records-only. +- Additive + version-pinned + per-repo revertible. ## Self-challenge — top doubts -1. **3-repo cascade for a ~15-line engine change feels heavy.** But the canonical-catalog requirement forces it: a side-file (B) splits the catalog. The engine change is small + the schema was built for it. -2. **Assumption #1 (live delegation read for all 30 domains)** — if `GetDomainDelegation` fails for some domains (rate-limit, shape drift), enumerate must continue + emit partial (skip-with-warning), not abort the whole import. -3. **DO delegation not captured** — DO NS are self-referential (ns1.digitalocean.com); the registrar (Hover) holds the real delegation. Capturing DO delegation is YAGNI for this portfolio; the design notes it as future-optional, not built. +1. **Scope grew** (dual-NS + Import-vs-Read distinction) from the naive "import delegation." Justified: the naive version captured the WRONG (live) NS, defeating the user's staging requirement. The dual capture is the correct minimum. +2. **Assumption #1** — partial live-read failure handled by skip-with-warning + the import-dns.yml fail-gate now catches a failed delegation step (I-3). +3. **DO delegation not captured** — DO NS self-referential; registrar (Hover) holds the real delegation. YAGNI for this portfolio; future-optional, not built. ## ADR -Will record an ADR for the unified-Authority decision (chosen over side-file / NS-as-records) in the workflow repo. +ADR 0047 (workflow) records the unified-`Authority` decision + the registrar-vs-live dual-capture (I-1) + the consumer read model (I-2). From 229974460be6b7ac8c90c5a886ba1b02aad4744d Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 11:29:12 -0400 Subject: [PATCH 03/11] =?UTF-8?q?docs(dns):=20clarify=20I-1=20wiring=20?= =?UTF-8?q?=E2=80=94=20Import=20dual-fetch=20(registrar+live),=20Read=20un?= =?UTF-8?q?touched?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified provider.Import calls d.Read (provider.go:271); fix gives delegation Import its own GetDomainDelegation+live path bypassing the live-first Read, so drift/apply semantics are unchanged. EnumerateAll just lists; Import fetches. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/plans/2026-06-02-dns-delegation-portfolio-design.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/plans/2026-06-02-dns-delegation-portfolio-design.md b/docs/plans/2026-06-02-dns-delegation-portfolio-design.md index d9872fdb..8e4fb544 100644 --- a/docs/plans/2026-06-02-dns-delegation-portfolio-design.md +++ b/docs/plans/2026-06-02-dns-delegation-portfolio-design.md @@ -26,11 +26,11 @@ Snapshot{ provider, domain, ### Critical source-of-truth detail (finding I-1) -`DelegationDriver.Read` (drift/apply path) returns `lookupPublicNameservers` **first**, `GetDomainDelegation` only on fallback. `import-all` re-imports via `provider.Import` → that Read → so a naive delegation import captures **live** NS, not registrar intent. **Fix:** the catalog delegation path MUST source `registrar_nameservers` from `GetDomainDelegation` (registrar) explicitly, and `live_nameservers` from the public lookup — independent of the live-first `Read`. The existing `Read` (drift) stays **unchanged** (no DNS-provider-ecosystem drift blast radius). Implementation: `EnumerateAll("infra.dns_delegation")` fetches both per domain and emits them in `Outputs`; the hover `Import("infra.dns_delegation", domain)` path returns the same registrar+live `Outputs` (so the re-import in `runInfraImportAllWithDeps` persists registrar truth, not the live-first Read). +`DelegationDriver.Read` (drift/apply path) returns `lookupPublicNameservers` **first**, `GetDomainDelegation` only on fallback. `import-all` re-imports via `provider.Import` → which (verified, provider.go:271) calls `d.Read` → so a naive delegation import captures **live** NS, not registrar intent. **Fix:** give `HoverProvider.Import("infra.dns_delegation", domain)` its own dual-fetch path — `GetDomainDelegation` (registrar, authoritative) + `lookupPublicNameservers` (live) → `Outputs{registrar_nameservers, live_nameservers, domain_id}` — **bypassing the live-first `Read`**. `Import` is a provider method, so this needs NO change to `DelegationDriver.Read` (drift/apply path stays exactly as-is → zero DNS-provider drift blast radius). Per-domain `GetDomainDelegation` failures are isolated by `runInfraImportAllWithDeps`'s existing per-zone failure collection (Import error → recorded + continue, not abort). ### 3-repo cascade -1. **workflow-plugin-hover** — `HoverProvider.EnumerateAll` currently `!= "infra.dns"` → hard error. Add an `infra.dns_delegation` case: `ListDomains` → per domain: `GetDomainDelegation` (registrar) + `lookupPublicNameservers` (live) → emit `ResourceOutput{ProviderID: domain, Type: "infra.dns_delegation", Outputs: {registrar_nameservers, live_nameservers, domain_id}}`. **Per-domain failures skip-with-warning + continue** (do not abort the 30-domain enumerate; matches import-all's per-zone isolation). The delegation `Import` path returns the same dual `Outputs`. `Read`/drift unchanged. Release v0.5.1. +1. **workflow-plugin-hover** — (a) `HoverProvider.EnumerateAll` currently `!= "infra.dns"` → hard error; add an `infra.dns_delegation` case that just **lists** domains (`ListDomains` → emit `ResourceOutput{ProviderID: domain, Type: "infra.dns_delegation"}`). (b) `HoverProvider.Import("infra.dns_delegation", domain)` gets a dual-fetch path: `GetDomainDelegation` + `lookupPublicNameservers` → `Outputs{registrar_nameservers, live_nameservers, domain_id}` (NOT via `Read`). `runInfraImportAllWithDeps` calls `Import` per enumerated domain and isolates per-domain failures. `DelegationDriver.Read`/drift unchanged. Release v0.5.1. 2. **workflow** — `record.FromResourceStates` (`dns/record/canonicalize.go`) currently `continue`s on every non-`infra.dns` state, one Snapshot per state. **Restructure to group by `(provider, domain)`** (control-flow change, not a one-liner): build `map[provider+domain]*Snapshot`; `infra.dns` → `Records`; `infra.dns_delegation` → `Authority = {registrar_nameservers, live_nameservers}` (read from `st.Outputs`). A domain with delegation but no `infra.dns` state → authority-only snapshot (`records: []`). Deterministic emit order (sort by provider,domain). Release (next wfctl minor). 3. **gocodealone-dns** — `import-dns.yml`: add a Hover `--type infra.dns_delegation` import into the **same** `.state/hover/` (so the portfolio merge sees both); **add the new step's outcome to the final fail-gate** (currently hardcoded to `import-do`/`import-hover` only — finding I-3). Bump wfctl pin (engine change) + hover pin (v0.5.1). The two Hover imports **share `browser_profile_dir`** → the second reuses Imperva clearance (cookie reuse, no second full login → no extra lockout risk; finding m-2). From 7dc3648f01f80281a6c47c4d593fdaa70a2d9696 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 11:34:30 -0400 Subject: [PATCH 04/11] =?UTF-8?q?docs(dns):=20design=20rev3=20=E2=80=94=20?= =?UTF-8?q?fold=20I-NEW-3=20(key=20shape),=20I-NEW-4=20(portfolio=20owners?= =?UTF-8?q?hip),=20T-1=20(test)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Import emits nameservers(=registrar) primary key for Diff-compat + supplemental registrar/live keys (no spurious drift). Delegation import step runs second + owns the merged-portfolio write. Update TestFromResourceStatesSkipsNonDNS. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/plans/2026-06-02-dns-delegation-portfolio-design.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/plans/2026-06-02-dns-delegation-portfolio-design.md b/docs/plans/2026-06-02-dns-delegation-portfolio-design.md index 8e4fb544..ad4130ee 100644 --- a/docs/plans/2026-06-02-dns-delegation-portfolio-design.md +++ b/docs/plans/2026-06-02-dns-delegation-portfolio-design.md @@ -26,13 +26,15 @@ Snapshot{ provider, domain, ### Critical source-of-truth detail (finding I-1) -`DelegationDriver.Read` (drift/apply path) returns `lookupPublicNameservers` **first**, `GetDomainDelegation` only on fallback. `import-all` re-imports via `provider.Import` → which (verified, provider.go:271) calls `d.Read` → so a naive delegation import captures **live** NS, not registrar intent. **Fix:** give `HoverProvider.Import("infra.dns_delegation", domain)` its own dual-fetch path — `GetDomainDelegation` (registrar, authoritative) + `lookupPublicNameservers` (live) → `Outputs{registrar_nameservers, live_nameservers, domain_id}` — **bypassing the live-first `Read`**. `Import` is a provider method, so this needs NO change to `DelegationDriver.Read` (drift/apply path stays exactly as-is → zero DNS-provider drift blast radius). Per-domain `GetDomainDelegation` failures are isolated by `runInfraImportAllWithDeps`'s existing per-zone failure collection (Import error → recorded + continue, not abort). +`DelegationDriver.Read` (drift/apply path) returns `lookupPublicNameservers` **first**, `GetDomainDelegation` only on fallback. `import-all` re-imports via `provider.Import` → which (verified, provider.go:271) calls `d.Read` → so a naive delegation import captures **live** NS, not registrar intent. **Fix:** add an `if resourceType == "infra.dns_delegation"` branch to `HoverProvider.Import` (BEFORE the `d.Read` call) with its own dual-fetch — `GetDomainDelegation` (registrar) + `lookupPublicNameservers` (live) — **bypassing the live-first `Read`**. `Import` is a provider method with access to `p`'s hover client, so this needs NO change to `DelegationDriver.Read` (drift/apply stays exactly as-is → zero DNS-provider drift blast radius). + +**Outputs key shape (finding I-NEW-3 — avoid spurious drift):** `DelegationDriver.Diff`/`parseDelegationSpec` read `Outputs["nameservers"]`. So the Import branch MUST emit `"nameservers": ` as the PRIMARY key (registrar is what `SetNameservers` writes → keeps Diff/adoption consistent, no perpetual drift) AND add `"registrar_nameservers"` + `"live_nameservers"` as supplemental keys. `FromResourceStates` reads the supplemental keys (falling back to `nameservers`) for `Authority`. Per-domain `GetDomainDelegation` failures are isolated by `runInfraImportAllWithDeps`'s per-zone failure collection (Import error → recorded + continue, not abort). ### 3-repo cascade 1. **workflow-plugin-hover** — (a) `HoverProvider.EnumerateAll` currently `!= "infra.dns"` → hard error; add an `infra.dns_delegation` case that just **lists** domains (`ListDomains` → emit `ResourceOutput{ProviderID: domain, Type: "infra.dns_delegation"}`). (b) `HoverProvider.Import("infra.dns_delegation", domain)` gets a dual-fetch path: `GetDomainDelegation` + `lookupPublicNameservers` → `Outputs{registrar_nameservers, live_nameservers, domain_id}` (NOT via `Read`). `runInfraImportAllWithDeps` calls `Import` per enumerated domain and isolates per-domain failures. `DelegationDriver.Read`/drift unchanged. Release v0.5.1. -2. **workflow** — `record.FromResourceStates` (`dns/record/canonicalize.go`) currently `continue`s on every non-`infra.dns` state, one Snapshot per state. **Restructure to group by `(provider, domain)`** (control-flow change, not a one-liner): build `map[provider+domain]*Snapshot`; `infra.dns` → `Records`; `infra.dns_delegation` → `Authority = {registrar_nameservers, live_nameservers}` (read from `st.Outputs`). A domain with delegation but no `infra.dns` state → authority-only snapshot (`records: []`). Deterministic emit order (sort by provider,domain). Release (next wfctl minor). -3. **gocodealone-dns** — `import-dns.yml`: add a Hover `--type infra.dns_delegation` import into the **same** `.state/hover/` (so the portfolio merge sees both); **add the new step's outcome to the final fail-gate** (currently hardcoded to `import-do`/`import-hover` only — finding I-3). Bump wfctl pin (engine change) + hover pin (v0.5.1). The two Hover imports **share `browser_profile_dir`** → the second reuses Imperva clearance (cookie reuse, no second full login → no extra lockout risk; finding m-2). +2. **workflow** — `record.FromResourceStates` (`dns/record/canonicalize.go`) currently `continue`s on every non-`infra.dns` state, one Snapshot per state. **Restructure to group by `(provider, domain)`** (control-flow change, not a one-liner): build `map[provider+domain]*Snapshot`; `infra.dns` → `Records`; `infra.dns_delegation` → `Authority = {registrar_nameservers, live_nameservers}` (read from `st.Outputs`). A domain with delegation but no `infra.dns` state → authority-only snapshot (`records: []`). Deterministic emit order (sort by provider,domain). **Test update (finding T-1):** the existing `TestFromResourceStatesSkipsNonDNS` asserts non-`infra.dns` states yield 0 snapshots — that's now false for `infra.dns_delegation` (→ authority-only snapshot). Update it: `infra.dns_delegation` is consumed (authority), genuinely-unknown types are still skipped. Add cases for both-layers / records-only / delegation-only. Release (next wfctl minor). +3. **gocodealone-dns** — `import-dns.yml`: add a Hover `--type infra.dns_delegation` import that runs **after** the `infra.dns` import, against the **same** `.state/hover/`. **Portfolio ownership (finding I-NEW-4):** both `import-all` invocations call `dumpPortfolioToFile`; the LAST one (delegation, running second) reads the shared state store containing BOTH types → its `-o zones/hover.portfolio.json` IS the merged records+authority portfolio, overwriting the records-only one from the first step. So: the delegation step owns the `-o zones/hover.portfolio.json` write; ordering (delegation second) is load-bearing. **Add the delegation step's outcome to the final fail-gate** (currently hardcoded to `import-do`/`import-hover` only — finding I-3). Bump wfctl pin (engine change) + hover pin (v0.5.1). The two Hover imports **share `browser_profile_dir`** → the second reuses Imperva clearance (cookie reuse, no second full login → no extra lockout risk; finding m-2). ### Consumer read semantics (finding I-2) From cdbe37fffdbd28772c1c2c3452e75212150a891d Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 11:36:29 -0400 Subject: [PATCH 05/11] =?UTF-8?q?docs(dns):=20implementation=20plan=20?= =?UTF-8?q?=E2=80=94=20delegation=20portfolio=20(3=20PRs,=206=20tasks)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-06-02-dns-delegation-portfolio.md | 167 ++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 docs/plans/2026-06-02-dns-delegation-portfolio.md diff --git a/docs/plans/2026-06-02-dns-delegation-portfolio.md b/docs/plans/2026-06-02-dns-delegation-portfolio.md new file mode 100644 index 00000000..fcf5f781 --- /dev/null +++ b/docs/plans/2026-06-02-dns-delegation-portfolio.md @@ -0,0 +1,167 @@ +# DNS delegation in portfolio (both layers) Implementation Plan + +> **For the implementing agent:** REQUIRED SUB-SKILL: Use autodev:executing-plans to implement this plan task-by-task. ALWAYS prefix Go commands with `GOWORK=off GOTOOLCHAIN=auto`. Ignore editor "undefined symbol" diagnostics from a sibling repo's go.work — the CLI build is the truth. + +**Goal:** Capture registrar NS delegation alongside hosted records in the canonical DNS portfolio, so the catalog shows where each domain's DNS is live (`authority`) AND its hosted records per provider (`records`, incl. staging records before an NS switch). + +**Architecture:** 3-repo cascade. (1) workflow engine: `FromResourceStates` merges `infra.dns` (records) + `infra.dns_delegation` (authority) by `(provider,domain)` into one Snapshot; `Sanitize` allow-lists Authority keys. (2) workflow-plugin-hover: `EnumerateAll` lists domains for `infra.dns_delegation`; `HoverProvider.Import` dual-fetches registrar (GetDomainDelegation) + live (public NS) — bypassing the live-first `Read` (drift unchanged). (3) gocodealone-dns: import-dns.yml adds a delegation import (runs second, owns the merged portfolio) + fail-gate + pin bumps. + +**Tech Stack:** Go 1.26, workflow IaC provider/portfolio (`workflow.dns-portfolio.export.v1`), wfctl `infra import-all`, GitHub Actions self-hosted runner. + +**Base branch:** main (each repo) + +**Design:** docs/plans/2026-06-02-dns-delegation-portfolio-design.md · **ADR:** decisions/0047-dns-delegation-portfolio-authority.md + +--- + +## Scope Manifest + +**PR Count:** 3 +**Tasks:** 6 +**Estimated Lines of Change:** ~450 (informational; not enforced) + +**Out of scope:** +- DO (or other-provider) delegation enumeration — DO NS are self-referential; the registrar (Hover) holds the delegation truth. Future-optional. +- Changing `DelegationDriver.Read` / drift / apply semantics — explicitly left as-is (zero drift blast radius). +- Any DNS mutation — import is read-only; no `apply`/`SetNameservers`. +- A new wfctl portfolio-export subcommand (Option F) — reuse `import-all --format portfolio`. + +**PR Grouping:** + +| PR # | Title | Tasks | Branch | Repo | +|------|-------|-------|--------|------| +| 1 | Portfolio: merge infra.dns_delegation into Snapshot.Authority | Task 1, Task 2 | feat/dns-delegation-portfolio | workflow | +| 2 | Hover: enumerate + import infra.dns_delegation (registrar+live NS) | Task 3, Task 4 | feat/hover-delegation-enumerate | workflow-plugin-hover | +| 3 | DNS catalog: import Hover delegation, both layers | Task 5, Task 6 | chore/dns-catalog-delegation | gocodealone-dns | + +**Deploy ordering (load-bearing):** PR1 merge → workflow/wfctl release; PR2 merge → hover v0.5.1 release; THEN PR3 (bumps both pins, re-runs import). PR3's validation depends on both releases. + +**Status:** Draft + +--- + +### Task 1: `FromResourceStates` merges delegation into `Snapshot.Authority` + +**Repo:** workflow **Files:** +- Modify: `dns/record/canonicalize.go` +- Modify: `dns/record/canonicalize_test.go` + +**Step 1: Write/adjust failing tests.** In `canonicalize_test.go`: +- Update `TestFromResourceStatesSkipsNonDNS` (finding T-1): a genuinely-unknown type (e.g. `infra.compute`) is still skipped (0 snapshots), but `infra.dns_delegation` is now CONSUMED. +- `TestFromResourceStates_DelegationPopulatesAuthority`: a state `{Type:"infra.dns_delegation", Provider:"hover", ProviderID:"x.com", Outputs:{"registrar_nameservers":["ns1.dnsimple.com"],"live_nameservers":["ns1.digitalocean.com"]}}` → one snapshot with `Authority["registrar_nameservers"]==["ns1.dnsimple.com"]` and `Authority["live_nameservers"]==["ns1.digitalocean.com"]`, `Records` empty. +- `TestFromResourceStates_MergesBothLayersByDomain`: an `infra.dns` state + an `infra.dns_delegation` state, SAME `(provider="hover", domain="x.com")` → exactly ONE snapshot carrying both `Records` (from infra.dns) and `Authority` (from delegation). +- `TestFromResourceStates_DelegationOnlyDomain` (finding m-1): delegation state with no matching infra.dns → one authority-only snapshot, `Records: []` (non-nil-or-empty per existing Validate). +- Keep existing records-only behavior: N `infra.dns` states → N snapshots. + +Run: `GOWORK=off GOTOOLCHAIN=auto go test ./dns/record -run TestFromResourceStates -count=1 -v` → expect FAIL. + +**Step 2: Implement.** Restructure `FromResourceStates` to group by key `provider+"\x00"+domain`: +- Iterate states; resolve `domain` (ProviderID, fall back `AppliedConfig["domain"]`); skip if domain empty. +- For `infra.dns`: get-or-create the snapshot, append `Records` (existing `pickRecords`/`recordFromMap`). +- For `infra.dns_delegation`: get-or-create the snapshot, set `Authority` from `st.Outputs` reading `registrar_nameservers` + `live_nameservers` (each `[]any`→`[]string` via a small helper; copy only those keys — do NOT copy the whole Outputs map). Tolerate missing key (omit it). +- For any other type: `continue` (still skipped). +- Emit snapshots sorted by `(provider, domain)` for determinism. +- Preserve `ID` (first state that creates the group sets it). + +Run the tests → expect PASS. + +**Step 3: Full package + repo verification.** +`GOWORK=off GOTOOLCHAIN=auto go test ./dns/record ./cmd/wfctl -count=1` (cmd/wfctl exercises dumpPortfolioToFile) → all green. +`GOWORK=off GOTOOLCHAIN=auto golangci-lint run --new-from-rev=origin/main ./dns/... ./cmd/wfctl/...` → exit 0. + +**Step 4: Commit.** `git commit -m "feat(dns): merge infra.dns_delegation into Snapshot.Authority"` + +Rollback: revert commit → `FromResourceStates` returns to records-only; no schema break. (Runtime-affecting via wfctl release — see deploy ordering.) + +### Task 2: `Sanitize` allow-lists `Authority` keys + +**Repo:** workflow **Files:** +- Modify: `dns/record/sanitize.go` +- Modify: `dns/record/sanitize_test.go` + +**Step 1: Failing test.** `TestSanitizeStripsUnknownAuthorityKeys`: a snapshot with `Authority{"registrar_nameservers":[...],"live_nameservers":[...],"secret_token":"x"}` → after `Sanitize`, `registrar_nameservers`+`live_nameservers` remain, `secret_token`+any key ∉ allow-list `{registrar_nameservers, live_nameservers, domain_id}` removed. Records sanitization unchanged. +Run: `GOWORK=off GOTOOLCHAIN=auto go test ./dns/record -run TestSanitize -count=1 -v` → FAIL. + +**Step 2: Implement.** In `Sanitize`, after the Records pass, for each snapshot with non-nil `Authority`, delete keys not in the allow-list. (NS are public; this guards future callers from leaking non-NS data via the free-form `map[string]any`.) + +**Step 3:** `GOWORK=off GOTOOLCHAIN=auto go test ./dns/record -count=1` → green. `golangci-lint run --new-from-rev=origin/main ./dns/...` → exit 0. + +**Step 4: Commit.** `git commit -m "feat(dns): sanitize allow-lists Snapshot.Authority keys"` + +Rollback: revert commit. + +### Task 3: Hover `EnumerateAll` lists domains for `infra.dns_delegation` + +**Repo:** workflow-plugin-hover **Files:** +- Modify: `internal/provider.go` (`EnumerateAll`) +- Modify: `internal/provider_test.go` + +**Step 1: Failing test.** `TestEnumerateAll_DelegationListsDomains`: a stub domains-lister returning 2 domains → `EnumerateAll(ctx, "infra.dns_delegation")` returns 2 `ResourceOutput`s, each `Type=="infra.dns_delegation"`, `ProviderID==domain.Name`. Also assert an unknown type still errors `"resource type %q not supported"`. +Run: `GOWORK=off GOTOOLCHAIN=auto go test ./internal -run TestEnumerateAll -count=1 -v` → FAIL. + +**Step 2: Implement.** Change the `EnumerateAll` guard: accept `infra.dns` AND `infra.dns_delegation`; for either, `ListDomains` → emit `ResourceOutput{ProviderID:d.Name, Type:resourceType, Outputs: {"domain_id": d.ID}}` (delegation NS are fetched in Import, Task 4 — keep enumerate cheap: one `ListDomains` call). Reject other types as before. + +**Step 3:** `GOWORK=off GOTOOLCHAIN=auto go test ./internal -count=1` → green. + +**Step 4: Commit.** `git commit -m "feat(provider): EnumerateAll lists domains for infra.dns_delegation"` + +### Task 4: Hover `Import` dual-fetches registrar + live NS (bypass Read) + +**Repo:** workflow-plugin-hover **Files:** +- Modify: `internal/provider.go` (`Import`) +- Modify: `internal/drivers/delegation.go` (export a dual-fetch helper) +- Modify: `internal/drivers/delegation_test.go` +- Modify: `internal/provider_test.go` + +**Step 1: Failing tests.** +- `delegation_test.go` `TestDelegationImportRead_DualNS`: with a stub client whose `GetDomainDelegation` returns `["ns1.dnsimple.com"]` and a stub public-NS resolver returning `["ns1.digitalocean.com"]`, the new dual-fetch helper returns `Outputs{"nameservers":["ns1.dnsimple.com"], "registrar_nameservers":["ns1.dnsimple.com"], "live_nameservers":["ns1.digitalocean.com"], "domain_id":...}`. (`nameservers` == registrar = PRIMARY key so `Diff`/`parseDelegationSpec` stay consistent — finding I-NEW-3.) +- `provider_test.go` `TestImport_DelegationUsesRegistrarNotLiveRead`: `HoverProvider.Import(ctx, "x.com", "infra.dns_delegation")` returns a `ResourceState` whose `Outputs["registrar_nameservers"]` comes from `GetDomainDelegation` (NOT the live-first `Read` path) — assert via a stub where registrar≠live and confirm `registrar_nameservers`==registrar. +Run: `GOWORK=off GOTOOLCHAIN=auto go test ./internal ./internal/drivers -run 'Delegation|Import' -count=1 -v` → FAIL. + +**Step 2: Implement.** +- In `internal/drivers/delegation.go`, add an exported `func (d *DelegationDriver) ReadForImport(ctx, ref) (*interfaces.ResourceOutput, error)`: call `d.client.GetDomainDelegation(ctx, domain)` (registrar) for the authoritative NS; call the existing `lookupPublicNameservers(ctx, domain)` (live) best-effort (ignore error → omit `live_nameservers`); build `Outputs{"nameservers": registrar, "registrar_nameservers": registrar, "live_nameservers": live, "domain_id": ...}` (`[]any` values). Do NOT modify `Read` (drift path). +- In `internal/provider.go` `Import`, BEFORE the generic `d.Read` call: `if resourceType == "infra.dns_delegation" { if dd, ok := d.(*drivers.DelegationDriver); ok { out, err := dd.ReadForImport(ctx, ref); ... build/return ResourceState } }`. Fall through to `d.Read` for `infra.dns`. + +**Step 3:** `GOWORK=off GOTOOLCHAIN=auto go build ./... && GOWORK=off GOTOOLCHAIN=auto go vet ./... && GOWORK=off GOTOOLCHAIN=auto go test ./... -count=1` → all green (existing delegation Diff/Read tests stay green — `nameservers` key preserved). `golangci-lint run --new-from-rev=origin/main` → exit 0. + +**Step 4: Commit.** `git commit -m "feat(provider): Import dual-fetches registrar+live NS for delegation"` + +**Plugin runtime validation (Step 1b trigger — plugin loading path):** the live proof is the gocodealone-dns import re-run in Task 6; locally confirm the plugin still builds + loads (existing `TestPluginBinaryEmbedsManifest` green). After merge → release **v0.5.1**. + +Rollback: revert commits → delegation enumerate returns "not supported", Import unchanged; pin consumers back to v0.5.0. + +### Task 5: import-dns.yml — Hover delegation import (second) + fail-gate + +**Repo:** gocodealone-dns **Files:** +- Modify: `.github/workflows/import-dns.yml` + +**Step 1: Add the delegation import step** AFTER the existing `Import Hover DNS zones` step, `id: import-hover-delegation`, `continue-on-error: true`, same `HOVER_*` env, same `--config infra/hover.wfctl.yaml --provider hover --plugin-dir data/plugins`, but `--type infra.dns_delegation` and `-o zones/hover.portfolio.json` (it reads the shared `.state/hover/` populated by BOTH imports → emits the MERGED records+authority portfolio, overwriting the records-only one — finding I-NEW-4). The first (`infra.dns`) Hover step keeps `-o zones/hover.portfolio.json` too (records-only, then overwritten). Ensure both Hover steps use the SAME default `browser_profile_dir` (no override → shared `$XDG_STATE` profile → cookie reuse, finding m-2). + +**Step 2: Update the fail-gate** (`Fail run if any provider import failed`, finding I-3): add `[ "${{ steps.import-hover-delegation.outcome }}" = "failure" ] && failed="$failed hover-delegation"`. + +**Step 3: Lint the workflow.** `actionlint .github/workflows/import-dns.yml` (or confirm YAML parses); the real exercise is Task 6's dispatch. + +**Step 4: Commit.** `git commit -m "ci(dns): import Hover delegation (both layers) + fail-gate"` + +Rollback: revert commit → import returns to records-only. + +### Task 6: Pin bumps + live catalog validation + +**Repo:** gocodealone-dns **Files:** +- Modify: `.github/wfctl-version` (bump to the workflow release carrying Task 1/2) +- Modify: `wfctl.yaml` + `.wfctl-lock.yaml` (hover v0.5.0 → v0.5.1) + +**Precondition:** PR1 merged + workflow released; PR2 merged + hover **v0.5.1** released. + +**Step 1:** Bump `.github/wfctl-version` to the new workflow release tag; bump hover pin to v0.5.1 in both `wfctl.yaml` and `.wfctl-lock.yaml`. + +**Step 2: Commit + open PR3.** `git commit -m "chore(dns): bump wfctl + hover v0.5.1 for delegation catalog"` + +**Step 3: Multi-component live validation (the real-consumer proof).** After PR3 merges, dispatch `import-dns.yml` on the self-hosted runner. Assert from the run log + the catalog PR's `zones/hover.portfolio.json`: +- `imported N infra.dns zones via provider "hover"` AND `imported N infra.dns_delegation` (or equivalent) — both steps succeed, fail-gate green. +- Snapshots carry `authority.registrar_nameservers` (non-empty) AND `records`. +- At least one domain shows `registrar_nameservers` ≠ a hover nameserver (delegated-away → its Hover records are staging/placeholder). +- Exactly one snapshot per `(provider,domain)` (no duplicates from the merge). +Expected: the resulting catalog-refresh PR shows both layers; no 401/ErrBotChallenge. + +Rollback: revert pins (wfctl + hover→v0.5.0) + the import step (Task 5) → catalog returns to records-only. From b75890aab260d0d02d12787aab4a840479d82ec1 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 11:43:49 -0400 Subject: [PATCH 06/11] =?UTF-8?q?docs(dns):=20plan=20rev2=20=E2=80=94=20fi?= =?UTF-8?q?x=20C-1=20(type-namespace=20import=20IDs)=20+=20C-2=20(--format?= =?UTF-8?q?=20portfolio)=20+=20I-1/2/3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds Task 2 (type-namespaced import-all state IDs so dns+delegation for one domain don't overwrite on disk) + a pre-merge wfctl merge test. Task 6 gets explicit --format portfolio + exact insertion point + fail-gate. Records:[] on authority-only snapshots. 7 tasks / 3 PRs. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-06-02-dns-delegation-portfolio.md | 169 +++++++++--------- 1 file changed, 88 insertions(+), 81 deletions(-) diff --git a/docs/plans/2026-06-02-dns-delegation-portfolio.md b/docs/plans/2026-06-02-dns-delegation-portfolio.md index fcf5f781..2d97a817 100644 --- a/docs/plans/2026-06-02-dns-delegation-portfolio.md +++ b/docs/plans/2026-06-02-dns-delegation-portfolio.md @@ -4,7 +4,7 @@ **Goal:** Capture registrar NS delegation alongside hosted records in the canonical DNS portfolio, so the catalog shows where each domain's DNS is live (`authority`) AND its hosted records per provider (`records`, incl. staging records before an NS switch). -**Architecture:** 3-repo cascade. (1) workflow engine: `FromResourceStates` merges `infra.dns` (records) + `infra.dns_delegation` (authority) by `(provider,domain)` into one Snapshot; `Sanitize` allow-lists Authority keys. (2) workflow-plugin-hover: `EnumerateAll` lists domains for `infra.dns_delegation`; `HoverProvider.Import` dual-fetches registrar (GetDomainDelegation) + live (public NS) — bypassing the live-first `Read` (drift unchanged). (3) gocodealone-dns: import-dns.yml adds a delegation import (runs second, owns the merged portfolio) + fail-gate + pin bumps. +**Architecture:** 3-repo cascade. (1) workflow engine: `FromResourceStates` merges `infra.dns` (records) + `infra.dns_delegation` (authority) by `(provider,domain)` into one Snapshot; import-all state IDs become type-namespaced so two resource types for one domain don't overwrite each other on disk; `Sanitize` allow-lists Authority keys. (2) workflow-plugin-hover: `EnumerateAll` lists domains for `infra.dns_delegation`; `HoverProvider.Import` dual-fetches registrar (GetDomainDelegation) + live (public NS) — bypassing the live-first `Read` (drift unchanged). (3) gocodealone-dns: import-dns.yml adds a delegation import (runs second, owns the merged portfolio) + fail-gate + pin bumps. **Tech Stack:** Go 1.26, workflow IaC provider/portfolio (`workflow.dns-portfolio.export.v1`), wfctl `infra import-all`, GitHub Actions self-hosted runner. @@ -17,24 +17,24 @@ ## Scope Manifest **PR Count:** 3 -**Tasks:** 6 -**Estimated Lines of Change:** ~450 (informational; not enforced) +**Tasks:** 7 +**Estimated Lines of Change:** ~550 (informational; not enforced) **Out of scope:** - DO (or other-provider) delegation enumeration — DO NS are self-referential; the registrar (Hover) holds the delegation truth. Future-optional. - Changing `DelegationDriver.Read` / drift / apply semantics — explicitly left as-is (zero drift blast radius). - Any DNS mutation — import is read-only; no `apply`/`SetNameservers`. -- A new wfctl portfolio-export subcommand (Option F) — reuse `import-all --format portfolio`. +- A new wfctl portfolio-export subcommand — reuse `import-all --format portfolio`. **PR Grouping:** | PR # | Title | Tasks | Branch | Repo | |------|-------|-------|--------|------| -| 1 | Portfolio: merge infra.dns_delegation into Snapshot.Authority | Task 1, Task 2 | feat/dns-delegation-portfolio | workflow | -| 2 | Hover: enumerate + import infra.dns_delegation (registrar+live NS) | Task 3, Task 4 | feat/hover-delegation-enumerate | workflow-plugin-hover | -| 3 | DNS catalog: import Hover delegation, both layers | Task 5, Task 6 | chore/dns-catalog-delegation | gocodealone-dns | +| 1 | Portfolio: type-namespaced import IDs + merge delegation into Authority | Task 1, Task 2, Task 3 | feat/dns-delegation-portfolio | workflow | +| 2 | Hover: enumerate + import infra.dns_delegation (registrar+live NS) | Task 4, Task 5 | feat/hover-delegation-enumerate | workflow-plugin-hover | +| 3 | DNS catalog: import Hover delegation, both layers | Task 6, Task 7 | chore/dns-catalog-delegation | gocodealone-dns | -**Deploy ordering (load-bearing):** PR1 merge → workflow/wfctl release; PR2 merge → hover v0.5.1 release; THEN PR3 (bumps both pins, re-runs import). PR3's validation depends on both releases. +**Deploy ordering (load-bearing):** PR1 merge → workflow/wfctl release (minor — behavioral change to `FromResourceStates` + import-all state IDs); PR2 merge → hover v0.5.1 release; THEN PR3 (bumps both pins, re-runs import). PR3 is independently revertible (revert pins) but NOT independently deployable. **Status:** Draft @@ -42,126 +42,133 @@ ### Task 1: `FromResourceStates` merges delegation into `Snapshot.Authority` -**Repo:** workflow **Files:** -- Modify: `dns/record/canonicalize.go` -- Modify: `dns/record/canonicalize_test.go` +**Repo:** workflow **Files:** Modify `dns/record/canonicalize.go`, `dns/record/canonicalize_test.go` -**Step 1: Write/adjust failing tests.** In `canonicalize_test.go`: -- Update `TestFromResourceStatesSkipsNonDNS` (finding T-1): a genuinely-unknown type (e.g. `infra.compute`) is still skipped (0 snapshots), but `infra.dns_delegation` is now CONSUMED. -- `TestFromResourceStates_DelegationPopulatesAuthority`: a state `{Type:"infra.dns_delegation", Provider:"hover", ProviderID:"x.com", Outputs:{"registrar_nameservers":["ns1.dnsimple.com"],"live_nameservers":["ns1.digitalocean.com"]}}` → one snapshot with `Authority["registrar_nameservers"]==["ns1.dnsimple.com"]` and `Authority["live_nameservers"]==["ns1.digitalocean.com"]`, `Records` empty. -- `TestFromResourceStates_MergesBothLayersByDomain`: an `infra.dns` state + an `infra.dns_delegation` state, SAME `(provider="hover", domain="x.com")` → exactly ONE snapshot carrying both `Records` (from infra.dns) and `Authority` (from delegation). -- `TestFromResourceStates_DelegationOnlyDomain` (finding m-1): delegation state with no matching infra.dns → one authority-only snapshot, `Records: []` (non-nil-or-empty per existing Validate). -- Keep existing records-only behavior: N `infra.dns` states → N snapshots. +**Step 1: Tests.** In `canonicalize_test.go`: +- Update `TestFromResourceStatesSkipsNonDNS` (finding T-1): a genuinely-unknown type (`infra.compute`) is still skipped (0 snapshots); `infra.dns_delegation` is now CONSUMED. +- `TestFromResourceStates_DelegationPopulatesAuthority`: state `{Type:"infra.dns_delegation", Provider:"hover", ProviderID:"x.com", Outputs:{"registrar_nameservers":[]any{"ns1.dnsimple.com"},"live_nameservers":[]any{"ns1.digitalocean.com"}}}` → one snapshot, `Authority["registrar_nameservers"]==[]any{"ns1.dnsimple.com"}`, `Authority["live_nameservers"]==[]any{"ns1.digitalocean.com"}`, `Records != nil && len(Records)==0`. +- `TestFromResourceStates_MergesBothLayersByDomain`: an `infra.dns` state + an `infra.dns_delegation` state, SAME `(provider="hover", ProviderID="x.com")` → exactly ONE snapshot carrying both `Records` and `Authority`. +- `TestFromResourceStates_DelegationOnlyDomain` (finding m-1): delegation-only → authority-only snapshot, `Records: []Record{}` (non-nil → JSON `"records":[]`, finding I-NEW-2). +- Records-only unchanged: N `infra.dns` states → N snapshots. -Run: `GOWORK=off GOTOOLCHAIN=auto go test ./dns/record -run TestFromResourceStates -count=1 -v` → expect FAIL. +Run: `GOWORK=off GOTOOLCHAIN=auto go test ./dns/record -run TestFromResourceStates -count=1 -v` → FAIL. -**Step 2: Implement.** Restructure `FromResourceStates` to group by key `provider+"\x00"+domain`: -- Iterate states; resolve `domain` (ProviderID, fall back `AppliedConfig["domain"]`); skip if domain empty. -- For `infra.dns`: get-or-create the snapshot, append `Records` (existing `pickRecords`/`recordFromMap`). -- For `infra.dns_delegation`: get-or-create the snapshot, set `Authority` from `st.Outputs` reading `registrar_nameservers` + `live_nameservers` (each `[]any`→`[]string` via a small helper; copy only those keys — do NOT copy the whole Outputs map). Tolerate missing key (omit it). -- For any other type: `continue` (still skipped). -- Emit snapshots sorted by `(provider, domain)` for determinism. -- Preserve `ID` (first state that creates the group sets it). +**Step 2: Implement.** Restructure to group by `provider+"\x00"+domain`: +- get-or-create the snapshot per key; initialize `Records: []Record{}` (non-nil). +- `infra.dns` → append `Records` (existing `pickRecords`/`recordFromMap`). +- `infra.dns_delegation` → set `Authority` reading ONLY `registrar_nameservers` + `live_nameservers` from `st.Outputs` (each `[]any`; copy only those keys — never the whole Outputs map; omit a missing key). +- other types → `continue`. +- emit sorted by `(provider, domain)`; first state to create a group sets `ID`. -Run the tests → expect PASS. +Run tests → PASS. -**Step 3: Full package + repo verification.** -`GOWORK=off GOTOOLCHAIN=auto go test ./dns/record ./cmd/wfctl -count=1` (cmd/wfctl exercises dumpPortfolioToFile) → all green. -`GOWORK=off GOTOOLCHAIN=auto golangci-lint run --new-from-rev=origin/main ./dns/... ./cmd/wfctl/...` → exit 0. +**Step 3: Verify.** `GOWORK=off GOTOOLCHAIN=auto go test ./dns/record -count=1` green; `golangci-lint run --new-from-rev=origin/main ./dns/...` exit 0. **Step 4: Commit.** `git commit -m "feat(dns): merge infra.dns_delegation into Snapshot.Authority"` -Rollback: revert commit → `FromResourceStates` returns to records-only; no schema break. (Runtime-affecting via wfctl release — see deploy ordering.) +Rollback: revert → records-only; no schema break. (Runtime-affecting via wfctl release.) -### Task 2: `Sanitize` allow-lists `Authority` keys +### Task 2: Type-namespace import-all state IDs (fixes overwrite collision) -**Repo:** workflow **Files:** -- Modify: `dns/record/sanitize.go` -- Modify: `dns/record/sanitize_test.go` +**Repo:** workflow **Files:** Modify `cmd/wfctl/infra_import_all.go` (`buildResourceStateFromImport`), `cmd/wfctl/infra_import_all_format_test.go` -**Step 1: Failing test.** `TestSanitizeStripsUnknownAuthorityKeys`: a snapshot with `Authority{"registrar_nameservers":[...],"live_nameservers":[...],"secret_token":"x"}` → after `Sanitize`, `registrar_nameservers`+`live_nameservers` remain, `secret_token`+any key ∉ allow-list `{registrar_nameservers, live_nameservers, domain_id}` removed. Records sanitization unchanged. -Run: `GOWORK=off GOTOOLCHAIN=auto go test ./dns/record -run TestSanitize -count=1 -v` → FAIL. +**Why:** `buildResourceStateFromImport` sets `spec.Name = sanitizeImportedZoneName(zoneName)` → `resourceStateFromImportedState` sets `ID = spec.Name` (infra.go:38) → `SaveResource` writes `sanitizeStateID(ID)+".json"`. For one domain, `infra.dns` and `infra.dns_delegation` imports produce the SAME ID/file → the second OVERWRITES the first → the portfolio merge never sees both layers (adversarial CRITICAL-1, verified). -**Step 2: Implement.** In `Sanitize`, after the Records pass, for each snapshot with non-nil `Authority`, delete keys not in the allow-list. (NS are public; this guards future callers from leaking non-NS data via the free-form `map[string]any`.) +**Step 1: Tests.** In `infra_import_all_format_test.go`: +- `TestDumpPortfolio_MergesDnsAndDelegationForSameDomain`: pre-populate a state store with an `infra.dns` state AND an `infra.dns_delegation` state for the SAME domain (distinct IDs now), run `dumpPortfolioToFile`, assert the output has ONE snapshot for that domain carrying both `records` (non-empty) and `authority.registrar_nameservers`. This catches CRITICAL-1 at unit level, pre-merge (finding I-NEW-3). +- `TestBuildResourceStateFromImport_TypeNamespacedID`: `buildResourceStateFromImport("example.com","example.com","infra.dns","hover",...)` and the same with `"infra.dns_delegation"` produce DISTINCT `ID`s (so distinct on-disk files), while both retain `ProviderID == "example.com"` (domain unchanged). -**Step 3:** `GOWORK=off GOTOOLCHAIN=auto go test ./dns/record -count=1` → green. `golangci-lint run --new-from-rev=origin/main ./dns/...` → exit 0. +Run → FAIL. + +**Step 2: Implement.** In `buildResourceStateFromImport`, set `Name: resourceType + "/" + sanitizeImportedZoneName(zoneName)` (so `sanitizeStateID` maps `/`→`_` → `infra.dns_example-com.json` vs `infra.dns_delegation_example-com.json`). Do NOT change `ProviderID` (stays the bare domain via `cloudID`) — `FromResourceStates` keys the snapshot domain on `ProviderID`, so the portfolio domain is unaffected. Backward-compatible: single-type import-all runs have no collision; `.state/` dirs are ephemeral/gitignored so no orphan migration. + +Run tests → PASS. + +**Step 3: Verify.** `GOWORK=off GOTOOLCHAIN=auto go test ./cmd/wfctl -count=1` green; `golangci-lint run --new-from-rev=origin/main ./cmd/wfctl/...` exit 0. + +**Step 4: Commit.** `git commit -m "fix(wfctl): type-namespace import-all state IDs to avoid cross-type overwrite"` + +Rollback: revert → import IDs return to domain-only (single-type imports unaffected). (Runtime-affecting via wfctl release.) + +### Task 3: `Sanitize` allow-lists `Authority` keys + +**Repo:** workflow **Files:** Modify `dns/record/sanitize.go`, `dns/record/sanitize_test.go` + +**Step 1: Test.** `TestSanitizeStripsUnknownAuthorityKeys`: `Authority{"registrar_nameservers":...,"live_nameservers":...,"secret_token":"x"}` → after `Sanitize`, allow-list `{registrar_nameservers, live_nameservers, domain_id}` kept, others removed; Records sanitization unchanged. Run → FAIL. + +**Step 2: Implement.** After the Records pass, for each snapshot with non-nil `Authority`, delete keys ∉ allow-list. + +**Step 3:** `GOWORK=off GOTOOLCHAIN=auto go test ./dns/record -count=1` green; `golangci-lint run --new-from-rev=origin/main ./dns/...` exit 0. **Step 4: Commit.** `git commit -m "feat(dns): sanitize allow-lists Snapshot.Authority keys"` -Rollback: revert commit. +Rollback: revert. -### Task 3: Hover `EnumerateAll` lists domains for `infra.dns_delegation` +### Task 4: Hover `EnumerateAll` lists domains for `infra.dns_delegation` -**Repo:** workflow-plugin-hover **Files:** -- Modify: `internal/provider.go` (`EnumerateAll`) -- Modify: `internal/provider_test.go` +**Repo:** workflow-plugin-hover **Files:** Modify `internal/provider.go` (`EnumerateAll`), `internal/provider_test.go` -**Step 1: Failing test.** `TestEnumerateAll_DelegationListsDomains`: a stub domains-lister returning 2 domains → `EnumerateAll(ctx, "infra.dns_delegation")` returns 2 `ResourceOutput`s, each `Type=="infra.dns_delegation"`, `ProviderID==domain.Name`. Also assert an unknown type still errors `"resource type %q not supported"`. -Run: `GOWORK=off GOTOOLCHAIN=auto go test ./internal -run TestEnumerateAll -count=1 -v` → FAIL. +**Step 1: Test.** `TestEnumerateAll_DelegationListsDomains`: stub lister with 2 domains → `EnumerateAll(ctx,"infra.dns_delegation")` returns 2 `ResourceOutput`s, each `Type=="infra.dns_delegation"`, `ProviderID==domain.Name`; unknown type still errors `"resource type %q not supported"`. Run → FAIL. -**Step 2: Implement.** Change the `EnumerateAll` guard: accept `infra.dns` AND `infra.dns_delegation`; for either, `ListDomains` → emit `ResourceOutput{ProviderID:d.Name, Type:resourceType, Outputs: {"domain_id": d.ID}}` (delegation NS are fetched in Import, Task 4 — keep enumerate cheap: one `ListDomains` call). Reject other types as before. +**Step 2: Implement.** Accept `infra.dns` AND `infra.dns_delegation` in the guard; for either, `ListDomains` → emit `ResourceOutput{ProviderID:d.Name, Type:resourceType}` (delegation NS fetched in Import — keep enumerate to one `ListDomains` call; no per-domain `domain_id` Output needed). Reject other types. -**Step 3:** `GOWORK=off GOTOOLCHAIN=auto go test ./internal -count=1` → green. +**Step 3:** `GOWORK=off GOTOOLCHAIN=auto go test ./internal -count=1` green. **Step 4: Commit.** `git commit -m "feat(provider): EnumerateAll lists domains for infra.dns_delegation"` -### Task 4: Hover `Import` dual-fetches registrar + live NS (bypass Read) +### Task 5: Hover `Import` dual-fetches registrar + live NS (bypass Read) + +**Repo:** workflow-plugin-hover **Files:** Modify `internal/provider.go` (`Import`), `internal/drivers/delegation.go`, `internal/drivers/delegation_test.go`, `internal/provider_test.go` -**Repo:** workflow-plugin-hover **Files:** -- Modify: `internal/provider.go` (`Import`) -- Modify: `internal/drivers/delegation.go` (export a dual-fetch helper) -- Modify: `internal/drivers/delegation_test.go` -- Modify: `internal/provider_test.go` +**Step 1: Tests.** +- `delegation_test.go` `TestDelegationReadForImport_DualNS`: stub client `GetDomainDelegation`→`["ns1.dnsimple.com"]`, stub public resolver→`["ns1.digitalocean.com"]`; the new `ReadForImport` returns `Outputs{"nameservers":[]any{"ns1.dnsimple.com"}, "registrar_nameservers":[]any{"ns1.dnsimple.com"}, "live_nameservers":[]any{"ns1.digitalocean.com"}, "domain_id":...}`. (`nameservers`==registrar=PRIMARY so existing `Diff`/`parseDelegationSpec`/`nameserversFromOutputs` stay consistent — no spurious drift, finding I-NEW-3.) +- `TestDelegationReadForImport_LiveLookupFailsGracefully`: public resolver errors → `live_nameservers` omitted; `nameservers`/`registrar_nameservers` still from registrar. +- `provider_test.go` `TestImport_DelegationUsesRegistrarNotLiveRead`: with a stub where registrar≠live, `HoverProvider.Import(ctx,"x.com","infra.dns_delegation")` returns a `ResourceState` whose `Outputs["registrar_nameservers"]`==registrar (proves it bypassed the live-first `Read`). -**Step 1: Failing tests.** -- `delegation_test.go` `TestDelegationImportRead_DualNS`: with a stub client whose `GetDomainDelegation` returns `["ns1.dnsimple.com"]` and a stub public-NS resolver returning `["ns1.digitalocean.com"]`, the new dual-fetch helper returns `Outputs{"nameservers":["ns1.dnsimple.com"], "registrar_nameservers":["ns1.dnsimple.com"], "live_nameservers":["ns1.digitalocean.com"], "domain_id":...}`. (`nameservers` == registrar = PRIMARY key so `Diff`/`parseDelegationSpec` stay consistent — finding I-NEW-3.) -- `provider_test.go` `TestImport_DelegationUsesRegistrarNotLiveRead`: `HoverProvider.Import(ctx, "x.com", "infra.dns_delegation")` returns a `ResourceState` whose `Outputs["registrar_nameservers"]` comes from `GetDomainDelegation` (NOT the live-first `Read` path) — assert via a stub where registrar≠live and confirm `registrar_nameservers`==registrar. -Run: `GOWORK=off GOTOOLCHAIN=auto go test ./internal ./internal/drivers -run 'Delegation|Import' -count=1 -v` → FAIL. +Run → FAIL. **Step 2: Implement.** -- In `internal/drivers/delegation.go`, add an exported `func (d *DelegationDriver) ReadForImport(ctx, ref) (*interfaces.ResourceOutput, error)`: call `d.client.GetDomainDelegation(ctx, domain)` (registrar) for the authoritative NS; call the existing `lookupPublicNameservers(ctx, domain)` (live) best-effort (ignore error → omit `live_nameservers`); build `Outputs{"nameservers": registrar, "registrar_nameservers": registrar, "live_nameservers": live, "domain_id": ...}` (`[]any` values). Do NOT modify `Read` (drift path). -- In `internal/provider.go` `Import`, BEFORE the generic `d.Read` call: `if resourceType == "infra.dns_delegation" { if dd, ok := d.(*drivers.DelegationDriver); ok { out, err := dd.ReadForImport(ctx, ref); ... build/return ResourceState } }`. Fall through to `d.Read` for `infra.dns`. +- `internal/drivers/delegation.go`: add `func (d *DelegationDriver) ReadForImport(ctx, ref) (*interfaces.ResourceOutput, error)` — `GetDomainDelegation` (registrar, authoritative); `lookupPublicNameservers` best-effort (error → omit `live_nameservers`); build `Outputs{nameservers:registrar(primary), registrar_nameservers:registrar, live_nameservers:live, domain_id:...}` (`[]any` via existing `nameserversToAny`). Do NOT touch `Read`. +- `internal/provider.go` `Import`: BEFORE the generic `d.Read`, `if resourceType == "infra.dns_delegation" { dd, ok := d.(*drivers.DelegationDriver); if ok { out, err := dd.ReadForImport(...); build+return ResourceState } }`. (`drivers` is already imported at provider.go:14; `p.ResourceDriver("infra.dns_delegation")` returns the `*DelegationDriver`.) Fall through to `d.Read` for `infra.dns`. -**Step 3:** `GOWORK=off GOTOOLCHAIN=auto go build ./... && GOWORK=off GOTOOLCHAIN=auto go vet ./... && GOWORK=off GOTOOLCHAIN=auto go test ./... -count=1` → all green (existing delegation Diff/Read tests stay green — `nameservers` key preserved). `golangci-lint run --new-from-rev=origin/main` → exit 0. +**Step 3: Verify.** `GOWORK=off GOTOOLCHAIN=auto go build ./... && go vet ./... && go test ./... -count=1` all green (existing delegation `Diff`/`Read` tests stay green — `nameservers` primary key preserved). `golangci-lint run --new-from-rev=origin/main` exit 0. **Step 4: Commit.** `git commit -m "feat(provider): Import dual-fetches registrar+live NS for delegation"` -**Plugin runtime validation (Step 1b trigger — plugin loading path):** the live proof is the gocodealone-dns import re-run in Task 6; locally confirm the plugin still builds + loads (existing `TestPluginBinaryEmbedsManifest` green). After merge → release **v0.5.1**. +**Plugin runtime validation (Step 1b — plugin loading path):** live proof is Task 7's import re-run; locally confirm the plugin builds + `TestPluginBinaryEmbedsManifest` green. After merge → release **v0.5.1**. -Rollback: revert commits → delegation enumerate returns "not supported", Import unchanged; pin consumers back to v0.5.0. +Rollback: revert → delegation enumerate "not supported", Import unchanged; pin consumers back to v0.5.0. -### Task 5: import-dns.yml — Hover delegation import (second) + fail-gate +### Task 6: import-dns.yml — Hover delegation import (second) + fail-gate -**Repo:** gocodealone-dns **Files:** -- Modify: `.github/workflows/import-dns.yml` +**Repo:** gocodealone-dns **Files:** Modify `.github/workflows/import-dns.yml` -**Step 1: Add the delegation import step** AFTER the existing `Import Hover DNS zones` step, `id: import-hover-delegation`, `continue-on-error: true`, same `HOVER_*` env, same `--config infra/hover.wfctl.yaml --provider hover --plugin-dir data/plugins`, but `--type infra.dns_delegation` and `-o zones/hover.portfolio.json` (it reads the shared `.state/hover/` populated by BOTH imports → emits the MERGED records+authority portfolio, overwriting the records-only one — finding I-NEW-4). The first (`infra.dns`) Hover step keeps `-o zones/hover.portfolio.json` too (records-only, then overwritten). Ensure both Hover steps use the SAME default `browser_profile_dir` (no override → shared `$XDG_STATE` profile → cookie reuse, finding m-2). +**Step 1: Add the delegation import step** with `id: import-hover-delegation`, inserted **AFTER** the `Import Hover DNS zones` step (id: import-hover) and **BEFORE** the `Derive ownership from _workflow-dns-policy TXT (Hover)` step (finding I-1, so ownership derivation reads the merged portfolio). `continue-on-error: true`, same `HOVER_*` env, same `mkdir -p zones` + `wfctl infra import-all --config infra/hover.wfctl.yaml --provider hover --type infra.dns_delegation --format portfolio --plugin-dir data/plugins -o zones/hover.portfolio.json`. **`--format portfolio` is REQUIRED** (finding C-2; default is `state`). It reads the shared `.state/hover/` (now containing BOTH type-namespaced states after Task 2) → emits the MERGED records+authority portfolio, overwriting the records-only file from `import-hover`. Both Hover steps use the default `browser_profile_dir` (no override → shared `$XDG_STATE` profile → cookie reuse, finding m-2). No edit to `infra/hover.wfctl.yaml` (the provider registers both drivers at Initialize). **Step 2: Update the fail-gate** (`Fail run if any provider import failed`, finding I-3): add `[ "${{ steps.import-hover-delegation.outcome }}" = "failure" ] && failed="$failed hover-delegation"`. -**Step 3: Lint the workflow.** `actionlint .github/workflows/import-dns.yml` (or confirm YAML parses); the real exercise is Task 6's dispatch. +**Step 3: Verify.** `actionlint .github/workflows/import-dns.yml` (or confirm YAML parses) → exit 0. Real exercise is Task 7's dispatch. **Step 4: Commit.** `git commit -m "ci(dns): import Hover delegation (both layers) + fail-gate"` -Rollback: revert commit → import returns to records-only. +Rollback: revert → import returns to records-only. -### Task 6: Pin bumps + live catalog validation +### Task 7: Pin bumps + live catalog validation -**Repo:** gocodealone-dns **Files:** -- Modify: `.github/wfctl-version` (bump to the workflow release carrying Task 1/2) -- Modify: `wfctl.yaml` + `.wfctl-lock.yaml` (hover v0.5.0 → v0.5.1) +**Repo:** gocodealone-dns **Files:** Modify `.github/wfctl-version`, `wfctl.yaml`, `.wfctl-lock.yaml` -**Precondition:** PR1 merged + workflow released; PR2 merged + hover **v0.5.1** released. +**Precondition:** PR1 merged + workflow released (minor bump — behavioral); PR2 merged + hover **v0.5.1** released. -**Step 1:** Bump `.github/wfctl-version` to the new workflow release tag; bump hover pin to v0.5.1 in both `wfctl.yaml` and `.wfctl-lock.yaml`. +**Step 1:** Bump `.github/wfctl-version` to the new workflow release (minor — `FromResourceStates`/import-all behavioral change); bump hover v0.5.0→v0.5.1 in `wfctl.yaml` + `.wfctl-lock.yaml`. **Step 2: Commit + open PR3.** `git commit -m "chore(dns): bump wfctl + hover v0.5.1 for delegation catalog"` -**Step 3: Multi-component live validation (the real-consumer proof).** After PR3 merges, dispatch `import-dns.yml` on the self-hosted runner. Assert from the run log + the catalog PR's `zones/hover.portfolio.json`: -- `imported N infra.dns zones via provider "hover"` AND `imported N infra.dns_delegation` (or equivalent) — both steps succeed, fail-gate green. -- Snapshots carry `authority.registrar_nameservers` (non-empty) AND `records`. -- At least one domain shows `registrar_nameservers` ≠ a hover nameserver (delegated-away → its Hover records are staging/placeholder). -- Exactly one snapshot per `(provider,domain)` (no duplicates from the merge). -Expected: the resulting catalog-refresh PR shows both layers; no 401/ErrBotChallenge. +**Step 3: Multi-component live validation (real-consumer proof).** After PR3 merges, dispatch `import-dns.yml` on the self-hosted runner. Assert from the run log + the catalog PR's `zones/hover.portfolio.json`: +- both `import-hover` and `import-hover-delegation` succeed; fail-gate green. +- portfolio `schema == "workflow.dns-portfolio.export.v1"` (confirms `--format portfolio` took). +- snapshots carry `authority.registrar_nameservers` (non-empty) AND `records` (non-empty for at least the Hover-hosted domains) — proves the merge produced BOTH layers in ONE snapshot (the CRITICAL-1 regression check). +- exactly one snapshot per `(provider,domain)` (no duplicates). +- at least one domain shows `registrar_nameservers` ≠ a hover nameserver (delegated-away → its Hover records are staging/placeholder). +Expected: catalog-refresh PR shows both layers; no 401/ErrBotChallenge. -Rollback: revert pins (wfctl + hover→v0.5.0) + the import step (Task 5) → catalog returns to records-only. +Rollback: revert pins (wfctl + hover→v0.5.0) + the Task 6 import step → catalog returns to records-only. From 3763fc6976cfef1367186540d2e45e40ae0b272e Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 11:50:10 -0400 Subject: [PATCH 07/11] =?UTF-8?q?docs(dns):=20plan=20rev3=20=E2=80=94=20de?= =?UTF-8?q?rive=20snap.ID=20(no=20slash),=20drop=20unused=20domain=5Fid,?= =?UTF-8?q?=20clarify=20o.Type=20advisory?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves plan-adversarial rev2 Importants: I-NEW-1 (snap.ID from provider+domain, not type-namespaced st.ID), I-NEW-2 (o.Type advisory; --type is authoritative), domain_id dropped from delegation Outputs + Sanitize allow-list. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/plans/2026-06-02-dns-delegation-portfolio.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/plans/2026-06-02-dns-delegation-portfolio.md b/docs/plans/2026-06-02-dns-delegation-portfolio.md index 2d97a817..44e9281a 100644 --- a/docs/plans/2026-06-02-dns-delegation-portfolio.md +++ b/docs/plans/2026-06-02-dns-delegation-portfolio.md @@ -47,7 +47,7 @@ **Step 1: Tests.** In `canonicalize_test.go`: - Update `TestFromResourceStatesSkipsNonDNS` (finding T-1): a genuinely-unknown type (`infra.compute`) is still skipped (0 snapshots); `infra.dns_delegation` is now CONSUMED. - `TestFromResourceStates_DelegationPopulatesAuthority`: state `{Type:"infra.dns_delegation", Provider:"hover", ProviderID:"x.com", Outputs:{"registrar_nameservers":[]any{"ns1.dnsimple.com"},"live_nameservers":[]any{"ns1.digitalocean.com"}}}` → one snapshot, `Authority["registrar_nameservers"]==[]any{"ns1.dnsimple.com"}`, `Authority["live_nameservers"]==[]any{"ns1.digitalocean.com"}`, `Records != nil && len(Records)==0`. -- `TestFromResourceStates_MergesBothLayersByDomain`: an `infra.dns` state + an `infra.dns_delegation` state, SAME `(provider="hover", ProviderID="x.com")` → exactly ONE snapshot carrying both `Records` and `Authority`. +- `TestFromResourceStates_MergesBothLayersByDomain`: an `infra.dns` state + an `infra.dns_delegation` state, SAME `(provider="hover", ProviderID="x.com")` → exactly ONE snapshot carrying both `Records` and `Authority`; assert `snap.ID` contains NO `/` and equals `"hover-x-com"` (derived from provider+domain, NOT the type-namespaced `st.ID` — finding I-NEW-1). - `TestFromResourceStates_DelegationOnlyDomain` (finding m-1): delegation-only → authority-only snapshot, `Records: []Record{}` (non-nil → JSON `"records":[]`, finding I-NEW-2). - Records-only unchanged: N `infra.dns` states → N snapshots. @@ -58,7 +58,8 @@ Run: `GOWORK=off GOTOOLCHAIN=auto go test ./dns/record -run TestFromResourceStat - `infra.dns` → append `Records` (existing `pickRecords`/`recordFromMap`). - `infra.dns_delegation` → set `Authority` reading ONLY `registrar_nameservers` + `live_nameservers` from `st.Outputs` (each `[]any`; copy only those keys — never the whole Outputs map; omit a missing key). - other types → `continue`. -- emit sorted by `(provider, domain)`; first state to create a group sets `ID`. +- set `snap.ID = provider + "-" + sanitizeDomainForID(domain)` (add a tiny unexported helper in `dns/record` that lowercases + replaces runs of non-alphanumeric — incl. `.` and `/` — with `-`). Do NOT inherit `st.ID` (Task 2 makes it type-namespaced, e.g. `infra.dns/x.com`, which would leak a `/` into the portfolio JSON — finding I-NEW-1). The domain comes from `st.ProviderID` (the bare domain), so it's stable across both layers. +- emit sorted by `(provider, domain)`. Run tests → PASS. @@ -94,9 +95,9 @@ Rollback: revert → import IDs return to domain-only (single-type imports unaff **Repo:** workflow **Files:** Modify `dns/record/sanitize.go`, `dns/record/sanitize_test.go` -**Step 1: Test.** `TestSanitizeStripsUnknownAuthorityKeys`: `Authority{"registrar_nameservers":...,"live_nameservers":...,"secret_token":"x"}` → after `Sanitize`, allow-list `{registrar_nameservers, live_nameservers, domain_id}` kept, others removed; Records sanitization unchanged. Run → FAIL. +**Step 1: Test.** `TestSanitizeStripsUnknownAuthorityKeys`: `Authority{"registrar_nameservers":...,"live_nameservers":...,"secret_token":"x"}` → after `Sanitize`, allow-list `{registrar_nameservers, live_nameservers}` kept, others removed; Records sanitization unchanged. Run → FAIL. -**Step 2: Implement.** After the Records pass, for each snapshot with non-nil `Authority`, delete keys ∉ allow-list. +**Step 2: Implement.** After the Records pass, for each snapshot with non-nil `Authority`, delete keys ∉ allow-list `{registrar_nameservers, live_nameservers}`. **Step 3:** `GOWORK=off GOTOOLCHAIN=auto go test ./dns/record -count=1` green; `golangci-lint run --new-from-rev=origin/main ./dns/...` exit 0. @@ -108,7 +109,7 @@ Rollback: revert. **Repo:** workflow-plugin-hover **Files:** Modify `internal/provider.go` (`EnumerateAll`), `internal/provider_test.go` -**Step 1: Test.** `TestEnumerateAll_DelegationListsDomains`: stub lister with 2 domains → `EnumerateAll(ctx,"infra.dns_delegation")` returns 2 `ResourceOutput`s, each `Type=="infra.dns_delegation"`, `ProviderID==domain.Name`; unknown type still errors `"resource type %q not supported"`. Run → FAIL. +**Step 1: Test.** `TestEnumerateAll_DelegationListsDomains`: stub lister with 2 domains → `EnumerateAll(ctx,"infra.dns_delegation")` returns 2 `ResourceOutput`s, `ProviderID==domain.Name` for each; unknown type still errors `"resource type %q not supported"`. (Note: `o.Type` on the output is advisory — the authoritative resource type for state persistence is the `--type` flag threaded as `resourceType` into `buildResourceStateFromImport`, exercised by Task 2's `TestBuildResourceStateFromImport_TypeNamespacedID`; this test just verifies the domain listing — finding I-NEW-2.) Run → FAIL. **Step 2: Implement.** Accept `infra.dns` AND `infra.dns_delegation` in the guard; for either, `ListDomains` → emit `ResourceOutput{ProviderID:d.Name, Type:resourceType}` (delegation NS fetched in Import — keep enumerate to one `ListDomains` call; no per-domain `domain_id` Output needed). Reject other types. @@ -121,14 +122,14 @@ Rollback: revert. **Repo:** workflow-plugin-hover **Files:** Modify `internal/provider.go` (`Import`), `internal/drivers/delegation.go`, `internal/drivers/delegation_test.go`, `internal/provider_test.go` **Step 1: Tests.** -- `delegation_test.go` `TestDelegationReadForImport_DualNS`: stub client `GetDomainDelegation`→`["ns1.dnsimple.com"]`, stub public resolver→`["ns1.digitalocean.com"]`; the new `ReadForImport` returns `Outputs{"nameservers":[]any{"ns1.dnsimple.com"}, "registrar_nameservers":[]any{"ns1.dnsimple.com"}, "live_nameservers":[]any{"ns1.digitalocean.com"}, "domain_id":...}`. (`nameservers`==registrar=PRIMARY so existing `Diff`/`parseDelegationSpec`/`nameserversFromOutputs` stay consistent — no spurious drift, finding I-NEW-3.) +- `delegation_test.go` `TestDelegationReadForImport_DualNS`: stub client `GetDomainDelegation`→`["ns1.dnsimple.com"]`, stub public resolver→`["ns1.digitalocean.com"]`; the new `ReadForImport` returns `Outputs{"nameservers":[]any{"ns1.dnsimple.com"}, "registrar_nameservers":[]any{"ns1.dnsimple.com"}, "live_nameservers":[]any{"ns1.digitalocean.com"}}`. (`nameservers`==registrar=PRIMARY so existing `Diff`/`parseDelegationSpec`/`nameserversFromOutputs` stay consistent — no spurious drift, finding I-NEW-3.) - `TestDelegationReadForImport_LiveLookupFailsGracefully`: public resolver errors → `live_nameservers` omitted; `nameservers`/`registrar_nameservers` still from registrar. - `provider_test.go` `TestImport_DelegationUsesRegistrarNotLiveRead`: with a stub where registrar≠live, `HoverProvider.Import(ctx,"x.com","infra.dns_delegation")` returns a `ResourceState` whose `Outputs["registrar_nameservers"]`==registrar (proves it bypassed the live-first `Read`). Run → FAIL. **Step 2: Implement.** -- `internal/drivers/delegation.go`: add `func (d *DelegationDriver) ReadForImport(ctx, ref) (*interfaces.ResourceOutput, error)` — `GetDomainDelegation` (registrar, authoritative); `lookupPublicNameservers` best-effort (error → omit `live_nameservers`); build `Outputs{nameservers:registrar(primary), registrar_nameservers:registrar, live_nameservers:live, domain_id:...}` (`[]any` via existing `nameserversToAny`). Do NOT touch `Read`. +- `internal/drivers/delegation.go`: add `func (d *DelegationDriver) ReadForImport(ctx, ref) (*interfaces.ResourceOutput, error)` — `GetDomainDelegation` (registrar, authoritative); `lookupPublicNameservers` best-effort (error → omit `live_nameservers`); build `Outputs{nameservers:registrar(primary), registrar_nameservers:registrar, live_nameservers:live}` (`[]any` via existing `nameserversToAny`; omit the unused `domain_id`). Do NOT touch `Read`. - `internal/provider.go` `Import`: BEFORE the generic `d.Read`, `if resourceType == "infra.dns_delegation" { dd, ok := d.(*drivers.DelegationDriver); if ok { out, err := dd.ReadForImport(...); build+return ResourceState } }`. (`drivers` is already imported at provider.go:14; `p.ResourceDriver("infra.dns_delegation")` returns the `*DelegationDriver`.) Fall through to `d.Read` for `infra.dns`. **Step 3: Verify.** `GOWORK=off GOTOOLCHAIN=auto go build ./... && go vet ./... && go test ./... -count=1` all green (existing delegation `Diff`/`Read` tests stay green — `nameservers` primary key preserved). `golangci-lint run --new-from-rev=origin/main` exit 0. From bca0a5fc673750e10fbffb46fc537d41325b0870 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 11:51:47 -0400 Subject: [PATCH 08/11] chore: lock scope for dns-delegation-portfolio (alignment passed) --- docs/plans/2026-06-02-dns-delegation-portfolio.md | 2 +- docs/plans/2026-06-02-dns-delegation-portfolio.md.scope-lock | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 docs/plans/2026-06-02-dns-delegation-portfolio.md.scope-lock diff --git a/docs/plans/2026-06-02-dns-delegation-portfolio.md b/docs/plans/2026-06-02-dns-delegation-portfolio.md index 44e9281a..43fdb8da 100644 --- a/docs/plans/2026-06-02-dns-delegation-portfolio.md +++ b/docs/plans/2026-06-02-dns-delegation-portfolio.md @@ -36,7 +36,7 @@ **Deploy ordering (load-bearing):** PR1 merge → workflow/wfctl release (minor — behavioral change to `FromResourceStates` + import-all state IDs); PR2 merge → hover v0.5.1 release; THEN PR3 (bumps both pins, re-runs import). PR3 is independently revertible (revert pins) but NOT independently deployable. -**Status:** Draft +**Status:** Locked 2026-06-02T15:51:47Z --- diff --git a/docs/plans/2026-06-02-dns-delegation-portfolio.md.scope-lock b/docs/plans/2026-06-02-dns-delegation-portfolio.md.scope-lock new file mode 100644 index 00000000..7610f334 --- /dev/null +++ b/docs/plans/2026-06-02-dns-delegation-portfolio.md.scope-lock @@ -0,0 +1 @@ +0aeb1aef093bf8d6b9f6330d3d44e31b72aa8149a00701f18a1aa6da9275d91f From d82e75a930d07fea6f89dd33f709da71c836843b Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 11:58:58 -0400 Subject: [PATCH 09/11] feat(dns): merge infra.dns_delegation into Snapshot.Authority FromResourceStates now groups states by (provider, domain) key so that infra.dns records and infra.dns_delegation authority NS data are merged into a single Snapshot per zone. Adds sanitizeDomainForID helper for deterministic slug IDs. Unknown state types continue to be skipped. Co-Authored-By: Claude Opus 4.8 (1M context) --- dns/record/canonicalize.go | 132 +++++++++++++++++++++++++++----- dns/record/canonicalize_test.go | 121 ++++++++++++++++++++++++++++- 2 files changed, 230 insertions(+), 23 deletions(-) diff --git a/dns/record/canonicalize.go b/dns/record/canonicalize.go index fa7e7a76..296199e3 100644 --- a/dns/record/canonicalize.go +++ b/dns/record/canonicalize.go @@ -1,10 +1,23 @@ package record -import "github.com/GoCodeAlone/workflow/interfaces" +import ( + "sort" + "strings" + "unicode" + + "github.com/GoCodeAlone/workflow/interfaces" +) // FromResourceStates converts imported IaC state into a canonical Portfolio. // Reads each infra.dns ResourceState's records (Outputs preferred, else // AppliedConfig), renaming provider-specific value keys to the canonical "value". +// Each infra.dns_delegation state populates Snapshot.Authority with +// registrar_nameservers and live_nameservers for the matching domain. +// States of other types are silently skipped. +// +// Snapshots are grouped by (Provider, Domain) so that infra.dns and +// infra.dns_delegation states for the same domain are merged into one Snapshot. +// Output is sorted by (Provider, Domain) for deterministic order. // // Provider value-key divergence (verified against provider drivers): // - DigitalOcean + Cloudflare emit "data" @@ -12,38 +25,115 @@ import "github.com/GoCodeAlone/workflow/interfaces" // - Namecheap emits "address" // // The valueAlias helper resolves the first non-empty of: data → content → address → value. -// Non-infra.dns states are silently skipped. func FromResourceStates(states []interfaces.ResourceState) Portfolio { p := Portfolio{Schema: SchemaV1} - for i := range states { - st := &states[i] - if st.Type != "infra.dns" { - continue + + type key struct{ provider, domain string } + order := []key{} + snapByKey := map[key]*Snapshot{} + + getOrCreate := func(provider, domain string) *Snapshot { + k := key{provider, domain} + if s, ok := snapByKey[k]; ok { + return s } - recs := pickRecords(st.Outputs, st.AppliedConfig) - snap := Snapshot{ - ID: st.ID, - Provider: st.Provider, - Domain: st.ProviderID, + s := &Snapshot{ + ID: provider + "-" + sanitizeDomainForID(domain), + Provider: provider, + Domain: domain, + Records: []Record{}, } - // Fall back to AppliedConfig["domain"] if ProviderID is empty. - if snap.Domain == "" { - if d, ok := st.AppliedConfig["domain"].(string); ok { - snap.Domain = d + snapByKey[k] = s + order = append(order, k) + return s + } + + for i := range states { + st := &states[i] + + switch st.Type { + case "infra.dns": + domain := st.ProviderID + if domain == "" { + if d, ok := st.AppliedConfig["domain"].(string); ok { + domain = d + } } - } - for _, raw := range recs { - m, ok := raw.(map[string]any) - if !ok { + if domain == "" { continue } - snap.Records = append(snap.Records, recordFromMap(m)) + snap := getOrCreate(st.Provider, domain) + recs := pickRecords(st.Outputs, st.AppliedConfig) + for _, raw := range recs { + m, ok := raw.(map[string]any) + if !ok { + continue + } + snap.Records = append(snap.Records, recordFromMap(m)) + } + + case "infra.dns_delegation": + domain := st.ProviderID + if domain == "" { + if d, ok := st.AppliedConfig["domain"].(string); ok { + domain = d + } + } + if domain == "" { + continue + } + snap := getOrCreate(st.Provider, domain) + for _, nsKey := range []string{"registrar_nameservers", "live_nameservers"} { + if v, ok := st.Outputs[nsKey]; ok { + if slice, ok := v.([]any); ok { + cp := make([]any, len(slice)) + copy(cp, slice) + if snap.Authority == nil { + snap.Authority = map[string]any{} + } + snap.Authority[nsKey] = cp + } + } + } + + default: + continue + } + } + + // Sort by (provider, domain) for deterministic output. + sort.Slice(order, func(i, j int) bool { + if order[i].provider != order[j].provider { + return order[i].provider < order[j].provider } - p.Snapshots = append(p.Snapshots, snap) + return order[i].domain < order[j].domain + }) + + for _, k := range order { + p.Snapshots = append(p.Snapshots, *snapByKey[k]) } return p } +// sanitizeDomainForID converts a domain string into an ID-safe slug: +// lowercase, runs of non-alphanumeric runes (incl. '.' and '/') replaced +// with a single '-', leading/trailing '-' trimmed. +func sanitizeDomainForID(s string) string { + s = strings.ToLower(s) + var b strings.Builder + inRun := false + for _, r := range s { + if unicode.IsLetter(r) || unicode.IsDigit(r) { + b.WriteRune(r) + inRun = false + } else if !inRun { + b.WriteByte('-') + inRun = true + } + } + return strings.Trim(b.String(), "-") +} + // pickRecords returns the records slice from Outputs if non-empty, // otherwise falls back to AppliedConfig. func pickRecords(outputs, appliedConfig map[string]any) []any { diff --git a/dns/record/canonicalize_test.go b/dns/record/canonicalize_test.go index 89af4c9e..ef4779b3 100644 --- a/dns/record/canonicalize_test.go +++ b/dns/record/canonicalize_test.go @@ -53,13 +53,130 @@ func TestFromResourceStatesAliasesValueKey(t *testing.T) { } func TestFromResourceStatesSkipsNonDNS(t *testing.T) { + // infra.compute is a truly-unknown type and must be silently skipped. + // infra.dns_delegation is now CONSUMED (not skipped), so it produces a snapshot. states := []interfaces.ResourceState{ - {Type: "infra.droplet", Provider: "digitalocean", ProviderID: "droplet-1"}, + {Type: "infra.compute", Provider: "digitalocean", ProviderID: "vm-1"}, {Type: "infra.spaces_key", Provider: "digitalocean", ProviderID: "key-1"}, } p := record.FromResourceStates(states) if len(p.Snapshots) != 0 { - t.Fatalf("non-dns states should be skipped; got %d snapshots", len(p.Snapshots)) + t.Fatalf("genuinely-unknown states should be skipped; got %d snapshots", len(p.Snapshots)) + } +} + +func TestFromResourceStates_DelegationPopulatesAuthority(t *testing.T) { + states := []interfaces.ResourceState{ + { + Type: "infra.dns_delegation", + Provider: "hover", + ProviderID: "x.com", + Outputs: map[string]any{ + "registrar_nameservers": []any{"ns1.dnsimple.com"}, + "live_nameservers": []any{"ns1.digitalocean.com"}, + }, + }, + } + p := record.FromResourceStates(states) + if len(p.Snapshots) != 1 { + t.Fatalf("want 1 snapshot from delegation state; got %d", len(p.Snapshots)) + } + snap := p.Snapshots[0] + if snap.ID != "hover-x-com" { + t.Errorf("want snap.ID == %q; got %q", "hover-x-com", snap.ID) + } + if snap.Records == nil { + t.Errorf("Records must be non-nil (empty slice, not null)") + } + if len(snap.Records) != 0 { + t.Errorf("want 0 records; got %d", len(snap.Records)) + } + rns, ok := snap.Authority["registrar_nameservers"] + if !ok { + t.Fatalf("Authority missing registrar_nameservers") + } + wantRNS := []any{"ns1.dnsimple.com"} + rnsSlice, ok := rns.([]any) + if !ok || len(rnsSlice) != len(wantRNS) || rnsSlice[0] != wantRNS[0] { + t.Errorf("registrar_nameservers = %v; want %v", rns, wantRNS) + } + lns, ok := snap.Authority["live_nameservers"] + if !ok { + t.Fatalf("Authority missing live_nameservers") + } + wantLNS := []any{"ns1.digitalocean.com"} + lnsSlice, ok := lns.([]any) + if !ok || len(lnsSlice) != len(wantLNS) || lnsSlice[0] != wantLNS[0] { + t.Errorf("live_nameservers = %v; want %v", lns, wantLNS) + } +} + +func TestFromResourceStates_MergesBothLayersByDomain(t *testing.T) { + states := []interfaces.ResourceState{ + { + Type: "infra.dns", + Provider: "hover", + ProviderID: "x.com", + Outputs: map[string]any{ + "records": []any{ + map[string]any{"type": "A", "name": "@", "content": "1.2.3.4", "ttl": 300}, + }, + }, + }, + { + Type: "infra.dns_delegation", + Provider: "hover", + ProviderID: "x.com", + Outputs: map[string]any{ + "registrar_nameservers": []any{"ns1.dnsimple.com"}, + "live_nameservers": []any{"ns1.digitalocean.com"}, + }, + }, + } + p := record.FromResourceStates(states) + if len(p.Snapshots) != 1 { + t.Fatalf("want exactly 1 merged snapshot; got %d", len(p.Snapshots)) + } + snap := p.Snapshots[0] + if snap.ID != "hover-x-com" { + t.Errorf("want snap.ID == %q; got %q", "hover-x-com", snap.ID) + } + if len(snap.Records) == 0 { + t.Errorf("want non-empty Records from infra.dns state") + } + if _, ok := snap.Authority["registrar_nameservers"]; !ok { + t.Errorf("want Authority[registrar_nameservers] from infra.dns_delegation state") + } + if _, ok := snap.Authority["live_nameservers"]; !ok { + t.Errorf("want Authority[live_nameservers] from infra.dns_delegation state") + } +} + +func TestFromResourceStates_DelegationOnlyDomain(t *testing.T) { + states := []interfaces.ResourceState{ + { + Type: "infra.dns_delegation", + Provider: "hover", + ProviderID: "x.com", + Outputs: map[string]any{ + "registrar_nameservers": []any{"ns1.dnsimple.com"}, + "live_nameservers": []any{"ns1.digitalocean.com"}, + }, + }, + } + p := record.FromResourceStates(states) + if len(p.Snapshots) != 1 { + t.Fatalf("want 1 authority-only snapshot; got %d", len(p.Snapshots)) + } + snap := p.Snapshots[0] + if snap.Records == nil { + t.Errorf("Records must be non-nil empty slice (so JSON marshals to [] not null)") + } + if len(snap.Records) != 0 { + t.Errorf("want 0 records; got %d", len(snap.Records)) + } + if snap.Authority == nil { + t.Errorf("want non-nil Authority") } } From 888f3c43bf05f45ba3410bd2b3a83fe424550b9a Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 12:04:57 -0400 Subject: [PATCH 10/11] fix(wfctl): type-namespace import-all state IDs to avoid cross-type overwrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Importing infra.dns then infra.dns_delegation for the same domain previously produced the same state ID (e.g. "example-com") and therefore the same on-disk filename, causing the second import to silently overwrite the first. buildResourceStateFromImport now prefixes the sanitized zone name with the resource type using "/" as a separator; sanitizeStateID maps "/" → "_", yielding distinct filenames: infra.dns_example-com.json infra.dns_delegation_example-com.json ProviderID is unchanged (bare domain) so record.FromResourceStates continues to merge both types into a single portfolio snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/wfctl/infra_import_all.go | 10 +- cmd/wfctl/infra_import_all_format_test.go | 124 ++++++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) diff --git a/cmd/wfctl/infra_import_all.go b/cmd/wfctl/infra_import_all.go index 342d4543..f209eb1a 100644 --- a/cmd/wfctl/infra_import_all.go +++ b/cmd/wfctl/infra_import_all.go @@ -227,8 +227,16 @@ func runInfraImportAllWithDeps(ctx context.Context, provider interfaces.IaCProvi // for ProviderID resolution + AppliedConfig hashing + timestamp normalization // so the synthesized state matches the single-resource import path exactly. func buildResourceStateFromImport(zoneName, cloudID, resourceType, providerType string, imported *interfaces.ResourceState) (interfaces.ResourceState, error) { + // Prefix the sanitized zone name with the resource type so that importing + // two different types (e.g. infra.dns and infra.dns_delegation) for the + // same domain produces DISTINCT IDs and therefore distinct on-disk + // filenames. sanitizeStateID maps "/" → "_", so + // "infra.dns/example-com" → infra.dns_example-com.json + // "infra.dns_delegation/example-com" → infra.dns_delegation_example-com.json + // ProviderID stays as the bare cloudID (domain) so record.FromResourceStates + // can group both types into a single portfolio snapshot by (Provider, Domain). spec := interfaces.ResourceSpec{ - Name: sanitizeImportedZoneName(zoneName), + Name: resourceType + "/" + sanitizeImportedZoneName(zoneName), Type: resourceType, } return resourceStateFromImportedState(spec, providerType, imported, cloudID) diff --git a/cmd/wfctl/infra_import_all_format_test.go b/cmd/wfctl/infra_import_all_format_test.go index 9a3c508f..a7250a68 100644 --- a/cmd/wfctl/infra_import_all_format_test.go +++ b/cmd/wfctl/infra_import_all_format_test.go @@ -156,3 +156,127 @@ func TestImportAllSanitizeFlagRequiresPortfolioFormat(t *testing.T) { t.Errorf("error should mention 'sanitize'; got: %v", err) } } + +// TestBuildResourceStateFromImport_TypeNamespacedID pins the CRITICAL-1 fix: +// importing infra.dns and infra.dns_delegation for the SAME domain must +// produce DISTINCT state IDs (and therefore distinct on-disk filenames) so +// that a second import does not overwrite the first. ProviderID must stay the +// bare domain in both cases — record.FromResourceStates keys the snapshot +// domain on ProviderID, not on ID. +func TestBuildResourceStateFromImport_TypeNamespacedID(t *testing.T) { + imported := &interfaces.ResourceState{ + ProviderID: "example.com", + Type: "infra.dns", + } + + dnsState, err := buildResourceStateFromImport("example.com", "example.com", "infra.dns", "hover", imported) + if err != nil { + t.Fatalf("buildResourceStateFromImport infra.dns: %v", err) + } + + delegImported := &interfaces.ResourceState{ + ProviderID: "example.com", + Type: "infra.dns_delegation", + } + delegState, err := buildResourceStateFromImport("example.com", "example.com", "infra.dns_delegation", "hover", delegImported) + if err != nil { + t.Fatalf("buildResourceStateFromImport infra.dns_delegation: %v", err) + } + + // IDs must be distinct so SaveResource writes to different filenames. + if dnsState.ID == delegState.ID { + t.Errorf("infra.dns and infra.dns_delegation produced the same ID %q; want distinct IDs", dnsState.ID) + } + + // ProviderID must remain the bare domain so FromResourceStates can group + // both states into a single snapshot. + if dnsState.ProviderID != "example.com" { + t.Errorf("infra.dns ProviderID = %q; want %q", dnsState.ProviderID, "example.com") + } + if delegState.ProviderID != "example.com" { + t.Errorf("infra.dns_delegation ProviderID = %q; want %q", delegState.ProviderID, "example.com") + } + + // Verify the on-disk filenames are also distinct via sanitizeStateID. + dnsFname := sanitizeStateID(dnsState.ID) + ".json" + delegFname := sanitizeStateID(delegState.ID) + ".json" + if dnsFname == delegFname { + t.Errorf("sanitized filenames are the same %q; want distinct files", dnsFname) + } +} + +// TestDumpPortfolio_MergesDnsAndDelegationForSameDomain pins the end-to-end +// portfolio merge contract (CRITICAL-1 class): when BOTH an infra.dns state +// and an infra.dns_delegation state for the same domain are present in the +// store (using type-namespaced IDs so they coexist), dumpPortfolioToFile must +// produce exactly ONE snapshot for that domain carrying both records and +// authority.registrar_nameservers. +func TestDumpPortfolio_MergesDnsAndDelegationForSameDomain(t *testing.T) { + store := &fakeStateStore{} + + // infra.dns state — type-namespaced ID, bare domain as ProviderID. + _ = store.SaveResource(context.Background(), interfaces.ResourceState{ + ID: "infra.dns/example-com", + Name: "infra.dns/example-com", + Type: "infra.dns", + Provider: "hover", + ProviderID: "example.com", + Outputs: map[string]any{ + "records": []any{ + map[string]any{"type": "A", "name": "@", "data": "192.0.2.1", "ttl": 300}, + }, + }, + }) + + // infra.dns_delegation state — distinct ID, same bare domain as ProviderID. + _ = store.SaveResource(context.Background(), interfaces.ResourceState{ + ID: "infra.dns_delegation/example-com", + Name: "infra.dns_delegation/example-com", + Type: "infra.dns_delegation", + Provider: "hover", + ProviderID: "example.com", + Outputs: map[string]any{ + "registrar_nameservers": []any{"ns1.example.net", "ns2.example.net"}, + }, + }) + + dir := t.TempDir() + out := filepath.Join(dir, "portfolio.json") + if err := dumpPortfolioToFile(context.Background(), store, out, false); err != nil { + t.Fatalf("dumpPortfolioToFile: %v", err) + } + + data, err := os.ReadFile(out) + if err != nil { + t.Fatalf("read: %v", err) + } + var p record.Portfolio + if err := json.Unmarshal(data, &p); err != nil { + t.Fatalf("unmarshal portfolio: %v", err) + } + + // Exactly one snapshot for example.com. + if len(p.Snapshots) != 1 { + t.Fatalf("want 1 snapshot for example.com; got %d: %+v", len(p.Snapshots), p.Snapshots) + } + snap := p.Snapshots[0] + + // Snapshot must carry at least one record from the infra.dns state. + if len(snap.Records) == 0 { + t.Error("snapshot has no records; want at least one (from infra.dns state)") + } + + // Snapshot must carry authority.registrar_nameservers from the delegation state. + if snap.Authority == nil { + t.Fatal("snapshot.authority is nil; want registrar_nameservers from infra.dns_delegation state") + } + ns, ok := snap.Authority["registrar_nameservers"] + if !ok { + t.Errorf("snapshot.authority missing registrar_nameservers; got %+v", snap.Authority) + } else { + nsSlice, ok := ns.([]any) + if !ok || len(nsSlice) == 0 { + t.Errorf("registrar_nameservers = %v; want non-empty slice", ns) + } + } +} From 7ee7b8ca28d94d75724897b88f8e37adaaf12f24 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 12:06:27 -0400 Subject: [PATCH 11/11] feat(dns): sanitize allow-lists Snapshot.Authority keys Guard Snapshot.Authority with an allow-list {registrar_nameservers, live_nameservers} so Sanitize strips any unknown/sensitive keys that a future caller might put there; nil Authority is handled without panic. Co-Authored-By: Claude Opus 4.8 (1M context) --- dns/record/sanitize.go | 13 ++++++ dns/record/sanitize_test.go | 79 +++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/dns/record/sanitize.go b/dns/record/sanitize.go index eeb13c3f..b143f139 100644 --- a/dns/record/sanitize.go +++ b/dns/record/sanitize.go @@ -19,6 +19,13 @@ import ( // - Private/reserved IP ranges (RFC-1918, RFC-6598 CGNAT, loopback, // link-local, IPv6 ULA, RFC-5737/3849 documentation) are left as-is. // +// authorityAllowList is the exhaustive set of keys permitted in +// Snapshot.Authority after sanitization. Any key not in this set is removed. +var authorityAllowList = map[string]bool{ + "registrar_nameservers": true, + "live_nameservers": true, +} + // Sanitize sets p.Sanitized = true. func Sanitize(p *Portfolio) { for si := range p.Snapshots { @@ -43,6 +50,12 @@ func Sanitize(p *Portfolio) { } } } + // Strip any Authority key not in the allow-list. + for key := range p.Snapshots[si].Authority { + if !authorityAllowList[key] { + delete(p.Snapshots[si].Authority, key) + } + } } p.Sanitized = true } diff --git a/dns/record/sanitize_test.go b/dns/record/sanitize_test.go index 70220b71..3e1c4b09 100644 --- a/dns/record/sanitize_test.go +++ b/dns/record/sanitize_test.go @@ -324,6 +324,85 @@ func TestSanitizePreservesExistingExampleIPs(t *testing.T) { } } +// TestSanitizeStripsUnknownAuthorityKeys pins that Sanitize removes any key +// from Snapshot.Authority that is NOT in the allow-list +// {registrar_nameservers, live_nameservers}, while leaving allowed keys intact. +// A nil Authority must not panic, and Records sanitization must still work. +func TestSanitizeStripsUnknownAuthorityKeys(t *testing.T) { + // Snapshot with a non-nil Authority containing both allowed and disallowed keys. + p := record.Portfolio{ + Schema: record.SchemaV1, + Snapshots: []record.Snapshot{ + { + Provider: "digitalocean", + Domain: "example.com", + Authority: map[string]any{ + "registrar_nameservers": []any{"ns1.x"}, + "live_nameservers": []any{"ns2.x"}, + "secret_token": "leak", + "domain_id": "12345", + }, + Records: []record.Record{ + // A public IP that should be redacted (records pass still works). + {Type: "A", Name: "@", Value: "8.8.8.8", TTL: 300}, + }, + }, + { + // Snapshot with nil Authority — must not panic. + Provider: "cloudflare", + Domain: "other.com", + Records: []record.Record{}, + }, + }, + } + + record.Sanitize(&p) + + auth := p.Snapshots[0].Authority + + // Allowed keys must remain, values deep-equal to input. + rns, ok := auth["registrar_nameservers"] + if !ok { + t.Error("registrar_nameservers was removed; it must remain") + } else { + wantRNS := []any{"ns1.x"} + got, _ := rns.([]any) + if len(got) != 1 || got[0] != wantRNS[0] { + t.Errorf("registrar_nameservers = %v; want %v", got, wantRNS) + } + } + + lns, ok := auth["live_nameservers"] + if !ok { + t.Error("live_nameservers was removed; it must remain") + } else { + wantLNS := []any{"ns2.x"} + got, _ := lns.([]any) + if len(got) != 1 || got[0] != wantLNS[0] { + t.Errorf("live_nameservers = %v; want %v", got, wantLNS) + } + } + + // Disallowed keys must be removed. + if _, found := auth["secret_token"]; found { + t.Error("secret_token was NOT removed; it must be stripped by Sanitize") + } + if _, found := auth["domain_id"]; found { + t.Error("domain_id was NOT removed; it must be stripped by Sanitize") + } + + // Nil Authority on second snapshot must not panic (verified by reaching here). + + // Records sanitization still works: public IP was redacted. + gotIP := p.Snapshots[0].Records[0].Value + if gotIP == "8.8.8.8" { + t.Error("public IP 8.8.8.8 was NOT redacted; Records sanitization must still run") + } + if !isExampleIP(gotIP) { + t.Errorf("after Sanitize, A record value = %q; want an RFC-5737 example IP", gotIP) + } +} + // isExampleIP returns true if the IP is in an RFC-5737 documentation range. func isExampleIP(ip string) bool { return strings.HasPrefix(ip, "192.0.2.") ||