Scan the LAN, bounded to configured subnets (#257) #81

Closed
claude wants to merge 1 commits from overnight/netscan into overnight/smarthome
Contributor

Scan the LAN, bounded to configured subnets (#257)

internal/netscan/ discovers hosts on the network Maven is configured to look at:
a TCP-connect scan (net.DialTimeout, no raw sockets, no privileges) plus a read
of the kernel's ARP cache. Wired as a read-only query source, "network", so
"какие устройства в сети?" is answered by a scan instead of by whatever old note
happens to be nearest.

Scanning is a read, but an unbounded scanner on a home LAN is noisy and easy to
point somewhere it should not go, so the package is built around four bounds:

  • Scan takes NO target argument. The range comes from the config block and
    from nowhere else, so there is no exported way to scan an arbitrary prefix
    and nothing an utterance, the router, or a scanned host says can retarget
    it. That is asserted directly: the test watches every address handed to the
    dialer and fails if one falls outside the configured prefix. The ARP cache —
    the one input the network itself populates — is filtered to the configured
    range for the same reason.
  • Every configured CIDR must be private (RFC1918 / CGNAT / link-local) and no
    larger than 1024 addresses. 8.8.8.0/24, 0.0.0.0/0 and 10.0.0.0/8 are refused
    at config load, not after the packets have left.
  • Rate-limited to a configured connections-per-second across the whole scan,
    so it looks like background traffic rather than a portscan.
  • Bounded in total by MaxHosts, a per-connection timeout, a 20s turn budget
    and the context; a canceled scan stops dialing immediately.

Off unless configured: dark without "enabled": true, and applyDefaults
normalises a disabled block to nil. deploy/mavend.json carries it disabled.

BLUETOOTH IS NOT SHIPPED, AND IS BLOCKED, NOT SKIPPED. The plan's other half
(internal/bluetooth/, RSSI presence probes) needs a bluez stack that is not
here: bluetoothctl and hcitool are not installed, bluetoothd is not installed,
the bluetooth unit is inactive, and org.bluez is not on the system bus. hci0
exists as a kernel device and nothing can talk to it. The docker deploy is
further away still — it would need host networking, the D-Bus system socket
passed in, and CAP_NET_ADMIN. Writing an exec wrapper around a binary that does
not exist, against an output format nothing here can produce, would be a guess
dressed as a feature. It needs a decision about privileging the container before
any of it is worth writing.

Vikunja #257

Scan the LAN, bounded to configured subnets (#257) internal/netscan/ discovers hosts on the network Maven is configured to look at: a TCP-connect scan (net.DialTimeout, no raw sockets, no privileges) plus a read of the kernel's ARP cache. Wired as a read-only query source, "network", so "какие устройства в сети?" is answered by a scan instead of by whatever old note happens to be nearest. Scanning is a read, but an unbounded scanner on a home LAN is noisy and easy to point somewhere it should not go, so the package is built around four bounds: - Scan takes NO target argument. The range comes from the config block and from nowhere else, so there is no exported way to scan an arbitrary prefix and nothing an utterance, the router, or a scanned host says can retarget it. That is asserted directly: the test watches every address handed to the dialer and fails if one falls outside the configured prefix. The ARP cache — the one input the network itself populates — is filtered to the configured range for the same reason. - Every configured CIDR must be private (RFC1918 / CGNAT / link-local) and no larger than 1024 addresses. 8.8.8.0/24, 0.0.0.0/0 and 10.0.0.0/8 are refused at config load, not after the packets have left. - Rate-limited to a configured connections-per-second across the whole scan, so it looks like background traffic rather than a portscan. - Bounded in total by MaxHosts, a per-connection timeout, a 20s turn budget and the context; a canceled scan stops dialing immediately. Off unless configured: dark without "enabled": true, and applyDefaults normalises a disabled block to nil. deploy/mavend.json carries it disabled. BLUETOOTH IS NOT SHIPPED, AND IS BLOCKED, NOT SKIPPED. The plan's other half (internal/bluetooth/, RSSI presence probes) needs a bluez stack that is not here: bluetoothctl and hcitool are not installed, bluetoothd is not installed, the bluetooth unit is inactive, and org.bluez is not on the system bus. hci0 exists as a kernel device and nothing can talk to it. The docker deploy is further away still — it would need host networking, the D-Bus system socket passed in, and CAP_NET_ADMIN. Writing an exec wrapper around a binary that does not exist, against an output format nothing here can produce, would be a guess dressed as a feature. It needs a decision about privileging the container before any of it is worth writing. Vikunja #257
claude added 1 commit 2026-08-01 04:41:35 +02:00
internal/netscan/ discovers hosts on the network Maven is configured to look at:
a TCP-connect scan (net.DialTimeout, no raw sockets, no privileges) plus a read
of the kernel's ARP cache. Wired as a read-only query source, "network", so
"какие устройства в сети?" is answered by a scan instead of by whatever old note
happens to be nearest.

Scanning is a read, but an unbounded scanner on a home LAN is noisy and easy to
point somewhere it should not go, so the package is built around four bounds:

  - Scan takes NO target argument. The range comes from the config block and
    from nowhere else, so there is no exported way to scan an arbitrary prefix
    and nothing an utterance, the router, or a scanned host says can retarget
    it. That is asserted directly: the test watches every address handed to the
    dialer and fails if one falls outside the configured prefix. The ARP cache —
    the one input the network itself populates — is filtered to the configured
    range for the same reason.
  - Every configured CIDR must be private (RFC1918 / CGNAT / link-local) and no
    larger than 1024 addresses. 8.8.8.0/24, 0.0.0.0/0 and 10.0.0.0/8 are refused
    at config load, not after the packets have left.
  - Rate-limited to a configured connections-per-second across the whole scan,
    so it looks like background traffic rather than a portscan.
  - Bounded in total by MaxHosts, a per-connection timeout, a 20s turn budget
    and the context; a canceled scan stops dialing immediately.

Off unless configured: dark without "enabled": true, and applyDefaults
normalises a disabled block to nil. deploy/mavend.json carries it disabled.

BLUETOOTH IS NOT SHIPPED, AND IS BLOCKED, NOT SKIPPED. The plan's other half
(internal/bluetooth/, RSSI presence probes) needs a bluez stack that is not
here: bluetoothctl and hcitool are not installed, bluetoothd is not installed,
the bluetooth unit is inactive, and org.bluez is not on the system bus. hci0
exists as a kernel device and nothing can talk to it. The docker deploy is
further away still — it would need host networking, the D-Bus system socket
passed in, and CAP_NET_ADMIN. Writing an exec wrapper around a binary that does
not exist, against an output format nothing here can produce, would be a guess
dressed as a feature. It needs a decision about privileging the container before
any of it is worth writing.

Vikunja #257
claude reviewed 2026-08-01 11:38:06 +02:00
claude left a comment
Author
Contributor

Scan(ctx) taking no target is the correct API, and it is what makes the rest of the bounds hold. There is no exported path from an utterance to a range, so the prompt-injection story is closed by shape rather than by validation. Rejecting a non-private prefix and anything wider than a /22 at config load, in netscan.Validate, means the daemon cannot be talked into a scan it would not have done at boot. TCP-connect plus an ARP-table read, with no raw sockets and no root, is the right cost for the answer. The Russian count inflection in hostWord is correct including the teens.

1. results is sized by hosts and written by host-port pairs. A dense subnet deadlocks the scan.

results := make(chan result, len(targets))

Each worker sends one value per open port, so the number of sends is bounded by len(targets) * len(s.cfg.Ports), not by len(targets). Nothing drains the channel until after wg.Wait() and close(results).

Walk it with subnets: ["192.168.1.0/28"] and the default four ports. targets yields 14 addresses, so the buffer holds 14. Six live devices answer on three ports each, which is ordinary for a router, a NAS and a couple of Pis: 18 sends. Send 15 blocks. The worker holding it never returns, wg.Wait() never returns, close and the drain loop are never reached.

The context does not save it. The goroutines are blocked on a channel send with no select on ctx.Done(), so scanBudget expiring changes nothing. scanSummary never returns, the voice turn that called it hangs for the life of the process, and 16 goroutines plus the semaphore leak with it.

The test suite cannot see this: the stub dialer would have to open more ports than there are targets. Size the buffer len(targets)*len(s.cfg.Ports), or drain in a goroutine started before the loop.

2. The default scan cannot finish inside the default budget, and a truncated scan is reported as the network.

deploy/mavend.json ships 192.168.1.0/24, four ports, rate: 50, max_hosts: 256. targets returns 254 addresses. That is 1016 probes. The ticker interval is time.Second / 50, 20ms. 1016 × 20ms is 20.32 seconds. scanBudget is 20 seconds.

So the shipped configuration always hits the deadline, roughly 16 probes short, plus whatever the 400ms tail dials cost. The addresses are walked in ascending order, so it is always the top of the range that goes unprobed. A device parked at .250 is invisible, deterministically, and re-asking does not help.

Scan then returns nil for the error on the break scan path. scanSummary has no way to know the run was cut off and says "нашла 6 устройств" as a statement about the LAN. Return a sentinel or a bool for a truncated run and have her say "успела посмотреть не всю сеть". Either raise the default rate, or lower max_hosts, or raise scanBudget so the shipped numbers are self-consistent.

3. With two subnets, only the first is ever scanned, silently.

targets iterates cfg.Subnets in order and returns as soon as len(out) >= MaxHosts. Validate bounds each subnet at MaxPrefixHosts separately and never looks at the sum.

subnets: ["192.168.1.0/24", "192.168.2.0/24"] with the default max_hosts of 256 therefore yields 254 addresses from the first subnet and 2 from the second. Configuration passes, boot logs nothing, and the second LAN is 99% dark. He asks what is on the network and gets an answer about one of the two ranges he named.

Either round-robin across subnets, or make Validate reject a configuration whose subnets sum past MaxHosts, or say in NetScanConfig that MaxHosts is consumed in order.

4. The reply reads raw IPv4 addresses out loud.

scanSummary builds "192.168.1.1 (80, 443); 192.168.1.14 (22)" and that string is the spoken reply. Piper will read it as a digit stream, and this is the query path, so it goes to the voice client as well as to /chat. Six of them in one sentence is not an answer anybody can use through a speaker.

The MAC is already collected and thrown away. Speak the count and the shape ("нашла 6 устройств, из них два с вебом"), and put the address list where it can be read. That is also the case for writing the scan into #283's intake journal, which it currently does not touch at all.

Smaller notes:

  • queryNetwork claims the turn with handled=true when h.netscan is nil, so an unconfigured box stops falling through to recall for "сколько устройств в сети?". Same shape as the queryHome note on PR 80, and the same fix: return "", false.
  • Nothing about a scan is written anywhere. No fact, no note, no envelope on /events. A scan is a read, so that is defensible, but it means there is no record that Maven put packets on the LAN at 03:00 and no way to answer "when did she last scan".
  • No caching. Two questions in a row are two full scans, each up to 20 seconds and 1016 connections. A short result cache would make the repeat question free and halve the traffic.
  • Host.Up is defined and never called.
  • The network-word list matches on the bare substring "сети", which is inside "посетил". It needs a device noun and an ask on top, so the reachable false positives are contrived ("сколько машин я посетил?"). Whole-token matching with homeWord would close it, and that helper is already imported from netscan.go's sibling file.
  • Validate allows rate: 100000. Scan floors the interval at 1ms, so the real ceiling is about 1000 connections per second. Worth a stated bound rather than a silent floor, given the package comment promises a scan looks like background traffic.
`Scan(ctx)` taking no target is the correct API, and it is what makes the rest of the bounds hold. There is no exported path from an utterance to a range, so the prompt-injection story is closed by shape rather than by validation. Rejecting a non-private prefix and anything wider than a /22 at config load, in `netscan.Validate`, means the daemon cannot be talked into a scan it would not have done at boot. TCP-connect plus an ARP-table read, with no raw sockets and no root, is the right cost for the answer. The Russian count inflection in `hostWord` is correct including the teens. **1. `results` is sized by hosts and written by host-port pairs. A dense subnet deadlocks the scan.** ```go results := make(chan result, len(targets)) ``` Each worker sends one value per open port, so the number of sends is bounded by `len(targets) * len(s.cfg.Ports)`, not by `len(targets)`. Nothing drains the channel until after `wg.Wait()` and `close(results)`. Walk it with `subnets: ["192.168.1.0/28"]` and the default four ports. `targets` yields 14 addresses, so the buffer holds 14. Six live devices answer on three ports each, which is ordinary for a router, a NAS and a couple of Pis: 18 sends. Send 15 blocks. The worker holding it never returns, `wg.Wait()` never returns, `close` and the drain loop are never reached. The context does not save it. The goroutines are blocked on a channel send with no `select` on `ctx.Done()`, so `scanBudget` expiring changes nothing. `scanSummary` never returns, the voice turn that called it hangs for the life of the process, and 16 goroutines plus the semaphore leak with it. The test suite cannot see this: the stub dialer would have to open more ports than there are targets. Size the buffer `len(targets)*len(s.cfg.Ports)`, or drain in a goroutine started before the loop. **2. The default scan cannot finish inside the default budget, and a truncated scan is reported as the network.** `deploy/mavend.json` ships `192.168.1.0/24`, four ports, `rate: 50`, `max_hosts: 256`. `targets` returns 254 addresses. That is 1016 probes. The ticker interval is `time.Second / 50`, 20ms. 1016 × 20ms is 20.32 seconds. `scanBudget` is 20 seconds. So the shipped configuration always hits the deadline, roughly 16 probes short, plus whatever the 400ms tail dials cost. The addresses are walked in ascending order, so it is always the top of the range that goes unprobed. A device parked at .250 is invisible, deterministically, and re-asking does not help. `Scan` then returns `nil` for the error on the `break scan` path. `scanSummary` has no way to know the run was cut off and says "нашла 6 устройств" as a statement about the LAN. Return a sentinel or a bool for a truncated run and have her say "успела посмотреть не всю сеть". Either raise the default rate, or lower `max_hosts`, or raise `scanBudget` so the shipped numbers are self-consistent. **3. With two subnets, only the first is ever scanned, silently.** `targets` iterates `cfg.Subnets` in order and returns as soon as `len(out) >= MaxHosts`. `Validate` bounds each subnet at `MaxPrefixHosts` separately and never looks at the sum. `subnets: ["192.168.1.0/24", "192.168.2.0/24"]` with the default `max_hosts` of 256 therefore yields 254 addresses from the first subnet and 2 from the second. Configuration passes, boot logs nothing, and the second LAN is 99% dark. He asks what is on the network and gets an answer about one of the two ranges he named. Either round-robin across subnets, or make `Validate` reject a configuration whose subnets sum past `MaxHosts`, or say in `NetScanConfig` that `MaxHosts` is consumed in order. **4. The reply reads raw IPv4 addresses out loud.** `scanSummary` builds `"192.168.1.1 (80, 443); 192.168.1.14 (22)"` and that string is the spoken reply. Piper will read it as a digit stream, and this is the query path, so it goes to the voice client as well as to `/chat`. Six of them in one sentence is not an answer anybody can use through a speaker. The MAC is already collected and thrown away. Speak the count and the shape ("нашла 6 устройств, из них два с вебом"), and put the address list where it can be read. That is also the case for writing the scan into #283's intake journal, which it currently does not touch at all. Smaller notes: - `queryNetwork` claims the turn with `handled=true` when `h.netscan` is nil, so an unconfigured box stops falling through to recall for "сколько устройств в сети?". Same shape as the `queryHome` note on PR 80, and the same fix: return `"", false`. - Nothing about a scan is written anywhere. No fact, no note, no envelope on `/events`. A scan is a read, so that is defensible, but it means there is no record that Maven put packets on the LAN at 03:00 and no way to answer "when did she last scan". - No caching. Two questions in a row are two full scans, each up to 20 seconds and 1016 connections. A short result cache would make the repeat question free and halve the traffic. - `Host.Up` is defined and never called. - The network-word list matches on the bare substring `"сети"`, which is inside `"посетил"`. It needs a device noun and an ask on top, so the reachable false positives are contrived ("сколько машин я посетил?"). Whole-token matching with `homeWord` would close it, and that helper is already imported from `netscan.go`'s sibling file. - `Validate` allows `rate: 100000`. `Scan` floors the interval at 1ms, so the real ceiling is about 1000 connections per second. Worth a stated bound rather than a silent floor, given the package comment promises a scan looks like background traffic.
kami closed this pull request 2026-08-01 14:52:07 +02:00
Owner

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Pull request closed

Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#81