From 935eb74eca1e6bbabe91ca8d7486d5a73826d53d Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 11:05:25 -0500 Subject: [PATCH 01/14] Own Matrix E2EE in the adapter Olm/Megolm state moves onto the chat_matrix() client, so E2EE becomes a property of the client a consumer already holds rather than a second API it has to learn. corteza carried this in R/matrix_crypto.R, in parallel to the contract, and reached around chat_poll()$raw to decrypt. - chat_matrix() gains e2ee and crypto_store. e2ee = FALSE is the default and is the previous behaviour exactly: no crypto context, no store touched, no keys published. - chat_send() routes encrypted rooms through mx_send_encrypted(), building the same m.room.message content the cleartext path PUTs, so markdown and mentions render the same either way. The room is asked about per send rather than once at init: a room that turns on encryption between polls must not get one cleartext message first. - chat_poll() folds decrypted events into the messages list it already builds, as ordinary chat_message records. A decrypt that throws warns and loses that traffic; the poll still returns the cleartext messages and advances the cursor, so one missing Megolm session cannot stall the loop. - chat_message() gains encrypted and sender_verified. sender_verified is NULL on cleartext, where the transport asserts the sender and there is nothing to verify, and FALSE on an encrypted message whose claimed sender did not bind to a verified device -- a real answer, not a missing one. - chat_capabilities()$e2ee answers for this client, from whether it holds a crypto context, not from whether mx.crypto happens to be installed. Reporting TRUE off an install would invite a consumer to hand a secret to a client that PUTs it in the clear. Two fixes to what was ported. The crypto store is keyed on the app namespace via mx_crypto_store_dir() instead of dirname(config)/crypto: corteza's version tied the device identity to wherever the config file sat, so moving the config silently minted a new identity and lost every Megolm session. Because that key is the app name alone, an e2ee client built from an explicit path with no app is now an error rather than a guess -- two bots built that way would have shared one Olm account and the second would have come up wearing the first's device keys. And the encrypted-room cache is one room id per line instead of JSON, since chat.api has no dependencies and a character vector needs no parser. A .crypto seam replaces the four crypto operations, which is what lets the e2ee = TRUE paths be tested on a runner with neither mx.crypto nor a Rust toolchain. 69 new assertions; each was checked against a mutated source. 299/299 pass, R CMD check --as-cran clean but for the expected new-submission NOTE. mx.client bound raised to 0.2.0 for mx_crypto_known_devices() and mx_crypto_process_sync(devices = ), which is what makes sender_verified answerable. --- DESCRIPTION | 6 +- R/contract.R | 17 +- R/matrix-crypto.R | 256 +++++++++++++++++++++++++++++ R/matrix.R | 121 ++++++++++++-- inst/tinytest/test_matrix.R | 316 +++++++++++++++++++++++++++++++++++- man/chat_matrix.Rd | 43 ++++- man/chat_message.Rd | 29 +++- man/chat_send.Rd | 24 ++- man/chat_slack.Rd | 9 +- 9 files changed, 784 insertions(+), 37 deletions(-) create mode 100644 R/matrix-crypto.R diff --git a/DESCRIPTION b/DESCRIPTION index 489823f..97b2313 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,8 +1,8 @@ Package: chat.api Type: Package Title: Transport-Agnostic Chat Contract for R Agents -Version: 0.0.1.2 -Date: 2026-07-20 +Version: 0.0.1.3 +Date: 2026-08-05 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), email = "troy@cornball.ai", @@ -21,7 +21,7 @@ URL: https://github.com/cornball-ai/chat.api BugReports: https://github.com/cornball-ai/chat.api/issues Suggests: mx.api, - mx.client (>= 0.1.1.2), + mx.client (>= 0.2.0), mx.crypto, slackr, tinytest diff --git a/R/contract.R b/R/contract.R index 80a40e2..afb34b2 100644 --- a/R/contract.R +++ b/R/contract.R @@ -151,17 +151,30 @@ chat_disconnect.default <- function(client, ...) { #' adapter-specific and may already be normalized by the transport #' package (Matrix hands over an extracted record, not the timeline #' event), so it is an escape hatch, not a guarantee of completeness. +#' @param encrypted Logical: did this message arrive end-to-end +#' encrypted? FALSE on transports without E2EE and on cleartext +#' messages in rooms that have it. +#' @param sender_verified Logical: does the sender identifier bind to a +#' device whose keys this client verified? NULL on cleartext messages, +#' where the transport asserts the sender and there is nothing to +#' verify. On an encrypted message FALSE is a real answer, not a +#' missing one: the payload decrypted, but its claimed sender could not +#' be tied to a verified device, so the identifier is the homeserver's +#' word rather than cryptographic fact. #' @return A list with class \code{chat_message}. #' @export chat_message <- function(id, channel, sender, body, ts, thread = NULL, markup = "plain", kind = "message", self = NULL, - mentions = NULL, raw = NULL) { + mentions = NULL, raw = NULL, encrypted = FALSE, + sender_verified = NULL) { stopifnot(is.character(id), is.character(channel), is.character(sender), is.character(body)) structure(list(id = id, channel = channel, sender = sender, body = body, ts = ts, thread = thread, markup = markup, kind = kind, self = self, - mentions = mentions, raw = raw), + mentions = mentions, raw = raw, + encrypted = isTRUE(encrypted), + sender_verified = sender_verified), class = "chat_message") } diff --git a/R/matrix-crypto.R b/R/matrix-crypto.R new file mode 100644 index 0000000..eb05b9e --- /dev/null +++ b/R/matrix-crypto.R @@ -0,0 +1,256 @@ +# Matrix end-to-end encryption for the chat.api adapter. +# +# Olm/Megolm state owned by the adapter, so a consumer never handles +# crypto itself. All cryptography is delegated to mx.client and +# mx.crypto, both Suggests, both behind requireNamespace. +# +# Ported from corteza, which carried this in parallel to the contract and +# reached around chat_poll()$raw to do it. None of it is exported: the +# whole point is that E2EE is a property of the client a consumer already +# holds, not a second API it has to learn. + +matrix_crypto_available <- function() { + requireNamespace("mx.client", quietly = TRUE) && + requireNamespace("mx.crypto", quietly = TRUE) +} + +# Where this identity's Olm account and sessions live. +# +# mx.client's own convention, under R_user_dir(app, "data"), deliberately +# NOT derived from the config file's location. corteza computed +# dirname(config)/crypto, which ties the crypto identity to wherever the +# config happens to sit: point a bot at a different config path and it +# silently mints a new device identity and loses every Megolm session it +# had. Keying on the app namespace instead keeps one store per identity +# however the config is addressed. +matrix_crypto_store <- function(app = NULL) { + mx.client::mx_crypto_store_dir(app = app %||% "chat.api") +} + +# The crypto operations the adapter calls, as a replaceable set. Same +# reason chat_matrix() takes .sync and .extract: the Matrix methods have +# to be verifiable on a runner with neither mx.client nor a Rust +# toolchain, and E2EE is the one part of the adapter that cannot be +# exercised against a real homeserver in a test. +matrix_crypto_ops <- function(override = NULL) { + ops <- list(init = matrix_crypto_init, + encrypted = matrix_room_is_encrypted, + send = matrix_crypto_send, + decrypt = matrix_crypto_decrypt) + for (nm in names(override)) { + ops[[nm]] <- override[[nm]] + } + ops +} + +# Load or create the account, publish keys, restore sessions and the +# known-encrypted-room set. Returns an environment: the poll loop mutates +# sessions in place, and callers hold the same object across polls. +# +# The store directory is derived here rather than by the caller so that +# mx.client is only needed on the path that actually does crypto: a +# .crypto seam replaces this whole function, and a test using one must +# not pay for mx_crypto_store_dir(). +matrix_crypto_init <- function(mx, store = NULL, app = NULL) { + if (!matrix_crypto_available()) { + stop("Matrix end-to-end encryption requires the 'mx.client' and ", + "'mx.crypto' packages.", call. = FALSE) + } + store <- store %||% matrix_crypto_store(app = app) + acct <- mx.client::mx_crypto_account(store) + mx.client::mx_crypto_publish_keys(mx, acct, store, n_otks = 50L) + crypto <- new.env(parent = emptyenv()) + crypto$account <- acct + crypto$store <- store + # No cached client. The access token rotates -- chat_poll() relogins -- + # and a copy taken here goes stale the moment it does. Every send and + # decrypt takes the adapter's live env$mx. + crypto$sessions <- mx.client::mx_crypto_sessions_load(store) + crypto$encrypted <- matrix_crypto_load_encrypted(store) + crypto$self_curve <- + mx.crypto::mxc_account_identity_keys(acct)$curve25519 + # Ask each joined room for its encryption state up front rather than + # waiting for a sync to mention it. Best-effort per room; a transient + # failure defers that room to sync-time detection. + found <- matrix_crypto_scan_rooms(mx) + fresh <- setdiff(found, crypto$encrypted) + if (length(fresh)) { + crypto$encrypted <- c(crypto$encrypted, fresh) + matrix_crypto_save_encrypted(crypto) + } + crypto +} + +# Joined rooms advertising m.room.encryption, by direct state query. +matrix_crypto_scan_rooms <- function(mx) { + s <- tryCatch(mx.client::mx_client_session(mx), error = function(e) NULL) + if (is.null(s)) { + return(character()) + } + rooms <- tryCatch(mx.api::mx_rooms(s), error = function(e) character()) + out <- character() + for (rid in rooms) { + if (isTRUE(tryCatch(mx.client::mx_room_encrypted(mx, rid), + error = function(e) FALSE))) { + out <- c(out, rid) + } + } + out +} + +# One room id per line, not JSON: chat.api has no dependencies and a +# character vector does not need a parser. corteza wrote +# encrypted_rooms.json here; the name differs so a leftover file is +# ignored rather than read as room ids. Nothing is lost by ignoring it -- +# the room set is a cache, and matrix_crypto_scan_rooms() rebuilds it at +# init from the homeserver's own state. +matrix_crypto_room_file <- function(store) { + file.path(store, "encrypted_rooms.txt") +} + +matrix_crypto_load_encrypted <- function(store) { + f <- matrix_crypto_room_file(store) + if (!file.exists(f)) { + return(character()) + } + rooms <- readLines(f, warn = FALSE) + unique(rooms[nzchar(rooms)]) +} + +matrix_crypto_save_encrypted <- function(crypto) { + dir.create(crypto$store, showWarnings = FALSE, recursive = TRUE) + writeLines(crypto$encrypted, matrix_crypto_room_file(crypto$store)) +} + +# Rooms advertising m.room.encryption in this sync (state or timeline). +matrix_detect_encrypted_rooms <- function(sync) { + out <- character() + joined <- sync$rooms$join %||% list() + for (rid in names(joined)) { + evs <- c(joined[[rid]]$state$events %||% list(), + joined[[rid]]$timeline$events %||% list()) + for (ev in evs) { + if (isTRUE(ev$type == "m.room.encryption")) { + out <- c(out, rid) + break + } + } + } + out +} + +# Decrypt a sync: recover room keys from to-device, decrypt encrypted +# timeline events, refresh the encrypted-room set. Mutates `crypto`. +# +# `devices` is the verified device list for the senders in this sync. +# mx.client only reports sender_verified = TRUE when a payload's claimed +# (sender, ed25519, curve25519) binds to a device whose device_keys +# verified, so without this every decrypted message would come back +# unattributed. Fetching it is a /keys/query round trip, so it is skipped +# when the sync carries no encrypted traffic. +matrix_crypto_decrypt <- function(crypto, sync, mx) { + fresh <- setdiff(matrix_detect_encrypted_rooms(sync), crypto$encrypted) + if (length(fresh)) { + crypto$encrypted <- c(crypto$encrypted, fresh) + matrix_crypto_save_encrypted(crypto) + } + senders <- matrix_encrypted_senders(sync) + devices <- if (length(senders)) { + tryCatch(mx.client::mx_crypto_known_devices(mx, senders), + error = function(e) NULL) + } else { + NULL + } + res <- mx.client::mx_crypto_process_sync( + crypto$account, crypto$sessions, sync, crypto$self_curve, + self_id = mx$user_id, devices = devices) + crypto$sessions <- res$sessions + mx.client::mx_crypto_sessions_save(crypto$sessions, crypto$store) + res$events +} + +# Distinct senders of m.room.encrypted timeline events in this sync, so +# the device query asks about the people who actually spoke rather than +# every room member. +matrix_encrypted_senders <- function(sync) { + out <- character() + for (room in sync$rooms$join %||% list()) { + for (ev in room$timeline$events %||% list()) { + if (isTRUE(ev$type == "m.room.encrypted") && + !is.null(ev$sender)) { + out <- c(out, ev$sender) + } + } + } + unique(out) +} + +# Is this room one the adapter knows to be encrypted? +matrix_room_is_encrypted <- function(crypto, mx, room_id) { + if (is.null(crypto)) { + return(FALSE) + } + if (room_id %in% crypto$encrypted) { + return(TRUE) + } + # Not in the cached set: ask, so a room that turned on encryption + # between polls does not get a cleartext send. A failed lookup answers + # FALSE, which is the pre-encryption behaviour rather than a hard error. + enc <- isTRUE(tryCatch(mx.client::mx_room_encrypted(mx, room_id), + error = function(e) FALSE)) + if (enc) { + crypto$encrypted <- c(crypto$encrypted, room_id) + matrix_crypto_save_encrypted(crypto) + } + enc +} + +# The m.room.message content an encrypted send wraps. Deliberately the +# same shape mx_send_text() PUTs in the clear -- markdown becomes +# formatted_body, mentions become pills plus m.mentions -- so a room's +# encryption state changes the envelope and nothing a reader sees. +matrix_crypto_content <- function(text, msgtype = "m.text", markdown = FALSE, + mentions = NULL) { + content <- list(msgtype = msgtype, body = text) + if (isTRUE(markdown) || length(mentions)) { + html <- mx.client::mx_markdown_to_html(text) + if (length(mentions)) { + html <- mx.client::mx_pill_mentions(html, mentions) + } + content$format <- "org.matrix.custom.html" + content$formatted_body <- html + } + if (length(mentions)) { + content[["m.mentions"]] <- list(user_ids = as.list(mentions)) + } + content +} + +# Encrypt and send. Returns the event id, or NULL on failure so the +# caller sees the same "no event id" signal the cleartext path gives. +matrix_crypto_send <- function(crypto, mx, room_id, text, msgtype = "m.text", + markdown = FALSE, mentions = NULL) { + s <- tryCatch(mx.client::mx_client_session(mx), error = function(e) NULL) + members <- if (is.null(s)) { + character() + } else { + tryCatch(mx.api::mx_room_members(s, room_id), + error = function(e) character()) + } + content <- matrix_crypto_content(text, msgtype = msgtype, + markdown = markdown, mentions = mentions) + res <- tryCatch( + mx.client::mx_send_encrypted(mx, crypto$account, crypto$sessions, + room_id, content, crypto$store, + member_ids = members), + error = function(e) { + warning("chat.api: encrypted send failed: ", conditionMessage(e), + call. = FALSE) + NULL + }) + if (is.null(res)) { + return(NULL) + } + crypto$sessions <- res$sessions + res$event_id +} diff --git a/R/matrix.R b/R/matrix.R index 9954395..05d90d0 100644 --- a/R/matrix.R +++ b/R/matrix.R @@ -33,6 +33,21 @@ #' refreshed config lands back in the client, so the next poll and any #' send use the new token. FALSE lets \code{mx_error_M_UNKNOWN_TOKEN} #' propagate. +#' @param e2ee Logical. Own Olm/Megolm state in the adapter, so +#' \code{\link{chat_send}} encrypts for rooms that advertise +#' \code{m.room.encryption} and \code{\link{chat_poll}} decrypts +#' inbound. FALSE (the default) is the previous behaviour exactly: +#' cleartext only, no crypto state, and \code{e2ee} stays FALSE in +#' \code{\link{chat_capabilities}}. Requires the 'mx.crypto' package, +#' which needs a Rust toolchain. +#' @param crypto_store Character or NULL. Directory holding the pickled +#' Olm account and sessions. NULL derives it from \code{app} via +#' \code{mx.client::mx_crypto_store_dir()}, which keys one store per +#' app namespace regardless of where the config file lives, so moving a +#' config does not cost the device identity. Because that key is the app +#' name alone, an \code{e2ee = TRUE} client built from an explicit +#' \code{path} with no \code{app} is an error: name one or the other, or +#' two such clients would quietly share a device identity. #' @param .sync Testing seam: replacement for #' \code{mx.client::mx_sync_update}. Leave NULL in production. #' @param .extract Testing seam: replacement for @@ -48,6 +63,13 @@ #' \code{mx.api::mx_typing}. Leave NULL in production. It is resolved #' when \code{chat_typing()} is called, not here, so a NULL seam costs #' nothing on installs without mx.api. +#' @param .crypto Testing seam: a named list overriding any of the +#' adapter's four crypto operations -- \code{init}, \code{encrypted}, +#' \code{send}, \code{decrypt}. Leave NULL in production. Supplying +#' \code{init} is what lets \code{e2ee = TRUE} be tested on a runner +#' with neither mx.crypto nor a Rust toolchain; E2EE is the one part of +#' this adapter with no honest way to reach a real homeserver from a +#' test. #' @return A \code{chat_client} of class \code{chat_matrix}. #' \code{\link{chat_poll}} on this class returns \code{first_run} and #' \code{client} alongside \code{messages}, \code{cursor}, and @@ -59,9 +81,10 @@ #' poll cycle. #' @export chat_matrix <- function(app = NULL, path = NULL, save_cursor = TRUE, - mx = NULL, relogin = TRUE, .sync = NULL, + mx = NULL, relogin = TRUE, e2ee = FALSE, + crypto_store = NULL, .sync = NULL, .extract = NULL, .send = NULL, .media = NULL, - .typing = NULL) { + .typing = NULL, .crypto = NULL) { seams <- list(.sync, .extract, .send, .media) if ((is.null(mx) || any(vapply(seams, is.null, logical(1)))) && !requireNamespace("mx.client", quietly = TRUE)) { @@ -78,13 +101,34 @@ chat_matrix <- function(app = NULL, path = NULL, save_cursor = TRUE, } env <- new.env(parent = emptyenv()) env$mx <- mx + # Crypto state lives on the same env as the client, so a consumer + # holds one object and never touches Olm or Megolm itself. NULL when + # e2ee is off, which is the default and is exactly the pre-existing + # behaviour. + env$crypto <- NULL + ops <- matrix_crypto_ops(.crypto) + if (isTRUE(e2ee)) { + # An explicit config path with no app names one specific identity, + # but the default store is keyed on the app namespace alone, so + # two bots loaded that way would share one account.pickle and the + # second would come up wearing the first's device keys. Deriving + # the store from the config's own directory instead only trades + # that for a quieter failure: move the config and the bot mints a + # new identity and loses every Megolm session it had. So ask. + if (is.null(crypto_store) && !is.null(path) && is.null(app)) { + stop("chat_matrix(e2ee = TRUE) with an explicit `path` needs ", + "`app` or `crypto_store` too, so the crypto store is ", + "unique to this identity.", call. = FALSE) + } + env$crypto <- ops$init(mx, store = crypto_store, app = app) + } structure(list(env = env, app = app, save_cursor = isTRUE(save_cursor), - relogin = isTRUE(relogin), + relogin = isTRUE(relogin), e2ee = isTRUE(e2ee), sync_fn = .sync %||% mx.client::mx_sync_update, extract_fn = .extract %||% mx.client::mx_extract_text_events, send_fn = .send %||% mx.client::mx_send_text, media_fn = .media %||% mx.client::mx_send_media, - typing_fn = .typing), + typing_fn = .typing, crypto_ops = ops), class = c("chat_matrix", "chat_client")) } @@ -169,6 +213,37 @@ chat_poll.chat_matrix <- function(client, since = NULL, timeout = NULL, ...) { mentions = unlist(r$mentions, use.names = FALSE), raw = r) }) + # Decrypted traffic folds in alongside the cleartext, in the same + # chat_message shape, so a consumer cannot tell which arrived + # encrypted except by reading $encrypted. That is the whole point of + # owning crypto here: corteza used to run its own decrypt off $raw and + # concatenate the results itself. + if (!is.null(client$env$crypto)) { + dec <- tryCatch( + client$crypto_ops$decrypt(client$env$crypto, res$sync, + client$env$mx), + error = function(e) { + warning("chat.api: decrypt failed: ", conditionMessage(e), + call. = FALSE) + list() + }) + for (d in dec) { + ms <- d$ts %||% event_ts[[as.character(d$event_id)]] + messages[[length(messages) + 1L]] <- chat_message( + id = as.character(d$event_id), + channel = as.character(d$room_id), + sender = as.character(d$sender), + body = as.character(d$body), + ts = if (is.null(ms)) as.POSIXct(NA) else + as.POSIXct(ms / 1000, origin = "1970-01-01"), + markup = "plain", kind = matrix_kind(d$msgtype), + self = isTRUE(d$is_self), + mentions = unlist(d$mentions, use.names = FALSE), + encrypted = TRUE, + sender_verified = isTRUE(d$sender_verified), + raw = d) + } + } # first_run says the sync ran without a stored cursor, so these # messages are the homeserver's backfill baseline, not new traffic. # A consumer that drops it replays its whole history as fresh mail @@ -180,8 +255,8 @@ chat_poll.chat_matrix <- function(client, since = NULL, timeout = NULL, ...) { # with the token the homeserver just rejected. # # raw carries the full sync response for Matrix-specific consumers - # (invites, reactions, E2EE decryption) that the generic contract - # does not model yet + # (invites, reactions) that the generic contract does not model yet. + # E2EE no longer needs it. list(messages = messages, cursor = res$client$sync_token, first_run = isTRUE(res$first_run), client = res$client, raw = res$sync) @@ -217,6 +292,24 @@ chat_send.chat_matrix <- function(client, channel, text, # attachment's echo as somebody else's message: corteza recognizes # its own traffic by event id. if (!length(media_ids) || any(nzchar(text))) { + crypto <- client$env$crypto + # Encrypted rooms take the Megolm path. Asked per send rather than + # trusting the cached room set, so a room that turned on + # encryption between polls does not get one cleartext message + # first. + # + # markup and mentions carry through: the encrypted branch builds + # the same m.room.message content the cleartext one does, so a + # markdown send renders the same either way. Only the envelope + # differs. + if (!is.null(crypto) && + client$crypto_ops$encrypted(crypto, client$env$mx, channel)) { + event <- client$crypto_ops$send( + crypto, client$env$mx, channel, text, msgtype = msgtype, + markdown = identical(markup, "markdown"), + mentions = list(...)$mentions) + return(invisible(c(media_ids, as.character(event)))) + } event <- client$send_fn(client$env$mx, text, room = channel, msgtype = msgtype, markdown = identical(markup, "markdown"), ...) @@ -275,15 +368,15 @@ chat_capabilities.chat_matrix <- function(client, ...) { # and mx_extract_text_events filters to m.text, so an m.reaction can # be neither sent nor received here. # - # e2ee is FALSE because chat_send goes through mx_send_text, which - # PUTs a cleartext m.room.message no matter what the room's - # encryption state says, and chat_poll never sees an - # m.room.encrypted event. Reporting TRUE off an installed mx.crypto - # invites a consumer to hand this adapter a secret for an encrypted - # room and have it land on the homeserver in the clear. Flip it when - # the adapter routes through mx_send_encrypted and decrypts inbound. + # e2ee answers for this client, not for Matrix and not for the + # installed packages: it is TRUE only when chat_matrix(e2ee = TRUE) + # built a crypto context, which is what actually routes sends through + # mx_send_encrypted and decrypts inbound. Reporting TRUE off an + # installed mx.crypto would invite a consumer to hand a secret to a + # client that PUTs it on the homeserver in the clear. list(threads = FALSE, thread_replies = FALSE, edits = FALSE, - reactions = FALSE, files = TRUE, typing = TRUE, e2ee = FALSE, + reactions = FALSE, files = TRUE, typing = TRUE, + e2ee = !is.null(client$env$crypto), identity_override = FALSE, markup_dialects = c("plain", "markdown"), max_message_bytes = NA_integer_) } diff --git a/inst/tinytest/test_matrix.R b/inst/tinytest/test_matrix.R index dc7bd9a..0cc8940 100644 --- a/inst/tinytest/test_matrix.R +++ b/inst/tinytest/test_matrix.R @@ -37,9 +37,13 @@ wrap_sync <- function(events, room = "!room:ex") { # A fully seamed client. recs is what the .extract seam hands back, so # the mapping under test never depends on which mx.client is installed. +# send and media are named parameters rather than `...` pass-throughs so +# a test can watch the cleartext path without colliding with the defaults +# this helper already supplies. seam_client <- function(recs = list(), sync = wrap_sync(list()), first_run = FALSE, token = "s1", mx = fake_mx(), - record = NULL, ...) { + record = NULL, send = function(...) "$id", + media = function(...) NULL, ...) { chat_matrix(mx = mx, .sync = function(client, ...) { if (!is.null(record)) { @@ -55,7 +59,7 @@ seam_client <- function(recs = list(), sync = wrap_sync(list()), } recs }, - .send = function(...) "$id", .media = function(...) NULL, ...) + .send = send, .media = media, ...) } # ---- Constructor ---- @@ -422,11 +426,12 @@ expect_false(caps$thread_replies) # filter means an m.reaction can be neither sent nor received here expect_false(caps$reactions) expect_false("chat_react" %in% getNamespaceExports("chat.api")) -# e2ee: chat_send goes through mx_send_text, which PUTs a cleartext -# m.room.message whatever the room's encryption state says, and -# chat_poll never sees an m.room.encrypted event. Reporting TRUE off an -# installed mx.crypto invites a consumer to hand this adapter a secret -# for an encrypted room and have it land on the homeserver in the clear. +# e2ee answers for this client. `s` was built without e2ee = TRUE, so it +# holds no crypto context and chat_send goes through mx_send_text, which +# PUTs a cleartext m.room.message whatever the room's encryption state +# says. Reporting TRUE off an installed mx.crypto would invite a consumer +# to hand this client a secret and have it land on the homeserver in the +# clear. The e2ee = TRUE half is asserted in the E2EE block below. expect_false(caps$e2ee) expect_true(is.logical(caps$e2ee) && length(caps$e2ee) == 1L) expect_true(caps$files) @@ -464,3 +469,300 @@ local({ expect_identical(only, "$media3") expect_identical(sent$text, before) }) + +# ---- E2EE ---- +# The .crypto seam stands in for the whole crypto module, so these run +# with neither mx.crypto nor a Rust toolchain. What is under test is the +# adapter's routing: which path a send takes, and whether decrypted +# events reach the caller as ordinary chat_message records. + +# A fake crypto context plus ops that record what the adapter asked for. +fake_crypto <- function(encrypted_rooms = "!enc:ex", decrypted = list(), + event_id = "$enc1", decrypt_error = FALSE) { + log <- new.env(parent = emptyenv()) + log$init <- list() + log$asked <- character() + log$sent <- list() + log$decrypted <- 0L + ctx <- new.env(parent = emptyenv()) + ctx$encrypted <- encrypted_rooms + ops <- list( + init = function(mx, store = NULL, app = NULL) { + log$init[[length(log$init) + 1L]] <- list(mx = mx, store = store, + app = app) + ctx + }, + encrypted = function(crypto, mx, room_id) { + log$asked <- c(log$asked, room_id) + room_id %in% crypto$encrypted + }, + send = function(crypto, mx, room_id, text, msgtype = "m.text", + markdown = FALSE, mentions = NULL) { + log$sent[[length(log$sent) + 1L]] <- list( + room_id = room_id, text = text, msgtype = msgtype, + markdown = markdown, mentions = mentions, mx = mx) + event_id + }, + decrypt = function(crypto, sync, mx) { + log$decrypted <- log$decrypted + 1L + if (isTRUE(decrypt_error)) { + stop("no session for that megolm stream") + } + decrypted + }) + list(ops = ops, log = log, ctx = ctx) +} + +# e2ee is opt-in. Without it there is no crypto context, so nothing the +# adapter does can consult one -- this is the pre-E2EE behaviour exactly. +plain <- seam_client() +expect_null(plain$env$crypto) +expect_false(plain$e2ee) +expect_false(chat_capabilities(plain)$e2ee) + +# init is not called either: an unused crypto module must not touch the +# store or publish keys. +f <- fake_crypto() +off <- seam_client(e2ee = FALSE, .crypto = f$ops) +expect_identical(length(f$log$init), 0L) +expect_null(off$env$crypto) + +# e2ee = TRUE builds the context, and the capability follows the context +# rather than the installed packages. +f <- fake_crypto() +enc <- seam_client(e2ee = TRUE, .crypto = f$ops) +expect_identical(length(f$log$init), 1L) +expect_true(is.environment(enc$env$crypto)) +expect_true(chat_capabilities(enc)$e2ee) + +# The store is derived inside init, not by the constructor, so mx.client +# is only needed on the path that does crypto. app and an explicit +# crypto_store both reach it. +f <- fake_crypto() +seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) +expect_identical(f$log$init[[1L]]$app, "cornelius") +expect_null(f$log$init[[1L]]$store) +f <- fake_crypto() +seam_client(e2ee = TRUE, crypto_store = "/tmp/store", .crypto = f$ops) +expect_identical(f$log$init[[1L]]$store, "/tmp/store") + +# An explicit config path with no app and no crypto_store cannot name a +# unique store: the default is keyed on the app namespace alone, so two +# bots built that way would share one Olm account and the second would +# come up wearing the first's device keys. That is an error, not a guess. +expect_error(chat_matrix(mx = fake_mx(), path = "/tmp/bot.json", e2ee = TRUE, + .sync = function(...) NULL, + .extract = function(...) list(), + .send = function(...) "$id", + .media = function(...) NULL, + .crypto = fake_crypto()$ops), + "unique to this identity") +# The same path with an app names one, so it is allowed. +f <- fake_crypto() +ok <- chat_matrix(mx = fake_mx(), path = "/tmp/bot.json", app = "tiny", + e2ee = TRUE, .sync = function(...) NULL, + .extract = function(...) list(), + .send = function(...) "$id", .media = function(...) NULL, + .crypto = f$ops) +expect_true(is.environment(ok$env$crypto)) +# ... and so does crypto_store on its own. +expect_true(is.environment( + chat_matrix(mx = fake_mx(), path = "/tmp/bot.json", e2ee = TRUE, + crypto_store = "/tmp/store", .sync = function(...) NULL, + .extract = function(...) list(), + .send = function(...) "$id", .media = function(...) NULL, + .crypto = fake_crypto()$ops)$env$crypto)) + +# ---- Encrypted sends ---- + +# An encrypted room takes the Megolm path and the cleartext send never +# runs. This is the assertion that matters most: a regression here PUTs +# plaintext into a room the user believes is encrypted. +f <- fake_crypto() +cleartext <- new.env(parent = emptyenv()); cleartext$n <- 0L +enc <- seam_client(e2ee = TRUE, .crypto = f$ops, + send = function(...) { cleartext$n <- cleartext$n + 1L + "$clear1" }) +id <- chat_send(enc, "!enc:ex", "secret") +expect_identical(id, "$enc1") +expect_identical(cleartext$n, 0L) +expect_identical(length(f$log$sent), 1L) +expect_identical(f$log$sent[[1L]]$text, "secret") +expect_identical(f$log$sent[[1L]]$room_id, "!enc:ex") + +# A cleartext room on the same client still goes out in the clear: e2ee +# is a per-room property, not a per-client one. +id <- chat_send(enc, "!plain:ex", "public") +expect_identical(id, "$clear1") +expect_identical(cleartext$n, 1L) +expect_identical(length(f$log$sent), 1L) + +# The room is asked about on every send rather than once at init, so a +# room that turns on encryption between polls does not get one cleartext +# message first. +expect_identical(f$log$asked, c("!enc:ex", "!plain:ex")) +f$ctx$encrypted <- c(f$ctx$encrypted, "!plain:ex") +expect_identical(chat_send(enc, "!plain:ex", "now secret"), "$enc1") +expect_identical(cleartext$n, 1L) + +# markup, kind, and mentions carry into the encrypted branch. Dropping +# them would make an encrypted room silently lose formatting and pings +# that the same call gets in the clear. +f <- fake_crypto() +enc <- seam_client(e2ee = TRUE, .crypto = f$ops) +chat_send(enc, "!enc:ex", "**bold**", markup = "markdown", + mentions = "@carol:ex", kind = "notice") +expect_true(f$log$sent[[1L]]$markdown) +expect_identical(f$log$sent[[1L]]$mentions, "@carol:ex") +expect_identical(f$log$sent[[1L]]$msgtype, "m.notice") +chat_send(enc, "!enc:ex", "plain words") +expect_false(f$log$sent[[2L]]$markdown) +expect_null(f$log$sent[[2L]]$mentions) +expect_identical(f$log$sent[[2L]]$msgtype, "m.text") + +# The encrypted send takes the client's live mx, not a copy captured at +# init. A relogin replaces the token mid-poll, and a cached config would +# spend the rest of the run authenticating with the rejected one. +enc$env$mx <- fake_mx(sync_token = "rotated") +chat_send(enc, "!enc:ex", "after relogin") +expect_identical(f$log$sent[[3L]]$mx$sync_token, "rotated") + +# Attachments plus encrypted text: media ids first, encrypted event last, +# same contract the cleartext path reports. +f <- fake_crypto() +enc <- seam_client(e2ee = TRUE, .crypto = f$ops, + media = function(mx, file, ...) paste0("$m", basename(file))) +ids <- chat_send(enc, "!enc:ex", "see these", files = c("/a.png", "/b.png")) +expect_identical(ids, c("$ma.png", "$mb.png", "$enc1")) + +# A failed encrypted send reports no event id, the same way the cleartext +# path does, rather than inventing one or falling back to plaintext. +f <- fake_crypto(event_id = NULL) +enc <- seam_client(e2ee = TRUE, .crypto = f$ops, + send = function(...) stop("must not be reached")) +expect_identical(chat_send(enc, "!enc:ex", "secret"), character(0)) + +# ---- Decryption on poll ---- + +dec_rec <- function(event_id, body = "psst", sender = "@alice:ex", + room_id = "!enc:ex", msgtype = "m.text", + sender_verified = TRUE, is_self = FALSE, ts = NULL, + mentions = NULL) { + list(room_id = room_id, event_id = event_id, sender = sender, + body = body, msgtype = msgtype, sender_verified = sender_verified, + is_self = is_self, ts = ts, mentions = mentions) +} + +# Decrypted events come out as ordinary chat_message records, folded in +# beside the cleartext ones. corteza used to run its own decrypt off +# $raw and concatenate the results itself; that is what this replaces. +f <- fake_crypto(decrypted = list(dec_rec("$e1", ts = 1700000000000))) +enc <- seam_client(recs = list(rec("$c1", body = "in the clear")), + e2ee = TRUE, .crypto = f$ops) +res <- chat_poll(enc) +expect_identical(length(res$messages), 2L) +expect_identical(f$log$decrypted, 1L) +expect_identical(res$messages[[1L]]$body, "in the clear") +expect_false(res$messages[[1L]]$encrypted) +# sender_verified is NULL on cleartext: the transport asserts the sender +# and there is nothing to verify, which is not the same answer as FALSE. +expect_null(res$messages[[1L]]$sender_verified) +m <- res$messages[[2L]] +expect_inherits(m, "chat_message") +expect_identical(m$id, "$e1") +expect_identical(m$channel, "!enc:ex") +expect_identical(m$sender, "@alice:ex") +expect_identical(m$body, "psst") +expect_true(m$encrypted) +expect_true(m$sender_verified) +expect_identical(m$kind, "message") + +# An unverified sender is FALSE, not NULL: the payload decrypted, but the +# claimed sender is the homeserver's word rather than a verified device. +f <- fake_crypto(decrypted = list(dec_rec("$e1", sender_verified = FALSE))) +res <- chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops)) +expect_false(res$messages[[1L]]$sender_verified) +# A missing flag is also FALSE. Absence of proof is not proof. +f <- fake_crypto(decrypted = list(dec_rec("$e1", sender_verified = NULL))) +res <- chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops)) +expect_false(res$messages[[1L]]$sender_verified) + +# Decrypted msgtypes map into the contract's kind vocabulary, same as +# cleartext, so a cross-adapter kind filter treats both alike. +f <- fake_crypto(decrypted = list(dec_rec("$e1", msgtype = "m.notice"), + dec_rec("$e2", msgtype = "m.emote"))) +res <- chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops)) +expect_identical(res$messages[[1L]]$kind, "notice") +expect_identical(res$messages[[2L]]$kind, "emote") + +# A decrypted record without its own ts takes the timestamp off the sync, +# and an unknown time stays NA rather than becoming the poll's clock. +f <- fake_crypto(decrypted = list(dec_rec("$e1"))) +res <- chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops, + sync = wrap_sync(list(ev("$e1", ts = 1700000000000))))) +expect_false(is.na(res$messages[[1L]]$ts)) +f <- fake_crypto(decrypted = list(dec_rec("$nowhere"))) +res <- chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops)) +expect_true(is.na(res$messages[[1L]]$ts)) + +# The bot's own encrypted echo is marked self, so a consumer replying to +# inbound mail does not answer itself. +f <- fake_crypto(decrypted = list(dec_rec("$e1", is_self = TRUE))) +res <- chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops)) +expect_true(res$messages[[1L]]$self) + +# A decrypt that throws warns and loses the encrypted traffic, but the +# poll still returns the cleartext messages and advances the cursor. One +# room with a missing Megolm session must not stall the whole loop. +f <- fake_crypto(decrypt_error = TRUE) +enc <- seam_client(recs = list(rec("$c1")), e2ee = TRUE, .crypto = f$ops, + token = "s2") +expect_warning(res <- chat_poll(enc), "decrypt failed") +expect_identical(length(res$messages), 1L) +expect_identical(res$cursor, "s2") + +# Nothing consults crypto when e2ee is off, so an installed mx.crypto +# cannot change what a cleartext client does. +f <- fake_crypto(decrypted = list(dec_rec("$e1"))) +res <- chat_poll(seam_client(recs = list(rec("$c1")), .crypto = f$ops)) +expect_identical(f$log$decrypted, 0L) +expect_identical(length(res$messages), 1L) + +# ---- Encrypted-room cache file ---- +# One room id per line, not JSON: chat.api has no dependencies. A stale +# encrypted_rooms.json from corteza is ignored, and losing the cache +# costs nothing because init rescans the homeserver's own room state. +store <- file.path(tempfile("chatapi-store")) +dir.create(store) +expect_identical(chat.api:::matrix_crypto_load_encrypted(store), character()) +ctx <- new.env(parent = emptyenv()) +ctx$store <- store +ctx$encrypted <- c("!a:ex", "!b:ex") +chat.api:::matrix_crypto_save_encrypted(ctx) +expect_identical(chat.api:::matrix_crypto_load_encrypted(store), + c("!a:ex", "!b:ex")) +# An empty set round trips as an empty set, not as one blank room id. +ctx$encrypted <- character() +chat.api:::matrix_crypto_save_encrypted(ctx) +expect_identical(chat.api:::matrix_crypto_load_encrypted(store), character()) +# A leftover JSON file is not read as room ids. +writeLines('["!json:ex"]', file.path(store, "encrypted_rooms.json")) +expect_identical(chat.api:::matrix_crypto_load_encrypted(store), character()) +unlink(store, recursive = TRUE) +# Saving into a store that does not exist yet creates it. +store2 <- tempfile("chatapi-store2") +ctx$store <- store2 +ctx$encrypted <- "!c:ex" +chat.api:::matrix_crypto_save_encrypted(ctx) +expect_true(dir.exists(store2)) +expect_identical(chat.api:::matrix_crypto_load_encrypted(store2), "!c:ex") +unlink(store2, recursive = TRUE) + +# The ops table defaults to the real implementations and overrides +# selectively, so a seam that names one operation does not blank the rest. +ops <- chat.api:::matrix_crypto_ops() +expect_identical(sort(names(ops)), c("decrypt", "encrypted", "init", "send")) +expect_identical(ops$send, chat.api:::matrix_crypto_send) +partial <- chat.api:::matrix_crypto_ops(list(send = function(...) "$x")) +expect_identical(partial$decrypt, chat.api:::matrix_crypto_decrypt) +expect_false(identical(partial$send, chat.api:::matrix_crypto_send)) diff --git a/man/chat_matrix.Rd b/man/chat_matrix.Rd index 08241d1..2eb501d 100644 --- a/man/chat_matrix.Rd +++ b/man/chat_matrix.Rd @@ -3,9 +3,21 @@ \alias{chat_matrix} \title{Create a Matrix chat client} \usage{ -chat_matrix(app = NULL, path = NULL, save_cursor = TRUE, mx = NULL, - relogin = TRUE, .sync = NULL, .extract = NULL, .send = NULL, - .media = NULL, .typing = NULL) +chat_matrix( + app = NULL, + path = NULL, + save_cursor = TRUE, + mx = NULL, + relogin = TRUE, + e2ee = FALSE, + crypto_store = NULL, + .sync = NULL, + .extract = NULL, + .send = NULL, + .media = NULL, + .typing = NULL, + .crypto = NULL +) } \arguments{ \item{app}{Application namespace passed to @@ -37,6 +49,23 @@ refreshed config lands back in the client, so the next poll and any send use the new token. FALSE lets \code{mx_error_M_UNKNOWN_TOKEN} propagate.} +\item{e2ee}{Logical. Own Olm/Megolm state in the adapter, so +\code{\link{chat_send}} encrypts for rooms that advertise +\code{m.room.encryption} and \code{\link{chat_poll}} decrypts +inbound. FALSE (the default) is the previous behaviour exactly: +cleartext only, no crypto state, and \code{e2ee} stays FALSE in +\code{\link{chat_capabilities}}. Requires the 'mx.crypto' package, +which needs a Rust toolchain.} + +\item{crypto_store}{Character or NULL. Directory holding the pickled +Olm account and sessions. NULL derives it from \code{app} via +\code{mx.client::mx_crypto_store_dir()}, which keys one store per +app namespace regardless of where the config file lives, so moving a +config does not cost the device identity. Because that key is the app +name alone, an \code{e2ee = TRUE} client built from an explicit +\code{path} with no \code{app} is an error: name one or the other, or +two such clients would quietly share a device identity.} + \item{.sync}{Testing seam: replacement for \code{mx.client::mx_sync_update}. Leave NULL in production.} @@ -56,6 +85,14 @@ without mx.client installed; \code{chat_typing()} and \code{mx.api::mx_typing}. Leave NULL in production. It is resolved when \code{chat_typing()} is called, not here, so a NULL seam costs nothing on installs without mx.api.} + +\item{.crypto}{Testing seam: a named list overriding any of the +adapter's four crypto operations -- \code{init}, \code{encrypted}, +\code{send}, \code{decrypt}. Leave NULL in production. Supplying +\code{init} is what lets \code{e2ee = TRUE} be tested on a runner +with neither mx.crypto nor a Rust toolchain; E2EE is the one part of +this adapter with no honest way to reach a real homeserver from a +test.} } \value{ A \code{chat_client} of class \code{chat_matrix}. diff --git a/man/chat_message.Rd b/man/chat_message.Rd index 68fbbe0..c810a47 100644 --- a/man/chat_message.Rd +++ b/man/chat_message.Rd @@ -3,8 +3,21 @@ \alias{chat_message} \title{Construct a normalized chat message} \usage{ -chat_message(id, channel, sender, body, ts, thread = NULL, markup = "plain", - kind = "message", self = NULL, mentions = NULL, raw = NULL) +chat_message( + id, + channel, + sender, + body, + ts, + thread = NULL, + markup = "plain", + kind = "message", + self = NULL, + mentions = NULL, + raw = NULL, + encrypted = FALSE, + sender_verified = NULL +) } \arguments{ \item{id}{Message identifier (character).} @@ -46,6 +59,18 @@ exactly as the transport layer handed it over. Shape is adapter-specific and may already be normalized by the transport package (Matrix hands over an extracted record, not the timeline event), so it is an escape hatch, not a guarantee of completeness.} + +\item{encrypted}{Logical: did this message arrive end-to-end +encrypted? FALSE on transports without E2EE and on cleartext +messages in rooms that have it.} + +\item{sender_verified}{Logical: does the sender identifier bind to a +device whose keys this client verified? NULL on cleartext messages, +where the transport asserts the sender and there is nothing to +verify. On an encrypted message FALSE is a real answer, not a +missing one: the payload decrypted, but its claimed sender could not +be tied to a verified device, so the identifier is the homeserver's +word rather than cryptographic fact.} } \value{ A list with class \code{chat_message}. diff --git a/man/chat_send.Rd b/man/chat_send.Rd index 0b897e1..99c5a8e 100644 --- a/man/chat_send.Rd +++ b/man/chat_send.Rd @@ -3,9 +3,19 @@ \alias{chat_send} \title{Send a message through a chat client} \usage{ -chat_send(client, channel, text, markup = c("plain", "markdown"), - thread = NULL, reply_to = NULL, identity = NULL, files = NULL, - kind = "message", notify = TRUE, ...) +chat_send( + client, + channel, + text, + markup = c("plain", "markdown"), + thread = NULL, + reply_to = NULL, + identity = NULL, + files = NULL, + kind = "message", + notify = TRUE, + ... +) } \arguments{ \item{client}{A \code{chat_client}.} @@ -37,7 +47,13 @@ supported.} \item{...}{Adapter-specific options.} } \value{ -The sent message id (character), invisibly. +Character vector of the message ids this call created, in the + order they were sent, invisibly. Usually length one. A platform that + splits a send into several events returns one id per event: the + Matrix adapter sends each attachment as its own event, so a send + with files returns the attachment ids followed by the text id. + Callers that track their own traffic by id must handle every + element, or an unclaimed event reads as somebody else's message. } \description{ Send a message through a chat client diff --git a/man/chat_slack.Rd b/man/chat_slack.Rd index 57b9641..ecbf5fd 100644 --- a/man/chat_slack.Rd +++ b/man/chat_slack.Rd @@ -3,8 +3,13 @@ \alias{chat_slack} \title{Create a Slack chat client} \usage{ -chat_slack(channels = character(), token = Sys.getenv("SLACK_TOKEN"), - username = NULL, .history = NULL, .post = NULL) +chat_slack( + channels = character(), + token = Sys.getenv("SLACK_TOKEN"), + username = NULL, + .history = NULL, + .post = NULL +) } \arguments{ \item{channels}{Character vector of channels to poll.} From ed6f3ce7b77d96e49634ce2a5ce2539192c2852d Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 11:05:41 -0500 Subject: [PATCH 02/14] rformat + document --- R/matrix-crypto.R | 24 +++++++++++------------- R/matrix.R | 32 ++++++++++++++++---------------- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/R/matrix-crypto.R b/R/matrix-crypto.R index eb05b9e..09b4a7c 100644 --- a/R/matrix-crypto.R +++ b/R/matrix-crypto.R @@ -35,8 +35,7 @@ matrix_crypto_store <- function(app = NULL) { matrix_crypto_ops <- function(override = NULL) { ops <- list(init = matrix_crypto_init, encrypted = matrix_room_is_encrypted, - send = matrix_crypto_send, - decrypt = matrix_crypto_decrypt) + send = matrix_crypto_send, decrypt = matrix_crypto_decrypt) for (nm in names(override)) { ops[[nm]] <- override[[nm]] } @@ -68,7 +67,7 @@ matrix_crypto_init <- function(mx, store = NULL, app = NULL) { crypto$sessions <- mx.client::mx_crypto_sessions_load(store) crypto$encrypted <- matrix_crypto_load_encrypted(store) crypto$self_curve <- - mx.crypto::mxc_account_identity_keys(acct)$curve25519 + mx.crypto::mxc_account_identity_keys(acct)$curve25519 # Ask each joined room for its encryption state up front rather than # waiting for a sync to mention it. Best-effort per room; a transient # failure defers that room to sync-time detection. @@ -176,8 +175,7 @@ matrix_encrypted_senders <- function(sync) { out <- character() for (room in sync$rooms$join %||% list()) { for (ev in room$timeline$events %||% list()) { - if (isTRUE(ev$type == "m.room.encrypted") && - !is.null(ev$sender)) { + if (isTRUE(ev$type == "m.room.encrypted") && !is.null(ev$sender)) { out <- c(out, ev$sender) } } @@ -240,14 +238,14 @@ matrix_crypto_send <- function(crypto, mx, room_id, text, msgtype = "m.text", content <- matrix_crypto_content(text, msgtype = msgtype, markdown = markdown, mentions = mentions) res <- tryCatch( - mx.client::mx_send_encrypted(mx, crypto$account, crypto$sessions, - room_id, content, crypto$store, - member_ids = members), - error = function(e) { - warning("chat.api: encrypted send failed: ", conditionMessage(e), - call. = FALSE) - NULL - }) + mx.client::mx_send_encrypted(mx, crypto$account, crypto$sessions, + room_id, content, crypto$store, + member_ids = members), + error = function(e) { + warning("chat.api: encrypted send failed: ", conditionMessage(e), + call. = FALSE) + NULL + }) if (is.null(res)) { return(NULL) } diff --git a/R/matrix.R b/R/matrix.R index 05d90d0..715cd16 100644 --- a/R/matrix.R +++ b/R/matrix.R @@ -82,9 +82,9 @@ #' @export chat_matrix <- function(app = NULL, path = NULL, save_cursor = TRUE, mx = NULL, relogin = TRUE, e2ee = FALSE, - crypto_store = NULL, .sync = NULL, - .extract = NULL, .send = NULL, .media = NULL, - .typing = NULL, .crypto = NULL) { + crypto_store = NULL, .sync = NULL, .extract = NULL, + .send = NULL, .media = NULL, .typing = NULL, + .crypto = NULL) { seams <- list(.sync, .extract, .send, .media) if ((is.null(mx) || any(vapply(seams, is.null, logical(1)))) && !requireNamespace("mx.client", quietly = TRUE)) { @@ -220,13 +220,13 @@ chat_poll.chat_matrix <- function(client, since = NULL, timeout = NULL, ...) { # concatenate the results itself. if (!is.null(client$env$crypto)) { dec <- tryCatch( - client$crypto_ops$decrypt(client$env$crypto, res$sync, - client$env$mx), - error = function(e) { - warning("chat.api: decrypt failed: ", conditionMessage(e), - call. = FALSE) - list() - }) + client$crypto_ops$decrypt(client$env$crypto, res$sync, + client$env$mx), + error = function(e) { + warning("chat.api: decrypt failed: ", conditionMessage(e), + call. = FALSE) + list() + }) for (d in dec) { ms <- d$ts %||% event_ts[[as.character(d$event_id)]] messages[[length(messages) + 1L]] <- chat_message( @@ -235,7 +235,7 @@ chat_poll.chat_matrix <- function(client, since = NULL, timeout = NULL, ...) { sender = as.character(d$sender), body = as.character(d$body), ts = if (is.null(ms)) as.POSIXct(NA) else - as.POSIXct(ms / 1000, origin = "1970-01-01"), + as.POSIXct(ms / 1000, origin = "1970-01-01"), markup = "plain", kind = matrix_kind(d$msgtype), self = isTRUE(d$is_self), mentions = unlist(d$mentions, use.names = FALSE), @@ -303,9 +303,9 @@ chat_send.chat_matrix <- function(client, channel, text, # markdown send renders the same either way. Only the envelope # differs. if (!is.null(crypto) && - client$crypto_ops$encrypted(crypto, client$env$mx, channel)) { - event <- client$crypto_ops$send( - crypto, client$env$mx, channel, text, msgtype = msgtype, + client$crypto_ops$encrypted(crypto, client$env$mx, channel)) { + event <- client$crypto_ops$send(crypto, client$env$mx, channel, + text, msgtype = msgtype, markdown = identical(markup, "markdown"), mentions = list(...)$mentions) return(invisible(c(media_ids, as.character(event)))) @@ -376,8 +376,8 @@ chat_capabilities.chat_matrix <- function(client, ...) { # client that PUTs it on the homeserver in the clear. list(threads = FALSE, thread_replies = FALSE, edits = FALSE, reactions = FALSE, files = TRUE, typing = TRUE, - e2ee = !is.null(client$env$crypto), - identity_override = FALSE, markup_dialects = c("plain", "markdown"), + e2ee = !is.null(client$env$crypto), identity_override = FALSE, + markup_dialects = c("plain", "markdown"), max_message_bytes = NA_integer_) } From 68dc268f84d6b339a81f3896c0748f34a3e21fb6 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 11:09:38 -0500 Subject: [PATCH 03/14] Intern the crypto context per identity One identity is one Olm account. A consumer that rebuilds its client per use -- corteza does, deliberately, so the access token that rotates mid-loop is never cached in a stale config -- would otherwise load a second account and republish 50 one-time keys on every build, with two contexts writing over each other's pickles in the same store. Interning reconciles a short-lived client with a long-lived crypto identity: the token is derived at use, the account is not. The key is a plain identity string (the store path, or "app:") rather than a resolved directory, because resolving one calls into mx.client and the .crypto seam exists so these paths run without it. --- R/matrix-crypto.R | 43 ++++++++++++++++++++++++++++++ R/matrix.R | 9 ++++++- inst/tinytest/test_matrix.R | 53 +++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 1 deletion(-) diff --git a/R/matrix-crypto.R b/R/matrix-crypto.R index 09b4a7c..0db6317 100644 --- a/R/matrix-crypto.R +++ b/R/matrix-crypto.R @@ -27,6 +27,49 @@ matrix_crypto_store <- function(app = NULL) { mx.client::mx_crypto_store_dir(app = app %||% "chat.api") } +# Crypto contexts already built in this session, keyed per identity. +# +# One identity is one Olm account, and building a second client for the +# same identity must not mint a second context: the account pickle, its +# published one-time keys, and the Megolm session set are all per-device +# state, and two contexts writing one store would overwrite each other's +# pickles. +# +# This is what lets a consumer rebuild its client as often as it likes. +# corteza builds one per use on purpose -- the access token rotates +# mid-loop, so anything holding a config from before a relogin keeps +# authenticating with the rejected one -- and interning means that costs +# an environment lookup rather than an account load and a 50-key upload. +.crypto_cache <- new.env(parent = emptyenv()) + +# The identity a store belongs to, as a cache key. Deliberately not the +# resolved directory: resolving one calls into mx.client, and a .crypto +# seam exists precisely so the e2ee paths run without it. +matrix_crypto_cache_key <- function(store = NULL, app = NULL) { + store %||% paste0("app:", app %||% "chat.api") +} + +matrix_crypto_context <- function(key, factory) { + if (!is.null(.crypto_cache[[key]])) { + return(.crypto_cache[[key]]) + } + ctx <- factory() + .crypto_cache[[key]] <- ctx + ctx +} + +# Drop interned contexts. Tests need it because they build many clients +# per identity on purpose; nothing in production calls it, since forgetting +# a context loses the Megolm sessions decrypted since the last save. +matrix_crypto_forget <- function(key = NULL) { + if (is.null(key)) { + rm(list = ls(.crypto_cache), envir = .crypto_cache) + } else if (!is.null(.crypto_cache[[key]])) { + rm(list = key, envir = .crypto_cache) + } + invisible(NULL) +} + # The crypto operations the adapter calls, as a replaceable set. Same # reason chat_matrix() takes .sync and .extract: the Matrix methods have # to be verifiable on a runner with neither mx.client nor a Rust diff --git a/R/matrix.R b/R/matrix.R index 715cd16..c83dc89 100644 --- a/R/matrix.R +++ b/R/matrix.R @@ -120,7 +120,14 @@ chat_matrix <- function(app = NULL, path = NULL, save_cursor = TRUE, "`app` or `crypto_store` too, so the crypto store is ", "unique to this identity.", call. = FALSE) } - env$crypto <- ops$init(mx, store = crypto_store, app = app) + # Interned per identity: rebuilding a client for the same bot + # reuses its one Olm account instead of loading a second and + # republishing its keys. A consumer that derives a fresh client + # per use, so the rotating access token is never cached, still + # keeps one crypto identity across the whole run. + env$crypto <- matrix_crypto_context( + matrix_crypto_cache_key(crypto_store, app), + function() ops$init(mx, store = crypto_store, app = app)) } structure(list(env = env, app = app, save_cursor = isTRUE(save_cursor), relogin = isTRUE(relogin), e2ee = isTRUE(e2ee), diff --git a/inst/tinytest/test_matrix.R b/inst/tinytest/test_matrix.R index 0cc8940..fb5ceb4 100644 --- a/inst/tinytest/test_matrix.R +++ b/inst/tinytest/test_matrix.R @@ -477,8 +477,12 @@ local({ # events reach the caller as ordinary chat_message records. # A fake crypto context plus ops that record what the adapter asked for. +# Contexts are interned per identity, so a fresh fake starts by dropping +# whatever the previous one interned; otherwise every case after the first +# would silently reuse an earlier fake's context. fake_crypto <- function(encrypted_rooms = "!enc:ex", decrypted = list(), event_id = "$enc1", decrypt_error = FALSE) { + chat.api:::matrix_crypto_forget() log <- new.env(parent = emptyenv()) log$init <- list() log$asked <- character() @@ -573,6 +577,55 @@ expect_true(is.environment( .send = function(...) "$id", .media = function(...) NULL, .crypto = fake_crypto()$ops)$env$crypto)) +# ---- One Olm account per identity ---- +# A consumer that derives a fresh client per use -- corteza does, so the +# rotating access token is never cached -- must not mint a second Olm +# account each time. Interning is what reconciles a short-lived client +# with a long-lived crypto identity. + +f <- fake_crypto() +a <- seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) +b <- seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) +expect_identical(length(f$log$init), 1L) +expect_true(identical(a$env$crypto, b$env$crypto)) + +# Two identities are two contexts. Sharing one would have the second bot +# come up wearing the first's device keys. +f <- fake_crypto() +one <- seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) +two <- seam_client(e2ee = TRUE, app = "tiny", .crypto = f$ops) +expect_identical(length(f$log$init), 2L) +expect_identical(f$log$init[[1L]]$app, "cornelius") +expect_identical(f$log$init[[2L]]$app, "tiny") +# The fake returns one shared environment, so identity of the context +# cannot be the assertion here; that init ran twice is. +expect_identical(sort(names(f$log$init[[1L]])), c("app", "mx", "store")) + +# An explicit crypto_store keys on the store, and the app default keys on +# the app, so the two never collide. +f <- fake_crypto() +seam_client(e2ee = TRUE, crypto_store = "/tmp/store-a", .crypto = f$ops) +seam_client(e2ee = TRUE, crypto_store = "/tmp/store-a", .crypto = f$ops) +seam_client(e2ee = TRUE, crypto_store = "/tmp/store-b", .crypto = f$ops) +expect_identical(length(f$log$init), 2L) + +# The cache key is a plain identity string, not a resolved directory: +# resolving one calls into mx.client, and the seam exists so the e2ee +# paths run without it. +expect_identical(chat.api:::matrix_crypto_cache_key(NULL, NULL), "app:chat.api") +expect_identical(chat.api:::matrix_crypto_cache_key(NULL, "tiny"), "app:tiny") +expect_identical(chat.api:::matrix_crypto_cache_key("/s", "tiny"), "/s") + +# Forgetting one identity leaves the others interned. +f <- fake_crypto() +seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) +seam_client(e2ee = TRUE, app = "tiny", .crypto = f$ops) +chat.api:::matrix_crypto_forget("app:tiny") +seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) +seam_client(e2ee = TRUE, app = "tiny", .crypto = f$ops) +expect_identical(length(f$log$init), 3L) +expect_identical(f$log$init[[3L]]$app, "tiny") + # ---- Encrypted sends ---- # An encrypted room takes the Megolm path and the cleartext send never From 140b1a693bde83b1f8e8169ec2d8d7daa2536b30 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 13:18:32 -0500 Subject: [PATCH 04/14] Fail closed on every E2EE boundary Eight defects in the first pass, all at the edges where the adapter decides between the encrypted path and the clear one. Each had the same shape: a failure treated as an answer. 1. An unanswerable encryption state is no longer a plaintext room. matrix_room_is_encrypted() turned every lookup error -- expired token, timeout, 500 -- into FALSE, and chat_send() then used the cleartext path, so a room not already in the cache could leak one message whenever its state could not be checked. It now aborts the send. A room already known encrypted still short-circuits, so a transient failure cannot block one. 2. Attachments are refused in encrypted rooms. The encryption check ran after the upload loop, so files reached the homeserver in the clear and only the text took the Megolm path -- and an attachment-only send never consulted crypto at all. The check now runs before anything is uploaded, and chat_capabilities()$files reports FALSE on an e2ee client rather than advertising something that fails in exactly the rooms such a client exists for. 3. Stores and interned contexts are keyed to a Matrix device. The cache key was the app name or a raw store string, so two clients built the documented way -- chat_matrix(mx = ..., e2ee = TRUE) -- collapsed onto one entry and shared an Olm account; the explicit-path guard never covered ready mx clients, and is gone in favour of a key that cannot collide by construction. An Olm account belongs to a device, so the key and the store path are (user_id, device_id), both required. The directory name is still sanitized and so still not injective, which is why the exact identity is written into the store and compared on every open: a collision, a copied store, or a changed device_id is an error instead of a silent account swap. 4. A sync is not consumed until its crypto state is on disk. The cursor was committed inside mx_sync_update(); a crash between that and the session save skipped the sync carrying a room key permanently, since the homeserver never re-sends it. On e2ee clients the cursor is now written after the crypto state, through a .save seam. And decrypt errors propagate: mx.client already skips an individual event it has no Megolm session for, so a throw here is to-device processing or persistence, and swallowing it while keeping the advanced cursor acknowledged a sync whose keys were lost. Cleartext clients keep the cursor inside the sync, where the poison-pill protection wants it. 5. Crypto initialization happens after a request has succeeded, not at construction. Publishing keys is authenticated, and the stored token may already be rejected at process start; building in chat_matrix() put that upload ahead of any relogin, so the constructor threw, the poll loop never ran, and every restart repeated with the same dead token. The context is built on first poll or send, off the config that just worked. chat_capabilities()$e2ee therefore reads the setting rather than the context, so it cannot flip after the first poll. 6. Failed membership discovery no longer reports a successful send. mx_send_encrypted() derives its recipients from member_ids, so the character() that a swallowed lookup error produced shared the room key with nobody, posted the m.room.encrypted event anyway, and returned an event id -- unreadable to the room, recorded as sent by the caller. Membership failure and an empty member list both abort. Send errors propagate too, matching the cleartext path. 7. Sender verification can no longer be falsely negative forever. sender_bound is stamped once, when a room key arrives over to-device, and persisted with the session; a later sync carrying a timeline message from that sender does not rebind it. The device query asked only about timeline senders, so a key arriving in a to-device-only sync -- the normal case, since the key is shared before the message -- was recorded unverified permanently. To-device envelope senders are now included. 8. Mixed timelines keep the homeserver's order. Decrypted records were appended after the cleartext ones, so an encrypted message followed by a plain reply came back reversed, which reorders a room's commands against the messages they act on. Both sets are now folded back into sync order; anything the sync did not position sorts last, in arrival order. Two of these initially escaped their own tests: matrix_room_is_encrypted() and matrix_crypto_send() sit behind the .crypto seam, so mutating them changed nothing the suite could see. They are now driven directly against stubbed mx.client/mx.api entry points in test_matrix_mxclient.R, along with matrix_crypto_init()'s store binding. 354 -> 386 assertions, ten mutations checked, all caught. R CMD check clean but for the expected new-submission NOTE. --- DESCRIPTION | 2 +- R/matrix-crypto.R | 201 ++++++++++--- R/matrix.R | 236 ++++++++++----- inst/tinytest/test_matrix.R | 434 +++++++++++++++++++++------ inst/tinytest/test_matrix_mxclient.R | 233 ++++++++++++++ man/chat_matrix.Rd | 32 +- 6 files changed, 920 insertions(+), 218 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 97b2313..1387137 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: chat.api Type: Package Title: Transport-Agnostic Chat Contract for R Agents -Version: 0.0.1.3 +Version: 0.0.1.4 Date: 2026-08-05 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), diff --git a/R/matrix-crypto.R b/R/matrix-crypto.R index 0db6317..83237f1 100644 --- a/R/matrix-crypto.R +++ b/R/matrix-crypto.R @@ -14,17 +14,65 @@ matrix_crypto_available <- function() { requireNamespace("mx.crypto", quietly = TRUE) } +# The Matrix device an Olm account belongs to: (user_id, device_id). +# +# Both are required. An Olm account is a device's identity, not a user's, +# so a store keyed on anything coarser is a store two identities can end +# up sharing -- and the second to open it comes up wearing the first's +# device keys. +matrix_crypto_identity <- function(mx) { + uid <- mx$user_id %||% "" + did <- mx$device_id %||% "" + if (!nzchar(uid) || !nzchar(did)) { + stop("chat.api: end-to-end encryption needs both user_id and ", + "device_id on the Matrix config. An Olm account belongs to a ", + "device, and a store that cannot name one is a store two ", + "identities can share.", call. = FALSE) + } + c(user_id = uid, device_id = did) +} + # Where this identity's Olm account and sessions live. # -# mx.client's own convention, under R_user_dir(app, "data"), deliberately -# NOT derived from the config file's location. corteza computed -# dirname(config)/crypto, which ties the crypto identity to wherever the -# config happens to sit: point a bot at a different config path and it -# silently mints a new device identity and loses every Megolm session it -# had. Keying on the app namespace instead keeps one store per identity -# however the config is addressed. -matrix_crypto_store <- function(app = NULL) { - mx.client::mx_crypto_store_dir(app = app %||% "chat.api") +# Under mx.client's own convention, R_user_dir(app, "data")/crypto, with a +# per-device subdirectory. Deliberately NOT derived from the config file's +# location: corteza computed dirname(config)/crypto, which ties the +# identity to wherever the config happens to sit, so a moved config +# silently minted a new device and lost every Megolm session it had. +# +# The directory name is sanitized and therefore not injective -- "@a/b:ex" +# and "@a_b:ex" land in the same place. That is why the exact identity is +# written into the store and checked on every open: a collision has to +# fail loudly rather than hand one bot another's account. +matrix_crypto_store <- function(mx, app = NULL) { + id <- matrix_crypto_identity(mx) + slug <- gsub("[^A-Za-z0-9._-]", "_", + paste(id[["user_id"]], id[["device_id"]], sep = "|")) + file.path(mx.client::mx_crypto_store_dir(app = app %||% "chat.api"), slug) +} + +# Bind a store to one device identity, and refuse to open one bound to +# another. Written on first use; compared exactly thereafter, so a +# sanitization collision, a copied store directory, or a device_id change +# is an error instead of a silent account swap. +matrix_crypto_bind_identity <- function(store, mx) { + id <- matrix_crypto_identity(mx) + f <- file.path(store, "identity.txt") + if (file.exists(f)) { + have <- readLines(f, warn = FALSE) + want <- c(id[["user_id"]], id[["device_id"]]) + if (!identical(have[seq_along(want)], want)) { + stop("chat.api: crypto store ", store, " belongs to ", + paste(have, collapse = "/"), ", not ", + paste(want, collapse = "/"), + ". Refusing to open another device's Olm account.", + call. = FALSE) + } + return(invisible(store)) + } + dir.create(store, showWarnings = FALSE, recursive = TRUE) + writeLines(c(id[["user_id"]], id[["device_id"]]), f) + invisible(store) } # Crypto contexts already built in this session, keyed per identity. @@ -42,11 +90,18 @@ matrix_crypto_store <- function(app = NULL) { # an environment lookup rather than an account load and a 50-key upload. .crypto_cache <- new.env(parent = emptyenv()) -# The identity a store belongs to, as a cache key. Deliberately not the -# resolved directory: resolving one calls into mx.client, and a .crypto -# seam exists precisely so the e2ee paths run without it. -matrix_crypto_cache_key <- function(store = NULL, app = NULL) { - store %||% paste0("app:", app %||% "chat.api") +# The cache key: the device identity, plus wherever its store was asked +# for. Deliberately not the resolved directory -- resolving one calls into +# mx.client, and a .crypto seam exists precisely so the e2ee paths run +# without it. +# +# The identity has to be in the key. Keying on app or store alone had two +# clients built the documented way, chat_matrix(mx = ..., e2ee = TRUE), +# collapse onto one "app:chat.api" entry and share an Olm account. +matrix_crypto_cache_key <- function(store = NULL, app = NULL, mx = NULL) { + id <- matrix_crypto_identity(mx) + paste(store %||% paste0("app:", app %||% "chat.api"), id[["user_id"]], + id[["device_id"]], sep = "\n") } matrix_crypto_context <- function(key, factory) { @@ -58,6 +113,36 @@ matrix_crypto_context <- function(key, factory) { ctx } +# The client's crypto context, built on first use. +# +# Deliberately not built in chat_matrix(). Initialization publishes keys, +# which is an authenticated request, and the token on disk may already be +# rejected at process start -- that is the case mx_with_relogin() exists +# for. Building at construction put the upload before any relogin could +# happen, so chat_matrix() threw, corteza's poll loop never got to run, +# and every restart repeated with the same dead token. Now the context is +# built after a request has succeeded, off the config that succeeded. +# +# The result is stashed on the client's env, which is shared with every +# copy of the client, so this costs one lookup after the first call. +matrix_crypto_require <- function(client) { + if (!isTRUE(client$e2ee)) { + return(NULL) + } + if (!is.null(client$env$crypto)) { + return(client$env$crypto) + } + mx <- client$env$mx + ctx <- matrix_crypto_context( + matrix_crypto_cache_key(client$crypto_store, client$app, mx), + function() { + client$crypto_ops$init(mx, store = client$crypto_store, + app = client$app) + }) + client$env$crypto <- ctx + ctx +} + # Drop interned contexts. Tests need it because they build many clients # per identity on purpose; nothing in production calls it, since forgetting # a context loses the Megolm sessions decrypted since the last save. @@ -98,7 +183,8 @@ matrix_crypto_init <- function(mx, store = NULL, app = NULL) { stop("Matrix end-to-end encryption requires the 'mx.client' and ", "'mx.crypto' packages.", call. = FALSE) } - store <- store %||% matrix_crypto_store(app = app) + store <- store %||% matrix_crypto_store(mx, app = app) + matrix_crypto_bind_identity(store, mx) acct <- mx.client::mx_crypto_account(store) mx.client::mx_crypto_publish_keys(mx, acct, store, n_otks = 50L) crypto <- new.env(parent = emptyenv()) @@ -203,6 +289,7 @@ matrix_crypto_decrypt <- function(crypto, sync, mx) { } else { NULL } + res <- mx.client::mx_crypto_process_sync( crypto$account, crypto$sessions, sync, crypto$self_curve, self_id = mx$user_id, devices = devices) @@ -211,9 +298,17 @@ matrix_crypto_decrypt <- function(crypto, sync, mx) { res$events } -# Distinct senders of m.room.encrypted timeline events in this sync, so -# the device query asks about the people who actually spoke rather than -# every room member. +# Distinct senders the device query has to cover, so it asks about the +# people who actually spoke rather than every room member. +# +# Both halves matter, and the to-device half is the one that is easy to +# miss. sender_bound is stamped once, when a room key arrives over +# to-device, and it is persisted with the Megolm session; a later sync +# that finally carries a timeline message from that sender does not rebind +# it. A room key that arrives in a to-device-only sync -- which is the +# normal case, since the key is shared before the message -- would +# therefore be recorded unverified forever, and every message decrypted +# with it would report sender_verified = FALSE. matrix_encrypted_senders <- function(sync) { out <- character() for (room in sync$rooms$join %||% list()) { @@ -223,10 +318,22 @@ matrix_encrypted_senders <- function(sync) { } } } + for (ev in sync$to_device$events %||% list()) { + if (isTRUE(ev$type == "m.room.encrypted") && !is.null(ev$sender)) { + out <- c(out, ev$sender) + } + } unique(out) } # Is this room one the adapter knows to be encrypted? +# +# Fails closed. A room not already known encrypted is asked about, so one +# that turned encryption on between polls does not get a cleartext send -- +# and if that question cannot be answered, the caller gets an error rather +# than FALSE. An expired token, a timeout, or a 500 all used to come back +# as "not encrypted", which sent the message in the clear: on an E2EE +# client, unknown is not the same answer as unencrypted. matrix_room_is_encrypted <- function(crypto, mx, room_id) { if (is.null(crypto)) { return(FALSE) @@ -234,16 +341,19 @@ matrix_room_is_encrypted <- function(crypto, mx, room_id) { if (room_id %in% crypto$encrypted) { return(TRUE) } - # Not in the cached set: ask, so a room that turned on encryption - # between polls does not get a cleartext send. A failed lookup answers - # FALSE, which is the pre-encryption behaviour rather than a hard error. - enc <- isTRUE(tryCatch(mx.client::mx_room_encrypted(mx, room_id), - error = function(e) FALSE)) - if (enc) { + enc <- withCallingHandlers( + tryCatch(mx.client::mx_room_encrypted(mx, room_id), + error = function(e) { + stop("chat.api: cannot determine the encryption state of ", room_id, + ": ", conditionMessage(e), + ". Refusing to send, because an unanswered question is ", + "not a plaintext room.", call. = FALSE) + }), warning = function(w) invokeRestart("muffleWarning")) + if (isTRUE(enc)) { crypto$encrypted <- c(crypto$encrypted, room_id) matrix_crypto_save_encrypted(crypto) } - enc + isTRUE(enc) } # The m.room.message content an encrypted send wraps. Deliberately the @@ -267,31 +377,32 @@ matrix_crypto_content <- function(text, msgtype = "m.text", markdown = FALSE, content } -# Encrypt and send. Returns the event id, or NULL on failure so the -# caller sees the same "no event id" signal the cleartext path gives. +# Encrypt and send. Errors propagate, the same way the cleartext path's do +# -- a caller that wants "no event id" on failure wraps the call, and one +# that does not should hear about it. +# +# Membership discovery is not best-effort. mx_send_encrypted() derives the +# recipient devices from member_ids, so an empty list means it shares the +# room key with nobody, posts the m.room.encrypted event anyway, and +# returns an event id. On a fresh outbound session nobody in the room can +# read that message, while the caller records a successful send. Swallowing +# the lookup error bought exactly that outcome, so it is gone: skipping +# individual devices that cannot be verified is mx.client's decision to +# make, and it is a different one. matrix_crypto_send <- function(crypto, mx, room_id, text, msgtype = "m.text", markdown = FALSE, mentions = NULL) { - s <- tryCatch(mx.client::mx_client_session(mx), error = function(e) NULL) - members <- if (is.null(s)) { - character() - } else { - tryCatch(mx.api::mx_room_members(s, room_id), - error = function(e) character()) + members <- mx.api::mx_room_members(mx.client::mx_client_session(mx), + room_id) + if (!length(members)) { + stop("chat.api: no members found for ", room_id, + ". Refusing to send an encrypted event whose room key would ", + "reach nobody.", call. = FALSE) } content <- matrix_crypto_content(text, msgtype = msgtype, markdown = markdown, mentions = mentions) - res <- tryCatch( - mx.client::mx_send_encrypted(mx, crypto$account, crypto$sessions, - room_id, content, crypto$store, - member_ids = members), - error = function(e) { - warning("chat.api: encrypted send failed: ", conditionMessage(e), - call. = FALSE) - NULL - }) - if (is.null(res)) { - return(NULL) - } + res <- mx.client::mx_send_encrypted(mx, crypto$account, crypto$sessions, + room_id, content, crypto$store, + member_ids = members) crypto$sessions <- res$sessions res$event_id } diff --git a/R/matrix.R b/R/matrix.R index c83dc89..456409f 100644 --- a/R/matrix.R +++ b/R/matrix.R @@ -39,15 +39,25 @@ #' inbound. FALSE (the default) is the previous behaviour exactly: #' cleartext only, no crypto state, and \code{e2ee} stays FALSE in #' \code{\link{chat_capabilities}}. Requires the 'mx.crypto' package, -#' which needs a Rust toolchain. +#' which needs a Rust toolchain, and both \code{user_id} and +#' \code{device_id} on the wrapped config. +#' +#' Three things change when it is on. The crypto state is built on the +#' first poll or send rather than here, so key publication happens after +#' a relogin rather than before one. The sync cursor is written after +#' the crypto state instead of inside the sync, so a sync carrying a +#' room key is not marked consumed until that key is on disk. And +#' attachments are refused in encrypted rooms, because Matrix media +#' uploads are not encrypted by this adapter -- +#' \code{\link{chat_capabilities}} reports \code{files = FALSE} to +#' match. #' @param crypto_store Character or NULL. Directory holding the pickled #' Olm account and sessions. NULL derives it from \code{app} via -#' \code{mx.client::mx_crypto_store_dir()}, which keys one store per -#' app namespace regardless of where the config file lives, so moving a -#' config does not cost the device identity. Because that key is the app -#' name alone, an \code{e2ee = TRUE} client built from an explicit -#' \code{path} with no \code{app} is an error: name one or the other, or -#' two such clients would quietly share a device identity. +#' \code{mx.client::mx_crypto_store_dir()}, with a per-device +#' subdirectory, so moving a config does not cost the device identity +#' and two bots under one app namespace do not share an Olm account. +#' Either way the store records the \code{(user_id, device_id)} it +#' belongs to and refuses to open for a different one. #' @param .sync Testing seam: replacement for #' \code{mx.client::mx_sync_update}. Leave NULL in production. #' @param .extract Testing seam: replacement for @@ -70,6 +80,10 @@ #' with neither mx.crypto nor a Rust toolchain; E2EE is the one part of #' this adapter with no honest way to reach a real homeserver from a #' test. +#' @param .save Testing seam: replacement for +#' \code{mx.client::mx_client_save}. Leave NULL in production. Only +#' reached on an \code{e2ee} client, which writes the sync cursor after +#' the crypto state rather than inside the sync. #' @return A \code{chat_client} of class \code{chat_matrix}. #' \code{\link{chat_poll}} on this class returns \code{first_run} and #' \code{client} alongside \code{messages}, \code{cursor}, and @@ -84,7 +98,7 @@ chat_matrix <- function(app = NULL, path = NULL, save_cursor = TRUE, mx = NULL, relogin = TRUE, e2ee = FALSE, crypto_store = NULL, .sync = NULL, .extract = NULL, .send = NULL, .media = NULL, .typing = NULL, - .crypto = NULL) { + .crypto = NULL, .save = NULL) { seams <- list(.sync, .extract, .send, .media) if ((is.null(mx) || any(vapply(seams, is.null, logical(1)))) && !requireNamespace("mx.client", quietly = TRUE)) { @@ -105,37 +119,29 @@ chat_matrix <- function(app = NULL, path = NULL, save_cursor = TRUE, # holds one object and never touches Olm or Megolm itself. NULL when # e2ee is off, which is the default and is exactly the pre-existing # behaviour. + # No crypto context yet, even with e2ee on. Initialization publishes + # keys, which is an authenticated request, and the stored token may + # already be rejected at process start. Doing it here put that upload + # ahead of any relogin, so the constructor threw before the poll loop + # existed and every restart repeated with the same dead token. It is + # built on first use instead, off a config that has just worked. See + # matrix_crypto_require(). env$crypto <- NULL - ops <- matrix_crypto_ops(.crypto) + # The identity check runs now rather than at first use, so a config + # that can never key a store fails while the caller is still looking + # at the constructor. if (isTRUE(e2ee)) { - # An explicit config path with no app names one specific identity, - # but the default store is keyed on the app namespace alone, so - # two bots loaded that way would share one account.pickle and the - # second would come up wearing the first's device keys. Deriving - # the store from the config's own directory instead only trades - # that for a quieter failure: move the config and the bot mints a - # new identity and loses every Megolm session it had. So ask. - if (is.null(crypto_store) && !is.null(path) && is.null(app)) { - stop("chat_matrix(e2ee = TRUE) with an explicit `path` needs ", - "`app` or `crypto_store` too, so the crypto store is ", - "unique to this identity.", call. = FALSE) - } - # Interned per identity: rebuilding a client for the same bot - # reuses its one Olm account instead of loading a second and - # republishing its keys. A consumer that derives a fresh client - # per use, so the rotating access token is never cached, still - # keeps one crypto identity across the whole run. - env$crypto <- matrix_crypto_context( - matrix_crypto_cache_key(crypto_store, app), - function() ops$init(mx, store = crypto_store, app = app)) + matrix_crypto_identity(mx) } structure(list(env = env, app = app, save_cursor = isTRUE(save_cursor), relogin = isTRUE(relogin), e2ee = isTRUE(e2ee), + crypto_store = crypto_store, sync_fn = .sync %||% mx.client::mx_sync_update, extract_fn = .extract %||% mx.client::mx_extract_text_events, send_fn = .send %||% mx.client::mx_send_text, media_fn = .media %||% mx.client::mx_send_media, - typing_fn = .typing, crypto_ops = ops), + save_fn = .save %||% mx.client::mx_client_save, + typing_fn = .typing, crypto_ops = matrix_crypto_ops(.crypto)), class = c("chat_matrix", "chat_client")) } @@ -160,6 +166,42 @@ matrix_event_times <- function(sync) { out } +# Each timeline event's position in the sync, by event id. Walks the same +# structure matrix_event_times() does, and for the same reason: it is the +# only place the homeserver's own ordering survives, since the cleartext +# extractor and the decrypt step each return their own events and neither +# knows about the other's. +# +# Not origin_server_ts. Two events in one room can share a millisecond, +# and a homeserver's clock is not the ordering the room agreed on. +matrix_event_order <- function(sync) { + out <- list() + i <- 0L + for (room in sync$rooms$join %||% list()) { + for (ev in room$timeline$events %||% list()) { + i <- i + 1L + if (!is.null(ev$event_id)) { + out[[as.character(ev$event_id)]] <- i + } + } + } + out +} + +# Sort messages into sync order. Unpositioned events keep their arrival +# order at the end rather than being dropped or interleaved by guesswork, +# and the tie-break on index makes the result deterministic either way. +matrix_order_messages <- function(messages, positions) { + if (length(messages) < 2L) { + return(messages) + } + pos <- vapply(messages, function(m) { + p <- positions[[as.character(m$id)]] + if (is.null(p)) Inf else as.numeric(p) + }, numeric(1)) + messages[order(pos, seq_along(pos))] +} + # Matrix msgtype -> the contract's kind vocabulary. chat_send maps the # same pair in the other direction, so a message that makes a round trip # keeps its kind. Leaving m.text on the record would make every Matrix @@ -180,9 +222,23 @@ chat_poll.chat_matrix <- function(client, since = NULL, timeout = NULL, ...) { if (!is.null(since)) { client$env$mx$sync_token <- since } + # With E2EE on the cursor is written after the crypto state, not + # inside the sync. mx_sync_update(save = TRUE) commits the advanced + # token before anything parses the response, which is what makes a + # malformed event survivable -- crash, restart, resume past it. But a + # sync carrying a room key is only consumed once that key is on disk, + # and a crash between the two loses it permanently: the key is never + # re-sent, so every later message in that room is undecryptable. + # + # So the poison-pill protection is traded for durability, and only on + # e2ee clients. What is left in the window is one sync's worth of + # replay, which is idempotent -- room keys and Megolm sessions are + # keyed by session id. + defer_save <- isTRUE(client$e2ee) && isTRUE(client$save_cursor) do_sync <- function(mx) { client$sync_fn(mx, timeout = as.integer((timeout %||% 0) * 1000), - save = client$save_cursor, app = client$app, ...) + save = isTRUE(client$save_cursor) && !defer_save, + app = client$app, ...) } # The relogin wrapper hands the retry a re-authenticated config, so # the sync has to run off whatever config it is given rather than @@ -197,7 +253,28 @@ chat_poll.chat_matrix <- function(client, since = NULL, timeout = NULL, ...) { } client$env$mx <- res$client + # Crypto runs before the cursor is committed and after the sync has + # succeeded: after, so a relogin has already replaced a rejected token + # and initialization publishes keys with one that works; before, so a + # sync whose room keys did not reach disk is not marked consumed. + # + # Errors propagate. mx.client already skips an individual event it has + # no Megolm session for, so a throw here is the other kind of failure + # -- to-device processing, or the session save itself -- and swallowing + # it while keeping the advanced cursor acknowledged a sync whose keys + # were lost. + dec <- list() + if (isTRUE(client$e2ee)) { + crypto <- matrix_crypto_require(client) + dec <- client$crypto_ops$decrypt(crypto, res$sync, client$env$mx) + } + if (defer_save) { + client$env$mx <- client$save_fn(client$env$mx, app = client$app) + res$client <- client$env$mx + } + event_ts <- matrix_event_times(res$sync) + event_pos <- matrix_event_order(res$sync) recs <- client$extract_fn(res$sync, self_id = res$client$user_id) messages <- lapply(recs, function(r) { # ts is the event's own origin_server_ts, from the record when @@ -225,32 +302,29 @@ chat_poll.chat_matrix <- function(client, since = NULL, timeout = NULL, ...) { # encrypted except by reading $encrypted. That is the whole point of # owning crypto here: corteza used to run its own decrypt off $raw and # concatenate the results itself. - if (!is.null(client$env$crypto)) { - dec <- tryCatch( - client$crypto_ops$decrypt(client$env$crypto, res$sync, - client$env$mx), - error = function(e) { - warning("chat.api: decrypt failed: ", conditionMessage(e), - call. = FALSE) - list() - }) - for (d in dec) { - ms <- d$ts %||% event_ts[[as.character(d$event_id)]] - messages[[length(messages) + 1L]] <- chat_message( - id = as.character(d$event_id), - channel = as.character(d$room_id), - sender = as.character(d$sender), - body = as.character(d$body), - ts = if (is.null(ms)) as.POSIXct(NA) else - as.POSIXct(ms / 1000, origin = "1970-01-01"), - markup = "plain", kind = matrix_kind(d$msgtype), - self = isTRUE(d$is_self), - mentions = unlist(d$mentions, use.names = FALSE), - encrypted = TRUE, - sender_verified = isTRUE(d$sender_verified), - raw = d) - } + for (d in dec) { + ms <- d$ts %||% event_ts[[as.character(d$event_id)]] + messages[[length(messages) + 1L]] <- chat_message( + id = as.character(d$event_id), + channel = as.character(d$room_id), + sender = as.character(d$sender), + body = as.character(d$body), + ts = if (is.null(ms)) as.POSIXct(NA) else + as.POSIXct(ms / 1000, origin = "1970-01-01"), + markup = "plain", kind = matrix_kind(d$msgtype), + self = isTRUE(d$is_self), + mentions = unlist(d$mentions, use.names = FALSE), + encrypted = TRUE, + sender_verified = isTRUE(d$sender_verified), + raw = d) } + # Back into the order the homeserver sent them. Appending the + # decrypted events put every one of them after every cleartext one, + # so an encrypted message followed by a plain reply came out + # backwards -- which reorders a room's commands against the messages + # they act on. Anything the sync did not position sorts last, in the + # order it arrived. + messages <- matrix_order_messages(messages, event_pos) # first_run says the sync ran without a stored cursor, so these # messages are the homeserver's backfill baseline, not new traffic. # A consumer that drops it replays its whole history as fresh mail @@ -276,6 +350,25 @@ chat_send.chat_matrix <- function(client, channel, text, identity = NULL, files = NULL, kind = "message", notify = TRUE, ...) { markup <- match.arg(markup) + # Resolved before anything is uploaded or posted. The encryption + # question used to be asked after the attachment loop had already run, + # which put the files on the homeserver in the clear before the text + # took the Megolm path -- and an attachment-only send never asked at + # all. + crypto <- matrix_crypto_require(client) + encrypted <- !is.null(crypto) && + client$crypto_ops$encrypted(crypto, client$env$mx, channel) + if (encrypted && !is.null(files)) { + # mx_send_media() posts an ordinary cleartext m.file event: the + # upload is not encrypted, and neither is the URL. There is no + # encrypted-attachment path to fall back to, so this refuses + # rather than leaking. chat_capabilities()$files reports FALSE on + # an e2ee client for the same reason. + stop("chat.api: cannot send attachments to the encrypted room ", + channel, ". Matrix media uploads are not encrypted by this ", + "adapter, and posting them would put the file on the ", + "homeserver in the clear.", call. = FALSE) + } media_ids <- character() if (!is.null(files)) { for (f in files) { @@ -299,18 +392,11 @@ chat_send.chat_matrix <- function(client, channel, text, # attachment's echo as somebody else's message: corteza recognizes # its own traffic by event id. if (!length(media_ids) || any(nzchar(text))) { - crypto <- client$env$crypto - # Encrypted rooms take the Megolm path. Asked per send rather than - # trusting the cached room set, so a room that turned on - # encryption between polls does not get one cleartext message - # first. - # # markup and mentions carry through: the encrypted branch builds # the same m.room.message content the cleartext one does, so a # markdown send renders the same either way. Only the envelope # differs. - if (!is.null(crypto) && - client$crypto_ops$encrypted(crypto, client$env$mx, channel)) { + if (encrypted) { event <- client$crypto_ops$send(crypto, client$env$mx, channel, text, msgtype = msgtype, markdown = identical(markup, "markdown"), @@ -376,14 +462,22 @@ chat_capabilities.chat_matrix <- function(client, ...) { # be neither sent nor received here. # # e2ee answers for this client, not for Matrix and not for the - # installed packages: it is TRUE only when chat_matrix(e2ee = TRUE) - # built a crypto context, which is what actually routes sends through - # mx_send_encrypted and decrypts inbound. Reporting TRUE off an - # installed mx.crypto would invite a consumer to hand a secret to a - # client that PUTs it on the homeserver in the clear. + # installed packages: it is TRUE when this client was built with + # e2ee = TRUE, which is what routes sends through mx_send_encrypted + # and decrypts inbound. Reporting TRUE off an installed mx.crypto + # would invite a consumer to hand a secret to a client that PUTs it in + # the clear. It reads the setting rather than the crypto context + # because the context is built on first use, and a capability that + # flipped after the first poll would be worse than either answer. + # + # files is FALSE on an e2ee client. mx_send_media() posts a cleartext + # m.file event whatever the room's encryption state says, and this + # adapter has no encrypted-attachment path, so chat_send() refuses + # attachments to an encrypted room. TRUE would advertise something + # that fails in exactly the rooms such a client exists for. list(threads = FALSE, thread_replies = FALSE, edits = FALSE, - reactions = FALSE, files = TRUE, typing = TRUE, - e2ee = !is.null(client$env$crypto), identity_override = FALSE, + reactions = FALSE, files = !isTRUE(client$e2ee), typing = TRUE, + e2ee = isTRUE(client$e2ee), identity_override = FALSE, markup_dialects = c("plain", "markdown"), max_message_bytes = NA_integer_) } diff --git a/inst/tinytest/test_matrix.R b/inst/tinytest/test_matrix.R index fb5ceb4..92bae7b 100644 --- a/inst/tinytest/test_matrix.R +++ b/inst/tinytest/test_matrix.R @@ -5,9 +5,12 @@ # The half that pins the adapter to mx.client's real signatures lives in # test_matrix_mxclient.R, which announces its skip. -fake_mx <- function(sync_token = NULL) { - list(user_id = "@bot:ex", server = "https://ex.invalid", - sync_token = sync_token) +# device_id is here because an Olm account belongs to a device: an e2ee +# client refuses a config that cannot name one. +fake_mx <- function(sync_token = NULL, user_id = "@bot:ex", + device_id = "DEV1") { + list(user_id = user_id, server = "https://ex.invalid", + device_id = device_id, sync_token = sync_token) } # An extractor-shaped record: the seven fields mx_extract_text_events @@ -50,7 +53,13 @@ seam_client <- function(recs = list(), sync = wrap_sync(list()), record$sync[[length(record$sync) + 1L]] <- c( list(client = client), list(...)) } - list(sync = sync, client = fake_mx(token), first_run = first_run) + # The post-sync config keeps the identity it went in with. A + # real sync (or relogin) rotates the token, never the + # user_id/device_id, and crypto is keyed on those. + list(sync = sync, + client = fake_mx(token, user_id = client$user_id, + device_id = client$device_id), + first_run = first_run) }, .extract = function(sync_resp, self_id, ...) { if (!is.null(record)) { @@ -473,15 +482,17 @@ local({ # ---- E2EE ---- # The .crypto seam stands in for the whole crypto module, so these run # with neither mx.crypto nor a Rust toolchain. What is under test is the -# adapter's routing: which path a send takes, and whether decrypted -# events reach the caller as ordinary chat_message records. +# adapter's routing: which path a send takes, when crypto state is built +# and persisted, and whether decrypted events reach the caller as +# ordinary chat_message records in the order the homeserver sent them. # A fake crypto context plus ops that record what the adapter asked for. # Contexts are interned per identity, so a fresh fake starts by dropping # whatever the previous one interned; otherwise every case after the first # would silently reuse an earlier fake's context. fake_crypto <- function(encrypted_rooms = "!enc:ex", decrypted = list(), - event_id = "$enc1", decrypt_error = FALSE) { + event_id = "$enc1", decrypt_error = FALSE, + lookup_error = FALSE, init_error = FALSE) { chat.api:::matrix_crypto_forget() log <- new.env(parent = emptyenv()) log$init <- list() @@ -494,10 +505,16 @@ fake_crypto <- function(encrypted_rooms = "!enc:ex", decrypted = list(), init = function(mx, store = NULL, app = NULL) { log$init[[length(log$init) + 1L]] <- list(mx = mx, store = store, app = app) + if (isTRUE(init_error)) { + stop("keys could not be published") + } ctx }, encrypted = function(crypto, mx, room_id) { log$asked <- c(log$asked, room_id) + if (isTRUE(lookup_error)) { + stop("cannot determine the encryption state of ", room_id) + } room_id %in% crypto$encrypted }, send = function(crypto, mx, room_id, text, msgtype = "m.text", @@ -523,59 +540,101 @@ plain <- seam_client() expect_null(plain$env$crypto) expect_false(plain$e2ee) expect_false(chat_capabilities(plain)$e2ee) +expect_true(chat_capabilities(plain)$files) # init is not called either: an unused crypto module must not touch the # store or publish keys. f <- fake_crypto() off <- seam_client(e2ee = FALSE, .crypto = f$ops) +chat_poll(off) +chat_send(off, "!enc:ex", "hi") expect_identical(length(f$log$init), 0L) expect_null(off$env$crypto) -# e2ee = TRUE builds the context, and the capability follows the context -# rather than the installed packages. +# ---- Crypto is built on first use, not at construction ---- +# Initialization publishes keys, which is an authenticated request. The +# stored token may already be rejected at process start -- that is what +# mx_with_relogin() is for -- so building here put the upload ahead of +# any relogin. chat_matrix() threw, the poll loop never ran, and every +# restart repeated with the same dead token. + f <- fake_crypto() enc <- seam_client(e2ee = TRUE, .crypto = f$ops) +expect_identical(length(f$log$init), 0L) +expect_null(enc$env$crypto) +# The capability answers from the setting, not the context, so it does not +# flip after the first poll. +expect_true(chat_capabilities(enc)$e2ee) +# ... and files is FALSE, because mx_send_media posts a cleartext m.file +# event whatever the room's encryption state says. +expect_false(chat_capabilities(enc)$files) + +# A construction that would have died now survives to the first poll, +# which is where relogin gets its chance. +expect_silent(seam_client(e2ee = TRUE, .crypto = fake_crypto(init_error = TRUE)$ops)) +expect_error(chat_poll(seam_client(e2ee = TRUE, + .crypto = fake_crypto(init_error = TRUE)$ops)), + "keys could not be published") + +# The first poll builds it, off the post-sync config rather than the one +# the client was constructed with -- that is the whole point of waiting. +f <- fake_crypto() +enc <- seam_client(e2ee = TRUE, .crypto = f$ops, token = "after-relogin") +chat_poll(enc) expect_identical(length(f$log$init), 1L) +expect_identical(f$log$init[[1L]]$mx$sync_token, "after-relogin") expect_true(is.environment(enc$env$crypto)) -expect_true(chat_capabilities(enc)$e2ee) +# Later polls reuse it. +chat_poll(enc) +expect_identical(length(f$log$init), 1L) -# The store is derived inside init, not by the constructor, so mx.client -# is only needed on the path that does crypto. app and an explicit -# crypto_store both reach it. -f <- fake_crypto() -seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) -expect_identical(f$log$init[[1L]]$app, "cornelius") -expect_null(f$log$init[[1L]]$store) +# A send builds it too, for a client that sends before it polls. f <- fake_crypto() -seam_client(e2ee = TRUE, crypto_store = "/tmp/store", .crypto = f$ops) -expect_identical(f$log$init[[1L]]$store, "/tmp/store") - -# An explicit config path with no app and no crypto_store cannot name a -# unique store: the default is keyed on the app namespace alone, so two -# bots built that way would share one Olm account and the second would -# come up wearing the first's device keys. That is an error, not a guess. -expect_error(chat_matrix(mx = fake_mx(), path = "/tmp/bot.json", e2ee = TRUE, +enc <- seam_client(e2ee = TRUE, .crypto = f$ops) +chat_send(enc, "!enc:ex", "secret") +expect_identical(length(f$log$init), 1L) + +# ---- The store is bound to a device identity ---- +# An Olm account belongs to a device, not a user. A config that cannot +# name one cannot key a store, and the check runs at construction so the +# caller hears about it there. +expect_error(chat_matrix(mx = list(user_id = "@bot:ex"), e2ee = TRUE, .sync = function(...) NULL, .extract = function(...) list(), .send = function(...) "$id", .media = function(...) NULL, .crypto = fake_crypto()$ops), - "unique to this identity") -# The same path with an app names one, so it is allowed. + "user_id and device_id") +expect_error(chat_matrix(mx = list(device_id = "DEV1"), e2ee = TRUE, + .sync = function(...) NULL, + .extract = function(...) list(), + .send = function(...) "$id", + .media = function(...) NULL, + .crypto = fake_crypto()$ops), + "user_id and device_id") +# e2ee off does not need one: a cleartext client has no account to key. +expect_silent(seam_client(mx = list(user_id = "@bot:ex"))) + +# The cache key carries the identity. Keying on app or store alone had two +# clients built the documented way -- chat_matrix(mx = ..., e2ee = TRUE) +# with no app and no store -- collapse onto one entry and share an account. +k <- chat.api:::matrix_crypto_cache_key +expect_false(identical(k(NULL, NULL, fake_mx(user_id = "@a:ex")), + k(NULL, NULL, fake_mx(user_id = "@b:ex")))) +expect_false(identical(k(NULL, NULL, fake_mx(device_id = "D1")), + k(NULL, NULL, fake_mx(device_id = "D2")))) +expect_false(identical(k("/s", NULL, fake_mx(device_id = "D1")), + k("/s", NULL, fake_mx(device_id = "D2")))) +expect_identical(k(NULL, NULL, fake_mx()), k(NULL, NULL, fake_mx())) +expect_false(identical(k(NULL, "a", fake_mx()), k(NULL, "b", fake_mx()))) + +# Two default clients for different bots get two contexts. f <- fake_crypto() -ok <- chat_matrix(mx = fake_mx(), path = "/tmp/bot.json", app = "tiny", - e2ee = TRUE, .sync = function(...) NULL, - .extract = function(...) list(), - .send = function(...) "$id", .media = function(...) NULL, - .crypto = f$ops) -expect_true(is.environment(ok$env$crypto)) -# ... and so does crypto_store on its own. -expect_true(is.environment( - chat_matrix(mx = fake_mx(), path = "/tmp/bot.json", e2ee = TRUE, - crypto_store = "/tmp/store", .sync = function(...) NULL, - .extract = function(...) list(), - .send = function(...) "$id", .media = function(...) NULL, - .crypto = fake_crypto()$ops)$env$crypto)) +chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops, + mx = fake_mx(user_id = "@one:ex"))) +chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops, + mx = fake_mx(user_id = "@two:ex"))) +expect_identical(length(f$log$init), 2L) # ---- One Olm account per identity ---- # A consumer that derives a fresh client per use -- corteza does, so the @@ -586,43 +645,34 @@ expect_true(is.environment( f <- fake_crypto() a <- seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) b <- seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) +chat_poll(a) +chat_poll(b) expect_identical(length(f$log$init), 1L) expect_true(identical(a$env$crypto, b$env$crypto)) -# Two identities are two contexts. Sharing one would have the second bot -# come up wearing the first's device keys. +# An explicit crypto_store keys on the store plus the identity, so two +# stores are two contexts and one store is one. f <- fake_crypto() -one <- seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) -two <- seam_client(e2ee = TRUE, app = "tiny", .crypto = f$ops) +chat_send(seam_client(e2ee = TRUE, crypto_store = "/tmp/store-a", + .crypto = f$ops), "!enc:ex", "x") +chat_send(seam_client(e2ee = TRUE, crypto_store = "/tmp/store-a", + .crypto = f$ops), "!enc:ex", "x") +chat_send(seam_client(e2ee = TRUE, crypto_store = "/tmp/store-b", + .crypto = f$ops), "!enc:ex", "x") expect_identical(length(f$log$init), 2L) -expect_identical(f$log$init[[1L]]$app, "cornelius") -expect_identical(f$log$init[[2L]]$app, "tiny") -# The fake returns one shared environment, so identity of the context -# cannot be the assertion here; that init ran twice is. -expect_identical(sort(names(f$log$init[[1L]])), c("app", "mx", "store")) - -# An explicit crypto_store keys on the store, and the app default keys on -# the app, so the two never collide. -f <- fake_crypto() -seam_client(e2ee = TRUE, crypto_store = "/tmp/store-a", .crypto = f$ops) -seam_client(e2ee = TRUE, crypto_store = "/tmp/store-a", .crypto = f$ops) -seam_client(e2ee = TRUE, crypto_store = "/tmp/store-b", .crypto = f$ops) -expect_identical(length(f$log$init), 2L) - -# The cache key is a plain identity string, not a resolved directory: -# resolving one calls into mx.client, and the seam exists so the e2ee -# paths run without it. -expect_identical(chat.api:::matrix_crypto_cache_key(NULL, NULL), "app:chat.api") -expect_identical(chat.api:::matrix_crypto_cache_key(NULL, "tiny"), "app:tiny") -expect_identical(chat.api:::matrix_crypto_cache_key("/s", "tiny"), "/s") +expect_identical(f$log$init[[1L]]$store, "/tmp/store-a") +expect_identical(f$log$init[[2L]]$store, "/tmp/store-b") # Forgetting one identity leaves the others interned. f <- fake_crypto() -seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) -seam_client(e2ee = TRUE, app = "tiny", .crypto = f$ops) -chat.api:::matrix_crypto_forget("app:tiny") -seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) -seam_client(e2ee = TRUE, app = "tiny", .crypto = f$ops) +one <- seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) +two <- seam_client(e2ee = TRUE, app = "tiny", .crypto = f$ops) +chat_poll(one) +chat_poll(two) +chat.api:::matrix_crypto_forget( + chat.api:::matrix_crypto_cache_key(NULL, "tiny", fake_mx())) +chat_poll(seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops)) +chat_poll(seam_client(e2ee = TRUE, app = "tiny", .crypto = f$ops)) expect_identical(length(f$log$init), 3L) expect_identical(f$log$init[[3L]]$app, "tiny") @@ -658,6 +708,53 @@ f$ctx$encrypted <- c(f$ctx$encrypted, "!plain:ex") expect_identical(chat_send(enc, "!plain:ex", "now secret"), "$enc1") expect_identical(cleartext$n, 1L) +# ---- An unanswerable encryption state is not a plaintext room ---- +# The lookup used to turn every failure -- expired token, timeout, 500 -- +# into FALSE, and the send went out in the clear. On an e2ee client that +# is a leak, so it aborts instead. +f <- fake_crypto(lookup_error = TRUE) +cleartext <- new.env(parent = emptyenv()); cleartext$n <- 0L +enc <- seam_client(e2ee = TRUE, .crypto = f$ops, + send = function(...) { cleartext$n <- cleartext$n + 1L + "$clear1" }) +expect_error(chat_send(enc, "!unknown:ex", "secret"), + "cannot determine the encryption state") +expect_identical(cleartext$n, 0L) +expect_identical(length(f$log$sent), 0L) +# A cleartext client is untouched: with no crypto context there is no +# lookup to fail, and the send goes out as it always did. +plain <- seam_client(.crypto = f$ops, + send = function(...) { cleartext$n <- cleartext$n + 1L + "$clear1" }) +expect_identical(chat_send(plain, "!unknown:ex", "hi"), "$clear1") +expect_identical(cleartext$n, 1L) + +# ---- Attachments are refused in encrypted rooms ---- +# mx_send_media() posts an ordinary cleartext m.file event. The check used +# to run after the upload loop, so the files reached the homeserver in the +# clear and only the text took the Megolm path -- and an attachment-only +# send never asked at all. +f <- fake_crypto() +uploads <- new.env(parent = emptyenv()); uploads$n <- 0L +enc <- seam_client(e2ee = TRUE, .crypto = f$ops, + media = function(mx, file, ...) { + uploads$n <- uploads$n + 1L + paste0("$m", basename(file)) + }) +expect_error(chat_send(enc, "!enc:ex", "see these", files = "/a.png"), + "cannot send attachments to the encrypted room") +# Nothing was uploaded: the refusal comes before the loop, not after it. +expect_identical(uploads$n, 0L) +expect_identical(length(f$log$sent), 0L) +# Attachment-only sends are refused on the same terms. +expect_error(chat_send(enc, "!enc:ex", "", files = "/a.png"), + "cannot send attachments") +expect_identical(uploads$n, 0L) +# A plaintext room on the same client still takes attachments. +ids <- chat_send(enc, "!plain:ex", "see these", files = c("/a.png", "/b.png")) +expect_identical(ids, c("$ma.png", "$mb.png", "$id")) +expect_identical(uploads$n, 2L) + # markup, kind, and mentions carry into the encrypted branch. Dropping # them would make an encrypted room silently lose formatting and pings # that the same call gets in the clear. @@ -680,20 +777,15 @@ enc$env$mx <- fake_mx(sync_token = "rotated") chat_send(enc, "!enc:ex", "after relogin") expect_identical(f$log$sent[[3L]]$mx$sync_token, "rotated") -# Attachments plus encrypted text: media ids first, encrypted event last, -# same contract the cleartext path reports. +# A failed encrypted send propagates, the same way the cleartext path's +# failures do. Reporting no event id would have a caller record the +# message as sent-but-unacknowledged rather than as failed. f <- fake_crypto() -enc <- seam_client(e2ee = TRUE, .crypto = f$ops, - media = function(mx, file, ...) paste0("$m", basename(file))) -ids <- chat_send(enc, "!enc:ex", "see these", files = c("/a.png", "/b.png")) -expect_identical(ids, c("$ma.png", "$mb.png", "$enc1")) - -# A failed encrypted send reports no event id, the same way the cleartext -# path does, rather than inventing one or falling back to plaintext. -f <- fake_crypto(event_id = NULL) enc <- seam_client(e2ee = TRUE, .crypto = f$ops, send = function(...) stop("must not be reached")) -expect_identical(chat_send(enc, "!enc:ex", "secret"), character(0)) +enc$crypto_ops$send <- function(...) stop("olm session could not be opened") +expect_error(chat_send(enc, "!enc:ex", "secret"), + "olm session could not be opened") # ---- Decryption on poll ---- @@ -707,8 +799,8 @@ dec_rec <- function(event_id, body = "psst", sender = "@alice:ex", } # Decrypted events come out as ordinary chat_message records, folded in -# beside the cleartext ones. corteza used to run its own decrypt off -# $raw and concatenate the results itself; that is what this replaces. +# beside the cleartext ones. corteza used to run its own decrypt off $raw +# and concatenate the results itself. f <- fake_crypto(decrypted = list(dec_rec("$e1", ts = 1700000000000))) enc <- seam_client(recs = list(rec("$c1", body = "in the clear")), e2ee = TRUE, .crypto = f$ops) @@ -730,6 +822,41 @@ expect_true(m$encrypted) expect_true(m$sender_verified) expect_identical(m$kind, "message") +# ---- Mixed timelines keep the homeserver's order ---- +# Appending the decrypted events put every one of them after every +# cleartext one, so an encrypted message followed by a plain reply came +# back the other way round -- which reorders a room's commands against +# the messages they act on. +f <- fake_crypto(decrypted = list(dec_rec("$first", body = "encrypted 1st"))) +enc <- seam_client(recs = list(rec("$second", body = "clear 2nd")), + e2ee = TRUE, .crypto = f$ops, + sync = wrap_sync(list(ev("$first"), ev("$second")))) +res <- chat_poll(enc) +expect_identical(vapply(res$messages, function(m) m$id, character(1)), + c("$first", "$second")) +expect_true(res$messages[[1L]]$encrypted) +expect_false(res$messages[[2L]]$encrypted) + +# Interleaved, three deep, to catch an ordering that only looks right +# when the encrypted event happens to come first. +f <- fake_crypto(decrypted = list(dec_rec("$b"), dec_rec("$d"))) +enc <- seam_client(recs = list(rec("$a"), rec("$c"), rec("$e")), + e2ee = TRUE, .crypto = f$ops, + sync = wrap_sync(list(ev("$a"), ev("$b"), ev("$c"), + ev("$d"), ev("$e")))) +expect_identical(vapply(chat_poll(enc)$messages, function(m) m$id, + character(1)), + c("$a", "$b", "$c", "$d", "$e")) + +# An event the sync did not position keeps its arrival order at the end +# rather than being dropped. +f <- fake_crypto(decrypted = list(dec_rec("$nowhere"))) +enc <- seam_client(recs = list(rec("$a")), e2ee = TRUE, .crypto = f$ops, + sync = wrap_sync(list(ev("$a")))) +expect_identical(vapply(chat_poll(enc)$messages, function(m) m$id, + character(1)), + c("$a", "$nowhere")) + # An unverified sender is FALSE, not NULL: the payload decrypted, but the # claimed sender is the homeserver's word rather than a verified device. f <- fake_crypto(decrypted = list(dec_rec("$e1", sender_verified = FALSE))) @@ -764,15 +891,56 @@ f <- fake_crypto(decrypted = list(dec_rec("$e1", is_self = TRUE))) res <- chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops)) expect_true(res$messages[[1L]]$self) -# A decrypt that throws warns and loses the encrypted traffic, but the -# poll still returns the cleartext messages and advances the cursor. One -# room with a missing Megolm session must not stall the whole loop. +# ---- A sync is not consumed until its crypto state is on disk ---- +# mx_sync_update(save = TRUE) commits the advanced cursor before anything +# parses the response, which is what makes a malformed event survivable. +# But a sync carrying a room key is only consumed once that key is +# persisted: the homeserver never re-sends it, so a crash in between +# leaves every later message in that room undecryptable. + +saves <- new.env(parent = emptyenv()) +saves$order <- character() +saved_client <- function(f, ...) { + seam_client(e2ee = TRUE, .crypto = f$ops, + .save = function(client, app = NULL, ...) { + saves$order <- c(saves$order, "cursor") + client + }, ...) +} + +# The sync does not write the cursor itself... +f <- fake_crypto() +f$ops$decrypt <- function(crypto, sync, mx) { + saves$order <- c(saves$order, "crypto") + list() +} +trace <- new.env(); trace$sync <- list(); trace$extract <- list() +saves$order <- character() +chat_poll(saved_client(f, record = trace)) +expect_false(trace$sync[[1L]]$save) +# ... it is written afterwards, and after the crypto state. +expect_identical(saves$order, c("crypto", "cursor")) + +# A crypto failure means the sync was not consumed: no cursor write, and +# the error is not swallowed. mx.client already skips an individual event +# it has no session for, so a throw here is to-device processing or the +# session save itself. f <- fake_crypto(decrypt_error = TRUE) -enc <- seam_client(recs = list(rec("$c1")), e2ee = TRUE, .crypto = f$ops, - token = "s2") -expect_warning(res <- chat_poll(enc), "decrypt failed") -expect_identical(length(res$messages), 1L) -expect_identical(res$cursor, "s2") +saves$order <- character() +expect_error(chat_poll(saved_client(f)), "no session for that megolm stream") +expect_identical(saves$order, character()) + +# save_cursor = FALSE still means nothing is written, either way. +f <- fake_crypto() +saves$order <- character() +chat_poll(saved_client(f, save_cursor = FALSE)) +expect_identical(saves$order, character()) + +# A cleartext client keeps the cursor inside the sync, where the +# poison-pill protection wants it. Only e2ee pays for durability. +trace2 <- new.env(); trace2$sync <- list(); trace2$extract <- list() +chat_poll(seam_client(record = trace2)) +expect_true(trace2$sync[[1L]]$save) # Nothing consults crypto when e2ee is off, so an installed mx.crypto # cannot change what a cleartext client does. @@ -816,6 +984,86 @@ unlink(store2, recursive = TRUE) ops <- chat.api:::matrix_crypto_ops() expect_identical(sort(names(ops)), c("decrypt", "encrypted", "init", "send")) expect_identical(ops$send, chat.api:::matrix_crypto_send) -partial <- chat.api:::matrix_crypto_ops(list(send = function(...) "$x")) -expect_identical(partial$decrypt, chat.api:::matrix_crypto_decrypt) -expect_false(identical(partial$send, chat.api:::matrix_crypto_send)) + +# ---- The store is bound to the device that owns it ---- +# The directory name is sanitized and therefore not injective: "@a/b:ex" +# and "@a_b:ex" land in the same place. The exact identity is written +# into the store and compared on every open, so a collision -- or a +# copied store, or a changed device_id -- is an error rather than one bot +# silently opening another's Olm account. +store <- tempfile("chatapi-id") +chat.api:::matrix_crypto_bind_identity(store, fake_mx(user_id = "@a:ex", + device_id = "D1")) +expect_true(file.exists(file.path(store, "identity.txt"))) +expect_identical(readLines(file.path(store, "identity.txt")), + c("@a:ex", "D1")) +# Reopening with the same identity is fine, and does not rewrite it. +expect_silent(chat.api:::matrix_crypto_bind_identity( + store, fake_mx(user_id = "@a:ex", device_id = "D1"))) +# A different user, or the same user on a different device, is not. +expect_error(chat.api:::matrix_crypto_bind_identity( + store, fake_mx(user_id = "@b:ex", device_id = "D1")), + "belongs to") +expect_error(chat.api:::matrix_crypto_bind_identity( + store, fake_mx(user_id = "@a:ex", device_id = "D2")), + "belongs to") +# The collision the sanitization allows is caught by the same check. +expect_identical( + chat.api:::matrix_crypto_store(fake_mx(user_id = "@a/b:ex", + device_id = "D"), app = "t"), + chat.api:::matrix_crypto_store(fake_mx(user_id = "@a_b:ex", + device_id = "D"), app = "t")) +shared <- tempfile("chatapi-collide") +chat.api:::matrix_crypto_bind_identity(shared, fake_mx(user_id = "@a/b:ex", + device_id = "D")) +expect_error(chat.api:::matrix_crypto_bind_identity( + shared, fake_mx(user_id = "@a_b:ex", device_id = "D")), "belongs to") +unlink(c(store, shared), recursive = TRUE) + +# Both halves of the identity are required, wherever it is asked for. +expect_error(chat.api:::matrix_crypto_identity(list(user_id = "@a:ex")), + "user_id and device_id") +expect_error(chat.api:::matrix_crypto_identity(list(device_id = "D")), + "user_id and device_id") +expect_error(chat.api:::matrix_crypto_identity( + list(user_id = "@a:ex", device_id = "")), "user_id and device_id") +expect_identical( + chat.api:::matrix_crypto_identity(list(user_id = "@a:ex", + device_id = "D"))[["device_id"]], + "D") + +# ---- The device query covers to-device senders ---- +# sender_bound is stamped once, when a room key arrives over to-device, +# and persisted with the Megolm session; a later sync carrying a timeline +# message from that sender does not rebind it. Room keys normally arrive +# before the message they are for, so asking only about timeline senders +# recorded them unverified forever. +senders <- chat.api:::matrix_encrypted_senders +td <- function(events) list(to_device = list(events = events)) +expect_identical( + senders(td(list(list(type = "m.room.encrypted", sender = "@a:ex")))), + "@a:ex") +# Timeline and to-device are unioned, and each sender appears once. +sync <- c(td(list(list(type = "m.room.encrypted", sender = "@a:ex"), + list(type = "m.room.encrypted", sender = "@b:ex"))), + list(rooms = list(join = list(`!r:ex` = list(timeline = list( + events = list(list(type = "m.room.encrypted", + sender = "@a:ex")))))))) +expect_identical(sort(senders(sync)), c("@a:ex", "@b:ex")) +# Other to-device types are not senders to ask about. +expect_identical( + senders(td(list(list(type = "m.room_key_request", sender = "@a:ex")))), + character()) +expect_identical(senders(list()), character()) + +# ---- Timeline order ---- +pos <- chat.api:::matrix_event_order +expect_identical(pos(wrap_sync(list(ev("$a"), ev("$b"))))[["$b"]], 2L) +expect_identical(length(pos(list())), 0L) +# Positions run across rooms, so the walk is the sync's own order rather +# than a per-room one that would interleave arbitrarily. +two <- list(rooms = list(join = list( + `!r1:ex` = list(timeline = list(events = list(ev("$a")))), + `!r2:ex` = list(timeline = list(events = list(ev("$b"))))))) +expect_identical(pos(two)[["$a"]], 1L) +expect_identical(pos(two)[["$b"]], 2L) diff --git a/inst/tinytest/test_matrix_mxclient.R b/inst/tinytest/test_matrix_mxclient.R index 33f408c..c46a9d7 100644 --- a/inst/tinytest/test_matrix_mxclient.R +++ b/inst/tinytest/test_matrix_mxclient.R @@ -263,3 +263,236 @@ boom <- chat_matrix(mx = list(user_id = "@bot:ex", .send = fake_send, .media = fake_media, .typing = function(...) stop("homeserver down")) expect_false(chat_typing(boom, "!room:ex")) + +# ---- The real crypto boundary functions ---- +# These sit behind chat_matrix()'s .crypto seam, so the adapter-routing +# tests in test_matrix.R never reach them. Their failure modes are the +# two that leak, so they are driven directly here against stubbed +# mx.client / mx.api entry points. + +crypto_ctx <- function(encrypted = character(), store = tempfile("cs")) { + ctx <- new.env(parent = emptyenv()) + ctx$encrypted <- encrypted + ctx$store <- store + ctx$account <- NULL + ctx$sessions <- list() + ctx +} + +stub <- function(name, value, ns) { + orig <- get(name, envir = asNamespace(ns)) + assignInNamespace(name, value, ns = ns) + orig +} + +mx_cfg <- list(user_id = "@bot:ex", device_id = "DEV", + server = "https://ex.invalid", token = "tok") + +# An unanswerable encryption state aborts rather than answering FALSE. +# Turning an expired token, a timeout, or a 500 into "not encrypted" sent +# the message in the clear. +local({ + orig <- stub("mx_room_encrypted", + function(...) stop("Matrix error [M_UNKNOWN_TOKEN]"), + "mx.client") + on.exit(assignInNamespace("mx_room_encrypted", orig, ns = "mx.client")) + expect_error( + chat.api:::matrix_room_is_encrypted(crypto_ctx(), mx_cfg, "!r:ex"), + "cannot determine the encryption state") +}) + +# A room that answers TRUE is remembered, so the next send skips the +# lookup -- which is also what keeps a transient failure from blocking a +# room already known to be encrypted. +local({ + ctx <- crypto_ctx() + orig <- stub("mx_room_encrypted", function(...) TRUE, "mx.client") + on.exit(assignInNamespace("mx_room_encrypted", orig, ns = "mx.client")) + expect_true(chat.api:::matrix_room_is_encrypted(ctx, mx_cfg, "!r:ex")) + expect_true("!r:ex" %in% ctx$encrypted) +}) +local({ + ctx <- crypto_ctx(encrypted = "!r:ex") + orig <- stub("mx_room_encrypted", function(...) stop("down"), "mx.client") + on.exit(assignInNamespace("mx_room_encrypted", orig, ns = "mx.client")) + expect_true(chat.api:::matrix_room_is_encrypted(ctx, mx_cfg, "!r:ex")) +}) + +# A room that answers FALSE is a plaintext room, and is not cached: the +# next send asks again, so a room that turns encryption on is caught. +local({ + asked <- 0L + ctx <- crypto_ctx() + orig <- stub("mx_room_encrypted", + function(...) { asked <<- asked + 1L; FALSE }, "mx.client") + on.exit(assignInNamespace("mx_room_encrypted", orig, ns = "mx.client")) + expect_false(chat.api:::matrix_room_is_encrypted(ctx, mx_cfg, "!r:ex")) + expect_false(chat.api:::matrix_room_is_encrypted(ctx, mx_cfg, "!r:ex")) + expect_identical(asked, 2L) + expect_identical(ctx$encrypted, character()) +}) + +# No crypto context is not an encrypted room, and asks nobody. +local({ + orig <- stub("mx_room_encrypted", function(...) stop("must not be asked"), + "mx.client") + on.exit(assignInNamespace("mx_room_encrypted", orig, ns = "mx.client")) + expect_false(chat.api:::matrix_room_is_encrypted(NULL, mx_cfg, "!r:ex")) +}) + +# Membership discovery is not best-effort. mx_send_encrypted() derives +# the recipient devices from member_ids: an empty list shares the room key +# with nobody, posts the event anyway, and returns an event id, so the +# room cannot read the message while the caller records a successful send. +local({ + sent <- 0L + o1 <- stub("mx_client_session", function(...) list(server = "s"), + "mx.client") + o2 <- stub("mx_room_members", function(...) stop("500 from homeserver"), + "mx.api") + o3 <- stub("mx_send_encrypted", + function(...) { sent <<- sent + 1L + list(event_id = "$e", sessions = list()) }, + "mx.client") + on.exit({ + assignInNamespace("mx_client_session", o1, ns = "mx.client") + assignInNamespace("mx_room_members", o2, ns = "mx.api") + assignInNamespace("mx_send_encrypted", o3, ns = "mx.client") + }) + expect_error(chat.api:::matrix_crypto_send(crypto_ctx(), mx_cfg, "!r:ex", + "secret"), + "500 from homeserver") + expect_identical(sent, 0L) +}) + +# An empty member list is the same refusal, reached without an error to +# propagate. +local({ + sent <- 0L + o1 <- stub("mx_client_session", function(...) list(server = "s"), + "mx.client") + o2 <- stub("mx_room_members", function(...) character(), "mx.api") + o3 <- stub("mx_send_encrypted", + function(...) { sent <<- sent + 1L + list(event_id = "$e", sessions = list()) }, + "mx.client") + on.exit({ + assignInNamespace("mx_client_session", o1, ns = "mx.client") + assignInNamespace("mx_room_members", o2, ns = "mx.api") + assignInNamespace("mx_send_encrypted", o3, ns = "mx.client") + }) + expect_error(chat.api:::matrix_crypto_send(crypto_ctx(), mx_cfg, "!r:ex", + "secret"), + "reach nobody") + expect_identical(sent, 0L) +}) + +# A session that cannot be built is the same: no send. +local({ + sent <- 0L + o1 <- stub("mx_client_session", function(...) stop("no server in config"), + "mx.client") + o3 <- stub("mx_send_encrypted", + function(...) { sent <<- sent + 1L + list(event_id = "$e", sessions = list()) }, + "mx.client") + on.exit({ + assignInNamespace("mx_client_session", o1, ns = "mx.client") + assignInNamespace("mx_send_encrypted", o3, ns = "mx.client") + }) + expect_error(chat.api:::matrix_crypto_send(crypto_ctx(), mx_cfg, "!r:ex", + "secret"), + "no server in config") + expect_identical(sent, 0L) +}) + +# With members, the send goes through and carries them, plus the content +# the cleartext path would have PUT. +local({ + seen <- NULL + o1 <- stub("mx_client_session", function(...) list(server = "s"), + "mx.client") + o2 <- stub("mx_room_members", function(...) c("@a:ex", "@b:ex"), "mx.api") + o3 <- stub("mx_send_encrypted", + function(client, account, sessions, room_id, content, store_dir, + recipients = NULL, member_ids = NULL) { + seen <<- list(content = content, member_ids = member_ids, + room_id = room_id) + list(event_id = "$enc", sessions = "new") + }, "mx.client") + on.exit({ + assignInNamespace("mx_client_session", o1, ns = "mx.client") + assignInNamespace("mx_room_members", o2, ns = "mx.api") + assignInNamespace("mx_send_encrypted", o3, ns = "mx.client") + }) + ctx <- crypto_ctx() + expect_identical(chat.api:::matrix_crypto_send(ctx, mx_cfg, "!r:ex", + "**hi**", + markdown = TRUE), "$enc") + expect_identical(seen$member_ids, c("@a:ex", "@b:ex")) + expect_identical(seen$content$body, "**hi**") + expect_identical(seen$content$format, "org.matrix.custom.html") + expect_true(grepl("hi", seen$content$formatted_body)) + # The updated session set is kept, or the next send re-shares the key. + expect_identical(ctx$sessions, "new") +}) + +# The content an encrypted send wraps is the one mx_send_text() PUTs in +# the clear, so a room's encryption state changes the envelope and +# nothing a reader sees. +content <- chat.api:::matrix_crypto_content +expect_identical(content("hi")$body, "hi") +expect_null(content("hi")$formatted_body) +expect_identical(content("hi", msgtype = "m.notice")$msgtype, "m.notice") +expect_true(grepl("hi", + content("**hi**", markdown = TRUE)$formatted_body)) +m <- content("hi", mentions = "@c:ex") +expect_identical(m[["m.mentions"]]$user_ids, list("@c:ex")) +expect_identical(m$format, "org.matrix.custom.html") + +# matrix_crypto_init() binds the store before it opens the account, so a +# store belonging to another device is refused rather than unpickled. +# Every mx.client entry point below is stubbed: what is under test is the +# order of the adapter's own steps, not the cryptography. +if (requireNamespace("mx.crypto", quietly = TRUE)) { + local({ + opened <- 0L + o1 <- stub("mx_crypto_account", + function(...) { opened <<- opened + 1L; "acct" }, + "mx.client") + o2 <- stub("mx_crypto_publish_keys", function(...) invisible(TRUE), + "mx.client") + o3 <- stub("mx_crypto_sessions_load", function(...) list(), + "mx.client") + o4 <- stub("mx_client_session", function(...) stop("offline"), + "mx.client") + o5 <- stub("mxc_account_identity_keys", + function(...) list(curve25519 = "C", ed25519 = "E"), + "mx.crypto") + on.exit({ + assignInNamespace("mx_crypto_account", o1, ns = "mx.client") + assignInNamespace("mx_crypto_publish_keys", o2, ns = "mx.client") + assignInNamespace("mx_crypto_sessions_load", o3, ns = "mx.client") + assignInNamespace("mx_client_session", o4, ns = "mx.client") + assignInNamespace("mxc_account_identity_keys", o5, ns = "mx.crypto") + }) + + store <- tempfile("initstore") + a <- list(user_id = "@a:ex", device_id = "D1", token = "t", + server = "https://ex.invalid") + ctx <- chat.api:::matrix_crypto_init(a, store = store) + expect_identical(ctx$store, store) + expect_identical(opened, 1L) + expect_identical(readLines(file.path(store, "identity.txt")), + c("@a:ex", "D1")) + + # Another device pointed at the same store is refused, and the + # account is never opened for it. + b <- list(user_id = "@b:ex", device_id = "D2", token = "t", + server = "https://ex.invalid") + expect_error(chat.api:::matrix_crypto_init(b, store = store), + "belongs to") + expect_identical(opened, 1L) + unlink(store, recursive = TRUE) + }) +} diff --git a/man/chat_matrix.Rd b/man/chat_matrix.Rd index 2eb501d..817771e 100644 --- a/man/chat_matrix.Rd +++ b/man/chat_matrix.Rd @@ -16,7 +16,8 @@ chat_matrix( .send = NULL, .media = NULL, .typing = NULL, - .crypto = NULL + .crypto = NULL, + .save = NULL ) } \arguments{ @@ -55,16 +56,26 @@ propagate.} inbound. FALSE (the default) is the previous behaviour exactly: cleartext only, no crypto state, and \code{e2ee} stays FALSE in \code{\link{chat_capabilities}}. Requires the 'mx.crypto' package, -which needs a Rust toolchain.} +which needs a Rust toolchain, and both \code{user_id} and +\code{device_id} on the wrapped config. + +Three things change when it is on. The crypto state is built on the +first poll or send rather than here, so key publication happens after +a relogin rather than before one. The sync cursor is written after +the crypto state instead of inside the sync, so a sync carrying a +room key is not marked consumed until that key is on disk. And +attachments are refused in encrypted rooms, because Matrix media +uploads are not encrypted by this adapter -- +\code{\link{chat_capabilities}} reports \code{files = FALSE} to +match.} \item{crypto_store}{Character or NULL. Directory holding the pickled Olm account and sessions. NULL derives it from \code{app} via -\code{mx.client::mx_crypto_store_dir()}, which keys one store per -app namespace regardless of where the config file lives, so moving a -config does not cost the device identity. Because that key is the app -name alone, an \code{e2ee = TRUE} client built from an explicit -\code{path} with no \code{app} is an error: name one or the other, or -two such clients would quietly share a device identity.} +\code{mx.client::mx_crypto_store_dir()}, with a per-device +subdirectory, so moving a config does not cost the device identity +and two bots under one app namespace do not share an Olm account. +Either way the store records the \code{(user_id, device_id)} it +belongs to and refuses to open for a different one.} \item{.sync}{Testing seam: replacement for \code{mx.client::mx_sync_update}. Leave NULL in production.} @@ -93,6 +104,11 @@ adapter's four crypto operations -- \code{init}, \code{encrypted}, with neither mx.crypto nor a Rust toolchain; E2EE is the one part of this adapter with no honest way to reach a real homeserver from a test.} + +\item{.save}{Testing seam: replacement for +\code{mx.client::mx_client_save}. Leave NULL in production. Only +reached on an \code{e2ee} client, which writes the sync cursor after +the crypto state rather than inside the sync.} } \value{ A \code{chat_client} of class \code{chat_matrix}. From a8136def8f0ba28a1ac225085c55ce52ad519be7 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 14:05:49 -0500 Subject: [PATCH 05/14] Hold the in-memory cursor too, and stop forcing mx.client on every client Two blockers from review, one of which was turning CI red. The advanced cursor was kept off disk on a crypto failure but left live on the client, which only moved the skip. A caller that caught the decrypt error and polled the same client again resumed from the token the failed sync produced, and the room keys in it are gone for good -- the homeserver does not re-send them. Reproduced against 0.0.1.4: after a decrypt error, env$mx$sync_token was the new token, not the old one. The e2ee path now holds the pre-sync cursor on the client until the crypto state is safe, and only then makes the advanced one live. A relogin's refreshed credentials survive the rollback, because a rotated token is not what makes a sync consumed -- dropping it would have the retry authenticate with the one the homeserver just rejected. Cleartext clients are untouched: nothing between their sync and their cursor can fail, and the sync already wrote it. And save_fn was defaulted in the constructor. The other four seams read `.seam %||% mx.client::fn`, and R's lazy `%||%` never forces the mx.client side when a seam is supplied -- which is exactly what makes the documented four-seams-without-mx.client configuration work. A fifth default resolved unconditionally forced mx.client on every client, cleartext ones included, and both CI legs failed with "there is no package called 'mx.client'". It is resolved where it is used now, on the deferred-save path, which only e2ee reaches. Verified the way the claim is made: chat.api installed into a library holding nothing but base, where mx.client is absent. The documented configuration polls and sends, and test_matrix.R runs 236 assertions clean. It could not have before -- that file had also picked up a matrix_crypto_store() assertion that resolves a path through mx.client, which has moved to test_matrix_mxclient.R where the skip is announced. CI now installs mx.api and mx.client so test_matrix_mxclient.R runs rather than skipping. That file holds the direct tests for the two crypto boundary functions that leak if they fail open, and they sit behind the .crypto seam where nothing in test_matrix.R can reach them -- so without this they would have gone back to skipping into a green check the moment the constructor stopped needing mx.client. mx.crypto is Linux-only, from the r2u binary, since building it needs a Rust toolchain; its one test is conditional and reports its absence rather than passing silently. Also corrects the e2ee documentation, which claimed first "poll or send" publishes keys after a relogin. mx_with_relogin() wraps only the sync, so that is true of a first poll and not of a send that precedes one. 403 assertions, 0 fails, both new fixes mutation-checked. R CMD check clean but for the expected new-submission NOTE. --- .github/workflows/ci.yaml | 38 +++++++++ DESCRIPTION | 2 +- R/matrix.R | 58 +++++++++---- inst/tinytest/test_matrix.R | 123 ++++++++++++++++++++++++--- inst/tinytest/test_matrix_mxclient.R | 24 ++++++ man/chat_matrix.Rd | 21 +++-- 6 files changed, 229 insertions(+), 37 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 8bb75b3..610acd7 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -27,5 +27,43 @@ jobs: - name: Dependencies run: ./run.sh install_deps + # test_matrix_mxclient.R exit_file()s without mx.client, and that + # file holds the direct tests for the crypto boundary functions -- + # the encryption-state lookup and the encrypted send, both of which + # leak if they fail open. They sit behind chat_matrix()'s .crypto + # seam, so nothing in test_matrix.R can reach them. install_deps + # takes hard dependencies only, so without this they skip + # themselves into a green check. + - name: Install Matrix stack + run: | + for pkg in mx.api mx.client; do + git clone --depth 1 "https://github.com/cornball-ai/$pkg.git" "$RUNNER_TEMP/$pkg" + R CMD INSTALL "$RUNNER_TEMP/$pkg" + done + + # mx.crypto needs a Rust toolchain to build from source, so it is + # Linux-only here, off the r2u binary rapt already has sources for. + # Its one test -- that matrix_crypto_init() binds the store before + # opening the account -- is conditional and does not run on macOS. + - name: Install mx.crypto (Linux) + if: runner.os == 'Linux' + run: Rscript -e 'install.packages("mx.crypto")' + + # Fail loudly rather than letting the suite skip. mx.crypto is + # reported but not required, since the macOS leg does without it. + - name: Verify Matrix stack + run: | + Rscript -e ' + for (p in c("mx.api", "mx.client")) { + if (!requireNamespace(p, quietly = TRUE)) + stop("chat.api CI needs ", p, ": without it ", + "test_matrix_mxclient.R skips itself", call. = FALSE) + message(p, " ", as.character(utils::packageVersion(p))) + } + message("mx.crypto ", + if (requireNamespace("mx.crypto", quietly = TRUE)) + as.character(utils::packageVersion("mx.crypto")) + else "absent (crypto-init test will not run)")' + - name: Test run: ./run.sh run_tests diff --git a/DESCRIPTION b/DESCRIPTION index 1387137..242f43b 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: chat.api Type: Package Title: Transport-Agnostic Chat Contract for R Agents -Version: 0.0.1.4 +Version: 0.0.1.5 Date: 2026-08-05 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), diff --git a/R/matrix.R b/R/matrix.R index 456409f..4d6c43c 100644 --- a/R/matrix.R +++ b/R/matrix.R @@ -42,15 +42,18 @@ #' which needs a Rust toolchain, and both \code{user_id} and #' \code{device_id} on the wrapped config. #' -#' Three things change when it is on. The crypto state is built on the -#' first poll or send rather than here, so key publication happens after -#' a relogin rather than before one. The sync cursor is written after -#' the crypto state instead of inside the sync, so a sync carrying a -#' room key is not marked consumed until that key is on disk. And -#' attachments are refused in encrypted rooms, because Matrix media -#' uploads are not encrypted by this adapter -- -#' \code{\link{chat_capabilities}} reports \code{files = FALSE} to -#' match. +#' Three things change when it is on. The crypto state is built on +#' first use rather than here: from \code{\link{chat_poll}} that means +#' key publication happens after the sync, so a relogin has already +#' replaced a rejected token, while a \code{\link{chat_send}} that runs +#' before any poll publishes with the token it has -- the same exposure +#' any direct send already carries, since \code{mx_with_relogin()} +#' wraps only the sync. The sync cursor is held back until the crypto +#' state is safe, on disk and on the client, so a sync carrying a room +#' key is not marked consumed until that key is stored. And attachments +#' are refused in encrypted rooms, because Matrix media uploads are not +#' encrypted by this adapter -- \code{\link{chat_capabilities}} reports +#' \code{files = FALSE} to match. #' @param crypto_store Character or NULL. Directory holding the pickled #' Olm account and sessions. NULL derives it from \code{app} via #' \code{mx.client::mx_crypto_store_dir()}, with a per-device @@ -140,7 +143,16 @@ chat_matrix <- function(app = NULL, path = NULL, save_cursor = TRUE, extract_fn = .extract %||% mx.client::mx_extract_text_events, send_fn = .send %||% mx.client::mx_send_text, media_fn = .media %||% mx.client::mx_send_media, - save_fn = .save %||% mx.client::mx_client_save, + # Left unresolved, like typing_fn. The four seams above + # are only defaulted when their seam is NULL, and R's + # lazy `%||%` never forces the mx.client side when one + # is supplied -- which is what makes the documented + # four-seams-without-mx.client configuration work. + # Defaulting this one here forced mx.client on every + # client, cleartext ones included, and broke exactly + # that configuration. It is resolved where it is used, + # on the deferred-save path, which only e2ee reaches. + save_fn = .save, typing_fn = .typing, crypto_ops = matrix_crypto_ops(.crypto)), class = c("chat_matrix", "chat_client")) } @@ -251,7 +263,20 @@ chat_poll.chat_matrix <- function(client, since = NULL, timeout = NULL, ...) { } else { do_sync(client$env$mx) } - client$env$mx <- res$client + # On an e2ee client the advanced cursor is held back until the crypto + # state is safe, in memory as well as on disk. Keeping the cursor off + # disk but live on the client only moves the skip: a caller that + # catches the decrypt error and polls the same client again resumes + # from the token the failed sync produced, and the room keys in it are + # gone for good. The refreshed credentials are kept either way -- a + # relogin is not what makes a sync consumed. + if (isTRUE(client$e2ee)) { + held <- res$client + held$sync_token <- client$env$mx$sync_token + client$env$mx <- held + } else { + client$env$mx <- res$client + } # Crypto runs before the cursor is committed and after the sync has # succeeded: after, so a relogin has already replaced a rejected token @@ -267,10 +292,13 @@ chat_poll.chat_matrix <- function(client, since = NULL, timeout = NULL, ...) { if (isTRUE(client$e2ee)) { crypto <- matrix_crypto_require(client) dec <- client$crypto_ops$decrypt(crypto, res$sync, client$env$mx) - } - if (defer_save) { - client$env$mx <- client$save_fn(client$env$mx, app = client$app) - res$client <- client$env$mx + if (defer_save) { + save_fn <- client$save_fn %||% mx.client::mx_client_save + res$client <- save_fn(res$client, app = client$app) + } + # Nothing above threw, so the sync is consumed and its cursor + # becomes the live one. + client$env$mx <- res$client } event_ts <- matrix_event_times(res$sync) diff --git a/inst/tinytest/test_matrix.R b/inst/tinytest/test_matrix.R index 92bae7b..2e04106 100644 --- a/inst/tinytest/test_matrix.R +++ b/inst/tinytest/test_matrix.R @@ -43,11 +43,17 @@ wrap_sync <- function(events, room = "!room:ex") { # send and media are named parameters rather than `...` pass-throughs so # a test can watch the cleartext path without colliding with the defaults # this helper already supplies. +# +# save is seamed too. An e2ee client defers the cursor write out of the +# sync, and the real writer is mx.client::mx_client_save -- correct in +# production, and the one thing that would make this file need mx.client +# after all. seam_client <- function(recs = list(), sync = wrap_sync(list()), first_run = FALSE, token = "s1", mx = fake_mx(), record = NULL, send = function(...) "$id", - media = function(...) NULL, ...) { - chat_matrix(mx = mx, + media = function(...) NULL, + save = function(client, ...) client, ...) { + chat_matrix(mx = mx, .save = save, .sync = function(client, ...) { if (!is.null(record)) { record$sync[[length(record$sync) + 1L]] <- c( @@ -576,13 +582,26 @@ expect_error(chat_poll(seam_client(e2ee = TRUE, .crypto = fake_crypto(init_error = TRUE)$ops)), "keys could not be published") -# The first poll builds it, off the post-sync config rather than the one -# the client was constructed with -- that is the whole point of waiting. +# The first poll builds it off the post-sync credentials rather than the +# ones the client was constructed with -- that is the whole point of +# waiting, since a relogin during the sync is what replaces a rejected +# token. Asserted on the access token, not the cursor: the cursor is +# deliberately still the pre-sync one at this point, held back until the +# crypto state is safe. f <- fake_crypto() -enc <- seam_client(e2ee = TRUE, .crypto = f$ops, token = "after-relogin") +enc <- chat_matrix(mx = fake_mx(), e2ee = TRUE, .crypto = f$ops, + .sync = function(client, ...) { + c2 <- fake_mx("advanced") + c2$token <- "after-relogin" + list(sync = wrap_sync(list()), client = c2, + first_run = FALSE) + }, + .extract = function(...) list(), + .send = function(...) "$id", .media = function(...) NULL, + .save = function(client, ...) client) chat_poll(enc) expect_identical(length(f$log$init), 1L) -expect_identical(f$log$init[[1L]]$mx$sync_token, "after-relogin") +expect_identical(f$log$init[[1L]]$mx$token, "after-relogin") expect_true(is.environment(enc$env$crypto)) # Later polls reuse it. chat_poll(enc) @@ -902,7 +921,7 @@ saves <- new.env(parent = emptyenv()) saves$order <- character() saved_client <- function(f, ...) { seam_client(e2ee = TRUE, .crypto = f$ops, - .save = function(client, app = NULL, ...) { + save = function(client, app = NULL, ...) { saves$order <- c(saves$order, "cursor") client }, ...) @@ -930,6 +949,53 @@ saves$order <- character() expect_error(chat_poll(saved_client(f)), "no session for that megolm stream") expect_identical(saves$order, character()) +# ... and "not consumed" has to hold in memory too, not just on disk. +# Leaving the advanced cursor on the client only moved the skip: a caller +# that catches the error and polls the same client again would resume +# past the sync whose room keys were lost, and the homeserver never +# re-sends them. +f <- fake_crypto(decrypt_error = TRUE) +retry <- seam_client(e2ee = TRUE, .crypto = f$ops, token = "advanced", + mx = fake_mx(sync_token = "before"), + save = function(client, ...) client) +expect_error(chat_poll(retry), "no session") +expect_identical(retry$env$mx$sync_token, "before") +# The next poll asks from where the failed one started. +trace6 <- new.env(); trace6$sync <- list(); trace6$extract <- list() +retry2 <- seam_client(e2ee = TRUE, .crypto = f$ops, token = "advanced", + mx = fake_mx(sync_token = "before"), record = trace6, + save = function(client, ...) client) +expect_error(chat_poll(retry2), "no session") +expect_error(chat_poll(retry2), "no session") +expect_identical(trace6$sync[[2L]]$client$sync_token, "before") + +# A relogin's refreshed credentials survive the rollback: a rotated token +# is not what makes a sync consumed, and dropping it would have the retry +# authenticate with the one the homeserver just rejected. +f <- fake_crypto(decrypt_error = TRUE) +rl <- chat_matrix(mx = fake_mx(sync_token = "before"), e2ee = TRUE, + .crypto = f$ops, + .sync = function(client, ...) { + c2 <- fake_mx("advanced") + c2$token <- "rotated" + list(sync = wrap_sync(list()), client = c2, + first_run = FALSE) + }, + .extract = function(...) list(), + .send = function(...) "$id", .media = function(...) NULL, + .save = function(client, ...) client) +expect_error(chat_poll(rl), "no session") +expect_identical(rl$env$mx$token, "rotated") +expect_identical(rl$env$mx$sync_token, "before") + +# A successful poll does advance it, or nothing would ever move. +f <- fake_crypto() +ok <- seam_client(e2ee = TRUE, .crypto = f$ops, token = "advanced", + mx = fake_mx(sync_token = "before"), + save = function(client, ...) client) +chat_poll(ok) +expect_identical(ok$env$mx$sync_token, "advanced") + # save_cursor = FALSE still means nothing is written, either way. f <- fake_crypto() saves$order <- character() @@ -1008,11 +1074,9 @@ expect_error(chat.api:::matrix_crypto_bind_identity( store, fake_mx(user_id = "@a:ex", device_id = "D2")), "belongs to") # The collision the sanitization allows is caught by the same check. -expect_identical( - chat.api:::matrix_crypto_store(fake_mx(user_id = "@a/b:ex", - device_id = "D"), app = "t"), - chat.api:::matrix_crypto_store(fake_mx(user_id = "@a_b:ex", - device_id = "D"), app = "t")) +# (That the two identities really do share a directory is asserted in +# test_matrix_mxclient.R: resolving the path calls into mx.client, and +# nothing in this file may.) shared <- tempfile("chatapi-collide") chat.api:::matrix_crypto_bind_identity(shared, fake_mx(user_id = "@a/b:ex", device_id = "D")) @@ -1067,3 +1131,38 @@ two <- list(rooms = list(join = list( `!r2:ex` = list(timeline = list(events = list(ev("$b"))))))) expect_identical(pos(two)[["$a"]], 1L) expect_identical(pos(two)[["$b"]], 2L) + +# ---- The four seams still run without mx.client ---- +# chat_matrix() documents mx plus .sync/.extract/.send/.media as enough +# to poll and send on a host with no mx.client. That works because `%||%` +# is lazy: the mx.client:: default on each of those four is never forced +# when a seam is supplied. Adding a fifth default that was resolved +# unconditionally broke it for every client, cleartext ones included, and +# turned both CI legs red with "there is no package called 'mx.client'". +# +# The proof has to be that no mx.client symbol is referenced eagerly, and +# an installed mx.client makes that invisible here -- so this reads the +# constructor's own body instead of relying on the environment. +ctor <- deparse(body(chat_matrix)) +defaults <- grep("mx\\.client::", ctor, value = TRUE) +# Every remaining mx.client:: reference in the returned list is behind a +# `%||%` whose left side is a seam. +listed <- grep("_fn = ", defaults, value = TRUE) +expect_true(all(grepl("%\\|\\|%", listed))) +expect_false(any(grepl("save_fn = .*mx\\.client::", ctor))) + +# And by construction: a client built the documented way never touches +# save_fn's default, because only the deferred-save path resolves one. +seamed <- chat_matrix(mx = fake_mx(), .sync = function(...) NULL, + .extract = function(...) list(), + .send = function(...) "$id", + .media = function(...) NULL) +expect_null(seamed$save_fn) +expect_false(seamed$e2ee) +# A supplied .save is kept verbatim rather than being wrapped. +mine <- function(client, ...) client +expect_identical(chat_matrix(mx = fake_mx(), .sync = function(...) NULL, + .extract = function(...) list(), + .send = function(...) "$id", + .media = function(...) NULL, + .save = mine)$save_fn, mine) diff --git a/inst/tinytest/test_matrix_mxclient.R b/inst/tinytest/test_matrix_mxclient.R index c46a9d7..abae7b6 100644 --- a/inst/tinytest/test_matrix_mxclient.R +++ b/inst/tinytest/test_matrix_mxclient.R @@ -496,3 +496,27 @@ if (requireNamespace("mx.crypto", quietly = TRUE)) { unlink(store, recursive = TRUE) }) } + +# The store path is per device, and its sanitization is not injective: +# "@a/b:ex" and "@a_b:ex" resolve to the same directory. That is why the +# identity is written into the store and compared on open -- the +# collision is caught there, in test_matrix.R. Resolving the path calls +# mx_crypto_store_dir(), which is why this half lives here. +local({ + a <- list(user_id = "@a/b:ex", device_id = "D") + b <- list(user_id = "@a_b:ex", device_id = "D") + expect_identical(chat.api:::matrix_crypto_store(a, app = "t"), + chat.api:::matrix_crypto_store(b, app = "t")) + # Two devices of one user are two stores, and two users are two too. + one <- list(user_id = "@a:ex", device_id = "D1") + two <- list(user_id = "@a:ex", device_id = "D2") + expect_false(identical(chat.api:::matrix_crypto_store(one, app = "t"), + chat.api:::matrix_crypto_store(two, app = "t"))) + expect_false(identical(chat.api:::matrix_crypto_store(one, app = "t"), + chat.api:::matrix_crypto_store( + list(user_id = "@b:ex", device_id = "D1"), + app = "t"))) + # It sits under mx.client's own store convention for the app. + expect_true(startsWith(chat.api:::matrix_crypto_store(one, app = "t"), + mx.client::mx_crypto_store_dir(app = "t"))) +}) diff --git a/man/chat_matrix.Rd b/man/chat_matrix.Rd index 817771e..be12947 100644 --- a/man/chat_matrix.Rd +++ b/man/chat_matrix.Rd @@ -59,15 +59,18 @@ cleartext only, no crypto state, and \code{e2ee} stays FALSE in which needs a Rust toolchain, and both \code{user_id} and \code{device_id} on the wrapped config. -Three things change when it is on. The crypto state is built on the -first poll or send rather than here, so key publication happens after -a relogin rather than before one. The sync cursor is written after -the crypto state instead of inside the sync, so a sync carrying a -room key is not marked consumed until that key is on disk. And -attachments are refused in encrypted rooms, because Matrix media -uploads are not encrypted by this adapter -- -\code{\link{chat_capabilities}} reports \code{files = FALSE} to -match.} +Three things change when it is on. The crypto state is built on +first use rather than here: from \code{\link{chat_poll}} that means +key publication happens after the sync, so a relogin has already +replaced a rejected token, while a \code{\link{chat_send}} that runs +before any poll publishes with the token it has -- the same exposure +any direct send already carries, since \code{mx_with_relogin()} +wraps only the sync. The sync cursor is held back until the crypto +state is safe, on disk and on the client, so a sync carrying a room +key is not marked consumed until that key is stored. And attachments +are refused in encrypted rooms, because Matrix media uploads are not +encrypted by this adapter -- \code{\link{chat_capabilities}} reports +\code{files = FALSE} to match.} \item{crypto_store}{Character or NULL. Directory holding the pickled Olm account and sessions. NULL derives it from \code{app} via From d8f84f5ecd261390e03273155c598a7a0a018d98 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 14:11:15 -0500 Subject: [PATCH 06/14] CI: install curl and jsonlite before mx.api chat.api has no dependencies of its own, so install_deps brings in nothing and mx.api's Imports were missing. --- .github/workflows/ci.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 610acd7..37f1ad3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -34,8 +34,13 @@ jobs: # seam, so nothing in test_matrix.R can reach them. install_deps # takes hard dependencies only, so without this they skip # themselves into a green check. + # + # curl and jsonlite come first and explicitly: chat.api has no + # dependencies of its own, so install_deps brings in nothing, and + # mx.api needs both. - name: Install Matrix stack run: | + Rscript -e 'install.packages(c("curl", "jsonlite"))' for pkg in mx.api mx.client; do git clone --depth 1 "https://github.com/cornball-ai/$pkg.git" "$RUNNER_TEMP/$pkg" R CMD INSTALL "$RUNNER_TEMP/$pkg" From 5b9f4007c6ff92cef5ddb6968698654c97eb0b85 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 15:08:44 -0500 Subject: [PATCH 07/14] Key the crypto cache on the device identity alone The cache key carried the store as well as (user_id, device_id), and the tests said two stores for one device were two contexts. Matrix gives a device one long-lived ed25519 and one curve25519 key for the life of that device_id, so that was wrong in two ways: two stores minted two Olm accounts, and therefore two identity keys for one device; and two spellings of one directory produced two independent mutable contexts over one set of pickles, each overwriting the other's Megolm sessions. identity.txt already enforced store -> identity. This is the direction that file cannot see, identity -> store. The key is now the identity and nothing else, and the store request is recorded beside the context. A second store for a device that already has one is refused rather than silently ignored -- the caller asked for a store and would otherwise have got a different one without being told -- and rather than silently honoured, which is the bug. Re-homing a device's account is a re-provision, and a re-provisioned device gets a new device_id; matrix_crypto_forget() is the way out for anything that really means to start over. Store requests normalize first, so `/tmp/s` and `/tmp/s/` are one request. Trailing separators are stripped by hand because normalizePath() leaves a path that does not exist yet exactly as given, and a store's first use is exactly when it does not exist. corteza's default path never triggered any of this -- one config, one store -- but chat_matrix() is public and this is its contract. Also corrects two comments that outlived their claims: the initialization comment said the context is built after a successful request, which is true from chat_poll() and not from a chat_send() that precedes any poll (the public docs were already fixed); and corteza's floor rationale explained 0.0.1.3 while requiring 0.0.1.5. 412 assertions, 245 of them on a bare runner with no mx.* installed. Three mutations checked, all caught. --- DESCRIPTION | 2 +- R/matrix-crypto.R | 73 ++++++++++++++++++++------- inst/tinytest/test_matrix.R | 99 ++++++++++++++++++++++++++----------- 3 files changed, 128 insertions(+), 46 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 242f43b..36ff3cd 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: chat.api Type: Package Title: Transport-Agnostic Chat Contract for R Agents -Version: 0.0.1.5 +Version: 0.0.1.6 Date: 2026-08-05 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), diff --git a/R/matrix-crypto.R b/R/matrix-crypto.R index 83237f1..d53e7dc 100644 --- a/R/matrix-crypto.R +++ b/R/matrix-crypto.R @@ -90,26 +90,58 @@ matrix_crypto_bind_identity <- function(store, mx) { # an environment lookup rather than an account load and a 50-key upload. .crypto_cache <- new.env(parent = emptyenv()) -# The cache key: the device identity, plus wherever its store was asked -# for. Deliberately not the resolved directory -- resolving one calls into -# mx.client, and a .crypto seam exists precisely so the e2ee paths run -# without it. +# The cache key is the device identity and nothing else. # -# The identity has to be in the key. Keying on app or store alone had two -# clients built the documented way, chat_matrix(mx = ..., e2ee = TRUE), -# collapse onto one "app:chat.api" entry and share an Olm account. -matrix_crypto_cache_key <- function(store = NULL, app = NULL, mx = NULL) { +# Matrix gives a device one long-lived ed25519 and one curve25519 key, +# for the life of that device_id. So the key cannot include the store: +# with the store in it, two stores for one device_id minted two Olm +# accounts and therefore two identity keys for a device the spec says has +# one -- and two spellings of the same directory produced two mutable +# contexts over one set of pickles, each overwriting the other's Megolm +# sessions. +# +# identity.txt already enforces store -> identity. This is the other +# direction, identity -> store, which that file cannot see. +matrix_crypto_cache_key <- function(mx) { id <- matrix_crypto_identity(mx) - paste(store %||% paste0("app:", app %||% "chat.api"), id[["user_id"]], - id[["device_id"]], sep = "\n") + paste(id[["user_id"]], id[["device_id"]], sep = "\n") } -matrix_crypto_context <- function(key, factory) { - if (!is.null(.crypto_cache[[key]])) { - return(.crypto_cache[[key]]) +# A store request, normalized enough that two spellings of one directory +# are one request. NULL means "derive the default", which is itself a +# stable request: two clients that both defer resolve the same way. +# +# Trailing separators are stripped by hand because normalizePath() leaves +# a path that does not exist yet exactly as given -- and a store's first +# use is precisely when it does not exist. For one that does, it also +# resolves `..` and symlinks, so two routes to one directory collapse. +matrix_crypto_store_spec <- function(store = NULL, app = NULL) { + if (is.null(store)) { + return(paste0("app:", app %||% "chat.api")) + } + p <- path.expand(store) + trimmed <- sub("(.)/+$", "\\1", p) + normalizePath(trimmed, mustWork = FALSE) +} + +# One identity, one context, one store. A second store for a device +# already holding one is refused rather than silently ignored or silently +# honoured: re-homing a device's Olm account is a re-provision, and a +# re-provisioned device gets a new device_id. matrix_crypto_forget() is +# the way out for anything that really means to start over. +matrix_crypto_context <- function(key, spec, factory) { + prev <- .crypto_cache[[key]] + if (!is.null(prev)) { + if (!identical(prev$spec, spec)) { + stop("chat.api: this Matrix device already has a crypto store ", + "at ", prev$spec, "; refusing a second at ", spec, + ". A device has one Olm identity, so two stores would be ", + "two identity keys for one device_id.", call. = FALSE) + } + return(prev$ctx) } ctx <- factory() - .crypto_cache[[key]] <- ctx + .crypto_cache[[key]] <- list(ctx = ctx, spec = spec) ctx } @@ -120,8 +152,14 @@ matrix_crypto_context <- function(key, factory) { # rejected at process start -- that is the case mx_with_relogin() exists # for. Building at construction put the upload before any relogin could # happen, so chat_matrix() threw, corteza's poll loop never got to run, -# and every restart repeated with the same dead token. Now the context is -# built after a request has succeeded, off the config that succeeded. +# and every restart repeated with the same dead token. +# +# What waiting buys depends on which call gets here first. From +# chat_poll() the sync has already run inside mx_with_relogin(), so the +# keys go up on a token that just worked. From a chat_send() that +# precedes any poll they go up on whatever token the config holds -- +# nothing wraps a send, so that is the same exposure any direct send +# already has, and it is still strictly later than the constructor. # # The result is stashed on the client's env, which is shared with every # copy of the client, so this costs one lookup after the first call. @@ -134,7 +172,8 @@ matrix_crypto_require <- function(client) { } mx <- client$env$mx ctx <- matrix_crypto_context( - matrix_crypto_cache_key(client$crypto_store, client$app, mx), + matrix_crypto_cache_key(mx), + matrix_crypto_store_spec(client$crypto_store, client$app), function() { client$crypto_ops$init(mx, store = client$crypto_store, app = client$app) diff --git a/inst/tinytest/test_matrix.R b/inst/tinytest/test_matrix.R index 2e04106..491f1a8 100644 --- a/inst/tinytest/test_matrix.R +++ b/inst/tinytest/test_matrix.R @@ -634,18 +634,39 @@ expect_error(chat_matrix(mx = list(device_id = "DEV1"), e2ee = TRUE, # e2ee off does not need one: a cleartext client has no account to key. expect_silent(seam_client(mx = list(user_id = "@bot:ex"))) -# The cache key carries the identity. Keying on app or store alone had two -# clients built the documented way -- chat_matrix(mx = ..., e2ee = TRUE) -# with no app and no store -- collapse onto one entry and share an account. +# The cache key is the device identity and nothing else. Matrix gives a +# device one ed25519 and one curve25519 key for the life of that +# device_id, so a key that also carried the store let two stores mint two +# Olm accounts -- two identity keys for one device. k <- chat.api:::matrix_crypto_cache_key -expect_false(identical(k(NULL, NULL, fake_mx(user_id = "@a:ex")), - k(NULL, NULL, fake_mx(user_id = "@b:ex")))) -expect_false(identical(k(NULL, NULL, fake_mx(device_id = "D1")), - k(NULL, NULL, fake_mx(device_id = "D2")))) -expect_false(identical(k("/s", NULL, fake_mx(device_id = "D1")), - k("/s", NULL, fake_mx(device_id = "D2")))) -expect_identical(k(NULL, NULL, fake_mx()), k(NULL, NULL, fake_mx())) -expect_false(identical(k(NULL, "a", fake_mx()), k(NULL, "b", fake_mx()))) +expect_false(identical(k(fake_mx(user_id = "@a:ex")), + k(fake_mx(user_id = "@b:ex")))) +expect_false(identical(k(fake_mx(device_id = "D1")), + k(fake_mx(device_id = "D2")))) +expect_identical(k(fake_mx()), k(fake_mx())) +# Same device, different token or cursor: still one key. +expect_identical(k(fake_mx(sync_token = "s1")), k(fake_mx())) + +# Store requests normalize, so two spellings of one directory are one +# request rather than two contexts writing over each other's pickles. +spec <- chat.api:::matrix_crypto_store_spec +expect_identical(spec("/tmp/store-a"), spec("/tmp/store-a/")) +expect_identical(spec("/tmp/store-a"), spec("/tmp/store-a///")) +expect_false(identical(spec("/tmp/store-a"), spec("/tmp/store-b"))) +# Root survives the trailing-slash strip rather than becoming "". +expect_true(nzchar(spec("/"))) +# For a directory that exists, `..` and symlinks collapse too. +local({ + d <- file.path(tempfile("specdir"), "s") + dir.create(d, recursive = TRUE) + on.exit(unlink(dirname(d), recursive = TRUE)) + expect_identical(spec(d), spec(file.path(dirname(d), "..", + basename(dirname(d)), "s"))) +}) +# A deferred store is a stable request too: two clients that both defer +# resolve the same way, and the app is what distinguishes them. +expect_identical(spec(NULL), spec(NULL)) +expect_false(identical(spec(NULL, "a"), spec(NULL, "b"))) # Two default clients for different bots get two contexts. f <- fake_crypto() @@ -669,31 +690,53 @@ chat_poll(b) expect_identical(length(f$log$init), 1L) expect_true(identical(a$env$crypto, b$env$crypto)) -# An explicit crypto_store keys on the store plus the identity, so two -# stores are two contexts and one store is one. +# One explicit store, asked for twice, is one context -- including when +# the second spelling of it differs. f <- fake_crypto() chat_send(seam_client(e2ee = TRUE, crypto_store = "/tmp/store-a", .crypto = f$ops), "!enc:ex", "x") -chat_send(seam_client(e2ee = TRUE, crypto_store = "/tmp/store-a", +chat_send(seam_client(e2ee = TRUE, crypto_store = "/tmp/store-a/", .crypto = f$ops), "!enc:ex", "x") -chat_send(seam_client(e2ee = TRUE, crypto_store = "/tmp/store-b", - .crypto = f$ops), "!enc:ex", "x") -expect_identical(length(f$log$init), 2L) -expect_identical(f$log$init[[1L]]$store, "/tmp/store-a") -expect_identical(f$log$init[[2L]]$store, "/tmp/store-b") +expect_identical(length(f$log$init), 1L) + +# A second store for a device that already has one is refused. Two stores +# would be two Olm accounts, and the spec gives a device_id one identity. +# It is not silently ignored either: the caller asked for a store and +# would otherwise have got a different one without being told. +expect_error(chat_send(seam_client(e2ee = TRUE, crypto_store = "/tmp/store-b", + .crypto = f$ops), "!enc:ex", "x"), + "already has a crypto store") +expect_identical(length(f$log$init), 1L) +# The default store and an explicit one conflict the same way. +expect_error(chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops)), + "already has a crypto store") -# Forgetting one identity leaves the others interned. +# A different app is a different store, so on the same device it +# conflicts too. In production it does not arise: cornelius and tiny are +# different user_ids, hence different devices. f <- fake_crypto() -one <- seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops) -two <- seam_client(e2ee = TRUE, app = "tiny", .crypto = f$ops) -chat_poll(one) -chat_poll(two) -chat.api:::matrix_crypto_forget( - chat.api:::matrix_crypto_cache_key(NULL, "tiny", fake_mx())) chat_poll(seam_client(e2ee = TRUE, app = "cornelius", .crypto = f$ops)) -chat_poll(seam_client(e2ee = TRUE, app = "tiny", .crypto = f$ops)) +expect_error(chat_poll(seam_client(e2ee = TRUE, app = "tiny", + .crypto = f$ops)), + "already has a crypto store") + +# Forgetting an identity is how to re-home one deliberately, and it +# leaves the other identities interned. +f <- fake_crypto() +one <- fake_mx(user_id = "@one:ex") +two <- fake_mx(user_id = "@two:ex") +chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops, mx = one)) +chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops, mx = two)) +chat.api:::matrix_crypto_forget(chat.api:::matrix_crypto_cache_key(two)) +chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops, mx = one)) +chat_poll(seam_client(e2ee = TRUE, .crypto = f$ops, mx = two)) expect_identical(length(f$log$init), 3L) -expect_identical(f$log$init[[3L]]$app, "tiny") +expect_identical(f$log$init[[3L]]$mx$user_id, "@two:ex") +# ... and after forgetting, a new store for that device is allowed. +chat.api:::matrix_crypto_forget(chat.api:::matrix_crypto_cache_key(one)) +chat_send(seam_client(e2ee = TRUE, crypto_store = "/tmp/rehomed", + .crypto = f$ops, mx = one), "!enc:ex", "x") +expect_identical(f$log$init[[length(f$log$init)]]$store, "/tmp/rehomed") # ---- Encrypted sends ---- From 924efa68d08bcfc6c313d29844e5c4f62ab542c7 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 15:30:43 -0500 Subject: [PATCH 08/14] Check the account against the keys the homeserver already published The one-store-per-device rule lived only in .crypto_cache, which meant it lived only for the length of a process. Restart, or call matrix_crypto_forget(), and a changed crypto_store would mint a fresh Olm account and go on to publish different long-lived keys under the old device_id. "Re-homing needs a new device_id" was a comment, not a check. So init now asks the homeserver what keys it already holds for this device and refuses if they are not this account's. That record outlives the process, which is the property the cache could never have. A device the server has never seen is the first run and proceeds. A query that cannot be answered is an error: init is about to publish keys to that same homeserver, so being unable to ask it anything is not a state to publish from, and the caller retries on the next poll. The check runs before the upload, so a mismatch never reaches it. Three things now hold the invariant, at three lifetimes: identity.txt binds a store to a device, the cache binds a device to one store within a process, and this binds a device to its keys for as long as the homeserver remembers them. Path normalization is lexical now, not normalizePath(). That resolves symlinks, which this does not, but it returns a path that does not exist yet exactly as given -- and a store's first use is exactly when it does not exist. Two calls for one directory would disagree depending on whether it had been created between them, which is worse than not following symlinks: the comparison has to be stable over the store's whole life, not accurate at one moment of it. So `/missing/a/../b` and `/missing/b` are one spec, `/missing/./b` too, and `C:/` keeps its slash -- the trailing-separator strip this replaces ate the slash off a Windows drive root and left `C:`. 431 assertions, 256 of them on a bare runner. Four mutations, all caught. --- DESCRIPTION | 2 +- R/matrix-crypto.R | 105 +++++++++++++++++++--- R/matrix.R | 13 ++- inst/tinytest/test_matrix.R | 33 ++++++- inst/tinytest/test_matrix_mxclient.R | 126 +++++++++++++++++++++++++++ man/chat_matrix.Rd | 13 ++- 6 files changed, 273 insertions(+), 19 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 36ff3cd..50c5135 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: chat.api Type: Package Title: Transport-Agnostic Chat Contract for R Agents -Version: 0.0.1.6 +Version: 0.0.1.7 Date: 2026-08-05 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), diff --git a/R/matrix-crypto.R b/R/matrix-crypto.R index d53e7dc..e4b3e5a 100644 --- a/R/matrix-crypto.R +++ b/R/matrix-crypto.R @@ -107,21 +107,58 @@ matrix_crypto_cache_key <- function(mx) { paste(id[["user_id"]], id[["device_id"]], sep = "\n") } -# A store request, normalized enough that two spellings of one directory -# are one request. NULL means "derive the default", which is itself a -# stable request: two clients that both defer resolve the same way. -# -# Trailing separators are stripped by hand because normalizePath() leaves -# a path that does not exist yet exactly as given -- and a store's first -# use is precisely when it does not exist. For one that does, it also -# resolves `..` and symlinks, so two routes to one directory collapse. +# A store request, normalized so two spellings of one directory are one +# request. NULL means "derive the default", which is itself a stable +# request: two clients that both defer resolve the same way. matrix_crypto_store_spec <- function(store = NULL, app = NULL) { if (is.null(store)) { return(paste0("app:", app %||% "chat.api")) } - p <- path.expand(store) - trimmed <- sub("(.)/+$", "\\1", p) - normalizePath(trimmed, mustWork = FALSE) + matrix_normalize_path(store) +} + +# Lexical path normalization: expand ~, make it absolute, fold separators, +# and resolve . and .. by hand. +# +# Deliberately not normalizePath(). That resolves symlinks, which this +# does not, but it returns a path that does not exist yet exactly as +# given -- and a store's first use is exactly when it does not exist. Two +# calls for one directory would then disagree depending on whether it had +# been created between them, which is worse than not resolving symlinks +# at all: the comparison this feeds has to be stable over the store's +# whole life, not accurate at one moment of it. +# +# The trailing-separator strip this replaces also ate the slash off a +# Windows drive root, turning "C:/" into "C:". +matrix_normalize_path <- function(p) { + p <- gsub("\\\\", "/", path.expand(p)) + root <- "" + if (grepl("^[A-Za-z]:/", p)) { + root <- substr(p, 1L, 3L) + p <- substring(p, 4L) + } else if (startsWith(p, "/")) { + root <- "/" + p <- substring(p, 2L) + } else { + # Relative to wherever the caller stood. Two callers in different + # directories mean two stores, and saying so beats merging them. + return(matrix_normalize_path(file.path(gsub("\\\\", "/", getwd()), p))) + } + out <- character() + for (part in strsplit(p, "/")[[1L]]) { + if (!nzchar(part) || identical(part, ".")) { + next + } + if (identical(part, "..")) { + if (length(out)) { + out <- out[-length(out)] + } + # Above the root there is nothing to go up to; drop it. + next + } + out <- c(out, part) + } + paste0(root, paste(out, collapse = "/")) } # One identity, one context, one store. A second store for a device @@ -209,6 +246,51 @@ matrix_crypto_ops <- function(override = NULL) { ops } +# Does this account hold the keys the homeserver already has for this +# device? Matrix gives a device_id one ed25519 and one curve25519 for its +# whole life, and the homeserver is the authority on which. +# +# The in-process cache cannot answer this. It stops two contexts existing +# at once, and nothing more: restart the process, or call +# matrix_crypto_forget(), and a changed crypto_store mints a fresh +# account that would go on to publish different long-lived keys under the +# old device_id. So "re-homing needs a new device_id" was commentary. This +# is the enforcement, and it is durable because the record it checks +# against lives on the server rather than in this session. +# +# A device the server has never seen is the first-run case and proceeds. +# A query that cannot be answered is an error: init is about to publish +# keys to that same homeserver, so it is not a host we can be off-line +# from, and the caller retries on the next poll. +matrix_crypto_check_published <- function(mx, acct) { + mine <- mx.crypto::mxc_account_identity_keys(acct) + devs <- tryCatch(mx.client::mx_crypto_known_devices(mx, mx$user_id), + error = function(e) { + stop("chat.api: cannot read this device's published keys from the ", + "homeserver (", conditionMessage(e), "). Refusing to publish ", + "an Olm identity that may conflict with one already there.", + call. = FALSE) + }) + for (d in devs) { + if (!identical(d$device_id, mx$device_id)) { + next + } + if (identical(d$ed25519, mine$ed25519) && + identical(d$curve25519, mine$curve25519)) { + return(invisible(TRUE)) + } + stop("chat.api: the homeserver already has different device keys ", + "for ", mx$user_id, "/", mx$device_id, " (ed25519 ", + substr(d$ed25519 %||% "", 1L, 12L), "..., this store has ", + substr(mine$ed25519 %||% "", 1L, 12L), + "...). This store holds another device's Olm account, or the ", + "one this device used has been lost. A device has one identity ", + "for its lifetime, so log in again for a new device_id rather ", + "than republishing under this one.", call. = FALSE) + } + invisible(TRUE) +} + # Load or create the account, publish keys, restore sessions and the # known-encrypted-room set. Returns an environment: the poll loop mutates # sessions in place, and callers hold the same object across polls. @@ -225,6 +307,7 @@ matrix_crypto_init <- function(mx, store = NULL, app = NULL) { store <- store %||% matrix_crypto_store(mx, app = app) matrix_crypto_bind_identity(store, mx) acct <- mx.client::mx_crypto_account(store) + matrix_crypto_check_published(mx, acct) mx.client::mx_crypto_publish_keys(mx, acct, store, n_otks = 50L) crypto <- new.env(parent = emptyenv()) crypto$account <- acct diff --git a/R/matrix.R b/R/matrix.R index 4d6c43c..46964ca 100644 --- a/R/matrix.R +++ b/R/matrix.R @@ -59,8 +59,17 @@ #' \code{mx.client::mx_crypto_store_dir()}, with a per-device #' subdirectory, so moving a config does not cost the device identity #' and two bots under one app namespace do not share an Olm account. -#' Either way the store records the \code{(user_id, device_id)} it -#' belongs to and refuses to open for a different one. +#' +#' A Matrix device has one Olm identity for its whole life, and three +#' things hold that. The store records the \code{(user_id, device_id)} +#' it belongs to and refuses to open for a different one. Within a +#' process, one device gets one context and one store, so pointing a +#' second store at it is an error rather than a second account. And on +#' first use the account's keys are checked against the ones the +#' homeserver already has for that \code{device_id} -- which is the +#' only one of the three that survives a restart, and so the one that +#' catches a store swapped between runs. Log in again for a new +#' \code{device_id} rather than re-homing an existing one. #' @param .sync Testing seam: replacement for #' \code{mx.client::mx_sync_update}. Leave NULL in production. #' @param .extract Testing seam: replacement for diff --git a/inst/tinytest/test_matrix.R b/inst/tinytest/test_matrix.R index 491f1a8..0396d84 100644 --- a/inst/tinytest/test_matrix.R +++ b/inst/tinytest/test_matrix.R @@ -653,15 +653,42 @@ spec <- chat.api:::matrix_crypto_store_spec expect_identical(spec("/tmp/store-a"), spec("/tmp/store-a/")) expect_identical(spec("/tmp/store-a"), spec("/tmp/store-a///")) expect_false(identical(spec("/tmp/store-a"), spec("/tmp/store-b"))) -# Root survives the trailing-slash strip rather than becoming "". -expect_true(nzchar(spec("/"))) -# For a directory that exists, `..` and symlinks collapse too. + +# Dot segments fold whether or not the directory exists. normalizePath() +# only folds them for a path that does, and a store's first use is +# exactly when it does not -- so two calls for one directory would have +# disagreed depending on whether it had been created in between. +n <- chat.api:::matrix_normalize_path +expect_identical(n("/missing/a/../b"), n("/missing/b")) +expect_identical(n("/missing/./b"), n("/missing/b")) +expect_identical(n("/missing/a/b/../.."), n("/missing")) +# Roots survive, including a Windows drive root -- the trailing-slash +# strip this replaced turned "C:/" into "C:". +expect_identical(n("/"), "/") +expect_identical(n("C:/"), "C:/") +expect_identical(n("C:/x/../y"), "C:/y") +expect_identical(n("C:\\x\\y"), "C:/x/y") +# There is nothing above a root to climb to. +expect_identical(n("/../x"), "/x") +# ~ expands, and a relative path resolves against the caller's directory +# rather than merging with an unrelated one of the same name. +expect_true(startsWith(n("~/x"), "/")) +expect_identical(n("rel/x"), n(file.path(getwd(), "rel/x"))) +expect_true(startsWith(n("rel/x"), "/")) +# Directories that do exist still fold, which is the common case. local({ d <- file.path(tempfile("specdir"), "s") dir.create(d, recursive = TRUE) on.exit(unlink(dirname(d), recursive = TRUE)) expect_identical(spec(d), spec(file.path(dirname(d), "..", basename(dirname(d)), "s"))) + # ... and the same path gives the same spec before and after it + # exists, which is the property the whole comparison rests on. + gone <- file.path(tempfile("specgone"), "s") + before <- spec(gone) + dir.create(gone, recursive = TRUE) + on.exit(unlink(dirname(gone), recursive = TRUE), add = TRUE) + expect_identical(before, spec(gone)) }) # A deferred store is a stable request too: two clients that both defer # resolve the same way, and the app is what distinguishes them. diff --git a/inst/tinytest/test_matrix_mxclient.R b/inst/tinytest/test_matrix_mxclient.R index abae7b6..523f8d6 100644 --- a/inst/tinytest/test_matrix_mxclient.R +++ b/inst/tinytest/test_matrix_mxclient.R @@ -469,12 +469,17 @@ if (requireNamespace("mx.crypto", quietly = TRUE)) { o5 <- stub("mxc_account_identity_keys", function(...) list(curve25519 = "C", ed25519 = "E"), "mx.crypto") + # A homeserver that has never seen this device. The published-key + # check is exercised on its own further down. + o6 <- stub("mx_crypto_known_devices", function(...) list(), + "mx.client") on.exit({ assignInNamespace("mx_crypto_account", o1, ns = "mx.client") assignInNamespace("mx_crypto_publish_keys", o2, ns = "mx.client") assignInNamespace("mx_crypto_sessions_load", o3, ns = "mx.client") assignInNamespace("mx_client_session", o4, ns = "mx.client") assignInNamespace("mxc_account_identity_keys", o5, ns = "mx.crypto") + assignInNamespace("mx_crypto_known_devices", o6, ns = "mx.client") }) store <- tempfile("initstore") @@ -520,3 +525,124 @@ local({ expect_true(startsWith(chat.api:::matrix_crypto_store(one, app = "t"), mx.client::mx_crypto_store_dir(app = "t"))) }) + +# ---- One identity per device, durably ---- +# The in-process cache stops two contexts existing at once and nothing +# more: restart, or matrix_crypto_forget(), and a changed crypto_store +# would mint a fresh account and publish different long-lived keys under +# the old device_id. The homeserver is the authority on which keys a +# device has, so that is what init checks against. +if (requireNamespace("mx.crypto", quietly = TRUE)) { + init_stubs <- function(known, ...) { + o <- list( + mx_crypto_account = stub("mx_crypto_account", function(...) "acct", + "mx.client"), + mx_crypto_publish_keys = stub("mx_crypto_publish_keys", + function(...) invisible(TRUE), + "mx.client"), + mx_crypto_sessions_load = stub("mx_crypto_sessions_load", + function(...) list(), "mx.client"), + mx_client_session = stub("mx_client_session", + function(...) stop("offline"), + "mx.client"), + mx_crypto_known_devices = stub("mx_crypto_known_devices", known, + "mx.client"), + mxc_account_identity_keys = stub( + "mxc_account_identity_keys", + function(...) list(curve25519 = "CURVE-ours", + ed25519 = "ED-ours"), "mx.crypto")) + o + } + restore_stubs <- function(o) { + for (nm in names(o)) { + ns <- if (nm == "mxc_account_identity_keys") "mx.crypto" else + "mx.client" + assignInNamespace(nm, o[[nm]], ns = ns) + } + } + me <- list(user_id = "@a:ex", device_id = "D1", token = "t", + server = "https://ex.invalid") + + # A device the homeserver has never seen is the first run. + local({ + published <- NULL + o <- init_stubs(function(client, user_ids) { + published <<- user_ids + list() + }) + on.exit(restore_stubs(o)) + store <- tempfile("pub1") + expect_true(is.environment(chat.api:::matrix_crypto_init(me, + store = store))) + # It asks about its own user, not the room's members. + expect_identical(published, "@a:ex") + unlink(store, recursive = TRUE) + }) + + # Keys that match are this device's own account: proceed. + local({ + o <- init_stubs(function(...) list( + list(user_id = "@a:ex", device_id = "D1", + curve25519 = "CURVE-ours", ed25519 = "ED-ours"))) + on.exit(restore_stubs(o)) + store <- tempfile("pub2") + expect_true(is.environment(chat.api:::matrix_crypto_init(me, + store = store))) + unlink(store, recursive = TRUE) + }) + + # Keys that differ mean this store is not this device's. This is the + # case a restart plus a changed crypto_store produced, which nothing + # in the process cache could have seen. + local({ + o <- init_stubs(function(...) list( + list(user_id = "@a:ex", device_id = "D1", + curve25519 = "CURVE-theirs", ed25519 = "ED-theirs"))) + on.exit(restore_stubs(o)) + store <- tempfile("pub3") + expect_error(chat.api:::matrix_crypto_init(me, store = store), + "already has different device keys") + unlink(store, recursive = TRUE) + }) + + # Another device of the same user is not this device, and does not + # collide with it. + local({ + o <- init_stubs(function(...) list( + list(user_id = "@a:ex", device_id = "D2", + curve25519 = "CURVE-other", ed25519 = "ED-other"))) + on.exit(restore_stubs(o)) + store <- tempfile("pub4") + expect_true(is.environment(chat.api:::matrix_crypto_init(me, + store = store))) + unlink(store, recursive = TRUE) + }) + + # A query that cannot be answered is an error, not a shrug. init is + # about to publish keys to that same homeserver, so being unable to + # ask it anything is not a state to publish from. + local({ + o <- init_stubs(function(...) stop("504 from homeserver")) + on.exit(restore_stubs(o)) + store <- tempfile("pub5") + expect_error(chat.api:::matrix_crypto_init(me, store = store), + "cannot read this device's published keys") + unlink(store, recursive = TRUE) + }) + + # The check runs before the upload, so a mismatch never publishes. + local({ + uploads <- 0L + o <- init_stubs(function(...) list( + list(user_id = "@a:ex", device_id = "D1", + curve25519 = "CURVE-theirs", ed25519 = "ED-theirs"))) + assignInNamespace("mx_crypto_publish_keys", + function(...) { uploads <<- uploads + 1L + invisible(TRUE) }, ns = "mx.client") + on.exit(restore_stubs(o)) + store <- tempfile("pub6") + expect_error(chat.api:::matrix_crypto_init(me, store = store)) + expect_identical(uploads, 0L) + unlink(store, recursive = TRUE) + }) +} diff --git a/man/chat_matrix.Rd b/man/chat_matrix.Rd index be12947..c3c73ec 100644 --- a/man/chat_matrix.Rd +++ b/man/chat_matrix.Rd @@ -77,8 +77,17 @@ Olm account and sessions. NULL derives it from \code{app} via \code{mx.client::mx_crypto_store_dir()}, with a per-device subdirectory, so moving a config does not cost the device identity and two bots under one app namespace do not share an Olm account. -Either way the store records the \code{(user_id, device_id)} it -belongs to and refuses to open for a different one.} + +A Matrix device has one Olm identity for its whole life, and three +things hold that. The store records the \code{(user_id, device_id)} +it belongs to and refuses to open for a different one. Within a +process, one device gets one context and one store, so pointing a +second store at it is an error rather than a second account. And on +first use the account's keys are checked against the ones the +homeserver already has for that \code{device_id} -- which is the +only one of the three that survives a restart, and so the one that +catches a store swapped between runs. Log in again for a new +\code{device_id} rather than re-homing an existing one.} \item{.sync}{Testing seam: replacement for \code{mx.client::mx_sync_update}. Leave NULL in production.} From 0c3ddc46ef4530cb9ba49ac2aabb72d7a9ce8af5 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 15:55:36 -0500 Subject: [PATCH 09/14] Tell an unverifiable device record from an absent one matrix_crypto_check_published() went through mx_crypto_known_devices(), which verifies signatures and drops what fails with a warning. That is right for choosing who to encrypt to and wrong for asking what this device already published: a tampered or signature-stripped entry for our own device came back as no entry, collapsed "present but unverifiable" into "absent", and got published over. The same failure-read-as-absence this whole review keeps finding, one layer down. It reads the raw /keys/query response now, with three outcomes that stay three: absent is a first run, present and valid is compared, present and unverifiable is an error. The tests stub the query rather than the already-filtered helper, so the third case is reachable at all -- it was not before. Still out of reach: a homeserver that omits the device on purpose looks exactly like a first run over this channel. Closing that needs key pinning or cross-signing, noted in the code and the docs rather than implied away. Path roots are no longer all "/". Collapsing every leading slash run made //server/share/x, \\server\share\x and /server/share/x one spec, and on Windows the first two are a UNC share while the third is a local path -- so a request naming one store would have been handed another's context, which is the collision the check exists to stop. Three or more leading slashes are POSIX, not UNC, and still collapse. "C:relative" keeps its drive prefix instead of being resolved against getwd(): Windows resolves it against the current directory on that drive, which R cannot reconstruct portably, and a spec that cannot be resolved is better left unmerged than merged wrong. 444 assertions, 266 of them on a bare runner. Three mutations, all caught. --- DESCRIPTION | 2 +- R/matrix-crypto.R | 113 ++++++++++++++++++------- R/matrix.R | 15 +++- inst/tinytest/test_matrix.R | 24 ++++++ inst/tinytest/test_matrix_mxclient.R | 120 +++++++++++++++++++-------- man/chat_matrix.Rd | 15 +++- 6 files changed, 213 insertions(+), 76 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 50c5135..1f6f482 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: chat.api Type: Package Title: Transport-Agnostic Chat Contract for R Agents -Version: 0.0.1.7 +Version: 0.0.1.8 Date: 2026-08-05 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), diff --git a/R/matrix-crypto.R b/R/matrix-crypto.R index e4b3e5a..6e9f8bb 100644 --- a/R/matrix-crypto.R +++ b/R/matrix-crypto.R @@ -132,20 +132,14 @@ matrix_crypto_store_spec <- function(store = NULL, app = NULL) { # Windows drive root, turning "C:/" into "C:". matrix_normalize_path <- function(p) { p <- gsub("\\\\", "/", path.expand(p)) - root <- "" - if (grepl("^[A-Za-z]:/", p)) { - root <- substr(p, 1L, 3L) - p <- substring(p, 4L) - } else if (startsWith(p, "/")) { - root <- "/" - p <- substring(p, 2L) - } else { + r <- matrix_path_root(p) + if (!nzchar(r[["root"]])) { # Relative to wherever the caller stood. Two callers in different # directories mean two stores, and saying so beats merging them. return(matrix_normalize_path(file.path(gsub("\\\\", "/", getwd()), p))) } out <- character() - for (part in strsplit(p, "/")[[1L]]) { + for (part in strsplit(r[["tail"]], "/")[[1L]]) { if (!nzchar(part) || identical(part, ".")) { next } @@ -158,7 +152,38 @@ matrix_normalize_path <- function(p) { } out <- c(out, part) } - paste0(root, paste(out, collapse = "/")) + paste0(r[["root"]], paste(out, collapse = "/")) +} + +# Split a path into the root it is anchored to and the rest. +# +# The root is not always "/". Collapsing every leading slash run into one +# made //server/share/x, \\server\share\x and /server/share/x the same +# spec, and on Windows the first two are a UNC share while the third is a +# local path -- so a request naming one store would have been handed the +# context of another, which is the collision the whole check exists to +# stop. Three or more leading slashes are POSIX, not UNC, and collapse. +# +# "C:relative" is drive-relative: Windows resolves it against the current +# directory *on that drive*, which is not something R can reconstruct +# portably. It keeps its drive prefix rather than being resolved against +# getwd(), so it stays distinct from both "C:/relative" and the cwd. A +# spec that cannot be resolved is better left unmerged than merged wrong. +matrix_path_root <- function(p) { + if (grepl("^//[^/]", p)) { + m <- regmatches(p, regexpr("^//[^/]+(/[^/]+)?", p)) + return(c(root = paste0(m, "/"), tail = substring(p, nchar(m) + 1L))) + } + if (grepl("^[A-Za-z]:/", p)) { + return(c(root = substr(p, 1L, 3L), tail = substring(p, 4L))) + } + if (grepl("^[A-Za-z]:", p)) { + return(c(root = substr(p, 1L, 2L), tail = substring(p, 3L))) + } + if (startsWith(p, "/")) { + return(c(root = "/", tail = substring(p, 2L))) + } + c(root = "", tail = p) } # One identity, one context, one store. A second store for a device @@ -258,37 +283,63 @@ matrix_crypto_ops <- function(override = NULL) { # is the enforcement, and it is durable because the record it checks # against lives on the server rather than in this session. # -# A device the server has never seen is the first-run case and proceeds. +# Three outcomes, and they have to stay three: +# +# absent -- first run, proceed and publish +# present, valid -- compare against this account +# present, invalid -- abort +# +# The raw /keys/query response is read rather than +# mx_crypto_known_devices(), which verifies signatures and drops what +# fails with a warning. That is right for choosing who to encrypt to and +# wrong here: a tampered or signature-stripped entry for our own device +# would come back as no entry, collapse "present but unverifiable" into +# "absent", and publish over it. Another failure read as absence. +# +# What this cannot see is a homeserver that omits the device on purpose. +# That is indistinguishable from a first run over this channel alone, and +# closing it needs key pinning or cross-signing -- tracked separately. +# # A query that cannot be answered is an error: init is about to publish # keys to that same homeserver, so it is not a host we can be off-line # from, and the caller retries on the next poll. matrix_crypto_check_published <- function(mx, acct) { mine <- mx.crypto::mxc_account_identity_keys(acct) - devs <- tryCatch(mx.client::mx_crypto_known_devices(mx, mx$user_id), - error = function(e) { + uid <- mx$user_id + did <- mx$device_id + entry <- tryCatch({ + s <- mx.client::mx_client_session(mx) + resp <- mx.api::mx_keys_query(s, device_keys = stats::setNames( + list(list()), uid)) + (resp$device_keys %||% list())[[uid]][[did]] + }, error = function(e) { stop("chat.api: cannot read this device's published keys from the ", "homeserver (", conditionMessage(e), "). Refusing to publish ", "an Olm identity that may conflict with one already there.", call. = FALSE) }) - for (d in devs) { - if (!identical(d$device_id, mx$device_id)) { - next - } - if (identical(d$ed25519, mine$ed25519) && - identical(d$curve25519, mine$curve25519)) { - return(invisible(TRUE)) - } - stop("chat.api: the homeserver already has different device keys ", - "for ", mx$user_id, "/", mx$device_id, " (ed25519 ", - substr(d$ed25519 %||% "", 1L, 12L), "..., this store has ", - substr(mine$ed25519 %||% "", 1L, 12L), - "...). This store holds another device's Olm account, or the ", - "one this device used has been lost. A device has one identity ", - "for its lifetime, so log in again for a new device_id rather ", - "than republishing under this one.", call. = FALSE) - } - invisible(TRUE) + if (is.null(entry)) { + return(invisible(TRUE)) + } + keys <- tryCatch(mx.crypto::mxc_verify_device_keys(entry, uid, did), + error = function(e) { + stop("chat.api: the homeserver has an entry for ", uid, "/", did, + " whose device keys do not verify (", conditionMessage(e), + "). Refusing to publish over a device record that cannot be ", + "checked -- an unverifiable entry is not an absent one.", + call. = FALSE) + }) + if (identical(keys$ed25519, mine$ed25519) && + identical(keys$curve25519, mine$curve25519)) { + return(invisible(TRUE)) + } + stop("chat.api: the homeserver already has different device keys for ", + uid, "/", did, " (ed25519 ", substr(keys$ed25519 %||% "", 1L, 12L), + "..., this store has ", substr(mine$ed25519 %||% "", 1L, 12L), + "...). This store holds another device's Olm account, or the one ", + "this device used has been lost. A device has one identity for its ", + "lifetime, so log in again for a new device_id rather than ", + "republishing under this one.", call. = FALSE) } # Load or create the account, publish keys, restore sessions and the diff --git a/R/matrix.R b/R/matrix.R index 46964ca..4e230d1 100644 --- a/R/matrix.R +++ b/R/matrix.R @@ -66,10 +66,17 @@ #' process, one device gets one context and one store, so pointing a #' second store at it is an error rather than a second account. And on #' first use the account's keys are checked against the ones the -#' homeserver already has for that \code{device_id} -- which is the -#' only one of the three that survives a restart, and so the one that -#' catches a store swapped between runs. Log in again for a new -#' \code{device_id} rather than re-homing an existing one. +#' homeserver already has for that \code{device_id} -- the only one of +#' the three that survives a restart, and so the one that catches a +#' store swapped between runs. Log in again for a new \code{device_id} +#' rather than re-homing an existing one. +#' +#' That last check distinguishes three cases: no published device is a +#' first run, a published device whose keys match is this account, and +#' a published device that either differs or fails signature +#' verification is an error. It cannot see a homeserver that omits the +#' device deliberately, which is indistinguishable from a first run +#' without key pinning or cross-signing. #' @param .sync Testing seam: replacement for #' \code{mx.client::mx_sync_update}. Leave NULL in production. #' @param .extract Testing seam: replacement for diff --git a/inst/tinytest/test_matrix.R b/inst/tinytest/test_matrix.R index 0396d84..28c9620 100644 --- a/inst/tinytest/test_matrix.R +++ b/inst/tinytest/test_matrix.R @@ -670,6 +670,30 @@ expect_identical(n("C:/x/../y"), "C:/y") expect_identical(n("C:\\x\\y"), "C:/x/y") # There is nothing above a root to climb to. expect_identical(n("/../x"), "/x") + +# A UNC share is not the local path that happens to spell the same. On +# Windows //server/share/x is a remote mount and /server/share/x is a +# local directory; collapsing every leading slash run made them one spec, +# so a request for one store would have been handed the other's context. +expect_identical(n("//server/share/x"), "//server/share/x") +expect_identical(n("\\\\server\\share\\x"), "//server/share/x") +expect_false(identical(n("//server/share/x"), n("/server/share/x"))) +# Three or more leading slashes are POSIX, not UNC, and collapse. +expect_identical(n("///server/share/x"), "/server/share/x") +# Dot segments fold inside a UNC path, and not past the share. +expect_identical(n("//server/share/a/../x"), "//server/share/x") +expect_identical(n("//server/share/../../x"), "//server/share/x") +# Two shares on one host are two roots. +expect_false(identical(n("//server/one/x"), n("//server/two/x"))) + +# "C:relative" is drive-relative -- Windows resolves it against the +# current directory on that drive, which R cannot reconstruct portably. +# It keeps its prefix rather than being merged with either the absolute +# path or the working directory, because a spec that cannot be resolved +# is better left unmerged than merged wrong. +expect_identical(n("C:relative"), "C:relative") +expect_false(identical(n("C:relative"), n("C:/relative"))) +expect_false(identical(n("C:relative"), n("relative"))) # ~ expands, and a relative path resolves against the caller's directory # rather than merging with an unrelated one of the same name. expect_true(startsWith(n("~/x"), "/")) diff --git a/inst/tinytest/test_matrix_mxclient.R b/inst/tinytest/test_matrix_mxclient.R index 523f8d6..7209c1f 100644 --- a/inst/tinytest/test_matrix_mxclient.R +++ b/inst/tinytest/test_matrix_mxclient.R @@ -464,22 +464,24 @@ if (requireNamespace("mx.crypto", quietly = TRUE)) { "mx.client") o3 <- stub("mx_crypto_sessions_load", function(...) list(), "mx.client") - o4 <- stub("mx_client_session", function(...) stop("offline"), + o4 <- stub("mx_client_session", function(...) list(server = "s"), "mx.client") o5 <- stub("mxc_account_identity_keys", function(...) list(curve25519 = "C", ed25519 = "E"), "mx.crypto") - # A homeserver that has never seen this device. The published-key - # check is exercised on its own further down. - o6 <- stub("mx_crypto_known_devices", function(...) list(), - "mx.client") + # A homeserver that has never seen this device, and no rooms. The + # published-key check is exercised on its own further down. + o6 <- stub("mx_keys_query", function(...) list(device_keys = list()), + "mx.api") + o7 <- stub("mx_rooms", function(...) character(), "mx.api") on.exit({ assignInNamespace("mx_crypto_account", o1, ns = "mx.client") assignInNamespace("mx_crypto_publish_keys", o2, ns = "mx.client") assignInNamespace("mx_crypto_sessions_load", o3, ns = "mx.client") assignInNamespace("mx_client_session", o4, ns = "mx.client") assignInNamespace("mxc_account_identity_keys", o5, ns = "mx.crypto") - assignInNamespace("mx_crypto_known_devices", o6, ns = "mx.client") + assignInNamespace("mx_keys_query", o6, ns = "mx.api") + assignInNamespace("mx_rooms", o7, ns = "mx.api") }) store <- tempfile("initstore") @@ -532,9 +534,14 @@ local({ # would mint a fresh account and publish different long-lived keys under # the old device_id. The homeserver is the authority on which keys a # device has, so that is what init checks against. +# +# The raw /keys/query response is stubbed rather than +# mx_crypto_known_devices(), because that helper drops entries whose +# signatures fail and this check has to tell "no entry" from "an entry +# that will not verify". if (requireNamespace("mx.crypto", quietly = TRUE)) { - init_stubs <- function(known, ...) { - o <- list( + init_stubs <- function(query, verify = NULL) { + list( mx_crypto_account = stub("mx_crypto_account", function(...) "acct", "mx.client"), mx_crypto_publish_keys = stub("mx_crypto_publish_keys", @@ -543,47 +550,57 @@ if (requireNamespace("mx.crypto", quietly = TRUE)) { mx_crypto_sessions_load = stub("mx_crypto_sessions_load", function(...) list(), "mx.client"), mx_client_session = stub("mx_client_session", - function(...) stop("offline"), + function(...) list(server = "s"), "mx.client"), - mx_crypto_known_devices = stub("mx_crypto_known_devices", known, - "mx.client"), + mx_keys_query = stub("mx_keys_query", query, "mx.api"), + mxc_verify_device_keys = stub( + "mxc_verify_device_keys", + verify %||% function(...) list(curve25519 = "CURVE-ours", + ed25519 = "ED-ours"), + "mx.crypto"), mxc_account_identity_keys = stub( "mxc_account_identity_keys", function(...) list(curve25519 = "CURVE-ours", ed25519 = "ED-ours"), "mx.crypto")) - o } restore_stubs <- function(o) { - for (nm in names(o)) { - ns <- if (nm == "mxc_account_identity_keys") "mx.crypto" else - "mx.client" - assignInNamespace(nm, o[[nm]], ns = ns) + ns_of <- function(nm) { + if (startsWith(nm, "mxc_")) "mx.crypto" + else if (nm == "mx_keys_query") "mx.api" + else "mx.client" } + for (nm in names(o)) assignInNamespace(nm, o[[nm]], ns = ns_of(nm)) } me <- list(user_id = "@a:ex", device_id = "D1", token = "t", server = "https://ex.invalid") + entry_for <- function(...) { + list(device_keys = list(`@a:ex` = list(...))) + } + with_init <- function(stubs, expr) { + o <- stubs + on.exit(restore_stubs(o)) + force(expr) + } # A device the homeserver has never seen is the first run. local({ - published <- NULL - o <- init_stubs(function(client, user_ids) { - published <<- user_ids - list() + asked <- NULL + o <- init_stubs(function(session, device_keys, ...) { + asked <<- names(device_keys) + entry_for() }) on.exit(restore_stubs(o)) store <- tempfile("pub1") expect_true(is.environment(chat.api:::matrix_crypto_init(me, store = store))) # It asks about its own user, not the room's members. - expect_identical(published, "@a:ex") + expect_identical(asked, "@a:ex") unlink(store, recursive = TRUE) }) # Keys that match are this device's own account: proceed. local({ - o <- init_stubs(function(...) list( - list(user_id = "@a:ex", device_id = "D1", - curve25519 = "CURVE-ours", ed25519 = "ED-ours"))) + o <- init_stubs(function(...) entry_for(D1 = list(keys = "whatever"))) on.exit(restore_stubs(o)) store <- tempfile("pub2") expect_true(is.environment(chat.api:::matrix_crypto_init(me, @@ -595,9 +612,10 @@ if (requireNamespace("mx.crypto", quietly = TRUE)) { # case a restart plus a changed crypto_store produced, which nothing # in the process cache could have seen. local({ - o <- init_stubs(function(...) list( - list(user_id = "@a:ex", device_id = "D1", - curve25519 = "CURVE-theirs", ed25519 = "ED-theirs"))) + o <- init_stubs( + function(...) entry_for(D1 = list(keys = "whatever")), + verify = function(...) list(curve25519 = "CURVE-theirs", + ed25519 = "ED-theirs")) on.exit(restore_stubs(o)) store <- tempfile("pub3") expect_error(chat.api:::matrix_crypto_init(me, store = store), @@ -605,14 +623,43 @@ if (requireNamespace("mx.crypto", quietly = TRUE)) { unlink(store, recursive = TRUE) }) + # An entry that will not verify is NOT an absent entry. A tampered or + # signature-stripped record for our own device used to be dropped by + # mx_crypto_known_devices() with a warning, come back as "no device", + # and get published over. + local({ + o <- init_stubs( + function(...) entry_for(D1 = list(keys = "tampered")), + verify = function(...) stop("signature does not verify")) + on.exit(restore_stubs(o)) + store <- tempfile("pub4") + expect_error(chat.api:::matrix_crypto_init(me, store = store), + "do not verify") + unlink(store, recursive = TRUE) + }) + + # ... and it does not publish over one either. + local({ + uploads <- 0L + o <- init_stubs( + function(...) entry_for(D1 = list(keys = "tampered")), + verify = function(...) stop("signature does not verify")) + assignInNamespace("mx_crypto_publish_keys", + function(...) { uploads <<- uploads + 1L + invisible(TRUE) }, ns = "mx.client") + on.exit(restore_stubs(o)) + store <- tempfile("pub5") + expect_error(chat.api:::matrix_crypto_init(me, store = store)) + expect_identical(uploads, 0L) + unlink(store, recursive = TRUE) + }) + # Another device of the same user is not this device, and does not # collide with it. local({ - o <- init_stubs(function(...) list( - list(user_id = "@a:ex", device_id = "D2", - curve25519 = "CURVE-other", ed25519 = "ED-other"))) + o <- init_stubs(function(...) entry_for(D2 = list(keys = "other"))) on.exit(restore_stubs(o)) - store <- tempfile("pub4") + store <- tempfile("pub6") expect_true(is.environment(chat.api:::matrix_crypto_init(me, store = store))) unlink(store, recursive = TRUE) @@ -624,7 +671,7 @@ if (requireNamespace("mx.crypto", quietly = TRUE)) { local({ o <- init_stubs(function(...) stop("504 from homeserver")) on.exit(restore_stubs(o)) - store <- tempfile("pub5") + store <- tempfile("pub7") expect_error(chat.api:::matrix_crypto_init(me, store = store), "cannot read this device's published keys") unlink(store, recursive = TRUE) @@ -633,14 +680,15 @@ if (requireNamespace("mx.crypto", quietly = TRUE)) { # The check runs before the upload, so a mismatch never publishes. local({ uploads <- 0L - o <- init_stubs(function(...) list( - list(user_id = "@a:ex", device_id = "D1", - curve25519 = "CURVE-theirs", ed25519 = "ED-theirs"))) + o <- init_stubs( + function(...) entry_for(D1 = list(keys = "whatever")), + verify = function(...) list(curve25519 = "CURVE-theirs", + ed25519 = "ED-theirs")) assignInNamespace("mx_crypto_publish_keys", function(...) { uploads <<- uploads + 1L invisible(TRUE) }, ns = "mx.client") on.exit(restore_stubs(o)) - store <- tempfile("pub6") + store <- tempfile("pub8") expect_error(chat.api:::matrix_crypto_init(me, store = store)) expect_identical(uploads, 0L) unlink(store, recursive = TRUE) diff --git a/man/chat_matrix.Rd b/man/chat_matrix.Rd index c3c73ec..a705467 100644 --- a/man/chat_matrix.Rd +++ b/man/chat_matrix.Rd @@ -84,10 +84,17 @@ it belongs to and refuses to open for a different one. Within a process, one device gets one context and one store, so pointing a second store at it is an error rather than a second account. And on first use the account's keys are checked against the ones the -homeserver already has for that \code{device_id} -- which is the -only one of the three that survives a restart, and so the one that -catches a store swapped between runs. Log in again for a new -\code{device_id} rather than re-homing an existing one.} +homeserver already has for that \code{device_id} -- the only one of +the three that survives a restart, and so the one that catches a +store swapped between runs. Log in again for a new \code{device_id} +rather than re-homing an existing one. + +That last check distinguishes three cases: no published device is a +first run, a published device whose keys match is this account, and +a published device that either differs or fails signature +verification is an error. It cannot see a homeserver that omits the +device deliberately, which is indistinguishable from a first run +without key pinning or cross-signing.} \item{.sync}{Testing seam: replacement for \code{mx.client::mx_sync_update}. Leave NULL in production.} From 59ccc507dc63b35b22e4b179b4f6131ea71fbe40 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 16:02:21 -0500 Subject: [PATCH 10/14] test_irc.R: define %||% before using it Base R only grew `%||%` in 4.4.0. The file defined its own at line 17 and used one at line 7, so every run so far had been borrowing base's -- invisible on the R this usually runs against, an error on the R 4.0 the package's Depends claims to support. The Ubuntu CI leg picked distro R 4.3.3 instead of r2u's 4.6.1 this run, which is an apt-source flake rather than anything here, but it is the only reason the bug ever surfaced. --- inst/tinytest/test_irc.R | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/inst/tinytest/test_irc.R b/inst/tinytest/test_irc.R index 0651543..0c1a0d8 100644 --- a/inst/tinytest/test_irc.R +++ b/inst/tinytest/test_irc.R @@ -2,6 +2,13 @@ # (serverSocket + socketAccept, R >= 4.0). Skips where binding a # localhost port is not permitted. +# Defined before its first use, and defined at all. Base R only grew +# `%||%` in 4.4.0, so the line below used to borrow base's silently: fine +# on the R this usually runs against, an error on the R 4.0 this package +# claims to support, and the Ubuntu runner picked distro R 4.3.3 one day +# and said so. +`%||%` <- function(a, b) if (is.null(a)) b else a + ss <- tryCatch(serverSocket(0L), error = function(e) NULL) if (!is.null(ss)) { port <- summary(ss)$port %||% NULL @@ -14,8 +21,6 @@ if (!is.null(ss)) { } } -`%||%` <- function(a, b) if (is.null(a)) b else a - if (!is.null(ss)) { cl <- chat_irc("127.0.0.1", port, nick = "testbot", channels = "#lab") srv <- socketAccept(ss, blocking = FALSE, open = "r+") From f040108b08fbfbad7dd92f50396bb41a27975b84 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 16:05:12 -0500 Subject: [PATCH 11/14] Spell out the last inherited %||% in the tests test_matrix_mxclient.R had one too, in the stub helper added this round. Every test file now supplies its own or does without, verified by running each of the five with an inherited %||% poisoned to error. --- inst/tinytest/test_matrix_mxclient.R | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/inst/tinytest/test_matrix_mxclient.R b/inst/tinytest/test_matrix_mxclient.R index 7209c1f..16c9e06 100644 --- a/inst/tinytest/test_matrix_mxclient.R +++ b/inst/tinytest/test_matrix_mxclient.R @@ -553,10 +553,16 @@ if (requireNamespace("mx.crypto", quietly = TRUE)) { function(...) list(server = "s"), "mx.client"), mx_keys_query = stub("mx_keys_query", query, "mx.api"), + # Spelled out rather than `verify %||% default`: base R has + # no %||% before 4.4, and this package supports R 4.0. mxc_verify_device_keys = stub( "mxc_verify_device_keys", - verify %||% function(...) list(curve25519 = "CURVE-ours", - ed25519 = "ED-ours"), + if (is.null(verify)) { + function(...) list(curve25519 = "CURVE-ours", + ed25519 = "ED-ours") + } else { + verify + }, "mx.crypto"), mxc_account_identity_keys = stub( "mxc_account_identity_keys", From 8d8f011cdc4bfaed6a6fc10a70c118a49010b41d Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 16:19:45 -0500 Subject: [PATCH 12/14] Treat a partial /keys/query as unanswered, and refuse drive-relative stores Two more boundaries where a failure was reading as an absence. /keys/query answers 200 with a `failures` map when it could not reach a server, returning whatever it did manage. The check ignored it, so an empty device_keys beside a non-empty failures map was classified as a first run and published over. This query names exactly one user -- ours -- so any failure at all means the question went unanswered, and a request that throws and one that succeeds with nothing in it are the same state to a caller about to publish keys. An empty failures map is still the ordinary first-run answer and is tested to stay that way. And "C:store" is drive-relative: Windows resolves it against the current directory of that drive, which R cannot read. Treating "C:" as a root to fold against made "C:../x", "C:x" and "C:a/../../x" one spec when they are three directories, so two requested stores could still land on one cached context. Keeping unresolved leading ".." would make the spec honest without making it canonical -- two spellings of one directory would stay two -- so there is no correct answer available and this asks for one that is. Absolute drive paths are unaffected. 451 assertions, 270 on a bare runner. Two mutations, both caught. --- DESCRIPTION | 2 +- R/matrix-crypto.R | 47 +++++++++++++++++++++------- R/matrix.R | 15 +++++++-- inst/tinytest/test_matrix.R | 21 ++++++++----- inst/tinytest/test_matrix_mxclient.R | 35 +++++++++++++++++++++ man/chat_matrix.Rd | 15 +++++++-- 6 files changed, 108 insertions(+), 27 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 1f6f482..b915ed3 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: chat.api Type: Package Title: Transport-Agnostic Chat Contract for R Agents -Version: 0.0.1.8 +Version: 0.0.1.9 Date: 2026-08-05 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), diff --git a/R/matrix-crypto.R b/R/matrix-crypto.R index 6e9f8bb..8ba5b83 100644 --- a/R/matrix-crypto.R +++ b/R/matrix-crypto.R @@ -132,6 +132,21 @@ matrix_crypto_store_spec <- function(store = NULL, app = NULL) { # Windows drive root, turning "C:/" into "C:". matrix_normalize_path <- function(p) { p <- gsub("\\\\", "/", path.expand(p)) + # "C:store" is drive-relative: Windows resolves it against the current + # directory *on drive C*, which R cannot read and this cannot fold. + # Treating "C:" as a root made "C:../x", "C:x" and "C:a/../../x" one + # spec when they are three different directories, so two stores could + # still share a context. Keeping unresolved leading ".." would make + # the spec honest and still not canonical -- two spellings of one + # directory would stay two. There is no correct answer available, so + # this asks for one that is. + if (grepl("^[A-Za-z]:([^/]|$)", p)) { + stop("chat.api: crypto_store cannot be a Windows drive-relative ", + "path (", p, "). It names a directory that depends on the ", + "current directory of that drive, which cannot be resolved ", + "here, so two such paths cannot be told apart. Give an ", + "absolute path, like C:/store.", call. = FALSE) + } r <- matrix_path_root(p) if (!nzchar(r[["root"]])) { # Relative to wherever the caller stood. Two callers in different @@ -164,11 +179,8 @@ matrix_normalize_path <- function(p) { # context of another, which is the collision the whole check exists to # stop. Three or more leading slashes are POSIX, not UNC, and collapse. # -# "C:relative" is drive-relative: Windows resolves it against the current -# directory *on that drive*, which is not something R can reconstruct -# portably. It keeps its drive prefix rather than being resolved against -# getwd(), so it stays distinct from both "C:/relative" and the cwd. A -# spec that cannot be resolved is better left unmerged than merged wrong. +# Drive-relative paths never reach here: matrix_normalize_path() rejects +# them, because "C:" is a drive and not a root to fold components against. matrix_path_root <- function(p) { if (grepl("^//[^/]", p)) { m <- regmatches(p, regexpr("^//[^/]+(/[^/]+)?", p)) @@ -177,9 +189,6 @@ matrix_path_root <- function(p) { if (grepl("^[A-Za-z]:/", p)) { return(c(root = substr(p, 1L, 3L), tail = substring(p, 4L))) } - if (grepl("^[A-Za-z]:", p)) { - return(c(root = substr(p, 1L, 2L), tail = substring(p, 3L))) - } if (startsWith(p, "/")) { return(c(root = "/", tail = substring(p, 2L))) } @@ -307,17 +316,31 @@ matrix_crypto_check_published <- function(mx, acct) { mine <- mx.crypto::mxc_account_identity_keys(acct) uid <- mx$user_id did <- mx$device_id - entry <- tryCatch({ + resp <- tryCatch({ s <- mx.client::mx_client_session(mx) - resp <- mx.api::mx_keys_query(s, device_keys = stats::setNames( - list(list()), uid)) - (resp$device_keys %||% list())[[uid]][[did]] + mx.api::mx_keys_query(s, + device_keys = stats::setNames(list(list()), uid)) }, error = function(e) { stop("chat.api: cannot read this device's published keys from the ", "homeserver (", conditionMessage(e), "). Refusing to publish ", "an Olm identity that may conflict with one already there.", call. = FALSE) }) + # /keys/query answers 200 with a `failures` map when it could not + # reach a server, and the device_keys it returns are whatever it did + # manage. This query names one user -- ours -- so any failure at all + # means our question went unanswered, and the empty result that comes + # back with it is not evidence of a device that was never published. + # A request that throws and a request that succeeds with nothing in it + # are the same state to a caller about to publish keys. + if (length(resp$failures)) { + stop("chat.api: the homeserver could not answer for ", + paste(names(resp$failures), collapse = ", "), + " when asked about this device's published keys. Refusing to ", + "publish an Olm identity against an unanswered question.", + call. = FALSE) + } + entry <- (resp$device_keys %||% list())[[uid]][[did]] if (is.null(entry)) { return(invisible(TRUE)) } diff --git a/R/matrix.R b/R/matrix.R index 4e230d1..891a987 100644 --- a/R/matrix.R +++ b/R/matrix.R @@ -74,9 +74,18 @@ #' That last check distinguishes three cases: no published device is a #' first run, a published device whose keys match is this account, and #' a published device that either differs or fails signature -#' verification is an error. It cannot see a homeserver that omits the -#' device deliberately, which is indistinguishable from a first run -#' without key pinning or cross-signing. +#' verification is an error. A \code{/keys/query} that returns a +#' \code{failures} map is an error too, since the empty result beside +#' it is an unanswered question rather than an absent device. It cannot +#' see a homeserver that omits the device deliberately, which is +#' indistinguishable from a first run without key pinning or +#' cross-signing. +#' +#' A Windows drive-relative \code{crypto_store} (\code{"C:store"}) is +#' rejected. It names a directory relative to the current directory of +#' that drive, which cannot be read here, so two such paths cannot be +#' told apart and two stores could share one context. Pass an absolute +#' path. #' @param .sync Testing seam: replacement for #' \code{mx.client::mx_sync_update}. Leave NULL in production. #' @param .extract Testing seam: replacement for diff --git a/inst/tinytest/test_matrix.R b/inst/tinytest/test_matrix.R index 28c9620..fd23a77 100644 --- a/inst/tinytest/test_matrix.R +++ b/inst/tinytest/test_matrix.R @@ -686,14 +686,19 @@ expect_identical(n("//server/share/../../x"), "//server/share/x") # Two shares on one host are two roots. expect_false(identical(n("//server/one/x"), n("//server/two/x"))) -# "C:relative" is drive-relative -- Windows resolves it against the -# current directory on that drive, which R cannot reconstruct portably. -# It keeps its prefix rather than being merged with either the absolute -# path or the working directory, because a spec that cannot be resolved -# is better left unmerged than merged wrong. -expect_identical(n("C:relative"), "C:relative") -expect_false(identical(n("C:relative"), n("C:/relative"))) -expect_false(identical(n("C:relative"), n("relative"))) +# A drive-relative path is refused rather than folded. Windows resolves +# "C:store" against the current directory of drive C, which R cannot +# read, so "C:../x", "C:x" and "C:a/../../x" are three directories that +# no amount of folding here can tell apart -- and two of them sharing a +# spec is two stores sharing a context. +expect_error(n("C:relative"), "drive-relative") +expect_error(n("C:../x"), "drive-relative") +expect_error(n("C:a/../../x"), "drive-relative") +expect_error(n("C:"), "drive-relative") +expect_error(n("c:store"), "drive-relative") +expect_error(spec("C:relative"), "drive-relative") +# An absolute drive path is fine, and is what the error asks for. +expect_identical(n("C:/relative"), "C:/relative") # ~ expands, and a relative path resolves against the caller's directory # rather than merging with an unrelated one of the same name. expect_true(startsWith(n("~/x"), "/")) diff --git a/inst/tinytest/test_matrix_mxclient.R b/inst/tinytest/test_matrix_mxclient.R index 16c9e06..8eda688 100644 --- a/inst/tinytest/test_matrix_mxclient.R +++ b/inst/tinytest/test_matrix_mxclient.R @@ -683,6 +683,41 @@ if (requireNamespace("mx.crypto", quietly = TRUE)) { unlink(store, recursive = TRUE) }) + # /keys/query answers 200 with a `failures` map when it could not + # reach a server, and returns whatever it did manage. This query names + # one user -- ours -- so any failure means the question went + # unanswered, and the empty device_keys beside it is not evidence of a + # device that was never published. A request that throws and one that + # succeeds with nothing in it are the same state here. + local({ + uploads <- 0L + o <- init_stubs(function(...) list( + failures = list(ex = list(errcode = "M_UNKNOWN", + error = "Failed to fetch keys")), + device_keys = list())) + assignInNamespace("mx_crypto_publish_keys", + function(...) { uploads <<- uploads + 1L + invisible(TRUE) }, ns = "mx.client") + on.exit(restore_stubs(o)) + store <- tempfile("pub9") + expect_error(chat.api:::matrix_crypto_init(me, store = store), + "could not answer for ex") + expect_identical(uploads, 0L) + unlink(store, recursive = TRUE) + }) + + # An empty failures map is not a failure: that is the ordinary + # first-run answer and must not start erroring. + local({ + o <- init_stubs(function(...) list(failures = list(), + device_keys = list())) + on.exit(restore_stubs(o)) + store <- tempfile("pub10") + expect_true(is.environment(chat.api:::matrix_crypto_init(me, + store = store))) + unlink(store, recursive = TRUE) + }) + # The check runs before the upload, so a mismatch never publishes. local({ uploads <- 0L diff --git a/man/chat_matrix.Rd b/man/chat_matrix.Rd index a705467..bcd6653 100644 --- a/man/chat_matrix.Rd +++ b/man/chat_matrix.Rd @@ -92,9 +92,18 @@ rather than re-homing an existing one. That last check distinguishes three cases: no published device is a first run, a published device whose keys match is this account, and a published device that either differs or fails signature -verification is an error. It cannot see a homeserver that omits the -device deliberately, which is indistinguishable from a first run -without key pinning or cross-signing.} +verification is an error. A \code{/keys/query} that returns a +\code{failures} map is an error too, since the empty result beside +it is an unanswered question rather than an absent device. It cannot +see a homeserver that omits the device deliberately, which is +indistinguishable from a first run without key pinning or +cross-signing. + +A Windows drive-relative \code{crypto_store} (\code{"C:store"}) is +rejected. It names a directory relative to the current directory of +that drive, which cannot be read here, so two such paths cannot be +told apart and two stores could share one context. Pass an absolute +path.} \item{.sync}{Testing seam: replacement for \code{mx.client::mx_sync_update}. Leave NULL in production.} From e25c196d33044693fd9616ace5291c47a9be276f Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 16:41:55 -0500 Subject: [PATCH 13/14] Require the mx.client that will not send to nobody cornball-ai/mx.client#19 fixes three ways mx_send_encrypted() could report success while the message reached nobody: a self-filter that compared device_id without user_id and so dropped other accounts sharing a device name, an empty recipient list that still posted the event, and partial /keys/query and /keys/claim responses read as though the successful half were the whole answer. This adapter calls straight into that function and cannot work around any of them, so the floor goes to 0.2.0.1 and CI enforces it rather than declaring it. Every step of reading that floor is checked: a parse that quietly finds nothing would wave through exactly what the check exists to catch, which is what corteza's first version of this did on macOS. CI is red until #19 merges, by construction. --- .github/workflows/ci.yaml | 22 +++++++++++++++++++++- DESCRIPTION | 4 ++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 37f1ad3..19a45fa 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -56,6 +56,13 @@ jobs: # Fail loudly rather than letting the suite skip. mx.crypto is # reported but not required, since the macOS leg does without it. + # + # The mx.client floor is enforced, not just declared: the adapter + # calls straight into mx_send_encrypted() and cannot work around + # what an older one gets wrong. Every step of reading that floor is + # checked, because a parse that quietly finds nothing would wave + # through exactly what this is here to catch -- which is what the + # first version of corteza's equivalent did on macOS. - name: Verify Matrix stack run: | Rscript -e ' @@ -63,8 +70,21 @@ jobs: if (!requireNamespace(p, quietly = TRUE)) stop("chat.api CI needs ", p, ": without it ", "test_matrix_mxclient.R skips itself", call. = FALSE) - message(p, " ", as.character(utils::packageVersion(p))) } + sug <- read.dcf("DESCRIPTION")[1L, "Suggests"] + if (is.na(sug)) stop("no Suggests field in DESCRIPTION") + e <- grep("^mx[.]client", + trimws(strsplit(sug, ",")[[1L]]), value = TRUE) + if (!length(e)) stop("no mx.client entry in Suggests") + m <- regmatches(e, regexpr("[0-9][0-9.-]*", e)) + if (!length(m)) stop("mx.client Suggests entry declares no floor") + floor <- package_version(m) + have <- utils::packageVersion("mx.client") + message("mx.client ", have, " (>= ", floor, ")") + if (have < floor) + stop("mx.client ", have, " is below the ", floor, + " chat.api declares", call. = FALSE) + message("mx.api ", utils::packageVersion("mx.api")) message("mx.crypto ", if (requireNamespace("mx.crypto", quietly = TRUE)) as.character(utils::packageVersion("mx.crypto")) diff --git a/DESCRIPTION b/DESCRIPTION index b915ed3..028edff 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: chat.api Type: Package Title: Transport-Agnostic Chat Contract for R Agents -Version: 0.0.1.9 +Version: 0.0.1.10 Date: 2026-08-05 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), @@ -21,7 +21,7 @@ URL: https://github.com/cornball-ai/chat.api BugReports: https://github.com/cornball-ai/chat.api/issues Suggests: mx.api, - mx.client (>= 0.2.0), + mx.client (>= 0.2.0.1), mx.crypto, slackr, tinytest From 82152b9eedb33473b740bea3bf739481d9153f66 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Wed, 5 Aug 2026 16:57:14 -0500 Subject: [PATCH 14/14] Track the mx.client floor to 0.2.0.2 0.2.0.2 measures the zero-recipient guard against the devices the homeserver named rather than the ones that verified, so a room whose only other device is malformed no longer reads as a room of one, and refuses an explicitly empty recipient list. --- DESCRIPTION | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 028edff..419cecf 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: chat.api Type: Package Title: Transport-Agnostic Chat Contract for R Agents -Version: 0.0.1.10 +Version: 0.0.1.11 Date: 2026-08-05 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), @@ -21,7 +21,7 @@ URL: https://github.com/cornball-ai/chat.api BugReports: https://github.com/cornball-ai/chat.api/issues Suggests: mx.api, - mx.client (>= 0.2.0.1), + mx.client (>= 0.2.0.2), mx.crypto, slackr, tinytest