carlos / platform Public

1. P1 — §§7, 8.2: Connection can remove computed headers and poison a cache variant. A client can send Connection: X-Carlos-Geo-Country. The edge sets the country before caching, but ReverseProxy removes connection-nominated headers before Rewrite. The app therefore sees no country, while the cache stores its response under the computed country. Evidence: edge.go:2258, edge.go:1776, and reverseproxy.go:482. Fix: restore the trusted geo values in every proxy’s Rewrite, after hop-header removal, from the same request result used for cache matching. Test connection-nominated geo headers through all six proxies, including cache replay.

1. P1 — §§3, 4: pointer CAS does not protect blob deletion. Publisher A can change the pointer from X to Y, pause before deleting X, then publisher B can publish X again. A subsequently deletes the now-current blob X. Both pointer writes can satisfy their etag conditions. Republishing identical bytes also needs explicit handling to avoid deleting the current blob or failing its create-only upload. Evidence: store.go:37 defines conditional writes but unconditional deletion. Fix: serialize publish and off operations through cleanup under the single-console assumption; make existing identical blobs idempotent; never delete the newly selected hash. Test X→Y→X, identical publishes, and publish/off races.

1. P1 — §8.2: backhauled door requests acquire the box’s location. The door substitutes the box’s public address, and its context marker explicitly does not cross a hop. A door request routed elsewhere therefore reaches the home edge as an ordinary verified hop, where the proposed lookup produces authoritative-looking geo headers for the sending box. Evidence: door.go:28, door.go:124, and edge.go:1436. Fix: carry an authenticated, stripped-at-ingress geo-suppression signal across backhaul, without granting the remote request door authority. Test a door call whose target lives on another box.

1. P1 — §§5.2, 10.4, 13: indefinite stale serving does not satisfy GeoLite’s update obligations. GeoLite’s EULA requires ceasing use and destroying superseded data within 30 days after an update’s release. A warning after 45 days, continued serving during failures, and an optional feed do not ensure compliance. The existing reader also remains open indefinitely: main.go:1143. Fix: define a GeoLite-specific expiry and removal policy covering edge mappings, local files, bucket copies and console overrides; alert before the deadline. Remove the claim that configuring the feed alone meets the obligation. GeoLite EULA §6.

1. P2 — §§5.2, 11.3: the local database and digest are not committed atomically. Renaming B over current.mmdb and then writing current.sha256 leaves a crash window containing B with A’s digest. A subsequent pointer rollback to A makes the “same sha” branch retain B indefinitely. A full disk can produce the same inconsistent state. OpenMMDB verifies structure, not agreement with the sidecar digest: mmdb.go:29. Fix: atomically select an immutable generation containing its identity, or recompute and verify the digest at boot. Specify sync, write-error and cleanup behavior, and test failure between every commit step.

1. P2 — §11.3: the console watcher has no off transition. An inode-change watcher does not specify what happens when the edge deletes current.mmdb. An existing memory mapping survives unlinking until explicitly closed, so the endpoint and region guesses can continue using data after geoip off. Evidence: mmdb.go:42. Fix: define missing-file detection as Swap(nil) for the mirrored path, distinguish it from transient access errors, and specify startup without a file followed by first publication. Keep the explicit override’s separate lifecycle.

1. P2 — §§4.1, 5.2, 10.3: the resource limits are incomplete. Capping decompressed MMDB output does not bound an archive downloaded to disk first, tar members skipped before the MMDB, or concurrent uploads. Verification itself also allocates a map of data offsets and decodes the data section, beyond the mapped file: mmdb.go:34, verifier.go:102. Fix: bound compressed input, total expanded archive data, upload concurrency and operation duration; clean temporary files on every exit; measure verification’s peak memory on supported small boxes. Apply download bounds during copying, not after disk consumption.

1. P2 — §§4.1, 5.2: streaming support is planned on the wrong missing interface. Streaming reads already exist as release.Opener and OpenObject, including S3, directory and prefixed stores. Conversely, blob publication through Store.Put requires the entire database as []byte. Evidence: store.go:221 and store.go:637. Fix: reuse OpenObject; specify a bounded streaming upload interface and implementations for publication. Do not introduce a second optional read interface that existing wrappers fail to forward.

1. P2 — §6: implementing Locate through the proposed result can change existing answers. Today, valid coordinates succeed without a country. Without coordinates, Locate tries the registered country when the primary country has no representative point. The new Lookup declares a country-less record unsuccessful and exposes only one country, losing information needed for both behaviors. Evidence: mmdb.go:88 and mmdb.go:100. Fix: share decoding and validation beneath both methods while preserving their distinct success rules and fallback information. Add regression cases for both shapes.

1. P2 — §§8.3, 9: tunnel analytics lose the country. Request context does not travel over HTTP/2. The remote edge skips lookup and retains headers, but the spec never reconstructs the analytics context from those trusted headers. Its observations therefore become _unknown despite delivering a country to the app. Evidence: tunnelclient.go:265 and analytics.go:119. Fix: explicitly populate the request result from validated hub headers only under tunnelKey, and test country observations at the remote serving edge.

