From 5b3fce1096a637f73cd2328e49feebefdded159c Mon Sep 17 00:00:00 2001 From: Thales <> Date: Wed, 26 Aug 2026 09:49:01 +0100 Subject: [PATCH 1/2] The shell recognises its own backend by token, not by PID (#457) Every Windows portable launch since 0.14.0 has failed with "Another program is already using port 8000", naming StemDeck's own healthy backend as the intruder, after a full ninety second wait. #424 taught the shell to check *which* process answers /api/health, so a second StemDeck could no longer adopt the first one's backend and, with it, the first one's library. It established that identity by comparing the PID in the health payload against `child.id()`, which assumes the process that binds the port is the process we spawned. On the Windows portable build it is not. `python/Scripts/python.exe` is a venv launcher pointing at `python/base/python.exe`, and Windows has no exec, so the launcher starts the real interpreter as a child of its own. The process that binds the port is a grandchild, and its PID can never equal `child.id()`. Measured on a local CPU-only package: StemDeck.exe 6868 python.exe 6788 python\Scripts\python.exe <- child.id() python.exe 11636 python\base\python.exe <- binds :8000 So the comparison could not succeed, the poll loop ran to its deadline, and the foreign PID it had been recording all along became the error message. Linux and macOS were unaffected: python-build-standalone puts a real binary at python/bin/python with no launcher in front of it, which is why this only ever showed up on Windows. Identity now travels in the environment, which survives any number of re-execs: the shell generates a per-launch token, passes it as STEMDECK_INSTANCE_TOKEN, and the backend echoes it from /api/health. A backend that reports no token predates this shell and falls back to the PID comparison it was built for, so a half-updated install still starts. What #424 protects against is unchanged, and slightly stronger. A second StemDeck generates its own token, so the first instance's backend is refused on a mismatch rather than on a PID coincidence. The token only has to be unique per launch, not unguessable. It answers "is this the process I just started", and anything on loopback that wanted to lie could read the token out of the health response anyway. Deriving it keeps an RNG dependency out of a crate that has no other use for one. Verified by building the CPU-only portable package and running it: the app reached its window, and the health payload carried the token from a PID that was never the child. --- app/main.py | 23 ++-- desktop/src-tauri/src/main.rs | 210 ++++++++++++++++++++++++++++------ tests/test_health_api.py | 28 ++++- 3 files changed, 214 insertions(+), 47 deletions(-) diff --git a/app/main.py b/app/main.py index 78b76115..14bce40f 100644 --- a/app/main.py +++ b/app/main.py @@ -293,15 +293,24 @@ def health() -> dict[str, object]: "ffmpeg_configured": FFMPEG_BIN.is_file(), "demucs_model": DEMUCS_MODEL, "demucs_device": get_demucs_device(), - # Which process is answering. The desktop shell spawns this backend and - # then polls this endpoint to know it came up -- but a 200 alone only - # proves *something* is listening on that port, not that it is the child - # the shell just started. When a second StemDeck was launched, the new + # Who is answering. The desktop shell spawns this backend and then polls + # this endpoint to know it came up -- but a 200 alone only proves + # *something* is listening on that port, not that it is the backend the + # shell just started. When a second StemDeck was launched, the new # window adopted the already-running instance's backend, and with it - # that instance's data directory and library (#424). The shell compares - # this against the PID it spawned, so a stranger on the port is refused - # rather than silently trusted. + # that instance's data directory and library (#424). + # + # The token is the answer to that, and the PID is now only diagnostic + # (it is what the shell names in its port-conflict message). #424 used + # the PID for identity, which assumed the process that binds the port is + # the one the shell spawned. On the Windows portable build it is not: + # python/Scripts/python.exe is a venv launcher and Windows has no exec, + # so it starts python/base/python.exe as a child and *that* is the + # process here. The comparison could never succeed and every Windows + # portable launch timed out (#457). The environment survives any number + # of re-execs, so identity travels there instead. "pid": os.getpid(), + "instance": os.environ.get("STEMDECK_INSTANCE_TOKEN", ""), } diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index b09d79ee..852f5483 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -9,7 +9,10 @@ use std::{ net::{SocketAddr, TcpStream}, path::{Path, PathBuf}, process::{Child, Command, Output, Stdio}, - sync::Mutex, + sync::{ + atomic::{AtomicU64, Ordering}, + Mutex, + }, thread, time::{Duration, Instant, SystemTime, UNIX_EPOCH}, }; @@ -1507,6 +1510,7 @@ fn start_backend( .and_then(|bin_dir| bin_dir.parent().map(|venv| (venv, bin_dir))) .and_then(|(venv, bin_dir)| bundled_python_home(venv, bin_dir).map(|(home, _)| home)); + let instance_token = new_instance_token(); let mut cmd = Command::new(python); cmd.args([ "-m", @@ -1548,6 +1552,11 @@ fn start_backend( .map(|dir| ("STEMDECK_SETTINGS_MIRROR", dir.join("settings.json"))), ) .env("STEMDECK_PARENT_PID", std::process::id().to_string()) + // How the backend proves it is ours when it answers /api/health. + // The environment is the only channel that survives the Windows + // venv launcher re-execing into python/base (#457), which is why + // this exists rather than a PID comparison. See wait_for_health. + .env("STEMDECK_INSTANCE_TOKEN", &instance_token) .env("PYTHONUNBUFFERED", "1") .env("XDG_CACHE_HOME", data_dir.join("cache")) .env("TORCH_HOME", data_dir.join("models").join("torch")) @@ -1575,7 +1584,13 @@ fn start_backend( // Release the reserved port immediately after spawn so uvicorn can bind it. drop(port_guard); - if let Err(err) = wait_for_health(&mut child, port, Duration::from_secs(90), &log_path) { + if let Err(err) = wait_for_health( + &mut child, + port, + &instance_token, + Duration::from_secs(90), + &log_path, + ) { let _ = child.kill(); let _ = child.wait(); return Err(err); @@ -3399,14 +3414,58 @@ fn reserve_port(host: &str, desired: u16) -> Result<(u16, Socket), String> { free_port(host) } +/// A fresh identity for the backend this launch is about to spawn, handed to +/// it as `STEMDECK_INSTANCE_TOKEN` and echoed back by `/api/health`. +/// +/// It has to be unique per launch, not unguessable: it answers "is the process +/// on this port the one I just started", and anything on the loopback +/// interface that wanted to lie could already read the token out of the health +/// response. So it is derived from the clock, this process and a counter +/// rather than drawn from a CSPRNG, which keeps the shell free of an RNG +/// dependency it has no other use for. +fn new_instance_token() -> String { + static SEQUENCE: AtomicU64 = AtomicU64::new(0); + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0); + let mut hasher = Sha256::new(); + hasher.update(std::process::id().to_le_bytes()); + hasher.update(nanos.to_le_bytes()); + hasher.update(SEQUENCE.fetch_add(1, Ordering::Relaxed).to_le_bytes()); + format!("{:x}", hasher.finalize())[..32].to_string() +} + +/// Who is answering `/api/health`. Both fields are optional because either can +/// be absent from a backend older than the shell asking. +#[derive(Debug, Default, PartialEq)] +struct HealthIdentity { + pid: Option, + instance: Option, +} + /// Wait until *our own* backend answers on `port`. /// /// Identity matters as much as liveness here. A 200 only proves something is /// listening; before #424 that was enough, so a second StemDeck launched while /// one was already running would adopt the first instance's backend, and with /// it the first instance's data directory and library, with nothing on screen -/// to suggest anything was wrong. The health payload carries the answering -/// process's PID, and only the child we just spawned is accepted. +/// to suggest anything was wrong. +/// +/// #424 established that identity by comparing the PID in the health payload +/// against the child we spawned, which assumed the process that binds the port +/// is the process we started. On the Windows portable build it is not (#457). +/// There `python/Scripts/python.exe` is a venv launcher pointing at +/// `python/base/python.exe`, and Windows has no `exec`, so the launcher starts +/// the real interpreter as a *child of its own*. The PID that binds the port is +/// therefore a grandchild and can never equal `child.id()`. Every Windows +/// portable user got the full ninety second timeout followed by "Another +/// program is already using port 8000", naming StemDeck's own healthy backend +/// as the intruder. +/// +/// So identity travels in the environment instead, where it survives any number +/// of re-execs: the backend echoes back the token we gave it. A backend that +/// predates the token falls back to the PID comparison it was built for. /// /// Watching the child also turns the common failure into a fast, clear one: a /// backend that cannot bind its port exits within a second or so, and there is @@ -3414,6 +3473,7 @@ fn reserve_port(host: &str, desired: u16) -> Result<(u16, Socket), String> { fn wait_for_health( child: &mut Child, port: u16, + token: &str, timeout: Duration, log_path: &Path, ) -> Result<(), String> { @@ -3441,12 +3501,19 @@ fn wait_for_health( )); } match health_once(port) { - Ok(pid) if pid == expected_pid => return Ok(()), + // Our token came back: this is the backend we started, whatever + // process ended up holding the socket. + Ok(id) if id.instance.as_deref() == Some(token) => return Ok(()), + // No token at all means a backend older than this shell, which can + // only be identified the #424 way. Anything that does report a + // token reports *a different one*, so it is not ours and the PID is + // not consulted. + Ok(id) if id.instance.is_none() && id.pid == Some(expected_pid) => return Ok(()), // Something is listening, but it is not the process we started. // Keep waiting rather than failing outright: our child is still // alive, and if it never gets the port it will exit and be caught // above. What must never happen is returning Ok for this. - Ok(pid) => foreign_pid = Some(pid), + Ok(id) => foreign_pid = id.pid, Err(_) => {} } thread::sleep(interval); @@ -3494,9 +3561,9 @@ fn file_tail(path: &Path, max_lines: usize) -> String { .unwrap_or_default() } -/// Returns the PID the backend reports for itself, so the caller can tell our -/// own child apart from any other process that happens to hold the port. -fn health_once(port: u16) -> Result { +/// Returns what the process on `port` claims about itself, so the caller can +/// tell our own backend apart from anything else holding the port. +fn health_once(port: u16) -> Result { let mut stream = TcpStream::connect(("127.0.0.1", port)).map_err(|e| e.to_string())?; stream .set_read_timeout(Some(Duration::from_secs(2))) @@ -3511,22 +3578,35 @@ fn health_once(port: u16) -> Result { if !(response.starts_with("HTTP/1.1 200") || response.starts_with("HTTP/1.0 200")) { return Err("health endpoint did not return 200".to_string()); } - parse_health_pid(&response).ok_or_else(|| "health response carried no pid".to_string()) + parse_health_identity(&response) + .ok_or_else(|| "health response was not a JSON object".to_string()) } -/// Pull `"pid"` out of a raw HTTP response. Deliberately parses only the JSON -/// body: the headers are not JSON, and a `pid` appearing there (or in a header -/// value) must not be mistaken for the backend's own. -fn parse_health_pid(response: &str) -> Option { +/// Pull the identity fields out of a raw HTTP response. Deliberately parses +/// only the JSON body: the headers are not JSON, and a `pid` appearing there +/// (or in a header value) must not be mistaken for the backend's own. +/// +/// An empty `instance` is the same as none. Every distribution but the desktop +/// shell runs the backend without a token (Docker, Unraid, a source checkout), +/// and reporting `""` for all of them must not let them match each other. +fn parse_health_identity(response: &str) -> Option { let body = response .split_once("\r\n\r\n") .map(|(_, body)| body) .or_else(|| response.split_once("\n\n").map(|(_, body)| body))?; let start = body.find('{')?; let json: serde_json::Value = serde_json::from_str(body[start..].trim()).ok()?; - json.get("pid")? - .as_u64() - .and_then(|p| u32::try_from(p).ok()) + Some(HealthIdentity { + pid: json + .get("pid") + .and_then(|v| v.as_u64()) + .and_then(|p| u32::try_from(p).ok()), + instance: json + .get("instance") + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .map(str::to_string), + }) } fn ensure_ffmpeg(data_dir: &Path) -> Result { @@ -5175,41 +5255,67 @@ b6052160df96b31c9b1e33854a4dcda3d4b57641b880270f31736fb9f445d384 ffmpeg-n7.1-la } #[test] - fn health_pid_comes_from_the_body_only() { + fn health_identity_comes_from_the_body_only() { let ok = "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n\r\n\ - {\"name\":\"StemDeck\",\"status\":\"ok\",\"pid\":4242}"; - assert_eq!(super::parse_health_pid(ok), Some(4242)); + {\"name\":\"StemDeck\",\"status\":\"ok\",\"pid\":4242,\"instance\":\"abc\"}"; + assert_eq!( + super::parse_health_identity(ok), + Some(super::HealthIdentity { + pid: Some(4242), + instance: Some("abc".to_string()), + }) + ); // A header must never be mistaken for the payload, or a stranger could - // claim to be our child just by setting one. - let header_only = "HTTP/1.1 200 OK\r\nX-Pid: 4242\r\n\r\n{\"status\":\"ok\"}"; - assert_eq!(super::parse_health_pid(header_only), None); + // claim to be our backend just by setting one. + let header_only = + "HTTP/1.1 200 OK\r\nX-Pid: 4242\r\nX-Instance: abc\r\n\r\n{\"status\":\"ok\"}"; + assert_eq!( + super::parse_health_identity(header_only), + Some(super::HealthIdentity::default()) + ); - // An older backend that does not report a pid cannot be verified, so it - // must not be accepted as ours. - let no_pid = "HTTP/1.1 200 OK\r\n\r\n{\"name\":\"StemDeck\",\"status\":\"ok\"}"; - assert_eq!(super::parse_health_pid(no_pid), None); + // Every non-desktop distribution runs without a token and reports "". + // Read as a token, they would all match one another. + let untokened = "HTTP/1.1 200 OK\r\n\r\n{\"pid\":7,\"instance\":\"\"}"; + assert_eq!( + super::parse_health_identity(untokened), + Some(super::HealthIdentity { + pid: Some(7), + instance: None, + }) + ); assert_eq!( - super::parse_health_pid("HTTP/1.1 200 OK\r\n\r\nnot json"), + super::parse_health_identity("HTTP/1.1 200 OK\r\n\r\nnot json"), None ); - assert_eq!(super::parse_health_pid(""), None); + assert_eq!(super::parse_health_identity(""), None); } - /// Stands in for the *other* StemDeck: something already listening on the - /// port, answering /api/health with a 200 that is not ours. - fn other_instance_on_a_port(pid: u32) -> u16 { + #[test] + fn instance_tokens_differ_between_launches() { + assert_ne!(super::new_instance_token(), super::new_instance_token()); + assert_eq!(super::new_instance_token().len(), 32); + } + + /// A stand-in backend on a port, answering /api/health with the pid and + /// token it is told to claim. + fn responder_on_a_port(pid: u32, instance: &str) -> u16 { use std::io::{Read, Write}; let listener = std::net::TcpListener::bind(("127.0.0.1", 0)).unwrap(); let port = listener.local_addr().unwrap().port(); + let instance = instance.to_string(); std::thread::spawn(move || { for stream in listener.incoming().take(16) { let Ok(mut stream) = stream else { continue }; let mut buf = [0u8; 512]; let _ = stream.read(&mut buf); - let body = format!("{{\"name\":\"StemDeck\",\"status\":\"ok\",\"pid\":{pid}}}"); + let body = format!( + "{{\"name\":\"StemDeck\",\"status\":\"ok\",\"pid\":{pid},\ + \"instance\":\"{instance}\"}}" + ); let _ = stream.write_all( format!( "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n\ @@ -5251,12 +5357,13 @@ b6052160df96b31c9b1e33854a4dcda3d4b57641b880270f31736fb9f445d384 ffmpeg-n7.1-la // port while the backend we spawned dies. Before the fix this returned // Ok, the shell pointed the window at that backend, and the second // install quietly drove the first install's library. - let port = other_instance_on_a_port(999_999); + let port = responder_on_a_port(999_999, "a-different-launch"); let mut child = briefly_alive_child(); let result = super::wait_for_health( &mut child, port, + "our-token", Duration::from_secs(20), Path::new("does-not-exist.log"), ); @@ -5271,15 +5378,42 @@ b6052160df96b31c9b1e33854a4dcda3d4b57641b880270f31736fb9f445d384 ffmpeg-n7.1-la } #[test] - fn our_own_backend_is_accepted() { - // The other half: verification must not be so strict that a healthy - // start is rejected. A responder reporting our child's pid is ours. + fn our_own_backend_is_accepted_from_a_process_we_did_not_spawn_directly() { + // #457. On the Windows portable build the process that binds the port + // is a *grandchild*: python/Scripts/python.exe is a venv launcher and + // Windows has no exec, so it starts python/base/python.exe beneath + // itself. A pid that will never equal child.id() is the normal case, + // not a stranger, and requiring equality timed out every launch. + let mut child = briefly_alive_child(); + let grandchild_pid = child.id().wrapping_add(4); + let port = responder_on_a_port(grandchild_pid, "our-token"); + + let result = super::wait_for_health( + &mut child, + port, + "our-token", + Duration::from_secs(20), + Path::new("does-not-exist.log"), + ); + let _ = child.kill(); + let _ = child.wait(); + + assert!(result.is_ok(), "rejected our own backend: {result:?}"); + } + + #[test] + fn a_backend_older_than_the_token_still_starts() { + // Verification must not be so strict that a healthy start is rejected. + // A backend that reports no token at all predates this shell and can + // only be identified the #424 way, so the pid comparison still stands + // for it. let mut child = briefly_alive_child(); - let port = other_instance_on_a_port(child.id()); + let port = responder_on_a_port(child.id(), ""); let result = super::wait_for_health( &mut child, port, + "our-token", Duration::from_secs(20), Path::new("does-not-exist.log"), ); diff --git a/tests/test_health_api.py b/tests/test_health_api.py index 10e74847..52799495 100644 --- a/tests/test_health_api.py +++ b/tests/test_health_api.py @@ -23,8 +23,8 @@ def test_health_identifies_the_answering_process(): # The desktop shell spawns this backend and polls /api/health to know it # started. A 200 alone only proves *something* holds the port: a second # StemDeck used to adopt the first instance's backend, and with it the first - # instance's data directory and library (#424). The shell compares this pid - # against the child it spawned, so it must be the real one. + # instance's data directory and library (#424). The pid is what the shell + # names when it reports a port conflict, so it must be the real one. import os from app.main import app @@ -33,6 +33,30 @@ def test_health_identifies_the_answering_process(): assert client.get("/api/health").json()["pid"] == os.getpid() +def test_health_echoes_the_instance_token(monkeypatch): + # How the shell recognises its own backend (#457). The pid cannot do it: on + # the Windows portable build the venv launcher re-execs into + # python/base/python.exe, so the process that binds the port is a grandchild + # of the shell and its pid never matches the child that was spawned. The + # environment survives that re-exec, so identity travels there. + from app.main import app + + monkeypatch.setenv("STEMDECK_INSTANCE_TOKEN", "deadbeef") + with TestClient(app) as client: + assert client.get("/api/health").json()["instance"] == "deadbeef" + + +def test_health_reports_an_empty_token_when_unset(monkeypatch): + # Docker, Unraid and source checkouts have no shell to hand them a token. + # The field is always present so the shell can tell "no token" apart from a + # backend too old to have the field at all. + from app.main import app + + monkeypatch.delenv("STEMDECK_INSTANCE_TOKEN", raising=False) + with TestClient(app) as client: + assert client.get("/api/health").json()["instance"] == "" + + # --- version source precedence (#421) --------------------------------------- # # The in-app updater replaces backend/ but never python/, where the installed From c87ba904ae99e683a2f456a098cb20678ec8a64e Mon Sep 17 00:00:00 2001 From: Thales <> Date: Wed, 26 Aug 2026 10:05:30 +0100 Subject: [PATCH 2/2] Test the configured port with one the OS will not hand out (#461) a_free_port_is_granted_as_asked failed in CI asserting 62251 against 62250. The adjacent numbers are the tell: the test binds an ephemeral port to learn a free number, drops it, and asks reserve_port to claim that exact number back. Cargo runs this binary's tests in parallel and several of them stand up throwaway listeners, so a concurrent bind(0) can be handed the number in the gap. claim_port then fails, reserve_port falls back to free_port as designed, and the next port up comes back. The test was racing itself. reserve_port was right in the failing run. Scan a fixed range below the ephemeral range instead. Nothing calling bind(0) can be given a port outside it, which closes the window rather than narrowing it. It is also the honest shape of the thing under test: reserve_port is given a configured port, 8000 by default, never one the OS just handed out. Found while adding a third ephemeral listener in #457's tests, which is what took this from theoretical to observed. --- desktop/src-tauri/src/main.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index 852f5483..b6f918f9 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -5222,9 +5222,22 @@ b6052160df96b31c9b1e33854a4dcda3d4b57641b880270f31736fb9f445d384 ffmpeg-n7.1-la fn a_free_port_is_granted_as_asked() { // The fallback must not fire needlessly: the user's configured port is // honoured whenever it genuinely is available. - let probe = std::net::TcpListener::bind(("0.0.0.0", 0)).unwrap(); - let wanted = probe.local_addr().unwrap().port(); - drop(probe); + // + // The port must come from outside the OS ephemeral range. This test can + // only establish that a port is free by binding it and letting go, and + // reserve_port then has to re-claim it. If that number came from + // bind(0), any other test in this binary calling bind(0) in that gap is + // handed the number we just released, claim_port fails, and the + // fallback returns the next port up -- which is how this failed in CI, + // asserting 62251 against 62250. Cargo runs these in parallel and + // several of them stand up throwaway listeners. + // + // A fixed port is also the honest shape of the thing under test: + // reserve_port is given a configured port (8000 by default), never one + // the OS just handed out. + let wanted = (21_000..21_200) + .find(|port| std::net::TcpListener::bind(("0.0.0.0", *port)).is_ok()) + .expect("no free port in 21000..21200 to test with"); let (got, _guard) = super::reserve_port("0.0.0.0", wanted).unwrap();