From 4353e5ea5adb4fd8251cacb5a618097a0e24281f Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Tue, 4 Aug 2026 10:53:18 -0500 Subject: [PATCH 1/3] Release 0.2.0 Minor, not patch: mx_crypto_room_key_payload() takes four further required arguments, so five-argument calls now error. NEWS collapsed from 0.1.1.1-0.1.1.5 into one section grouped security / breaking / new / other. cran-comments rewritten from the first-submission text. --- DESCRIPTION | 4 +- NEWS.md | 101 ++++++++++++++++++++++++----------------------- cran-comments.md | 64 ++++++++++++++++++++++++------ 3 files changed, 106 insertions(+), 63 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 4f06806..10a332a 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,8 +1,8 @@ Package: mx.client Type: Package Title: Stateful Matrix Client Helpers -Version: 0.1.1.5 -Date: 2026-06-13 +Version: 0.2.0 +Date: 2026-08-04 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), email = "troy@cornball.ai", diff --git a/NEWS.md b/NEWS.md index a178688..65fd8b7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,25 +1,9 @@ -# mx.client 0.1.1.5 +# mx.client 0.2.0 -* `print()` method for `mx_client_config`: masks `token` and `password` - as `` (or `` when empty) instead of falling through to - the default list print, which showed credentials verbatim. `sync_token` - and every other field stay visible. `unclass()` still exposes the raw - values for the cases that need them. - -# mx.client 0.1.1.4 - -* New `mx_set_displayname(client, name)`: client-level wrapper over - `mx.api::mx_set_displayname()` that builds the session from the - client config and retries once through `mx_with_relogin()` on a - rejected token, so long-running bots can rename themselves without - hand-rolling session or relogin plumbing. Extracted from corteza's - Matrix model badge (corteza#155), which keeps the badge policy and - calls this for the profile update. - -# mx.client 0.1.1.3 +## Security -* **HIGH** (security): homeserver-supplied keys are now verified before - use. `mx_crypto_known_devices()` checks each device's Ed25519 +* **HIGH**: homeserver-supplied keys are now verified before use. + `mx_crypto_known_devices()` checks each device's Ed25519 self-signature with `mx.crypto::mxc_verify_device_keys()`, and `mx_crypto_claim_otks()` checks each claimed one-time key with `mx.crypto::mxc_verify_one_time_key()` against that verified Ed25519. @@ -28,12 +12,12 @@ Curve25519 key for any device and read everything sent to that user. A device that fails verification is dropped with a warning and the remaining devices still receive the message. -* **HIGH** (security): Olm to-device payloads now carry the - `sender`, `recipient`, `recipient_keys`, and `keys` fields the spec - requires. `mx_crypto_room_key_payload()` emitted only `type` and - `content`, leaving the receiver nothing to authenticate against; - other clients reject such payloads. -* **HIGH** (security): `mx_crypto_process_sync()` and +* **HIGH**: Olm to-device payloads now carry the `sender`, + `recipient`, `recipient_keys`, and `keys` fields the spec requires. + `mx_crypto_room_key_payload()` emitted only `type` and `content`, + leaving the receiver nothing to authenticate against; other clients + reject such payloads. +* **HIGH**: `mx_crypto_process_sync()` and `mx_crypto_handle_to_device()` now confirm a decrypted Olm payload names this device as recipient before acting on it. Decrypting only proves the message was encrypted to our key, not that it was meant @@ -51,41 +35,58 @@ on its own: a hostile homeserver writes both, so it can make them corroborate each other. Binding to a self-signed `device_keys` object is the part it cannot forge. Callers that pass no `devices` still - decrypt but always get `sender_verified = FALSE`, as do events - decrypted with a session from a store written before this change. -* `mx_crypto_room_key_payload()` gains `sender_user_id`, - `sender_ed25519`, `recipient_user_id`, and `recipient_ed25519`; - `mx_crypto_encrypt_for_devices()` gains `sender_user_id` and now - requires each recipient to carry a verified `ed25519`; - `mx_crypto_handle_to_device()` gains `self_id`, `self_ed25519`, and - `devices`, and its result carries `sender_bound`. -* Session stores written before this release still load: a bare - `megolm_in` pickle is read as a session with no attested sender - rather than discarded, so a running client keeps its history. -* `Suggests: mx.crypto (>= 0.2.0)`, the first version providing the - verification helpers. -* New `inst/tinytest/test_transport.R` covers the verification paths - against tampered fixture responses. `R/transport.R` previously had no - test file at all. + decrypt but always get `sender_verified = FALSE`. -# mx.client 0.1.1.2 +## Breaking changes -* New: `mx_table_html()` and `mx_send_table()` render a data frame, - matrix, or list as the conservative Matrix table HTML that clients such - as FluffyChat 2.6.0+ accept: a bare `` of ``, `
`, and - `` nodes, with no CSS, colspan, rowspan, or custom attributes. - A plain-text `body` is generated alongside for clients that ignore +* `mx_crypto_room_key_payload()` requires four further arguments: + `sender_user_id`, `sender_ed25519`, `recipient_user_id`, and + `recipient_ed25519`. They are what the payload's authentication block + is built from, so they have no defaults; a five-argument call now + errors rather than silently emitting an unauthenticatable payload. +* `mx_crypto_encrypt_for_devices()` gains `sender_user_id` and now + requires each recipient to carry a verified `ed25519`. Recipients + from `mx_crypto_known_devices()` already do. +* `mx_crypto_handle_to_device()` gains `self_id`, `self_ed25519`, and + `devices`; its result carries `sender_bound`. + +## New + +* `mx_table_html()` and `mx_send_table()` render a data frame, matrix, + or list as the conservative Matrix table HTML that clients such as + FluffyChat 2.6.0+ accept: a bare `` of ``, `
`, and + `` nodes, with no CSS, colspan, rowspan, or custom attributes. A + plain-text `body` is generated alongside for clients that ignore `formatted_body`. * `mx_markdown_to_html()` converts GitHub-style pipe tables to the same table HTML, honouring the `:---`/`:---:`/`---:` alignment row. -* New: `inst/skills/mx.client/matrix-messaging/SKILL.md`. +* `mx_set_displayname(client, name)`: client-level wrapper over + `mx.api::mx_set_displayname()` that builds the session from the client + config and retries once through `mx_with_relogin()` on a rejected + token, so long-running bots can rename themselves without + hand-rolling session or relogin plumbing. +* `print()` method for `mx_client_config` masks `token` and `password` + as `` (or `` when empty) instead of falling through to + the default list print, which showed credentials verbatim. + `sync_token` and every other field stay visible; `unclass()` still + exposes the raw values. -# mx.client 0.1.1.1 +## Other * `mx_extract_text_events()` keeps the event's `origin_server_ts` as a `ts` field (milliseconds since the epoch, NULL when the server omits it). Records previously carried no time at all, leaving consumers to stamp messages with their own poll clock. +* Session stores written before this release still load: a bare + `megolm_in` pickle is read as a session with no attested sender + rather than discarded, so a running client keeps its history. Events + decrypted with such a session report `sender_verified = FALSE`. +* `Suggests: mx.crypto (>= 0.2.0)`, the first version providing the + verification helpers. +* New `inst/skills/mx.client/matrix-messaging/SKILL.md`. +* New `inst/tinytest/test_transport.R` covers the key-verification + paths against tampered fixture responses. `R/transport.R` previously + had no test file at all. # mx.client 0.1.1 diff --git a/cran-comments.md b/cran-comments.md index 313e77f..91c49ef 100644 --- a/cran-comments.md +++ b/cran-comments.md @@ -2,20 +2,62 @@ ## Submission -First submission of mx.client to CRAN. +Minor release, 0.1.1 -> 0.2.0. + +The headline is a set of security fixes to the optional end-to-end +encryption path. mx.client built and consumed Matrix E2EE payloads +without verifying anything the homeserver returned: + +* `/keys/query` device keys were read straight out of the response with + their Ed25519 self-signature ignored, so a malicious or compromised + homeserver could substitute its own Curve25519 key for any device and + read everything sent to that user. +* `/keys/claim` one-time keys were taken with their signatures + discarded. +* Outbound Olm payloads omitted the `sender` / `recipient` / + `recipient_keys` / `keys` block the Matrix spec requires, leaving + receivers nothing to authenticate against. +* Inbound Olm payloads were acted on without checking they named this + device as recipient. + +All four are fixed, using the verification helpers `mx.crypto` 0.2.0 +added for this purpose. `Suggests` is now `mx.crypto (>= 0.2.0)`. + +The release also adds Matrix table rendering (`mx_table_html()`, +`mx_send_table()`, and pipe-table support in `mx_markdown_to_html()`), +`mx_set_displayname()`, and a `print()` method for `mx_client_config` +that masks credentials. + +## Breaking changes + +`mx_crypto_room_key_payload()` takes four further arguments +(`sender_user_id`, `sender_ed25519`, `recipient_user_id`, +`recipient_ed25519`). They are what the payload's authentication block +is built from, so they deliberately have no defaults: a five-argument +call now errors rather than silently emitting a payload no receiver can +authenticate. `mx_crypto_encrypt_for_devices()` and +`mx_crypto_handle_to_device()` gain arguments in the same vein. + +This is why the version takes a minor bump rather than a patch. There +are no reverse dependencies on CRAN. ## Test environments -* Ubuntu 24.04, R 4.x (local): R CMD check --as-cran, 0 errors, 0 warnings, - 1 note (new submission) -* win-builder R-devel and R-release (check_win_devel), 2026-06-13 +* Ubuntu 24.04, R 4.5.3 (local): `R CMD check --as-cran`, 0 errors, + 0 warnings, 0 notes +* Windows 10, R 4.6.0: `R CMD check --as-cran`, Status OK +* Windows 10, R-devel (2026-07-21 r90286 ucrt): `R CMD check --as-cran`, + Status OK +* GitHub Actions (ubuntu-latest, macos-latest) via r-ci, R-release +* win-builder R-devel and R-release ## Notes for the reviewer -* `\dontrun{}` examples are limited to functions that require a live - Matrix homeserver session or account credentials (login, sync, send, - room lookup, key upload/claim). Everything that can run locally has a - runnable or `\donttest{}` example writing only to `tempdir()`. -* The 'mx.crypto' package in Suggests is on CRAN; it is optional because - it needs a Rust toolchain to build, and all non-encrypted functionality - works without it. +* `\dontrun{}` examples are limited to functions needing a live Matrix + homeserver session or account credentials (login, sync, send, room + lookup, key upload/claim). Everything runnable locally has a runnable + or `\donttest{}` example writing only to `tempdir()`. +* `mx.crypto` in Suggests is on CRAN. It is optional because it needs a + Rust toolchain to build, and every non-encrypted code path works + without it. The Windows checks above ran with + `_R_CHECK_FORCE_SUGGESTS_=false` for that reason. From 04df099a07c4dd7aac110e1b05b9d9bfcd9d9db4 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Tue, 4 Aug 2026 13:35:51 -0500 Subject: [PATCH 2/3] cran-comments: drop the win-builder claim Those runs were never made. Listing only what was actually run: Ubuntu 24.04 R 4.5.3, Windows 10 R 4.6.0, and Windows R-devel. --- cran-comments.md | 1 - 1 file changed, 1 deletion(-) diff --git a/cran-comments.md b/cran-comments.md index 91c49ef..01fd757 100644 --- a/cran-comments.md +++ b/cran-comments.md @@ -49,7 +49,6 @@ are no reverse dependencies on CRAN. * Windows 10, R-devel (2026-07-21 r90286 ucrt): `R CMD check --as-cran`, Status OK * GitHub Actions (ubuntu-latest, macos-latest) via r-ci, R-release -* win-builder R-devel and R-release ## Notes for the reviewer From 0bacd2937155be7dc6bc72bfb1afa52860d4ed95 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Tue, 4 Aug 2026 14:48:40 -0500 Subject: [PATCH 3/3] Add tools/check_tarball.sh R CMD build packages the working directory, not the git tree, and skips only a fixed set of dot-entries. Anything untracked in the package root ships. CI checks out a clean tree and can never see it, so this has to run locally before submission. Ported from whisper with an allowlist matching what this package ships. Excluded from the tarball via .Rbuildignore. --- .Rbuildignore | 1 + tools/check_tarball.sh | 119 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100755 tools/check_tarball.sh diff --git a/.Rbuildignore b/.Rbuildignore index adfeeab..746f931 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -4,3 +4,4 @@ ^CLAUDE\.md$ ^cran-comments\.md$ ^README\.Rmd$ +^tools$ diff --git a/tools/check_tarball.sh b/tools/check_tarball.sh new file mode 100755 index 0000000..351ffc0 --- /dev/null +++ b/tools/check_tarball.sh @@ -0,0 +1,119 @@ +#!/usr/bin/env bash +# Validate what R CMD build actually puts in the tarball. +# +# WHY THIS EXISTS, AND WHY IT MUST BE RUN LOCALLY +# +# R CMD build packages the working DIRECTORY, not the git tree. Anything +# sitting in the package root that .Rbuildignore does not exclude ships -- +# including untracked scratch directories. R CMD build does NOT skip +# arbitrary dot-directories; it excludes only a fixed known set (.git, +# .Rhistory, ...), so a stray `.something/` is packaged in full. +# +# Two real incidents in this codebase: +# - a `git worktree add` left a complete second copy of the package at +# whisper/wtest_path/, which doubled the tarball +# - a temporary clone of an unrelated package at +# subtitles/.godotR-boot-review/ contributed 179 of 213 tarball entries +# +# Neither was tracked by git. A CI job checks out a clean tree, so CI can +# never see either of them. **Only a local run against the real working +# tree catches this class**, which is why this is a mandatory +# pre-submission step and not merely a CI job. +# +# Portability: this is a mandatory local step, so it must run on whatever +# machine is submitting. Kept to POSIX-ish tools -- no `stat -c` (GNU-only), +# no `du -b` (GNU-only), and no parsing of `tar -tv` output, whose column +# layout differs between GNU tar and bsdtar. Sizes come from `du -sk` on +# extracted entries instead. +# +# Usage: +# tools/check_tarball.sh [package-dir] # default: repo root +# +# Exit status 0 if the tarball contains only expected top-level entries. + +set -uo pipefail + +pkg_dir=${1:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)} +pkg_dir=$(cd "$pkg_dir" && pwd) + +# Exactly what whisper ships today. Deliberately tight rather than the +# generic R-package set: if the package legitimately gains `vignettes` or +# `src`, that is a real change and belongs in a commit that adds the entry +# here. Anything unlisted is either that, or contamination. +# +# One per line, matched with `grep -Fqx`: fixed-string, whole-line. A +# substring or regex match here fails OPEN -- an entry named `R.*` would +# match `R` as a pattern and be waved through as expected content. +allowed_list() { + cat <<'EOF' +build +DESCRIPTION +NAMESPACE +NEWS.md +README.md +R +man +inst +tests +vignettes +EOF +} + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +echo "building from: $pkg_dir" +if ! (cd "$work" && R CMD build "$pkg_dir" >build.log 2>&1); then + echo "::error::R CMD build failed" + tail -20 "$work/build.log" + exit 1 +fi + +tarball=$(ls "$work"/*.tar.gz 2>/dev/null | head -1) +if [ -z "$tarball" ]; then + echo "::error::no tarball produced" + exit 1 +fi +echo "tarball: $(basename "$tarball") ($(wc -c <"$tarball" | tr -d ' ') bytes)" + +# Extract and measure on disk: avoids `tar -tv` column-layout differences +# between GNU tar and bsdtar, and gives portable sizes via `du -sk`. +mkdir -p "$work/x" +if ! tar xzf "$tarball" -C "$work/x"; then + echo "::error::could not extract tarball" + exit 1 +fi +root=$(ls "$work/x" | head -1) +pkg_root="$work/x/$root" + +# Three globs, because each misses what the others catch: `*` skips all +# dotfiles, `.[!.]*` catches `.foo` but not `..foo`, and `..?*` catches +# `..foo` without matching `.` or `..` themselves. A missed entry fails +# OPEN. `-e || -L` so a dangling symlink is inspected rather than skipped +# (`-e` follows the link and is false when the target is gone). +fail=0 +count=0 +for path in "$pkg_root"/* "$pkg_root"/.[!.]* "$pkg_root"/..?*; do + [ -e "$path" ] || [ -L "$path" ] || continue + e=$(basename "$path") + count=$((count + 1)) + if ! allowed_list | grep -Fqx -- "$e"; then + if [ -L "$path" ] && [ ! -e "$path" ]; then + echo "::error::unexpected top-level entry: ${e} (dangling symlink)" + else + kb=$(du -sk "$path" | awk '{print $1}') + echo "::error::unexpected top-level entry: ${e} (${kb} KB)" + fi + fail=1 + fi +done + +if [ "$fail" -eq 0 ]; then + echo "tarball contents: OK (${count} top-level entries)" +else + echo + echo "R CMD build packages the working directory, not the git tree." + echo "Untracked scratch files in the package root ship unless" + echo ".Rbuildignore excludes them. Remove them, or add an exclusion." +fi +exit "$fail"