Stop the web UI serving numbers the enforcer is not applying
Some checks are pending
dependency freshness / dependencies on newest stable (push) Waiting to run
md-naming / markdown naming convention (push) Waiting to run
pre-commit / pre-commit (push) Waiting to run
Tests / test (push) Waiting to run

A server started as `python -c "from ..._web_server import serve; serve(
port=8000)"` ran for five hours after the code beneath it changed, reporting
an 8h gaming budget while the daemon enforced 6h and had already cut gaming
off. Two separate holes let that happen.

The staleness check only ran from cmd_serve, so a server started any other
way never met it. And is_our_server matched argv containing
"steam_backlog_enforcer.main", which a `python -c` process does not have --
so that server could not even be identified as ours: `run.sh serve` would
have refused the port and named it as a stranger rather than replacing it.

_serve_stale puts the check inside the server, where no launch path can skip
it. Every route, static included, returns 503 once any package .py is newer
than this process, because a fresh page fetching stale numbers is the same
lie with extra steps. The answer is memoised for five seconds so it does not
stat the package per request; that bounds the staleness window instead of
leaving it open forever.

Refusing is only half a fix -- a process that 503s forever is honest and
useless -- so a refused request also stands the server down. The new
steam-backlog-enforcer-web.service supplies the other half with
Restart=always: it dies on stale code and returns on current code with
nobody having to remember anything. A user unit, not a system one; it serves
read-only views and needs no privilege. install.sh installs and enables it
rather than printing instructions.

is_our_server now substring-matches the joined argv, so run.sh serve,
python -m ...main serve and a bare python -c are all recognised.

newest_py_since moves into _serve_stale and _serve_guard delegates to it.
Both needed the same scan, and two copies would have been free to disagree
about whether a server is current -- which is the entire failure being fixed.

Verified end to end: healthy 200, touch _budget_view.py, next request 503,
systemd replaces the process, back to 200 reporting 6.0h and blocked=True.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fcwu78wk17FWMHbbfbyicc
This commit is contained in:
Krzysztof Rudnicki 2026-08-29 23:39:40 +02:00
parent 8d31e8e9af
commit 433a1d6c28
8 changed files with 444 additions and 25 deletions

View File

@ -32,6 +32,38 @@ else
echo "(The rest of this tool is unaffected.)"
fi
# Install the web-UI user unit. No root needed: it serves read-only views and
# needs no privilege. Automated rather than documented, because the whole
# reason this unit exists is that a hand-started server outlived its own code.
install_web_user_unit() {
local target_user home_dir dst src
target_user="${SUDO_USER:-$USER}"
home_dir="$(getent passwd "$target_user" | cut -d: -f6)"
if [[ -z $home_dir ]]; then
echo "Warning: cannot resolve home for $target_user; skipping web unit."
return 0
fi
src="$SCRIPT_DIR/steam-backlog-enforcer-web.service"
dst="$home_dir/.config/systemd/user/steam-backlog-enforcer-web.service"
mkdir -p "$(dirname "$dst")"
sed "s|__REPO_DIR__|$SCRIPT_DIR|g" "$src" > "$dst"
if [[ $EUID -eq 0 ]]; then
chown "$target_user" "$dst"
sudo -u "$target_user" \
XDG_RUNTIME_DIR="/run/user/$(id -u "$target_user")" \
systemctl --user daemon-reload
sudo -u "$target_user" \
XDG_RUNTIME_DIR="/run/user/$(id -u "$target_user")" \
systemctl --user enable --now steam-backlog-enforcer-web || true
else
systemctl --user daemon-reload
systemctl --user enable --now steam-backlog-enforcer-web || true
fi
echo "Installed and enabled: $dst"
}
install_web_user_unit
# Install systemd service (system-level, runs as root).
#
# Non-interactive callers (install_core_system.sh, CI, `vm run`) have no stdin,

View File

