Audit remote contracts and keep Noise failures terminal - #902
Conversation
Deploying mouseterm with
|
| Latest commit: |
a2f80c7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://43d27e76.mouseterm.pages.dev |
| Branch Preview URL: | https://spec-cleanup-remote-contract.mouseterm.pages.dev |
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
lib/src/remote/burrow/one-time-runtime.ts still says a link is reserved at message 1. That contradicts the reservation timing this PR corrects in remote-security-model.md -> "One-time connection" and the one-time FAIL IF row. The code reserves only after readMessage, writeMessage, and handshake.session all succeed, but three comments there say otherwise:
- the
ReservedPhonedoc: "A phone that completed message 1: the link is reserved for it." - the
#onInitdoc: "Noise message 1 against the link's key. The first valid one reserves the link" - the
#onInitcatch: "The link stays open: nothing decrypted against it, so nobody holds it." A message 1 can decrypt and then fail atwriteMessageor on a non-empty payload.
burrow-runtime.ts got the matching catch-comment fix in this PR, so these look like the missed twins. They're outside the diff, so I can't post them as suggestions. I can push the comment-only fix if you'd like.
GitHub Actions
Waiting
…e the connection table The PR hardened NoiseHandshake against concurrent steps (re-checks in #step, session, isComplete, and #split), added a Proxy-wrapped crypto.subtle microtask-observer test, and wrote a new Noise Must rule plus rationale. No caller can produce that race: all six handshake call sites create their own handshake and await each step in order, and each inbound init gets a fresh responder, so no Relay, network attacker, or peer can drive concurrent steps on one handshake. The base #step comment already calls concurrent stepping a caller bug. Revert noise.ts, noise.test.mjs, the rule, and its rationale. The PR also replaced remote-network.md's connectionsFor row-by-condition table with one vague sentence pointing at NetworkSettings.tsx. Which connection is listed under which condition is behaviour a reader trusts a level by, not shape; restore the table verbatim (every row re-checked against the current connectionsFor and still correct). remote-api.md lost the bold **Scope: protocol-v1** label that relay.md still cites by name; restore it ahead of the PR's compact Must rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
The title and the second paragraph of the description still describe the Noise hardening, but 5f07986 reverted it. The diff no longer touches noise.ts or its tests. Its one executable change is now a single line in noise-transport.ts. As written, the description has the PR keeping Noise failures terminal and exposing a session only after the final step commits, and calls that "the only executable change". The "Trimmed in review" note at the bottom contradicts that. The title becomes the squash commit subject, so the reverted change would land in history. Consider retitling it to something like "Audit remote contracts against the shipped code" and removing that paragraph.
| @@ -59,7 +63,7 @@ phone, or fabricate a request (rationale). The only path back out is | |||
| - **FAIL IF** the Burrow's Noise static is ever sent to the Relay, or a Burrow runs with halves that do not correspond: it is minted locally *before* the enrollment request and never sent in it, persisted only where `burrowToken` is, and `BurrowService` derives the public point from the private half and compares before starting — a mismatch keeps the Burrow down (rationale). | |||
| - **FAIL IF** `remote-lib-common/src/security/` stops being the shared implementation: the Relay, the Burrow, and the Pocket client must verify assertions, presence challenges, handshakes, and transport framing with the same modules. Conformance is proven against an independent implementation's published vector (`remote-lib-common/test/noise.test.mjs`), never against a value the production state machine computed, and this section's properties are driven end to end by `remote-lib-common/test/security-guarantees.test.mjs`. | |||
| - **FAIL IF** `scripts/e2e-lint.mjs` and `scripts/e2e-lint-selftest.mjs` stop running in the root `pnpm test`, or a rule is added to the lint without the self-test proving it load-bearing. Each rule in `RULES` names the line above that it enforces, or one in `docs/specs/security-hosted.md` -> "Rendezvous boundary" (rationale). | |||
There was a problem hiding this comment.
This line is the one FAIL in today's security audit (#908). The two RelayRoom rules in scripts/e2e-lint.mjs cite docs/specs/security-hosted.md -> "Relay boundary", which this line doesn't name, although the lint already accepts either section. Since this PR already edits the next line, folding the fix in here avoids a conflicting bot PR:
| - **FAIL IF** `scripts/e2e-lint.mjs` and `scripts/e2e-lint-selftest.mjs` stop running in the root `pnpm test`, or a rule is added to the lint without the self-test proving it load-bearing. Each rule in `RULES` names the line above that it enforces, or one in `docs/specs/security-hosted.md` -> "Rendezvous boundary" (rationale). | |
| - **FAIL IF** `scripts/e2e-lint.mjs` and `scripts/e2e-lint-selftest.mjs` stop running in the root `pnpm test`, or a rule is added to the lint without the self-test proving it load-bearing. Each rule in `RULES` names the line above that it enforces, or one in `docs/specs/security-hosted.md` -> "Rendezvous boundary" or "Relay boundary" (rationale). |
2 participants
Audit the remote trust, API, network-policy, and security references against the shipped implementations. Correct invitation reservation timing, pending-handshake admission, terminal-directory scope, direct signaling, Hosted boundaries, and Windows storage qualifications. Canonical interfaces own method and payload inventories; cross-boundary rules and staged designs remain.
A concurrent Noise step previously poisoned the handshake while allowing its first caller to publish transport ciphers. Keep failure terminal and expose a session only after the final step commits. This is the only executable change; the rest is specs, rationale, and source comments.
Validation: 338 protocol/security tests; strict library types; spec/public-doc gates and 139 security-lint mutations. Stacked on #897.
Trimmed in review. Reverted the Noise concurrency re-checks, their Proxy-wrapped
crypto.subtletest, and the new Must rule. All six handshake call sites create their own handshake and await each step in order, so no Relay, attacker or peer can drive concurrent steps. Restored theconnectionsFortable in remote-network.md; every row was re-checked against the code. Restored the bold Scope: protocol-v1 label that relay.md cites.🤖 Generated with Claude Code