Invariants & tripwires#

Rules in the PacketRelay codebase that read like tidy-up opportunities and are not. Each entry states the rule, why breaking it looks reasonable, what actually goes wrong, and the test that fails when you try. If you are about to simplify one of these, read its entry first.

Every unregister path checks same_channel#

Rule. A connection may only remove its own registration. unregister (session.rs:383-451) and unregister_member (session.rs:299-331) both take the caller's mpsc::Sender and compare it against the stored one with Sender::same_channel before touching anything. A mismatch returns false and mutates nothing.

Why breaking it looks safe. The handler knows its session_id, its Role and its member_id. session.members.retain(|m| m.member_id != member_id) is one line, obviously correct, and does not need the sender threaded through four call sites. Passing a channel handle around purely to compare it against another channel handle reads like defensive noise.

What actually goes wrong. Three of the five registration paths replace rather than reject on reconnect. A desktop reconnect silently overwrites desktop_tx (session.rs:169-175), a host reconnect overwrites host_tx (session.rs:202-206), and a room member reusing its member_id has its tx replaced in place (session.rs:227-256). The old socket is still open at that moment; its task discovers the close a moment later and runs cleanup.

Without the ownership check, that late cleanup evicts the successor — the connection that just reconnected and is working. The session then has no desktop, no host, or one fewer member than it should, and the surviving peers receive a peer_disconnected, host_disconnected or member_left for a client that is still connected. Worse, when the eviction empties the last slot the whole session is removed (session.rs:412-414, session.rs:439-441), and for a room that also drops every TOFU pin through forget_session. The reconnecting client's next frame goes nowhere.

The failure is timing-dependent, invisible in logs beyond an ordinary disconnect line, and reproduces only when the old and new connections overlap — which is exactly what a flaky network produces.

Pinned by. Three tests in session.rs, one per session kind, each of which asserts a negative:

• unregister_member_after_reconnect_is_noop (session.rs:587) — registers a member, reconnects it with a new channel, then runs the first connection's cleanup and asserts the member is still there.
• stale_bridge_cleanup_does_not_remove_reconnected_desktop (session.rs:618).
• stale_broadcast_cleanup_does_not_remove_reconnected_host (session.rs:650).

A test that pins a deliberate no-op is not redundant. It is the only thing standing between the one-line simplification and a class of bug that only appears under reconnect load.

jarvis-room-hello-v1 is a cross-repository contract#

Rule. ROOM_HELLO_SIG_DOMAIN = "jarvis-room-hello-v1" (protocol.rs:13), ROOM_HELLO_SEP = 0x1F (protocol.rs:18), and the field order domain ‹0x1F› session_id ‹0x1F› member_id ‹0x1F› pubkey ‹0x1F› nonce (protocol.rs:42-61) are frozen. So is the outer wrapping: what is actually signed is STANDARD base64(canonical_bytes) as an ASCII string, not the raw bytes (protocol.rs:63-73).

Why breaking it looks safe. Every part of it looks like an implementation detail with no external reader. The domain string embeds a product name this project no longer uses. 0x1F is an unusual choice where a colon or a newline would be more readable. Signing the base64 of the bytes rather than the bytes themselves is a wasted encode. Reordering the fields to match the struct definition is a two-minute change that leaves every test in protocol.rs passing, because those three tests cover response serialisation, not the signing payload.

What actually goes wrong. Four Room clients in other repositories construct these bytes themselves before signing — protocol.rs:11-12 names them: desktop pair, desktop presence, desktop chat JS, mobile chat JS. They are not built from this crate and they do not import anything from it. A change here does not break them; it makes every signature they produce fail to verify.

The relay's only symptom is "invalid room hello signature" on every room join — a message that is deliberately coarse (see below) and therefore names neither the cause nor which client is affected. Nothing logs "the canonical bytes changed", because from the relay's point of view nothing did.

