Skip to content

Commit d261291

Browse files
committed
fix(stream): expire stream tokens and order init before the services it patches
Two faults found by reading the live container, not the source. The s6 oneshot that patches nginx and selkies had no ordering edge to anything that consumes its work. On the running host selkies and nginx started at 06:35:13 and init.sh finished writing at 06:35:21, so every patch landed on disk with no effect on the processes already up: the stream gate was absent from the live nginx (zero /verify hits, an unauthenticated GET / answering 200) and both selkies fixes were inert, which is why the gamepad INFO spam and the climbing EVDEV client count survived a patch that reported success. init-services now depends on init-pcsx2-config. The slow networked work moves to a new init-pcsx2-deps that only svc-broker waits for, so apt and the patches.zip download cannot delay the stream stack. Stream tokens now carry an idle TTL, refreshed on every admitted request, so an abandoned session stops holding the gate open. A re-issue demotes the previous token instead of dropping it, keeping it valid for a grace window: relaunching into an open tab meant the browser was still replaying the old stream_sid while RomM navigated to the new URL, and every one of those requests 403'd, which the client reports as a dropped connection and retries in a loop. A query token that outranks a stale cookie now re-cookies the browser so the tab moves across before the window closes. The gate's 403s now name their reason, which reaches stdout through the existing error choke point, and /status reports the token only while it is still live rather than handing back a dead one.
1 parent a25d0eb commit d261291

11 files changed

Lines changed: 249 additions & 77 deletions

File tree

README.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ Save states don't care about any of this. The requirement is only for memory-car
122122
| `PCSX2_LOG_PATH` | `/config/pcsx2-qt.log` | Captures pcsx2-qt stdout and stderr. Renderer and Vulkan failures show up here. Appended across launches. |
123123
| `BROKER_LOG_LEVEL` | `INFO` | `DEBUG`, `INFO`, `WARNING`, `ERROR`. The xdotool window-found messages are DEBUG. |
124124
| `PUID` / `PGID` | `1000` | Standard LinuxServer UID/GID. Also used to chown files the broker writes for PCSX2, which runs as `abc` and has to overwrite them later. |
125+
| `STREAM_TOKEN_TTL` | `43200.0` | Idle seconds before a stream token stops working. Every admitted request slides it forward, so it only fires on a session nobody is watching. Stops an abandoned session leaving the stream gate open forever. |
126+
| `STREAM_TOKEN_GRACE` | `120.0` | How long the previous token keeps working after a relaunch mints a new one, so an already-open tab is not cut off mid-navigation. |
125127

126128
## API
127129

@@ -226,6 +228,8 @@ It is refused while a game is running or a launch is in flight, because PCSX2 ho
226228

227229
**RomM shows no play button.** Confirm `streaming.enabled: true`, confirm the platform slug matches, restart RomM after config changes, and check what RomM thinks it has: `curl http://romm:5000/api/streaming/config`.
228230

231+
**"An error occurred, restarting stream", over and over.** The stream client says this whenever its WebSocket drops, so the useful signal is on the broker side: `docker logs pcsx2 | grep "/verify"`. A `403` there names the reason (`no stream token in the request`, `stream token expired`, `stream token superseded by a newer launch`), and the token is redacted so the line is safe to share. No `/verify` lines *at all* means the gate is not running: either the mod's init never patched nginx (`docker logs pcsx2 | grep broker-mod`), or your reverse proxy is reaching the ungated plain-HTTP vhost on port 3000 instead of the SSL vhost on 3001. Point it at 3001 and do not publish 3000: the gate is the only thing standing between the ROM library and anyone who finds the address.
232+
229233
## Development
230234

