carlos / platform Public

Round 6 review of 00ace13c.

| Previous finding | Disposition | Evidence from revision 6 | |---|---|---| | R3-1. Expiry/install races | Resolved | §5.3 retains expiry checks before download and installation, with genMu serializing selection, expiry and cleanup. | | R3-2. Unprotected console Locate | Resolved | §5.4 retains locked Swappable.Locate and puts the wrapper in Config.GeoIP. | | R3-3. Coordinate-only results | Resolved | §8.1a accepts coordinate-only hits; §9 counts their country as _unknown. | | R3-4. Heartbeat freshness | Resolved | §5.5 retains status.BoxStale and the jittered-heartbeat test. | | R1-4, carried forward: orphan destruction | Resolved | §3 retains startup and daily bucket sweeps, followed by §15’s noncurrent-version expiration. | | R1-16, carried forward: rollout verification | Partly resolved | §5.5 now reconciles active fleet registrations and uses LoadBoxesComplete everywhere. However, every platform edges record remains expected forever, while retired boxes retain those records: instancesplacement.go:86. A retired box therefore permanently blocks “ready.” Define an explicit retirement mechanism that removes it from expected inventory; stale heartbeats alone must not establish retirement. | | R4-1. Duplicate tunnel analytics | Resolved | §8.4 now requires zero remote observations and exactly one at the hub, matching §9. | | R4-2. Publication deadline across backhaul | Resolved | §4.2 retains staging followed by a 202 response and a polled publication job. | | R5-1. Concurrent upload staging | Resolved | §4.2 reserves the mutex before reading the body, transfers mutex and directory ownership to the job, and tests refusal while the first upload is stalled. |

1. P2 — §§8.2–8.4, 9: plaintext tunnel suppression breaks the hub’s cache contract. The hub computes geo headers, but the plaintext tunnel receiver discards them. An app returning Vary: X-Carlos-Geo-Country therefore produces a response without receiving a country, while the hub caches that response under, for example, IE. After the channel reconnects over TLS, the hub can replay it although the app would now receive IE. Fleet fallback between plaintext and TLS connections has the same problem. Hub-only analytics also retains the ingress country despite §9’s plaintext-off rule.

Evidence: edge.go:1657 matches cached variants against the hub’s request; edge.go:1776 stores its Vary values. Connection selection happens later at tunnel.go:393, and reconnect replaces the channel without cache invalidation at tunnel.go:209.

Fix: define how the hub determines and pins the destination’s geo capability before cache matching, keeping app headers, variants and analytics consistent. Cover reconnect and fleet-selection races. Test plaintext→TLS, TLS→plaintext and mixed-transport fleet fallback with an app varying on country.

1. P2 — §§4.1–4.2: console crashes leave staged uploads without cleanup. Cleanup on handler or job exit does not run after abrupt process termination. A crash during staging or publication can leave up to 1 GiB under the console’s data directory. Repeated crashes and retries accumulate abandoned files despite the single-upload slot.

Evidence: §4.1 specifies exit-path cleanup; §4.2 transfers that responsibility to the job. The reused registry stores jobs only in process-local maps at apisystemupdate.go:374; its sweep removes finished map entries, not abandoned files.

Fix: use a dedicated GeoIP staging directory and remove abandoned staging directories at console startup, before admitting uploads or starting the feed. Test restart and retry after abrupt termination during both staging and job execution.

Verdict: revise.