Skip to content

feat(dm): portable sent DMs — seal a sender copy into each DM (#432)#433

Draft
sanity wants to merge 2 commits into
mainfrom
worktree-feat-portable-sent-dms
Draft

feat(dm): portable sent DMs — seal a sender copy into each DM (#432)#433
sanity wants to merge 2 commits into
mainfrom
worktree-feat-portable-sent-dms

Conversation

@sanity

@sanity sanity commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Problem

An in-room DM is stored in the room contract as ECIES ciphertext encrypted only to the recipient. The sender cannot decrypt their own sent DM from contract state, so historically the sender's plaintext lived only in a device-local chat-delegate cache (outbound_dms, #256). That cache never travels the network, so on a new node — or after a delegate-key change strands it (the migration gate only probes outbound_dms when the current delegate has no rooms) — the user sees sent — ciphertext only instead of their own sent messages.

Reported by Ivvor: "you can't read your own sent DMs any more on the new node." Closes #432.

Approach

compose_direct_message now seals a second ECIES envelope of the same body to the sender's own member_vk, carried in a new DirectMessage.sender_ciphertext: Option<Vec<u8>>. Any device holding the sender's signing key recovers its own sent DMs from contract state via open_own_direct_message, exactly the way received DMs already work. The UI (dm_thread_modal) and CLI (riverctl dm list) fall back to this sender copy on an outbound-cache miss before showing the placeholder.

Why a struct field and not a combined envelope inside the opaque ciphertext: unseal_dm_from_sender consumes everything after byte 44 as the AES-GCM ciphertext, so appending a second envelope would break old recipients. A separate signed field is the only backwards-compatible carrier — at the cost of a room-contract wire change.

Wire-format discipline

  • Option, #[serde(default, skip_serializing_if = "Option::is_none")], placed last, so a pre-change message serializes byte-identically and old state still decodes / re-PUTs (permissionless migration intact).
  • Covered by the sender signature: appended to build_direct_message_signed_bytes only when Some, so a legacy (None) message signs over the exact pre-change bytes and still verifies, and the copy can't be stripped or grafted without invalidating the signature.
  • Size-capped to MAX_DM_CIPHERTEXT_BYTES in both verify and apply_delta.

Forward-looking: recovers sent DMs authored after this change on any device; it does not resurrect DMs sent before it (they carry no sender copy). Historical recovery on the same node would need the separate outbound-cache migration-gate fix.

Migration

Adding the field changes both the room-contract and chat-delegate WASM (both compile ChatRoomStateV1), so both re-key. V28 registry entries added to legacy_delegates.toml and common/legacy_room_contracts.toml with the pre-change hashes; WASMs rebuilt via sync-wasm. river-core 0.1.15 → 0.1.16, riverctl 0.1.79 → 0.1.80.

Testing

New tests in common/tests/direct_messages_test.rs:

  • signature back-compat (None layout == legacy prefix), signature coverage (strip/graft both fail), verify + apply_delta size caps
  • end-to-end sender-copy round-trips: sender reads own DM with only their key; recipient still reads; third party reads neither; legacy message → open_own errs

Also closes a pre-existing CI gap: common/tests/direct_messages_test.rs never ran in CI (only --lib + specific --test targets did), so the whole DM verify/apply_delta/compose surface was ungated. Added a build.yml step running it with --features ecies-randomized.

Green locally: 62 DM integration tests, 229 river-core lib, 479 river-ui bin; migration + delegate-key + wasm-sync + wbindgen checks; fmt; clippy (no new warnings).

Not done here (deploy-time, needs your call)

  • Draft pending multi-model review (high-risk wire/crypto/contract surface — Full tier).
  • Republishing the River UI (new contract key + delegate migration ritual) and releasing riverctl are deploy steps, not in this PR.

[AI-assisted - Claude]

🤖 Generated with Claude Code

https://claude.ai/code/session_01RKvh1NuNCx4ucxsXPPRNZP

sanity and others added 2 commits July 21, 2026 11:47
## Problem

An in-room DM is stored in the room contract as ECIES ciphertext encrypted
ONLY to the recipient. The sender cannot decrypt their own sent DM from
contract state, so historically the sender's plaintext lived only in a
device-local chat-delegate cache (`outbound_dms`, #256). That cache never
travels the network, so on a new node — or after a delegate-key change strands
it (per the migration gate) — a user sees "sent — ciphertext only" instead of
their own sent messages. Reported by Ivvor: "you can't read your own sent DMs
any more on the new node." Closes #432.

## Approach

`compose_direct_message` now seals a SECOND ECIES envelope of the same body to
the sender's own `member_vk`, carried in a new
`DirectMessage.sender_ciphertext: Option<Vec<u8>>`. Any device holding the
sender's signing key recovers its own sent DMs from contract state via
`open_own_direct_message`, exactly the way received DMs already work. The UI
(`dm_thread_modal`) and CLI (`riverctl dm list`) fall back to this sender copy
on an outbound-cache miss before showing the placeholder.

Wire-format discipline:
- The field is `Option`, `#[serde(default, skip_serializing_if = "Option::is_none")]`,
  placed last, so a pre-#432 message serializes byte-identically and old state
  still decodes/re-PUTs (permissionless migration intact).
- It is covered by the sender signature: appended to
  `build_direct_message_signed_bytes` only when `Some`, so a legacy (`None`)
  message signs over the exact pre-#432 bytes and still verifies, and the copy
  cannot be stripped or grafted without invalidating the signature.
- Size-capped to `MAX_DM_CIPHERTEXT_BYTES` in both `verify` and `apply_delta`.

Forward-looking: this recovers sent DMs authored AFTER this change on any
device; it does NOT resurrect DMs sent before it (they carry no sender copy).

## Migration

Adding the field changes both the room-contract AND chat-delegate WASM (both
compile `ChatRoomStateV1`), so both re-key. Registry entries V28 added to
`legacy_delegates.toml` and `common/legacy_room_contracts.toml` with the
pre-change hashes; WASMs rebuilt via `sync-wasm`. river-core bumped 0.1.15 →
0.1.16 and riverctl 0.1.79 → 0.1.80 (embeds the new room_contract.wasm).

## Testing

- New tests in `common/tests/direct_messages_test.rs`: signature back-compat
  (None layout == legacy), signature coverage (strip/graft both fail), verify
  + apply_delta size caps, and end-to-end sender-copy round-trips (sender reads
  own DM with only their key; recipient still reads; third party reads neither;
  legacy message → open_own errs).
- Closed a pre-existing CI gap: `common/tests/direct_messages_test.rs` never ran
  in CI (only `--lib` + specific `--test` targets did). Added a build.yml step
  running it with `--features ecies-randomized`.
- Green locally: 62 DM integration tests, 229 river-core lib, 479 river-ui bin;
  migration + delegate-key + wasm-sync + wbindgen checks; fmt; clippy (no new
  warnings).

Note: not published/deployed — republishing the River UI (new contract key +
delegate migration) and releasing riverctl are deploy-time steps.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RKvh1NuNCx4ucxsXPPRNZP
Review follow-ups on the #432 sender-copy change (crypto/wire/migration core
unchanged — three adversarial reviewers verified it independently):

- Outbound invite self-copies rendered as full interactive Accept CARDS on a
  cacheless node (via the shared inbound closure), breaking the documented
  "BodyKind::Invite is inbound-only" invariant and showing the sender an
  enabled Accept button on their OWN sent invite. Now they render as the same
  "[Invitation] …" summary the outbound cache stores. Extracted the pure,
  unit-tested `resolve_outbound_dm_body` (cache-hit → self-copy → placeholder;
  self-copy decrypted only on a cache miss) and reverted the inbound path to
  its own `render_inbound_body` closure with the restored can_participate
  rationale comment.
- riverctl `dm list`: the "you have an invitation, run `dm accept`" tip now
  excludes OUTBOUND invites (`is_invite && !outgoing`) — since #432 a sent
  invite decodes to is_invite on a cacheless node, but `dm accept` is
  inbound-only, so the tip led nowhere.
- Fixed three stale `#431` references (an unrelated PR) → `#432` in
  dm_thread_modal.rs that an earlier sweep missed.
- Documented the doubled-ciphertext bound in the module threat-model docs.
- Added river-ui unit tests for `resolve_outbound_dm_body` (cache-hit,
  text self-copy, invite→summary, legacy→placeholder) — the render fallback
  was previously untested at the crate level.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RKvh1NuNCx4ucxsXPPRNZP
@sanity

sanity commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Multi-model review (Full tier)

External-model substitution (recorded per policy): Codex was invoked (codex review --base main) but hit its usage limit (You've hit your usage limit … try again Jul 25); the Gemini CLI tier is deprecated/unavailable. With both external models down and the reset several days off, I used the diverse-Claude-subagent fallback: three independent adversarial reviewers, each blind to the others, with distinct lenses — (1) crypto + wire-compat, (2) code-first correctness, (3) big-picture / test-coverage. Re-running the real external pass before merge is advisable (Codex resets Jul 25).

Consensus

All three independently confirmed the crypto, signature coverage, serde back-compat, and migration are sound. Two reviewers reproduced the V28 migration hashes by hand and confirmed the committed wire-format snapshot (direct_messages_wire_format.hex) is byte-unchanged vs main (legacy None messages sign/serialize identically). CI's check-delegate-migration, check-room-contract-migration, and check-wasm-sync agree.

Findings and disposition (all addressed in e785c9b)

# Sev Finding Fix
F1 Med riverctl dm list "you have an invitation" tip counted OUTBOUND invites (sender-copy fallback decodes them), but dm accept is inbound-only → tip led nowhere tip now is_invite && !outgoing
F2/4a Med Outbound invite self-copies rendered as full Accept cards (shared inbound closure), breaking the BodyKind::Invite-inbound-only invariant + showing the sender an enabled Accept button on their own invite outbound invites now render as the [Invitation] … summary via the pure, unit-tested resolve_outbound_dm_body; inbound reverted to its own render_inbound_body
6 Low Three #431 refs (unrelated PR) in dm_thread_modal.rs #432
4b Low Refactor dropped the can_participate() rationale comment restored
F3 Nit Module threat-model doc didn't note the doubled ciphertext documented
Cov Med Render fallback untested at crate level added 4 river-ui unit tests for resolve_outbound_dm_body

Deferred (tracked, not fixed here)

No blocking findings remain. Draft pending CI (build + playwright) and your call on deploy (republish + delegate migration + riverctl release are deploy-time steps).

[AI-assisted - Claude]

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.

Can't read your own sent DMs on a new node (sender plaintext is device-local only)

1 participant