aboutsummaryrefslogtreecommitdiffstats
path: root/packages
diff options
context:
space:
mode:
authorChristophe Besson <cbesson@gmail.com>2026-08-14 03:51:55 +0200
committerChristophe Besson <cbesson@gmail.com>2026-08-14 03:51:55 +0200
commit9f0904247116005bba8e32b9428dc6a2b994e705 (patch)
tree636bc6d803cdd27cf429c3fd78abccd58182efdf /packages
parent9a483774e97f8612b00e3d92c4d5ebc00c21980a (diff)
downloadmeshbay-9f0904247116005bba8e32b9428dc6a2b994e705.tar.gz
fix(client): capture the challenge values before joining, not after
join_request signs a transcript over the node key and the node nonce, and runs before the GEK proof — a first-time member has no key to prove with. Both values were read further down, beside the proof that also uses them, so by the time joinGroup() ran neither was set and every invited member got "Handshake incomplete — reconnect and retry". They are now recorded the moment the challenge arrives. Third bug of the same shape found in a browser, and the reason is worth writing down: QE/deploy/e2e.py cannot catch any of them. It is a second implementation of the client, written in the right order by construction, so it passes while the SPA fails. It proves the protocol; it proves nothing about app.js. So this adds ordering guards over transport.js — source-level, which is not how one would normally test behaviour, but it is what sees this class of mistake: - node_pk and nonce_node are captured before joinGroup() runs - the join happens before the GEK proof - the ack still verifies the key the challenge announced Verified the way the suite requires: each fails against the source as it was, on the ordering assertion rather than on a missing marker. e2e.py also waits for the node to re-register rather than reporting "no nodes" at whoever just restarted the hub. Tests: 337 across the three packages. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'packages')
-rw-r--r--packages/meshbay-hub/src/meshbay_hub/static/transport.js21
-rw-r--r--packages/meshbay-hub/tests/test_spa_ordering.py87
2 files changed, 100 insertions, 8 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport.js b/packages/meshbay-hub/src/meshbay_hub/static/transport.js
index c200674..5ead80e 100644
--- a/packages/meshbay-hub/src/meshbay_hub/static/transport.js
+++ b/packages/meshbay-hub/src/meshbay_hub/static/transport.js
@@ -174,6 +174,18 @@ class MeshBayTransport {
throw new Error('Node requires GEK proof but no crypto available');
}
+ // Recorded the moment the challenge arrives, because everything below may
+ // need them — joining, in particular, happens before the proof and signs a
+ // transcript over both. Reading them further down, next to the proof that
+ // also uses them, meant join_request ran with neither.
+ //
+ // nonce_node ties a join to this connection, so one cannot be lifted onto
+ // another. node_pk is announced here because a first-time member has no
+ // GEK and so cannot complete the handshake that would prove it; it is
+ // unverified at this point and checked against the ack below.
+ this._nonceNode = window.MeshBayCrypto.b64decode(reply.nonce);
+ this.nodePk = reply.node_pk || null;
+
// Recover session keys from node if not available locally (P2P keypair bundle)
if (!this._sessionKeys && this._bundleKey && window.MeshBayKeys) {
const kpResp = await this._sendAndWait({
@@ -258,14 +270,7 @@ class MeshBayTransport {
_extractDtlsFingerprint(this._pc.localDescription.sdp),
_extractDtlsFingerprint(this._rawAnswerSdp),
);
- const nonceNode = C.b64decode(reply.nonce);
- // Kept for the life of the connection: a join_request is signed over it,
- // which is what stops one being lifted onto another connection.
- this._nonceNode = nonceNode;
- // Announced in the challenge because joining needs it before the ack: a
- // first-time member has no GEK, so they cannot complete the handshake that
- // would prove this key. Unverified here; checked against the ack below.
- this.nodePk = reply.node_pk || null;
+ const nonceNode = this._nonceNode; // captured when the challenge arrived
const gid = groupId || '';
const proof = await C.handshakeProof(
diff --git a/packages/meshbay-hub/tests/test_spa_ordering.py b/packages/meshbay-hub/tests/test_spa_ordering.py
new file mode 100644
index 0000000..839698a
--- /dev/null
+++ b/packages/meshbay-hub/tests/test_spa_ordering.py
@@ -0,0 +1,87 @@
+"""
+Ordering guards for the SPA's connect() flow.
+
+These are source-level checks, which is not how one would normally test
+behaviour. They exist because a specific class of bug shipped to a live browser
+twice and no other test could see it: `connect()` is a long sequence in which
+later steps read values earlier steps set, and the Python end-to-end client in
+QE/deploy/ cannot catch a mistake there — it is a different implementation,
+written in the right order by construction, so it passes while the browser fails.
+
+Concretely: join_request signs a transcript over the node key and the node nonce,
+and runs *before* the GEK proof, because a first-time member has no GEK to prove.
+Both values were being read further down, next to the proof that also uses them,
+so every invited member hit "Handshake incomplete — reconnect and retry".
+
+If you restructure connect(), these will fail. Check the invariant still holds —
+that nothing reads a value assigned later — and then move the markers.
+"""
+
+from pathlib import Path
+
+import pytest
+
+STATIC = (Path(__file__).resolve().parents[1]
+ / "src" / "meshbay_hub" / "static")
+TRANSPORT = STATIC / "transport.js"
+
+pytestmark = pytest.mark.skipif(
+ not TRANSPORT.exists(), reason="SPA sources not present")
+
+
+def _positions(*needles: str) -> list[int]:
+ source = TRANSPORT.read_text()
+ out = []
+ for needle in needles:
+ idx = source.find(needle)
+ assert idx != -1, f"{needle!r} is gone from transport.js — update this test"
+ out.append(idx)
+ return out
+
+
+def test_challenge_values_are_captured_before_joining():
+ """
+ joinGroup() signs over node_pk and nonce_node, so both must be recorded when
+ the challenge arrives — not later, beside the proof.
+ """
+ # Deliberately loose markers: what matters is where the assignment happens,
+ # not how it is spelled, so a reordering fails on the ordering assertion
+ # below rather than on a missing string.
+ node_pk, nonce_node, join_call = _positions(
+ "this.nodePk = reply.node_pk",
+ "this._nonceNode = ",
+ "await this.joinGroup(",
+ )
+ assert node_pk < join_call, (
+ "node_pk is read from the challenge after joinGroup() runs — the join "
+ "would sign a transcript naming nothing")
+ assert nonce_node < join_call, (
+ "nonce_node is captured after joinGroup() runs — the join would not be "
+ "bound to this connection")
+
+
+def test_join_happens_before_the_gek_proof():
+ """
+ The whole point of joining in the pre-proof window: someone who has never
+ held the group key cannot produce a proof, so the key has to arrive first.
+ """
+ join_call, proof = _positions(
+ "await this.joinGroup(",
+ "await C.handshakeProof(",
+ )
+ assert join_call < proof, (
+ "the join must happen before the GEK proof — a first-time member has no "
+ "key to prove with")
+
+
+def test_the_ack_still_verifies_the_announced_node_key():
+ """
+ Taking node_pk from the challenge is only safe because the ack proves it and
+ the client compares the two. Losing that check would leave the announcement
+ trusted on its own.
+ """
+ source = TRANSPORT.read_text()
+ assert "Node identity changed during the handshake" in source, (
+ "the challenge's node_pk must be checked against the ack's")
+ assert "verifyNodeSignature" in source, (
+ "the ack's signature over the handshake transcript must still be verified")