XIP-82: External commit invites - #138
Conversation
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. New XIP specification document in a directory owned by another user (jhaaaa). Three unresolved review comments identify issues with step ordering, timestamp convergence, and clock-skew handling in the specification that warrant resolution by the designated owner. You can customize Macroscope's approvability policy. Learn more. |
Shareable group invites enabling QR-code or link-based joins via atomic MLS External Commits with admin-controlled policy. Also adds HPKE, NAPI, and openmls to the cspell dictionary.
1ac135e to
edf4369
Compare
…g, max_uses, confidential-components direction (#140) * XIP-82: review revisions — typed newtypes, versioned-slot anti-poisoning, max_uses cap, confidential-components direction * XIP-82: implied-grant + blob-refresh model + review-round fixes allow_external_commit itself authorizes the joiner-self-entry insert (implied grant); external_committer_permissions reduced to a reserved deny-by-default surface. Rework blob refresh to committer-immediate (primary, no jitter) + member existence-check backstop + rare deep-check self-heal, justified by the group_state_hash unforgeability (witness) argument. Add service-contract rules for (epoch, group_state_hash) lookup and an existence query; align join-side step 10, the overview diagram, and the service-visible-metadata note. Address the open Macroscope review threads: measure expire_in_ns from epoch start (not GroupInfo export); join-side selection iterates descending-epoch through all survivors (lower-epoch fallback); align the key-holder-poison wording with the redirection limitation; document the joiner local-clock vs envelope-timestamp skew; specify opaque refresh_pointers as application-mediated; clarify admitted_via is the GroupInfo's active id (epoch-bound, so rotation can't break in-flight joins); forbid cross-origin redirects; resolve the test-cases-vs-follow-ups contradiction.
| 9. When `EXTERNAL_COMMIT_POLICY.expire_in_ns != 0`: the commit's envelope timestamp minus the current epoch's start timestamp does not exceed `expire_in_ns`. The epoch-start timestamp is the envelope timestamp of the message by which the validator entered or observed the current epoch — the epoch's commit, for members that processed it; the corresponding `Welcome` (published together with that commit), for members added at that epoch; the group-creation timestamp at the initial epoch. These differ across members only by publish latency (seconds), and not at all by *when* a validator processes the commit — so `expire_in_ns` is a coarse bound and SHOULD be set to values (minutes or more) for which that skew is immaterial. |
There was a problem hiding this comment.
🟡 Medium XIPs/xip-82-external-commit-invites.md:586
Check 9 defines the epoch-start timestamp as the envelope timestamp of the message by which each validator entered the epoch — the commit for existing members, but the Welcome for members added at that epoch. The document itself admits these timestamps "differ across members only by publish latency (seconds)." For a small but legal expire_in_ns (e.g., a few seconds), an existing member computes commit_ts − commit_epoch_start and rejects the external commit as stale, while a member added via Welcome at that same epoch computes commit_ts − welcome_epoch_start (a smaller delta) and accepts it. This violates the stated guarantee that "the accept/reject decision is therefore deterministic and convergent across the membership," splitting the group. Consider defining the epoch-start timestamp as a single canonical value all members agree on — the commit's envelope timestamp — rather than the per-validator entry message.
+9. When `EXTERNAL_COMMIT_POLICY.expire_in_ns != 0`: the commit's envelope timestamp minus the current epoch's start timestamp does not exceed `expire_in_ns`. The epoch-start timestamp is the envelope timestamp of the **commit that began the current epoch** — the same value for every member, regardless of how they entered the epoch. Members added via `Welcome` at that epoch obtain the commit's timestamp from the delivery topic (the commit is published alongside the `Welcome`); at the initial epoch it is the group-creation timestamp. Because every validator uses the same epoch-start value — not the `Welcome` timestamp or any per-validator entry time — the staleness verdict is identical across the membership regardless of publish latency or when a member syncs.🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @XIPs/xip-82-external-commit-invites.md around line 586:
Check 9 defines the epoch-start timestamp as the envelope timestamp of the message by which each validator *entered* the epoch — the commit for existing members, but the `Welcome` for members added at that epoch. The document itself admits these timestamps "differ across members only by publish latency (seconds)." For a small but legal `expire_in_ns` (e.g., a few seconds), an existing member computes `commit_ts − commit_epoch_start` and rejects the external commit as stale, while a member added via `Welcome` at that same epoch computes `commit_ts − welcome_epoch_start` (a smaller delta) and accepts it. This violates the stated guarantee that "the accept/reject decision is therefore deterministic and convergent across the membership," splitting the group. Consider defining the epoch-start timestamp as a single canonical value all members agree on — the commit's envelope timestamp — rather than the per-validator entry message.
| One clock caveat: check 3.2 compares against the joiner's local `now_ns`, whereas validators evaluate the policy time bounds against delivery-service envelope timestamps (receive-side checks 8–9). A joiner whose clock lags the delivery service can therefore accept a blob already past its bound in the envelope-timestamp domain and still hit the zombie join. The window is bounded by the joiner's clock skew: clients SHOULD keep their clock synchronized (e.g. NTP) and MAY subtract a small margin from `now_ns`, and admins SHOULD set `expire_in_ns` well above plausible client skew (it is a coarse, minutes-or-more bound anyway — see check 9). |
There was a problem hiding this comment.
🟡 Medium XIPs/xip-82-external-commit-invites.md:609
Step 3.2's clock-skew caveat says clients MAY subtract a margin from now_ns, but the expiry check is now_ns < blob.expires_at_ns. Subtracting from now_ns makes the check less strict (the blob appears less expired), which increases the zombie-join risk the paragraph is trying to prevent. The margin should go in the opposite direction — add it to now_ns so a lagging client rejects blobs that are closer to expiry.
| One clock caveat: check 3.2 compares against the joiner's local `now_ns`, whereas validators evaluate the policy time bounds against delivery-service envelope timestamps (receive-side checks 8–9). A joiner whose clock lags the delivery service can therefore accept a blob already past its bound in the envelope-timestamp domain and still hit the zombie join. The window is bounded by the joiner's clock skew: clients SHOULD keep their clock synchronized (e.g. NTP) and MAY subtract a small margin from `now_ns`, and admins SHOULD set `expire_in_ns` well above plausible client skew (it is a coarse, minutes-or-more bound anyway — see check 9). | |
| + One clock caveat: check 3.2 compares against the joiner's local `now_ns`, whereas validators evaluate the policy time bounds against delivery-service envelope timestamps (receive-side checks 8–9). A joiner whose clock lags the delivery service can therefore accept a blob already past its bound in the envelope-timestamp domain and still hit the zombie join. The window is bounded by the joiner's clock skew: clients SHOULD keep their clock synchronized (e.g. NTP) and MAY add a small margin to `now_ns`, and admins SHOULD set `expire_in_ns` well above plausible client skew (it is a coarse, minutes-or-more bound anyway — see check 9). |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @XIPs/xip-82-external-commit-invites.md around line 609:
Step 3.2's clock-skew caveat says clients MAY subtract a margin from `now_ns`, but the expiry check is `now_ns < blob.expires_at_ns`. Subtracting from `now_ns` makes the check *less* strict (the blob appears less expired), which increases the zombie-join risk the paragraph is trying to prevent. The margin should go in the opposite direction — add it to `now_ns` so a lagging client rejects blobs that are closer to expiry.
Summary
Introduces External Commit Invites: a shareable invite token (QR / link / NFC) that lets a non-member join an XMTP group on their own via an atomic MLS External Commit (RFC 9420 §12.4.3.2), without any existing member being online at scan time.
The mechanism is a two-piece design:
ExternalInvitePayload— small shareable capability (key + service slot id + opaque service pointer). What's encoded in the QR.EncryptedGroupInfoBlob— large encrypted MLS state held by an out-of-band service. Carries plaintextepoch+group_state_hashso the service can totally-order uploads by epoch and reject equal-epoch forks without ever seeing decrypted GroupInfo.Admin control lives in a new well-known AppData component:
EXTERNAL_COMMIT_POLICY— runtime-toggleable master switch plus the active invite'ssymmetric_keyandexternal_group_id. Storing the key in group state is what lets a printed QR survive epochs — any just-joined member can re-export and re-upload under the same key without the issuing admin being online.Per-component declarative authorization for external committers lives in a new field on
ComponentMetadata:ComponentMetadata.external_committer_permissions— symmetric twin of the existingpermissionsfield, evaluated against external commits. Absent = all-Deny.Lifecycle invariants spelled out in the spec: enable must atomically populate the invite coordinates, revoke must atomically clear them, re-enable must use fresh material (no key revival).
Discussion
I've raised the design with the team in #review-napkin over the past week. Key decisions captured in the Rationale and "Alternatives considered" sections:
external_group_id(≥4 bytes, 16 random RECOMMENDED) — not a hash of the group_id. AEAD already prevents the swap-ciphertext attack.Reference implementation
libxmtp PR stack (open):
MlsGroup::create_external_invite+Client::join_group_by_external_inviteProto: xmtp/proto#333 (generic intent), xmtp/proto#334 (this XIP's wire format).
openmls: upstream PR in flight; XMTP fork carries the patch.
Test plan
cspellagainst the repo dictionary).🤖 Generated with Claude Code
Note
Add XIP-82 specification for external commit invites in XMTP groups
Adds xip-82-external-commit-invites.md, a new XMTP Improvement Proposal defining how external clients can join MLS groups without prior group membership.
ExternalInvitePayloadandEncryptedGroupInfoBlobfor carrying group secrets to external joinersExternalCommitPolicypermission governing whether external commits are allowedChanges since #138 opened
SymmetricKey,GroupStateHash, andServicePointerto replace raw bytes fields in the wire format section, updatedExternalInvitePayloadV1.service_pointerto use optional typed pointer andsymmetric_keyto useSymmetricKey, and updatedEncryptedGroupInfoBlobV1.group_state_hashto useGroupStateHash[b702cb0]ExternalCommitPolicyV1semantics withmax_usesfield,refresh_pointersfield, enforcement and lifecycle rules, clarifiedallow_external_commiton/off invariants requiring absence of per-invite fields on revoke, changedsymmetric_keytoSymmetricKeytype, clarifiedexpire_in_nsorigin as epoch start, and updated thatallow_external_commitauthorizes the joiner's ownGROUP_MEMBERSHIPwrite [b702cb0]GROUP_MEMBERSHIPentry withadmitted_via_external_group_id, disallow PSK proposals, and added immutability rules for the admitted-via tag [b702cb0]admitted_viatag equality to the activeexternal_group_id, and enforcemax_usesby counting tagged membership entries [b702cb0]GroupInfoconsistency checks, select freshest valid candidate with fork fallthrough, execute pre-commit policy checks, publish the commit, and conditionally refresh with rotation race handling, including resource-bound and fetch-hygiene guidance [b702cb0]EncryptedGroupInfoBlobV1specifying that metadata is AAD-bound, nonce generation requirements, and definingexpires_at_nsas the effective expiry derived from policy bounds [b702cb0]group_state_hashwitness [b702cb0]external_group_id[b702cb0]max_usesis based on membership entries, and introduced a design sketch for confidential components as future work [b702cb0]max_usesenforcement, member refresh flows, and revoke-clears checks [b702cb0]unforgeableto the spell-check dictionary [b702cb0]📊 Macroscope summarized b702cb0. 2 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.