carlos / platform Public

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 | | 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 explicitly documents this rotation window; homemap.go:930 removes failed-hop identity; 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.

1. 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 accepts HTTP, and 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.

1. 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 begins the handler, while 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.

1. 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 associates lookup with one reader; the proposed split interface is at 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.

1. 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 creates a new HTTP request context, and 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.

1. 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 defines unconditional deletion, and 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.

1. 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 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.

1. 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. Fix: explicitly add sustained-clear bookkeeping for geoip, with restart behavior and tests, or specify immediate clearing.

1. 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 currently installs a separate console reader; the proposed dependency and polling intervals are at design.md:292 and 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.