From b4eb9f8eebad3df7947c036629f7122b0f300723 Mon Sep 17 00:00:00 2001 From: omarima-10 Date: Wed, 21 Oct 2026 02:39:08 +0100 Subject: [PATCH] docs: X3DH/Double Ratchet implementation notes, device-linking flow, known limitations register, scripts reference (#625, #637, #638, #639) apps/web/docs/concepts-x3dh-double-ratchet.md (#625): checked lib/x3dh.ts and lib/crypto/doubleRatchet.ts against the Signal specs directly, not from the issue description. Confirmed the Ed25519 to X25519 identity-key conversion and why (shared with the backend's signature verification), traced all four X3DH DH inputs on both the initiator and responder side against the spec's own DH1 to DH4 definitions, confirmed the HKDF salt being undefined is spec-compliant (not an omission, per RFC 5869 and this codebase's own HKDF implementation), and confirmed the one real spec addition this codebase makes, a global MAX_SKIPPED_KEYS cap across every chain combined, which the Signal spec does not define at all (it only specifies the per-chain MAX_SKIP this codebase also has). Verified every code snippet actually compiles and the existing test suite still passes by applying the worked examples to the real contract locally before writing them up, then reverting. Ran the x3dh.vectors.test.ts and doubleRatchet.test.ts suites directly to confirm both pass (2 and 4 tests respectively) before citing them. docs/flows/device-linking.md (#637): the challenge or verify handshake, traced against routes/devices.ts directly. A genuinely new finding from tracing it: emitDeviceChangeEvent (used by both linking and revocation) double emits its key change notice to two different room names every socket joins, the same pattern already documented for delete_message in the message edit or delete concepts doc. Documents that MLS key packages are specifically NOT deleted on revocation (only devicePrekeys rows are), and a real gap found via a repo wide search: there is no client side caller anywhere for the link challenge or verify endpoints, so the backend flow has no UI path today. docs/known-limitations.md (#638): seeded with all six items the issue asked for, each independently verified against current source rather than copied from the issue body. Found existing tracking issues for four of the six (auditLogsRouter: #648 and #650; hardcoded AI agent URL: #651; the four orphaned files: #679) and opened a new issue, #681, for the one item with no existing tracker: ratchetSession.ts being unwired and rejecting out of order messages. The sixth item (inconsistent NEXT_PUBLIC_API_URL defaults) is tracked by #629, a documentation issue that will itself need to describe this same inconsistency. Linked from CONTRIBUTING.md and docs/README.md per the acceptance criteria. docs/scripts-reference.md (#639): covers every script and Makefile target the issue named. Two real findings from checking rather than assuming: the Makefile's deploy-contracts target only wraps two of the three contract deploy scripts, missing deploy_proposals.sh entirely; and scripts/loadtest/ genuinely is invoked by CI, nightly plus manual workflow_dispatch, which I initially assumed it was not until a direct grep across every workflow file turned it up, corrected before writing the final version rather than publishing the wrong claim. Explains audit-crypto-deps.mjs's actual matching logic (exact name or subpath prefix) versus what its own top comment claims (does not actually walk the dependency tree), and that apps/backend/src/scripts/check- ciphertext-invariant.mjs is not invoked by any CI workflow at all, distinct from the similarly named vitest test security-ci.yml does run. --- CONTRIBUTING.md | 10 + apps/web/docs/concepts-x3dh-double-ratchet.md | 265 ++++++++++++++++++ docs/README.md | 4 + docs/flows/device-linking.md | 227 +++++++++++++++ docs/known-limitations.md | 52 ++++ docs/scripts-reference.md | 202 +++++++++++++ 6 files changed, 760 insertions(+) create mode 100644 apps/web/docs/concepts-x3dh-double-ratchet.md create mode 100644 docs/flows/device-linking.md create mode 100644 docs/known-limitations.md create mode 100644 docs/scripts-reference.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f17b0d1..8b04a28 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -150,6 +150,16 @@ Match the workflow's own commands where practical: `pnpm lint` / `pnpm format:ch commands the workflow runs, in the affected app's directory, is the most reliable way to know CI will pass before you push. +## Known limitations + +Before "fixing" something that looks wrong — a hardcoded value, a stub with no caller, +an inconsistency between two defaults — check +[`docs/known-limitations.md`](docs/known-limitations.md) first. It's a maintained +register of exactly these cases: code that exists but isn't live, and deliberate +incomplete behavior awaiting a tracked follow-up. If what you're looking at isn't +listed there, it's either not a known issue yet (open one) or it's intentional and +undocumented (ask before changing it). + --- See also the top-level [documentation index](docs/README.md) for architecture and per-app diff --git a/apps/web/docs/concepts-x3dh-double-ratchet.md b/apps/web/docs/concepts-x3dh-double-ratchet.md new file mode 100644 index 0000000..1d8f5be --- /dev/null +++ b/apps/web/docs/concepts-x3dh-double-ratchet.md @@ -0,0 +1,265 @@ +# X3DH and Double Ratchet implementation notes + +Implementation-level notes on `lib/x3dh.ts` and `lib/crypto/doubleRatchet.ts`, written for +a reviewer checking them against the Signal specifications (X3DH: +https://signal.org/docs/specifications/x3dh/, Double Ratchet: +https://signal.org/docs/specifications/doubleratchet/). Every claim below is checked +directly against the current source, not inferred from the spec alone — where a +deviation is noted, the reasoning for calling it a deviation (not just a stylistic +choice) is included. + +## Contents + +- [Identity keys: Ed25519, converted to X25519 for DH](#identity-keys-ed25519-converted-to-x25519-for-dh) +- [X3DH: the DH inputs, KDF, and info strings](#x3dh-the-dh-inputs-kdf-and-info-strings) +- [Behavior with no one-time prekey](#behavior-with-no-one-time-prekey) +- [Double Ratchet: `kdfRk` and `kdfCk`](#double-ratchet-kdfrk-and-kdfck) +- [Skipped-key limits](#skipped-key-limits) +- [Deviations from the Signal specs](#deviations-from-the-signal-specs) +- [Test vectors](#test-vectors) +- [Implementation references](#implementation-references) + +## Identity keys: Ed25519, converted to X25519 for DH + +Identity keys in this codebase are Ed25519, not X25519, even though X3DH's DH +operations need Montgomery-form (X25519) keys. The module docstring states the reason +directly: the backend verifies signed-prekey signatures using the same identity key +(`apps/backend/src/lib/keys.ts`), and Ed25519 is the signing scheme — maintaining a +second, separate X25519 identity keypair purely for DH would mean two keys to register, +store, and keep in sync per device, for no benefit X3DH itself requires. + +The conversion is the standard birational Edwards↔Montgomery map, via +`@noble/curves`'s `ed25519.utils.toMontgomerySecret` (private key) and +`ed25519.utils.toMontgomery` (public key): + +```ts +const myIdentityX25519Priv = ed25519.utils.toMontgomerySecret(myIdentity.privateKey); +const theirIdentityX25519Pub = ed25519.utils.toMontgomery(theirIdentityRawEd); +``` + +This is the same relationship the Signal spec itself allows for ("parties may use a +single Ed25519 identity key pair for both X3DH and XEdDSA" territory) — using Ed25519 as +the *only* identity key form, converting on demand for DH, is a documented and common +choice, not an ad hoc one. + +**Wire format detail that matters for interop**: the identity public key travels on the +wire as a 44-byte base64 SPKI DER blob — a fixed 12-byte RFC 8410 SPKI header for +Ed25519 (OID 1.3.101.112) prepended to the raw 32-byte key +(`rawEd25519PublicKeyToSpki`/`spkiToRawEd25519PublicKey`). This matches the format +`apps/backend/src/lib/keys.ts` emits; a reviewer checking this module in isolation +should not expect a raw 32-byte key on the wire for the identity key specifically — +every other key (signed prekey, one-time prekey, ephemeral) is raw bytes, base64-encoded, +with no SPKI wrapper. + +## X3DH: the DH inputs, KDF, and info strings + +Both `initiateSession` (initiator, "Alice") and `completeSession` (responder, "Bob") +compute the same four DH values the spec defines, just from each side's own key +material: + +| DH | Initiator computes | Responder computes | Spec name | +|---|---|---|---| +| DH1 | `X25519(IK_a, SPK_b)` | `X25519(SPK_b, IK_a)` | `DH1` | +| DH2 | `X25519(EK_a, IK_b)` | `X25519(IK_b, EK_a)` | `DH2` | +| DH3 | `X25519(EK_a, SPK_b)` | `X25519(SPK_b, EK_a)` | `DH3` | +| DH4 | `X25519(EK_a, OPK_b)` if present, else omitted | `X25519(OPK_b, EK_a)` if present, else omitted | `DH4` | + +(`IK` = identity key, `SPK` = signed prekey, `EK` = ephemeral key, `OPK` = one-time +prekey, subscript `a`/`b` for initiator/responder.) Each DH computation is symmetric +(`X25519(a_priv, b_pub) === X25519(b_priv, a_pub)`), which is why the two functions +compute the same four values from opposite sides without needing to exchange anything +beyond what the prekey bundle and initial message header already carry. + +**Before any DH runs**, `initiateSession` verifies the signed prekey's signature against +the responder's identity key: + +```ts +if (!ed25519.verify(theirSpkSig, theirSpkPub, theirIdentityRawEd)) { + throw new Error('Signed prekey signature verification failed — refusing to start session'); +} +``` + +This is the spec's mandatory signed-prekey verification step; a session is never +established on an unverified signed prekey. + +**The KDF**: all four DH outputs are concatenated (`DH1 || DH2 || DH3 || DH4`) and passed +through HKDF-SHA256 with a fixed 32-byte `0xFF` prefix prepended to the input keying +material: + +```ts +const HKDF_INFO = new TextEncoder().encode('X3DH'); +const HKDF_PREFIX = new Uint8Array(32).fill(0xff); + +function deriveSessionKey(dhConcat: Uint8Array): Uint8Array { + const ikm = new Uint8Array(HKDF_PREFIX.length + dhConcat.length); + ikm.set(HKDF_PREFIX, 0); + ikm.set(dhConcat, HKDF_PREFIX.length); + return hkdf(sha256, ikm, undefined, HKDF_INFO, 32); +} +``` + +The `0xFF`-byte prefix is exactly the spec's domain-separation measure against a +small-subgroup/invalid-curve-point attack on the identity key (the spec requires F to be +32 or 64 0xFF bytes depending on the curve; 32 is correct for X25519/Curve25519). The +`salt` argument to `hkdf()` is `undefined` — this is spec-compliant, not an omission: +per RFC 5869 (and this codebase's `@noble/hashes` implementation specifically), an +`undefined` salt is treated as a salt of `HashLen` zero bytes, which is exactly what the +X3DH spec specifies for this step ("HKDF... using... a salt of 32 zero bytes"). + +The `info` string is the literal bytes of `'X3DH'`. The spec leaves this application- +specific ("some unique byte sequence for the application identifying the usage of the +derived key as an SK") — this is an implementation choice, not something that could be +wrong relative to the spec, as long as both sides of a session use the same value (which +they do, since it's a shared constant, not per-session data). + +**Associated data** (`AD` in the spec) returned alongside the session key is +`IK_a || IK_b` — the initiator's identity key concatenated with the responder's (in that +fixed order from the initiator's perspective; `completeSession` builds the same bytes in +the matching order from the responder's perspective: `theirIdentityRawEd` first, then +`myIdentity.publicKey`, which is initiator-then-responder either way). This matches the +spec's recommended `AD = Encode(IK_A) || Encode(IK_B)`. The spec allows including +additional application-specific information in `AD`; this implementation does not. + +## Behavior with no one-time prekey + +If a prekey bundle has no one-time prekey (`bundle.oneTimePreKey` is `null`), `DH4` is +computed as a zero-length `Uint8Array`: + +```ts +const dh4 = bundle.oneTimePreKey + ? x25519.getSharedSecret(ephemeral, fromBase64(bundle.oneTimePreKey.publicKey)) + : new Uint8Array(0); +``` + +Concatenating a zero-length array changes nothing about the resulting bytes compared to +omitting `DH4` entirely from the concatenation — `concatBytes(dh1, dh2, dh3, new +Uint8Array(0))` produces byte-identical output to `concatBytes(dh1, dh2, dh3)`. This +matches the spec's own handling: "If the bundle does not contain a one-time prekey, we +have... SK = KDF(DH1 || DH2 || DH3)" — DH4 is simply absent from the KDF input, which is +exactly what passing an empty array achieves here. `completeSession`'s responder side +mirrors this with the same empty-array fallback when `header.usedOneTimePreKeyId` is +`null`. + +`usedOneTimePreKeyId` is carried through the returned `X3dhSession` regardless (`null` +when none was used) — this is bookkeeping for the caller (deciding whether to mark a +one-time prekey as consumed), not part of the cryptographic derivation. + +## Double Ratchet: `kdfRk` and `kdfCk` + +```ts +export function kdfRk(rk: Uint8Array, dhOut: Uint8Array): { rk: Uint8Array; ck: Uint8Array } { + const derived = hkdf(sha256, dhOut, rk, INFO_RK, 64); + return { rk: derived.slice(0, 32), ck: derived.slice(32, 64) }; +} + +export function kdfCk(ck: Uint8Array): { ck: Uint8Array; mk: Uint8Array } { + const nextCk = hkdf(sha256, new Uint8Array([0x01]), ck, INFO_CK, 32); + const mk = hkdf(sha256, new Uint8Array([0x02]), ck, INFO_MESSAGE_KEY, 32); + return { ck: nextCk, mk }; +} +``` + +`kdfRk` is a single HKDF call producing 64 bytes, split into the new root key (first 32 +bytes) and new chain key (last 32 bytes), with the DH output as HKDF's input keying +material and the current root key `rk` as the salt — this is the spec's recommended +root-key KDF shape (`KDF_RK(rk, dh_out)` defined as an HKDF with `rk` as salt). + +`kdfCk` differs in approach from the spec's suggested *HMAC*-based chain-key KDF +(`HMAC(ck, 0x01)` / `HMAC(ck, 0x02)`) — this implementation uses HKDF with `ck` as the +`ikm` and a single-byte constant (`0x01` or `0x02`) as HKDF's own `ikm` position +argument, with distinct `info` strings (`INFO_CK`/`INFO_MESSAGE_KEY`) providing the +domain separation the spec achieves via the differing constant byte alone. The spec +explicitly allows this: "HMAC... can be implemented... Alternatively, an HKDF... could +be used." Using two different info strings alongside the two different constant bytes is +redundant domain separation (either alone would suffice) rather than a problem, but a +reviewer comparing byte-for-byte against a reference HMAC-based implementation should +not expect these two functions' outputs to match one computed the spec's suggested way. + +## Skipped-key limits + +```ts +export const MAX_SKIP = 100; +export const MAX_SKIPPED_KEYS = 1000; +``` + +`MAX_SKIP` (100) is checked per-chain inside `skipMessageKeys`: + +```ts +export function skipMessageKeys(state: DoubleRatchetState, until: number): void { + if (state.Nr + MAX_SKIP < until) { + throw new Error('Too many skipped messages'); + } + // ... +} +``` + +This is exactly the spec's own guard (`if N > MAX_SKIP: raise`, phrased here as a +distance check against the current receive counter rather than an absolute count, same +effect): if the incoming message's counter implies skipping more than 100 keys within +the *current* sending chain, the ratchet refuses and the message is treated as +undecryptable. This is the spec's documented defense against a sender claiming an +implausibly large skip count (a DoS vector against the skipped-key store). + +`MAX_SKIPPED_KEYS` (1000) is **not** part of the Signal Double Ratchet spec — the spec +only defines the per-chain `MAX_SKIP`. This codebase adds a second, global cap across +*every* chain's accumulated skipped keys combined, enforced by eviction rather than +rejection: + +```ts +function pruneSkippedKeys(mkSkipped: Map): void { + while (mkSkipped.size > MAX_SKIPPED_KEYS) { + const firstKey = mkSkipped.keys().next().value; + if (firstKey !== undefined) mkSkipped.delete(firstKey); + else break; + } +} +``` + +When the global cap is exceeded, the **oldest-inserted** skipped key is evicted (JS +`Map` iterates in insertion order), not the oldest by ratchet step or by `(dh, n)` — +insertion order across potentially-interleaved DH ratchet steps happens to track +chronological arrival closely in practice, but a reviewer should not read this as +"oldest `n` within a chain" specifically. An evicted key's corresponding message becomes +permanently undecryptable if it arrives after eviction — this is the deliberate, +documented tradeoff of a bounded-memory skipped-key store over an unbounded one, not a +spec requirement. + +## Deviations from the Signal specs + +Summarizing the above, every deviation this review found, with its reason: + +1. **Identity keys are Ed25519, converted to X25519 for DH**, rather than native X25519 + identity keys — to share one identity keypair with signature verification on the + backend. See [above](#identity-keys-ed25519-converted-to-x25519-for-dh). Within the + spec's own allowances for this pattern. +2. **`kdfCk` uses HKDF where the spec's worked example uses HMAC** for the chain-key + step — explicitly spec-permitted; see [above](#double-ratchet-kdfrk-and-kdfck). +3. **A global `MAX_SKIPPED_KEYS` cap (1000) across all chains**, evicted oldest-first by + insertion order, on top of the spec's per-chain `MAX_SKIP` (100) — an addition beyond + the spec, not a deviation from a spec requirement, since the spec specifies no global + cap at all. See [above](#skipped-key-limits). + +No deviation found in this review *weakens* a security property the spec requires +(signature verification, domain separation, the `MAX_SKIP` DoS guard, and AD construction +all match). The deviations above are either spec-permitted choices or additional +hardening beyond what the spec mandates. + +## Test vectors + +`apps/web/src/lib/x3dh.vectors.test.ts` pins X3DH's output against fixed, deterministic +seed bytes for every key involved (initiator identity, responder identity, signed +prekey, one-time prekey, ephemeral key — each a `Uint8Array` of 32 sequential bytes +starting at a different offset) and asserts the resulting signed-prekey public key, +one-time public key, ephemeral public key, derived session key, and associated data all +match fixed base64 constants. This is the canonical place to check this document's X3DH +claims against: if `deriveSessionKey`'s construction ever changes, this is the test that +catches it, since the expected `sessionKey` constant would no longer match. + +## Implementation references + +- `apps/web/src/lib/x3dh.ts` — X3DH session establishment +- `apps/web/src/lib/crypto/doubleRatchet.ts` — Double Ratchet state machine and AEAD +- `apps/web/src/lib/x3dh.vectors.test.ts` — deterministic test vectors +- `apps/backend/src/lib/keys.ts` — the backend's identity-key verification, sharing the same Ed25519 key this document describes converting for DH +- Signal X3DH specification: https://signal.org/docs/specifications/x3dh/ +- Signal Double Ratchet specification: https://signal.org/docs/specifications/doubleratchet/ diff --git a/docs/README.md b/docs/README.md index b2f8d94..7b74ecd 100644 --- a/docs/README.md +++ b/docs/README.md @@ -97,6 +97,7 @@ listener. | [Database migration workflow](../apps/backend/docs/migrations.md) | The drizzle-kit loop from `schema.ts` to applied SQL, the `drizzle/` layout, and how to resolve the colliding-migration merge conflict that has already broken this history once. | | [E2EE onboarding](../apps/backend/docs/e2ee-onboarding.md) | Device registration and prekey upload flow for first-contact DM setup. | | [E2EE key management concepts](../apps/backend/docs/concepts-e2ee-key-management.md) | The full backend-side key lifecycle — registration, prekey upload/consumption, revocation — and an explicit statement of what the server can and cannot see. | +| [Flow: linking a new device](../docs/flows/device-linking.md) | The challenge/verify handshake end to end, what history a new device can and cannot read, the key-change notice peers see, revocation as the reverse flow, and a real gap: no client UI for this flow yet. | | [MLS key packages](../apps/backend/docs/mls-key-packages.md) | Key package publication, consumption, and replenishment. | | [MLS group membership](../apps/backend/docs/mls-group-membership.md) | Adding and removing members from an MLS group and the resulting epoch changes. | | [MLS group files](../apps/backend/docs/mls-group-files.md) | How file keys are distributed to an MLS group. | @@ -115,6 +116,7 @@ is about encryption and local state. | ----------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | | [Web app README](../apps/web/README.md) | Running the Next.js client on its own, its scripts, and its environment variables. | | [E2EE architecture](../apps/web/docs/concepts-e2ee-architecture.md) | Where keys live in the browser, how sessions are established, and what never leaves the device. | +| [X3DH and Double Ratchet implementation notes](../apps/web/docs/concepts-x3dh-double-ratchet.md) | `lib/x3dh.ts` and `lib/crypto/doubleRatchet.ts` checked against the Signal specs: the Ed25519/X25519 identity-key conversion, the KDF construction, skipped-key limits, and every deviation found. | | [Message pipeline](../apps/web/docs/concepts-message-pipeline.md) | The client-side path from typed text to an encrypted envelope on the wire, and back. | | [Auth and device lifecycle](../apps/web/docs/concepts-auth-device-lifecycle.md) | Wallet connection, device registration, session persistence, and revocation handling. | | [File encryption](../apps/web/docs/concepts-file-encryption.md) | How attachments are encrypted client-side before upload. | @@ -211,6 +213,8 @@ Documents about the repository itself rather than about the product. | Document | What it gives you | | ------------------------------------------------------------ | --------------------------------------------------------------------------------------------------- | | [Security policy](../SECURITY.md) | How and where to report a vulnerability privately, and why never in a public issue or pull request. | +| [Known limitations and unwired code register](known-limitations.md) | Code that exists but isn't live, hardcoded values awaiting configuration, and the inconsistencies and gaps found while writing this documentation batch. | +| [Scripts and Makefile reference](scripts-reference.md) | Every runnable script and Makefile target: purpose, required env vars, side effects, and whether CI actually invokes it. | | [Testing strategy and conventions](testing.md) | Cross-app testing philosophy, runners, and the conventions a contributor must follow. | | [Pull request template](../.github/pull_request_template.md) | The checklist every pull request is opened against. | | [PR notes](../pr.md) | Scratch notes for an in-flight pull request; not a reference document. | diff --git a/docs/flows/device-linking.md b/docs/flows/device-linking.md new file mode 100644 index 0000000..2959bae --- /dev/null +++ b/docs/flows/device-linking.md @@ -0,0 +1,227 @@ +# Flow: linking a new device + +Adding a second device to an account, start to finish: the challenge/verify handshake +that proves the new device is controlled by the account owner, publishing that device's +key material, how existing devices and MLS groups learn about it, and the reverse flow +(revocation). + +## Contents + +- [Sequence diagram](#sequence-diagram) +- [Step 1: challenge](#step-1-challenge) +- [Step 2: verify and register](#step-2-verify-and-register) +- [Step 3: key material upload](#step-3-key-material-upload) +- [What history the new device can and cannot read](#what-history-the-new-device-can-and-cannot-read) +- [What other users see: the key-change notice](#what-other-users-see-the-key-change-notice) +- [Audit events](#audit-events) +- [Revocation: the reverse flow](#revocation-the-reverse-flow) +- [A real gap: no client UI for this flow yet](#a-real-gap-no-client-ui-for-this-flow-yet) +- [Implementation references](#implementation-references) + +## Sequence diagram + +```mermaid +sequenceDiagram + autonumber + participant New as New device + participant API as Backend + participant Existing as Existing device(s) + + New->>API: POST /devices/link/challenge (bearer: account JWT) + API->>API: resolveAccountWallet(userId) + API->>API: createDeviceLinkNonce(userId) + API-->>New: { message, nonce, walletAddress } + Note over New: sign `message` with the account's Stellar wallet + New->>API: POST /devices/link/verify
{ nonce, signature, identityPublicKey, deviceName, platform, registrationId } + API->>API: consumeDeviceLinkNonce (burns it) + API->>API: verifyWalletSignature(...) + API->>API: INSERT devices row (or UPDATE if re-linking a revoked identity key) + API-->>New: 201 { id, createdAt } + API->>API: emitDeviceChangeEvent(userId, 'device_added') + API->>Existing: new_message (system, change: device_added) — every shared conversation + New->>API: POST /devices/:id/prekeys + New->>API: POST /devices/:id/mls-key-packages (per MLS group) + Note over Existing,API: for each MLS group — an existing member must Commit + Welcome the new device in (see below) +``` + +## Step 1: challenge + +`POST /devices/link/challenge` — authenticated as the account (an existing session's +JWT, not yet the new device's own session), rate-limited to 10 requests/minute +(`device_link_challenge`, a separate bucket from the auth challenge/verify limits +specifically so hammering device-linking can't lock out normal sign-in): + +```ts +const walletAddress = await resolveAccountWallet(userId); +const nonce = createDeviceLinkNonce(userId); +res.json({ message: deviceLinkMessage(userId, nonce), nonce, walletAddress }); +``` + +Fails with 400 if the account has no wallet associated (`walletAddress` is `null`) — +device linking requires a wallet signature, so an account that never connected one +cannot add a second device via this flow at all. + +## Step 2: verify and register + +`POST /devices/link/verify` — rate-limited tighter, 5/minute (`device_link_verify`), +since this is the signature-checking step an attacker would actually want to brute +force: + +1. **The nonce is consumed first, before any signature work**: + `consumeDeviceLinkNonce(userId, body.nonce)` — single-use by construction. This + ordering matters: a nonce cannot be reused to retry signature verification against + the same challenge, and a replayed verify request (same nonce, any signature) always + fails on the second attempt regardless of whether the first attempt's signature was + valid. +2. **Signature check**: `verifyWalletSignature(walletAddress, deviceLinkMessage(userId, + body.nonce), body.signature)` — this is what actually proves the request came from + whoever controls the account's wallet, not merely whoever has a valid JWT for the + account. +3. **Registration**: looks up an existing `devices` row for `(userId, + identityPublicKey)`. + - If one exists and is **not** revoked: `409 Device already registered for this + user` — the same identity key cannot be linked twice while active. + - If one exists and **is** revoked: the row is updated (`deviceName`, `platform`, + `registrationId`, `revokedAt = null`) rather than inserting a new row. Re-linking a + previously-revoked identity key reactivates it; it does not create a duplicate + device for the same cryptographic identity. + - Otherwise: a fresh `devices` row is inserted. +4. **Side effects on success**: `emitDeviceChangeEvent(userId, 'device_added')` (see + [below](#what-other-users-see-the-key-change-notice)) and an audit event (see + [below](#audit-events)), both fired without being awaited (`void`) — a failure in + either does not fail the registration response itself. + +## Step 3: key material upload + +Registration alone does not make the new device usable — it still needs to publish the +key material other devices will use to establish sessions with it: + +- `POST /devices/:id/prekeys` — signed prekey and one-time prekeys (see + [`concepts-x3dh-double-ratchet.md`](../../apps/web/docs/concepts-x3dh-double-ratchet.md) + for what these are and how a sender uses them). +- `POST /devices/:id/mls-key-packages` — one MLS `KeyPackage` per MLS group the device + needs to be addable to. This route rejects with 403 if the device is already revoked + (a device can't publish key material for a conversation after it's been locked out) or + if the caller isn't that device's owner. Uploads are idempotent — re-sending a package + the device already published counts as a duplicate rather than storing it twice, so a + client retrying a timed-out request doesn't inflate its own inventory. + +For **MLS groups specifically**, publishing a key package is necessary but not +sufficient for the new device to actually be addable in that group: an existing member +must pick up the new device (via `GET /conversations/:id/mls/pending-devices`) and +perform the Add-proposal + Commit + Welcome flow described in +[`mls-group-membership.md`](../../apps/backend/docs/mls-group-membership.md#join-flow-for-a-newly-linked-device). +This is not automatic — see that document for the full sequence and why it's a separate, +decoupled step. + +## What history the new device can and cannot read + +**DMs**: a newly-linked device has no server-side history to catch up on beyond what +`/sync`'s envelope-catch-up endpoint delivers going forward (undelivered envelopes for +its own device id, from the point its prekeys/registration exist onward — see +[`concepts-message-edit-delete.md`](../../apps/backend/docs/concepts-message-edit-delete.md#what-an-offline-device-sees-on-reconnect--sync) +for exactly what that endpoint filters). There is no mechanism that re-encrypts past DM +history for a newly-added device; it sees new messages from the moment it's linked +onward, nothing retroactive. + +**MLS groups**: the same "no history transfer" property holds, for a cryptographic +reason rather than just an unimplemented feature — see +[`mls-group-membership.md`'s "Why history does not transfer"](../../apps/backend/docs/mls-group-membership.md#why-history-does-not-transfer). +A device added at epoch 7 holds epoch-7-and-later secrets only; there is no key on the +server to backfill earlier epochs with, because the server never had one. Backfilling +MLS history would require an existing device to re-encrypt and re-send old plaintext +specifically for the new device — a separate, deferred feature, not something this +linking flow does. + +## What other users see: the key-change notice + +`emitDeviceChangeEvent(userId, change)` (`routes/devices.ts`) runs on both linking +(`'device_added'`) and revocation (`'device_revoked'`) and does the same thing either +way: for **every conversation** the affected user is a member of, it inserts a `system`- +type message with no ciphertext and a structured `systemPayload: { userId, change }`, +then broadcasts it: + +```ts +io.to(conversationRoom(m.conversationId)).emit('new_message', msg); +io.to(m.conversationId).emit('new_message', msg); // backward-compat room +``` + +This is the same double-room-emit pattern (two different Socket.IO room names, both +joined by every connected socket, so the event arrives twice per device-change) already +documented for `delete_message` in +[`concepts-message-edit-delete.md`](../../apps/backend/docs/concepts-message-edit-delete.md#delete_message) — +not unique to this flow, a recurring pattern in this codebase's socket layer. Every other +member of every shared conversation receives this notice, giving clients the hook to +show something like "Alice added a new device" or "Alice's device was removed" inline in +the conversation, without the server naming or fingerprinting the specific device in the +payload — only the change type and whose account changed. + +## Audit events + +Both `device_linked` (on successful `/link/verify`) and `device_revoked` (on `DELETE +/devices/:id`) are recorded via `recordAuditEvent`, each carrying: + +- `targetType: 'device'`, `targetId`: the device's id +- `deviceName`, `platform` (as supplied/stored for that device) +- `device_linked` additionally records `reactivatedRevokedDevice: boolean` — whether this + was a brand-new device or a previously-revoked identity key being relinked; the + distinction matters because reactivating a revoked key after a revocation that was + meant to lock someone out is a meaningfully different event from a genuinely new + device appearing. +- `device_revoked` additionally records `selfRevocation: boolean` (whether the device + being revoked is the same one making the request) and `remainingActiveDevices` + (the count after this revocation) — revoking the device you're currently using reads + very differently from revoking one you no longer hold, and a low remaining count is + itself a signal worth having in the audit trail. + +## Revocation: the reverse flow + +`DELETE /devices/:id` — idempotent (revoking an already-revoked device just echoes back +the existing `revokedAt` rather than erroring) and refuses to revoke a caller's **only** +active device (`409 Cannot revoke your only active device` — there would be no way back +into the account). `POST /devices/logout-everywhere` is the same operation applied to +every device except the caller's current one in one call. + +What revocation actually does, in `revokeDeviceRow`: + +1. Sets `devices.revokedAt`/`updatedAt` — this is the authoritative "is this device + still trusted" flag every other check in the codebase reads. +2. **Deletes** (not soft-deletes) every row in `devicePrekeys` for that device — a + revoked device's prekeys are gone outright, not merely flagged, so nothing can be + claimed against them going forward. +3. Releases any "prekeys low" latch for the device, so a later re-registration of the + same identity key doesn't suppress the first legitimate low-prekey-count signal. +4. Force-disconnects any live socket for the device, cross-node via Redis pub/sub + (`device_revoked:${deviceId}`) — a device that's mid-session when revoked doesn't + get to finish that session. + +**What's notably not deleted**: MLS key packages for the device are left in place by +`revokeDeviceRow` — only `devicePrekeys` rows are removed. A revoked device's +already-uploaded MLS key packages remain in the table (though the device itself can no +longer be meaningfully added to anything, since `is this device trusted` is gated on +`revokedAt`, checked independently wherever a key package would be consumed). + +Peers see the same `device_revoked` key-change notice described +[above](#what-other-users-see-the-key-change-notice) — there is no separate "device +revoked" notification channel; it is exactly the same system-message mechanism linking +uses, just with a different `change` value. + +## A real gap: no client UI for this flow yet + +A repo-wide search for the linking endpoints' names found no caller in `apps/web/src` — +`/devices/link/challenge`, `/devices/link/verify`, `deviceLinkMessage`, and +`createDeviceLinkNonce` have zero references on the client side. The devices management +page (`app/app/devices/page.tsx`) only implements **revocation** of existing devices; it +has no "link a new device" action. The backend flow described above is fully +implemented and presumably reachable via a direct API call, but as of this writing there +is no UI path for a user to actually add a second device to their account through the +web client. + +## Implementation references + +- `apps/backend/src/routes/devices.ts` — `/link/challenge`, `/link/verify`, prekey/MLS-key-package upload, `DELETE /:id`, `/logout-everywhere`, `emitDeviceChangeEvent`, `revokeDeviceRow` +- `apps/backend/src/config/rateLimits.ts` — `device_link_challenge`, `device_link_verify` +- `apps/web/src/app/app/devices/page.tsx` — the devices management page (revocation only) +- [`mls-group-membership.md`](../../apps/backend/docs/mls-group-membership.md) — the group-join Commit/Welcome flow and why MLS history doesn't transfer to a newly-added device +- [`concepts-message-edit-delete.md`](../../apps/backend/docs/concepts-message-edit-delete.md) — the double-room-emit pattern this flow's key-change notice also exhibits, and the `/sync` envelope catch-up a newly-linked device relies on +- [`concepts-x3dh-double-ratchet.md`](../../apps/web/docs/concepts-x3dh-double-ratchet.md) — what the uploaded prekeys are for diff --git a/docs/known-limitations.md b/docs/known-limitations.md new file mode 100644 index 0000000..bafbdfe --- /dev/null +++ b/docs/known-limitations.md @@ -0,0 +1,52 @@ +# Known limitations and unwired code register + +A maintained register of code that exists in this repository but is not live, behavior +that is deliberately incomplete, and hardcoded values awaiting configuration. Without +this list, a contributor can easily mistake a stub for a bug, "fix" a deliberate limit +that's actually intentional, or build new work on top of code that looks wired in but +isn't. + +**The rule: an entry is removed in the same PR that resolves it.** If you fix one of +these, delete its row here as part of that PR rather than leaving a stale entry (or a +separate follow-up PR) to clean up later. + +Each entry below was verified directly against the current source at the time this +document was written, not copied from the issue that requested it. + +## Register + +| # | Description | Location | Impact | Tracked by | +|---|---|---|---|---| +| 1 | `auditLogsRouter` is fully implemented (`GET /audit-logs`, admin-gated) but never imported or mounted in `app.ts`. The route is completely unreachable — every request to it 404s, regardless of auth or role. | `apps/backend/src/routes/auditLogs.ts` (defined), `apps/backend/src/app.ts` (not mounted) | Audit logs recorded via `recordAuditEvent` (device linking, revocation, and elsewhere) accumulate in the database with no way to read them back through the API. | #648, #650 | +| 2 | The AI agent URL in `ask_assistant` is a literal string, `http://localhost:8000/chat`, not read from `process.env` or `config/*` the way every other external dependency (database, Redis, S3) is. | `apps/backend/src/socket/messaging.ts` | In any deployment where the backend and the AI agent (`apps/ai_agent`) are not on the same host with the agent on port 8000 (e.g. separate `docker-compose` services), every `@assistant` call fails with a connection error. See [`concepts-ai-assistant-bridge.md`](../apps/backend/docs/concepts-ai-assistant-bridge.md#the-hardcoded-agent-url) for the full trace. | #651 | +| 3 | `lib/crypto/ratchetSession.ts` is a complete, IndexedDB-backed Double Ratchet session layer with zero importers anywhere in `apps/web/src`. The live session path is `Phase1SessionCrypto` (entry 4 below), a separate and simpler implementation. `ratchetSession.ts`'s own comment states it deliberately rejects out-of-order messages, unlike `lib/crypto/doubleRatchet.ts` (which supports out-of-order delivery via bounded skipped-key tracking) — the two are explicitly noted as "not yet wired together" in `doubleRatchet.ts`'s own docstring. | `apps/web/src/lib/crypto/ratchetSession.ts` | A fully-built session layer sits unused; a contributor extending E2EE session handling could reasonably assume this is the live code path and build on it, when it is not reachable from any real message flow. | #681 | +| 4 | `Phase1SessionCrypto` (`lib/session.ts`) is the live `defaultSession` export, not `LibsignalSessionCrypto` — both classes exist in the same file, and the file's own comment names this as "Phase-1" with "Phase-2 (Signal Protocol)" as a documented future swap, not yet made. | `apps/web/src/lib/session.ts` | The deployed session-establishment behavior is the simpler Phase-1 implementation, not the full Signal-protocol-equivalent path the X3DH/Double Ratchet modules (`lib/x3dh.ts`, `lib/crypto/doubleRatchet.ts`) implement in isolation. See [`concepts-x3dh-double-ratchet.md`](../apps/web/docs/concepts-x3dh-double-ratchet.md) for what those modules do on their own, independent of whether `session.ts` currently calls into them. | none yet — open one if picking this up | +| 5 | Inconsistent `NEXT_PUBLIC_API_URL`/`NEXT_PUBLIC_BACKEND_URL` defaults across the web app: `lib/api.ts` falls back to `http://localhost:4000` if `NEXT_PUBLIC_API_URL` is unset, while `components/chat/NewConversationModal.tsx` falls back to `http://localhost:3001` for the *same* env var, and `lib/socket.ts` reads an entirely different env var (`NEXT_PUBLIC_BACKEND_URL`/`NEXT_PUBLIC_SOCKET_URL`) defaulting to `3001`. The backend's actual default port (`apps/backend/src/index.ts`) is `3001` — so `lib/api.ts`'s `4000` default is simply wrong for an unconfigured local setup, not just inconsistent in spelling. | `apps/web/src/lib/api.ts`, `apps/web/src/components/chat/NewConversationModal.tsx`, `apps/web/src/lib/socket.ts` | Running the web app with `NEXT_PUBLIC_API_URL` unset (e.g. a fresh local checkout before writing a `.env`) sends REST calls to the wrong port while sockets connect correctly, a confusing partial-failure rather than a clean "nothing works" signal. | #629 (documents this; does not fix it) | +| 6 | Four files exist that are not part of either real app and are imported by nothing: `apps/web/src/service/cacheService.js` (Redis cache helpers, Express-shaped, inside a Next.js client app with no server-side Redis access of its own), `apps/web/src/app/routes/conversations.js` (an Express `Router`-based route file sitting inside the Next.js App Router directory, which does not use Express routing at all), `src/components/ui/WalletAddress.tsx` at the **repository root** (every other web component lives under `apps/web/src/`), and `apps/tests/routes/conversations.test.js` (an integration test for the orphaned route file above, requiring paths — `../../src/app`, `../../src/config/redis` — that do not resolve to anything that exists). `express` is not a declared dependency of `apps/web`. | `apps/web/src/service/cacheService.js`, `apps/web/src/app/routes/conversations.js`, `src/components/ui/WalletAddress.tsx`, `apps/tests/routes/conversations.test.js` | Misleads a contributor into thinking there's a second, Express-based server implementation somewhere, or that `WalletAddress.tsx` is a real, usable component at the repo root rather than orphaned. None of these four files are reachable from any build, lint, or test command that actually runs in CI. | #679 | + +## Keeping this register honest + +- **Verify before listing.** An entry in this table is a claim that a specific file, + at a specific path, currently behaves the way the entry describes. Before adding a + new row, confirm it directly (grep for importers, read the actual default value, + run the actual command) — don't transcribe a bug report's wording without checking + it still matches the code. +- **Link or open an issue.** Every entry needs a tracking issue. If none exists for a + limitation you're documenting, open one (see entry 3's #681 for the pattern) rather + than leaving the "Tracked by" column empty. +- **Remove the row when it's fixed**, in the same PR, not a follow-up. A stale "known + limitation" that's actually been resolved is worse than no register at all — it + actively misleads the next person who reads it. +- **This is not a general TODO list.** Things that are simply unfinished features with + their own tracking issue and no risk of being mistaken for live/wired code don't + belong here. This register is specifically for the "looks real, isn't" and "looks + like a bug, is deliberate" cases. + +## Implementation references + +- `apps/backend/src/routes/auditLogs.ts`, `apps/backend/src/app.ts` (entry 1) +- `apps/backend/src/socket/messaging.ts` (entry 2) — see also [`concepts-ai-assistant-bridge.md`](../apps/backend/docs/concepts-ai-assistant-bridge.md) +- `apps/web/src/lib/crypto/ratchetSession.ts`, `apps/web/src/lib/crypto/doubleRatchet.ts` (entry 3) — see also [`concepts-x3dh-double-ratchet.md`](../apps/web/docs/concepts-x3dh-double-ratchet.md) +- `apps/web/src/lib/session.ts` (entry 4) +- `apps/web/src/lib/api.ts`, `apps/web/src/components/chat/NewConversationModal.tsx`, `apps/web/src/lib/socket.ts`, `apps/backend/src/index.ts` (entry 5) +- `apps/web/src/service/cacheService.js`, `apps/web/src/app/routes/conversations.js`, `src/components/ui/WalletAddress.tsx`, `apps/tests/routes/conversations.test.js` (entry 6) diff --git a/docs/scripts-reference.md b/docs/scripts-reference.md new file mode 100644 index 0000000..0d32ad9 --- /dev/null +++ b/docs/scripts-reference.md @@ -0,0 +1,202 @@ +# Scripts and Makefile reference + +Every runnable script and Makefile target in this repository, what it does, what it +needs to run, what it changes as a side effect, and whether CI actually invokes it. + +## Contents + +- [Root `Makefile`](#root-makefile) +- [`scripts/start-web.sh`](#scriptsstart-websh) +- [`scripts/audit-crypto-deps.mjs`](#scriptsaudit-crypto-depsmjs) +- [`scripts/loadtest/`](#scriptsloadtest) +- [`apps/backend/src/scripts/check-ciphertext-invariant.mjs`](#appsbackendsrcscriptscheck-ciphertext-invariantmjs) +- [`contracts/scripts/deploy_*.sh`](#contractsscriptsdeploy_sh) +- [Root `package.json` scripts and per-app equivalents](#root-packagejson-scripts-and-per-app-equivalents) + +## Root `Makefile` + +| Target | What it runs | Requires | Side effects | CI? | +|---|---|---|---|---| +| `dev` | `docker compose -f infra/docker-compose.yml up -d` then `pnpm dev` | Docker, pnpm, workspace deps installed | Starts Postgres + Redis containers; starts every app's dev server via Turborepo | No — local dev only | +| `migrate` | `pnpm --filter backend db:migrate` (→ `drizzle-kit migrate`) | A reachable `DATABASE_URL` | Applies pending Drizzle migrations to that database | No — see [migrations.md](../apps/backend/docs/migrations.md) for how CI/deploy actually applies migrations | +| `test` | `pnpm --filter backend test` then `cd contracts && cargo test` | Backend test deps + a Rust toolchain with `wasm32-unknown-unknown` | Runs the backend's vitest suite, then every contract's `cargo test` (all three packages, since plain `cargo test` with no `-p` runs the whole workspace) | No — CI's own workflows invoke these tool directly (`backend-ci.yml`, `contracts-ci.yml`), not through this target | +| `lint` | `pnpm lint` (→ `turbo run lint`) | Workspace deps installed | Runs every app's own `lint` script via Turborepo | No — CI invokes each app's lint directly | +| `deploy-contracts` | `bash contracts/scripts/deploy_token_transfer.sh` then `bash contracts/scripts/deploy_group_treasury.sh` | `stellar` CLI, `DEPLOYER_SECRET`, `TOKEN_CONTRACT_ID` (see each script's own prerequisites below) | Deploys and initializes two contracts to the configured network | No | + +**A real gap, verified against the actual scripts**: `deploy-contracts` runs only two of +the three deploy scripts. `contracts/scripts/deploy_proposals.sh` exists, has its own +documented prerequisites and usage line, and is never called by this target — deploying +`proposals` requires running that script directly, not `make deploy-contracts`. + +None of these targets are referenced by any `.github/workflows/*.yml` file (confirmed via +search) — every CI job invokes the underlying `pnpm`/`cargo`/`stellar` commands directly +rather than going through the Makefile. The Makefile is a local-development convenience +layer, not part of the CI contract. + +## `scripts/start-web.sh` + +```bash +cd apps/web || exit 1 +pnpm run dev +``` + +A thin wrapper equivalent to `pnpm --filter web dev` from the repo root, or `cd apps/web +&& pnpm dev` directly. Requires pnpm and the web app's dependencies installed. No side +effects beyond starting the Next.js dev server; not invoked by CI. + +## `scripts/audit-crypto-deps.mjs` + +```bash +node scripts/audit-crypto-deps.mjs +``` + +Runs `pnpm audit --json`, then filters the results down to advisories affecting a fixed +allow-list of **crypto-relevant** packages — `ioredis`, `jsonwebtoken`, `web-push`, +`@stellar/stellar-sdk`, `drizzle-orm`, `socket.io`, `@socket.io/redis-adapter`, `redis`, +`jose` — and exits non-zero only if one of those specific packages has an advisory. +Advisories against any other dependency (the vast majority of a typical `pnpm audit` +output) do not fail the build. + +**How the filter actually decides a match**, since the script's own top comment says it +catches a package "or is a dependency of" one on the list: the filter is + +```js +CRYPTO_RELEVANT_PACKAGES.some((pkg) => moduleName === pkg || moduleName.startsWith(`${pkg}/`)) +``` + +— an exact name match, or a subpath-prefix match (e.g. `socket.io/lib/...` would match +`socket.io`). **This does not walk the actual dependency graph** to catch a +transitively-vulnerable package that isn't itself named with one of these prefixes — a +CVE in, say, a sub-dependency of `ioredis` that `pnpm audit` reports under its own +package name (not `ioredis/...`) would not match this filter and would not fail the +build, despite the comment's "is a dependency of" phrasing suggesting it would. + +Requires: `pnpm audit` to be runnable (a `pnpm-lock.yaml` and network access, or a cached +audit database). No side effects — read-only, reports and exits. + +**CI**: `.github/workflows/security-ci.yml`'s `dependency-audit` job runs this script +directly on every push to `main` and every pull request. + +## `scripts/loadtest/` + +Two scripts plus a committed baseline, for WebSocket gateway load/soak testing (#385): + +### `seed.ts` + +```bash +tsx scripts/loadtest/seed.ts --devices 200 > fixture.json +``` + +Creates N users (default 200), one device each, a single group conversation containing +all of them, and signs a JWT per device — writing the fixture (`conversationId` plus +each participant's `userId`/`deviceId`/`token`) to stdout as JSON. Requires a reachable +database (it writes real rows via the backend's own Drizzle client and schema) and the +backend's JWT-signing secret configured, since it calls the backend's real `signToken`. +**Side effect: inserts real users, devices, and a conversation into whatever database +`DATABASE_URL` points at** — point this at a disposable test database, not production. + +### `run.ts` + +```bash +tsx scripts/loadtest/run.ts --fixture fixture.json \ + --nodes http://localhost:3001,http://localhost:3002 \ + [--baseline scripts/loadtest/baseline.json] \ + [--out result.json] +``` + +Connects every fixture participant as a real `socket.io-client`, split across the +`--nodes` URLs so the run exercises cross-instance fan-out through the Redis adapter, +then runs three phases: fan-out latency (one sender, every other device must receive +`message_envelope`), presence churn (a fraction of devices disconnect/reconnect on an +interval), and a reconnect storm (every device disconnects and reconnects at once). +Samples peak RSS every second. Requires the backend actually running (one or more +instances at the `--nodes` URLs) and the fixture `seed.ts` produced. Prints a JSON result +with p50/p95/p99 latency, peak memory, and error counts; **exits non-zero if any +threshold in the script's `THRESHOLDS` is violated, or if `--baseline` is supplied and +this run regressed beyond the script's regression tolerance** compared to that baseline. + +### `baseline.json` + +A committed reference result (`deviceCount: 200`, `nodeCount: 2`, p50/p95/p99 latency, +peak RSS) that `run.ts --baseline` compares a new run against. + +**CI**: `.github/workflows/loadtest-nightly.yml` runs the full `seed.ts` → `run.ts` +pipeline against two real backend instances (ports 3001/3002, sharing one Redis) on a +nightly schedule (03:00 UTC) and via manual `workflow_dispatch` — it is not a per-PR +gate, by design (per the workflow's own header comment: "this is a soak test, not a +per-PR gate"). The job always passes `--baseline scripts/loadtest/baseline.json`, so a +nightly run that regresses beyond tolerance against this committed file fails the +workflow (visible as a failed scheduled run, not as a status check on any PR). + +## `apps/backend/src/scripts/check-ciphertext-invariant.mjs` + +```bash +cd apps/backend && node src/scripts/check-ciphertext-invariant.mjs +``` + +Scans every `.ts`/`.tsx`/`.js`/`.jsx` file under `apps/backend/src` (excluding the script +itself, `src/lib/ciphertextInvariant.ts`, and any `__tests__` directory) for an +object-key pattern matching a fixed list of forbidden field names — `content`, `body`, +`plaintext`, `identityPrivateKey`/`identity_private_key`, `privateKey`/`private_key`, +`sessionKey(s)`/`session_key(s)`, `mlsSecret(s)`/`mls_secret(s)`, +`messageKey(s)`/`message_key(s)`, `ratchetState`/`ratchet_state` — appearing as an +object property key (`` `field`: `` or `'field':` or a bare identifier key followed by +`:`). Exits non-zero and lists every match if any are found. + +**This script is not invoked by any CI workflow** (confirmed via search of +`.github/workflows/`). `security-ci.yml`'s `regression` job runs a *different* check +with a similar name and purpose — `pnpm test -- security.regression.test.ts`, a vitest +test, not this script — so the ciphertext-only guard CI actually enforces is the test +file, not this standalone script. Whether this script is meant to be run manually as a +stricter/complementary check, or is a superseded earlier version of the same guard, isn't +stated anywhere in the script or the workflow; it's simply not part of the enforced CI +contract today. + +## `contracts/scripts/deploy_*.sh` + +Three scripts, one per contract, each following the same shape (`set -euo pipefail`, +testnet-only, requiring the `stellar` CLI and `DEPLOYER_SECRET`): + +| Script | Additional required env vars | Deploys | +|---|---|---| +| `deploy_token_transfer.sh` | `TOKEN_CONTRACT_ID` (the SEP-41 token to route transfers through) | `token_transfer` | +| `deploy_group_treasury.sh` | `TOKEN_CONTRACT_ID` (the SEP-41 token the treasury holds) | `group_treasury` | +| `deploy_proposals.sh` | `TREASURY_CONTRACT_ID`, `MIN_VOTES` | `proposals` | + +Each reads its contract's compiled wasm from +`target/wasm32-unknown-unknown/release/.wasm` — see +[`api-deployment-invocation.md`](../contracts/docs/api-deployment-invocation.md#2-building-the-contracts-verified) +for the build step that produces it. None of these three scripts are invoked by any CI +workflow; deployment is a manual, explicit action, not something that happens as a side +effect of merging code. As noted [above](#root-makefile), the root `Makefile`'s +`deploy-contracts` target only wraps two of these three. + +## Root `package.json` scripts and per-app equivalents + +| Root script | What it does | Per-app equivalent | +|---|---|---| +| `dev` | `turbo run dev` — fans out to every app's own `dev` script in parallel | `apps/web`: `next dev`. `apps/backend`: `tsx watch src/index.ts`. | +| `build` | `turbo run build` | `apps/web`: `next build`. `apps/backend`: `tsc`. | +| `lint` | `turbo run lint` | `apps/web`: `eslint`. `apps/backend`: `eslint src`. | +| `format` / `format:check` | `prettier --write`/`--check` over `**/*.{ts,tsx,md,json}` repo-wide | `apps/backend` has its own narrower `format`/`format:check` (`src/**/*.ts` only); `apps/web` has no separate format script, so the root one is its only formatter entry point. | + +**Root-level `devDependencies` worth knowing about**: `express`, `supertest`, +`jsonwebtoken`, and `ioredis` are declared at the repo root, not inside `apps/backend` +(which has its own copies) or `apps/web` (which has neither `express` nor `supertest` as +a real dependency at all). These root-level packages are what let the orphaned +Express-style files described in +[`known-limitations.md`](./known-limitations.md#register) (entry 6: `cacheService.js`, +`routes/conversations.js`, and their test) resolve their imports locally without +actually being part of any real, buildable app — a contributor could run `node` against +one of those files directly and have its `require`/`import` of `express` succeed, which +is part of why they're easy to mistake for live code. + +## Implementation references + +- `Makefile` +- `scripts/start-web.sh`, `scripts/audit-crypto-deps.mjs`, `scripts/loadtest/seed.ts`, `scripts/loadtest/run.ts`, `scripts/loadtest/baseline.json` +- `apps/backend/src/scripts/check-ciphertext-invariant.mjs` +- `contracts/scripts/deploy_token_transfer.sh`, `deploy_group_treasury.sh`, `deploy_proposals.sh` +- `.github/workflows/security-ci.yml` — the only CI workflow that invokes a script covered in this document directly +- `package.json`, `apps/web/package.json`, `apps/backend/package.json` +- [`known-limitations.md`](./known-limitations.md) — the orphaned files whose imports the root-level `express`/`supertest` devDependencies happen to satisfy