api-server: dead logcli PIDs are never removed from the WebSocket session

[DavidePrincipi]

Steps to reproduce

• Open the System logs page and start a search in follow (tail) mode. Note the pid returned in the first logs-start response.
• Make the spawned logcli exit on its own, without sending logs-stop — e.g. stop the Loki module so the running query fails.
• Send a logs-stop action for that same pid.

Expected behavior

Once logcli has exited, its PID is no longer tracked by the connection, so the late logs-stop finds nothing to stop and no kill is issued.

Actual behavior

The server still replies to the late logs-stop with "logs follow stopped", and issues kill(-pid, SIGTERM) on the way — proving the dead PID is still tracked. A reply is only written when the PID matches an entry in the connection's process list, so it is a reliable signal that the entry was never removed.

That list is the childs key of the per-connection store provided by the Melody WebSocket library (github.com/olahol/melody). The PID is appended to it when the command starts (core/api-server/socket/action.go:200-207) and is removed only by the logs-stop handler, when a client-supplied PID matches (core/api-server/socket/action.go:81). When logcli exits by itself — Loki unreachable, a query rejected at runtime, or a failed cmd.Start() — the PID is never removed and stays in the list for the rest of the WebSocket session.

Two consequences:

1. The list grows by one dead PID per failed search in a long-lived session. The memory cost is negligible, but onDisconnect (core/api-server/socket/socket.go:81-85) then iterates over them.

2. syscall.Kill(-pid, syscall.SIGTERM) is issued for a stale PID. Against a dead PID it is a harmless ESRCH, but if the kernel recycles that PID as a process-group leader before the session closes, the negative-PID kill signals an unrelated process group. The same exposure exists through logs-stop: the frontend still holds the PID it was handed, and a stop for an already-dead search matches the stale entry and delivers the kill.

The blast radius of point 2 is limited: api-server runs as User=api-server (core/imageroot/etc/systemd/system/api-server.service), and kill(2) requires the sender's UID to match the target's real or saved-set UID. A recycled PID can therefore only be signalled if it is owned by the api-server user too — in practice one of api-server's own logcli children from another session or search, which would die silently mid-follow. It also needs PID wraparound within a single WebSocket session's lifetime. Low severity; the cleanup is worth doing on correctness grounds more than risk.

Suggested fix

Remove the PID from childs once cmd.Wait() returns, on every outcome (clean exit, error exit, signal).

The same list also has an unsynchronised read-modify-write worth addressing together: s.Get("childs") → mutate → s.Set("childs") is atomic per call in Melody but not as a sequence, and append(aChilds[:idx], aChilds[idx+1:]...) (core/api-server/socket/action.go:81) shifts the backing array in place. Two concurrent tail commands in one session can therefore lose an entry — another route to a PID that is never cleaned up.

Components

ns8-core 3.21.0 (also present on main, 3.21.1-dev.1) — core/api-server, WebSocket logs-start / logs-stop actions.

See also

Pre-existing behaviour, not introduced by any specific change. Found while reviewing NethServer/ns8-core#1272, which made these logcli failure paths observable for the first time; deliberately kept out of that PR's scope.