@ -0,0 +1,26 @@
[Unit]
Description=Steam Backlog Enforcer web UI (localhost only)
After=network-online.target
[Service]
Type=simple
WorkingDirectory=__REPO_DIR__
ExecStart=__REPO_DIR__/run.sh serve
Restart=always
RestartSec=3s
Environment=PYTHONUNBUFFERED=1
# Why this unit exists at all: the UI used to be a hand-started `python -c`
# that stayed up for hours after the code beneath it changed, reporting an 8h
# gaming budget while the daemon enforced 6h. _serve_stale now makes the server
# refuse to answer once its own source has moved on -- but refusing is only
# half a fix if nothing brings it back. Restart=always is the other half: the
# server dies on stale code and returns on current code, with no one having to
# remember anything.
#
# Deliberately a *user* unit, not a system one. It reads state and serves
# read-only views; it needs no privilege, and running it as root would give a
# network-facing process rights the daemon's own hardening notes warn about.
[Install]
WantedBy=default.target

View File

@ -20,6 +20,8 @@ import signal
import socket
import time
from steam_backlog_enforcer._serve_stale import newest_py_after
_PROC = Path("/proc")
_NET_TCP = (_PROC / "net" / "tcp", _PROC / "net" / "tcp6")
@ -27,13 +29,12 @@ _NET_TCP = (_PROC / "net" / "tcp", _PROC / "net" / "tcp6")
# not a bind, and must not be mistaken for the process owning the port.
_LISTEN_STATE = "0A"
# argv fragments that identify one of our own web servers.
_OWN_MODULE = "steam_backlog_enforcer.main"
# argv fragments identifying our own web servers, matched as substrings over
# the joined argv: a `python -c` server never holds "…enforcer.main", so an
# exact match missed it entirely. See _serve_stale for what that cost.
_OWN_PACKAGE = "steam_backlog_enforcer"
_OWN_COMMAND = "serve"
# Package tree whose .py mtimes decide whether a running server is stale.
_PACKAGE_ROOT = Path(__file__).resolve().parent
# A wildcard IPv4 bind renders as an all-zero address column. Spelled as hex
# rather than _hex_v4("0.0.0.0") so the dotted-quad literal never appears: this
# module only ever *matches* that address, it must never bind it.
@ -155,9 +156,14 @@ def read_cmdline(pid: int) -> list[str]:
def is_our_server(pid: int) -> bool:
"""Check whether *pid* is one of our own ``serve`` processes."""
argv = read_cmdline(pid)
return _OWN_MODULE in argv and _OWN_COMMAND in argv
"""Check whether *pid* is one of our own ``serve`` processes.
Recognises every launch form -- ``run.sh serve``, ``python -m …main
serve``, and a bare ``python -c``. One we fail to recognise can be neither
replaced when stale nor named accurately when it holds the port.
"""
argv = " ".join(read_cmdline(pid))
return _OWN_PACKAGE in argv and _OWN_COMMAND in argv
def process_started_at(pid: int) -> float | None:
@ -176,19 +182,11 @@ def process_started_at(pid: int) -> float | None:
def newest_py_since(started_at: float) -> Path | None:
"""Return the newest ``.py`` in the package modified after *started_at*.
Returning the path rather than a bool is deliberate: the restart message
names the file, so the decision to kill a server is always auditable.
Delegates to :mod:`_serve_stale`, which owns this scan because the server
itself needs the same answer about its own process. Two copies would be
free to disagree about whether a server is current.
"""
newest: Path | None = None
newest_mtime = started_at
for path in _PACKAGE_ROOT.rglob("*.py"):
try:
mtime = path.stat().st_mtime
except OSError:
continue
if mtime > newest_mtime:
newest, newest_mtime = path, mtime
return newest
return newest_py_after(started_at)
def _own_pid_chain() -> set[int]:

View File

