fix(web,tray,host): review follow-ups — pair the halves, and stop two comments overclaiming

Review of the parent commit. One real defect, the rest accuracy.

The resolver could hand back a MISMATCHED pair, which is the one invariant its
own header promised it never would: `nativeCert` came from `dirname(cert)` and
`nativeKey` from `dirname(key)`, two independent directories, so
("/a/cert.pem", "/b/key.pem") resolved to /a/native-cert.pem + /b/native-key.pem
— two unrelated files presented as a pair. No shipped launcher splits them, but
the guard is one comparison and it is the whole point of the module.

Rewritten off a suffix test instead of `node:path`, which fixes two more things
in passing. `node:path` resolves per-RUNTIME, so a POSIX CI runner reads
`C:\ProgramData\punktfunk\cert.pem` as one long filename and never swaps — and
Windows, where windows/service.rs hands us exactly that, is the platform the CI
job can never exercise. The suffix test gives the same answer everywhere and is
now covered by a win32 case. It also leaves the prefix VERBATIM, where
`join(dirname(p), …)` normalised /a/b/../cert.pem into a different directory the
moment `b` was a symlink.

Existence is no longer enough: `pf_paths::write_secret_file` is
create+truncate+write rather than temp+rename, so a console starting mid-write
could adopt a 0-byte cert and leave `Bun.serve` throwing on every restart. Not
every launcher retries forever — the Steam Deck unit is `Restart=on-failure`
under the default rate limit, i.e. permanently dead. The check is now a
non-empty stat, mirroring the host's own `!c.trim().is_empty()`. Verified: with
native-cert.pem truncated to 0 bytes the console starts and serves the legacy
pair.

Two comments of mine overclaimed and are corrected rather than left to mislead:

  * serve() said "cert.pem existing implies the native pair does too". False on
    an upgraded host whose native clients pinned the legacy cert — load_or_adopt
    returns it and writes no native files at all. The ordering claim that IS
    true is narrower: whenever that call writes a native pair, it does so before
    cert.pem appears.
  * the tray said the console entry "always opens the same URL regardless of the
    probe". True of the menu entry, but win.rs gates the tray-icon single-click
    on console_up. Also notes that the Windows probe was never pinned to begin
    with (punktfunk_config_dir is None off Linux), so that half is a no-op.

Rest is doc drift the parent commit annotated in two launchers but not the other
four: web.env.example, README, web-run.cmd, and the ci.yml comment that still
said the web test step was "Scoped to server/".

18/18 web tests (was 14), biome and rustfmt clean, and the runtime check re-run
against a fresh build: both pairs -> P-256 with SANs; 0-byte native cert ->
legacy RSA, console still serving.

.gitea/workflows/ci.yml

@@ -370,9 +370,11 @@ jobs:
         run: bun run build
       - name: Typecheck
         run: bun run lint
-      # Scoped to server/: the console's browser code has no test runner, but the gate that keeps a
-      # plugin's origin apart from the console's does — and its failure mode is a well-formed header
-      # that only a browser rejects, which nothing else here would catch.
+      # Scoped to server/ and nitro-entry/: the console's browser code has no test runner, but two
+      # gates here do — the one keeping a plugin's origin apart from the console's, whose failure
+      # mode is a well-formed header that only a browser rejects, and the one picking which of the
+      # host's two identities the console serves, whose failure mode is a cert no browser accepts.
+      # Neither would be caught anywhere else.
       - name: Test
         run: bun run test
 

crates/punktfunk-host/src/gamestream/mod.rs

