Skip to content

fix: reject malformed binary frames and decode empty metadata as {} - #19

Merged
JarbasAl merged 1 commit into
devfrom
fix/reject-malformed-frames
Sep 10, 2026
Merged

JarbasAl merged 1 commit into
devfrom
fix/reject-malformed-frames

Conversation

@openvoiceos-bot

Copy link
Copy Markdown
Contributor

🤖 Auto-generated by Claude Fable 5.1 (claude-fable-5-1) via Claude Code — NOT human-reviewed. Verify before acting.

hivemind/binary.py decode() read the 5-bit message-type field of a binary frame and returned whatever value it found (0-31) unvalidated, and returned the metadata block as raw, unparsed bytes that no caller ever decoded as JSON. This PR adds a MalformedBinaryFrame error, raised for an unassigned message-type code, a metadata length that overruns the remaining bytes, non-JSON metadata, or a truncated frame, and changes the metadata field to decode to the parsed JSON object ({} for a zero-length block). The one caller in hivemind/client.py now catches the new error, logs it, and drops the frame without closing the connection.

HIVEMIND-WIRE-1 §4.2: "Code 11 and codes 13–31 are unassigned. A sender MUST NOT emit them, and a receiver MUST reject a frame carrying one as malformed rather than mapping it to any type." Assigned codes are 0–10 and 12.

HIVEMIND-WIRE-1 §4.1: "A receiver MUST decode the block as a UTF-8 JSON object, and MUST treat a zero-length block as an equivalent empty field set without attempting to parse it."

Verified by execution:

# fail-before, unmodified decode() on an unassigned code (13):
UNFIXED decode of unassigned code 13 did NOT raise: 13

# fail-before, new test module against unmodified binary.py:
ImportError: cannot import name 'MalformedBinaryFrame' from 'hivemind.binary'

# after the fix:
test/test_binary.py .......... [12 passed]

# full suite:
85 passed, 5 warnings, 24 subtests passed in 44.99s

The 85 total is the pre-existing 78 plus 7 new tests (three unassigned-code rejections, an over-long metadata length, a truncated frame, a non-JSON metadata block, and a 200-iteration fuzz test asserting nothing but MalformedBinaryFrame or a clean decode ever escapes). No test was skipped. The pre-existing TestLegacyFallback suite and the legacy password-handshake fallback in client.py are untouched; that fallback is a separate, tracked backcompat decision, not part of this change.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: fa4b9eaa-03d3-455d-81b7-790dada1b6bd


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

HIVEMIND-WIRE-1 §4.2 states that codes 11 and 13-31 are unassigned and
"a receiver MUST reject a frame carrying one as malformed rather than
mapping it to any type." binary.py decode() read the 5-bit message-type
field and returned any value 0-31 unchecked, silently accepting a
reserved or unassigned code. §4.1 requires that "a receiver MUST decode
the block as a UTF-8 JSON object, and MUST treat a zero-length block as
an equivalent empty field set" — decode() instead returned the metadata
block as raw, unparsed bytes, so callers never saw {} for an empty block
or the parsed object for a populated one.

§4.1 also states: "A receiver MUST reject a frame whose frame-format
version it does not implement." decode() read the 8-bit frame-format
version field into protocol_version and returned it unchecked, so a
frame declaring version 2 (or any value other than the version 1 this
client implements) decoded and executed instead of being rejected.

decode() now raises a new MalformedBinaryFrame (a ValueError) for an
unassigned message-type code, an unimplemented frame-format version, a
metadata length that exceeds the remaining bytes, a metadata block that
is not valid JSON, or a truncated frame. A zero-length metadata block
decodes to {}; a non-empty one is parsed and returned as the JSON
object. The version check reads FRAME_FORMAT_VERSION, the same constant
encode() writes, rather than a literal. The one caller in client.py
that consumes decode() catches MalformedBinaryFrame, logs it, and drops
the frame without closing the connection.

Fail-before: decoding an unassigned code (13) against the unmodified
decode() returned {"msg_type": 13, ...} without raising; a frame
declaring frame-format version 2 (or 0, or 255) decoded and returned
protocol_version unchecked instead of raising. With only the version
check removed from decode() (constant kept for import), the three new
version tests failed: "AssertionError: MalformedBinaryFrame not
raised" for versions 0, 2 and 255. All three pass after the fix, with
the raised version named in the message.

Full suite: 88 passed (85 pre-existing plus 3 new), 0 skipped, 5
pre-existing deprecation warnings, 0 introduced.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@JarbasAl
JarbasAl force-pushed the fix/reject-malformed-frames branch from 96f19d1 to 605eb96 Compare September 10, 2026 15:16
JarbasAl added a commit that referenced this pull request Sep 10, 2026
… legacy_hub