@ -0,0 +1,103 @@
"""Self-check: is this server process running code that has since changed?
``_serve_guard`` already answers this about *other* processes, at launch time,
so ``serve`` can replace a stale server before taking the port. That guard has
two holes, both of which were open on 2026-08-29:
1. It only runs from ``cmd_serve``. A server started as
``python -c "from ..._web_server import serve; serve(port=8000)"`` never
passes through it.
2. :func:`~steam_backlog_enforcer._serve_guard.is_our_server` matches argv
holding ``steam_backlog_enforcer.main``, which such a process does not have,
so it is not even recognised as ours to replace.
The result was a UI reporting an 8-hour budget while the daemon enforced six,
which is precisely the divergence the shared ``rules_for`` seam exists to
prevent -- defeated not by a second copy of the logic but by a second copy of
the *process*.
So the check also lives here, inside the server, where no launch path can skip
it. A server that has been outrun by its own source stops answering rather than
answering wrongly: with a supervisor it comes straight back on fresh code, and
without one it is at least honestly silent. A wrong number is worse than no
number, because only one of the two gets believed.
"""
from __future__ import annotations
import logging
from pathlib import Path
import time
from typing import Final
logger = logging.getLogger(__name__)
# Package tree whose .py mtimes decide whether this process is out of date.
_PACKAGE_ROOT: Final = Path(__file__).resolve().parent
# Re-checking on every request would stat the whole package per request. The
# window this leaves is bounded and small: at worst one stale answer in this
# many seconds, versus the unbounded staleness it replaces.
_RECHECK_INTERVAL_SECONDS: Final = 5.0
_last_checked: dict[str, float] = {}
_last_result: dict[str, Path | None] = {}
def reset_cache() -> None:
"""Drop the memoised answer, forcing the next call to re-stat."""
_last_checked.clear()
_last_result.clear()
def newest_py_after(cutoff: float) -> Path | None:
"""Return the newest package ``.py`` modified after *cutoff*.
Args:
cutoff: Epoch seconds to compare mtimes against.
Returns:
The newest offending path, or ``None`` when the tree is older than
*cutoff*. Returning the path rather than a bool keeps the refusal
auditable: the log names the file that made this process obsolete.
"""
newest: Path | None = None
newest_mtime = cutoff
for path in _PACKAGE_ROOT.rglob("*.py"):
try:
mtime = path.stat().st_mtime
except OSError:
# A file removed mid-scan cannot make us stale; skip it quietly.
continue
if mtime > newest_mtime:
newest, newest_mtime = path, mtime
return newest
def outdated_source(started_at: float, *, now: float | None = None) -> Path | None:
"""Return the source file that makes this process stale, if any.
Args:
started_at: Epoch seconds this process began. Any package file newer
than this was not the file this process loaded.
now: Override for the current monotonic time (for testing).
Returns:
The newest changed path, or ``None`` when the running code is current.
"""
moment = now if now is not None else time.monotonic()
checked = _last_checked.get("at")
if checked is not None and moment - checked < _RECHECK_INTERVAL_SECONDS:
return _last_result.get("path")
result = newest_py_after(started_at)
_last_checked["at"] = moment
_last_result["path"] = result
if result is not None:
logger.error(
"This server is running outdated code: %s changed after the "
"process started. Refusing to serve rather than report numbers "
"the daemon is not enforcing. Restart it with `./run.sh serve`.",
result,
)
return result

View File

