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/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..01fd757 100644 --- a/cran-comments.md +++ b/cran-comments.md @@ -2,20 +2,61 @@ ## 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 ## 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. 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"