Skip to content

fix(dashboard): #1856 the two LAN ingress hops stop drawing as "this machine" - #1953

Closed
VijitSingh97 wants to merge 2 commits into
developfrom
fix/1856-lan-ingress-and-tor-probe
Closed

fix(dashboard): #1856 the two LAN ingress hops stop drawing as "this machine"#1953
VijitSingh97 wants to merge 2 commits into
developfrom
fix/1856-lan-ingress-and-tor-probe

Conversation

@VijitSingh97

Copy link
Copy Markdown
Collaborator

What was wrong

The Stack Topology diagram drew the two ingress hops as Local, which its own legend defines as "this machine" (web/static/topology.mjs:46-47). Both endpoints sit in the LAN zone (service/topology_graph.py:39-40), so both hops leave the box. The comment directly above those two edges already read "Ingress from your LAN" — only the code disagreed.

This is the security panel. Reading a hop that crosses onto your network as "this machine" overstates the trust boundary, which is the one thing this card exists to state correctly.

Second half, the same issue's row 13b: the #424 Tor egress probe was in neither privacy view and in no doc. tor_heal sends PROBE_URL through TOR_SOCKS_PROXY every five minutes while tor.auto_heal is on (service/tor_heal.py:63, :130-134). An operator auditing "All connections" was reading a list that omitted a connection.

What changed

  • egress.py:323-324 — the rigs and browser ingress edges route LAN, not LOCAL.
  • A "Tor egress probe" row in the egress list and a matching edge in the topology, both keyed on tor.auto_heal, both INACTIVE until it is on. Unconditional rows, like the four Tor-only dashboard clients beside them, so the existing exhaustive sweeps see them.
  • docs/privacy.md — the probe gets a row in the runtime-egress table (it was absent entirely), and the topology section explains why the two ingress hops read LAN.

No leak count, headline, legend entry or node changes. A LAN hop is not egress, so leaks is untouched — asserted, not assumed.

Budget, and where code went

egress.py was at 423/424 and this needs nine lines. check_monotonic refuses a ceiling raise, so edge/ext_node move to topology_graph.py, whose docstring already records "egress has no room" as why the route vocabulary lives there. They are re-exported; no importer changes. Final: 423/424.

That rename collided with for edge in edges: in the firewall-tagging pass, shadowing the import — the suite caught it, not review. Loop variable renamed to link.

_notify_knobs_shared_knobs (it now carries the probe knob both from-config builders read) — this is the line that paid for a second config read at two call sites.

Declined on merits: folding the five TOR if <flag> else INACTIVE conn/edge pairs into one table. It is real duplication, but it is a refactor of the whole panel during an RC cut, and it is not this bug.

Tier and what was run

Unit. tests/service/test_tor_probe_egress.py is a new module because test_egress.py is at 451/451; the ingress test there is corrected in place, line-neutral, and now checks both edges' route where it checked only the rigs one. The same ceiling kept tor_auto_heal out of the _KNOBS product, which costs little: the row and edge are unconditional, so the sweeps already cover the off branch, well-formedness and summary agreement. The new module pins the on branch.

Run by me at this head:

  • pytest (full dashboard suite): 2575 passed, rc 0
  • node --test dashboard/tests/frontend/: 579 pass / 0 fail, rc 0
  • make test-fakes: 31 passed, rc 0 — run because this changes the payload
  • make lint-py lint-js lint-docs-voice lint-operator-strings lint-topology lint-file-budget lint-trivy-parity lint-yaml lint-md lint-proto lint-toml: rc 0. The budget gate self-tested and then reported every file within its ceiling.

Not run: make lint-sh (no shell files in this diff, and the appliance lane holds the box), and no docker, KVM or bench work.

The frontend fixture, which is a wider change than it looks

tests/frontend/fixtures/state.json is regenerated, and the diff carries tari values and an indentation fix my change does not explain. It was already stale on develop and had been hand-edited — it held indentation json.dump cannot emit, and tari.local/remote values the generator does not produce.

Proven independently of this diff: tari_is_local() returns False at defaults — the default TARI_GRPC_ADDRESS is a loopback literal, which does not carry the local-monerod subnet prefix that check compares against (config/config.py:71-75) — and this branch touches no config file — so any faithful regeneration on develop yields the same values. Its drift guard (tests/web/test_xvb_views.py:503) compares dotted key paths only, never values, so value drift there is invisible to CI. Regenerating is the fixture's own documented contract.

What the operator sees on the appliance