The client chose the pre-v3 password handshake automatically whenever the
hub could not offer Noise (no PSK, a v2-capped hub, max_protocol_version=2,
or no mutual pattern/suite), violating HIVEMIND-CRYPTO-1 §3 ("The handshake
is mandatory on every connection: there is no cleartext, pre-shared-key, or
password alternative and no protocol-version ladder to negotiate down") and
§5 ("A node MUST run the mandatory Noise handshake on every connection and
reject any peer that cannot complete it, with no cleartext or legacy
fallback").

A hub that cannot complete Noise is now rejected: no shake response is sent
and last_error names the violated clause. legacy_hub (default False) is the
only way to reach a legacy hub; setting it prints a loud warning at connect
time naming the pre-v3 handshake's lack of PAKE/forward secrecy and the
computed next-major removal version. max_protocol_version below 3 now
requires legacy_hub=True at construction time, since it can only ever reach
a hub through that same legacy path.

Fail-before: reverting the source (keeping the new tests) failed 6 of 6
rewritten tests, including a TypeError on the missing legacy_hub kwarg and
an AssertionError expecting rejection where the client instead sent a
plaintext shake. Restoring the fix: full suite 87 passed (baseline 85 at
PR #19's head, +2 net across replaced/added legacy-handshake tests).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@JarbasAl
JarbasAl marked this pull request as ready for review September 10, 2026 20:20
@JarbasAl
JarbasAl merged commit f7b9602 into dev Sep 10, 2026
4 checks passed
JarbasAl added a commit that referenced this pull request Sep 10, 2026
… legacy_hub

The client chose the pre-v3 password handshake automatically whenever the
hub could not offer Noise (no PSK, a v2-capped hub, max_protocol_version=2,
or no mutual pattern/suite), violating HIVEMIND-CRYPTO-1 §3 ("The handshake
is mandatory on every connection: there is no cleartext, pre-shared-key, or
password alternative and no protocol-version ladder to negotiate down") and
§5 ("A node MUST run the mandatory Noise handshake on every connection and
reject any peer that cannot complete it, with no cleartext or legacy
fallback").

A hub that cannot complete Noise is now rejected: no shake response is sent
and last_error names the violated clause. legacy_hub (default False) is the
only way to reach a legacy hub; setting it prints a loud warning at connect
time naming the pre-v3 handshake's lack of PAKE/forward secrecy and the
computed next-major removal version. max_protocol_version below 3 now
requires legacy_hub=True at construction time, since it can only ever reach
a hub through that same legacy path.

Fail-before: reverting the source (keeping the new tests) failed 6 of 6
rewritten tests, including a TypeError on the missing legacy_hub kwarg and
an AssertionError expecting rejection where the client instead sent a
plaintext shake. Restoring the fix: full suite 87 passed (baseline 85 at
PR #19's head, +2 net across replaced/added legacy-handshake tests).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
JarbasAl added a commit that referenced this pull request Sep 10, 2026
… legacy_hub

The client chose the pre-v3 password handshake automatically whenever the
hub could not offer Noise (no PSK, a v2-capped hub, max_protocol_version=2,
or no mutual pattern/suite), violating HIVEMIND-CRYPTO-1 §3 ("The handshake
is mandatory on every connection: there is no cleartext, pre-shared-key, or
password alternative and no protocol-version ladder to negotiate down") and
§5 ("A node MUST run the mandatory Noise handshake on every connection and
reject any peer that cannot complete it, with no cleartext or legacy
fallback").

A hub that cannot complete Noise is now rejected: no shake response is sent
and last_error names the violated clause. legacy_hub (default False) is the
only way to reach a legacy hub; setting it prints a loud warning at connect
time naming the pre-v3 handshake's lack of PAKE/forward secrecy and the
computed next-major removal version. max_protocol_version below 3 now
requires legacy_hub=True at construction time, since it can only ever reach
a hub through that same legacy path.

Fail-before: reverting the source (keeping the new tests) failed 6 of 6
rewritten tests, including a TypeError on the missing legacy_hub kwarg and
an AssertionError expecting rejection where the client instead sent a
plaintext shake. Restoring the fix: full suite 87 passed (baseline 85 at
PR #19's head, +2 net across replaced/added legacy-handshake tests).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
JarbasAl added a commit that referenced this pull request Sep 10, 2026
… legacy_hub (#20)

The client chose the pre-v3 password handshake automatically whenever the
hub could not offer Noise (no PSK, a v2-capped hub, max_protocol_version=2,
or no mutual pattern/suite), violating HIVEMIND-CRYPTO-1 §3 ("The handshake
is mandatory on every connection: there is no cleartext, pre-shared-key, or
password alternative and no protocol-version ladder to negotiate down") and
§5 ("A node MUST run the mandatory Noise handshake on every connection and
reject any peer that cannot complete it, with no cleartext or legacy
fallback").

A hub that cannot complete Noise is now rejected: no shake response is sent
and last_error names the violated clause. legacy_hub (default False) is the
only way to reach a legacy hub; setting it prints a loud warning at connect
time naming the pre-v3 handshake's lack of PAKE/forward secrecy and the
computed next-major removal version. max_protocol_version below 3 now
requires legacy_hub=True at construction time, since it can only ever reach
a hub through that same legacy path.

Fail-before: reverting the source (keeping the new tests) failed 6 of 6
rewritten tests, including a TypeError on the missing legacy_hub kwarg and
an AssertionError expecting rejection where the client instead sent a
plaintext shake. Restoring the fix: full suite 87 passed (baseline 85 at
PR #19's head, +2 net across replaced/added legacy-handshake tests).

Co-authored-by: JarbasAi <jarbasai@mailfence.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants