amadan
steering: fix review findings — true issuance reasoning, one normalization, a name bound
F1. The autocert justification was FALSE and contradicted certsync.go's own correct reading of the same function: autocert orders under context.Background() with a 5-minute budget (autocert.go:274), NOT the handshake context, so a health checker hanging up does not cancel the order it triggered. The three comments now carry the reasons that actually hold: a per-host exception would have to breach the exact-registry Allowed rule (PR #180) in TWO places — issuerHostPolicy AND the mirror's m.allowed bound on the live challenge read-through; a steered name resolves to every armed edge, so issuance would hang on the cross-box challenge mirror for a name that is deployment infrastructure; and every attempt is paid for in live failed health checks and shared-account failed-validation budget. The outcome (wildcard parent) is unchanged; only the argument for it is now true. F2. Two-reads normalization gap, and it was a real outage shape: newWildcardFromEnv's own Getenv does not trim a trailing dot while steeringProbeHostFromEnv does, so PLATFORM_APPS_DOMAIN="oncarlos.com." answered probes on edge.oncarlos.com while arming parent "oncarlos.com." that certFor's CutSuffix can never match — no cert, every check failing, on the one name the box promises to serve. The parent now derives from e.steeringHost, which makes the read-once comment true, and the test fixture feeds the DOTTED form end-to-end (verified to fail against the old line). F3. The IAM comment implied apex protection the grant did not give: A UPSERT/DELETE was zone-wide, apex A included, with only the converger's code in the way. New optional steering_record_names adds the missing ForAllValues:StringLike bound on NormalizedRecordNames (mirroring host_route53's _acme-challenge bound); left empty the comment now says plainly what the grant covers. Steered custom domains need nothing here — they reach steering through a CNAME in the customer's own zone. F4 + minors, in the rollout notes: non-issuer edges cannot answer a check until the issuer's wildcard is published AND mirrored (30s poll + wake, hourly stand-down pass as fallback — both read out of the code, not assumed), so confirm per-EIP before treating steering live; TWO units need CARLOS_STEERING_ZONE_ID (the agent, which is converger and edge in one process, and the console), not "three consumers"; and set steering_record_names whenever steering_zone_ids is set. isApexDomain's comment no longer claims a conservatism publicsuffix does not provide. Gate: build, vet, GOOS=darwin, gofmt, full test suite, internal/edge -race — all clean. tofu validate + fmt clean with the variable set and unset (the variable VALIDATION blocks are not exercised by tofu validate here, same as the pre-existing zone-id one; the pattern regex was checked directly).
8 files changed,
+146
−45
docs/superpowers/plans/2026-08-23-multi-region-steering.md+36 −10infra/modules/README.md+1 −0infra/modules/region-host/main.tf+29 −11infra/modules/region-host/variables.tf+10 −0internal/console/view.go+8 −4internal/edge/steeringprobe.go+13 −6internal/edge/steeringprobe_test.go+16 −2internal/edge/wildcard.go+33 −12
diff --git a/docs/superpowers/plans/2026-08-23-multi-region-steering.md b/docs/superpowers/plans/2026-08-23-multi-region-steering.md| index e252a5931..999bbf135 100644 |
| --- a/docs/superpowers/plans/2026-08-23-multi-region-steering.md |
| +++ b/docs/superpowers/plans/2026-08-23-multi-region-steering.md |
| @@ -362,17 +362,43 @@ Pre-filled by Task 7 (fold into the list above at Task 8): |
| ANSWER, not the record. Retiring a region is therefore a two-step |
| operator action: stop the box, then delete its edges file from the |
| control bucket. Until that second step, the zone still names it. |
| -- **The steering env has THREE consumers on a box, not one.** |
| - `CARLOS_STEERING_ZONE_ID` gates (a) the converger duty in the agent, |
| - (b) the edge's steering-probe answer AND its `*.<apps-domain>` wildcard |
| - parent — the cert the health check handshake needs, which no per-host |
| - order can ever produce — and (c) the console's custom-domain guidance, |
| - which only offers the CNAME to `edge.<apps-domain>` where the value is |
| - set. All three read the same variable and validate it identically; a box |
| - whose edge/console did not get it stays on the old behaviour silently |
| - (dormant, not broken). The edge additionally needs a DNS provider |
| - configured (`CARLOS_DNS_PROVIDER`) — without one the apps-domain |
| +- **`CARLOS_STEERING_ZONE_ID` has to reach TWO units on a box.** The |
| + **agent unit** — `carlos agent` runs the converger duty and the edge in |
| + ONE process, so one read arms both the DNS pass and the edge's |
| + steering-probe answer plus its `*.<apps-domain>` wildcard parent (the |
| + cert the health-check handshake needs) — and the **console unit**, whose |
| + custom-domain guidance only offers the CNAME to `edge.<apps-domain>` |
| + where the value is set. Both read the same variable and validate it |
| + identically; a unit that did not get it stays on the old behaviour |
| + silently (dormant, not broken). The edge additionally needs a DNS |
| + provider configured (`CARLOS_DNS_PROVIDER`) — without one the apps-domain |
| wildcard is never issued and every health check fails at the handshake. |
| +- **Arming makes every NON-ISSUER edge unhealthy until the new wildcard |
| + reaches it, and the converger does not wait.** Only the elected issuer |
| + orders `*.<apps-domain>`; the other edges SERVE it, which means they |
| + cannot answer a health check until the PEM has been published to |
| + `control/certs/` and mirrored down. Two paths, both verified in the code |
| + rather than assumed: the ordinary one is the cert mirror's 30s poll, |
| + which detects changed wildcard material and wakes the wildcard manager |
| + (`edge.go`: `mirror.wake = w.refresh`), so the gap is the issuer's own |
| + DNS-01 order (TXT publish, propagation wait, validation — a couple of |
| + minutes) plus one poll; the fallback, if that wake is ever missed, is the |
| + non-issuer's own stand-down pass, which is HOURLY (`wildcardManager.pass` |
| + shortens its 12h sleep to 1h when it stood down). Meanwhile the converger |
| + has already created the health checks and bound the records, so steered |
| + traffic concentrates on the issuer for the duration. Sequence |
| + accordingly: arm, then confirm the wildcard in EVERY edge's journal |
| + (`edge: wildcard *.<apps-domain> obtained` on the issuer, `loaded from |
| + cache` on the others) — or |
| + `curl --resolve edge.<apps-domain>:443:<EIP> https://edge.<apps-domain>/` |
| + once per EIP, which is the same check Route53 will run — BEFORE treating |
| + steering as live. |
| +- **Set `steering_record_names` when you set `steering_zone_ids`.** Left |
| + empty, the IAM grant covers A-record UPSERT/DELETE across the whole |
| + steering zone, the apex A included; `["*.<apps-domain>"]` bounds it to |
| + the names the converger actually writes. Steered CUSTOM domains need |
| + nothing here — they reach steering through a CNAME in the customer's own |
| + zone. |
| - **`PLATFORM_EDGE_IPS` must list every armed edge's EIP once steering is |
| armed**, not just the local box's: the DNS check that stamps a custom |
| domain `pointing` compares the resolved address against that list, and a |
diff --git a/infra/modules/README.md b/infra/modules/README.md| index b8de86a3a..f0c68eea7 100644 |
| --- a/infra/modules/README.md |
| +++ b/infra/modules/README.md |
| @@ -67,6 +67,7 @@ bucket, so a wrong-region wake fails loudly at the IAM layer. |
| | `ssm_parameter_arn` | SSM parameter ARN pattern the host may read, e.g. `arn:aws:ssm:<region>:<account>:parameter/carlos/*`. | |
| | `route53_zone_ids` | Hosted zones the box may publish ACME DNS-01 challenge TXTs into (wildcard certs). Bare `Z...` ids; empty = no Route53 access. | |
| | `steering_zone_ids` | Hosted zones the DNS-steering converger may write latency A records into, plus the health checks that withdraw a dead edge. Bare `Z...` ids; empty = no steering grant (the default — steering also needs `CARLOS_STEERING_ZONE_ID` in the box env). | |
| +| `steering_record_names` | Name bound on that grant, e.g. `["*.oncarlos.com"]`. Empty (default) grants A-record UPSERT/DELETE on the WHOLE zone, apex included — set it. | |
| | `ami_name_pattern` | AMI name filter (default `debian-13-arm64-*`). | |
| | `ami_owner` | AMI owner account (default `136693071363`, Debian). | |
| | `enable_auto_recovery` | Arm the EC2 auto-recovery alarm (HA ladder rung 0) — a system-status-check failure recovers the instance onto healthy hardware, same instance id, EIP and volumes. Default `true`; set `false` only where an external supervisor owns recovery. | |
diff --git a/infra/modules/region-host/main.tf b/infra/modules/region-host/main.tf| index 3fa21d42a..c145aac8f 100644 |
| --- a/infra/modules/region-host/main.tf |
| +++ b/infra/modules/region-host/main.tf |
| @@ -157,20 +157,38 @@ resource "aws_iam_role_policy" "host_steering" { |
| Version = "2012-10-17" |
| Statement = [ |
| { |
| - # The write. Conditioned as tightly as the converger's own behaviour |
| - # allows: it only ever UPSERTs or DELETEs A records (desiredRecord is a |
| - # latency A set; converge builds no other type or action), so an apex |
| - # NS/SOA/MX in a steering zone is out of this role's reach by IAM and |
| - # not merely by code. |
| + # The write. Two bounds, and it is worth being exact about what each |
| + # one does and does not buy: |
| + # |
| + # - TYPE and ACTION are bounded always: the converger only ever |
| + # UPSERTs or DELETEs A records (desiredRecord is a latency A set; |
| + # converge builds no other type or action), so the zone's NS, SOA, |
| + # MX, TXT and CNAME records are out of this role's reach by IAM. |
| + # - NAME is bounded only if steering_record_names is set. WITHOUT it |
| + # this grant covers every A record in the zone — the APEX A |
| + # included, which on both our zones is a live site — and the only |
| + # thing standing in the way is the converger's own code |
| + # (isManaged, conflictedNames). That is a code bound, not an IAM |
| + # one. Set the variable to "*.<apps-domain>": it covers the |
| + # steering hostname and every steered platform host, and steered |
| + # CUSTOM domains need no grant here at all — they reach steering |
| + # through a CNAME in the customer's own zone. |
| Effect = "Allow" |
| Action = ["route53:ChangeResourceRecordSets"] |
| Resource = [for z in var.steering_zone_ids : "arn:aws:route53:::hostedzone/${z}"] |
| - Condition = { |
| - "ForAllValues:StringEquals" = { |
| - "route53:ChangeResourceRecordSetsRecordTypes" = ["A"] |
| - "route53:ChangeResourceRecordSetsActions" = ["UPSERT", "DELETE"] |
| - } |
| - } |
| + Condition = merge( |
| + { |
| + "ForAllValues:StringEquals" = { |
| + "route53:ChangeResourceRecordSetsRecordTypes" = ["A"] |
| + "route53:ChangeResourceRecordSetsActions" = ["UPSERT", "DELETE"] |
| + } |
| + }, |
| + length(var.steering_record_names) > 0 ? { |
| + "ForAllValues:StringLike" = { |
| + "route53:ChangeResourceRecordSetsNormalizedRecordNames" = var.steering_record_names |
| + } |
| + } : {}, |
| + ) |
| }, |
| { |
| # The read, split out because ListResourceRecordSets supports none of |
diff --git a/infra/modules/region-host/variables.tf b/infra/modules/region-host/variables.tf| index 3bfd76587..e46144582 100644 |
| --- a/infra/modules/region-host/variables.tf |
| +++ b/infra/modules/region-host/variables.tf |
| @@ -87,6 +87,16 @@ variable "steering_zone_ids" { |
| } |
| } |
| +variable "steering_record_names" { |
| + description = "Optional NAME bound on the steering grant: DNS name patterns (IAM StringLike, matched against route53:ChangeResourceRecordSetsNormalizedRecordNames — lowercase, no trailing dot) this role may write. Normally exactly one entry, \"*.<apps-domain>\", which covers the steering hostname edge.<apps-domain> and every steered platform host under it; steered CUSTOM domains resolve through a CNAME in the CUSTOMER's own zone, which this role has no grant on at all. Empty (the default) means NO name bound: the role may then UPSERT or DELETE any A record in the named zones, the APEX A included — often the marketing site — with only the converger's own managed-set rules standing in the way. Set it." |
| + type = list(string) |
| + default = [] |
| + validation { |
| + condition = alltrue([for n in var.steering_record_names : can(regex("^[*a-z0-9][a-z0-9.*_-]*\\.[a-z]{2,}$", n))]) |
| + error_message = "steering_record_names entries must be lowercase DNS name patterns with a dot (e.g. \"*.oncarlos.com\") — a bare \"*\" is not a bound and is refused." |
| + } |
| +} |
| + |
| variable "enable_auto_recovery" { |
| description = "Arm an EC2 auto-recovery alarm on the host (HA ladder rung 0)." |
| type = bool |
diff --git a/internal/console/view.go b/internal/console/view.go| index 2ed4c95d6..5138ecf92 100644 |
| --- a/internal/console/view.go |
| +++ b/internal/console/view.go |
| @@ -145,10 +145,14 @@ func domainGuidance(host string, t DNSTargets) string { |
| // x/net), and the PSL is the only correct answer here: label counting calls |
| // "example.co.uk" a subdomain and would hand an apex a CNAME it cannot use. |
| // |
| -// An unknown suffix (a name whose TLD is not in the list — a test domain, |
| -// an internal name) is treated as an APEX: the conservative answer keeps |
| -// guidance to the A-record form, which always works, instead of proposing a |
| -// CNAME that might be refused by the member's registrar. |
| +// The error case — EffectiveTLDPlusOne returns one when the name IS a public |
| +// suffix ("com", "co.uk") or is empty — is treated as an APEX, which keeps |
| +// guidance on the A-record form for input no member should be attaching |
| +// anyway. Note what this does NOT do: an unlisted TLD is not an error case. |
| +// publicsuffix falls back to the implicit "*" rule, so "example.internal" |
| +// comes back as its own eTLD+1 and reads as an apex, while |
| +// "app.example.internal" reads as a subdomain and is offered the CNAME — |
| +// which is the right answer for both, and not extra conservatism. |
| func isApexDomain(host string) bool { |
| h := strings.ToLower(strings.TrimSuffix(strings.TrimSpace(host), ".")) |
| etld1, err := publicsuffix.EffectiveTLDPlusOne(h) |
diff --git a/internal/edge/steeringprobe.go b/internal/edge/steeringprobe.go| index b740824d0..ac5c8fa05 100644 |
| --- a/internal/edge/steeringprobe.go |
| +++ b/internal/edge/steeringprobe.go |
| @@ -15,12 +15,19 @@ package edge |
| // (*.<acct>.<apps-domain>). No cert covered the name, so the probe died |
| // at the handshake and every record looked dead. The fix is in |
| // wildcard.go: with steering armed, the apps domain itself joins the |
| -// wildcard parent set, so *.<apps-domain> covers exactly this name — |
| -// issued over DNS-01, which is the only issuance path that can work |
| -// here. A per-host order cannot: the sole TLS client that ever sends |
| -// this SNI is the health checker, and autocert orders LAZILY out of the |
| -// handshake, so every order would be cancelled by the checker's |
| -// few-second timeout before it could finish, forever. |
| +// wildcard parent set, so *.<apps-domain> covers exactly this name, |
| +// issued over DNS-01. The narrow alternative — a per-host exception for |
| +// the single name — was rejected on three counts (the full argument is |
| +// at the arming site in wildcard.go): it would have to breach the |
| +// exact-registry Allowed rule (PR #180) in TWO places, issuerHostPolicy |
| +// AND the mirror's m.allowed bound on the live challenge read-through; |
| +// with the name resolving to every armed edge, the CA reaches an |
| +// arbitrary one, so issuance would hang on the cross-box challenge |
| +// mirror; and every attempt is paid for in failed health checks and in |
| +// the shared ACME account's failed-validation budget. NOT because an |
| +// order could not complete — autocert orders under context.Background() |
| +// with a 5-minute budget (autocert.go:274), so a checker that hangs up |
| +// on the handshake does not cancel the order it triggered. |
| // |
| // - HTTP: the hostname is not a registry route, so it fell through to |
| // unknownHost — a 404, or on a -fallback edge (every flagship box) a |
diff --git a/internal/edge/steeringprobe_test.go b/internal/edge/steeringprobe_test.go| index 2dad7c5f5..83b7c782a 100644 |
| --- a/internal/edge/steeringprobe_test.go |
| +++ b/internal/edge/steeringprobe_test.go |
| @@ -40,6 +40,9 @@ func TestSteeringProbeHostFromEnv(t *testing.T) { |
| {"apps domain alone is NOT the gate", "", "oncarlos.com", ""}, |
| {"malformed zone id stays dormant", "/hostedzone/Z123", "oncarlos.com", ""}, |
| {"armed", "Z0ABCDEF1234", "oncarlos.com", "edge.oncarlos.com"}, |
| + // The dotted form is the two-reads trap: it must normalize ONCE, so |
| + // that the probe host and the wildcard parent cannot disagree. Covered |
| + // end-to-end by TestSteeringArmsTheAppsDomainWildcardParent. |
| {"armed, case and dot normalized", "Z0ABCDEF1234", " OnCarlos.com. ", "edge.oncarlos.com"}, |
| {"armed with no apps domain: nothing to build from", "Z0ABCDEF1234", "", ""}, |
| } |
| @@ -142,7 +145,12 @@ func TestSteeringProbeInertWithoutSteering(t *testing.T) { |
| func TestSteeringArmsTheAppsDomainWildcardParent(t *testing.T) { |
| cache := autocert.DirCache(t.TempDir()) |
| t.Setenv("CARLOS_WILDCARD_DOMAINS", "") |
| - t.Setenv("PLATFORM_APPS_DOMAIN", "oncarlos.com") |
| + // DOTTED on purpose: PLATFORM_APPS_DOMAIN with a trailing dot is legal |
| + // input, newWildcardFromEnv's own Getenv does not trim it, and the parent |
| + // must still come out as the name certFor can match. Two reads of the |
| + // environment put the probe answer on edge.oncarlos.com and the parent on |
| + // "oncarlos.com." — no cert, every health check failing. |
| + t.Setenv("PLATFORM_APPS_DOMAIN", "oncarlos.com.") |
| t.Setenv("CARLOS_DNS_PROVIDER", "") |
| t.Setenv("CARLOS_DNSIMPLE_TOKEN", "tok") |
| @@ -159,7 +167,13 @@ func TestSteeringArmsTheAppsDomainWildcardParent(t *testing.T) { |
| t.Fatalf("unsteered parents = %v, want none", got) |
| } |
| - e.steeringHost = "edge.oncarlos.com" |
| + // Armed through the SAME derivation production uses — not a hand-set |
| + // field — so the normalization under test is the real one. |
| + t.Setenv("CARLOS_STEERING_ZONE_ID", "Z0ABCDEF1234") |
| + e.steeringHost = steeringProbeHostFromEnv() |
| + if e.steeringHost != "edge.oncarlos.com" { |
| + t.Fatalf("steeringHost = %q, want edge.oncarlos.com", e.steeringHost) |
| + } |
| w = newWildcardFromEnv(e, cache) |
| if !reflect.DeepEqual(w.staticDomains, []string{"oncarlos.com"}) { |
| t.Fatalf("steered staticDomains = %v, want [oncarlos.com]", w.staticDomains) |
diff --git a/internal/edge/wildcard.go b/internal/edge/wildcard.go| index aa3fdda91..bfc128ca3 100644 |
| --- a/internal/edge/wildcard.go |
| +++ b/internal/edge/wildcard.go |
| @@ -693,14 +693,30 @@ func newWildcardFromEnv(e *Edge, cache autocert.Cache) *wildcardManager { |
| // carries go.tito.io for the same reason: a domain whose one-label-deep |
| // hosts need certs). |
| // |
| - // DNS-01 is not merely convenient here, it is the only path that works: |
| - // the sole client that ever sends that SNI is the health checker, and a |
| - // per-host order is triggered lazily BY a handshake and cancelled with |
| - // it, so no checker-driven order could ever outlive the checker's own |
| - // few-second timeout. This manager's loop orders on its own schedule, |
| - // answers the challenge in our own zone, and needs no A record to exist |
| - // first — which matters, because the record only appears once the |
| - // converger runs. |
| + // DNS-01 through this manager, rather than a narrow per-host exception |
| + // for the one name, for three reasons — and NOT because a per-host order |
| + // could not complete (an earlier draft of this comment claimed that and |
| + // was wrong: autocert orders under context.Background() with a 5-minute |
| + // budget, autocert.go:274, so a health checker hanging up on the |
| + // handshake does not cancel the order it triggered): |
| + // |
| + // 1. the exception would have to breach the Allowed rule (PR #180) in |
| + // TWO places, not one: issuerHostPolicy's registry check, and the |
| + // mirror's m.allowed bound on the live HTTP-01 read-through |
| + // (certsync.go's challengeResponse). Allowed is deliberately |
| + // exact-registry-only, and it is what keeps an attacker-chosen name |
| + // from turning into an order or a bucket read; |
| + // 2. once a deployment steers, the name resolves to EVERY armed edge, so |
| + // the CA reaches an arbitrary one and per-host issuance would hang on |
| + // the cross-box challenge mirror for a name that is deployment |
| + // infrastructure rather than a member's route; |
| + // 3. every order attempt is paid for in live failed health checks (the |
| + // probe fails until the cert exists, and the records it gates are |
| + // withdrawn meanwhile) and in the shared ACME account's |
| + // failed-validation budget. The wildcard path spends neither: it |
| + // answers in our own zone, on its own schedule, and needs no A record |
| + // to exist first — which matters, because the record only appears |
| + // once the converger runs. |
| // |
| // Gated on the steering zone, not on the apps domain: both production |
| // deployments set an apps domain today and neither steers, and minting an |
| @@ -708,11 +724,16 @@ func newWildcardFromEnv(e *Edge, cache autocert.Cache) *wildcardManager { |
| // entry, and a change to which cert their existing one-label hosts are |
| // served from) is not something a dormant feature gets to do. |
| // |
| - // Read off the Edge rather than the environment a second time: New() |
| - // derived it once (edge.go), so the name this box ANSWERS on and the |
| - // parent it gets a certificate for cannot come from two different reads. |
| + // The parent is derived from e.steeringHost, NOT from this function's own |
| + // appsDomain read: New() normalized the environment once (edge.go, |
| + // steeringProbeHostFromEnv — which trims a trailing dot, and the Getenv |
| + // above does not). Two reads meant PLATFORM_APPS_DOMAIN "oncarlos.com." |
| + // answered probes on edge.oncarlos.com while arming a parent |
| + // "oncarlos.com." that certFor's CutSuffix could never match: no cert, |
| + // every health check failing, on the one name the box promises to serve. |
| + // One value, one normalization, both halves. |
| if e.steeringHost != "" { |
| - static = append(static, appsDomain) |
| + static = append(static, strings.TrimPrefix(e.steeringHost, "edge.")) |
| } |
| dns, reason := dnsProviderFromEnv() |
| if dns == nil { |