docs: replace the CLAUDE.md memory dump with a spec, threat model, ADRs and bridge contracts - #365
Merged
Merged
Conversation
…Rs and bridge contracts
CLAUDE.md had grown to 61 KB in a handful of paragraph-long bullets. It was
unreadable by people, unreviewable in a diff, and the only home for knowledge
that several parts of the codebase depend on. CHANGELOG.md had the same
problem for a different reason: 2,425 lines with no archiving story.
Relocate the durable knowledge into documents that can be reviewed, linked and
cited, and cut CLAUDE.md to repository instructions plus a pointer table
(61,107 -> 7,606 bytes).
New reference material under docs/:
spec/ implementation-independent protocol specification: identity
and address derivation, the message model with both
encodings and the frozen binary layout, the reserved prefix
registry with the signing gate and its two exemption
classes, the three encryption envelopes, the group
protocol, and capability negotiation
security/ threat model: assets, six adversary classes, five trust
boundaries, a control-to-attack table, and residual risks
R1 through R8 stated plainly
state-machines/ delivery and acknowledgement, outbox and retries, session
lifecycle, group message lifecycle, transport lifecycle,
with diagrams
adr/ fifteen decisions that are expensive to reverse or easy to
undo by accident, each with a "what would undo this" note
bridges/ the Rust to Swift/Kotlin/Python/TypeScript contract, every
rule of which fails silently when violated
CHANGELOG.md now holds unreleased changes plus the current release; older
releases move to docs/changelog/, one file per minor series. Content is
byte-identical apart from one blank line at the split boundary, verified by
diff. No CI job or script parses the changelog, so the split is safe.
Five Rust doc comments pointed at CLAUDE.md sections that no longer exist
there; they now name the specific document.
Deliberately not carried over: the per-feature "Coverage: test_x, test_y"
lists. They were the most staleness-prone part of the old file, and
enumerating test names in prose recreates the problem this change is
addressing. The docs name the behaviour that is pinned instead.
No behavioural change. cargo fmt, clippy and the full test suite are green.
Review of the restructure verified every load-bearing claim against the code at HEAD. These are the ones that did not hold. Three described pre-fix behaviour as current design: - A parked DM "cannot settle on its own". The park offers the frame to the mesh and an arriving ACK settles it Delivered with no pending ACK to match against; the offer and the settle arm are one change. Also corrects the diagram: park is entered from Pending, Parked reaches Delivered, and a no-transport send is a deferral rather than terminal. - The desync arm "not reachable from the drain". Only the parse-failure arm is; a drained frame classifies as desync routinely and schedules the re-key, so a re-key audit must include the drain. - Confirmation triggering only on a successful decrypt. The joiner confirms on the Welcome itself, and a probe or its ack confirms too; decrypt is the trigger added so the both-create owner is covered. Permanent refusals have two dispositions, not one. A policy refusal is acknowledged; a security refusal (identity mismatch, foreign session slot) is silent and unmarks the identifier, and is intercepted before classification so it cannot inherit the policy disposition. ADR 0005, ADR 0006 and the delivery state machine each stated only the first half. Other corrections: - ADR 0003 claimed every tiebreaker compares hash bytes. Only session ownership does; leave election, admin promotion and fork leader sort rendered strings. Each converges, so the rule is per tiebreaker and the hazard is harmonizing one site onto the other order. - ADR 0001 and the envelope spec treated the media envelope as a third switch-plus-capability layer. It has neither; the third is the sealed rich payload, and the media v2 form shares rich_versions. - The rich-field restore rules described an unconditional strip that would break the legacy-sender fallback the same document relies on. - Group protocol claimed the refused-frame identifier is unmarked. The relay path does; the mesh path does not, so a replay is acknowledged. Stated as a defect against the spec, with the conforming behaviour. - Current transport is device-global, not per peer; transport status has five values; scoring has seven factors including proximity. - Relay reconciliation evaluates removes, only adds go unevaluated. - Suppression keys on (group, committer, enforced): without the flag a report-only event suppresses the refusal alarm. - Bridge contracts: the mesh-wake guard is named react_native_mesh_wake_wiring_is_present (the old name was in two source comments too), error mapping is excluded from swift test, the FFI error match is no longer compiler-exhaustive under non_exhaustive, and the Android local-test workaround is written down here instead of pointing at a guide that never had it. - Size ratio is ~2.8x for envelope plus codec, ~2.7x for the envelope alone; the priority Medium fallback is binary-only; ten magic bytes remain, not eleven; only Wi-Fi Direct and Reticulum swallow a self-addressed frame, BLE fails closed. - ADR 0012 now records mark_key_package_synced as a surviving record-only delete rather than implying none remain. No behavioural change. The two source edits are comments naming the renamed guard test.
This was referenced Aug 15, 2026
Open
Member
Author
|
Filed the two code defects surfaced by the verification pass, plus one adjacent pre-existing one, so they are not lost when this merges:
The documentation in this PR states the current behaviour for #366 and #367 rather than the intended behaviour, and marks each as a defect, so neither doc has to change again when they are fixed. |
The first verification pass over these documents found roughly two dozen statements that did not survive contact with the code, and `174016b` fixed them. A second pass, checking every load-bearing claim again, found nine more. That is the interesting part. One pass over six thousand lines of prose converges but does not finish, and the residue lands in the same classes as the first round: a rule stated without the exception that makes it interesting, and a property that holds at one site described as if it held everywhere. The worst of them is in the specification, which is the document that claims a second implementation can be built from it. It described the `__MLS_WELCOME__` body as base64. It is JSON, and has always been JSON. Base64 is what an `__MLS_ENC__` body looks like, and what the `welcome_data` field *inside* a group Welcome looks like, so the mistake is at least explicable. An implementor following the spec would have emitted frames nothing can parse. Two invariants were stated stronger than the code delivers. Outbox S3 promised a resend is re-sealed and never replayed; re-seal provenance holds plaintext, is deliberately never persisted, and an entry restored after a restart therefore replays verbatim. Session E3 promised a reset keeps queued plaintext; that is true of the desync re-key and false of the post-unblock reset, which drops the queue on purpose, because delivering messages queued before a block on the strength of an unblock is not what anyone asked for. Both now say which case they mean and why the other one differs. The group drain diagram said it fires on any successful decrypt. That is the 1:1 rule, carried into the group document, where it is wrong: the group drain fires when the epoch moves, and decrypting a message does not move the epoch. While at it, two code comments in the same family. `Address`'s ordering note instructed the reader to compare `Address` values at all four protocol tiebreakers. Exactly one of them does. The other three sort rendered strings, they each converge on their own, and "harmonizing" them is the specific change ADR 0003 exists to prevent. The comment was a standing invitation to introduce the bug the ADR forbids, sitting three files away from the ADR. A DORS comment also named a default switch hysteresis of 10, which it has not been for some time. The rest are bridge documents overstating what their guard tests cover: the TypeScript event guards pin tags, not field shapes, so a renamed event field still passes everything in the repository; one of the three Kotlin config-parser rules is pinned by a Rust guard rather than the Kotlin suite named next to it, and the Kotlin file pins literal defaults at *initial* parse, which is the opposite mechanism; and the podspec does glob `ble/` and `mesh/`, so half the rule stated above it does not apply. A document that claims more coverage than exists is worse than one that claims none, because it tells you to stop looking. No behavioural change. Comments and prose only.
Two passes over these documents fixed roughly thirty-five statements that did not survive contact with the code. A third pass, checking about four hundred claims across the spec, the state machines, the ADRs and the bridge contracts, found eighteen more. The headline one is the reason this pass was worth running. `174016b` corrected ADR 0003 and the `Address` rustdoc after finding that only one of the four protocol tiebreakers compares hash bytes; the other three sort rendered strings, and each converges on its own. It did not correct `docs/spec/identity.md`, which went on stating the blanket rule in the document that claims a second implementation can be built from it. A conforming implementor following it would have disagreed with deployed peers about who wins three of the four elections, with no way to detect the disagreement locally. That is a new error class, and it is the inverse of the one the second pass named: not a document that was never checked, but a fact that was corrected in one place and left standing in another. A correction has to be swept across every restatement, and "where else did I say this" is now the first question after fixing a claim. The rest, by area. Specification. The group refusal table justified acknowledging a refused Welcome or Commit by calling them signature-gated, in contrast with `__GRP_MLS_MSG__`. All three are gated; only `__MLS_ENC__` and `__GROUP_MSG__` are data-plane exempt. The dispositions were right and the reason was wrong, which is worse than it sounds in a column headed "Why", because a wrong reason invites the simplification the table exists to prevent. The real distinction is that a refused group message is an attribution failure, so there is no party the acknowledgement is owed to. "Base64 appears in two places only" was false: every group ciphertext field carries it, and `__MLS_ENC__` carries it only in the compact form. The reserved metadata table omitted `__ctrl_sig` and `__ctrl_pk`, so the rule forbidding applications to write reserved keys excluded the two that let an application forge the control plane. The membership report rate limit is keyed on enforcement mode as well as group and committer. State machines. Delivery and acknowledgements described a fifth outcome, `PolicyRejected`, that does not exist; a refused commit resolves to `Consumed`, as the same document's own flowchart showed ninety lines later. It also said the crypto-recovery switch gates the whole recoverable family, when session-not-ready deferral is deliberately unconditional. The outbox diagram carried a lifetime-expiry edge out of `Pending` that the sweep's own skip rule makes not the designed path, and described restart as dropping entries past the absolute cap when restore requires both windows lapsed while the in-process sweep needs either. Bridge contracts. The Kotlin document explained at length why running the tests locally fails; the predicate it blames was refined to test `node_modules/react-native/android` before this branch, and the pinned React Native ships no such directory, so the local run works. Swift said the Bluetooth manager is typechecked only by the symlink-farm harness, but CI typechecks it; two files are covered by neither, and the comment in ci.yml claiming there is one is itself stale. TypeScript stated the one-shot hold must be cleared behind a microtask yield, which names the wrong operation: removal at replay time is synchronous by design, and deferring it produces one delivery per registration. The eight-mebibyte record ceiling was filed under Keychain and Keystore, where no such limit exists, and described as "stay above" when the guard asserts equality across four sites in three languages, Python included. No behavioural change. Every claim in this commit was verified against the code, and then re-verified adversarially, which caught two wrong fixes and six overstatements before they landed.
Thirteen findings from an adversarial pass over ~570 verifiable claims. Three of them were restated in more than one document, so eighteen edits. Interop-grade: - The commit-rejection principal set said "every proposal's sender". The code scopes it to Add/Remove senders and deliberately excludes Update and PSK, because rejecting an admin's Add for batching a member's key update would fork the group. Fixed in the spec and in ADR 0009. - Priority was described as a lowercase-only closed set. The decoder also accepts the capitalized aliases the UniFFI bridges have always emitted, so a decoder built to the spec as written rejects frames this implementation produces. The threat model inverted two security dispositions its own sources state correctly: - Signature-gated refusals were said to be ACK-able. That is exactly the class the code silences; the refusal that keeps its ACK is the post-decrypt enforcement refusal, whose sender is already authenticated. Since the codebase gives the name "policy refusal" to both dispositions, the rule now says so explicitly. - Commit enforcement was called unsuitable for fleet-wide enablement. The config docs warn against enabling it on *part* of a fleet; a closed deployment is the sanctioned use. Also corrected: the "absent administrative view" phrasing that read as inverting the fail-open rule (three places), two Mermaid arms in the group message lifecycle that contradicted correct prose in the same file (the drain's missing PolicyRejected arm, and a retry edge that re-entered the logical-id mint), the claim that mesh transports enqueue unconditionally (BLE is fail-closed), the both-create owner "never receives a Welcome" premise (it never *adopts* one; three places), the forged-registration impact that omitted its correlation gate, a count of two hand-mirrored constant sets where the same section names four, "address-ordered" group tiebreakers that sort rendered strings, and two CLAUDE.md event names that do not exist.
Two fresh reviewers went over commit 1db49f7 and the doc set around it. One finding was a correction that replaced a wrong rule with another wrong rule; the rest were claims the earlier passes had not reached. The ACK rule in the threat model, again. The previous pass said an authenticated sender's permanent refusal keeps its acknowledgement. That is falsified by the group data plane: a group message is signature-gated, so its wire sender is proven, and a leaf credential that fails to bind is still refused silently. Authentication was never the discriminator. The rule now imports the one delivery-and-acks.md already stated correctly: a policy refusal is a statement about a frame, a security refusal is a statement about an attacker, and answering the second at all is the leak. Claims that contradicted the code: - ADR 0007 justified not persisting re-seal provenance with "a restart already re-derives what it needs". It does not: the field is #[serde(skip)] and a restored entry replays verbatim. The ADR now states that cost instead of arguing it away. - The spec's tri-state table filed all relay reconciliation under "not evaluated". Relay-reconciled removes do run an administrator check; only adds cannot. An implementation following the spec would have omitted a check whose absence lets any peer evict members from the fan-out cache. - The telemetry rule "never add a catch-all arm" is absolute in the document security work routes to, while its one carve-out and the condition that makes it safe (a fixed token, never the input) lived in an unlinked ADR. - message-delivery.md documented a telemetry reason field as carrying the relay's text, which is the leak the code exists to prevent, and capped retry backoff at 30s where the ladder runs to 300s. Added residual risk R9: the service prefix family is signature-gated but exempt from encryption, so discovery gossip and service request and response bodies travel in cleartext. This is the one application-supplied payload boundary 5 does not cover, and the threat model did not mention it at all. Also: three "every absent input fails open" overstatements (an unresolvable principal fails closed), R2's session-reset and never-lost claims qualified, R1's members cache named as the fan-out send cache it actually is, the forged-registration window swept into ADR 0014's triage table, two diagram edges qualified where their own prose already was, the drain's policy arm given the ack disposition its siblings state, BLE's refusal corrected from a safety net to something the transport manager's fallback defeats, the priority alias tolerance reattributed from the wire to the FFI boundary, and C5's constant-set enumeration reconciled with its own count in both places it appears.
Third and final correction round. Two reviewers went over 4a9fe0c; one approved the PR outright, the other found that the rewritten acknowledgement rule had picked up a new wrong qualifier. The acknowledgement rule, once more. Calling the identity bindings "post-decrypt" is wrong for the session slot binding, which runs before any AEAD precisely because every failure the MLS library raises below it happens before it authenticates anything. R2 of the same document already said so. The bullet also named three identity bindings where the code and delivery-and-acks.md name four. Both fixed, and the policy row now says its acknowledgement is an arrival-path rule: a frame that reaches the group buffer and later resolves as a policy refusal is never acknowledged, which is what group-message-lifecycle.md says one file over. R1 understated a forged membership answer. The members cache it corrupts also feeds the sealed rich payload gate, which requires every non-self member to be known rich-capable. An attacker-chosen identifier is not, so the gate closes and reply context and forward attribution move from inside the MLS AEAD to hop-visible cleartext while media secrets are dropped. That defeats a control this document lists by name, reached by the adversary that control names. The spec claimed a working relay-side remove path. The check is right and required, but a genuinely relay-originated remove is injected unattributed, so its sender can never be an administrator and the frame is always dropped: relay-native remove reconciliation is inert, and the live path is the removing administrator's own signed notification. Stated as the consequence of the rule that it is. Recovered the one thing the restructure actually lost: the leaf identity binding's mutation-check inventory, ten named sabotages plus the adversarial fixture behind the mls crate's test-utils feature, without which the use-time seam cannot be tested at all. ADR 0010 also gains the adopt-path ordering rule and the "what would undo this" section every other ADR carries. Also: the BLE fallback caveat swept into its two restatements, R9 no longer claims a design nobody can point at, the outbox repark edge qualified to plain DMs, and C5 notes that C6's retry fallbacks are a fifth set of the same shape.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CLAUDE.mdhad grown to 61 KB in a handful of paragraph-long bullets. It was unreadable by people, unreviewable in a diff, and the only home for knowledge that several parts of the codebase depend on.CHANGELOG.mdhad the same problem for a different reason: 2,425 lines with no archiving story.This relocates the durable knowledge into documents that can be reviewed, linked and cited, and cuts
CLAUDE.mdto repository instructions plus a pointer table: 61,107 → 7,606 bytes.New reference material
docs/spec/docs/security/threat-model.mddocs/state-machines/docs/adr/docs/bridges/Changelog archiving
CHANGELOG.mdnow holds unreleased changes plus the current release; older releases move todocs/changelog/, one file per minor series with its own release table.All 31 release sections are accounted for, none duplicated and none lost, verified by diffing the old and new release bodies section by section. Content is otherwise byte-identical apart from a blank line at the split boundary and three
docs/UPGRADING.mdlinks in the 0.19 and 0.20 sections, which had to become../UPGRADING.mdto keep resolving from their new depth. No CI job or script parses the changelog, so the split is safe.This adds a step to the release cut: after cutting, move the now-previous release's section into
docs/changelog/<major>.<minor>.mdand update both archive tables. The procedure is written down in the archive index.Related issues
None.
Type of change
docs— documentation onlyChecklist
cargo fmt --all -- --checkpassescargo clippy --workspace -- -D warningspassescargo test --workspacepasses (2,132 tests, 0 failures, doctests included)cargo-denyunaffected (no dependency, license or manifest changes)CHANGELOG.mdupdatedunsafeBreaking changes
None. No behavioural change of any kind. The non-markdown edits are all comments: five Rust doc comments that pointed at
CLAUDE.mdsections which no longer exist there and now name the specific document, plus one TypeScript and one Kotlin comment in174016bthat named a guard test by a stale name.Notes for reviewers
Deliberately not carried over: the per-feature
Coverage: test_x, test_y, ...lists. They were the most staleness-prone part of the old file, and enumerating test names in prose recreates exactly the problem this change is addressing. The docs name the behaviour that is pinned instead, and grep finds the tests. Happy to add a generated test index if reviewers want that back, but it should not be hand-maintained prose.One claim in the old
CLAUDE.mdwas already stale, which is itself an argument for the move: it described the wire v1 extension TLV registry as having only tag 1, butwire.rshas tag 2 (reply context) as well. The spec documents both, along with the rule that distinguishes them: tag 1 shipped in v1's first release so its absence would truncatecontent, which fixes the constraint that no future v1 tag may change meaning by being ignored.Where to focus review. The docs are long, but the load-bearing parts are the ones that describe shapes which are correct only as a whole, since those are what a future change is most likely to undo by accident:
Corrections welcome on any of these: the docs are an attempt to state the invariants precisely, and if one is stated wrong it is better caught now than cited later.
Every relative link and anchor in the new and edited files was verified to resolve.
Review corrections (
174016b)Every load-bearing claim in the new documents was verified against the code at HEAD. The review found roughly two dozen statements that did not hold, all now fixed in
174016b. The classes worth calling out, since they say something about where this kind of document goes wrong:Three stated pre-fix behaviour as current design. The parked-DM section said a parked message "cannot settle on its own", which was true before the offer-plus-settle pair landed; the delivery machine said the desync arm was unreachable from the drain, where it is in fact the routine case; the session machine said confirmation triggers only on a successful decrypt, when the joiner confirms on the Welcome itself. Prose that describes the bug rather than the fix is the failure mode this restructure is most exposed to, because the fix and the document were written at different times.
One security property was stated backwards in three places. Permanent refusals have two dispositions: a policy refusal is acknowledged, a security refusal is silent and unmarks the identifier. ADR 0005, ADR 0006 and the delivery state machine each carried only the first half, which would have licensed exactly the change the interception exists to prevent. The delivery document's own media section had it right, so the file contradicted itself.
ADR 0003's tiebreaker rule was wrong for three of its four named sites. Only session ownership compares hash bytes; leave election, admin promotion and fork leader sort rendered strings. Each converges on its own, so the correct invariant is per tiebreaker, and the hazard the ADR should have named is harmonizing one site onto the other order.
A bridge contract named a guard test that does not exist.
react_native_mesh_wake_task_key_matches_nativeis reallyreact_native_mesh_wake_wiring_is_present; the stale name was in two source comments as well, so those are fixed in the same commit. Also in that family:swift testdoes not cover error mapping (the suite is excluded from the package manifest), the FFI error match is no longer compiler-exhaustive now that the engine error types are#[non_exhaustive], and the Android local-test workaround now lives indocs/bridges/kotlin.mdinstead of pointing at a guide that never contained it.Two of the findings are code defects rather than documentation defects, and are documented as such rather than papered over:
docs/spec/group-protocol.mdstates the conforming behaviour and marks the current mesh behaviour a defect.mark_key_package_synceddeletes a key package record without purging its provider key, which ADR 0012 presented as a removed leak. It is now recorded as a surviving exception with the reasoning for and against.Both want their own issues; neither belongs in a docs-only PR.
No behavioural change in this PR.
cargo fmt,RUSTDOCFLAGS="-D warnings" cargo doc, the 24 React Native pinning guards, and the TypeScript typecheck all pass on the corrections.