fix(channel): carry attachments, and their captions

jlxq0 commented

2026-08-25 01:52:01 +00:00

Owner

Closes #106.

Three of Julian's messages never reached the agent tonight, each carrying a real
instruction. They were found only by dumping the room and grepping for
msgtype != m.text.

The reasoning was sound and the premise was wrong

channel.rs filtered on MessageType::Text, deliberately, and said why:

an image or file event also carries a body — the sender's chosen filename —
and pushing that as if it were a message misrepresents what arrived.

True of an uncaptioned upload. False of a captioned one — and Element on a
phone puts a photo's caption in content.body, which is the traffic that
actually arrives here.

The rule was already in this repository. mcp.rs:5245, on the send side:

Per Matrix spec: when filename ≠ body, body is the caption. When there is
no caption, use the filename as body.

Two paths, 4,600 lines apart, holding opposite premises about one field — and
the one that implements the spec was not the one the channel used. split_caption
is that rule applied on the way in.

What changes

m.image, m.file, m.audio and m.video are carried as attachments
rather than dropped:

<channel room="!r:…" sender="@julian:…" event="$e"
         attachment="m.image" filename="Screenshot.png">
  <matrix:message trust="external">the /mcp output you asked about</matrix:message>
</channel>

An agent can now tell "somebody said this" from "somebody sent a file called
this". room and event were already in the meta, so download_attachment
needs nothing new to fetch the bytes.

An uncaptioned upload delivers no prose at all. Its filename is metadata and
travels as an escaped attribute. The original objection was that a filename must
not masquerade as a message — labelling it is the fix, not dropping the event.

Replay changes with it, and that is the half that made this invisible

read_events_from_chunk fills untrusted_body from content.body for any
event that has one. So a caption dropped live reappeared on the next attach —
hours later, out of order, with nothing marking it as late.

That is why this presented as "Julian's messages are sometimes slow" rather
than "images are dropped". A message that never arrives is noticed; one that
arrives late and unlabelled is explained away.

carried_of is the same classification for the replay path, so the two now
agree in both directions — including refusing to replay an uncaptioned
upload's filename as prose, which is the mistake the live path was avoiding.

Security

A caption is untrusted prose from the same sender as any other message, so it
goes through content_sandbox::evaluate on both paths, unchanged. Filenames are
sender-chosen too and are escaped as meta values rather than trusted for looking
like filenames — the existing meta_values_cannot_break_out_of_the_attribute
test covers that escaping.

Nothing new is exposed: no mxc://, no new tool, no new surface.

Verification

cargo test          204 passed, 0 failed
cargo clippy --all-targets --all-features -- -D warnings
cargo fmt --all --check

Clippy is clean apart from two unknown lint errors that are present on
main too — this machine's clippy is older than the pins in mcp.rs.
Confirmed by running clippy on a stashed tree.

The four new tests were checked against broken code. Deleting the caption
arm of split_caption — which restores the old premise exactly — fails
a_captioned_upload_yields_the_caption_and_the_filename and
a_captioned_image_is_carried_as_an_attachment, and leaves the other twelve
green. So they can tell.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SP7njJZ7ZtrMgULuXh8ddq

Closes #106. Three of Julian's messages never reached the agent tonight, each carrying a real instruction. They were found only by dumping the room and grepping for `msgtype != m.text`. ## The reasoning was sound and the premise was wrong `channel.rs` filtered on `MessageType::Text`, deliberately, and said why: > an image or file event also carries a `body` — the sender's chosen filename — > and pushing that as if it were a message misrepresents what arrived. True of an uncaptioned upload. **False of a captioned one** — and Element on a phone puts a photo's caption in `content.body`, which is the traffic that actually arrives here. **The rule was already in this repository.** `mcp.rs:5245`, on the *send* side: > Per Matrix spec: when `filename` ≠ `body`, `body` is the caption. When there is > no caption, use the filename as `body`. Two paths, 4,600 lines apart, holding opposite premises about one field — and the one that implements the spec was not the one the channel used. `split_caption` is that rule applied on the way in. ## What changes `m.image`, `m.file`, `m.audio` and `m.video` are carried as **attachments** rather than dropped: ``` <channel room="!r:…" sender="@julian:…" event="$e" attachment="m.image" filename="Screenshot.png"> <matrix:message trust="external">the /mcp output you asked about</matrix:message> </channel> ``` An agent can now tell *"somebody said this"* from *"somebody sent a file called this"*. `room` and `event` were already in the meta, so `download_attachment` needs nothing new to fetch the bytes. **An uncaptioned upload delivers no prose at all.** Its filename is metadata and travels as an escaped attribute. The original objection was that a filename must not masquerade as a message — labelling it is the fix, not dropping the event. ## Replay changes with it, and that is the half that made this invisible `read_events_from_chunk` fills `untrusted_body` from `content.body` for **any** event that has one. So a caption dropped live *reappeared* on the next attach — hours later, out of order, with nothing marking it as late. That is why this presented as *"Julian's messages are sometimes slow"* rather than *"images are dropped"*. A message that never arrives is noticed; one that arrives late and unlabelled is explained away. `carried_of` is the same classification for the replay path, so the two now agree in **both** directions — including refusing to replay an uncaptioned upload's filename as prose, which is the mistake the live path was avoiding. ## Security A caption is untrusted prose from the same sender as any other message, so it goes through `content_sandbox::evaluate` on both paths, unchanged. Filenames are sender-chosen too and are escaped as meta values rather than trusted for looking like filenames — the existing `meta_values_cannot_break_out_of_the_attribute` test covers that escaping. Nothing new is exposed: no `mxc://`, no new tool, no new surface. ## Verification cargo test 204 passed, 0 failed cargo clippy --all-targets --all-features -- -D warnings cargo fmt --all --check Clippy is clean apart from two `unknown lint` errors that are **present on `main` too** — this machine's clippy is older than the pins in `mcp.rs`. Confirmed by running clippy on a stashed tree. **The four new tests were checked against broken code.** Deleting the caption arm of `split_caption` — which restores the old premise exactly — fails `a_captioned_upload_yields_the_caption_and_the_filename` and `a_captioned_image_is_carried_as_an_attachment`, and leaves the other twelve green. So they can tell. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01SP7njJZ7ZtrMgULuXh8ddq