Each element earns its place. The domain separator exists so a room-hello signature cannot be cross-presented to any other signer sharing the same ECDSA identity key; pair frames use the disjoint domain jarvis-pair-sig-v1 (protocol.rs:9). 0x1F is chosen because it is disjoint from the base64 alphabet, from hostnames and from the member_id charset, so the delimited fields cannot be made ambiguous by a crafted field value. The base64 wrapping exists so the payload round-trips losslessly through the clients' &str-based crypto APIs — CryptoService::{sign,verify} on desktop and Web Crypto on mobile — which cannot take arbitrary bytes.

Pinned by. golden_signed_hello_payload_vector (room_auth.rs:807), which asserts one literal:

signed_hello_payload("sid", "m1", "pk", 42)
    == "amFydmlzLXJvb20taGVsbG8tdjEfc2lkH20xH3BrHzQy"

Any change to the domain, the separator, the field order or the outer encoding moves that string. It is the only thing in this repository that catches the change, and it catches it as a one-line diff rather than as a client outage.

Verify is pure; commit mutates#

Rule. RoomAuthStore::verify (room_auth.rs:157) takes a read lock and leaves no state behind. RoomAuthStore::commit (room_auth.rs:214) takes the write lock and is the only thing that inserts or updates a Pin { pubkey, last_nonce }. The connection handler calls commit only after the slot is actually registered (connection.rs:217-242).

Why breaking it looks safe. The two functions re-check the same two conditions — pubkey equality and nonce monotonicity. Folding the pin write into verify removes the duplicated check, removes a second lock acquisition, and removes the whole verify-then-commit dance from the caller. One function, one lock, one pass.

What actually goes wrong. Between verify and commit the connection can still be refused, and there are four ways it happens: the room is at --max-room-members, ensure_session refuses on a kind clash or the global session cap, register_room fails, or the first send to the new client errors. If verifying had already written the pin, every one of those paths would leave a pin and a nonce high-water mark behind for a member that was never admitted.

That poisons the slot. The nonce high-water is now set to a value the legitimate client has already used, so its next hello — which will carry a nonce from the same millisecond range — fails the strict monotonic check and is rejected as a replay. A capacity rejection would have converted itself into a lockout that persists until the room empties and forget_session drops the pins (room_auth.rs:248). The client sees "invalid room hello signature" and has no way to distinguish it from a real crypto failure.

The split has a second job. commit re-checks under the write lock rather than trusting the verify that preceded it, so two concurrent valid hellos for the same slot cannot race a replay through the read-lock window. The loser is torn back down: connection.rs:227-241 unregisters the just-registered slot, calls forget_session if that emptied the room, and sends the reject message — so a lost race never leaves a slot held without a pin behind it.

Pinned by. verify_alone_leaves_no_pin (room_auth.rs:688) verifies a valid hello and asserts both pinned(…) and last_nonce(…) are still None. commit_advances_high_water_only_after_register (room_auth.rs:705) pins the other direction.

Nonce monotonicity is strict, not >=#

Rule. A hello is accepted only when its nonce is strictly greater than the last nonce accepted for that (session_id, member_id) slot. Both the read-side check (room_auth.rs:198) and the write-side re-check (room_auth.rs:228) are if nonce <= pin.last_nonce { return Err(StaleNonce) }.

Why breaking it looks safe. nonce is unix-epoch milliseconds (room_auth.rs:313-319). Two hellos from the same client in the same millisecond are not an attack, they are a fast reconnect, and rejecting the second one looks like an off-by-one that will bite a real user on a fast network. Relaxing <= to < — accepting an equal nonce — makes that case work and appears to cost nothing, because the signature still has to verify and the ±30 s freshness window still applies.

What actually goes wrong. Relaxing it removes the entire anti-replay guarantee. The freshness window (NONCE_WINDOW_MS = 30_000, room_auth.rs:77) is a sanity bound, not a defence: it is explicitly documented as such, and it means a captured hello stays fresh for up to thirty seconds either side of the relay's clock. A hello is a complete, self-contained, replayable credential during that window.

The strict comparison is what closes it. Once a hello has been committed, its own nonce is the high-water mark, so replaying that exact hello is rejected — the replay carries the same nonce, and the same nonce is no longer greater. Since room registration replaces the stored tx for a matching member_id (session.rs:227-256), a successful replay would evict the live connection holding the slot and hand routing for that member to whoever captured the frame. Under >= the credential becomes reusable for thirty seconds, which is long enough to be practical.

A genuine reconnect is unaffected because it signs a fresh hello with the current clock, which is strictly later than the one it sent before. Two hellos inside the same millisecond are the price, and they are the correct price.

Pinned by. Three tests in room_auth.rs, covering the equal, older and newer cases separately: replayed_same_nonce_rejected (room_auth.rs:619), older_nonce_rejected (room_auth.rs:640) and strictly_newer_reconnect_accepted (room_auth.rs:663). The first is the one that fails when <= becomes <; the third is what stops someone "fixing" it by tightening the check instead.

The session cap is enforced under the creation write lock#

Rule. ensure_session (session.rs:112) takes the write lock once and does the existence check, the kind check, the map.len() >= max_total_sessions test and the insert inside it (session.rs:117-132). The count is never read outside that lock and then acted on.

Why breaking it looks safe. A count() helper already exists on the store. Checking capacity with a cheap read lock before taking the expensive write lock is the textbook optimisation, and the window between them is microseconds.

What actually goes wrong. The window is exactly where the attack lives. Every inbound connection that reaches a hello can create a session — a desktop_hello creates a bridge, a spectator_hello auto-creates a broadcast (connection.rs:99-107), and a room_hello that passes verification creates a room. With the check outside the lock, N connections arriving together all read a count below the cap and all proceed to insert, so the map overshoots by as much as the concurrency. The cap is the only bound on the session map, and the map is unbounded process memory (session.rs:93).

The kind check has the same shape and the same reason. Reading the existing session's kind under a read lock and then inserting under a write lock lets two hellos of different kinds for the same session_id both observe "no session here" and both create one, with the loser's participants silently routed into a session of the wrong shape.

Pinned by. session_cap_is_atomic_under_concurrent_creation (session.rs:750) spawns 32 concurrent ensure_session calls against a store capped at 4, and asserts exactly 4 returned Ok(true) and exactly 28 returned Err("server at capacity"). The equality assertions are the point — a test that merely checked "at most 4 exist afterwards" would pass against several broken implementations.

Reject reasons stay coarse at the boundary and precise in the log#

Rule. RoomHelloRejectReason has five variants (room_auth.rs:82-99) and message() (room_auth.rs:106) collapses them to two client-facing strings:

The precise variant is logged server-side with reason = ?reason (connection.rs:130).

Why breaking it looks safe. This is the single most tempting change in the codebase. Five distinct failure modes are collapsed into two opaque strings, and a client developer debugging a room join cannot tell a clock-skew problem from a bad base64 encoding from a genuine key conflict. Returning the variant name costs one line and would save hours of support.

What actually goes wrong. The two groups are drawn along a disclosure line, not a convenience line. Splitting StaleNonce out of the first group tells an attacker that the signature verified and only the nonce was wrong, which converts a captured hello from "rejected" into "rejected, keep trying with a different timestamp" — a working oracle for replay timing. Splitting MalformedCrypto from BadSignature distinguishes an encoding error from a cryptographic failure, which is a probe for the exact encodings the verifier accepts.

The second group leaks slot state. PubkeyMismatch means the slot is already pinned to a different key; FingerprintMismatch means it is not pinned yet and the id embeds a fingerprint that does not match. Distinguishing them tells an enumerator which member_ids are live in a room whose session_id they hold — and the room session_id is a capability secret, so anything that turns possession of it into a roster is a real escalation.

room_auth.rs:103-105 states the rule in the source: coarse enough not to leak which check failed beyond what an attacker can already infer. The debugging need is answered on the other side — the server log has the exact variant, which is why the enum exists as an enum rather than as two strings.

