diff --git a/IMPLEMENTATION_DOCS.md b/IMPLEMENTATION_DOCS.md new file mode 100644 index 0000000..621f47e --- /dev/null +++ b/IMPLEMENTATION_DOCS.md @@ -0,0 +1,53 @@ +# Implementation docs + +Local index of the documents that track what this codebase actually implements. +Several docs under `docs/` and `apps/*/docs/` state "cross-referenced from +`IMPLEMENTATION_DOCS.md`" — this is the file they mean. + +Note: this file is listed in `.gitignore`, so it normally lives only in the +maintainer's working tree. It is committed here so the ADR index has a stable +cross-link from a repo-root document; if it should stay local, paste the +"Decisions" block below into your own copy and drop the file. + +## Decisions + +- [docs/adr/README.md](./docs/adr/README.md) — Architecture Decision Records: + the index, the template, and the backfilled decisions (one `devices` table, + `(createdAt, id)` ordering, ciphertext-only storage, per-device envelopes, the + sealed-box → Signal migration path, MLS for groups), plus the two decisions + that were shipped and superseded. + +## Architecture and operations + +- [docs/threat-model.md](./docs/threat-model.md) — what the gateway and its + stores can see, and the trust boundary. +- [docs/runbook.md](./docs/runbook.md) — scaling, failure modes, incident + response for the backend gateway. +- [docs/observability.md](./docs/observability.md) — metrics, logs and tracing. +- [docs/group-epoch-sync.md](./docs/group-epoch-sync.md) — the group-control + sequence log and how a client catches up. +- [docs/signal-integration.md](./docs/signal-integration.md) — Signal library + evaluation and the client-side crypto split. +- [docs/security/](./docs/security/) — rate limits, TLS/pinning, audit logging. + +## Backend + +- [apps/backend/docs/](./apps/backend/docs/) — REST/socket API references, + contract schemas, E2EE onboarding, device and prekey APIs, delivery fan-out, + push/storage jobs, security hardening. +- E2EE specifics: `e2ee-onboarding.md`, `message-encryption-migration.md` + (ciphertext-only cutover), `signal-migration.md` (Phase-1 → Signal), + `mls-group-membership.md`, `mls-group-files.md`, `mls-key-packages.md`. + +## Web + +- [apps/web/docs/](./apps/web/docs/) — client concepts (E2EE stack, message + pipeline, file encryption, auth/device lifecycle, local search) and contract + references (response types, IndexedDB schemas, REST/WebSocket clients). + +## AI agent and contracts + +- [apps/ai_agent/docs/](./apps/ai_agent/docs/) — chat, index search, transfer + analysis, RAG architecture, Weaviate schema. +- [contracts/docs/](./contracts/docs/) — Soroban contract APIs. +- [contracts/README.md](./contracts/README.md) — contract workspace overview. diff --git a/docs/adr/0000-template.md b/docs/adr/0000-template.md new file mode 100644 index 0000000..209d7b9 --- /dev/null +++ b/docs/adr/0000-template.md @@ -0,0 +1,52 @@ +# ADR-NNNN: + +- **Status:** proposed | accepted | superseded by ADR-NNNN (drop it if nothing supersedes this) +- **Date:** YYYY-MM-DD (for backfilled records: the date the record was written, with the note that the decision predates it) +- **Supersedes:** ADR-NNNN (drop the line if nothing, and link the replacement + record when there is one) + +## Context + +What forced a decision. Two or three short paragraphs at most: the constraint, the +thing that broke or would have broken, and the options that were actually on the +table. Link the docs, schema or code the decision is about, with file paths. + +Write down what the codebase looked like *before* the decision. An ADR whose +context only makes sense to someone who was in the review thread is not doing +its job. + +## Decision + +What was chosen, in the present tense, as a rule somebody can follow. If the +decision is enforceable in code (a check constraint, a validator, a rule in the +send path), say where. + +## Consequences + +What this buys, and what it costs. Both are required — an ADR with only upsides +is missing the part that will matter in six months. + +- Positive consequence. +- Negative consequence, or the thing this makes harder later. +- Follow-on work this created, with the issue number if there is one. + +## Alternatives rejected + +One bullet per alternative, each with the reason it lost. If the rejection +reason is not recorded anywhere in the repo and you are reconstructing it, +say so explicitly instead of writing a plausible rationale — an unverified +"we rejected X because Y" is worse than "the record does not say why X was +rejected". + +- **Alt A** — why it lost. +- **Alt B** — why it lost. + +## Sources + +Everything a reader needs to check this record against the code: + +- schema: `apps/backend/src/db/schema.ts` (``) +- migration: `apps/backend/drizzle/NNNN_....sql` +- implementation: `path/to/file.ts` +- tests: `apps/backend/src/__tests__/.test.ts` +- discussion: issue #NNN, PR #NNN (full links) diff --git a/docs/adr/0001-single-devices-table.md b/docs/adr/0001-single-devices-table.md new file mode 100644 index 0000000..c9efc52 --- /dev/null +++ b/docs/adr/0001-single-devices-table.md @@ -0,0 +1,91 @@ +# ADR-0001: One canonical `devices` table + +- **Status:** accepted +- **Date:** backfilled 2026-09-23 (decision predates this record) + +## Context + +Two device-shaped concerns arrived at different times: auth/prekey identity +(a device has an Ed25519 identity key, publishes prekeys, gets revoked) and +realtime/push plumbing (a device has a socket, a last-seen timestamp, a Web Push +subscription, and is an envelope recipient). + +The first pass gave the identity side its own table, +`user_devices` (PR #206), with `GET /devices` listing from it (PR #207). The +second pass needed "which devices should this envelope be fanned out to, and +which of them are online" — answered from a different table. Nothing kept the +two in sync: a device could be revoked in one and still be an envelope +recipient in the other, and a sibling-device coverage check +(`services/messageFanout.ts`, `fetchSiblingDeviceIds`) could pass while the +realtime layer delivered to a device the auth layer considered gone. + +## Decision + +There is exactly one device registry, `devices`, and every other device-shaped +fact is a column on it or a foreign key into it. + +- `devices` carries identity (`identity_public_key`, `registration_id`), + liveness (`last_seen_at`, `push_enabled`, `revoked_at`, `stale_flagged_at`), + and capability advertisement (`capabilities`). +- `messages.sender_device_id`, `message_envelopes.recipient_device_id`, + `push_subscriptions.device_id`, `device_prekeys.device_id`, + `device_key_history.device_id`, and the `mls_*` tables all FK into `devices`. +- `routes/userDevices.ts` is kept as a legacy alias route exposing only a + public-key lookup, not a second store. +- Uniqueness is per `(user_id, identity_public_key)` — the same identity key + re-registering is the same device, not a new row. + +## Consequences + +- Revocation is one write with one meaning: `revoked_at` on `devices` is + respected by the auth layer, the envelope fan-out path + (`isNull(devices.revokedAt)` in `lib/messageFanout.ts`, + `services/deliveryPipeline.ts`, `services/fanout.ts`, + `services/pushFilter.ts`) and push at the same time. There is no window + where the tables disagree. +- `revoked_at` is a timestamp rather than a boolean, so revocation keeps audit + history and records *when* it happened. +- The table is now on the hot path for auth, messaging, realtime and push. Every + new device-shaped requirement adds a column to it, and a partial index + (`devices_user_id_active_idx`) exists so the common "active devices for this + user" query does not have to scan revoked rows. +- Deleting a device row cascades to prekeys, envelopes and push subscriptions; + `DELETE /devices/:id` therefore revokes instead of deleting. + +## Alternatives rejected + +- **A separate `user_devices` table for identity, `devices` for realtime** + (shipped in PR #206, then merged away). Rejected because the two tables could + silently diverge: a revoked device would keep receiving envelopes. The + comment in `routes/devices.ts` records the merge explicitly. +- **A boolean `is_revoked` instead of `revoked_at`.** Rejected in favour of the + timestamp, which preserves audit history and supports the device-GC staleness + window. The frontend `Device.isRevoked` type is a leftover of the boolean + shape and is documented as drift (`apps/web/docs/contracts-response-types.md`, + DRIFT-2). +- **Deleting revoked device rows** rather than keeping them with `revoked_at`. + Rejected: it would cascade-delete audit-relevant history and orphan + envelope/prekey rows. + +## Sources + +- schema: `apps/backend/src/db/schema.ts:220-270` (`devices`, `devicePrekeys`) +- routes: `apps/backend/src/routes/devices.ts:1-7` (merge rationale), + `apps/backend/src/routes/userDevices.ts` +- migration: `apps/backend/drizzle/0000_lean_scrambler.sql` (`CREATE TABLE + "devices"`, `devices_user_identity_idx`, `devices_user_id_active_idx`) +- docs: `apps/backend/docs/api-devices.md:14-18` +- discussion: PR #206 (`user_devices` identity schema, later merged away), + PR #207 (`GET /devices`); issues #103/#107 are cited by the route comment + +Verified by: + +```sh +grep -rn "devices" apps/backend/drizzle/0000_lean_scrambler.sql +``` + +which shows `messages.sender_device_id`, +`message_envelopes.recipient_device_id`, `push_subscriptions.device_id`, +`device_prekeys.device_id` and the `mls_*` tables all referencing +`public.devices(id)`, and by the absence of any other `%device%` table in the +create statements. diff --git a/docs/adr/0002-message-ordering-by-createdat-id.md b/docs/adr/0002-message-ordering-by-createdat-id.md new file mode 100644 index 0000000..7e8dcc8 --- /dev/null +++ b/docs/adr/0002-message-ordering-by-createdat-id.md @@ -0,0 +1,101 @@ +# ADR-0002: Order messages by `(createdAt, id)`, not a per-conversation sequence counter + +- **Status:** accepted (supersedes [ADR-0007](./0007-per-conversation-sequence-numbers.md)) +- **Date:** backfilled 2026-09-23 (decision predates this record) +- **Supersedes:** ADR-0007 (per-conversation monotonic `sequence_number`) + +## Context + +Chat messages need a total order for rendering, and offline catch-up needs a +resumable cursor over what a device missed. + +Issue #178 asked for a per-conversation monotonic `sequence_number`, assigned +atomically inside the write transaction (`UPDATE conversations ... RETURNING` on +a `last_sequence_number` column) so concurrent senders could not duplicate or +skip values. PR #324 implemented exactly that: a `lastSequenceNumber` on +`conversations`, `messages.sequenceNumber` as `bigint`, assignment wired into +every insert path — REST, Socket.IO, AI assistant replies, system events — and +history pagination switched to `sequenceNumber` cursors. + +It was then dropped again. The reason is in the schema comment and the sync +route: a counter that is monotonic *within one conversation* cannot also be a +coherent cursor *across conversations*. Offline catch-up (`GET /sync`) returns +the envelopes a single device missed, and those span every conversation that +device participates in. A per-conversation counter gives those rows no shared +order and no comparable cursor; the client would have to keep one cursor per +conversation and poll each of them. + +The alternative — two counters, one per-conversation for display and one global +for sync — was judged not worth the complexity of keeping both correct under +concurrent writes. + +## Decision + +Chat messages are ordered by `(created_at, id)` and nothing else. + +- `messages` has `index('messages_conversation_created_idx').on(conversationId, createdAt)`. +- Cursors are the pair itself: `GET /conversations/:id/messages?before=` + resolves the message's `(createdAt, id)` + (`apps/backend/src/routes/conversations.ts:596-629`) and pages with + `createdAt < c.createdAt OR (createdAt = c.createdAt AND id < c.id)`. +- `GET /sync` uses the *envelope's* `(createdAt, id)` encoded as the opaque + cursor `":"` (`apps/backend/src/routes/sync.ts:21-22, + 119-142`). +- `id` is only a tiebreaker for same-millisecond inserts. It is a random UUID + and carries no chronological meaning. +- Where ordering must not be skippable — group control events — a separate, + strictly monotonic log is used instead of the message table; see + `docs/group-epoch-sync.md`. + +## Consequences + +- One ordering for the timeline and for sync. A cursor is comparable across + conversations, so offline catch-up is a single request per device. +- No counter to maintain, no row lock on `conversations` for every message + insert, and no second counter to keep consistent. +- Clock skew and same-millisecond writes are handled by the timestamp tiebreak + rather than being impossible; that is cheaper, but the order is only as good + as the server clock. Both timestamps are server-generated (`defaultNow()`), + which keeps a client from choosing its own position in the timeline. +- Pages can interleave when two messages share a millisecond; the `id` tiebreak + keeps pagination from skipping or duplicating rows, it does not make the + order meaningful. +- Live correctness gap, still open: seven frontend types still declare + `sequenceNumber`, and the client sorts on `msg.sequenceNumber ?? 0`, so live + ordering collapses to `0` for every synced message + (`apps/web/docs/contracts-response-types.md`, DRIFT-1). This ADR is about the + server model; the stale client types are a follow-up. + +## Alternatives rejected + +- **Per-conversation monotonic `sequence_number`** (#178, implemented in + PR #324, then reverted). Rejected because a per-conversation counter is not a + usable cross-conversation sync cursor. This is stated directly in the code: + `apps/backend/src/db/schema.ts:117-121` and + `apps/backend/src/routes/sync.ts:21-22`. +- **Two counters** — per-conversation for display, global for sync. Rejected as + not worth the complexity (schema comment, `schema.ts:117-121`). +- **Client-side ordering by local receive time.** Not recorded anywhere in the + repo; the server timestamps are used instead, but no review discussion + rejecting this was found, so treat the rejection as unverified. +- **Ordering by `id` alone.** Rejected implicitly: `id` is a random UUID + (`defaultRandom()`), so it cannot order anything; it is a tiebreak only. + +## Sources + +- schema: `apps/backend/src/db/schema.ts:117-121` (ordering comment), + `messages` table and `messages_conversation_created_idx` +- pagination: `apps/backend/src/routes/conversations.ts:596-629` +- sync cursor: `apps/backend/src/routes/sync.ts:21-22,119-142` +- the monotonic log that *does* exist, for group control: + `apps/backend/drizzle` `group_control_events`, `docs/group-epoch-sync.md:12-25` +- docs: `apps/backend/docs/api-conversations.md:123-124`, + `apps/backend/docs/api-messages-sync.md:236-241`, + `apps/web/docs/contracts-response-types.md` (DRIFT-1, DRIFT-4) +- discussion: issue #178, PR #324 (both about the counter that was dropped) + +Citation caveat: the comment at `schema.ts:117-121` cites "(#137)" for the +offline-sync cursor argument, and `sync.ts:21` cites "(#128)". Neither number +resolves to that topic in this repo — #137 is a `cargo clippy` job and #128 is +the `treasury_proposals` table — so the numbers were reused or renumbered at +some point. The *reasoning* is in the code; the issue numbers are not reliable. diff --git a/docs/adr/0003-ciphertext-only-message-storage.md b/docs/adr/0003-ciphertext-only-message-storage.md new file mode 100644 index 0000000..b8daa0b --- /dev/null +++ b/docs/adr/0003-ciphertext-only-message-storage.md @@ -0,0 +1,116 @@ +# ADR-0003: Messages are stored ciphertext-only; legacy plaintext is archived, not tombstoned + +- **Status:** accepted +- **Date:** backfilled 2026-09-23 (decision predates this record) + +## Context + +`messages` originally carried a plaintext `content` column with a GIN index, +which is what made server-side search (`GET /conversations/:id/search`) +possible. The move to E2EE added `messages.ciphertext` and per-recipient +`message_envelopes` rows, so for a while both models existed: some rows had +plaintext, some had ciphertext, and the read paths had to handle either. + +Two problems came out of that: + +1. A server that can read messages is not an E2EE server. Any code path that + can serve `content` is a path that can leak plaintext to a client that + expected ciphertext. +2. Search over a column the server must not be able to read is not a feature + that can be kept. + +The cutover had to decide what to do with rows whose `content` was already +non-null, because dropping the column is destructive. + +## Decision + +`messages` is ciphertext-shaped, permanently, and the old plaintext is moved +out of the way rather than kept in place. + +- The one-time migration (`apps/backend/drizzle/0003_ciphertext_only_messages.sql`) + copies every non-null `content` into `message_content_archive`, then drops + the column and its GIN index. It is guarded and idempotent: the column check + goes through `information_schema.columns`, and the drops use + `IF EXISTS`, so it is a no-op on a database that already ran `0000`. +- `message_content_archive` has **no route and no query path** through the API + — it exists for legal/compliance access on its own retention schedule. Its + `original_message_id` is deliberately not a foreign key, so hard-deleting a + message never cascades into the archive. +- The read path never fabricates an E2EE claim over old plaintext. + `serializeMessage()`'s fallback chain (envelope → base `ciphertext` → + `unavailable: true`) means a pre-cutover row resolves to + `{ ciphertext: null, unavailable: true }`. +- `messages_system_payload_check` keeps the other half of the invariant: a + `system` row carries structured metadata and **no** ciphertext, every other + row carries ciphertext (or an envelope) and no system payload. +- Server-side search is gone: `GET /conversations/:id/search` returns `410 Gone` + pointing at the migration doc (`apps/backend/src/routes/conversations.ts:661-666`). + Search is client-side over decrypted messages. + +## Consequences + +- Every read path has one branch to reason about. There is no lingering + plaintext branch that a later change could accidentally serve as if it were + ciphertext, and `validateMessagePayload` rejects any client payload with a + plaintext body. +- Old plaintext is not re-encrypted and not presented as encrypted. Users see + those messages as "unavailable", which is the honest outcome. +- The archive is a real plaintext liability sitting in the same database; it is + not covered by the background GC jobs in `services/envelopeGc.ts` / + `services/fileCleanup.ts`. Retention is an open policy decision + (`docs/message-encryption-migration.md`, "Retention of the archive"). +- Rollback is manual and partial — `drizzle/rollback/0003_ciphertext_only_messages.down.sql` + restores only what the archive holds. Messages sent after the cutover were + never plaintext anywhere, so there is nothing to restore for them. +- `messages.systemPayload` and `messages.ciphertext` are now mutually exclusive + by constraint, so a future field that is neither must be a new column, not a + reuse of either. + +## Alternatives rejected + +- **Tombstone in place** — keep a `content`-shaped or `legacyPlaintext` column + on `messages` forever, nulled or flagged so the "this row used to be + plaintext" fact stays attached to the row. Rejected because it permanently + taxes every future `messages` migration and query with a legacy-shaped column + (and its GIN index) for data that existed only before the cutover and will + never grow. Stated in `apps/backend/docs/message-encryption-migration.md` + ("Decision: archive-then-purge (not a tombstone)") and in the PR #482 + summary. +- **Re-encrypting the old plaintext into envelopes at migration time.** Not + done: the server cannot produce envelopes for a device set it does not + control, and re-encrypting would backdate an "encrypted" claim onto data that + was sent in the clear. The doc says this explicitly ("No silent E2EE claim + over old plaintext"). Marked here as the reasoning the repo records, but no + review thread debating it as a serious candidate was found. +- **Keeping server-side search over ciphertext** (e.g. searchable encryption). + Not addressed anywhere in the repo; the endpoint was removed rather than + redesigned. Treat as unverified / not considered. + +## Sources + +- schema: `apps/backend/src/db/schema.ts` (`messages`, + `messages_system_payload_check`) — `messages` has a nullable `ciphertext` + column and no plaintext `content` column +- migration: named as `apps/backend/drizzle/0003_ciphertext_only_messages.sql` + with a rollback at `apps/backend/drizzle/rollback/0003_ciphertext_only_messages.down.sql` + in the doc, but **neither file is present in this tree**: the migration + history was squashed to a single `apps/backend/drizzle/0000_lean_scrambler.sql` + and `message_content_archive` is not defined in `db/schema.ts` either. What is + verifiable here is the end state (no plaintext column, `410` on search) and + the policy document, not the archive table itself +- enforcement: `apps/backend/src/lib/validateMessagePayload.ts`, + `apps/backend/src/lib/messages.ts` (`serializeMessage`), + `apps/backend/src/routes/conversations.ts:661-666` (`410` on search) +- docs: `apps/backend/docs/message-encryption-migration.md` (canonical), + `docs/threat-model.md` ("What the server cannot see") +- tests: `apps/backend/src/__tests__/security.regression.test.ts` +- discussion: PR #482 (archive-then-purge "chosen over an in-place tombstone"), + PR #420 (ciphertext-only invariant enforced in code + CI), PR #410 + ("Server cannot decrypt" assertion test) + +Link caveat: the `410` response points readers at +`https://github.com/DripWave/clicked/blob/main/docs/message-encryption-migration.md`, +which is the wrong owner and the wrong path for this repo — the document that +exists is `apps/backend/docs/message-encryption-migration.md` under +`codebestia/clicked`. The response body is code, so it was not corrected here; +it is recorded so the next reader of the ADR is not sent to a 404. diff --git a/docs/adr/0004-per-device-envelopes.md b/docs/adr/0004-per-device-envelopes.md new file mode 100644 index 0000000..ca654e2 --- /dev/null +++ b/docs/adr/0004-per-device-envelopes.md @@ -0,0 +1,118 @@ +# ADR-0004: Per-device envelopes, not one shared ciphertext + +- **Status:** accepted +- **Date:** backfilled 2026-09-23 (decision predates this record) + +## Context + +A direct message has one sender device and, usually, several recipient +devices — the recipient may have a phone and a laptop, and the sender usually +has more than one device too. Something has to decide how many ciphertexts a +single message produces and who each one is decryptable by. + +The cheap option is one ciphertext encrypted for the whole conversation: encrypt +once with a key every member device can derive, store one blob, deliver it to +everyone. It makes the storage and fan-out trivial and the message body +identical for every reader. + +It also means every device in the conversation shares one key, so compromising +any one device's key compromises every message ever sent with that key, and +there is no per-device revocation: the only way to stop a device reading is to +rotate the key for everyone. + +## Decision + +A DM is stored as one `messages` row plus one `message_envelopes` row per +recipient device — including the sender's own other devices. + +- The wire payload is `envelopes: [{ recipientDeviceId, ciphertext }]` + (`apps/backend/src/lib/validateMessagePayload.ts`: for `text` the envelope + array must be non-empty, for file-bearing content types the envelope + ciphertext is what carries the encrypted file key). +- Each envelope carries its own `ciphertext`, one row per device, keyed by + `recipient_device_id`; the pair is `me_recipient_device_created_idx`. +- The server enforces **sibling-device coverage**: every non-revoked device the + sender owns must have an envelope, or the send is rejected with + `device_set_mismatch` and the missing ids + (`apps/backend/src/lib/messageFanout.ts:42`, `fetchSiblingDeviceIds`; + `socket/messaging.ts`, `routes/messages.ts`). +- Persistence is transactional: the `messages` row and all `message_envelopes` + rows commit together, or neither does. +- Delivery is per device: each socket joins its own `device:` room, and + `deliverMessage` emits `message_envelope` only to devices that have an + envelope row (`apps/backend/src/services/deliveryPipeline.ts`). `new_message` + goes to the conversation room without any ciphertext. +- Every envelope records which construction produced it + (`message_envelopes.protocol`), so an envelope stays interpretable after the + device's advertised capabilities change; see ADR-0005. +- Group messages are the deliberate exception and use one group ciphertext + (ADR-0006) — but mixing the two models on a single message is rejected. + +## Consequences + +- Key compromise is scoped to a device, not to a conversation. Revoking a + device (`devices.revoked_at`) stops future fan-out to it without touching + anyone else's key. +- One message costs N envelope rows, N ciphertexts, and a client-side encryption + pass per target device. A three-device recipient set plus two sender devices + is five ciphertexts for one line of text, and that grows with group size — + which is why groups do not use this model (ADR-0006). +- The client must know the exact device set before sending, and the set can + change between the client's fetch and the write. Hence `device_set_mismatch` + with the missing ids and a re-encrypt-and-retry loop rather than a silent + partial fan-out. +- A sender who omits their own sibling device creates messages the sender + cannot read back on their other laptop; the server-side check makes that a + server-rejected error instead of a silent data loss. +- Per-device storage growth is real, and is why `services/envelopeGc.ts` exists. + +## Alternatives rejected + +- **One ciphertext for the whole conversation, shared by every member device.** + Rejected: a single shared key means a single point of compromise and no + per-device revocation. This is the model used for *groups* via MLS, where a + ratchet tree gives per-epoch membership changes and removal actually removes + the ability to read (ADR-0006) — a shared static conversation key has neither + property. Note that the repo does not contain a review thread stating this + rejection in these terms: the code records the chosen model and the ban on + mixing models, not the debate. Treat this rejection rationale as inferred + except for the mixing ban, which is explicit. +- **Sending `envelopes` alongside `mlsEpoch`** (a hybrid: per-device envelopes + plus a group ciphertext on one message). Explicitly rejected: "the two + key-distribution models must not be mixed on one message, or recipients have + no unambiguous rule for which ciphertext to decrypt" + (`apps/backend/src/lib/validateMessagePayload.ts:15-19`, + `apps/backend/docs/mls-group-membership.md:278-283`). `envelopes` together + with `mlsEpoch` is a `400`. +- **Encrypting nothing and letting the recipient devices fail to decrypt.** + Rejected in the read paths: a device with no envelope gets an explicit + `unavailable` marker rather than the ciphertext, because failed decryption is + indistinguishable from tampering on the client + (`apps/web/docs/concepts-message-pipeline.md` §3). + +## Sources + +- schema: `apps/backend/src/db/schema.ts` (`messageEnvelopes`, + `messageEnvelopes.recipientDeviceId`, `me_recipient_device_created_idx`) +- migration: `apps/backend/drizzle/0000_lean_scrambler.sql` + (`CREATE TABLE "message_envelopes"`, FK to `public.devices(id)`) +- send path: `apps/backend/src/lib/validateMessagePayload.ts`, + `apps/backend/src/socket/messaging.ts`, `apps/backend/src/routes/messages.ts` +- fan-out: `apps/backend/src/lib/messageFanout.ts:1-140`, + `apps/backend/src/services/deliveryPipeline.ts` +- client: `apps/web/src/lib/crypto.ts` (`buildEnvelopes`), + `apps/web/docs/concepts-message-pipeline.md` §1.2-1.5 +- docs: `apps/backend/docs/api-websocket-events.md:252-262`, + `docs/threat-model.md` ("ciphertext travels inside per-device envelopes") +- discussion: issue #184 (`send_message` accepts per-device envelopes), + issue #180 (migrate insert paths to ciphertext + envelopes), issue #188 + (multi-device self-sync — the sender's own other devices), + issue #337 (migrate `send_file_message`/`ask_assistant` to envelopes), + PR #269, PR #310 + +Citation caveat: `apps/web/src/lib/crypto.ts:256` credits the one-ciphertext- +per-target-device rule to "(#138)" and the `device_set_mismatch` retry to +"(#133)". Neither number is about that work in this repo (#138 pins the +Soroban toolchain, #133 is a withdrawal action); the sibling-device issue is +#188. The behaviour is verified in code; the comment's issue numbers are not +reliable. diff --git a/docs/adr/0005-sealed-box-to-signal-migration.md b/docs/adr/0005-sealed-box-to-signal-migration.md new file mode 100644 index 0000000..4315ba5 --- /dev/null +++ b/docs/adr/0005-sealed-box-to-signal-migration.md @@ -0,0 +1,111 @@ +# ADR-0005: Phase-1 sealed box to Phase-2 Signal, migrated per device pair + +- **Status:** accepted (migration path defined and enforced; Phase-2 encryption + itself is not activated — the client adapter still throws) +- **Date:** backfilled 2026-09-23 (decision predates this record) + +## Context + +The shipped encryption is the Phase-1 sealed box: ECDH ephemeral key + HKDF + +AES-256-GCM, one independent box per recipient device per message. It has no +forward secrecy and no ratchet — a device's long-term key compromised later +decrypts recorded history (`docs/threat-model.md`, residual risk 7). + +Phase 2 is the Signal Double Ratchet, which fixes that. But clients update on +their own schedule, and a device cannot decrypt what it has no code for. So at +any moment a conversation contains devices on both sides of the line, and the +question is not "when do we switch" but "how do we switch without losing +history or sending anyone an unreadable message". + +## Decision + +The cutover is negotiated per device pair, recorded per envelope, and never +rewrites the past. Three rules, from `apps/backend/docs/signal-migration.md`: + +1. **History is never re-encrypted.** `message_envelopes.protocol` + (`e2ee_protocol`: `sealed_box | signal | mls`, default `sealed_box`) records + the construction that produced each envelope. Pre-cutover envelopes keep + decrypting on the Phase-1 path indefinitely. +2. **A device pair uses the strongest protocol both sides advertise**, taken + from `devices.capabilities.protocols`, not the weakest protocol in the + conversation. +3. **A weaker protocol than both sides support is refused.** `400 + unsupported_by_recipient` when the envelope names a protocol the recipient + does not advertise (it would be undecryptable on arrival, indistinguishable + from tampering), `409 downgrade` when both devices could do better. Both + responses list every offending envelope. + +The transition needs no new endpoint: a client that ships Signal support +re-verifies with a widened capability list, and `POST /auth/verify` treats a +newer `capabilities` document as the upgrade path. Rollout order is mandated: +ship a client that can *decrypt* both paths but still encrypts sealed box +first, then the client that can encrypt Signal. Reversing that order produces +envelopes the peer cannot open. + +## Consequences + +- Forward secrecy arrives for a pair as soon as both sides are capable. In a + large group, pairs flip independently, so the security improvement is not + held hostage to the slowest device in the conversation. +- The server keeps enforcing something it cannot verify cryptographically: it + checks the *declared* protocol against the advertised capabilities, not the + bytes. A compromised client can lie about what it sent, and the downgrade + refusal only catches the case where it lies *downward* while both sides + support better. +- Two decryption paths stay in the client forever. That is the price of not + rewriting history, and it is deliberate. +- Capability and protocol are now two different columns for two different + questions ("what can this device do now" vs "what encrypted this envelope + then"). Conflating them was the tempting simplification; it would make old + envelopes uninterpretable after a capability change. +- Phase-2 remains unfinished work: `apps/web/src/lib/signalClient.ts` is a stub + that throws, `session.ts` still defaults to `Phase1SessionCrypto`, and there + is no IndexedDB-backed Signal session store yet + (`docs/signal-integration.md`, activation checklist). The *migration path* is + accepted and enforced; the *migration* is not done. + +## Alternatives rejected + +- **A conversation-wide "everyone must be ready" gate** — hold the whole + conversation on sealed box until every device advertises Signal. Described in + the migration doc as "simpler to describe but strictly worse": it would delay + forward secrecy for every pair in a group until the last straggler updates, + which in a large group may be never. +- **Re-encrypting history to Signal** when a pair cuts over. Rejected: + re-wrapping old messages would need the old plaintext or session state, and it + would break on any device that has not upgraded. "The cutover changes what is + written next, never what was written before." +- **A per-conversation or per-device `protocol` field** instead of per + envelope. Rejected because a conversation legitimately contains both during + the transition, and `devices.capabilities` describes current ability, not what + encrypted a message last month (`signal-migration.md`, "Data model"). +- **`libsignal-protocol-javascript` or + `@privacyresearch/libsignal-protocol-typescript`** as the Phase-2 library. + Rejected in favour of `@signalapp/libsignal-client`: the former is archived and + unmaintained, the latter has no independent audit, and neither ships a full + Double Ratchet (evaluation table in `docs/signal-integration.md`). + +## Sources + +- schema: `apps/backend/src/db/schema.ts:172-215` (`e2eeProtocolEnum`, + `messageEnvelopes.protocol`, `devices.capabilities`) +- negotiation and enforcement: `apps/backend/src/lib/capabilities.ts` + (`KNOWN_PROTOCOLS`, `selectProtocol`), + `apps/backend/src/services/e2eeProtocol.ts` +- migration spec: `apps/backend/docs/signal-migration.md` (canonical), + `apps/backend/docs/message-encryption-migration.md` and + `apps/backend/docs/api-websocket-events.md` for capability fields +- library decision: `docs/signal-integration.md` +- client state: `apps/web/src/lib/signalClient.ts` (stub, throws), + `apps/web/src/lib/session.ts`, `apps/web/src/lib/signalSession.ts` +- tests: `apps/backend/src/__tests__/e2eeProtocol.test.ts`, + `signalMigration.routes.test.ts`, `signalInvariants.*.test.ts` +- discussion: PR #533 (Phase-1 → Signal migration path, which the `#364` + schema comment also refers to), issue #306 (Double Ratchet for DMs), + issue #362 (multi-device Signal sessions) + +Caveat: the migration doc names `drizzle/0004_envelope_protocol.sql` as the +migration that adds `message_envelopes.protocol`. That file is not present in +this tree — the migration history is squashed into +`apps/backend/drizzle/0000_lean_scrambler.sql`, where the column and the +`e2ee_protocol` enum do exist, with `DEFAULT 'sealed_box'`. diff --git a/docs/adr/0006-mls-for-groups.md b/docs/adr/0006-mls-for-groups.md new file mode 100644 index 0000000..aefccf4 --- /dev/null +++ b/docs/adr/0006-mls-for-groups.md @@ -0,0 +1,130 @@ +# ADR-0006: MLS for groups — one group ciphertext, epochs as the unit of membership + +- **Status:** accepted +- **Date:** backfilled 2026-09-23 (decision predates this record) + +## Context + +DMs use per-device envelopes (ADR-0004). Applying the same model to a group +means encrypting the message separately for every device of every member. In a +30-person group with 3 devices each that is 90 ciphertexts for one message, and +it grows again every time someone links a new device. + +It also has no answer for removal. If a member holds a key that decrypts +messages addressed to them and the sender must remember to stop addressing +them, "removed from the group" is a server-side bookkeeping rule rather than a +cryptographic fact — and a removed device that kept its key can still read +anything sent to it before the removal. + +Issue #370 fixed the shape: group messages are encrypted once with the group key +and stored as a single `messages.ciphertext`, delivered to every member device. + +## Decision + +Group conversations run MLS (RFC 9420), with the server as transport and +ordering service only. + +- The group's unit of membership is the **epoch**. A device's leaf exists from + the commit that added it, so it reads epoch `e` when + `joined_at_epoch <= e < (removed_at_epoch ?? infinity)` + (`apps/backend/src/lib/mlsVisibility.ts`). Membership is an epoch interval, + not a flag; a device removed and re-added gets a second row and genuinely + cannot read the epochs it was absent for. +- One ciphertext per group message, not one per device. The sender includes + `mlsEpoch`; when it is present the per-device envelope requirement does not + apply and `envelopes` alongside it is rejected. The sibling-device coverage + check is skipped, because a group ciphertext already reaches every device in + the tree. +- The server stores public group state only: `mls_groups` (id, cipher suite, + current epoch), `mls_group_members` (epoch intervals), `mls_commits` + (append-only commit log), `mls_welcomes` (Welcome messages held for offline + devices), `mls_key_packages`. It never holds a group secret or a ratchet key. +- Commits are epoch-serialized. `epoch` must be exactly `current_epoch + 1`; the + group row is locked for the write, so two simultaneous commits get one winner + and one `409` with the expected epoch to retry against. +- A message a device cannot decrypt comes back with `ciphertext: null`, + `unavailable: true` and a reason + (`mls_no_key_before_join`, `mls_no_key_after_removal`, + `mls_not_a_group_member`), applied server-side on both read paths. + +## Consequences + +- Group size no longer multiplies ciphertexts. File sharing follows the same + rule: the file is encrypted once with a random file key, and the file key + travels inside the MLS group message, so object storage holds one object + instead of ninety (`apps/backend/docs/mls-group-files.md`). +- Removal is cryptographic: a removed device holds no key for any later epoch. + The same property is why a newly linked device cannot read history — there is + no server-held key to hand it, by design. +- A newly linked device joins late and reads from its join epoch onward; earlier + messages render as a labelled "unavailable on this device" placeholder. Users + must be told this, and the API exposes `historyAvailableFromEpoch` so the + client can render one divider instead of a wall of broken rows. +- The server becomes a sequencing service for commits, with a lock on the group + row and a conflict path clients must handle by re-deriving and retrying. +- A device that is offline when it is added still gets added, because Welcomes + are held for it and claimed later. +- Group messaging is only reachable by clients that implement MLS. This is a + separate code path from the DM envelope path, not a variant of it. +- Groups with no MLS group (plain `dm`, or a group that never adopted MLS) fall + back to the per-device envelope model; `mls_epoch` is null for those rows. + +## Alternatives rejected + +- **Per-device envelope fan-out for group content and group files.** The + explicit rejected alternative: "In a 30-person group with 3 devices each that + is 90 copies of the same bytes, and it grows again on every new device" + (`apps/backend/docs/mls-group-files.md`, "Why one ciphertext"). MLS was chosen + because the group already agrees on epoch secrets, so no server-side key is + needed at all. +- **Sending `envelopes` together with `mlsEpoch`.** Rejected outright: + "mixing the two key-distribution models on a single message leaves recipients + with no unambiguous rule for which ciphertext to decrypt" + (`apps/backend/docs/mls-group-membership.md:278-283`; + `apps/backend/src/lib/validateMessagePayload.ts:15-19`). +- **Backfilling history to a newly linked device** (from an existing device + re-encrypting and re-sending past plaintext). Not done now — it is recorded as + a separate, deferred feature ("a history/recovery service", issue #372) and is + the only way the "no history for new device" property would change. +- **Surfacing unreadable messages as an error, or omitting them.** Rejected: + an error is indistinguishable from tampering and trains users to dismiss a + real signal; omitting them hides that the conversation happened and puts a + gap in the timeline. Placeholders keep sender, timestamp and epoch. +- **Checking the epoch window client-side.** Rejected: "the client cannot forget + to check" — the rule is applied on both server read paths + (`lib/mlsVisibility.ts`, used by `routes/conversations.ts` and the socket + `message_history` handler). +- **Sender-keys / a pairwise-ratchet group construction** — a common alternative + to MLS for groups. There is no record of it being evaluated in this repo, so + no rejection rationale can be given. Marked unverified. +- **A server-held group key** (server derives or stores group secrets to make + fan-out easy). Rejected implicitly by the whole design — the server is + explicitly a transport that never holds group secrets + (`apps/backend/src/db/schema.ts:316-330`, `docs/threat-model.md`). No review + thread found; treat as design posture rather than a recorded debate. + +## Sources + +- schema: `apps/backend/src/db/schema.ts:316-490` (`mlsGroups`, + `mlsGroupMembers`, `mlsCommits`, `mlsWelcomes`, `mlsKeyPackages`), + `messages.mlsEpoch` +- epoch rule: `apps/backend/src/lib/mlsVisibility.ts`, + `apps/backend/src/services/mlsGroups.ts` +- routes: `apps/backend/src/routes/mls.ts`; read paths + `apps/backend/src/routes/conversations.ts`, `apps/backend/src/socket/messaging.ts` +- docs: `apps/backend/docs/mls-group-membership.md` (canonical), + `apps/backend/docs/mls-group-files.md`, `apps/backend/docs/mls-key-packages.md`, + `docs/group-epoch-sync.md` (the separate group-control sequence log) +- tests: `apps/backend/src/__tests__/mls.routes.test.ts`, + `mlsGroups.service.test.ts`, `mlsVisibility.test.ts`, `mls.history.test.ts`, + `mlsGroupFiles.test.ts`, `mlsKeyPackages.test.ts` +- discussion: issue #370 (single ciphertext for groups), issue #372 (new-device + join and the deferred recovery service), PR #418, PR #425 (epoch sync + + system events), PR #426 (group MLS fan-out), PR #530, PR #531, PR #532 + +Caveat: the doc names migrations `0001_mls_group_state.sql`, +`0001_group_control_events.sql` and `0004_envelope_protocol.sql`. None of those +files are in this tree — `apps/backend/drizzle/` contains only +`0000_lean_scrambler.sql` (plus `meta/`), where the `mls_*` tables, the +`e2ee_protocol` enum and `group_control_events` all exist. The schema is +verifiable; the individual migration files named in the docs are not. diff --git a/docs/adr/0007-per-conversation-sequence-numbers.md b/docs/adr/0007-per-conversation-sequence-numbers.md new file mode 100644 index 0000000..9b6e0b5 --- /dev/null +++ b/docs/adr/0007-per-conversation-sequence-numbers.md @@ -0,0 +1,59 @@ +# ADR-0007: Per-conversation monotonic `sequence_number` for chat messages + +- **Status:** superseded by [ADR-0002](./0002-message-ordering-by-createdat-id.md) +- **Date:** backfilled 2026-09-23 (decision and its reversal both predate this record) + +## Context + +This record exists because the decision was made, shipped, and then reversed. +An index that only lists the surviving decisions hides the reversal, and the +next person to ask "why don't these rows have sequence numbers?" deserves the +history rather than a re-derivation. + +Issue #178 asked for gapless, strictly increasing per-conversation message +numbers, assigned atomically inside the write transaction so concurrent senders +could not duplicate a value. PR #324 implemented it: `lastSequenceNumber` on +`conversations`, `messages.sequenceNumber` as `bigint`, assignment wired into +REST, Socket.IO, AI assistant replies and system-event paths, and history +pagination switched from `(createdAt, id)` to `sequenceNumber` cursors. + +## Decision + +`messages.sequenceNumber` and `conversations.lastSequenceNumber` existed and +were assigned with `UPDATE conversations ... RETURNING` inside the message +persistence transaction. + +## Consequences + +- Within a conversation the order was gapless and strictly increasing, and + "did I miss anything?" was an integer comparison. +- Every message insert took a row lock on its conversation row. That is a real + serialization point on the hottest write path in the system, and it made + horizontal scaling of the send path a per-conversation bottleneck. +- Most importantly, the counter could not serve as a cursor for `GET /sync`, + which returns a device's missed envelopes across *all* its conversations. See + the reversal in ADR-0002. + +## Alternatives rejected + +None recorded at the time: the issue asked for the counter and the PR +implemented it. The rejected alternative — ordering by `(createdAt, id)` — is +what the codebase returned to, and is recorded in ADR-0002 rather than here. + +## Sources + +- discussion: issue #178 (closed; the request), PR #324 (merged; the + implementation) +- reversal: `apps/backend/src/db/schema.ts:117-121` (the ordering comment), + `apps/backend/src/routes/sync.ts:21-22`, + `apps/web/docs/contracts-response-types.md` (DRIFT-1 and DRIFT-4 — the + frontend types that were never cleaned up) +- surviving remnants of the model: the database column is gone from + `apps/backend/src/db/schema.ts`, but 79 references to `sequenceNumber` remain + across the repo (43 in `apps/web/src` — `hooks/useInboundPipeline.ts`, + `lib/crypto/types.ts`, the response-type declarations; + 8 in `apps/backend/src` — the optional `sequenceNumber` passthrough on the + `message_delivered` receipt path in `socket/messaging.ts:838-915` and a test + asserting envelopes do *not* carry it; 27 in docs, including + `apps/backend/docs/api-websocket-events.md:444` which still describes it as + "Per-conversation monotonic sequence number for ordering"). diff --git a/docs/adr/0008-separate-user-devices-table.md b/docs/adr/0008-separate-user-devices-table.md new file mode 100644 index 0000000..6404c02 --- /dev/null +++ b/docs/adr/0008-separate-user-devices-table.md @@ -0,0 +1,52 @@ +# ADR-0008: A separate `user_devices` table for device identity + +- **Status:** superseded by [ADR-0001](./0001-single-devices-table.md) +- **Date:** backfilled 2026-09-23 (decision and its reversal both predate this record) + +## Context + +This record exists because the decision shipped and was then undone. It is kept +so that the `GET /user-devices/:id/public-key` route — which still exists — has +an explanation, instead of looking like an accident. + +The first device-identity work needed a place to store an Ed25519 identity +public key per device, plus prekeys and revocation state. The realtime work +needed a list of "devices that can receive a socket delivery" with last-seen +timestamps and push subscriptions. These were treated as separate concerns and +got separate tables. + +PR #206 added the `user_devices` device-identity schema; PR #207 added +`GET /devices` listing from it. + +## Decision + +Device identity lived in `user_devices`; delivery/presence/push device state +lived in `devices`. + +## Consequences + +- Two tables described the same real-world object with no constraint tying them + together. A device revoked in one was still a delivery target in the other, + and neither table could tell the other it was wrong. +- Sibling-device coverage checks (issue #188) resolved against one table while + the auth layer resolved against the other, so "all of the sender's devices + have an envelope" could be true of the set the send path knew about and false + of the set auth would accept a login from. +- Two write paths had to be kept in step for every device lifecycle event + (register, revoke, logout-everywhere, stale-flag), and nothing enforced that + they were. + +## Alternatives rejected + +None recorded at the time. The alternative — one table with the union of the +columns — is what replaced it, and is recorded in ADR-0001. + +## Sources + +- discussion: PR #206 (added `user_devices`), PR #207 (`GET /devices` from it) +- reversal: `apps/backend/src/routes/devices.ts:1-7` (the merge rationale is in + the file's header comment), `apps/backend/docs/api-devices.md:14-17` +- remnant: `apps/backend/src/routes/userDevices.ts`, mounted at `/user-devices` + and documented in `apps/backend/docs/api-devices.md` as a legacy alias that + exposes a public-key lookup only +- current model: ADR-0001 (one canonical `devices` table) \ No newline at end of file diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..a6c0df9 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,61 @@ +# Architecture Decision Records + +Decisions this codebase has made, and the ones it made and took back. Each +record states the context, the decision, its consequences, what was rejected, +and where to check it in the code. + +These were backfilled from the repository — schema comments, route headers, the +`docs/` set, migration files and the review history — rather than written from +memory. Where the record does not substantiate why an alternative was rejected, +the ADR says so instead of supplying a rationale. + +## Records + +| ADR | Decision | Status | +| ------------------------------------------------------------ | ------------------------------------------------------------------------------- | ----------------------------------- | +| [0000-template.md](./0000-template.md) | Template for new records | — | +| [0001](./0001-single-devices-table.md) | One canonical `devices` table; no second device store | accepted (supersedes 0008) | +| [0002](./0002-message-ordering-by-createdat-id.md) | Order messages by `(createdAt, id)`, not a per-conversation sequence counter | accepted (supersedes 0007) | +| [0003](./0003-ciphertext-only-message-storage.md) | Ciphertext-only message storage; legacy plaintext archived, not tombstoned | accepted | +| [0004](./0004-per-device-envelopes.md) | Per-device envelopes for DMs, not one shared ciphertext | accepted | +| [0005](./0005-sealed-box-to-signal-migration.md) | Phase-1 sealed box → Phase-2 Signal, negotiated per device pair, history never rewritten | accepted (migration not yet activated) | +| [0006](./0006-mls-for-groups.md) | MLS for groups: one group ciphertext, epochs as the unit of membership | accepted | +| [0007](./0007-per-conversation-sequence-numbers.md) | Per-conversation monotonic `sequence_number` for chat messages | **superseded** by 0002 | +| [0008](./0008-separate-user-devices-table.md) | A separate `user_devices` table for device identity | **superseded** by 0001 | + +0007 and 0008 are numbered last because they were backfilled from the decisions +that replaced them, not because they came later. Their subject matter belongs +next to 0002 and 0001 respectively. + +## Statuses used + +- **accepted** — the rule stands and the code enforces it. +- **superseded** — the decision was made, shipped, and replaced. The record is + kept because the replacement only makes sense against what it replaced, and + because remnants of the old decision are still in the tree. +- **proposed** — not used yet; see the template. + +## Adding a record + +1. Copy `0000-template.md` to `NNNN-short-slug.md`, using the next free number. +2. Fill in context, decision, consequences and the alternatives that were + actually rejected — with the reason, and a note if the reason is not + recorded anywhere. +3. Add a row to the table above with its status. +4. If the record replaces another, edit the old one's status to + `superseded by ADR-NNNN` and link both ways. + +Keep records checkable: cite file paths, schema symbols, migration names and +issue/PR numbers, and prefer short over complete. A record nobody can verify is +a liability. + +## Further reading + +- `apps/backend/docs/signal-migration.md` — the Phase-1 → Signal cutover in full +- `apps/backend/docs/message-encryption-migration.md` — the ciphertext-only cutover in full +- `apps/backend/docs/mls-group-membership.md` — MLS group membership and new-device join +- `apps/backend/docs/api-devices.md` — the device registry surface +- `docs/threat-model.md` — what the server can and cannot see +- `docs/group-epoch-sync.md` — why group control has the sequence counter that chat messages don't + +Cross-referenced from [`IMPLEMENTATION_DOCS.md`](../../IMPLEMENTATION_DOCS.md). \ No newline at end of file