jlxq0 added 1 commit

2026-08-25 01:52:01 +00:00

fix(channel): carry attachments, and their captions

CI / cargo (pull_request) Successful in 1m18s

Details

CI / docker (pull_request) Has been skipped

Details

6a83b817fe

Closes #106.

Three of Julian's messages never reached the agent, each carrying a real
instruction. They were found only by dumping the room and grepping for
msgtype != m.text.

The live path filtered on MessageType::Text, and the comment gave the
reason: a media event's body is "the sender's chosen filename", so
pushing it as a message would misrepresent what arrived. That is true of
an uncaptioned upload and false of a captioned one — and Element on a
phone puts a photo's caption in content.body, which is the traffic that
actually arrives.

The rule was already in this repository. mcp.rs:5245, on the send side:
"Per Matrix spec: when filename != body, body is the caption. When there
is no caption, use the filename as body." Two paths, opposite premises
about one field, and the one implementing the spec was not the one the
channel used. split_caption is that rule, applied on the way in.

m.image, m.file, m.audio and m.video are now carried as attachments
rather than dropped: the caption is the body, and attachment= and
filename= go in the meta so an agent can tell "somebody said this" from
"somebody sent a file called this". room and event are already there, so
download_attachment needs nothing new to fetch it.

An uncaptioned upload delivers no prose at all. Its filename is metadata
and travels as an escaped attribute — the original objection was that a
filename must not masquerade as a message, and labelling it is the fix
rather than dropping the event.

Replay is changed with it, and that is the half that made this
invisible. read_events_from_chunk fills untrusted_body from content.body
for any event, so a caption dropped live reappeared on the next attach —
hours later, out of order, with nothing marking it as late. That is why
this presented as "Julian's messages are sometimes slow" rather than
"images are dropped". carried_of is the same classification for replay,
so the two paths now agree in both directions.

Captions go through content_sandbox exactly as message bodies do: a
caption is untrusted prose from the same sender, and filenames are
escaped as meta values rather than trusted for looking like filenames.

Verified: 204 tests pass. The four new ones were checked against broken
code — deleting the caption arm of split_caption fails two of them, so
they can tell.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SP7njJZ7ZtrMgULuXh8ddq

jlxq0 referenced this pull request from a commit

2026-08-25 02:08:52 +00:00

fix(channel): require a body in carried_of, so its answer is true

jlxq0 added 1 commit

2026-08-25 02:08:52 +00:00

fix(channel): require a body in carried_of, so its answer is true

CI / docker (pull_request) Has been cancelled

Details

CI / cargo (pull_request) Has been cancelled

Details

48ddc85f4a

From the Codex review of #107, which asked whether the live and replay
classifications can still disagree. They cannot — but it found one place
where carried_of answered a question it had not checked.

An m.text with no content.body was classified as Carried::Message, and
replay then delivered nothing because untrusted_body was absent. Not a
bug in behaviour: ruma will not deserialise such an event, so the live
path never sees one, and replay dropped it a few lines later anyway.

It was a bug in the classifier's honesty. carried_of exists so the two
paths give the same answer for the same event; an answer it has not
verified is the shape of thing that becomes wrong when somebody later
trusts it. body is required for every kind it carries now, including
m.text.

Verified against broken code: making the body lookup fall back to an
empty string fails the new test and leaves the other fourteen green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SP7njJZ7ZtrMgULuXh8ddq

jlxq0 added 1 commit

2026-08-25 02:10:03 +00:00

test(channel): a hostile filename cannot forge an attribute

CI / cargo (pull_request) Successful in 1m20s

Details

CI / docker (pull_request) Has been skipped

Details

a80fa738c1

A filename is chosen by the sender, exactly like a room id, and it now
lands in attribute position on the <channel> tag. The escaping that
protects it is build_params' attr_escape, which already had a test for
room ids — this asserts it for the field this PR adds rather than
assuming the shared path covers it.

Verified against broken code: dropping attr_escape from build_params
fails this test and the room-id one, and leaves the other fourteen
green.

The matching security question went to Codex and its run did not return
within ten minutes, so this is the answer in the form that outlives a
review comment. The caption half needed no new test: a caption goes
through content_sandbox::evaluate on the live path in the same call an
m.text body does, and on replay through read_events_from_chunk, which
wraps content.body for any event that has one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SP7njJZ7ZtrMgULuXh8ddq

jlxq0 merged commit 153e562b8a into main

2026-08-25 02:13:29 +00:00

jlxq0 deleted branch channel-attachments

2026-08-25 02:13:30 +00:00

jlxq0 referenced this pull request from a commit

2026-08-25 02:13:30 +00:00

Merge pull request 'fix(channel): carry attachments, and their captions' (#107) from channel-attachments into main

jlxq0 referenced this pull request from a commit

2026-08-25 02:15:46 +00:00

docs: what a merge to main does, and how to read a cancelled run

jlxq0 referenced this pull request

2026-08-25 02:16:01 +00:00