From dd0f6e483aa93fae31f544b5d9746f2388a769a8 Mon Sep 17 00:00:00 2001 From: Julian Cuni Date: Fri, 26 Jun 2026 16:47:12 +0200 Subject: [PATCH] =?UTF-8?q?fix(reader):=20real=20ICMP=20liveness=20?= =?UTF-8?q?=E2=80=94=20QR=20reader=20status=20was=20a=20hardcoded=20"ready?= =?UTF-8?q?"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two genuinely-offline QR readers showed GREEN: the adapter's healthCheck was hardcoded to { ready, "stub" } and never probed. These are PUSH devices (scan → GET our backend, resolve by serial) with NO TCP port, so a connect probe has nothing to hit — the stub "solved" that by lying. False-healthy is the worst failure for a status bar. - Optional reader IP field (monitor-ONLY; scans still resolve by serial, operation unchanged). - Unprivileged ICMP ping (drivers/icmp.ts): shells /bin/ping -c1, exit-0 = reply. No native dep, no CAP_NET_RAW. docker-compose.prod.yml sets net.ipv4.ping_group_range so it works for the non-root container user. - healthCheck: replies → ready, no reply → offline, NO IP → degraded ("set IP to monitor") — never a false green. Verified on hardware: readers (10.0.10.7/.8) answer ICMP on the device VLAN; UI Test connection → "● ready — ping 10.0.10.7". Tests: reader.test.ts (4). Claude-Session: https://claude.ai/code/session_01Xcm6ikLgGoCxxHrxtjkk5V --- docker-compose.prod.yml | 7 ++++ packages/devices/src/drivers/icmp.ts | 36 +++++++++++++++++ packages/devices/src/drivers/reader.test.ts | 43 +++++++++++++++++++++ packages/devices/src/drivers/reader.ts | 26 ++++++++++++- 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 packages/devices/src/drivers/icmp.ts create mode 100644 packages/devices/src/drivers/reader.test.ts diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index ce7656e..0b4a7d7 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -36,6 +36,13 @@ services: # No published port — only the proxy reaches the server, over the private network. expose: - "3000" + # Let the server ICMP-ping push-only readers (Dingtian/GEE QR) for an honest + # online/offline status WITHOUT CAP_NET_RAW: opening ping_group_range to all gids + # enables `/bin/ping` in unprivileged SOCK_DGRAM mode for the non-root runtime user. + # (The reader exposes no TCP port, so a connect-probe can't work — see reader.ts / + # wiki/entities/dingtian-qr-reader.md.) + sysctls: + - net.ipv4.ping_group_range=0 2147483647 logging: driver: json-file options: diff --git a/packages/devices/src/drivers/icmp.ts b/packages/devices/src/drivers/icmp.ts new file mode 100644 index 0000000..a7b8689 --- /dev/null +++ b/packages/devices/src/drivers/icmp.ts @@ -0,0 +1,36 @@ +import { execFile } from "node:child_process"; + +// Unprivileged ICMP liveness check for PUSH-only devices that expose no TCP port — +// e.g. the Dingtian/GEE QR readers, which GET our backend on each scan but listen on +// nothing. For those a TCP connect probe (what cameras/printers use) has nothing to +// connect to; ICMP echo is the only honest "powered + on-network" signal. +// +// We shell to the system `ping` rather than open a raw socket: Node's `dgram` is +// UDP-only (no IPPROTO_ICMP), and a raw socket needs CAP_NET_RAW. `/bin/ping` in +// SOCK_DGRAM mode runs WITHOUT NET_RAW when the kernel's `net.ipv4.ping_group_range` +// includes the runtime user's gid — which the booth compose sets as a sysctl (see +// docker-compose.prod.yml). So: no native dep, no NET_RAW. A ping only proves the box +// answers ICMP (not that the scan head works) — but it correctly flips red when the +// reader is unplugged/dead, which the old hardcoded "ready" never did. +// See wiki/entities/dingtian-qr-reader.md / device-status-monitoring.md. + +/** + * Send ONE ICMP echo to `host` and resolve true if it replied within `timeoutMs`. + * Never throws — any spawn/permission/timeout failure resolves false (treated as + * "not reachable"). Linux `ping` flags: `-n` numeric (no DNS), `-c 1` one packet, + * `-w`/`-W` deadline. We pass the host as a fixed arg (execFile, not a shell) so a + * crafted "host" can't inject a command. + */ +export function icmpPing(host: string, timeoutMs = 2000): Promise { + const deadlineSec = Math.max(1, Math.ceil(timeoutMs / 1000)); + return new Promise((resolve) => { + const child = execFile( + "ping", + ["-n", "-c", "1", "-w", String(deadlineSec), "-W", String(deadlineSec), host], + { timeout: timeoutMs + 500 }, + (err) => resolve(err == null), // exit 0 = a reply; anything else = no reply + ); + // If the binary is missing entirely, execFile emits 'error' (callback also fires). + child.on("error", () => resolve(false)); + }); +} diff --git a/packages/devices/src/drivers/reader.test.ts b/packages/devices/src/drivers/reader.test.ts new file mode 100644 index 0000000..94ebcbf --- /dev/null +++ b/packages/devices/src/drivers/reader.test.ts @@ -0,0 +1,43 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +// Reader health: push-only QR readers expose no TCP port, so liveness is an ICMP +// ping of the (optional) configured IP. With no IP we must NOT claim "ready" (the old +// stub did, hiding offline readers behind a green dot) — we report degraded instead. +// icmpPing is mocked so the test is deterministic + offline. + +const icmpPing = vi.fn<(host: string, timeoutMs?: number) => Promise>(); +vi.mock("./icmp.js", () => ({ icmpPing: (...a: [string, number?]) => icmpPing(...a) })); + +const { geeQrReaderDriver } = await import("./reader.js"); + +afterEach(() => { + icmpPing.mockReset(); +}); + +describe("QR reader healthCheck (ICMP liveness)", () => { + it("with an IP that replies → ready", async () => { + icmpPing.mockResolvedValue(true); + const r = geeQrReaderDriver.create({ serial: "H05M2AFA", host: "10.0.10.7" }); + expect(await r.healthCheck()).toEqual({ status: "ready", detail: "ping 10.0.10.7" }); + expect(icmpPing).toHaveBeenCalledWith("10.0.10.7"); + }); + + it("with an IP that does NOT reply → offline (this is the bug fix)", async () => { + icmpPing.mockResolvedValue(false); + const r = geeQrReaderDriver.create({ serial: "H05M2AFA", host: "10.0.10.7" }); + expect(await r.healthCheck()).toEqual({ status: "offline", detail: "no ping reply from 10.0.10.7" }); + }); + + it("with NO IP → degraded (never a false 'ready')", async () => { + const r = geeQrReaderDriver.create({ serial: "H05M2AFA" }); + const h = await r.healthCheck(); + expect(h.status).toBe("degraded"); + expect(icmpPing).not.toHaveBeenCalled(); // nothing to ping + }); + + it("exposes an optional host field for monitoring", () => { + const hostField = geeQrReaderDriver.configFields.find((f) => f.key === "host"); + expect(hostField).toBeDefined(); + expect(hostField!.required).toBe(false); // operation is push-by-serial; IP is monitor-only + }); +}); diff --git a/packages/devices/src/drivers/reader.ts b/packages/devices/src/drivers/reader.ts index 87bb14a..a410430 100644 --- a/packages/devices/src/drivers/reader.ts +++ b/packages/devices/src/drivers/reader.ts @@ -1,6 +1,7 @@ import type { DeviceHealth, ReaderDevice, ReaderEvent } from "../interfaces.js"; import type { DeviceConfig, ReaderDriver } from "../registry.js"; import { hostField, portField, stubLog } from "./common.js"; +import { icmpPing } from "./icmp.js"; // Reader drivers (RF / optical). Two integration paths: Wiegand reads reach the // access controller directly (autonomous); TCP-IP readers are seen host-side. @@ -18,8 +19,23 @@ class StubReader implements ReaderDevice { async disconnect(): Promise { stubLog(this.driverId, "disconnect"); } + /** + * Liveness. These readers PUSH (scan → GET our backend) and expose no TCP port, so + * there's nothing to connect-probe. If the admin gave the reader's IP we ICMP-ping + * it (powered + on-network); a reply → ready, no reply → offline. With NO IP we + * report `degraded` ("set IP to monitor") rather than a false `ready` — a push + * device that's silent is indistinguishable from a dead one, so claiming `ready` + * unconditionally (the old behaviour) hid offline readers behind a green dot. + */ async healthCheck(): Promise { - return { status: "ready", detail: "stub" }; + const host = this.config.host ? String(this.config.host) : ""; + if (!host) { + return { status: "degraded", detail: "push device — set IP to monitor" }; + } + const alive = await icmpPing(host); + return alive + ? { status: "ready", detail: `ping ${host}` } + : { status: "offline", detail: `no ping reply from ${host}` }; } onRead(cb: (r: ReaderEvent) => void): void { this.#cb = cb; @@ -80,6 +96,14 @@ export const geeQrReaderDriver: ReaderDriver = { required: true, help: "The reader's serial as it reports in each scan (the `cjihao` field). Used to map scans to this lane.", }, + { + // OPTIONAL: the reader pushes by serial (operation needs no IP), but giving its + // IP lets the status monitor ICMP-ping it for a real online/offline dot instead + // of an always-green stub. Leave blank to skip monitoring (shows "set IP"). + ...hostField, + required: false, + help: "Optional: the reader's IP, used ONLY to monitor it (ping). Scans still resolve by serial. Leave blank to skip liveness monitoring.", + }, ], create: (c) => new StubReader("gee-qr-reader", c), };