Note the same principle applied one level up: an absent credential yields "signed room hello required" (connection.rs:150), which is distinct because a client that sent no signature already knows it sent no signature.

Pinned by. The five reject paths each have their own test — bad_signature_rejected (room_auth.rs:478), presented_pubkey_not_signing_key_rejected (room_auth.rs:491), squatter_with_different_key_rejected (room_auth.rs:455), malformed_pubkey_rejected (room_auth.rs:592) and fingerprinted_member_id_wrong_key_rejected_pre_pin (room_auth.rs:754) — which assert the variant, so the mapping to messages can be reviewed as a single function rather than inferred from scattered string comparisons.

testdata/ is a compile-time input, not test data#

Rule. protocol.rs:161-169 pulls both fixture files into the binary with include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/testdata/…")). Any build tree that does not contain testdata/ fails to compile.

Why breaking it looks safe. The directory is named testdata, the only references to it are inside a #[cfg(test)] module, and Docker build contexts are usually trimmed to what the runtime needs. Dropping COPY testdata ./testdata from Dockerfile:7 looks like removing test material from a production image.

What actually goes wrong. include_str! is a macro expanded during compilation, and macro expansion happens before cfg stripping has any bearing on whether the file must exist. The build fails at cargo build --locked --release (Dockerfile:9) with a macro error naming a path — not a test failure, and not anything that mentions Docker. The image never builds.

The fixtures are compiled in on purpose: they are the same bytes the clients deserialise, so session_ready_json_matches_fixture and room_ready_json_matches_fixture compare what RelayResponse serialises against the shared file rather than against a copy pasted into a test.

Pinned by. Nothing in CI. .github/workflows/ci.yml does not run a Docker build, so this one is caught by cargo test --locked locally only if the file is missing from the source tree — a Dockerfile that stops copying it is caught at deploy time. That is the gap; treat Dockerfile:5-7 as load-bearing.

Shorter tripwires#

Same class, less to say about each.

The keepalive envelope is matched exactly. is_relay_keepalive_ping (connection.rs:480-488) requires length under 64, valid JSON, an object with exactly one key, that key "type", that value "ping". Loosening it to a substring or a two-key tolerance would let real application frames be swallowed as keepalives and never forwarded — a silent data loss with no error anywhere. Pinned by keepalive_detection_requires_exact_envelope (connection.rs:610), which asserts {"type":"ping","id":1} is not a keepalive.

Room session ids are redacted in logs; other ids are not. log_session (connection.rs:552-559) emits six characters plus a length for Role::Member and the full id for everyone else. This is not an oversight in either direction: a room session_id is the room's capability secret, so logging it in full hands room access to anyone with log read. The asymmetry is itself flagged as open work — bridge and broadcast ids also control admission and are logged in full (BACKLOG.md:145-148).

member_frame.member_id is relay-authenticated; the payload is not. protocol.rs:146-150 states it: consumers must treat the envelope's member_id as the sender identity, and any user_id inside payload is self-asserted. The relay stamps the envelope from the registered slot (connection.rs:367-388) and never reads the payload. Any client that trusts the inner field has undone the room's entire authentication.

A member never receives its own frame. Fan-out target selection excludes the sender (session.rs:260-275), pinned by room_targets_exclude_sender_include_others (session.rs:514). Clients rely on this for local echo; sending the sender its own frame would double every message in every room client at once.

Error message literals are asserted. ensure_session and the register functions return &'static str and the tests match on the exact text — session.rs:772 matches Err("server at capacity"). Rewording a message for readability is a test-visible change on purpose, because those strings cross the wire to clients.

Next#

• Agent orientation — layout, file ownership, conventions and the dead dependencies.
• Build & test — the gate ladder and what each test module covers.
• Room authentication — the full trust model behind four of these entries.
• Security model — what the relay authenticates and what it does not.