@ -17,9 +17,12 @@ import json
import logging
import mimetypes
from pathlib import Path
import threading
import time
from urllib.parse import urlsplit
from steam_backlog_enforcer._budget_view import build_budget_snapshot
from steam_backlog_enforcer._serve_stale import outdated_source
from steam_backlog_enforcer._web_dataset import build_web_dataset, dataset_to_payload
from steam_backlog_enforcer.config import State
from steam_backlog_enforcer.game_install import _echo
@ -40,6 +43,20 @@ _EXTRA_TEXT_TYPES = frozenset(
)
_NOT_BUILT_MSG = b"Frontend not built. Run: cd web && npm install && npm run build"
# Epoch seconds this process began, captured at import. Anything in the
# package newer than this is code we did not load.
_STARTED_AT = time.time()
# Set once a stale server has begun standing down, so concurrent requests
# do not each spawn a shutdown thread.
_RETIRING = threading.Event()
_STALE_MSG = (
b"This server is running outdated code and has stopped answering rather "
b"than report numbers the enforcer is not applying. Restart it: "
b"./run.sh serve"
)
class _Handler(BaseHTTPRequestHandler):
"""Serve the dataset JSON and the static frontend bundle (read-only)."""
@ -50,6 +67,14 @@ class _Handler(BaseHTTPRequestHandler):
def do_GET(self) -> None:
"""Dispatch a GET to one of the APIs or to a static file."""
stale = outdated_source(_STARTED_AT)
if stale is not None:
# Fail closed on every route, static included: a page served from a
# fresh bundle that then fetches numbers from stale code is the
# same lie with extra steps.
self._send(HTTPStatus.SERVICE_UNAVAILABLE, _STALE_MSG, "text/plain")
self._retire()
return
split = urlsplit(self.path)
if split.path == _API_DATASET:
self._serve_dataset()
@ -61,6 +86,24 @@ class _Handler(BaseHTTPRequestHandler):
else:
self._serve_static(split.path)
def _retire(self) -> None:
"""Answer this request, then stop the server so it can be replaced.
Refusing to serve is only half a fix: a process that 503s forever is
honest and useless. Exiting hands the problem to the supervisor, and
``steam-backlog-enforcer-web.service`` has ``Restart=always``, so the
server comes back on current code with nobody having to remember to
restart it. Started outside systemd it simply exits, which is still
better than serving numbers the enforcer is not applying.
``shutdown`` blocks until ``serve_forever`` returns and cannot be
called from the thread handling a request, so it goes to its own.
"""
if _RETIRING.is_set():
return
_RETIRING.set()
threading.Thread(target=self.server.shutdown, daemon=True).start()
def _serve_budget(self, *, demo: bool) -> None:
"""Build and send the gaming-budget snapshot as JSON."""
try:

View File

@ -17,7 +17,7 @@ import socket
from typing import TYPE_CHECKING
from unittest.mock import patch
from steam_backlog_enforcer import _serve_guard
from steam_backlog_enforcer import _serve_guard, _serve_stale
from steam_backlog_enforcer.tests._fake_proc import add_pid, make_proc, patch_proc
if TYPE_CHECKING:
@ -48,7 +48,7 @@ class TestNewestPySince:
def test_no_newer_file_returns_none(self, tmp_path: Path) -> None:
(tmp_path / "a.py").write_text("x", encoding="utf-8")
os.utime(tmp_path / "a.py", (1000.0, 1000.0))
with patch.object(_serve_guard, "_PACKAGE_ROOT", tmp_path):
with patch.object(_serve_stale, "_PACKAGE_ROOT", tmp_path):
assert _serve_guard.newest_py_since(2000.0) is None
def test_newer_file_is_named(self, tmp_path: Path) -> None:
@ -58,19 +58,19 @@ class TestNewestPySince:
nested.mkdir()
(nested / "b.py").write_text("x", encoding="utf-8")
os.utime(nested / "b.py", (3000.0, 3000.0))
with patch.object(_serve_guard, "_PACKAGE_ROOT", tmp_path):
with patch.object(_serve_stale, "_PACKAGE_ROOT", tmp_path):
assert _serve_guard.newest_py_since(2000.0) == nested / "b.py"
def test_the_newest_of_several_wins(self, tmp_path: Path) -> None:
for name, mtime in (("a.py", 3000.0), ("b.py", 4000.0), ("c.py", 3500.0)):
(tmp_path / name).write_text("x", encoding="utf-8")
os.utime(tmp_path / name, (mtime, mtime))
with patch.object(_serve_guard, "_PACKAGE_ROOT", tmp_path):
with patch.object(_serve_stale, "_PACKAGE_ROOT", tmp_path):
assert _serve_guard.newest_py_since(2000.0) == tmp_path / "b.py"
def test_vanished_file_is_skipped(self, tmp_path: Path) -> None:
(tmp_path / "gone.py").symlink_to(tmp_path / "missing.py")
with patch.object(_serve_guard, "_PACKAGE_ROOT", tmp_path):
with patch.object(_serve_stale, "_PACKAGE_ROOT", tmp_path):
assert _serve_guard.newest_py_since(1000.0) is None

