carlos / platform Public
| Previous finding | Disposition | Evidence from revision 8 at 58beffa3 | |---|---|---| | 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 assigns the wrapper to 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 orphan sweeps, followed by §15’s noncurrent-version expiration. | | R1-16, carried forward: rollout verification | Partly resolved | §5.5 still defers the retirement CLI/API operation and includes a retired box’s heartbeat for seven days; because every listed box must be fresh, deleting its edges record does not immediately remove the readiness blocker. Provide the operation and explicitly exclude retired boxes from readiness. | | R4-1. Duplicate tunnel analytics | Resolved | §§8.4 and 9 retain zero remote observations and exactly one at the hub. | | 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 retains mutex reservation before reading the body and ownership through job cleanup. | | R6-1. Plaintext tunnel cache mismatch | Resolved | §16 now explicitly requires the transport-independent tunnel mark, matching D2 and §§8.2–8.4. | | R6-2. Abandoned upload staging | Resolved | §4.2 retains the dedicated staging directory, startup cleanup and restart tests. | | R7-1. Publication of expired candidates | Resolved | §4.1 rejects expired candidates, rechecks before pointer replacement and forbids a force override. | | R7-2. Empty sanitized source | Resolved | §6 specifies the valid fallback identifier mmdb, preserving loaded-reader state across transports. | | R7-3. Attribution lost through overflow | Resolved | §9 exempts attributed sources from folding into _other. | | R7-4. Interface discrepancies | Resolved | §§4.1 and 6 now specify io.ReadSeeker, time.Time, uint64 and the Unix-seconds conversion. |
- P2 — §5.5: reconciliation needs an explicit namespace for each box identity. Platform heartbeats and all fleet prefixes are combined, but the spec never states that a label is unique only within its namespace. Two fleets can both contain
node-1; a fleet label can also equal a platform label. Joining these records by label allows a fresh, upgraded box to substitute for an old or missing box, incorrectly satisfying the rollout gate.
Evidence: the registry’s primary key is (fleet, label) at fleets.go:121. Fleet heartbeats contain the bare label at agent.go:3108; LoadBoxesComplete returns those records without adding their enclosing fleet namespace at fleet.go:234.
Fix: require reconciliation by (namespace, label), using a distinct platform namespace and each fleet’s name. Preserve that identity in expected inventory, heartbeat matching and blockers. Test identical labels across two fleets and between a fleet and the platform, with one box missing or running an old binary.
- P2 — §§4.2, 5.3: an in-flight download breaks the promised 11-minute
offbound. An edge can start downloading a generation just beforeoffdeletes the pointer. The already-open download can finish nine minutes later and install that generation without rereading the pointer. The loop then waits another ten to eleven minutes before observing the deletion. Headers can therefore remain enabled for roughly twenty minutes despite successful subsequent pointer reads; the console’s stated twelve-minute bound also fails.
Evidence: download completion proceeds directly to installation at geoipmirror.go:272. Installation checks expiry but not whether the pointer still selects the candidate at geoipmirror.go:340. The polling delay starts after the pass completes at geoipmirror.go:444.
Fix: either revise the documented bound to include in-flight work and pointer-read time, or keep pointer polling independent of downloads and prevent a superseded download from installing. State how read failures affect the guarantee. Test off during a stalled download, including subsequent console propagation.
- P3 — §3: the existing-blob rule still specifies size-only verification, contrary to the corrected implementation. Step 2 says an existing object with the expected name and size contains the same bytes. A same-sized corrupt object does not satisfy that inference. Following the written protocol could publish an unusable generation and sweep the previous usable blob.
Evidence: the committed publisher already requires stronger verification: geoippublish.go:194 calls verifyExistingBlob, which checks both byte count and SHA-256 at geoippublish.go:271.
Fix: update §3 to require the existing blob’s digest and size to match before pointer replacement. Retain a regression case where same-sized incorrect bytes leave the previous pointer and blob untouched.
Verdict: revise.