fix(reader): real ICMP liveness — QR reader status was a hardcoded "ready"
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
This commit is contained in:
@@ -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<boolean> {
|
||||
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));
|
||||
});
|
||||
}
|
||||
@@ -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<boolean>>();
|
||||
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
|
||||
});
|
||||
});
|
||||
@@ -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<void> {
|
||||
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<DeviceHealth> {
|
||||
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),
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user