test: cover the PIN gates; parse ?pin= instead of substring-matching it

The auth gate and the two HTTP PIN checks were inline in the socket handlers,
so the half of the security model that decides WHO is paired had no tests while
the half that decides what a paired client may say had ten. Extracted as pure
functions — auth_frame_ok, header_pin_ok, query_pin_ok, all over pin_matches —
and left the handlers to call them.

query_pin_ok now parses an actual query parameter. The old check was
target.contains("pin=<pin>") over the whole request target, which also accepted
?notpin=<pin> and ?pin=<pin>trailing-garbage. Neither is exploitable without
already knowing the PIN, so this is not a hole being closed so much as a check
being made to mean what it says — but the duplicate header logic in the two
handlers had already drifted apart once, which is the argument for one tested
function over two inline copies.

pin_matches refuses an empty PIN outright. load_or_create_pin regenerates on an
empty file so it should be unreachable, but that is a property of a different
function; if it lapsed, a bare `X-Pin:` header would authenticate everything.

Fifteen tests now. The new five pin: the auth frame must be exactly `auth
<pin>` (rejecting a prefix, a superstring, a case-variant verb, and an input
event sent before pairing), X-Pin matches case-insensitively by NAME only, the
query PIN is a parameter and not a substring, an empty PIN authorizes nothing,
and the two HTTP gates are not interchangeable — /shot must keep refusing a PIN
in the URL, where it would land in logs.

Verified by mutation: restoring the substring check fails the query test on
?notpin=, and dropping the empty-PIN guard fails its own. Verified live against
the running server too, since the query parser changed: the page's exact URL
shape still streams 200, pin-as-second-param works, ?notpin= and a
trailing-garbage PIN are now 403, /shot still takes the header either case and
still refuses a URL PIN.

Co-Authored-By: Claude Opus 5 <[email protected]>

 CLAUDE.md   |  23 ++++++---
 src/main.rs | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 165 insertions(+), 21 deletions(-)

diff --git a/CLAUDE.md b/CLAUDE.md
index 5a5bfb6..f530b67 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -64,13 +64,20 @@ differently, because of a browser constraint: `/shot` takes an `X-Pin` header (t
 `fetch()`es it), but `/stream` accepts `?pin=` in the query string, because an
 `<img src>` cannot carry headers.
 
+All three gates are pure functions — `auth_frame_ok`, `header_pin_ok`, `query_pin_ok`,
+all over `pin_matches` — so they are unit-tested rather than only reachable through a
+socket. `pin_matches` refuses an **empty** PIN outright: `load_or_create_pin`
+regenerates on an empty file so it should be unreachable, but that is a property of a
+*different* function, and if it lapsed, a bare `X-Pin:` header would authenticate
+everything. Gate on the dangerous state, don't trust the caller.
+
 Be honest about the resulting model rather than treating the PIN as security: it is
 **~20 bits, compared with `==` (not constant-time), over plain HTTP on 0.0.0.0**, cached
 in `localStorage`, and for `/stream` it travels in a URL — where it lands in any proxy
-or browser history that sees it. It is pairing, i.e. it stops the other devices on a
-trusted LAN from steering the desktop by accident. It is not a defense against someone
-who is on that network on purpose. For a hostile network the answer is a tunnel, not a
-longer PIN.
+or browser history that sees it. Nothing rate-limits attempts. It is pairing, i.e. it
+stops the other devices on a trusted LAN from steering the desktop by accident. It is
+not a defense against someone who is on that network on purpose. For a hostile network
+the answer is a tunnel, not a longer PIN.
 
 ## Framing: the control socket is one-shot
 
