Publish node_ref, not node_id, on every unauthenticated surface - #349
Merged
Merged
Conversation
node_ref has been minted at registration since the v1 API landed and read nowhere since, so every public payload still carries the private identifier D16 splits it from. This is the lookup that lets the boundary substitute one for the other. Cached on a 30s TTL behind a synchronous NullPool engine rather than queried per frame: the callers are executor threads with no loop to await on, which is the same constraint services/publication.py solved the same way. A failed refresh keeps the previous map, because an empty one fails every node closed and blanks the live map for what may be a momentary database error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of task 1 found _refresh() clearing and updating the shared _forward/_reverse dicts in place while ref_for/id_for_ref read them without the lock, so a concurrent caller could see an empty or half-filled map on every successful TTL refresh and wrongly resolve a registered node to None. services/publication.py avoids this by rebinding a single cached reference rather than mutating it; node_refs.py now does the same for both maps. The error backoff also never engaged before the first successful load, because both early-return checks were gated on _have_data rather than on time alone, so a database outage before the first refresh re-queried on every call instead of backing off. The checks are now unconditional on time, matching publication.py, and _have_data (now unread) was removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mirrors the four edits public_aircraft_payload makes, on the identity rather than on membership: single-node entries, arcs, contributing_node_ids and the detecting_nodes fan-out map. Fails closed. A real node with no registry row has its contribution dropped rather than published under its private id, because a fallback to node_id is exactly the disclosure this boundary exists to stop. Synthetic nodes pass through: they are not hardware at anyone's address and the map identifies them by these ids. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Substitution runs last on each path, after the private-node redaction and after the real-only and per-owner filters. Those filters match node_id against state.connected_nodes, so substituting earlier would hand them refs to compare against ids and every feed would come back empty. One published_bytes() carries that ordering, so a new publication path composes it rather than reinventing it. /api/v1/solver/aircraft?real_only=true needed the same treatment and the brief did not list it: it filters the published feed by connected-node ids, so it now translates that set through public_identity before matching. Left raw it returns nothing for a real fleet, and the route tests seed state directly rather than running the flush, so nothing would have said so. Costs the public_data is aircraft_data fast path: substitution always allocates, so there are no bytes left to reuse, and broadcast_aircraft no longer takes the serialised frame it can no longer use. The flush stops dumping the unredacted frame each second as a result. TestPublicOwnerSplit now seeds a node_ref for its public node. It patched the private set but wrote no rows, so after this change its assertions would have read an empty feed for want of a ref rather than for the redaction they pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
public_identity() logged once per unresolved lookup, which is fine at the 1 Hz aircraft flush but turns into one ERROR line per unregistered node per request on the unauthenticated real_only aircraft route, at whatever rate an anonymous caller chooses. Track ids already reported since the last cache refresh so each genuinely unresolvable node still logs, just not once per call; the tracking set is cleared under the same lock that rebinds the node_ref maps, so a node that regresses after becoming resolvable is reported again. The fail-closed behaviour (None, contribution dropped) is unchanged. Also drops two em-dashes left over from Task 3, in aircraft_flush.py's published_bytes docstring and test_aircraft_flush_refs.py's header comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Substitution already replaced the values on the way out, but kept the key names, so the feeds shipped "node_id": "nde…". A field named for the private identifier while carrying the public one invites a consumer to treat the two as interchangeable, which is the confusion this boundary exists to remove, and it leaves no way to tell a substituted payload from an unsubstituted one by looking at it. The published payloads now carry node_ref and contributing_node_refs, and the old names are gone rather than duplicated: a client that still finds node_id will keep reading it, and a deprecation window here would mean shipping the private name indefinitely. detecting_nodes keeps its name; it claims no identifier type and only its values changed. Nothing upstream moves. latest_aircraft_json and every node_id-keyed filter still work in ids; only what leaves is renamed. The worked example in the public API docs named a real production node, in a public repo, which was the one place the documentation was more disclosive than the payload it documented. Known gap: frontend/ still reads ac.node_id off the feed and is fixed in a later task of this migration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The listing published node ids as dict keys and defaulted the display name to the id whenever a node had no configured name, so both halves of the entry named the private identifier. The analytics maps and the overlaps payload had the same shape, and the overlaps payload is nothing but identifiers. Re-keying happens at construction of the published dict only. The fuzz key, the real-only intersection, the missed-detection refresh and the stale pipeline eviction all keep the true node_id: the offset is HMAC-keyed on it and re-keying would move every receiver in the fleet to a new published position. A node with no registry row is dropped rather than published under its id, so the counts come from the published entries and cannot reinstate it as an anonymous tally. An overlap zone names both of its nodes and goes whole when either is unresolvable, since half a named pair still describes a baseline. The seeded refs in test_publication.py carried a hyphen and could never have passed the NodeRef schema they now stand in for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The docstring promised the endpoint served node ids and was the standing argument for leaving it unauthenticated; the identifiers it serves changed under it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Keying /api/radar/analytics on node_ref while every entry still carried the node id inside it was worse than the leak it replaced. The payload named each node twice, once by ref and once by id, which is the mapping the whole migration keeps private, on a route with no authentication. One request de-anonymised the aircraft feed, the detection arcs, /api/v1/solver/aircraft and the overlaps payload at once. The transform is structural rather than a list of the six known paths. The analytics library is a submodule that grows fields on its own schedule, and a seventh node_id added upstream would have reappeared silently under any fix that named the current ones. It resolves values, not paths: a node id under an unfamiliar key, as a dict key, or spelled into a reputation penalty's prose is caught by the same walk. It lives beside substitute_identities because that is the sibling transform for the feed path, and identity substitution is what this module is for; public_location.py answers the geometry half of the same boundary. The test fixture is part of the fix. _seed_ref derived the ref from the node id, so "ret1a2b3c4d" became "nde0ret1a2b3c4d" and a substring assertion for the id matched the ref as well. No test in the file could have asserted what this commit needs, which is why none did. Refs are now minted the way node_auth mints them and are unrelated to the id they stand for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ten in The analytics snapshot is built for publication and is now keyed on node_ref, but the leaderboard used those keys to look up state.connected_nodes and state.latest_missed_detections, both keyed on node_id. A real node therefore came back with its name fallen back to the ref, online false and all four miss fields zeroed; only the figures living inside the value survived. Both existing tests used ids matching the synthetic prefix list, which publish as themselves, so nothing caught it. The route also recomputes from a node_id-keyed source when the snapshot has not warmed yet, so it answered in ids or in refs depending on whether the refresh had run. Resolving the snapshot back to node_ids fixes both: the handler now works in one key space whichever source answered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Round 1 deleted node_id from every /api/radar/analytics value and re-keyed both published maps on node_ref, but the dashboard still read n.node_id off the values (AnalyticsPage's chart labels) and keyed a lookup on it (NodeManagementPage's summaryMap), so both silently went blank against the new payload shape. A sweep of the rest of dashboard/src for the same two patterns found two more instances: ContributionPage.tsx has the identical value-read bug as AnalyticsPage, and DetectionsPage.tsx's node filter reads node_id off the aircraft feed, which a sibling task already renamed to node_ref. All four are fixed the same way: read identity from the map key (or the renamed field), not from a value that no longer carries it. Two further findings from the sweep are out of a small dashboard-only fix's reach and are reported rather than guessed at in .superpowers/sdd/2026-09-09-node-ref-migration/task-5-report.md: OverviewPage mixes the ref and node_id key spaces when merging the public node list with the caller's own nodes, and NodeDetailPage's per-node analytics call is broken against a backend route not yet migrated (already tracked as Task 6). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The old node_id path stops resolving rather than being kept as an alias: an identifier that still answers is still published, and the rename would buy nothing. Unknown ref, unknown node and private node give one indistinguishable 404, and no error detail quotes the value it was handed. The per-node analytics route now runs the same scrub as the cached listing. It builds its summary fresh, so it never passed through public_analytics, and re-keying the entry on the ref while leaving raw ids inside the value would publish exactly the mapping this boundary keeps. Same reasoning applied to the two per-node test routes and to the custody status maps, all unauthenticated and all keyed on the node: a map keyed on node ids discloses as much as a field holding one. The custody chain entries are the exception and keep the id they were signed with, because it sits inside the ECDSA preimage and the server holds only public keys. Drops the /api/test/radar3/* aliases rather than renaming them. They named a production site in the route path itself, which no response-body change reaches. /api/auth/me/nodes is authenticated and scoped to the owner, so its node_id stays; it gains the ref so the dashboard can merge it with /api/radar/nodes on one key space instead of holding each node twice under two. Resolution goes through id_for_identity rather than id_for_ref directly: a synthetic node publishes under its own id, so that id is its handle and has to keep resolving, or every simulated node's detail page 404s on staging. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The node_ref migration exists to stop the server handing out the id-to-ref
mapping, since one surface that publishes both de-anonymises every other one.
Two surfaces still did.
/api/radar/status named the default pipeline's node_id. That pipeline is a
process-wide fallback with no registry row, so there is no ref to publish for
it; the field is renamed and served null unconditionally, rather than resolved,
so a deployment that points the default at a real node does not name it here
either. The key stays present so a consumer reads a null instead of guessing at
a missing field.
/api/custody/chain/{node_ref} resolved a ref and then returned entry bodies that
each carry the raw node_id. Refs are enumerable from /api/radar/nodes, so one
request per ref was the whole mapping. The bodies cannot be rewritten: the
node_id sits inside the ECDSA preimage and the server holds only public keys.
So they are withheld, and the route serves the chain metadata that stands
without them. A caller needing the signed bodies needs an authenticated
surface. Nothing in dashboard/ or backend/scripts/ read them.
/api/custody/verify was checked for the same defect and does not have it: its
issues are the verifier's own summaries, and the one identifier they can carry
is the chain's own node_id, already rewritten in place. A test now builds an
entry that provokes that issue rather than leaving the reasoning unpinned.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both client sites asked whether a node was synthetic by prefix-matching its identifier against synth-, e2e-, test- and realnode-. Publishing node_ref makes every one of those tests silently false for a real node, so synthetic and real would have become indistinguishable to the map on the commit that renamed the field. The flag existed already, but only on /api/radar/nodes, and useNodes reads /api/radar/analytics. Publishing it there too is what lets the client stop parsing identity. It is derived from the true node_id rather than the ref, since is_synthetic_node is prefix-based and a ref matches none of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The public feeds now publish node_ref, and the map was still reading node_id off every one of them, so the client had to move with the wire in the same deploy. This carries the three composite key formats that embed the identifier: the detection oracle, the arc buffer and the locus cache. The arc buffer drops any entry with no identifier and says nothing, so a server still publishing node_id makes every arc, every detection ring and every in-beam line disappear rather than erroring. Nothing on the client can warn about it, hence the note on the guard: deploy this with the backend. useAuth now reads node_ref off /api/auth/me/nodes, which carries both identifiers. Its result is tested against the analytics-derived node identity, so on node_id the owner-only filter would have matched nothing and owner mode would have shown an empty map. Internal names follow the field rather than outliving it: a variable called nodeId holding a ref is a trap for whoever reads it next. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
testmap and staging-map are public and were showing the real fleet, because usesRealOnlyFeed is anchored to `map.` and the defensive synthetic filter it guards therefore never ran there. Widening that flag is the obvious fix and the wrong one: it would move the demo onto /ws/aircraft/live, which carries only real nodes, emptying the synthetic fleet the demo exists to show and taking the suite built on it with it. The flag conflated two decisions, so this adds the third state rather than moving the boundary of the first: production wants real and not synthetic, the demo wants synthetic and not real, the laptop wants both. The rationale sits in domains.ts, because the next reader will otherwise make exactly that change. Everything node-attributed is filtered together or the map contradicts itself. Aircraft, because a track kept without its node is a detection nothing on the map made; detection arcs, for the same reason; and detecting_nodes, because the detail panel prints that list by name and would spell out real refs on a surface whose node list no longer holds them. Also brings two earlier tasks' leftovers onto the current wire contract: the per-node analytics assertions in nodes.spec.ts, which still expected node_id at the root and in four nested blocks, and the multinode feed key in pipeline.md. The removed required keys are replaced by absence assertions rather than dropped, so the scrub is now what is being tested. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Filtering whole entries left the leak open in the middle of one. The backend gives every track within 8 km of an active simulated trail the same ground_truth_hex and then folds those entries onto a single winner, rebuilding its contributing list as the union of every folded member's nodes. An entry mixed that way is correctly kept on a demo surface, since a synthetic node did contribute to it, but it still carried the real node's ref: into the detection oracle, which does not gate on entry type, and out through the default-on "Detected by" field, which prints the list verbatim. The winner's own node_ref is the second half of the same problem, and the worse one: a real single-node winner that swallowed a synthetic member keeps its claiming node, which the panel prints by name. So each kept entry is now cleaned as well as filtered, and n_nodes follows the list it describes rather than quoting the size of a solve whose membership this surface withholds. The logic moves to its own module so it can be tested without mounting the hook. The e2e assertion against the private ret… id went with it: that id no longer reaches the wire, so the assertion could not fail. What a leak looks like here is a real node published as nde…, and the popup identity is now asserted to be a synthetic fleet id, which is strictly narrower than the regex it replaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The tracked blah2 config carried surveyed RX positions for two production sites while the API publishes those same nodes displaced, so the repo handed back the value public_location.py exists to withhold, and the pair discloses the offset. It is now a template of invented hosts and round coordinates, saying so in its own _README: a real deployment already supplies its list through the runtime overlay or BLAH2_NODES_FILE. The identities went with it, everywhere the pattern occurred rather than only where the brief listed it: a design note's runnable command, two docstrings, the poller's CLI defaults, a dashboard example hostname, TestRadar's node and site, and a dozen test fixtures. A real id in a fixture is still a real id in a public repo, and fixtures are what get copied. The test is the point: this recurs otherwise, and a reviewer's memory is not a control. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Main retired the blah2 bridge (#340, #344) and dropped the radar3 endpoint aliases (b038519) while this branch was scrubbing placeholder values into those same files. The scrub was only ever a stopgap for files that are now gone, so the three delete/modify conflicts resolve to main's deletion: blah2_nodes.json, retnode_poller.py and test_blah2_bridge.py. Both sides removed the radar3 aliases, and the alias hunks merged clean; what conflicted in routes/test.py was only the verification route's signature, kept as this branch's node_ref form. The runbook's bridge-editing procedure goes with the bridge, but the geometry check that replaced it is kept in node_ref terms, since public routes no longer resolve a node_id. node_sites' docstring cited the shipped template as the co-located example; with the template gone it states the shape directly. The contract regenerates byte-identical to main's, so nothing under /v1/nodes moved in the merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The repo held the surveyed RX coordinate in pipeline/passive_radar.py's DEFAULT_NODE_CONFIG, which main.py makes the process-wide fallback pipeline and which the unauthenticated /api/radar/status publishes through the fuzz. Input and displaced output therefore sat in the same public place under the same id, one subtraction apart, which is exactly what the ref migration exists to prevent. The fuzz is 0.5-1.0 km, so proximity alone identifies the site and removing the id pairing would not have been enough: the numbers had to go. The same position, and truncations of it that still land inside that annulus, were repeated across sixteen fixtures. Where a file's assertions are relative to the receiver the whole cluster is translated rather than moved, so the geometry each test was written against is preserved. Anything further from the true site than the fuzz radius is left alone: it discloses less than the public feed already does. The guard now covers coordinates and display names as well as identities. It holds them as digests rather than as values, since a guard that spelled out what it forbids would be the disclosure it exists to prevent; the identity regex covers the whole retnode.com family case-insensitively instead of radar3's two hosts; and it fails rather than passing when git ls-files returns nothing, which is how it would have reported clean in a non-repository. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The scan swallowed every unopenable path, and the five libs/ gitlinks are unopenable: git ls-files lists them, read_text raises IsADirectoryError. So the guard reported clean on submodules it had never looked at, while two of them are public repos still carrying the true receiver coordinate. Gitlinks are now separated out by mode and declared in UNSCANNED_SUBMODULES, which the test asserts is still the whole set, and any other path that will not open fails rather than being skipped. Their contents stay unscanned on purpose: keying this repo's suite to whichever commit of another repo is checked out would fail here for a leak that can only be fixed there. The vendor/ skip covered the identity and name checks as well, so a real node id in a vendored bundle would have passed; it now covers only the number scan it was justified by. The two-decimal coordinate rungs stay, since a pair of them locates the site inside the published fuzz donut, but they now fail only together: either alone is one token among the tree's hundreds of two-decimal literals, which is how an SVG path in a minified library came to be reported as a coordinate. Offender lines name which class matched, and the docstring records how to hash a value someone needs to add to the ban set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/api/radar/association/status was never touched by the ref migration. It is unauthenticated, it keys pending_tracks on the raw node id, and its overlaps block names both nodes of every pair by id. Its sibling /api/radar/association/overlaps publishes that same pair list under refs with a grid_points count that fingerprints each pair, so the two payloads join on a shared key and hand out the mapping the boundary exists to withhold. Both identifiers now resolve through public_identity, and a zone with one unresolvable side goes whole, the rule the overlaps payload already applies. /api/radar/nodes passed a node's configured `name` through unfiltered beside its ref. The v1 config schema has no such key and forbids extras, but canonical_config passes unknown keys through unvalidated, so a node connecting over TCP supplies one regardless, and /api/radar/analytics already rewrites the same value. public_name falls back to the ref whenever the supplied name is a node id, taking the connected fleet as well as the registry for its vocabulary so a node with no row cannot be named either. Alongside: a null contributing_node_ids no longer raises inside the 1 Hz flush, where the exception would blank the whole frame; owner_identity gives /api/auth/me/nodes the same answer without the ERROR line about dropping a contribution, which that owner-scoped route never does; and the comment about a null node_id now names where one actually comes from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/api/admin/leaderboard is gated on any logged-in user, not on an admin, and it was converting the ref-keyed analytics snapshot back into id space to report node_id. The row carries detections, frames, tracks, uptime, SNR, trust and reputation, which is what /api/radar/analytics publishes per node_ref, so an ordinary account could join the two and recover the mapping. The route now publishes the ref the snapshot is already keyed on, which removes the conversion rather than adding one, and keeps the reverse lookup only for the node_id-keyed state it still has to read (connected_nodes, missed detections). The cold path recomputes from a node_id-keyed source and so passes the same boundary, leaving out a node with no handle rather than naming it. The auth dependency is deliberately unchanged; that is separate work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/api/auth/me/nodes returns a null node_ref for an owned node with no registry
row. The map counted those nulls as owned nodes, which suppressed the "no
nodes linked to your account yet" hint for an owner who has none on the map.
The dashboard's node detail page read node_id from a payload that carries only
node_ref, and was correct only because the URL parameter happened to fill in
behind it. The overview linked a ref-less node to /nodes/{node_id}, a 404 on
the public route; its row is still listed, because this is the only place its
owner is told about it, but it is no longer a link.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The plan's check curled /api/radar/nodes and looked for surviving ret… keys. Pre-deploy that map is keyed on node_id by construction, so it returns the whole fleet whatever the registry holds and reports every node at risk. The question is whether each connected non-synthetic node_id has a nodes row with a non-null node_ref, which only the database can answer: the API serves what already resolves and can never report what is missing. The check lands in the runbook rather than the gitignored plan, so it ships. The node_dropout snippet in the same file read n['node_id'] while iterating the top-level object rather than its nodes map, so it raised instead of listing anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/api/test/mlat-history is unauthenticated and serves solve records whole. Each carried contributing_node_ids beside the adsb_hex that the aircraft feed publishes against contributing_node_refs, so two anonymous requests joined on the hex and recovered the ref-to-id mapping for the whole fleet, which de-anonymises every other surface this branch has moved into ref space. Measured against production: 101 records, 150 KB, no credentials. The beam entries were the sharper half. Each measures a range and a bearing from the node's true receiver to an aircraft the same record locates, and a range and a bearing to a known point is a position fix: it trilaterates straight through the displacement services/public_location.py applies, which is what every published receiver coordinate rests on. Substituting the identifier does not touch that, so the margins are withheld and the verdict is not. What is left says which node refused the solve and under which rule, against the envelope /api/radar/analytics already publishes for that node. fov_limit_km and fov_state go with the margins despite reading as envelope, because both are looked up at the true bearing to the aircraft and the coverage they are looked up in is itself published as a polygon. Sweeping the rest of the router found the same disclosure twice more, on routes this branch had already moved into ref space without looking at their geometry: the per-track delays on the node verification route are the bistatic range from the true receiver to a truth position the same entry gives, and mlat-verification's max_bistatic_angle_deg names the direction from a known aircraft towards the receiver of one of the contributing nodes. Both are withheld; nothing reads either. The identity pass is structural rather than path-named because these records name nodes under half a dozen keys written by a dozen solver call sites, and one key missed is the whole mapping. node_refs grows the entry point for it, along with a suffix rule so a field called node_id can never end up holding a ref, and a vocabulary widened by the ids a payload itself names: a 30 minute store outlives the connection of a node that was never registered, and the registry alone would let that id through untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The check reads /api/radar/nodes and asks which of the connected real nodes have a registry row carrying a ref. Its comment claimed an upgraded server answers "none" unconditionally, which is inverted: once the listing is keyed on node_ref the intersection with a set of node_ids is empty, so the check reports the entire fleet as having no handle at exactly the moment its answer matters least and its noise costs most. Selecting both columns into `have` answers a listing in either key space against the same set, so the check means the same thing before and after the deploy it is run for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit stated the line the /api/test/* surfaces are held to: a per-node constant is the envelope and may be published, a per-record value that varies with the true geometry is a measurement and goes. It then applied that line by hand, three field lists on three routes, and missed three fields. The one live on production was the worst of them. The per-node verification route published tracks[].solver_lat/solver_lon at six decimal places in the receiver's true frame, while the aircraft feed publishes those same two fields for that same node displaced with the icon. Any hex present in both at one moment gave the node's exact displacement by subtraction, which is the attack services/track_gates.py already names in a comment on the feed path and the reason core/state.py keeps track_histories_public. Withheld rather than translated: the route has no need of a position at all, and a translated one is another surface to invert. Nothing in frontend/ reads it. The other two were the same rule not reaching far enough. foreign_node_ids was treated as an identity field, but membership is _point_in_beam against the true NodeGeometry evaluated at a ground-truth point the record still publishes, and the beam shape is published per node on /api/radar/analytics, so each record is a labelled in-or-out sample against a known shape. The FOV shadow verdicts escaped a helper that rewrote beam_failures[] by name, and they are the stronger disclosure of the two: stamped for the nodes that passed as well as the ones that failed, so each bounds the receiver to a region where a failure only excludes one. today_pass goes with fov_verdict for that reason; rule stays, because it names which published constant an already-announced refusal breached. Three misses in one commit is a rule kept in the wrong place, so the withholding moves into services/public_geometry.py: the classification stated once, field by field, and a structural walk every one of these payloads passes through, so a field that arrives under a new key or one level deeper is withheld without an edit here. The one judgement a name cannot carry is scope, since solver_lat is a single node's own solve on the per-node route and a multinode position on mlat-verification; that is the node_scoped flag, and a test pins the multinode side so widening the shared set cannot quietly take the error lines off the map. node_refs gains a tuple branch, since orjson serialises a tuple as an array and an id inside one published raw, and a docstring stating that the vocabulary widens only from keys spelled node_id or node_ids, which is a constraint on whoever writes these payloads rather than a property of the walk. The verification test that let the first one through asserted the surviving track equalled a five-key literal, which reads as pinning the whole shape, over a fixture that never held solver_lat. It now drives the refresh task and asserts the published entry against the record that task really writes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
furthest_detections is a per-record value that varies with the true receiver geometry (a real aircraft fix plus its distance from the true receiver), so by public_geometry.py's own rule it belongs in _RECEIVER_RELATIVE alongside the other withheld measurements, not only in the two per-route field lists that happened to drop it by hand. Neither of those two drops (routes/test.py's detection-range route, public_location.public_node_summary) actually goes through without_receiver_geometry, so removing either would leak the field on a path the module's own pass does not reach; both stay, and a new direct test on the module pins the classification independently of that route plumbing. Also recorded the /api/test/solver-stats exception next to the contaminated entry it governs (that route builds its own dict and never calls without_receiver_geometry, so its windowed count is not this field), and corrected the module docstring's claim that a field under "a new key" is withheld by the same walk: the walk is structural over containers, so a known name is withheld at any nesting depth, but it matches on the leaf key name alone rather than on shape, so a renamed field is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hidesRealNodes matched its own prefix list, which covered testmap, staging-testmap and staging-map but not test-map or test-testmap, so the retina-test droplet's map surfaces kept rendering the real fleet. Those hostnames resolve publicly under retina.fm, which is what makes them a demo surface rather than a local one, and the laptop is still ruled out by the suffix test it shares with isProdRealRadar. Deriving the set from isMapDomain removes the second enumeration that could drift from the first: a hostname added to that regex is now covered without a matching edit here. Also drops the em-dashes from the comments this branch added, per the writing style in CLAUDE.md. Left in place where the character is rendered output rather than prose: the detail panel's empty-value placeholder and the Playwright describe titles, both of which match their file's existing convention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This was referenced Sep 10, 2026
Resolves 21 conflicting files against 53 commits on main, including the two pieces already split out of this PR (#351 identity scrub, #355 receiver-geometry withholding), #358 site markers / empirical coverage, #348 location privacy, #350/#353/#354 owner contact and retnode links, and the #343/#346/#347/#357 palette work. Rules applied: main's version is the base for the add/add files (public_geometry.py, test_no_real_identities.py); the PR's ref-keying is layered on top of main's rework everywhere else; substitution still runs last on every publication path and fails closed for unregistered real nodes. Follow-on edits outside the conflicted set, needed to compile or to keep main's tests honest under ref addressing: frontend nodeSites.ts (+test) now reads is_synthetic; backend test_node_ref.py and test_public_geometry.py address routes by ref / synthetic id; analytics.py's owner-private listing merge keys on the owner identity. Verified: backend 3495 passed / 2 skipped, ruff clean; frontend tsc, lint, vite build, vitest 232 passed; dashboard tsc, eslint, vitest 53 passed, build. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Contributor
|
Merged How it was resolved
Verification: backend 3495 passed / 2 skipped, ruff clean; frontend tsc, lint, build, vitest 232 passed; dashboard tsc, eslint, build, vitest 53 passed. Things the merge surfaced that need an author/owner decision, not a conflict fix
|
The merge with main left /api/test/mlat-history running the geometry pass twice: per record inside _published_records (this branch) and again over the whole payload at the three return sites (#355). The per-record pass is the one that has to stay, because it must precede the identity walk (foreign_node_ids is both a withheld measurement and an identity field the walk renames). The outer pass stripped nothing beyond it: every record list already goes through _published_records and the remaining top-level keys are window parameters and counts. tests/test_public_geometry.py still fetches all three shapes of the route and asserts nothing from the withheld set appears at any depth, so a future field added outside `records` fails there regardless. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Claude finished @jehanazad's task in 5m 55s —— View job Code Review — PR #349 (re-review after merge 972ee58 + commit e909c38)
Fixed since the last review
Findings from the prior review that are still open (verified against current HEAD)1. Owner-scoped feed still drops the owner's own unregistered node — worse than originally framed. 2. 3. 4. Two diagnostic routes still skip the private-node opt-out ( 5/6. Both admin dashboard pages still relabel 7. 8. New from this diff:
|
This was referenced Sep 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every unauthenticated surface published
node_id, the private per-board identifier that D16 splits fromnode_ref.node_refhas been minted at registration since the v1 API landed and read by nothing since. This makes the published surfaces speak it.Closes 123zgec1jxe, 123zgec1jxg, 123zgec1jxh, 123zgec1jxk and the server half of 86cb69d16. 123zgec1jxf is superseded: the bridge retirement in #340 removed the two nodes it was about.
What moved
services/node_refs.py, cached the wayservices/publication.pycaches the private set, because the callers are executor threads with no loop to await on.state.connected_nodes, so substituting earlier hands them refs to compare against ids and every feed comes back empty. This costs thepublic_data is aircraft_datafast path.public_analyticsstrips raw ids from the values at any depth, including ids written into free-text prose. Re-keying alone would have been worse than doing nothing: each entry would have named its ref beside its id, which is the mapping that de-anonymises every other surface at once.services/public_geometry.pyholds the rule in one place: a per-node constant is an envelope and may be published; a per-record value that varies with the true geometry is a measurement and must not be./api/test/node/{ref}/verificationwas publishingsolver_lat/solver_lonin the true frame while the feed published the translated pair for the same node, which is the displacement by subtraction.tests/test_no_real_identities.pyfails if they return. It found 80 offenders across 15 files on its first run.Deployment: backend and frontend must ship together
frontend/src/components/map/arcBuffer.tsdrops any entry with a falsy identifier, silently. A backend-only deploy empties the arc layer with no error anywhere.The pre-deploy check is in
docs/runbook.md, and it is a database query rather than a query of the public API: pre-migration the listing is keyed onnode_idby construction, so any check reading it reports the whole fleet at risk unconditionally.After deploying, open the environment's map in a real browser and confirm aircraft, solves and detection arcs all still draw.
Deliberately not in this PR
/api/data/archivekeys objects undernode_id=and the parquet rows carry the column. ADR §9 requires a fresh bucket rather than a rewrite, since the existing one also holds fused tracks, users-database backups and state snapshots.node_idsits inside an ECDSA signing preimage and the server holds only public keys, so it cannot be rewritten here. The unauthenticated route withholds the bodies instead, with the reasoning in-code.Follow-ups this surfaced
Filed rather than carried here, since each needs a decision this PR should not make:
position_error_kmon/api/test/node/{ref}/verificationis a distance from a published truth point to a now-withheld solve, and the feed publishes that solve displaced under the same hex. Rounded to 3 dp, two entries at different bearings solve the displacement outright. The highest-priority of these.libs/retina-geolocatorandlibs/retina-simulationare public repos still carrying the true RX coordinate scrubbed here. The guard declares them out of scope by design, which also means the goal is met only for this repo./api/admin/alertsis gated onget_current_userrather thanrequire_adminand returns raw ids, the same shape this PR fixed on the leaderboard.ChainEntryRequestdrops the five fields the signing preimage needs, so chain verification cannot succeed today. Worth settling before the custody re-key.CI
backend-testsfailed on the first run and passes on re-run. It wastest_known_lane.py::test_interval_gate_holds_between_passes, the race that test file's own fixture comment already documents from 2026-09-06. This branch touches no known-lane code, and the full suite passes locally under the exact CI invocation (-n 2 --dist worksteal): 3293 passed. Tracked as 123zgec25bd.claude-reviewfails, identically on both attempts: it initialises, then errors 426 ms later with one turn, zero cost and an emptymodelUsage. Nothing was inferred, so it is not a verdict on the diff. It is also not credentials, quota, a timeout, prompt size or project settings, all of which are measured and ruled out in 123zgec25dk. The action returnsis_error: truewith no message, so the log does not say what it is.Size
83 files, but 2324 of the 3691 added lines are tests, leaving roughly 1367 production lines. It is not split because the deploy is atomic: the client half is unusable without the server half, and shipping the server half alone breaks the map silently.
🤖 Generated with Claude Code