From ade2f38ee3be1cddcca9ab526553dbf53ff395cf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 20:35:27 -0500 Subject: [PATCH] cg-net: reap disconnected clients with a process sentinel Finding 5 of the 2026-07-29 security review: nothing watched for a connection ending, so players who left stayed on the client list forever and the host kept trying to send to them. cg-net--host-sentinel removes a closed connection from the list; it is installed on every accepted connection. This also tidies the drops made by the Finding 4 bounds, which delete-process over-limit peers. Seat numbers are still not reused after a departure -- deliberate, and recorded in the sentinel docstring: a stale seat must not be inherited by a stranger mid-game. Named in the handback as remaining behavior. Test cgt-net-reaps-disconnected fails against the previous code: after the client disconnected the list still held it (= 0 1). --- cg-net.el | 11 +++++++++++ test/card-games-tests.el | 26 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/cg-net.el b/cg-net.el index cb1666e..42a5105 100644 --- a/cg-net.el +++ b/cg-net.el @@ -214,6 +214,16 @@ The socket binds `cg-net-host-address' -- by default, this machine only." (delete-process (cg-net-host-server cg-net--host))) (setq cg-net--host nil))) +(defun cg-net--host-sentinel (proc _event) + "Reap PROC from the client list when its connection has ended. +Without this, departed players stay listed forever and the host keeps +sending to them. Seat numbers are deliberately not reused: a stale +seat must not be inherited by a stranger mid-game." + (unless (process-live-p proc) + (when cg-net--host + (setf (cg-net-host-clients cg-net--host) + (delq proc (cg-net-host-clients cg-net--host)))))) + (defun cg-net--host-accept (_server connection _message) "Set up an accepted CONNECTION: assign a seat and send the current state. A connection arriving past `cg-net-max-connections' is closed instead." @@ -229,6 +239,7 @@ A connection arriving past `cg-net-max-connections' is closed instead." (push connection (cg-net-host-clients cg-net--host)) (process-put connection 'cg-net-seat seat) (set-process-coding-system connection 'utf-8 'utf-8) + (set-process-sentinel connection #'cg-net--host-sentinel) (set-process-filter connection (cg-net--filter #'cg-net--host-handle)) (cg-net--send connection (list :type 'welcome :seat seat)) (cg-net--send connection diff --git a/test/card-games-tests.el b/test/card-games-tests.el index cc0f046..c74f344 100644 --- a/test/card-games-tests.el +++ b/test/card-games-tests.el @@ -181,6 +181,32 @@ Finding 4's second bound: the client list cannot grow without limit." (dolist (p procs) (when (process-live-p p) (delete-process p))) (cg-net-host-stop))))) +(ert-deftest cgt-net-reaps-disconnected () + "A client that disconnects is removed from the host's client list. +Finding 5 of the 2026-07-29 review: nothing watched for a connection +ending, so departed players stayed listed forever and the host kept +sending to them. A process sentinel now reaps them." + (condition-case _ + (delete-process + (make-network-process :name "cgt-probe7" :server t :service 0 + :host "127.0.0.1" :family 'ipv4)) + (error (ert-skip "TCP not available"))) + (cl-flet ((pump () (dotimes (_ 12) (accept-process-output nil 0.05)))) + (let* ((hgame (make-instance 'cgt-net-game :env (list :counter 0))) + (srv (cg-net-host-start hgame 0)) + (port (process-contact srv :service)) + (cgame (make-instance 'cgt-net-game :env (list :counter 0)))) + (unwind-protect + (progn + (cg-net-connect "127.0.0.1" port "Test" cgame) + (pump) + (should (= 1 (length (cg-net-host-clients cg-net--host)))) + (cg-net-disconnect) + (pump) + (should (= 0 (length (cg-net-host-clients cg-net--host))))) + (cg-net-disconnect) + (cg-net-host-stop))))) + ;;;; Gaps (ert-deftest cgt-gaps-deal ()