carlos / platform Public

geoip spec rev 3: answer Astra round 2 (9 new accepted); X-Carlos-Hop-Geo state, rejected-hop and TLS-only tunnel trust, lifecycle rule

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude agent, operated by paul@keymail.dev 5311a1ad58e66420d53250b954d3283cf9dde4b4 parent 5000944
3 files changed, +306 −65
  • docs/superpowers/plans/2026-10-06-platform-geoip-FACTS.md +32 −0
  • docs/superpowers/specs/2026-10-06-platform-geoip-design.md +228 −65
  • docs/superpowers/specs/CODEX-FINDINGS-GEOIP-R2.md +46 −0

CI passed — run details

diff --git a/docs/superpowers/plans/2026-10-06-platform-geoip-FACTS.md b/docs/superpowers/plans/2026-10-06-platform-geoip-FACTS.md
new file mode 100644
index 000000000..a5b5de4e9
--- /dev/null
+++ b/docs/superpowers/plans/2026-10-06-platform-geoip-FACTS.md
@@ -0,0 +1,32 @@
+# Platform GeoIP: measured facts (plan 1, task 1)
+
+Measured 2026-10-06 on DOCS-T1 against `dbip-city-lite-2026-10.mmdb`,
+downloaded from `https://download.db-ip.com/free/dbip-city-lite-2026-10.mmdb.gz`
+with `maxminddb-golang` v1.13.1.
+
+| Fact | Value |
+|---|---|
+| `database_type` | `DBIP-City-Lite` |
+| `build_epoch` | 2026-10-01 01:41:21 UTC |
+| File size (uncompressed) | 126,998,165 bytes (121 MiB) |
+| `node_count` / record size / IP version | 14,297,794 / 28 / 6 |
+| `Verify()` | 5.4 s; peak anonymous RSS 47 MiB, file-backed RSS 124 MiB; 3.4 GiB allocated in total (GC churn, not resident) |
+| Lookup (decode country, subdivisions, location) | ~1.4 µs each, single goroutine (1M random IPv4 lookups in 1.44 s) |
+| Subdivision ISO codes | **absent**. `subdivisions[0]` carries names only (e.g. `"California"`, `"Leinster"`) |
+| `location.accuracy_radius` | absent |
+
+Country probe (spec §4.1 step 4):
+
+| Address | Country | Subdivision name | Location |
+|---|---|---|---|
+| 8.8.8.8 | US | California | 37.42, -122.08 |
+| 1.1.1.1 | AU | New South Wales | -33.87, 151.21 |
+| 9.9.9.9 | US | California | 37.88, -122.27 |
+| 208.67.222.222 | US | California | 37.41, -121.95 |
+| 2001:4860:4860::8888 | CA | Quebec | 45.50, -73.57 |
+
+All five resolve, so the "at least three of five" rule passes with margin.
+
+GeoLite2 City was not measured: it needs a MaxMind account. The repo's
+MaxMind test fixtures (`internal/geo/testdata/GeoIP2-City-Test.mmdb`) carry
+subdivision ISO codes, which is the shape GeoLite2 City shares.
diff --git a/docs/superpowers/specs/2026-10-06-platform-geoip-design.md b/docs/superpowers/specs/2026-10-06-platform-geoip-design.md
index 57dadd4fe..d4973e2d9 100644
--- a/docs/superpowers/specs/2026-10-06-platform-geoip-design.md
+++ b/docs/superpowers/specs/2026-10-06-platform-geoip-design.md
@@ -1,7 +1,7 @@
# Platform GeoIP for apps: headers, a mirrored database, a lookup endpoint
**Date:** 2026-10-06
-**Revision:** 2
+**Revision:** 3
**Discussion:** amadan carlos/platform #241 (quoted in Appendix A)
**Branch:** `geoip-for-apps`
@@ -17,6 +17,15 @@
recompute, which located the sending box for a backhauled door request.
- Rev 2 makes one decision Paul has not seen, **D1** in §0: GeoLite2
databases expire 30 days after they were built.
+- Rev 3 answers Astra's round 2 (`CODEX-FINDINGS-GEOIP-R2.md`). Of round
+ 1's findings, 18 were resolved and 4 partly resolved. Round 2 adds 9 new
+ findings, 2 of them P1. Every one was checked and accepted, and changes
+ are marked "(codex R2-N)".
+ - Rev 3 adds an internal trust header, `X-Carlos-Hop-Geo`. It carries the
+ ingress edge's state (hit, miss, off) across a hop or the tunnel, and
+ no app ever sees it.
+ - It withholds geo on a rejected hop and on a plaintext tunnel.
+ - It adds a bucket lifecycle rule for GeoLite2's destroy duty.
## 0. Rulings and decisions
@@ -53,7 +62,14 @@ reverse.
- **What happens:**
- At expiry every edge and the console drop the reader and delete
their local copies.
- - The console deletes the pointer and the blob.
+ - The console deletes the blob and marks the pointer
+ `"expired": true` (§3).
+ - The deployment bucket is versioned, so a deleted blob survives as a
+ noncurrent version. A lifecycle rule on `control/geoip/` expires
+ noncurrent versions after 1 day (§15, codex R2 on R1-4).
+ - A `CONSOLE_GEOIP_DB` override file belongs to the operator. The
+ platform closes it at expiry and logs that it must be replaced, but
+ never deletes it.
- The box alert (§10.4) warns from day 21.
- **When it matters:** with the feed working, a GeoLite2 database never
gets near 30 days. Only a dead feed or a manual publish ever reaches
@@ -150,6 +166,11 @@ of its bytes, so a blob never changes after it is written.
and empty for a manual publish.
- **No pointer means the feature is off.** An unknown `v` is treated as no
pointer, logged once.
+- **An expired pointer** (`"expired": true`, set only by D1) means "a
+ database was published and has run out". Edges load nothing, and the
+ `geoip` rule keeps firing (§10.4) until a fresh publish or an explicit
+ `off`. Expiry is not the same as off, so it never clears an alert
+ (codex R2-7).
**Publishing,** holding the mutex:
@@ -166,8 +187,16 @@ of its bytes, so a blob never changes after it is written.
earlier failure, and never the current one, even in an X to Y to X
sequence.
-`off` and expiry take the same mutex. They delete the pointer (with
-`ifMatch`), then sweep every blob.
+`off` and expiry take the same mutex.
+
+- **`off`** deletes the pointer, then sweeps every blob.
+- **Expiry** rewrites the pointer with `"expired": true` (with `ifMatch`),
+ then sweeps every blob.
+
+`Store.Delete` has no condition. The pointer delete is therefore
+unconditional, and its safety rests on the single-writer rule above: one
+console, one mutex. Rev 3 drops the conditional delete that rev 2 asked
+for, rather than invent a store operation (codex R2-6).
An edge that read the old pointer just before a publish can find its blob
already deleted. It keeps what it has loaded and retries on the next poll,
@@ -192,9 +221,12 @@ by both the CLI verb and the feed:
`208.67.222.222` and `2001:4860:4860::8888`. Refuse the database unless
at least three of them return a country. That catches an ASN database or
a schema the decoder can't read.
-5. **Shrink check:** refuse a database with fewer than half the pointer's
- `node_count`. This guards against a truncated or wrong-product file. A
- manual publish can override it with `force`. The feed can't.
+5. **Shrink check:** when the new database has the same `database_type`
+ as the current pointer's, refuse it if it has fewer than half that
+ pointer's `node_count`. This guards against a truncated file. The check
+ is skipped when the types differ, because node counts across products
+ aren't comparable. A manual publish can override the check with
+ `force`. The feed can't (codex R2 on R1-14).
6. Publish (§3).
Plan 1, task 1 measures `Verify`'s peak memory on a real DB-IP City Lite
@@ -243,9 +275,11 @@ Under `carlos system`, through the console API:
- the feed's configuration and its last result;
- the per-box report from §5.4.
- `carlos system geoip off` deletes the pointer, then sweeps the blobs
- (§3). Every edge drops its reader within one poll, and the console within
- a minute (§11.3). If `CONSOLE_GEOIP_FEED` is set, `off` warns that the
- feed will publish again within a day and says to unset it.
+ (§3). Every edge drops its reader within one poll (at most 11 minutes,
+ with jitter). The console follows its own box's edge within a further
+ minute, so it takes at most 12 minutes (§11.3, codex R2-9). If
+ `CONSOLE_GEOIP_FEED` is set, `off` warns that the feed will publish
+ again within a day and says to unset it.
## 5. The edge mirror
@@ -289,8 +323,12 @@ old generation, or the new `current` with the new one.
logged, and the edge starts with no database.
5. A GeoLite2 generation that has passed its expiry (D1) is deleted, not
opened.
-- **Every 10 minutes,** with jitter, read the pointer (bounded read, at most
- 64 KiB):
+- **Every 10 minutes,** with up to 1 minute of jitter, read the pointer
+ (bounded read, at most 64 KiB). Each pass has a 10-minute deadline,
+ covering the pointer read and any blob download. A pass that misses it
+ is abandoned and its `.incoming-*` deleted (codex R2 on R1-7). The
+ GeoLite2 expiry check runs on its own one-minute ticker, with no network
+ access, so a stuck download can't delay it.
- **The pointer names the sha already loaded:** nothing.
- **It names a new sha:**
1. Stream the blob through `OpenObject` into `.incoming-<sha>`. Stop and
@@ -301,15 +339,15 @@ old generation, or the new `current` with the new one.
4. Write `current` (§5.2), open the generation and swap the reader
(§5.4).
5. Delete the previous generation.
- - **No pointer:** write the empty state to `current`, swap in a nil
- reader, then delete the generations.
+ - **No pointer, or an expired one:** write the empty state to
+ `current`, swap in a nil reader, then delete the generations.
- **The read fails** (network, permissions): keep what is loaded, and log
once an hour while it keeps failing.
- **A blob whose digest doesn't match** is deleted, logged once per sha,
and not retried until the pointer changes.
-- **GeoLite2 expiry (D1):** the loop also checks the loaded generation's
- `build_epoch`. Past expiry it swaps in nil and deletes the generation,
- whatever the pointer says.
+- **GeoLite2 expiry (D1):** the expiry ticker checks the loaded
+ generation's `build_epoch`. Past expiry it swaps in nil and deletes the
+ generation, whatever the pointer says.
### 5.4 Hot swap
@@ -320,11 +358,23 @@ type Swappable struct {
mu sync.RWMutex
cur *MMDB
}
-func (s *Swappable) Lookup(ip net.IP) (Result, bool) // RLock for the lookup
-func (s *Swappable) Source() string
-func (s *Swappable) Swap(next *MMDB) // Lock, replace, Unlock, then Close the old one
+// Snapshot is one answer from one reader: the result, that reader's
+// source, and whether any reader was loaded at all (codex R2-4).
+type Snapshot struct {
+ Result Result
+ Found bool
+ Source string // empty only when Loaded is false
+ Loaded bool
+}
+func (s *Swappable) Lookup(ip net.IP) Snapshot // one RLock covers lookup and source
+func (s *Swappable) Swap(next *MMDB) // Lock, replace, Unlock, then Close the old one
```
+The headers, analytics and the endpoint's attribution all come from one
+`Snapshot`, so a swap between the lookup and the response can't label
+DB-IP data as GeoLite2. There is deliberately no separate `Source()` on
+`Swappable`.
+
- The old mapping is closed only after the write lock has been taken and
released. No lookup can be in flight on an unmapped file.
- A lookup holds the read lock for a few microseconds, so a swap waits that
@@ -343,17 +393,21 @@ Each box's status heartbeat (`status/boxes/<label>.json`) gains a block on
"geoip": {"mode": "tunnel"}
```
-`carlos system geoip status` lists every box with a fresh heartbeat (newer
-than 5 minutes) from the existing fleet listing. It reports:
+`carlos system geoip status` lists every box record under
+`status/boxes/` written in the last 7 days. It reports:
+- **heartbeat older than 5 minutes:** "stale". The verdict can't vouch for
+ a box it can't currently see.
+- **record unreadable:** "unreadable".
- **no block:** "old binary". The headers can't be trusted while any such
box is serving.
- **`mode: tunnel`:** takes the hub's headers.
- **`mode: mirror`:** shows the box's sha against the pointer's.
-It ends with a one-line verdict: whether every fresh box runs a
-GeoIP-aware binary, and whether every mirroring box has loaded the current
-sha.
+It ends with a one-line verdict. **"Ready"** means every listed box is
+fresh, readable and GeoIP-aware, and every mirroring box has loaded the
+current sha. Anything else names the boxes in the way (codex R2 on R1-16).
+A box retired more than 7 days ago drops out of the list.
## 6. `internal/geo` changes
@@ -419,14 +473,48 @@ func OpenMMDBTrusted(path string) (*MMDB, error) // opens without Verify
## 8. Trust: who may set the headers
-### 8.1 The strip
+### 8.1 Order of operations (codex R2-3)
+
+1. **Capture and strip,** at the top of `ServeHTTP`, before any branch. The
+ edge copies `X-Carlos-Hop-Geo` and every `X-Carlos-Geo-*` header into an
+ untrusted `capturedGeo` value, then deletes them all from `r.Header`.
+ Nothing reads `capturedGeo` yet.
+2. **Authenticate:** `acceptHop` runs where it does today, and so does the
+ tunnel mark, which comes from the handler wrapper.
+3. **Decide,** after `acceptHop` and before either backhaul branch, the
+ door, the gate, analytics, the upgrade branch and the cache lookup. The
+ edge sets `geoResult` once, from the table in §8.2. Captured values are
+ adopted only when step 2 trusted the transport. Otherwise they are
+ discarded.
+4. **Expose:** the edge writes the app-visible headers from `geoResult`
+ back onto `r.Header`, so `Vary` matching sees exactly the values the app
+ will get. `X-Carlos-Hop-Geo` is never written back to `r.Header`.
+
+### 8.1a The internal trust header (codex R2-5)
+
+Without more information a downstream edge can't tell "the ingress looked
+up and missed" from "the ingress had no database" or "this was a door
+call", since all three send no app headers. So the edge that forwards over
+a backhaul or the tunnel adds one internal header:
-At the top of `ServeHTTP`, before any branch, the edge removes every
-`X-Carlos-Geo-*` header from the inbound request on every request. If the
-request is a verified hop or came over the tunnel (§8.3), it first parses
-and validates them (§8.4). It then writes the request's own geo result
-back onto `r.Header` before the cache lookup, so `Vary` matching sees
-exactly the values the app will get.
+```
+X-Carlos-Hop-Geo: hit;src=DBIP-City-Lite
+X-Carlos-Hop-Geo: miss;src=DBIP-City-Lite
+X-Carlos-Hop-Geo: off
+```
+
+- `hit` and `miss` mean a reader was loaded and answered. `off` means no
+ lookup was attempted: no database, a door request, a rejected hop, or a
+ plaintext tunnel.
+- It is set only by `backhaulProxy`, `tunnelProxy` and `fleetProxy`, and
+ only from `geoResult`.
+- It is stripped at capture (step 1) on every request, and adopted only
+ under the same trust as the app headers.
+- `setGeo` deletes it from every outbound request to an app, so it never
+ reaches app code.
+- An adopted `hit` without a valid country, or a missing or malformed
+ `X-Carlos-Hop-Geo` on a trusted transport, is treated as `off`. Fewer
+ countries are counted, but none is made up.
### 8.2 Computing: only at ingress
@@ -436,12 +524,14 @@ the validated headers it received.
| Request | Geo result |
|---|---|
-| Direct request | Look up `RemoteAddr` |
-| Door request (edge-local socket) | **None.** The door replaces `RemoteAddr` with the box's own public address, so a lookup would locate the box. |
-| Sidecar through loopback | None, because the address isn't locatable |
-| Verified backhaul hop (home edge) | Adopt the sending edge's validated headers. No lookup. |
-| Request over the tunnel (remote box) | Adopt the hub's validated headers. No lookup. |
-| Unverified hop | Treated as a direct request from the sender's address, as today. The headers were stripped. |
+| Direct request | Look up `RemoteAddr`: `hit` or `miss`, or `off` with no database |
+| Door request (edge-local socket) | **`off`.** The door replaces `RemoteAddr` with the box's own public address, so a lookup would locate the box. |
+| Sidecar through loopback | `miss`, because the address isn't locatable |
+| Verified backhaul hop (home edge) | Adopt the sending edge's validated headers and state. No lookup. |
+| Request over a TLS tunnel (remote box) | Adopt the hub's validated headers and state. No lookup. |
+| Request over a plaintext tunnel | **`off`.** Nothing authenticates the hub (§8.4). |
+| **Rejected hop:** it carries `X-Carlos-Hop`, but `acceptHop` didn't verify it | **`off`** (codex R2-1). The request is served as today, as an ordinary client with its forwarded headers stripped, but geo is withheld, because its `RemoteAddr` is the sending edge. A client that sends a bogus `X-Carlos-Hop` only suppresses its own geo. |
+| Any other request | The same as a direct request |
- **The sending edge** of a backhaul computes the result like any ingress
edge, then forwards it. A door request that is backhauled therefore
@@ -456,8 +546,11 @@ the validated headers it received.
Every proxy's `Rewrite` (unix, tcp, multi, tunnel, fleet, backhaul) calls
one helper, `setGeo(pr)`, after `setForwarded(pr)`. It:
-- deletes every `X-Carlos-Geo-*` header from `pr.Out`;
-- writes them back from the context's `geoResult`.
+- deletes every `X-Carlos-Geo-*` header and `X-Carlos-Hop-Geo` from
+ `pr.Out`;
+- writes the app headers back from the context's `geoResult`;
+- writes `X-Carlos-Hop-Geo` too, but only when the proxy forwards to
+ another edge (`backhaulProxy`, `tunnelProxy`, `fleetProxy`, §8.1a).
`ReverseProxy` removes headers named in `Connection` before calling
`Rewrite`. So a client sending `Connection: X-Carlos-Geo-Country` can no
@@ -472,6 +565,14 @@ under the same values `setGeo` writes, because both come from `geoResult`.
- **Tunnel:** a new context mark, `tunnelKey{}`.
- `tunnelclient` wraps the handler it gives `http2.Server.ServeConn`, so
every request it serves carries the mark, and nothing else sets it.
+ - **The mark is granted only when the tunnel was dialled over TLS** (the
+ `https` scheme, with certificate verification against the hub's name)
+ (codex R2-2). `tunnelclient` also accepts plain `http`
+ (`tunnelclient.go`, the dial switch). The registration token
+ authenticates the box to the hub, not the hub to the box, so over
+ plain HTTP an on-path peer could pose as the hub. Plaintext tunnels
+ keep working, with geo `off`.
+ - A test proves that a plaintext tunnel never sets the mark.
- Tunnel boxes do run the ordinary edge listeners on :80 and :443
alongside the tunnel (`internal/host/agent.go`, `tunnelLoop`). A
request through those listeners doesn't carry the mark, so it is
@@ -493,7 +594,12 @@ under the same values `setGeo` writes, because both come from `geoResult`.
- a request through a real `tunnelclient` connection that adopts the
hub's headers and counts the country in the remote box's analytics;
- a door call to an app whose home is another box, which reaches the app
- with no geo headers.
+ with no geo headers;
+ - a hop signed with a key the verifier doesn't have yet (the rotation
+ window), and a door call backhauled in that window. Both are served,
+ with no geo;
+ - miss, off and door states crossing both a backhaul and the tunnel, and
+ counted correctly at the far side.
### 8.5 Mixed versions during a roll
@@ -505,13 +611,20 @@ show that, and only then publishes a database and the app docs.
## 9. Analytics countries (codex R1-11, R1-12)
-- **Counting unit:** pageviews. A request counts toward `Countries` exactly
- when it counts toward `pages`, so the two tables add up the same way.
-- `analytics.Obs` gains `Country` and `GeoSource`, which `observeAnalytics`
- reads from the context's `geoResult`. It never looks up again.
-- A pageview with no country counts as `"_unknown"` when a database was
- loaded. With no database loaded it isn't counted in `Countries` at all,
- so "no GeoIP" never looks like "visitors from nowhere".
+- **Counting unit:** pageviews. A pageview counts toward `Countries` when
+ its `geoResult` state is `hit` or `miss`.
+- `analytics.Obs` gains `Country`, `GeoSource` and `GeoState`, which
+ `observeAnalytics` reads from the context's `geoResult`. It never looks
+ up again.
+- **By state:**
+ - `hit` counts the country.
+ - `miss` counts `"_unknown"`.
+ - `off` (no database, a door call, a rejected hop, a plaintext tunnel)
+ isn't counted in `Countries` at all, so "no GeoIP" never looks like
+ "visitors from nowhere".
+- The state crosses hops and the tunnel in `X-Carlos-Hop-Geo` (§8.1a), so
+ the edge that observes the pageview counts it the way the ingress edge
+ saw it.
- `Delta`, the flush and `Day` gain `GeoSources map[string]int64`
(`json:"geo_sources"`): pageviews counted per database type. This is the
provenance the attribution needs. A day's data can come from two sources
@@ -621,28 +734,45 @@ Today the console's alert watcher only reads per-box records under
deployment-level console alert would need a new pipeline. Instead, GeoIP
becomes a **box rule**, `geoip`, alongside `disk`, `mem`, `cpu` and `lag`.
-- **Evaluation:** each mirroring edge evaluates it on its own state, using
- `metrics.RuleState`'s existing sustain and hysteresis. The rule fires
- when any of these holds for 30 minutes:
+- **Evaluation:** each mirroring edge evaluates it on its own state. The
+ rule fires once any of these has held for 30 minutes, using
+ `metrics.RuleState`'s existing sustain:
- the loaded database is older than 45 days, for any type;
- a GeoLite2 database is within 9 days of its D1 expiry (21 days old);
- - a pointer exists but this box hasn't loaded it for over an hour (a bad
- blob or a full disk);
- - the feed reason: the console's last feed result, published in the
- pointer's sibling `control/geoip/feed.json`, has failed 7 times in a
- row. Only the console box evaluates this, so it fires once.
+ - the pointer is marked expired (§3);
+ - a live pointer exists but this box hasn't loaded it for over an hour
+ (a bad blob or a full disk);
+ - **the feed reason** (codex R2-7), evaluated only by the console box's
+ edge, so it fires once:
+ - the console's feed result, published at `control/geoip/feed.json`,
+ shows 7 consecutive failed checks. This reason is independent of
+ the pointer, so a fresh deployment whose feed has never succeeded
+ still alerts.
+ - `feed.json` is written by the console after every check (configured
+ feed, last success, consecutive failures, last failure category). It
+ is deleted when the feed is unset.
- **Delivery:** through the existing paths. The host's alert sink handles
the webhook (when `CARLOS_ALERT_WEBHOOK` is set, which on the flagship it
isn't yet, #192) and the console's alert watcher sends email.
`internal/host/alertsink.go` gains the `geoip` cases for its message
text.
-- **Clearing** follows the rule's hysteresis: the condition must be false
- for 30 minutes, for example after a publish, after `off` (no pointer
- means no rule), or when the feed recovers.
+- **Clearing is immediate** (codex R2-8): the first evaluation in which no
+ reason holds clears the rule. That matches what the existing machinery
+ does (`internal/metrics/alert.go`, which clears on the first reading
+ below the line). None of these conditions flaps, so no sustained-clear
+ bookkeeping is added.
+ - The pointer-related reasons clear on a fresh publish, or on an
+ explicit `off`, which deletes the pointer.
+ - The feed reason clears on the feed's next success, or when the feed
+ is unset.
+ - Expiry alone never clears anything.
- **Tests:**
- it fires with no other box alert present;
- it clears after a publish;
- - it clears after `off`.
+ - it clears after `off`;
+ - it stays firing through expiry;
+ - the feed reason fires with no pointer at all;
+ - it restarts with the rule firing.
## 11. The lookup endpoint and CLI (plan 3)
@@ -659,8 +789,9 @@ becomes a **box rule**, `geoip`, alongside `disk`, `mem`, `cpu` and `lag`.
- **200** `{"found": true, "country": "IE", "lat": 53.34, "lon": -6.27, "source": "DBIP-City-Lite", "attribution": "IP Geolocation by DB-IP", "attribution_url": "https://db-ip.com"}`.
- The fields follow §7: absent values are omitted, and the coordinates
are rounded the same way.
- - The source and attribution come from the reader that answered, which
- may be the `CONSOLE_GEOIP_DB` override.
+ - The source and attribution come from the same `Snapshot` as the
+ result (§5.4), so they always describe the reader that answered,
+ which may be the `CONSOLE_GEOIP_DB` override.
- **200** `{"found": false}` for an address that isn't locatable or isn't
in the database.
- **400** for a missing or malformed `ip`.
@@ -682,7 +813,7 @@ Once a minute it reads `/data/carlos/geoip/current` (§5.2):
|---|---|
| a new sha | open that generation with `OpenMMDBTrusted` and swap |
| the same sha | nothing |
-| `current` missing, or naming no database | `Swap(nil)` |
+| `current` missing, or naming no database | `Swap(nil)`. The console therefore follows `off` within one edge poll plus one console poll (§4.2). |
| any other read error | keep what's loaded, log once an hour |
- **Starting with no file** is the normal case before the first publish.
@@ -695,7 +826,9 @@ Once a minute it reads `/data/carlos/geoip/current` (§5.2):
its current rules: it's opened once at start, with `Verify`, and
replacing it means a rename plus a restart. Existing deployments keep
working unchanged. D1 applies to the override too: a GeoLite2 override
- past expiry is closed and logged, and the operator replaces it.
+ past expiry is closed and logged. The file is the operator's, so the
+ platform never deletes it, and the log line says it must be replaced
+ and the old one destroyed.
## 12. Logging
@@ -801,6 +934,18 @@ MaxMind key**.
## 15. Rollout
+0. **The bucket lifecycle rule:**
+ - `infra/modules/deployment-buckets` gains an
+ `aws_s3_bucket_lifecycle_configuration` on the deployment bucket, the
+ first one it has.
+ - Its single rule is filtered to the `control/geoip/` prefix. It
+ expires noncurrent versions after 1 day and aborts incomplete
+ multipart uploads after 1 day.
+ - It is applied (targeted) before the first GeoLite2 publish. DB-IP
+ Lite has no destroy duty, so a DB-IP-only deployment can apply it
+ later.
+ - For the flagship that means bumping the module ref in
+ `carlosframework/carloku-infrastructure`, which needs Paul's go-ahead.
1. Merge plan 1 and roll every box. There is no pointer yet, so no box
sets any header. Every edge strips forged ones.
2. Run `carlos system geoip status` until its verdict says every fresh box
@@ -818,7 +963,7 @@ Plans 2 and 3 roll the console, and plan 2 also rolls the boxes for the
| Plan | Contents |
|---|---|
-| 1 | `geo.Lookup`/`OpenMMDBTrusted`/`Swappable`; `release.PutStreamer` and its wrappers; the console publish path and `carlos system geoip publish/status/off`; the edge mirror, its local layout and box status; ingress computing, adopting, `setGeo` and the tunnel mark; GeoLite2 expiry; analytics `Countries`/`GeoSources` through to the console table; operator and app docs |
+| 1 | The `control/geoip/` lifecycle rule in `infra/modules/deployment-buckets` (applied later, §15); `geo.Lookup`/`OpenMMDBTrusted`/`Swappable`/`Snapshot`; `release.PutStreamer` and its wrappers; the console publish path and `carlos system geoip publish/status/off`; the edge mirror, its local layout and box status; capture/decide/expose ordering, ingress computing, adopting, `X-Carlos-Hop-Geo`, `setGeo`, the rejected-hop rule and the TLS-only tunnel mark; GeoLite2 expiry; analytics `Countries`/`GeoSources` through to the console table; operator and app docs |
| 2 | The feed (DB-IP and MaxMind sources, bounded input, integrity, type and monotonic checks, `feed.json`) and the `geoip` box rule |
| 3 | The console reader on the mirrored generation, the endpoint, `carlos geo` |
@@ -862,6 +1007,24 @@ Round 1 (`CODEX-FINDINGS-GEOIP-R1.md`): every finding accepted.
| 21 | P3 | Accepted. Lookup probes instead of metadata (§4.1) |
| 22 | P3 | Accepted. #241 quoted in Appendix A |
+Round 2 (`CODEX-FINDINGS-GEOIP-R2.md`): every finding accepted.
+
+| # | Sev | Disposition |
+|---|---|---|
+| R1-4 (partly) | | The S3 noncurrent-version lifecycle rule (§15 step 0); override files are the operator's to destroy (§0, §11.3) |
+| R1-7 (partly) | | A 10-minute deadline on mirror passes; expiry on its own ticker (§5.3) |
+| R1-14 (partly) | | The shrink check applies on a type match, whatever the origin (§4.1) |
+| R1-16 (partly) | | The verdict covers stale and unreadable boxes from 7 days of records (§5.5) |
+| 1 | P1 | Accepted. A rejected hop gets geo `off` (§8.2) |
+| 2 | P1 | Accepted. The tunnel mark needs a TLS dial (§8.4) |
+| 3 | P2 | Accepted. Capture, authenticate, decide, expose (§8.1) |
+| 4 | P2 | Accepted. `Snapshot` under one read lock (§5.4) |
+| 5 | P2 | Accepted. `X-Carlos-Hop-Geo` carries hit/miss/off (§8.1a, §9) |
+| 6 | P2 | Accepted. The pointer delete is unconditional under the single-writer mutex (§3) |
+| 7 | P2 | Accepted. The feed reason is independent of the pointer; an expired pointer is distinct from off (§3, §10.4) |
+| 8 | P3 | Accepted. Immediate clearing, stated (§10.4) |
+| 9 | P3 | Accepted. The off bound is one edge poll plus one console poll (§4.2) |
+
## Appendix A. The request (#241, verbatim)
> **Expose the platform's GeoIP lookup to apps (headers and/or lookup endpoint)**
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-GEOIP-R2.md b/docs/superpowers/specs/CODEX-FINDINGS-GEOIP-R2.md
new file mode 100644
index 000000000..11f58f9ad
--- /dev/null
+++ b/docs/superpowers/specs/CODEX-FINDINGS-GEOIP-R2.md
@@ -0,0 +1,46 @@
+Round 2 review of `50009443`.
+
+| Round 1 finding | Disposition | Evidence from revision 2 |
+|---|---|---|
+| 1. Connection-nominated headers | Resolved | §8.3 restores headers in every `Rewrite` from the result used for cache matching. |
+| 2. Blob deletion races | Resolved | §3 serializes publication, off and cleanup; repeated publication is idempotent and the sweep preserves the selected blob. |
+| 3. Backhauled door location | Resolved | §8.2 makes verified downstream edges adopt the ingress result, including its absence. |
+| 4. GeoLite stale-data obligations | Partly resolved | D1 adds expiry, but §11.3 only closes the override, and §3 does not address retained S3 versions. Ordinary deletion does not erase those versions. [AWS documentation](https://docs.aws.amazon.com/AmazonS3/latest/userguide/DeletingObjectVersions.html) |
+| 5. Database/digest atomicity | Resolved | §§5.2–5.3 replace the separate digest with immutable generations and an atomically selected `current`. |
+| 6. Console off transition | Resolved | §11.3 explicitly specifies `Swap(nil)` and startup without a file. |
+| 7. Resource bounds | Partly resolved | §§4.1 and 10.3 bound publication and feed operations; §5.3 still gives mirror downloads no deadline, including when expiry depends on that loop advancing. |
+| 8. Streaming interfaces | Resolved | §4.1 reuses `OpenObject` and specifies `PutStreamer` and wrapper forwarding. |
+| 9. `Locate` compatibility | Resolved | §6 preserves its separate success and fallback rules, with regression cases. |
+| 10. Tunnel country analytics | Resolved | §8.2 reconstructs `geoResult` from adopted headers. |
+| 11. Country aggregation consumers | Resolved | §9 specifies pageviews, merging, read aggregation, API output and rendering. |
+| 12. Historical attribution | Resolved | §9 persists `GeoSources`; §§11.1 and 13 specify answering-reader attribution and downstream duties. |
+| 13. Operator authority | Resolved | §4.2 explicitly selects the person-only `apiSystemRoller` gate. |
+| 14. Validation comparisons | Partly resolved | §3 adds authoritative `node_count` and §10.3 checks configured types, but §4.1 applies shrinking checks unconditionally while §10.3 limits them to pointers from a feed with the same type. |
+| 15. Alert delivery pipeline | Resolved | §10.4 specifies a box rule using host delivery and the console watcher. |
+| 16. Rollout verification | Partly resolved | §5.5 distinguishes tunnel consumers, but its verdict covers only fresh heartbeats; missing, stale or unreadable serving boxes remain unaccounted for. |
+| 17. Tunnel public listeners | Resolved | §8.4 acknowledges ordinary listeners and requires their requests to remain unmarked. |
+| 18. DB-IP checksums | Resolved | §10.3 acknowledges the published checksums and explicitly declines HTML scraping. |
+| 19. Cache variant budget | Resolved | §7 documents eviction and multiplied variation. |
+| 20. Coordinate accuracy | Resolved | §7 distinguishes formatting precision from location accuracy. |
+| 21. Schema probe | Resolved | §4.1 specifies concrete lookup probes instead of metadata inspection. |
+| 22. Original request | Resolved | Appendix A supplies the requested quotation. |
+
+1. **P1 — §8.2: failed hop verification still produces the sending box’s location.** During mesh-key propagation, a legitimate backhaul can fail verification. The specified unverified-hop fallback then looks up the sender’s `RemoteAddr`, producing authoritative-looking headers for the edge rather than the visitor. This also reintroduces box coordinates for a backhauled door request in that failure case. Evidence: [mesh.go:52](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/edge/mesh.go:52) explicitly documents this rotation window; [homemap.go:930](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/edge/homemap.go:930) removes failed-hop identity; [analytics.go:86](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/edge/analytics.go:86) falls back to the peer address. **Fix:** preserve a rejected-hop classification and suppress GeoIP on it, while retaining ordinary request-serving behavior. Test mismatched rotation keys and rejected door backhaul.
+
+2. **P1 — §8.4: the tunnel marker does not necessarily establish an authenticated hub.** The proposed wrapper marks every request served by `tunnelclient`, but its runtime dialer accepts plain HTTP. With that configuration, an on-path peer can impersonate the hub and supply arbitrary, syntactically valid geo headers. The registration token authenticates the box to its peer; it does not authenticate that peer to the box. Evidence: [tunnelclient.go:211](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/tunnelclient/tunnelclient.go:211) accepts HTTP, and [tunnel_client.go:49](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/host/tunnel_client.go:49) passes environment configuration through without enforcing HTTPS. **Fix:** require authenticated HTTPS in production tunnel wiring before granting geo trust. Keep plaintext test connections behind an explicit test seam, and test production refusal.
+
+3. **P2 — §§8.1, 8.4: stripping precedes the operation that establishes hop trust.** §8.1 requires parsing trusted headers and stripping all geo headers at the top of `ServeHTTP`, before any branch. At that location, a backhaul request does not yet carry `hopVerifiedKey`. A literal implementation discards its geo result before authentication, then cannot adopt it later. Evidence: [edge.go:1343](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/edge/edge.go:1343) begins the handler, while [edge.go:1402](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/edge/edge.go:1402) calls `acceptHop` after reserved-host branches. **Fix:** specify the ordering explicitly: capture and strip inbound geo values immediately, authenticate the hop, then adopt or compute before either backhaul branch, analytics or caching. Captured values must remain untrusted until authentication succeeds.
+
+4. **P2 — §§5.4, 6, 9, 11.1: lookup and source are separate snapshots.** `Swappable.Lookup` returns a `Result` without source metadata; callers must separately call `Source`. A swap between those calls can label DB-IP coordinates as GeoLite2, or attach an empty source after `off`. Individually locking both methods does not prevent this. Evidence: [mmdb.go:22](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/geo/mmdb.go:22) associates lookup with one reader; the proposed split interface is at [design.md:323](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/docs/superpowers/specs/2026-10-06-platform-geoip-design.md:323). **Fix:** return the result, source and database-availability state together under one read lock. Use that snapshot for headers, analytics and endpoint attribution. Test a swap between lookup completion and response construction.
+
+5. **P2 — §§7–9: downstream analytics cannot distinguish a lookup miss from GeoIP being off.** Both states produce no geo headers. After a hop or tunnel, reconstructing `geoResult` therefore cannot implement §9’s rule that a miss counts as `_unknown` while an unavailable database contributes nothing. Consulting the receiving edge’s database is also wrong: tunnel boxes have none, and mirror generations can differ. Evidence: [tunnelclient.go:265](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/tunnelclient/tunnelclient.go:265) creates a new HTTP request context, and [analytics.go:119](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/edge/analytics.go:119) constructs the observation at the receiving edge. **Fix:** define an authenticated internal representation carrying ingress availability and source even on misses, stripped before app delivery; alternatively revise the counting rule explicitly. Test miss, disabled and door states across both transports.
+
+6. **P2 — §3: conditional pointer deletion has no store operation.** The revised protocol requires deleting `current.json` with `ifMatch`, but `Store.Delete` accepts no condition. Implementers must either invent another interface or silently weaken the protocol; a read followed by ordinary deletion is not equivalent. Evidence: [store.go:54](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/release/store.go:54) defines unconditional deletion, and [store.go:797](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/release/store.go:797) implements it without a precondition. **Fix:** specify conditional deletion and forwarding through supported stores, including conflict behavior and tests, or explicitly remove this guarantee under the single-writer assumption.
+
+7. **P2 — §10.4: “no pointer means no rule” suppresses feed-failure alerts.** A fresh deployment whose configured feed fails seven times never obtains a pointer, so the clearing rule disables the very alert intended to report that failure. D1 deleting an expired pointer can similarly turn a continuing outage into a cleared alert. Evidence: [alertwatch.go:143](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/console/alertwatch.go:143) only delivers published rule state; it does not independently evaluate feed failures. **Fix:** evaluate feed health independently of pointer existence. Distinguish explicit off, first-publication failure and expiry, and test each transition.
+
+8. **P3 — §10.4: existing hysteresis does not provide a 30-minute clearing delay.** The spec attributes delayed clearing to the existing rule machinery, but that machinery clears immediately when a reading crosses the clearing threshold. Evidence: [alert.go:172](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/metrics/alert.go:172). **Fix:** explicitly add sustained-clear bookkeeping for `geoip`, with restart behavior and tests, or specify immediate clearing.
+
+9. **P3 — §§4.2, 11.3: the console cannot observe off within one minute of the command.** The console watches the edge’s local `current`, and the edge polls the bucket every ten minutes with jitter. Its one-minute watcher adds to that delay. Evidence: [main.go:295](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/cmd/console/main.go:295) currently installs a separate console reader; the proposed dependency and polling intervals are at [design.md:292](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/docs/superpowers/specs/2026-10-06-platform-geoip-design.md:292) and [design.md:678](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/docs/superpowers/specs/2026-10-06-platform-geoip-design.md:678). **Fix:** describe the bound as one edge poll plus one console poll, including jitter, or specify a direct off notification.
+
+Verdict: revise.
\ No newline at end of file