231235
The broker is one stdlib Python file with a stdlib `unittest` suite. Nothing to install, which is the point: the container has no pip, so the broker cannot grow a dependency without breaking the image.
@@ -238,6 +242,8 @@ CI runs the same tests under pytest, plus `ruff check .` against the shared [ruf
238242

239243
The suite covers what can be tested without a running emulator: save-file and memory-card archive handling (round trips, the last-write-wins mtime guard, path-traversal and subtree rejection, File-card refusal), `PCSX2.ini` patching, ROM path resolution, and the session state machine (crash-loop limiter, launch and card-op claims, deferred `load_slot` generation checks). Anything needing a real X display, PINE socket, or pcsx2-qt process is out of scope on purpose, because it is only provable on a live container.
240244

245+
**Init ordering matters more than it looks.** The mod ships two s6 oneshots. `init-pcsx2-config` rewrites files that base-image services read exactly once at startup (selkies' `input_handler.py`, the nginx site config, labwc's autostart), so `init-services` is made to depend on it and the whole service stack waits. Drop that edge and the patches still land on disk while changing nothing about the processes already running, which fails silently: the log says "Applied nginx stream gate" and the live nginx has no gate. Keep that script fast and offline. `init-pcsx2-deps` holds the slow networked work (apt, `patches.zip`) and only `svc-broker` waits for it, so a GitHub timeout cannot stall the stream.
246+
241247
Commits follow [Conventional Commits](https://www.conventionalcommits.org/) and releases are cut automatically on merge to `main`: `fix:` bumps the patch, `feat:` the minor, `feat!:` the major.
242248

243249
## Pinning a version

root/etc/s6-overlay/s6-rc.d/init-pcsx2-config/init.sh

Lines changed: 11 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
11
#!/usr/bin/with-contenv bash
22

3+
# Every patch below rewrites a file that a base-image service reads once, at
4+
# import or config-load time: selkies' input_handler.py, nginx's site config,
5+
# labwc's autostart. So init-services depends on this oneshot (see
6+
# init-services/dependencies.d/init-pcsx2-config), which puts the whole service
7+
# stack behind it. Without that edge s6 starts the services in parallel with
8+
# this script, they win the race by several seconds, and every patch here lands
9+
# on disk having no effect at all on the processes already running: the stream
10+
# gate is absent from the live nginx and the selkies fixes never load.
11+
# Keep this script fast and offline. Slow or networked work belongs in
12+
# init-pcsx2-deps, which only the broker waits for.
13+
314
# Clean up stale Wayland and X11 sockets to ensure the compositor starts on the
415
# default display indices (wayland-0, :0) even if the container was killed
516
# forcefully. Persistence of these lock files on the host-mapped /config
@@ -17,47 +28,6 @@ else
1728
echo "[broker-mod] Cleaned up stale display sockets."
1829
fi
1930

20-
# Ensure python3 is available for the broker service, and libshaderc for
21-
# PCSX2's Vulkan renderer. The base image ships without it, so on a host
22-
# with a GPU the GS device fails shader compilation and pcsx2-qt exits
23-
# within a second of booting any game (the gameless dashboard never
24-
# initializes GS, masking the breakage until a game is launched).
25-
_pkgs=()
26-
command -v python3 &>/dev/null || _pkgs+=(python3)
27-
ldconfig -p | grep -q libshaderc.so.1 || _pkgs+=(libshaderc1)
28-
if [ ${#_pkgs[@]} -gt 0 ]; then
29-
echo "[broker-mod] Installing: ${_pkgs[*]}"
30-
apt-get update -qq && apt-get install -y -qq "${_pkgs[@]}" \
31-
|| echo "[broker-mod] ERROR: failed to install ${_pkgs[*]}"
32-
fi
33-
34-
# Ubuntu's +dfsg PCSX2 package strips patches.zip from the resources dir,
35-
# so every game boot warns "Built-in game patches are not available" and
36-
# titles that rely on compatibility patches misbehave. Fetch the official
37-
# archive from the PCSX2 project. Non-fatal if offline: games still run.
38-
PATCHES_ZIP="/usr/share/PCSX2/resources/patches.zip"
39-
if [ ! -s "$PATCHES_ZIP" ]; then
40-
echo "[broker-mod] Downloading PCSX2 patches.zip..."
41-
# Download to a temp path and verify it is a well-formed zip before
42-
# installing: the release URL floats ("latest"), so a checksum can't be
43-
# pinned, but a truncated or non-zip response must never land in place.
44-
# Bounded so an unresponsive GitHub can't stall container init for an
45-
# optional file.
46-
if curl -fsSL --connect-timeout 10 --max-time 60 -o "$PATCHES_ZIP.tmp" \
47-
"https://github.com/PCSX2/pcsx2_patches/releases/latest/download/patches.zip" \
48-
&& python3 -c 'import sys, zipfile
49-
with zipfile.ZipFile(sys.argv[1]) as z:
50-
sys.exit(1 if z.testzip() is not None or not z.namelist() else 0)' \
51-
"$PATCHES_ZIP.tmp" 2>/dev/null; then
52-
chmod 644 "$PATCHES_ZIP.tmp"
53-
mv "$PATCHES_ZIP.tmp" "$PATCHES_ZIP"
54-
echo "[broker-mod] patches.zip installed."
55-
else
56-
rm -f "$PATCHES_ZIP.tmp"
57-
echo "[broker-mod] WARNING: patches.zip download failed or corrupt; built-in game patches unavailable."
58-
fi
59-
fi
60-
6131
# Lock down the sudoers rule so sudo accepts it (requires mode 0440).
6232
chmod 0440 /etc/sudoers.d/broker
6333
echo "[broker-mod] sudoers rule set."

root/etc/s6-overlay/s6-rc.d/init-pcsx2-deps/dependencies.d/init-config-end

Whitespace-only changes.
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#!/usr/bin/with-contenv bash
2+
3+
# Package and asset fetching, split out of init-pcsx2-config because it is slow
4+
# and networked. init-pcsx2-config gates the whole service stack (nginx, xorg,
5+
# selkies all wait on it), so anything that can spend a minute on apt-get or a
6+
# GitHub download has to live somewhere that only the broker waits for.
7+
8+
# Ensure python3 is available for the broker service, and libshaderc for
9+
# PCSX2's Vulkan renderer. The base image ships without it, so on a host
10+
# with a GPU the GS device fails shader compilation and pcsx2-qt exits
11+
# within a second of booting any game (the gameless dashboard never
12+
# initializes GS, masking the breakage until a game is launched).
13+
_pkgs=()
14+
command -v python3 &>/dev/null || _pkgs+=(python3)
15+
ldconfig -p | grep -q libshaderc.so.1 || _pkgs+=(libshaderc1)
16+
if [ ${#_pkgs[@]} -gt 0 ]; then
17+
echo "[broker-mod] Installing: ${_pkgs[*]}"
18+
apt-get update -qq && apt-get install -y -qq "${_pkgs[@]}" \
19+
|| echo "[broker-mod] ERROR: failed to install ${_pkgs[*]}"
20+
fi
21+
22+
# Ubuntu's +dfsg PCSX2 package strips patches.zip from the resources dir,
23+
# so every game boot warns "Built-in game patches are not available" and
24+
# titles that rely on compatibility patches misbehave. Fetch the official
25+
# archive from the PCSX2 project. Non-fatal if offline: games still run.
26+
PATCHES_ZIP="/usr/share/PCSX2/resources/patches.zip"
27+
if [ ! -s "$PATCHES_ZIP" ]; then
28+
echo "[broker-mod] Downloading PCSX2 patches.zip..."
29+
# Download to a temp path and verify it is a well-formed zip before
30+
# installing: the release URL floats ("latest"), so a checksum can't be
31+
# pinned, but a truncated or non-zip response must never land in place.
32+
# Bounded so an unresponsive GitHub can't stall container init for an
33+
# optional file.
34+
if curl -fsSL --connect-timeout 10 --max-time 60 -o "$PATCHES_ZIP.tmp" \
35+
"https://github.com/PCSX2/pcsx2_patches/releases/latest/download/patches.zip" \
36+
&& python3 -c 'import sys, zipfile
37+
with zipfile.ZipFile(sys.argv[1]) as z:
38+
sys.exit(1 if z.testzip() is not None or not z.namelist() else 0)' \
39+
"$PATCHES_ZIP.tmp" 2>/dev/null; then
40+
chmod 644 "$PATCHES_ZIP.tmp"
41+
mv "$PATCHES_ZIP.tmp" "$PATCHES_ZIP"
42+
echo "[broker-mod] patches.zip installed."
43+
else
44+
rm -f "$PATCHES_ZIP.tmp"
45+
echo "[broker-mod] WARNING: patches.zip download failed or corrupt; built-in game patches unavailable."
46+
fi
47+
fi
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
oneshot
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
/etc/s6-overlay/s6-rc.d/init-pcsx2-deps/init.sh

root/etc/s6-overlay/s6-rc.d/init-services/dependencies.d/init-pcsx2-config

Whitespace-only changes.

root/etc/s6-overlay/s6-rc.d/svc-broker/dependencies.d/init-pcsx2-deps

Whitespace-only changes.

root/etc/s6-overlay/s6-rc.d/user/contents.d/init-pcsx2-deps

Whitespace-only changes.

root/root/broker.py

Lines changed: 88 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,19 @@ def _gpu_env() -> dict[str, str]:
167167
RESUME_LOAD_WAIT = float(os.environ.get("RESUME_LOAD_WAIT", "90.0"))
168168
RESUME_LOAD_SETTLE = float(os.environ.get("RESUME_LOAD_SETTLE", "3.0"))
169169

170+
# Stream token lifetime. The TTL is idle time, not absolute: every admitted
171+
# request slides it forward, so it only fires on a session nobody is watching.
172+
# Without it a token minted by /launch stays valid until an explicit release,
173+
# and a container that loses its RomM side (crash, network partition, a user
174+
# who just closes the tab) leaves the gate open indefinitely.
175+
STREAM_TOKEN_TTL = float(os.environ.get("STREAM_TOKEN_TTL", "43200.0"))
176+
# How long the superseded token keeps working after a re-issue. Relaunching
177+
# into an already-open tab means the browser is still replaying the old
178+
# stream_sid cookie while RomM navigates the iframe to the new URL; without
179+
# this window every one of those in-flight requests 403s and the stream client
180+
# reports a dropped connection and retries in a loop.
181+
STREAM_TOKEN_GRACE = float(os.environ.get("STREAM_TOKEN_GRACE", "120.0"))
182+
170183
logging.basicConfig(
171184
level=getattr(logging, os.environ.get("BROKER_LOG_LEVEL", "INFO").upper(), logging.INFO),
172185
format="%(asctime)s [broker] %(levelname)s %(message)s",
@@ -231,35 +244,86 @@ def _gpu_env() -> dict[str, str]:
231244
# survives game exit so RomM can still pull after the session ends.
232245
"save_baseline": None,
233246
# Random per-session token gating the stream proxy on port 3001. Minted on
234-
# /launch, swapped for a cookie by the browser, cleared on release.
235-
"stream_token": None,
247+
# /launch, swapped for a cookie by the browser, cleared on release. The
248+
# expiry is a monotonic deadline refreshed on every admitted request; the
249+
# prev_* pair holds the token a re-issue replaced, valid for a short grace
250+
# window so an already-open tab is not cut off mid-relaunch.
251+
"stream_token": None,
252+
"stream_expires": 0.0,
253+
"stream_prev_token": None,
254+
"stream_prev_expires": 0.0,
236255
}
237256

238257
# ── Helpers ───────────────────────────────────────────────────────────────────
239258

240259
def _issue_stream_token() -> str:
241-
"""Mint a fresh stream token and bind it to the current session."""
260+
"""Mint a fresh stream token and bind it to the current session.
261+
262+
The token being replaced is demoted rather than dropped: it stays usable
263+
for STREAM_TOKEN_GRACE seconds so requests already in flight from an open
264+
tab still land. See STREAM_TOKEN_GRACE for why that matters.
265+
"""
242266
token = secrets.token_urlsafe(32)
267+
now = time.monotonic()
243268
with _session_lock:
269+
previous = _session["stream_token"]
270+
if previous:
271+
_session["stream_prev_token"] = previous
272+
_session["stream_prev_expires"] = now + STREAM_TOKEN_GRACE
244273
_session["stream_token"] = token
274+
_session["stream_expires"] = now + STREAM_TOKEN_TTL
245275
return token
246276

247277

248-
def _check_stream_token(token: str) -> bool:
249-
"""Constant-time check of token against the live session token."""
278+
def _check_stream_token(token: str) -> str | None:
279+
"""Judge token against the live session token, then the superseded one.
280+
281+
Returns None when the token is good, otherwise a short reason for the log.
282+
A hit on the live token slides its expiry forward: the TTL exists to close
283+
an abandoned session, not to interrupt someone who is still playing.
284+
"""
250285
if not token:
251-
return False
286+
return "no stream token in the request"
287+
now = time.monotonic()
252288
with _session_lock:
253289
current = _session["stream_token"]
254-
if not current:
255-
return False
256-
return hmac.compare_digest(token, current)
290+
if not current:
291+
return "no stream session is open"
292+
if hmac.compare_digest(token, current):
293+
if now >= _session["stream_expires"]:
294+
return "stream token expired after %.0fs idle" % STREAM_TOKEN_TTL
295+
_session["stream_expires"] = now + STREAM_TOKEN_TTL
296+
return None
297+
previous = _session["stream_prev_token"]
298+
if previous and hmac.compare_digest(token, previous):
299+
if now < _session["stream_prev_expires"]:
300+
return None
301+
_session["stream_prev_token"] = None
302+
_session["stream_prev_expires"] = 0.0
303+
return "stream token superseded by a newer launch"
304+
return "stream token does not match the open session"
257305

258306

259307
def _clear_stream_token() -> None:
260308
"""Drop the stream token so the gate rejects everything until next launch."""
261309
with _session_lock:
262310
_session["stream_token"] = None
311+
_session["stream_expires"] = 0.0
312+
_session["stream_prev_token"] = None
313+
_session["stream_prev_expires"] = 0.0
314+
315+
316+
def _live_stream_token() -> str | None:
317+
"""The session token if it is still inside its TTL, else None.
318+
319+
/status hands this to RomM so a reconnecting client can re-attach without
320+
a relaunch. An expired token would only send it into the 403 loop the TTL
321+
is there to end, so it is reported as absent.
322+
"""
323+
with _session_lock:
324+
if _session["stream_token"] and time.monotonic() < _session["stream_expires"]:
325+
return _session["stream_token"]
326+
return None
263327

264328

265329
def _extract_stream_token(query: str, cookie_header: str | None) -> str | None:
@@ -287,22 +351,28 @@ def _stream_cookie_value(token: str) -> str:
287351

288352
def _verify_stream_decision(
289353
original_uri: str, cookie_header: str | None
290-
) -> tuple[int, str | None]:
354+
) -> tuple[int, str | None, str | None]:
291355
"""Decide an nginx auth_request subrequest for the stream gate.
292356
293-
Returns (status, set_cookie). 200 admits the request, 403 rejects it.
357+
Returns (status, set_cookie, reason). 200 admits the request, 403 rejects
358+
it and carries the reason so the refusal is legible in the container log.
294359
When the token arrives in the query (the first iframe load), the caller
295360
gets a Set-Cookie so later requests carry stream_sid and the token drops
296361
out of the URL. A cookie-authed request that is already good gets no
297362
Set-Cookie back, so nginx does not rewrite it.
298363
"""
299364
query = urlparse(original_uri).query
300365
token = _extract_stream_token(query, cookie_header)
301-
if not _check_stream_token(token or ""):
302-
return 403, None
366+
reason = _check_stream_token(token or "")
367+
if reason:
368+
return 403, None, reason
303369
if "stream_token" in parse_qs(query):
304-
return 200, _stream_cookie_value(token)
305-
return 200, None
370+
# Re-cookie on the query bootstrap, and also when the query token is
371+
# the current one but the cookie still holds the superseded token:
372+
# the browser must be moved onto the new value before the grace
373+
# window closes, or the tab drops out the moment it does.
374+
return 200, _stream_cookie_value(token), None
375+
return 200, None, None
306376

307377

308378
def _validate_rom_path(raw: str) -> Path | None:
@@ -1760,14 +1830,14 @@ def _verify_stream(self) -> None:
17601830
# real request URI (carrying the stream_token query on first load) via
17611831
# X-Original-URI and the browser Cookie header; the broker returns 200
17621832
# to admit or 403 to reject, and a Set-Cookie on the query bootstrap.
1763-
status, set_cookie = _verify_stream_decision(
1833+
status, set_cookie, reason = _verify_stream_decision(
17641834
self.headers.get("X-Original-URI", ""),
17651835
self.headers.get("Cookie"),
17661836
)
17671837
headers = {"Set-Cookie": set_cookie} if set_cookie else None
17681838
body = {"ok": status == 200}
17691839
if status != 200:
1770-
body["error"] = "stream token missing, expired, or already claimed"
1840+
body["error"] = reason or "stream request rejected"
17711841
self._send_json(status, body, headers)
17721842

17731843
def _log_error_response(self, code: int, body: dict) -> None:
@@ -2156,7 +2226,7 @@ def _handle_GET(self):
21562226
# and nothing will restart it without an explicit POST /launch.
21572227
# Distinguishes a dead container from an idle dashboard.
21582228
"relaunch_abandoned": abandoned,
2159-
"stream_token": snap.get("stream_token") if active else None,
2229+
"stream_token": _live_stream_token() if active else None,
21602230
})
21612231
else:
21622232
self._send_json(404, {"error": "not found"})

0 commit comments

Comments
 (0)