In Advanced view, Stack Topology & Egress: the two arrows entering xmrig-proxy and caddy turn blue (LAN) instead of grey (Local). "All connections" gains a Tor egress probe row, showing Inactive unless tor.auto_heal: true. Nothing else moves — same headline, same badge, same node boxes.

What I did NOT do

#1856 stays open. Row 1's second half — the on-box local_miner internal edge — is not here. It needs a new node id, which touches TOPOLOGY_NODES, the frontend POS map, and two canonical-id lists that assert an exact set, one of them in test_egress.py at its ceiling. That is a layout decision plus a contract change in four places, not a colour fix, and it is a gap rather than a regression: the "Mining rigs" node is an external actor, so drawing its hop as LAN says nothing false about an on-box miner, which is undrawn today and stays undrawn.

Also untouched, per the issue's own split: rows 4/5/15 belong to #1849, row 2b to #1853. The issue's "Taste" suggestion — a sub-line counting LAN and unverified hops under the "All egress via Tor" headline — is not done; it is a new claim on the panel, not a correction.

docs/ is shared (_shared.paths), so no lane-override was needed; disclosing the edit here as the rule asks.

Merge

Not mine to merge. Needs a recorded non-author PASS at this head plus green CI, and the seat's --admin on develop.

VijitSingh97 and others added 2 commits September 6, 2026 09:42
…machine"

The Stack Topology diagram routed the rigs -> xmrig-proxy stratum hop and the
browser -> caddy HTTPS hop as LOCAL, while its own legend defines Local as "this
machine" and LAN as "a hop that leaves it but stays on your network", and both
nodes sit in the LAN zone (topology_graph.py:39-40). The comment above those two
edges already read "Ingress from your LAN"; only the code disagreed. Neither hop
is a leak either way, so no leak count, headline or legend entry changes.

Second half: the #424 Tor egress probe now appears in both privacy views.
tor_heal sends PROBE_URL through TOR_SOCKS_PROXY every five minutes while
tor.auto_heal is on (tor_heal.py:63, :130-134). It was in neither the "All
connections" list nor the diagram, nor anywhere in docs/privacy.md, so an
operator auditing that list was reading one that omitted a connection.

egress.py is at its file-budget ceiling, so edge/ext_node move to
topology_graph.py, whose docstring already records "egress has no room" as the
reason the route vocabulary lives there. That rename collided with the loop
variable in the firewall-tagging pass, which the suite caught, not review.

Tier: unit. test_tor_probe_egress.py is a new module because test_egress.py is
at 451/451; the ingress test there is corrected in place, line-neutral, and now
checks both edges' route where it checked only the rigs one.

The frontend fixture is regenerated. It was ALREADY stale on develop and had
been hand-edited: it carried tari values the generator does not produce at
defaults and indentation json.dump cannot emit. Its drift guard compares dotted
key paths only, never values, so value drift there is invisible to CI.

What the operator sees: the two ingress arrows turn blue (LAN) rather than grey,
and "All connections" gains a Tor egress probe row, inactive unless
tor.auto_heal is on.

NOT done, and #1856 stays open for it: the on-box local_miner internal edge
(row 1's second half) needs a new node id, which touches TOPOLOGY_NODES, the
frontend POS map and two canonical-id lists, one of them in test_egress.py at
its ceiling. Rows 4/5/15 are #1849's and row 2b is #1853's, per the issue.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR
Missed in the previous commit: it was staged before this edit, so the tree
shipped a comment naming _notify_knobs, which this branch renamed to
_shared_knobs. Comment only, line-neutral, test_egress.py stays 451/451.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Independent security review: RETURN at 166cf46d2a717c6c33876f874fc03a4d7b7fa035.

The graph always labels ingress as LAN and the new privacy documentation describes these as the only exposed listeners. The default Stratum listener binds all interfaces, and existing optional RPC/ZMQ/gRPC exposure is omitted. Use neutral wording or derive exposure from the actual bindings, so the view does not imply LAN confinement or a complete listener inventory.

Static Sol/high security review; no new network behavior was introduced and the existing Tor probe remains opt-in with no clearnet fallback. This finding concerns the accuracy of the new security explanation.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Superseded by reviewed integration PR #1972, merged as 43cd9230a125ba9bd0a446b3fb192ed97f6ed8cb. The ingress-label repair supersedes this original candidate. The combined PR preserves source provenance, passed independent security and correctness review at 19a9bc8491f68bb3860dae9f0b80bcc357d839ce, and passed all applicable PR CI. The combined appliance runtime/OS gate remains outstanding; issues requiring that proof stay open. Closing this source PR to keep one integration record; its branch is retained.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant