From f7bc2a8f683a14a8f5abe73035920453b8b16892 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:23:20 +1000 Subject: [PATCH 1/5] Android launcher: reap a previous launch's Lepton left by a SIGKILLed launcher The lock isn't inherited by Lepton, so a launcher killed before Lepton made its container left an untracked Lepton that a new Play could overlap. The launcher records its child's process group and, once it holds the lock, ends a recorded group that is still running this app.apk (never an unrelated reused id). Co-Authored-By: Claude Opus 5.5 (1M context) --- frame/android/lepton-app.sh | 16 ++++++++++++++++ tests/test_frame_android_library.py | 24 +++++++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/frame/android/lepton-app.sh b/frame/android/lepton-app.sh index 594a6a9..8f393e2 100644 --- a/frame/android/lepton-app.sh +++ b/frame/android/lepton-app.sh @@ -30,6 +30,20 @@ fi exec 9>"$DIR/launch.lock" flock -n 9 || { echo "Android app is already running" >&2; exit 1; } CONTAINER="lepton-steamlaunch-$SteamAppId" +# Lepton doesn't hold the lock, so a launcher SIGKILLed before Lepton made its +# container leaves a Lepton that nothing tracks: end that process group first. +# Only a group still running this app.apk, never an unrelated reused id. +PGID_FILE="$DIR/launch.pgid" +if [[ -f "$PGID_FILE" ]]; then + old="$(cat "$PGID_FILE")" + if [[ "$old" =~ ^[0-9]+$ ]] && ps -A -o pgid=,args= | awk -v g="$old" '$1 == g' | grep -qF -- "$DIR/app.apk"; then + echo "Stopping the previous launch (process group $old)" >&2 + kill -TERM -- "-$old" 2>/dev/null || true + for _ in 1 2 3 4 5 6 7 8 9 10; do kill -0 -- "-$old" 2>/dev/null || break; sleep 0.5; done + kill -KILL -- "-$old" 2>/dev/null || true + fi + rm -f "$PGID_FILE" +fi # Holding the lock means no launcher owns a running container: it was orphaned # (this script SIGKILLed), so stop it rather than refuse every later Play. if [[ "$(podman inspect --format '{{.State.Running}}' "$CONTAINER" 2>/dev/null || true)" == true ]]; then @@ -60,6 +74,7 @@ cleanup() { kill -KILL "$child" 2>/dev/null || true wait "$child" 2>/dev/null || true fi + rm -f "$PGID_FILE" } trap cleanup EXIT trap 'exit 143' TERM @@ -68,6 +83,7 @@ trap 'exit 129' HUP # 9>&-: the lock is this launcher's alone; Lepton's tree mustn't keep it held. setsid --wait "$LEPTON" waitforexitandrun -- "$DIR/app.apk" 9>&- & child=$! +echo "$child" > "$PGID_FILE" rc=0 wait "$child" || rc=$? child="" diff --git a/tests/test_frame_android_library.py b/tests/test_frame_android_library.py index fdee8cb..66ad857 100644 --- a/tests/test_frame_android_library.py +++ b/tests/test_frame_android_library.py @@ -24,7 +24,7 @@ spec.loader.exec_module(shortcuts) @unittest.skipIf(os.name == 'nt', 'POSIX launcher') class LauncherTests(unittest.TestCase): - def exercise(self, terminate, sig=signal.SIGTERM, blocked=None, orphan=False): + def exercise(self, terminate, sig=signal.SIGTERM, blocked=None, orphan=False, stale=None): with tempfile.TemporaryDirectory() as tmp: d = Path(tmp) app = d / 'Applications/Android/org.test.app' @@ -60,6 +60,13 @@ class LauncherTests(unittest.TestCase): saved = d / '.local/share/Steam/steamapps/compatdata/2800000001/internal/save' saved.parent.mkdir(parents=True) saved.write_text('saved game') + previous = None + if stale: + # A Lepton left by a SIGKILLed launcher (its own session), or an unrelated reused id. + args = [sys.executable, '-c', 'import time; time.sleep(30)'] + previous = subprocess.Popen(args + ([str(app / 'app.apk')] if stale == 'lepton' else ['other']), + start_new_session=True) + (app / 'launch.pgid').write_text(str(previous.pid)) proc = subprocess.Popen(['bash', str(app / 'launch.sh')], env=env, stdout=subprocess.PIPE, stderr=subprocess.PIPE) try: if blocked: @@ -73,6 +80,10 @@ class LauncherTests(unittest.TestCase): while not (d / 'started').exists() and proc.poll() is None and time.monotonic() < deadline: time.sleep(.02) self.assertTrue((d / 'started').exists(), 'launcher did not start Lepton') + if previous: + self.assertEqual(previous.poll() is not None, stale == 'lepton') + self.assertEqual((app / 'launch.pgid').read_text().strip(), + (d / 'started').read_text().strip()) # the new group is recorded self.assertFalse((d / 'inherited-lock').exists(), 'Lepton inherited the launch lock') if terminate: self.assertIsNone(proc.poll(), 'Steam-tracked wrapper exited during the session') @@ -84,7 +95,12 @@ class LauncherTests(unittest.TestCase): self.assertEqual(proc.returncode, 128 + sig if terminate else 23) self.assertEqual(saved.read_text(), 'saved game') self.assertTrue((app / 'app.apk').exists()) + self.assertFalse((app / 'launch.pgid').exists()) finally: + if previous and previous.poll() is None: + previous.kill() + if previous: + previous.wait() if proc.poll() is None: proc.kill() proc.communicate() @@ -105,6 +121,12 @@ class LauncherTests(unittest.TestCase): def test_duplicate_launch_leaves_existing_session_alone(self): self.exercise(False, blocked='LOCKED') + def test_previous_launch_left_by_sigkill_is_reaped_first(self): + self.exercise(True, stale='lepton') + + def test_reused_process_group_id_is_left_alone(self): + self.exercise(True, stale='other') + def test_orphaned_container_is_stopped_and_play_proceeds(self): # Container running but the lock free: its launcher was SIGKILLed. self.exercise(False, orphan=True) From d7cb967786aa6b7876ddc18c28c2c07c898287b9 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:23:20 +1000 Subject: [PATCH 2/5] Artwork fetches: the deadline bounds the whole request, trickling servers included Name resolution runs in a thread within the budget, a watchdog shuts the socket at the deadline, and the body is read one receive at a time with the remaining time as timeout. SteamGridDB goes through the same bounded fetch, without redirects. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_frame_android_library.py | 51 +++++++++++++++++++ ui/apk_sources/_images.py | 79 ++++++++++++++++++++++------- ui/frame_steamgriddb.py | 19 ++----- 3 files changed, 116 insertions(+), 33 deletions(-) diff --git a/tests/test_frame_android_library.py b/tests/test_frame_android_library.py index 66ad857..f594d50 100644 --- a/tests/test_frame_android_library.py +++ b/tests/test_frame_android_library.py @@ -189,6 +189,57 @@ class ArtworkTests(unittest.TestCase): with self.assertRaises(SourceError): art.fetch('file:///etc/passwd') + def trickle(self, head, seconds): + # A server that answers one byte every 20 ms, over a socketpair standing in for the network. + import socket + import threading + from apk_sources import _images + client, server = socket.socketpair() + def serve(): + try: + server.recv(65536) + for byte in head + b'x' * 1000: + server.sendall(bytes([byte])) + time.sleep(0.02) + except OSError: + pass + finally: + server.close() + threading.Thread(target=serve, daemon=True).start() + public = [(2, 1, 6, '', ('93.184.216.34', 80))] + with patch.object(_images.socket, 'getaddrinfo', return_value=public), \ + patch.object(_images.socket, 'create_connection', return_value=client): + start = time.monotonic() + with self.assertRaisesRegex(_images.SourceError, 'too long'): + _images.get('http://example.org/a.png', deadline=start + seconds) + return time.monotonic() - start + + def test_deadline_bounds_trickling_headers_and_body(self): + self.assertLess(self.trickle(b'HTTP/1.1 200 OK\r\nContent-Length: 1000\r\n\r\n', 0.15), 0.4) + self.assertLess(self.trickle(b'HTTP/1.1 200 OK\r\n', 0.15), 0.4) # headers never finish + + def test_deadline_covers_name_resolution(self): + import threading + from apk_sources import _images + gate = threading.Event() + with patch.object(_images.socket, 'getaddrinfo', side_effect=lambda *a, **k: gate.wait(5) and []): + start = time.monotonic() + with self.assertRaisesRegex(_images.SourceError, 'too long'): + _images.get('https://example.org/a.png', deadline=start + 0.1) + self.assertLess(time.monotonic() - start, 0.4) + gate.set() + with self.assertRaisesRegex(_images.SourceError, 'too long'): + _images.get('https://example.org/a.png', deadline=time.monotonic() - 1) + + def test_steamgriddb_uses_the_bounded_fetch_without_redirects(self): + import frame_steamgriddb as sgdb + from apk_sources import _images + with patch.object(_images, 'get', return_value=b'{"success": true, "data": [1]}') as get: + self.assertEqual(sgdb._get('/search/x', 'secret', time.monotonic() + 5), [1]) + self.assertEqual(get.call_args.kwargs['redirects'], 0) + self.assertEqual(get.call_args.args[1]['Authorization'], 'Bearer secret') + self.assertLessEqual(get.call_args.kwargs['deadline'] - time.monotonic(), 5) + def test_supplied_jpeg(self): data = (FIXTURES / 'icon.jpg').read_bytes() self.assertEqual(art.image_type(data), 'jpg') diff --git a/ui/apk_sources/_images.py b/ui/apk_sources/_images.py index 5b91081..6205c4c 100644 --- a/ui/apk_sources/_images.py +++ b/ui/apk_sources/_images.py @@ -65,53 +65,94 @@ def image_type(data): def fetch(url, redirects=3, deadline=None, limit=MAX_IMAGE): """deadline: time.monotonic() value by which the whole fetch, redirects included, must finish.""" + data = get(url, {'Accept': 'image/png,image/jpeg,image/webp,image/gif'}, redirects, deadline, limit) + return data, image_type(data) + + +def _resolve(host, port, timeout): + # getaddrinfo has no timeout of its own; a thread keeps a slow resolver inside the budget. + found = {} + + def run(): + try: + found['addresses'] = socket.getaddrinfo(host, port, type=socket.SOCK_STREAM) + except OSError as e: + found['error'] = e + worker = threading.Thread(target=run, daemon=True) + worker.start() + worker.join(timeout) + if 'error' in found: + raise found['error'] + if 'addresses' not in found: + raise SourceError('Artwork download took too long') + return found['addresses'] + + +def get(url, headers=None, redirects=3, deadline=None, limit=MAX_IMAGE): + """GET a public HTTP(S) URL within an overall deadline (default 60 s), redirects included. + + A watchdog shuts the socket at the deadline, so a server trickling bytes can't outlast it.""" + deadline = time.monotonic() + 60 if deadline is None else deadline + + def left(): + remaining = deadline - time.monotonic() + if remaining <= 0: + raise SourceError('Artwork download took too long') + return remaining if not valid_url(url): raise SourceError('Artwork URL is not allowed') - - def remaining(): - if deadline is None: - return 10 - left = deadline - time.monotonic() - if left <= 0: - raise SourceError('Artwork download took too long') - return min(10, left) p = urlsplit(url) port = p.port or (443 if p.scheme == 'https' else 80) - addresses = socket.getaddrinfo(p.hostname, port, type=socket.SOCK_STREAM) + addresses = _resolve(p.hostname, port, left()) if not addresses or any(not ipaddress.ip_address(a[4][0]).is_global for a in addresses): raise SourceError('Private network artwork is not allowed') # Connect to the checked IP, never resolve again between validation and use. - sock = socket.create_connection((addresses[0][4][0], port), timeout=remaining()) + live = [socket.create_connection((addresses[0][4][0], port), timeout=min(10, left()))] + + def expire(): + try: + live[0].shutdown(socket.SHUT_RDWR) + except OSError: + pass + watchdog = threading.Timer(left(), expire) + watchdog.daemon = True + watchdog.start() conn = http.client.HTTPConnection(p.hostname, port, timeout=10) try: if p.scheme == 'https': - sock = ssl.create_default_context().wrap_socket(sock, server_hostname=p.hostname) - conn.sock = sock + live[0] = ssl.create_default_context().wrap_socket(live[0], server_hostname=p.hostname) + conn.sock = live[0] path = p.path or '/' if p.query: path += '?' + p.query - conn.request('GET', path, headers={'User-Agent': 'FrameControl/0.3.1', 'Accept': 'image/png,image/jpeg,image/webp,image/gif'}) - sock.settimeout(remaining()) + conn.request('GET', path, headers={'User-Agent': 'FrameControl/0.3.1', **(headers or {})}) + live[0].settimeout(min(10, left())) response = conn.getresponse() if response.status in (301, 302, 303, 307, 308) and redirects: target = urljoin(url, response.getheader('Location', '')) conn.close() - return fetch(target, redirects - 1, deadline, limit) + return get(target, headers, redirects - 1, deadline, limit) if response.status != 200: raise SourceError('Artwork is unavailable') data = b'' while len(data) <= limit: - sock.settimeout(remaining()) - chunk = response.read(min(65536, limit + 1 - len(data))) + live[0].settimeout(min(10, left())) + chunk = response.read1(min(16384, limit + 1 - len(data))) # one receive at most if not chunk: break data += chunk if len(data) > limit: raise SourceError('Artwork is too large') - return data, image_type(data) + left() + return data + except (OSError, http.client.HTTPException) as e: + if time.monotonic() >= deadline: + raise SourceError('Artwork download took too long') from e + raise finally: + watchdog.cancel() conn.close() - sock.close() + live[0].close() def remember(url, data): diff --git a/ui/frame_steamgriddb.py b/ui/frame_steamgriddb.py index 6384d3d..017511c 100644 --- a/ui/frame_steamgriddb.py +++ b/ui/frame_steamgriddb.py @@ -6,7 +6,6 @@ import tempfile import time import unicodedata import urllib.parse -import urllib.request import frame_host @@ -51,19 +50,11 @@ def save_settings(body): def _get(path, key, deadline=None): - timeout = 12 if deadline is None else min(12, deadline - time.monotonic()) - if timeout <= 0: - raise ValueError('SteamGridDB lookup took too long') - request = urllib.request.Request(API + path, headers={'Authorization': 'Bearer ' + key, - 'User-Agent': 'FrameControl/1.0'}) - # Do not carry the credential to redirects or include it in error messages. - class NoRedirect(urllib.request.HTTPRedirectHandler): - def redirect_request(self, *args, **kwargs): - return None - with urllib.request.build_opener(NoRedirect()).open(request, timeout=timeout) as response: - data = response.read(MAX_JSON + 1) - if len(data) > MAX_JSON: - raise ValueError('SteamGridDB response too large') + from apk_sources import _images + # No redirects: the credential never goes anywhere but the API. Errors never contain it. + data = _images.get(API + path, {'Authorization': 'Bearer ' + key, 'Accept': 'application/json'}, redirects=0, + deadline=time.monotonic() + 12 if deadline is None else min(deadline, time.monotonic() + 12), + limit=MAX_JSON) result = json.loads(data) if not isinstance(result, dict) or not result.get('success') or not isinstance(result.get('data'), list): raise ValueError('SteamGridDB lookup failed') From 2df0f0e32a2c6c79eadf224e48b58b22ba020782 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:23:20 +1000 Subject: [PATCH 3/5] Tests: put LocalMode's Windows skip back; artwork settings tests run everywhere Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_server.py b/tests/test_server.py index 47334c2..a1f9626 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -234,7 +234,6 @@ class ServerGuards(unittest.TestCase): self.assertEqual(self.post("/api/nope", {})[0], 404) -@unittest.skipIf(os.name == "nt", "runs on the Frame (Linux); local-bin/ssh is a POSIX shell script") class ArtworkSettings(unittest.TestCase): """The settings panel's endpoints, with and without the page's X-Frame-UI key.""" @@ -285,6 +284,7 @@ class ArtworkSettings(unittest.TestCase): self.assertLess(page.index("async function api("), page.index('