feat(bridges): foreign-avatar proxy (LC-78-AVATAR-PROXY, on by default) #245

Merged

longjacksonle merged 6 commits from feat/lc-78-avatar-proxy into main 2026-05-28 05:15:56 +02:00

Conversation 0 Commits 6 Files changed 41 +1742 -39

longjacksonle commented 2026-05-28 05:15:31 +02:00

Owner

Copy link

Summary

LC-78-AVATAR-PROXY closes the v1 cliff where bridge messages rendered as initials. The server now fetches each foreign avatar URL exactly once, caches the bytes on disk, and serves them from a same-origin proxy URL. Viewer browsers never hit foreign homeservers; the foreign URL never appears in rendered HTML.

Stacked on #243 (LC-78 v1). Branch was cut from feat/lc-78-bridge-registration-surface; rebase onto main once #243 lands.

Behavior change on upgrade (read first)

The foreign-avatar proxy is ENABLED BY DEFAULT in v2. Operators who want to keep v1's reject-non-null posture must set LETS_CHAT_BRIDGE_AVATAR_PROXY_ENABLED=false BEFORE upgrading. The gate is checked per-request, so the flag works without a restart, but the behavior change applies from the first request the upgraded binary serves.

Daemons that observed v1's 400 may now start submitting foreign_avatar URLs successfully. If you bumped a daemon's config to omit foreign_avatar based on v1's reject, the server upgrade alone enables avatars; if your daemon still skips the field, no behavior changes until you reconfigure.

What ships