@@ -431,13 +431,16 @@ pub fn serve(
     //
     // Resolved BEFORE the legacy GameStream identity below, and that order is load-bearing twice
     // over. (1) The web console gates its start on `cert.pem` existing and then serves the native
-    // pair sitting next to it (web/nitro-entry/tls-paths.mjs); minting the legacy pair first leaves
-    // a first-run window where the console starts, finds no native pair, and serves the SAN-less
-    // RSA cert no browser accepts — for the rest of that boot. This way `cert.pem` existing implies
-    // the native pair does too. (2) In the degenerate case (native clients paired, but the cert
-    // they pinned is gone from disk) the old order let `load_or_create` mint a BRAND-NEW cert.pem
-    // that `load_or_adopt` then adopted while logging that it was preserving their pins — stranding
-    // them silently. Reading the dir first means that case reaches the branch written for it.
+    // pair sitting next to it (web/nitro-entry/tls-paths.mjs); minting the legacy pair first
+    // leaves a first-run window where the console starts, finds no native pair, and serves the
+    // SAN-less RSA cert no browser accepts — for the rest of that boot. Running first closes that
+    // window: whenever this call WRITES a native pair, it has done so before `cert.pem` appears.
+    // (It does not write one on an upgraded host whose native clients pinned the legacy cert —
+    // there the console correctly falls back to that same legacy pair.) (2) In the degenerate case
+    // (native clients paired, but the cert they pinned is gone from disk) the old order let
+    // `load_or_create` mint a BRAND-NEW cert.pem that `load_or_adopt` then adopted while logging
+    // that it was preserving their pins — stranding them silently. Reading the dir first means
+    // that case reaches the branch written for it.
     let native_ident = crate::identity::load_or_adopt(&np).context("native host identity")?;
     #[cfg(feature = "gamestream")]
     let state = {

crates/punktfunk-tray/src/status.rs

@@ -220,9 +220,11 @@ fn poll_loop(
     // cert would have hit it just as squarely, so the coupling goes rather than the symptom.
     //
     // Nothing is lost by dropping the pin: this probe sends no credentials, reads no body, and
-    // decides only a menu LABEL. A local port-squatter could make that label read "up" — but the
-    // entry is always present and always opens the same URL regardless of the probe, so it gains
-    // nothing it did not already have.
+    // decides only presentation — the menu entry's label, plus whether a tray-icon click opens
+    // the console or the menu (win.rs). A port-squatter could flip that, but the entry itself is
+    // unconditional and opens the same URL either way, and no browser ever pinned this cert. On
+    // Windows the probe was never pinned to begin with: `punktfunk_config_dir` returns None there,
+    // so `load_pin` was already None.
     let console_agent = agent(None);
     let mut last: Option<(TrayStatus, bool)> = None;
     // When the summary became unreachable while the service was running (grace anchor).

web/README.md

@@ -58,6 +58,8 @@ PORT=47992 HOST=0.0.0.0 \
   PUNKTFUNK_UI_TLS_KEY=~/.config/punktfunk/key.pem PUNKTFUNK_UI_SECURE=1 \
   bun run start           # = bun run .output/server/index.mjs
 # PUNKTFUNK_UI_TLS_* unset ⇒ plain HTTP (local dev); both set ⇒ HTTPS (HTTP/1.1 over TLS).
+# Naming cert.pem/key.pem serves native-cert.pem/native-key.pem instead when both sit beside them
+# (the identity split — nitro-entry/tls-paths.mjs); the legacy pair is the fallback, not the target.
 # The host's self-signed mgmt cert is accepted only for the proxy's loopback hop, scoped in code
 # (Bun per-request TLS: server/routes/api/[...].ts) — no process-wide NODE_TLS_REJECT_UNAUTHORIZED.
 # See .env.example.

web/nitro-entry/tls-paths.mjs

@@ -28,11 +28,48 @@
 // build key.pem is the Moonlight PAIRING SIGNING key, native-key.pem is only a TLS key.
 //
 // Swapped as a PAIR or not at all — a native cert with the legacy key is a server that cannot
-// complete a handshake with anyone. A host that never took the split (upgraded, native clients
-// still pinning the RSA cert, so `load_or_adopt` keeps serving it) has no native pair on disk and
-// falls through unchanged, as does a cert an operator supplied under any other name.
-import { existsSync } from "node:fs";
-import { basename, dirname, join } from "node:path";
+// complete a handshake with anyone, so both halves must be present AND must come from the same
+// directory. A host that never took the split (upgraded, native clients still pinning the RSA cert,
+// so `load_or_adopt` keeps serving it) has no native pair on disk and falls through unchanged, as
+// does a cert an operator supplied under any other name.
+import { statSync } from "node:fs";
+
+/**
+ * The directory prefix (separator included) of a path ending in `base`, or null if it does not.
+ *
+ * Deliberately NOT `node:path`: that resolves per-RUNTIME, so a POSIX build reads
+ * `C:\ProgramData\punktfunk\cert.pem` as one long filename — and Windows, where the service
+ * supervisor hands us exactly that (windows/service.rs), is the platform CI can never exercise.
+ * A suffix test gives the same answer everywhere. It also leaves the prefix VERBATIM, where
+ * `join(dirname(p), …)` would normalise `/a/b/../cert.pem` to a different directory than the one
+ * the operator named — which matters the moment `b` is a symlink.
+ *
+ * @param {string} p
+ * @param {string} base
+ * @returns {string | null}
+ */
+function dirPrefix(p, base) {
+	if (p === base) return ""; // bare relative name
+	if (!p.endsWith(base)) return null;
+	const sep = p[p.length - base.length - 1];
+	return sep === "/" || sep === "\\" ? p.slice(0, -base.length) : null;
+}
+
+/**
+ * A readable, NON-EMPTY file. Emptiness matters: `pf_paths::write_secret_file` is
+ * create+truncate+write rather than temp+rename, so a console starting mid-write could otherwise
+ * adopt a 0-byte cert and leave `Bun.serve` throwing on every restart — and not every launcher
+ * retries forever (the Steam Deck unit is `Restart=on-failure` under the default rate limit).
+ *
+ * @param {string} p
+ */
+function usable(p) {
+	try {
+		return statSync(p).size > 0;
+	} catch {
+		return false;
+	}
+}
 
 /**
  * @param {string | undefined} cert  PUNKTFUNK_UI_TLS_CERT, verbatim.
@@ -40,15 +77,15 @@ import { basename, dirname, join } from "node:path";
  * @param {(p: string) => boolean} [exists]  injected by the test; defaults to a real stat.
  * @returns {{cert: string | undefined, key: string | undefined}}
  */
-export function resolveUiTlsPaths(cert, key, exists = existsSync) {
+export function resolveUiTlsPaths(cert, key, exists = usable) {
 	// Half-configured TLS is the caller's error to report (it refuses to start); don't mask it by
 	// resolving one half of a pair that isn't there.
 	if (!cert || !key) return { cert, key };
-	if (basename(cert) !== "cert.pem" || basename(key) !== "key.pem") {
-		return { cert, key };
-	}
-	const nativeCert = join(dirname(cert), "native-cert.pem");
-	const nativeKey = join(dirname(key), "native-key.pem");
+	const dir = dirPrefix(cert, "cert.pem");
+	// Same directory, or we are not looking at a pair — see the PAIR note above.
+	if (dir === null || dir !== dirPrefix(key, "key.pem")) return { cert, key };
+	const nativeCert = `${dir}native-cert.pem`;
+	const nativeKey = `${dir}native-key.pem`;
 	return exists(nativeCert) && exists(nativeKey)
 		? { cert: nativeCert, key: nativeKey }
 		: { cert, key };

web/nitro-entry/tls-paths.test.ts

@@ -1,13 +1,14 @@
 // The pair swap is all-or-nothing, and the fallbacks are what keep legacy and custom-cert hosts
 // serving. A native cert with the legacy key would be a console nobody can handshake with, so the
-// mixed cases are the ones worth pinning down.
+// mixed cases are the ones worth pinning down — including the Windows shape, which the resolver
+// must get right without a win32 runtime to ask (see dirPrefix in tls-paths.mjs).
 import { describe, expect, it } from "bun:test";
 import { resolveUiTlsPaths } from "./tls-paths.mjs";
 
 const DIR = "/home/you/.config/punktfunk";
 const legacy = [`${DIR}/cert.pem`, `${DIR}/key.pem`] as const;
 const native = [`${DIR}/native-cert.pem`, `${DIR}/native-key.pem`] as const;
-/** `exists` over a fixed set of files on disk. */
+/** `exists` over a fixed set of usable files on disk. */
 const on =
 	(...files: string[]) =>
 	(p: string) =>
@@ -28,7 +29,7 @@ describe("resolveUiTlsPaths", () => {
 		});
 	});
 
-	it("never mixes halves when only one native file exists", () => {
+	it("never mixes halves when only one native file is usable", () => {
 		for (const half of native) {
 			expect(resolveUiTlsPaths(...legacy, on(...legacy, half))).toEqual({
 				cert: legacy[0],
@@ -37,11 +38,55 @@ describe("resolveUiTlsPaths", () => {
 		}
 	});
 
+	// The Windows service supervisor hands us backslash paths (windows/service.rs); node:path on a
+	// POSIX CI runner would read the whole thing as one filename and silently never swap.
+	it("resolves Windows paths without a win32 runtime", () => {
+		const win = ["C:\\ProgramData\\punktfunk", "D:\\pf"] as const;
+		for (const d of win) {
+			expect(
+				resolveUiTlsPaths(`${d}\\cert.pem`, `${d}\\key.pem`, () => true),
+			).toEqual({
+				cert: `${d}\\native-cert.pem`,
+				key: `${d}\\native-key.pem`,
+			});
+		}
+	});
+
+	it("refuses to pair halves from two different directories", () => {
+		expect(resolveUiTlsPaths("/a/cert.pem", "/b/key.pem", () => true)).toEqual({
+			cert: "/a/cert.pem",
+			key: "/b/key.pem",
+		});
+	});
+
+	it("leaves the prefix verbatim rather than normalising it away", () => {
+		// `join(dirname(p), …)` would collapse this to /a/native-cert.pem — a different directory
+		// the moment `b` is a symlink.
+		expect(
+			resolveUiTlsPaths("/a/b/../cert.pem", "/a/b/../key.pem", () => true),
+		).toEqual({
+			cert: "/a/b/../native-cert.pem",
+			key: "/a/b/../native-key.pem",
+		});
+	});
+
 	it("leaves an operator's own cert alone, native pair present or not", () => {
-		const own = [`${DIR}/lan-ca.pem`, `${DIR}/lan-ca.key`] as const;
-		expect(resolveUiTlsPaths(...own, on(...own, ...native))).toEqual({
-			cert: own[0],
-			key: own[1],
+		// Also covers the endsWith trap: "mycert.pem" ends with "cert.pem" but is not one.
+		for (const own of [
+			[`${DIR}/lan-ca.pem`, `${DIR}/lan-ca.key`],
+			[`${DIR}/mycert.pem`, `${DIR}/mykey.pem`],
+		] as const) {
+			expect(resolveUiTlsPaths(...own, on(...own, ...native))).toEqual({
+				cert: own[0],
+				key: own[1],
+			});
+		}
+	});
+
+	it("does not re-swap a pair that already names the native files", () => {
+		expect(resolveUiTlsPaths(...native, () => true)).toEqual({
+			cert: native[0],
+			key: native[1],
 		});
 	});
 

web/web-run.cmd

@@ -51,6 +51,9 @@ if exist "%ENDPOINTFILE%" for /f "usebackq tokens=1* delims==" %%A in ("%ENDPOIN
 rem No NODE_TLS_REJECT_UNAUTHORIZED: the host's self-signed cert is accepted only for the loopback
 rem proxy hop, scoped inside the proxy code (Bun per-request TLS), not process-wide.
 rem Serve HTTPS (HTTP/1.1 over TLS) with the host's identity cert; mark the session cookie Secure.
+rem These name the LEGACY pair; the server prefers native-cert.pem/native-key.pem beside them when
+rem both exist (the identity split - web\nitro-entry\tls-paths.mjs). Don't "fix" them to the native
+rem names: a host that never took the split has no native pair, and the fallback lives in there.
 set "PUNKTFUNK_UI_TLS_CERT=%CERTFILE%"
 set "PUNKTFUNK_UI_TLS_KEY=%KEYFILE%"
 set "PUNKTFUNK_UI_SECURE=1"

web/web.env.example

@@ -18,7 +18,9 @@ HOST=0.0.0.0
 
 # Serve the console over HTTPS (HTTP/1.1 over TLS) with the host's own identity cert. BOTH paths
 # set ⇒ HTTPS. (No HTTP/2 or HTTP/3: Bun.serve has no HTTP/2 server, and a browser won't speak
-# HTTP/3/QUIC against this self-signed, no-SAN host cert — so HTTP/1.1 over TLS is what's offered.)
+# HTTP/3/QUIC against a self-signed host cert — so HTTP/1.1 over TLS is what's offered.)
+# Name the LEGACY pair below: the server prefers native-cert.pem/native-key.pem beside it when both
+# exist (nitro-entry/tls-paths.mjs), and falls back to these on a host that never took the split.
 PUNKTFUNK_UI_TLS_CERT=%h/.config/punktfunk/cert.pem
 PUNKTFUNK_UI_TLS_KEY=%h/.config/punktfunk/key.pem
 # Mark the session cookie Secure (required once served over TLS):