@@ -158,9 +165,11 @@ The awkward part: **there is no WebSocket client on this machine** (no `websocat
 `wscat`, no python `websockets`), so the WS path — which is most of the logic — can only
 be driven from a real phone, or by writing a throwaway client.
 
-`translate()` is the exception, and it is where the crate's one invariant is actually
-enforced, so it carries the crate's only tests (`cargo test -p cce-remote`, 10 of them,
-in `main.rs`). They cover the accepted shapes and — more to the point — everything that
+The pure functions are the exception, and they are where the crate's invariants are
+actually enforced, so they carry all the tests (`cargo test -p cce-remote`, 15 of them,
+in `main.rs`) — `translate()` for what a paired client may say, and the three PIN gates
+for who is paired at all. They cover the accepted shapes and — more to the point —
+everything that
 must be refused: unknown verbs *including the compositor's own command names*,
 malformed and missing arguments, `wf` targets outside `safe_token`, unwhitelisted `cmd`
 names, and the property that no input can make the output span two lines (an embedded
diff --git a/src/main.rs b/src/main.rs
index 96465e8..7f056ac 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -97,6 +97,56 @@ fn safe_token(t: &str) -> bool {
         && t.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-' | ':'))
 }
 
+/// Whether `candidate` is the pairing PIN.
+///
+/// An empty PIN never authorizes anything. `load_or_create_pin` cannot produce
+/// one — it regenerates on an empty file — but that is a property of a
+/// different function, and if it ever stopped holding, a bare `X-Pin:` header
+/// or an `auth ` frame with nothing after it would authenticate every request.
+/// Gate on the dangerous state here rather than trusting the caller.
+fn pin_matches(candidate: &str, pin: &str) -> bool {
+    !pin.is_empty() && candidate == pin
+}
+
+/// The WebSocket auth gate: the FIRST frame must be `auth <pin>`. Everything
+/// else — a wrong PIN, a different verb, an input event sent before pairing —
+/// closes the connection, so no input can be injected unauthenticated.
+fn auth_frame_ok(frame: &str, pin: &str) -> bool {
+    frame
+        .strip_prefix("auth ")
+        .is_some_and(|candidate| pin_matches(candidate.trim(), pin))
+}
+
+/// PIN carried by an `X-Pin` header. The header name is matched
+/// case-insensitively (HTTP field names are), the value is not.
+fn header_pin_ok(request_head: &str, pin: &str) -> bool {
+    request_head.lines().any(|line| {
+        line.to_ascii_lowercase()
+            .starts_with("x-pin:")
+            .then(|| line["x-pin:".len()..].trim())
+            .is_some_and(|candidate| pin_matches(candidate, pin))
+    })
+}
+
+/// PIN carried as a `?pin=` query parameter — needed because an `<img src>`
+/// cannot send headers, so `/stream` has no other way to authenticate.
+///
+/// Parsed as an actual parameter rather than searched for as a substring: the
+/// old `target.contains("pin=<pin>")` also accepted `?notpin=<pin>` and
+/// `?pin=<pin>trailing-garbage`. Neither is exploitable without already knowing
+/// the PIN, but "close enough to the right string" is not a check.
+fn query_pin_ok(request_head: &str, pin: &str) -> bool {
+    request_head
+        .split_whitespace()
+        .nth(1)
+        .and_then(|target| target.split_once('?'))
+        .is_some_and(|(_, query)| {
+            query
+                .split('&')
+                .any(|kv| kv.strip_prefix("pin=").is_some_and(|c| pin_matches(c, pin)))
+        })
+}
+
 /// `f64::from_str` accepts "NaN" / "inf" / "infinity", and `{:.2}` formats them
 /// straight back out, so without this a frame of `m NaN NaN` would reach the
 /// compositor's pointer math verbatim. Reject rather than clamp: no legitimate
@@ -239,8 +289,7 @@ fn handle_ws(stream: TcpStream, pin: &str) {
     // wrong PIN) closes the connection before any input can be injected.
     let authed = matches!(
         ws.read(),
-        Ok(tungstenite::Message::Text(t))
-            if t.strip_prefix("auth ").map(str::trim) == Some(pin)
+        Ok(tungstenite::Message::Text(t)) if auth_frame_ok(&t, pin)
     );
     if !authed {
         eprintln!("[cce-remote] auth failed: {peer}");
@@ -294,13 +343,9 @@ fn handle_ws(stream: TcpStream, pin: &str) {
 /// the client closes the socket. PIN via X-Pin header or ?pin= query (an
 /// <img src> can't carry headers).
 fn handle_stream(mut stream: TcpStream, request_head: &str, pin: &str) {
-    let pin_ok = request_head.lines().any(|l| {
-        let lower = l.to_ascii_lowercase();
-        lower.starts_with("x-pin:") && l[6..].trim() == pin
-    }) || request_head
-        .split_whitespace()
-        .nth(1)
-        .is_some_and(|target| target.contains(&format!("pin={pin}")));
+    // Header OR query: an <img src> cannot carry a header, so /stream accepts
+    // the PIN in the URL. /shot does not — see handle_http.
+    let pin_ok = header_pin_ok(request_head, pin) || query_pin_ok(request_head, pin);
     if !pin_ok {
         let _ = write!(stream, "HTTP/1.1 403 Forbidden\r\nContent-Length: 0\r\nConnection: close\r\n\r\n");
         return;
@@ -371,11 +416,9 @@ fn handle_http(mut stream: TcpStream, request_head: &str, pin: &str) {
     // /shot: the focused window's screenshot, PIN-gated via the X-Pin header
     // (the page fetch()es it — an <img src> couldn't carry a header).
     if request_head.starts_with("GET /shot") {
-        let pin_ok = request_head.lines().any(|l| {
-            let lower = l.to_ascii_lowercase();
-            lower.starts_with("x-pin:") && l[6..].trim() == pin
-        });
-        if !pin_ok {
+        // Header only: the page fetch()es this one, so unlike /stream there is
+        // no reason to let the PIN travel in a URL (where it lands in logs).
+        if !header_pin_ok(request_head, pin) {
             let _ = write!(stream, "HTTP/1.1 403 Forbidden\r\nContent-Length: 0\r\nConnection: close\r\n\r\n");
             return;
         }
@@ -475,6 +518,98 @@ mod tests {
         cmds.into_iter().next().unwrap()
     }
 
+    // ---- the pairing PIN ----
+    //
+    // The other half of the security model: translate() decides what a paired
+    // client may say, these decide who is paired at all. Both are reachable by
+    // anyone who can open a socket to this port.
+
+    const PIN: &str = "123456";
+
+    fn head(lines: &[&str]) -> String {
+        format!("{}\r\n\r\n", lines.join("\r\n"))
+    }
+
+    #[test]
+    fn ws_auth_requires_exactly_auth_then_pin() {
+        assert!(auth_frame_ok("auth 123456", PIN));
+        assert!(auth_frame_ok("auth   123456  ", PIN)); // value is trimmed
+        for frame in [
+            "auth 123457",      // wrong PIN
+            "auth 12345",       // prefix of it
+            "auth 1234567",     // superstring of it
+            "auth ",            // empty candidate
+            "auth",             // no separator
+            "AUTH 123456",      // verb is case-sensitive
+            "auth123456",
+            " auth 123456",     // must be the whole frame, unprefixed
+            "m 1 2",            // an input event before pairing
+            "",
+        ] {
+            assert!(!auth_frame_ok(frame, PIN), "{frame:?} must not authenticate");
+        }
+    }
+
+    #[test]
+    fn an_empty_pin_authorizes_nothing() {
+        // load_or_create_pin() regenerates on an empty file, so this should be
+        // unreachable — which is exactly why it is worth pinning. A truncated
+        // PIN file must fail closed, not open.
+        assert!(!auth_frame_ok("auth ", ""));
+        assert!(!auth_frame_ok("auth", ""));
+        assert!(!header_pin_ok(&head(&["GET /shot HTTP/1.1", "X-Pin:"]), ""));
+        assert!(!header_pin_ok(&head(&["GET /shot HTTP/1.1", "X-Pin: "]), ""));
+        assert!(!query_pin_ok(&head(&["GET /stream?pin= HTTP/1.1"]), ""));
+    }
+
+    #[test]
+    fn x_pin_header_is_matched_case_insensitively_by_name_only() {
+        for name in ["X-Pin", "x-pin", "X-PIN", "x-PiN"] {
+            let h = head(&["GET /shot HTTP/1.1", &format!("{name}: {PIN}"), "Host: x"]);
+            assert!(header_pin_ok(&h, PIN), "{name} should be accepted");
+        }
+        // Value whitespace is trimmed; the value itself must match exactly.
+        assert!(header_pin_ok(&head(&["GET /shot HTTP/1.1", "X-Pin:   123456  "]), PIN));
+        for bad in ["X-Pin: 123457", "X-Pin: 12345", "X-Pin: 1234567", "X-Pin:", "X-Pinx: 123456"] {
+            let h = head(&["GET /shot HTTP/1.1", bad]);
+            assert!(!header_pin_ok(&h, PIN), "{bad:?} must not authenticate");
+        }
+        // No header at all.
+        assert!(!header_pin_ok(&head(&["GET /shot HTTP/1.1", "Host: x"]), PIN));
+    }
+
+    #[test]
+    fn query_pin_is_a_parameter_not_a_substring() {
+        assert!(query_pin_ok(&head(&["GET /stream?pin=123456 HTTP/1.1"]), PIN));
+        assert!(query_pin_ok(&head(&["GET /stream?pin=123456&g=7 HTTP/1.1"]), PIN));
+        assert!(query_pin_ok(&head(&["GET /stream?g=7&pin=123456 HTTP/1.1"]), PIN));
+        for bad in [
+            "GET /stream?notpin=123456 HTTP/1.1",  // substring match used to pass this
+            "GET /stream?pin=1234567 HTTP/1.1",    // and this
+            "GET /stream?xpin=123456 HTTP/1.1",
+            "GET /stream?pin=12345 HTTP/1.1",
+            "GET /stream?pin= HTTP/1.1",
+            "GET /stream?pin HTTP/1.1",
+            "GET /stream HTTP/1.1",                // no query at all
+            "GET /pin=123456 HTTP/1.1",            // in the PATH, not the query
+        ] {
+            assert!(!query_pin_ok(&head(&[bad]), PIN), "{bad:?} must not authenticate");
+        }
+    }
+
+    #[test]
+    fn the_two_http_gates_are_not_interchangeable() {
+        // /stream takes either (an <img src> cannot send headers); /shot takes
+        // the header only, so the PIN stays out of URLs and logs where it can.
+        let query_only = head(&["GET /stream?pin=123456 HTTP/1.1", "Host: x"]);
+        assert!(query_pin_ok(&query_only, PIN));
+        assert!(!header_pin_ok(&query_only, PIN), "/shot must not accept a URL PIN");
+
+        let header_only = head(&["GET /shot HTTP/1.1", "X-Pin: 123456"]);
+        assert!(header_pin_ok(&header_only, PIN));
+        assert!(!query_pin_ok(&header_only, PIN));
+    }
+
     #[test]
     fn pointer_and_scroll_carry_fixed_precision() {
         assert_eq!(one("m 1 -2"), "pointer-move-by 1.00 -2.00");