• New schema (migrations/chat/0056_bridge_avatar_proxies.sql): single table keyed by sha256 of the canonical foreign URL, with (hash, foreign_url, content_type, byte_size, fetched_at, last_seen_at, fetch_status, failure_reason). Disk files live in {data_dir}/bridge-avatars/{hash}, no extension (Content-Type comes from the row).
• Schema choice C (named in plan): hash, not URL, lives on the message row. The structural side-channel closure is that reading the HTML reveals nothing about which homeservers the room bridges with -- only opaque hashes.
• Fetch module (server/src/bridge_avatar.rs): reqwest GET with 5s timeout, 1 MiB byte cap (streamed reject mid-stream), magic-byte sniff via infer::get_from_path (Content-Type header is foreign-controlled and ignored), re-encode through the uploads pipeline (strips EXIF / XMP / IPTC / PNG text chunks; trust-posture parity with user uploads), atomic write via temp + rename. spawn_blocking isolates the sync decoder + any panic.
• POST endpoint flip: POST /api/v1/bridges/{id}/messages accepts foreign_avatar (was a v1 400-reject), validates URL shape + scheme + length, SSRF re-resolves at submit time, upserts the cache row, fires the fire-and-forget fetch task. Pre-flight failures (bad URL, private host, too-long URL, gate off) are fail-loud 400s.
• GET proxy endpoint: GET /media/bridge-avatar-proxy/{hash} is AuthUser-gated. Threat-model framing (per the plan's sharpening #2): the gate prevents anonymous fetches of leaked hashes and leaves an audit trail; it does NOT enforce room-of-origin scoping (that comes from the rendering surface controlling who sees the hash in the first place). Serves bytes with Content-Type from the row + Cache-Control: public, max-age=31536000, immutable (content-addressed, safe to long-cache).
• Render template: bridge arm renders <img src=/media/bridge-avatar-proxy/{hash}> with data-fallback + onerror swap to initials. A pending / failed fetch (proxy 404) degrades gracefully without a page refresh.
• GC sweep: piggybacks on the hourly orphan tick. Rows whose last_seen_at is older than 30 days AND no live messages.bridge_foreign_avatar references them get deleted (row + disk file together). Separate sweep marks pending rows older than 10 minutes as failed (crash recovery from a process restart mid-fetch).
• Operator gate: LETS_CHAT_BRIDGE_AVATAR_PROXY_ENABLED env var. Default true. Setting to false / 0 / no / off restores v1's reject posture (POST 400, GET 404 every hash with no fingerprinting between "feature off" and "hash unknown").

Pre-execution audits (per plan)

LC-152 SSRF audit (sharpening #1). ssrf::host_resolves_public does a real DNS re-resolve via tokio::net::lookup_host, rejects dual-record [public, private] answers, and runs at both submit time (the new endpoint validates here too) and fetch time (the fetch module). Residual TOCTOU window between this check and reqwest's own DNS call exists and is shared with the LC-75 outgoing-webhook delivery path; closing it requires a custom reqwest DNS resolver that pins the resolved IP. Not in scope here; flagged as a follow-up that affects both LC-75 and LC-78 deliveries.

Image-decoder spike (sharpening #4). server/tests/spike_image_decoder_hostile_corpus.rs runs 12 hostile inputs through image 0.25's decode path wrapped in catch_unwind: empty bytes, random bytes, format-signature-only for PNG/JPEG/GIF/WebP, format-signature-plus-garbage, truncated-after-IHDR, pixel-bomb dimension overflow (65535x65535), and 1 MiB of random bytes. All 12 pass: no panics at the 1 MiB cap. Production fetch wraps the decode in spawn_blocking for belt-and-suspenders panic isolation against a future image-crate bump that might regress.

Threat-model alignment

The plan's sharpening #2 reframed the auth gate honestly: it prevents anonymous fetches of leaked hashes and adds an audit trail, but does NOT enforce room-of-origin scoping. That framing is now in the route module's doc comment and reflected in docs/protocol-bridges.md.

Sharpening #3 (default-on flip is a behavior change) is named prominently at the top of this PR description and in the protocol-bridges doc.

Test plan

• Decoder spike: 12/12 pass at 1 MiB cap.
• Storage round-trip: 5/5 in db_bridge_avatar_proxies (upsert idempotency, mark_ok / mark_failed, sweep unreferenced, sweep pending orphans).
• Fetch module round-trip: 5/5 in bridge_avatar_fetch (local-receiver happy path with bytes round-trip through re-encode, HTTP 404 -> failed, Content-Type lie -> failed via magic-byte sniff, 2 MiB payload -> failed at byte cap, canonical_hash determinism).
• POST endpoint v2 contract: 9/9 in routes_api_bridge_messages (flipped + extended; existing v1 reject test renamed bridge_post_with_invalid_foreign_avatar_url_is_400 + new bridge_post_with_private_resolving_foreign_avatar_is_400 for SSRF rejection + new bridge_post_with_foreign_avatar_stores_hash_v2 for the happy path).
• Proxy GET: 5/5 in routes_media_bridge_avatar_proxy (anonymous denied, unknown hash 404, pending row 404, malformed hash 404, ok row serves bytes with Content-Type + immutable Cache-Control).
• Gate-off mode: 1/1 in routes_media_bridge_avatar_proxy_gate_off (separate binary so the process-global env var does not race other tests).
• just test clean (120 binaries pass, 0 failed).
• just test-saas clean (111 binaries pass, 0 failed; standalone-gated tests correctly skipped).
• Phase-24 Category 2 migration-list drift swept for migration 0056: 17 array-form + 1 verbose-form test files brought up to chat/0056.

Deferred (and why)

Deferred	Why
Re-fetch policy (refresh stale cache)	v2 never re-fetches. A daemon submitting the same URL again finds the existing cached row + updated last_seen_at; the bytes don't change. Adding a refresh button is a future LC-78-AVATAR-REFRESH.
Failed-fetch retry-after	failed is terminal. Render falls back to initials forever. Avoids repeated outbound traffic to dead homeservers; an operator who wants to retry clears the row manually.
Foreign-host allowlist	Operator trusts the daemon (operator-controlled) to send sensible URLs. SSRF + size + sniff + re-encode bound the damage of an unsanitized daemon. Allowlist is a follow-up if real abuse cases emerge.
Live re-render after async fetch	A bridge message broadcasts with initials if fetch hasn't completed by render time. Next page reload (or <img> retry on a subsequent navigation) shows the avatar. Adding a WS re-swap is polish; nil functional improvement.
Closing the LC-152 DNS-rebinding TOCTOU	Affects both LC-75 outgoing webhooks AND this new fetch path. Out of scope here; would land as a shared crate::http_client helper that pins the resolved IP through a custom reqwest DNS resolver.
Email-ingress (LC-77) outgoing-webhook coverage	Still a pre-existing gap (finalize_email_inbox_message_send doesn't enqueue LC-75 events). Bridges that subscribe via LC-75 still don't see email-authored messages. Tracked separately.

Verifying as the operator

1. Upgrade. Check /admin/bridges -- existing bridges keep working with no avatar (initials).
2. Configure your daemon to submit foreign_avatar in POST /api/v1/bridges/{id}/messages. Existing messages don't backfill; new messages start showing avatars.
3. Watch /data/bridge-avatars/ for files. Pending rows are 0-byte before the first fetch; size grows after.
4. To revert to v1's reject posture: LETS_CHAT_BRIDGE_AVATAR_PROXY_ENABLED=false in the env, no restart needed.

## Summary LC-78-AVATAR-PROXY closes the v1 cliff where bridge messages rendered as initials. The server now fetches each foreign avatar URL exactly once, caches the bytes on disk, and serves them from a same-origin proxy URL. Viewer browsers never hit foreign homeservers; the foreign URL never appears in rendered HTML. **Stacked on #243 (LC-78 v1).** Branch was cut from `feat/lc-78-bridge-registration-surface`; rebase onto main once #243 lands. ## Behavior change on upgrade (read first) > **The foreign-avatar proxy is ENABLED BY DEFAULT in v2.** Operators who want to keep v1's reject-non-null posture must set `LETS_CHAT_BRIDGE_AVATAR_PROXY_ENABLED=false` BEFORE upgrading. The gate is checked per-request, so the flag works without a restart, but the behavior change applies from the first request the upgraded binary serves. > > Daemons that observed v1's 400 may now start submitting `foreign_avatar` URLs successfully. If you bumped a daemon's config to omit `foreign_avatar` based on v1's reject, the server upgrade alone enables avatars; if your daemon still skips the field, no behavior changes until you reconfigure. ## What ships - **New schema** (`migrations/chat/0056_bridge_avatar_proxies.sql`): single table keyed by sha256 of the canonical foreign URL, with `(hash, foreign_url, content_type, byte_size, fetched_at, last_seen_at, fetch_status, failure_reason)`. Disk files live in `{data_dir}/bridge-avatars/{hash}`, no extension (Content-Type comes from the row). - **Schema choice C** (named in plan): hash, not URL, lives on the message row. The structural side-channel closure is that reading the HTML reveals nothing about which homeservers the room bridges with -- only opaque hashes. - **Fetch module** (`server/src/bridge_avatar.rs`): reqwest GET with 5s timeout, 1 MiB byte cap (streamed reject mid-stream), magic-byte sniff via `infer::get_from_path` (Content-Type header is foreign-controlled and ignored), re-encode through the uploads pipeline (strips EXIF / XMP / IPTC / PNG text chunks; trust-posture parity with user uploads), atomic write via temp + rename. `spawn_blocking` isolates the sync decoder + any panic. - **POST endpoint flip**: `POST /api/v1/bridges/{id}/messages` accepts `foreign_avatar` (was a v1 400-reject), validates URL shape + scheme + length, SSRF re-resolves at submit time, upserts the cache row, fires the fire-and-forget fetch task. Pre-flight failures (bad URL, private host, too-long URL, gate off) are fail-loud 400s. - **GET proxy endpoint**: `GET /media/bridge-avatar-proxy/{hash}` is AuthUser-gated. Threat-model framing (per the plan's sharpening #2): the gate prevents anonymous fetches of leaked hashes and leaves an audit trail; it does NOT enforce room-of-origin scoping (that comes from the rendering surface controlling who sees the hash in the first place). Serves bytes with `Content-Type` from the row + `Cache-Control: public, max-age=31536000, immutable` (content-addressed, safe to long-cache). - **Render template**: bridge arm renders `<img src=/media/bridge-avatar-proxy/{hash}>` with `data-fallback` + `onerror` swap to initials. A pending / failed fetch (proxy 404) degrades gracefully without a page refresh. - **GC sweep**: piggybacks on the hourly orphan tick. Rows whose `last_seen_at` is older than 30 days AND no live `messages.bridge_foreign_avatar` references them get deleted (row + disk file together). Separate sweep marks `pending` rows older than 10 minutes as `failed` (crash recovery from a process restart mid-fetch). - **Operator gate**: `LETS_CHAT_BRIDGE_AVATAR_PROXY_ENABLED` env var. Default true. Setting to `false` / `0` / `no` / `off` restores v1's reject posture (POST 400, GET 404 every hash with no fingerprinting between "feature off" and "hash unknown"). ## Pre-execution audits (per plan) **LC-152 SSRF audit (sharpening #1).** `ssrf::host_resolves_public` does a real DNS re-resolve via `tokio::net::lookup_host`, rejects dual-record `[public, private]` answers, and runs at both submit time (the new endpoint validates here too) and fetch time (the fetch module). Residual TOCTOU window between this check and reqwest's own DNS call exists and is shared with the LC-75 outgoing-webhook delivery path; closing it requires a custom reqwest DNS resolver that pins the resolved IP. **Not in scope here; flagged as a follow-up that affects both LC-75 and LC-78 deliveries.** **Image-decoder spike (sharpening #4).** `server/tests/spike_image_decoder_hostile_corpus.rs` runs 12 hostile inputs through `image 0.25`'s decode path wrapped in `catch_unwind`: empty bytes, random bytes, format-signature-only for PNG/JPEG/GIF/WebP, format-signature-plus-garbage, truncated-after-IHDR, pixel-bomb dimension overflow (65535x65535), and 1 MiB of random bytes. **All 12 pass: no panics at the 1 MiB cap.** Production fetch wraps the decode in `spawn_blocking` for belt-and-suspenders panic isolation against a future image-crate bump that might regress. ## Threat-model alignment The plan's sharpening #2 reframed the auth gate honestly: it prevents anonymous fetches of leaked hashes and adds an audit trail, but does NOT enforce room-of-origin scoping. That framing is now in the route module's doc comment and reflected in `docs/protocol-bridges.md`. Sharpening #3 (default-on flip is a behavior change) is named prominently at the top of this PR description and in the protocol-bridges doc. ## Test plan - [x] Decoder spike: 12/12 pass at 1 MiB cap. - [x] Storage round-trip: 5/5 in `db_bridge_avatar_proxies` (upsert idempotency, mark_ok / mark_failed, sweep unreferenced, sweep pending orphans). - [x] Fetch module round-trip: 5/5 in `bridge_avatar_fetch` (local-receiver happy path with bytes round-trip through re-encode, HTTP 404 -> failed, Content-Type lie -> failed via magic-byte sniff, 2 MiB payload -> failed at byte cap, canonical_hash determinism). - [x] POST endpoint v2 contract: 9/9 in `routes_api_bridge_messages` (flipped + extended; existing v1 reject test renamed `bridge_post_with_invalid_foreign_avatar_url_is_400` + new `bridge_post_with_private_resolving_foreign_avatar_is_400` for SSRF rejection + new `bridge_post_with_foreign_avatar_stores_hash_v2` for the happy path). - [x] Proxy GET: 5/5 in `routes_media_bridge_avatar_proxy` (anonymous denied, unknown hash 404, pending row 404, malformed hash 404, ok row serves bytes with `Content-Type` + immutable `Cache-Control`). - [x] Gate-off mode: 1/1 in `routes_media_bridge_avatar_proxy_gate_off` (separate binary so the process-global env var does not race other tests). - [x] `just test` clean (120 binaries pass, 0 failed). - [x] `just test-saas` clean (111 binaries pass, 0 failed; standalone-gated tests correctly skipped). - [x] Phase-24 Category 2 migration-list drift swept for migration 0056: 17 array-form + 1 verbose-form test files brought up to chat/0056. ## Deferred (and why) | Deferred | Why | |---|---| | Re-fetch policy (refresh stale cache) | v2 never re-fetches. A daemon submitting the same URL again finds the existing cached row + updated `last_seen_at`; the bytes don't change. Adding a refresh button is a future LC-78-AVATAR-REFRESH. | | Failed-fetch retry-after | `failed` is terminal. Render falls back to initials forever. Avoids repeated outbound traffic to dead homeservers; an operator who wants to retry clears the row manually. | | Foreign-host allowlist | Operator trusts the daemon (operator-controlled) to send sensible URLs. SSRF + size + sniff + re-encode bound the damage of an unsanitized daemon. Allowlist is a follow-up if real abuse cases emerge. | | Live re-render after async fetch | A bridge message broadcasts with initials if fetch hasn't completed by render time. Next page reload (or `<img>` retry on a subsequent navigation) shows the avatar. Adding a WS re-swap is polish; nil functional improvement. | | Closing the LC-152 DNS-rebinding TOCTOU | Affects both LC-75 outgoing webhooks AND this new fetch path. Out of scope here; would land as a shared `crate::http_client` helper that pins the resolved IP through a custom reqwest DNS resolver. | | Email-ingress (LC-77) outgoing-webhook coverage | Still a pre-existing gap (`finalize_email_inbox_message_send` doesn't enqueue LC-75 events). Bridges that subscribe via LC-75 still don't see email-authored messages. Tracked separately. | ## Verifying as the operator 1. Upgrade. Check `/admin/bridges` -- existing bridges keep working with no avatar (initials). 2. Configure your daemon to submit `foreign_avatar` in `POST /api/v1/bridges/{id}/messages`. Existing messages don't backfill; new messages start showing avatars. 3. Watch `/data/bridge-avatars/` for files. Pending rows are 0-byte before the first fetch; size grows after. 4. To revert to v1's reject posture: `LETS_CHAT_BRIDGE_AVATAR_PROXY_ENABLED=false` in the env, no restart needed.

longjacksonle added 6 commits 2026-05-28 05:15:31 +02:00

schema migration + db helpers 37f86a9355

LC-152 audit + image-decoder spike + fetch module e77490a323

flip POST endpoint + new GET proxy + render template 504cd821e2

flip POST endpoint + new GET proxy + render template 446e3263f0

tests across the 6 surfaces fce69ebf71

docs: name the proxied-avatar threat model in v2 ... 40c4fdc980

The threat-model section still described v1's reject-foreign-avatar
posture even though the scope table got the v2 update. Align the bullet
with the v2 design: SSRF + sniff + re-encode pipeline, residual TOCTOU
window shared with LC-75, decoder spike + spawn_blocking panic isolation,
operator escape via LETS_CHAT_BRIDGE_AVATAR_PROXY_ENABLED=false.

longjacksonle merged commit a843a3a89a into main 2026-05-28 05:15:56 +02:00

longjacksonle deleted branch feat/lc-78-avatar-proxy 2026-05-28 05:15:56 +02:00

longjacksonle referenced this pull request from a commit 2026-05-28 05:15:56 +02:00

Merge pull request 'feat(bridges): foreign-avatar proxy (LC-78-AVATAR-PROXY, on by default)' (#245) from feat/lc-78-avatar-proxy into main

nrupard referenced this pull request from a commit 2026-05-28 16:21:01 +02:00

style: rustfmt the LC-78 / LC-152 bridge files (LC-187)

nrupard referenced this pull request 2026-05-28 16:21:14 +02:00

style: rustfmt the LC-78/LC-152 bridge files (LC-187) #250

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

psa-systems/lets-chat!245

No description provided.