View File

@ -0,0 +1,137 @@
"""Tests for _serve_stale: a server must not outlive its own source.
On 2026-08-29 a hand-started ``python -c`` server stayed up for hours after the
code beneath it changed, reporting an 8h gaming budget while the daemon
enforced 6h. The launcher guard could not have helped: it only runs from
``cmd_serve``, which that process never went through. So the check lives in the
server, where no launch path can skip it.
"""
from __future__ import annotations
import logging
from typing import TYPE_CHECKING
from unittest.mock import patch
import pytest
from steam_backlog_enforcer._serve_stale import (
_RECHECK_INTERVAL_SECONDS,
outdated_source,
reset_cache,
)
if TYPE_CHECKING:
from collections.abc import Iterator
from pathlib import Path
_PKG = "steam_backlog_enforcer._serve_stale"
@pytest.fixture(autouse=True)
def _clear() -> Iterator[None]:
"""Keep the memo from leaking answers between tests.
Yields:
None, with the cache empty before and after.
"""
reset_cache()
yield
reset_cache()
@pytest.fixture
def fake_package(tmp_path: Path) -> Path:
"""Point the module at a throwaway package tree.
Args:
tmp_path: pytest temporary directory.
Returns:
The fake package root.
"""
root = tmp_path / "pkg"
root.mkdir()
(root / "module.py").write_text("x = 1\n", encoding="utf-8")
return root
def _touch(path: Path, mtime: float) -> None:
"""Set a file's mtime.
Args:
path: File to stamp.
mtime: Epoch seconds to set.
"""
path.write_text("x = 2\n", encoding="utf-8")
import os
os.utime(path, (mtime, mtime))
class TestOutdatedSource:
"""The check answers "has my own code moved on since I started"."""
def test_current_code_is_not_stale(self, fake_package: Path) -> None:
"""A tree older than the process start is fine."""
_touch(fake_package / "module.py", 1000.0)
with patch(f"{_PKG}._PACKAGE_ROOT", fake_package):
assert outdated_source(2000.0, now=0.0) is None
def test_a_newer_file_is_named(self, fake_package: Path) -> None:
"""The offending path is returned, so the refusal is auditable."""
changed = fake_package / "module.py"
_touch(changed, 3000.0)
with patch(f"{_PKG}._PACKAGE_ROOT", fake_package):
assert outdated_source(2000.0, now=0.0) == changed
def test_staleness_is_logged_at_error(
self,
fake_package: Path,
caplog: pytest.LogCaptureFixture,
) -> None:
"""Refusing to serve is loud: it needs to reach the journal."""
_touch(fake_package / "module.py", 3000.0)
with (
caplog.at_level(logging.ERROR),
patch(f"{_PKG}._PACKAGE_ROOT", fake_package),
):
outdated_source(2000.0, now=0.0)
assert "outdated code" in caplog.text
assert "run.sh serve" in caplog.text
def test_a_file_removed_mid_scan_is_skipped(self, fake_package: Path) -> None:
"""A vanished file cannot make the process stale."""
_touch(fake_package / "module.py", 3000.0)
with (
patch(f"{_PKG}._PACKAGE_ROOT", fake_package),
patch(
"steam_backlog_enforcer._serve_stale.Path.stat",
side_effect=OSError("gone"),
),
):
assert outdated_source(2000.0, now=0.0) is None
def test_the_answer_is_memoised(self, fake_package: Path) -> None:
"""Statting the whole package on every request would be wasteful."""
_touch(fake_package / "module.py", 1000.0)
with patch(f"{_PKG}._PACKAGE_ROOT", fake_package) as root:
outdated_source(2000.0, now=0.0)
# A change inside the memo window is deliberately not seen yet.
_touch(root / "module.py", 3000.0)
assert outdated_source(2000.0, now=1.0) is None
def test_the_memo_expires(self, fake_package: Path) -> None:
"""The staleness window is bounded, unlike the staleness it replaces."""
_touch(fake_package / "module.py", 1000.0)
with patch(f"{_PKG}._PACKAGE_ROOT", fake_package) as root:
outdated_source(2000.0, now=0.0)
_touch(root / "module.py", 3000.0)
later = _RECHECK_INTERVAL_SECONDS + 1
assert outdated_source(2000.0, now=later) is not None
def test_it_reads_the_clock_when_not_told_one(self, fake_package: Path) -> None:
"""The production path takes no `now`; exercise that branch too."""
_touch(fake_package / "module.py", 1000.0)
with patch(f"{_PKG}._PACKAGE_ROOT", fake_package):
assert outdated_source(2000.0) is None