1. P2 — §9: the country aggregation contract stops before its consumers. Adding Obs.Country, Delta and Day.Countries does not expose countries through ReadRange, Summary or the analytics response. None currently carries the field. The spec also leaves unspecified whether countries count all responses, pageviews or visitors. Evidence: read.go:32, read.go:98, and meter.go:163. Fix: choose the counting unit explicitly and include cross-box/date aggregation, API assembly and table rendering in plan 1, with mixed old/new-day tests.

1. P2 — §§9, 13: current-pointer attribution is wrong for historical analytics. After switching from DB-IP to GeoLite, the proposed table credits GeoLite for earlier DB-IP-derived counts. Turning GeoIP off removes the pointer while historical data remains. Day stores no source provenance: day.go:38. Fix: persist and aggregate the sources used for each day, or adopt an attribution policy that consistently covers every retained source. Derive live lookup attribution from the reader that answered, including overrides. Also document GeoLite’s downstream contractual duties; attribution alone is insufficient. DB-IP terms, GeoLite EULA §6.1.

1. P2 — §4.2: the antivirus precedent uses a different authority list. apiSystemOperator, used by antivirus, checks CARLOS_STORE_OPERATORS, not CARLOS_SYSTEM_OPERATORS. Copying that precedent would authorize the wrong people to publish deployment-wide geolocation data. Evidence: apisystem.go:39. Fix: name the exact GeoIP gate: person-only CLI registration plus the intended system-operator check. Explicitly decide whether ops-account owners qualify; apiSystemRoller admits them as well as listed system operators.

1. P2 — §§4.1, 10.3: validation lacks an authoritative comparison contract. The pointer does not include node_count, so the shrink check needs an unspecified read of the current database. The console mirror can lag or be overridden and is not a valid substitute. Feed validation also compares type only with the current pointer, leaving first publication without a configured-product check. Evidence: mmdb.go:22 and main.go:295. Fix: record verified node count in the pointer or load the exact referenced blob; bind comparisons to the etag being replaced; validate the downloaded type against the selected feed even when no pointer exists. Define which checks the manual-type exception relaxes.

1. P2 — §10.4: deployment-level alerting and webhook delivery do not already exist in the named watcher. alertPass reads only alerts/boxes/, returns early when there are no box alerts, and sends email. Webhook delivery lives in the host process. Setting CARLOS_ALERT_WEBHOOK therefore does not make this console-generated alert reach it. Evidence: alertwatch.go:102 and alertsink.go:99. Fix: specify the deployment alert’s identity, evaluation and clearing paths, persistence and delivery sinks. Test firing with no box-alert records and clearing after publication or off.

1. P2 — §§5.4, 8.4, 15: the rollout check cannot establish the required trust boundary. Tunnel boxes deliberately omit the GeoIP block, yet rollout requires every box to report a mirror before the contract becomes valid. Old and upgraded tunnel boxes therefore look identical through the proposed field. Existing heartbeats expose the running binary version separately: snapshot.go:254. Fix: make status distinguish supported tunnel consumers from unsupported binaries, check fresh heartbeats against the expected box inventory, and require hashes only from mirror participants.

1. P3 — §8.3: tunnel configuration does not disable public listeners. The agent starts tunnelLoop(e) alongside the normal edge serving loop, whose default listeners are :80 and :443. Evidence: agent.go:576 and edge.go:2952. Fix: remove the assertion that remote boxes lack public listeners. Treat listener isolation from tunnelKey as a present requirement and test the ordinary startup wiring.

1. P3 — §10.3: DB-IP does publish downloadable-file checksums. Its City Lite download page lists MD5 and SHA-1 checksums for the MMDB download. These are not a published SHA-256 API or independent authentication, but “publishes no checksum” is incorrect. OpenMMDB supplies structural verification, not publisher-checksum verification: mmdb.go:29. Fix: distinguish the available checksums from the stronger mechanism desired, then explicitly choose whether to consume them. DB-IP download page.

1. P3 — §7: country variation does not generally fit eight variants. Eight is the total per-URL allowance across all Vary combinations; a ninth country already causes eviction, and other varying headers multiply the combinations. Evidence: cache.go:19 and cache.go:345. Fix: document eviction and possible cache churn instead of claiming country-level variation fits the budget.

1. P3 — §7: two decimal places describe formatting precision, not database accuracy. GeoLite locations can represent areas with radii reaching hundreds of kilometres; the reader currently extracts coordinates without their accuracy radius. Evidence: mmdb.go:49. Fix: retain the approved rounding but remove the suggestion of roughly kilometre-level accuracy. Explain that coordinates represent an approximate area. MaxMind’s GeoLite accuracy documentation.

1. P3 — §4.1.3: MMDB metadata cannot supply the proposed record-schema probe. Metadata contains database type, build epoch, node count and format information—not a list of fields in the first record. Evidence: reader.go:42. Fix: remove that candidate mechanism and choose bounded record inspection or documented lookup probes.

1. P3 — §1 / review inputs: the originating request is not quoted. The brief says discussion #241 is quoted in §1, but that section contains only the design’s summary and goals: design.md:33. Retrieval through amadan was blocked by network restrictions, so agreement with the original request could not be checked. Fix: include the requested quotation or a pinned local copy.

Verdict: revise.