View File

@ -0,0 +1,80 @@
"""Tests for the stale-code refusal in _web_server.
Split out of test_web_server.py for the 250-line cap; the server harness stays
in that module and is imported here.
"""
from __future__ import annotations
from typing import TYPE_CHECKING
from unittest.mock import patch
import pytest
from steam_backlog_enforcer import _web_server
from steam_backlog_enforcer.tests.test_web_server import _PKG, _get, _running
if TYPE_CHECKING:
from pathlib import Path
class TestStaleServerRefuses:
"""A server outrun by its own source stops answering and stands down."""
def _clear_retiring(self) -> None:
"""Reset the module-level stand-down latch between tests."""
_web_server._RETIRING.clear()
@pytest.mark.parametrize("route", ["/api/budget", "/api/dataset", "/"])
def test_every_route_refuses_when_stale(self, tmp_path: Path, route: str) -> None:
"""Static too: a fresh page fetching stale numbers is the same lie.
One server per route on purpose -- the first refusal stands the server
down, so a second request against it would (correctly) never answer.
"""
self._clear_retiring()
changed = tmp_path / "changed.py"
changed.touch()
with (
patch(f"{_PKG}.outdated_source", return_value=changed),
_running() as port,
):
status, body, _ = _get(port, route)
assert status == 503
assert b"outdated code" in body
self._clear_retiring()
def test_it_stands_down_so_a_supervisor_can_replace_it(
self,
tmp_path: Path,
) -> None:
"""503 forever would be honest and useless; exiting self-heals."""
self._clear_retiring()
changed = tmp_path / "changed.py"
changed.touch()
with (
patch(f"{_PKG}.outdated_source", return_value=changed),
_running() as port,
):
_get(port, "/api/budget")
assert _web_server._RETIRING.is_set()
self._clear_retiring()
def test_a_second_refusal_does_not_stand_down_twice(
self,
tmp_path: Path,
) -> None:
"""The latch short-circuits, so concurrent refusals share one shutdown."""
self._clear_retiring()
changed = tmp_path / "changed.py"
changed.touch()
_web_server._RETIRING.set()
with (
patch(f"{_PKG}.outdated_source", return_value=changed),
_running() as port,
):
# Latch already set: this refusal must take the early return and
# leave the server up, which is why the request still completes.
status, _, _ = _get(port, "/api/budget")
assert status == 503
self._clear_retiring()