feat(media): support arbitrary file types with download cards#810
Merged
Conversation
Broaden upload/serve beyond image+video so any file works like Slack: generic files validate against a broad allowlist (docs, pdf, audio, archives, text/code with no magic bytes) and are force-downloaded (Content-Disposition: attachment + nosniff + CSP none). Active-content (html/svg/js) and native executables are denied at upload and neutralized at serve time. Original filename travels client→imeta (the relay stays content-addressed and never learns it). Backend: - sprout-media: generic process_file_upload path beside image/video; broad allowlist + 100MB cap (SPROUT_MAX_FILE_BYTES). - sprout-relay: serve allowlist widened; generic class serves as attachment. Ingest imeta validation widened in all three spots (filename key, MIME check, URL/ext check) so file messages pass. Frontend (desktop): - Composers (chat, DMs, forum/notes) accept any file via paperclip, drag, and paste; filename+mime carried in the imeta tag. - FileCard renderer (icon + name + size + download) for non-media attachments, recognized via a [filename](url) link matched to its imeta entry. Decision logic extracted to a pure, unit-tested resolveFileCard helper. - Generic file chip in ComposerAttachments. Housekeeping: - Extracted the self-contained ffmpeg transcode helpers from media.rs into media_transcode.rs to stay under the desktop line-size limit. Phase 1: generic file cards + download everywhere image upload exists. Inline PDF/audio preview is a planned phase-2 fast-follow. Tests: 253 relay + 37 media + 417 desktop-tauri unit tests; 348 desktop frontend unit tests (incl. 9 new resolveFileCard tests). Typecheck, biome, file-size check, and clippy -D warnings all clean. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Add a Playwright spec that locks in the generic-file frontend contract: paperclip upload -> composer chip with the filename -> send -> a FileCard renders in the timeline with the right name, blob href, and download attr. To make this testable through the mock Tauri bridge: - Add `pick_and_upload_media` + `upload_media_bytes` command handlers returning a BlobDescriptor (configurable via MockBridgeOptions.uploadDescriptors; defaults to a generic PDF). - Thread `mediaTags` through handleSendChannelMessage so imeta tags are echoed onto the stored event — the real relay does this, so it's a fidelity fix (and a prerequisite for any attachment to render in mock mode, image/video included). - Tag the FileCard anchor with data-testid="file-card" for a stable assertion target. - Register file-attachment.spec.ts in the smoke project. Scope note: this guards the UI wiring, not the live store/serve-as-download round-trip — that remains covered by the Rust media + relay tests. Verified: full smoke project (113 e2e) green, 348 frontend unit tests, typecheck, biome, and file-size check all clean. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Collaborator
Author
|
@codex please review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31641d8881
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (type.startsWith("image/")) return `\n`; | ||
| // Generic file: plain link, label is the original filename (fallback to url tail). | ||
| const label = filename || url.split("/").pop() || "file"; | ||
| return `\n[${label}](${url})`; |
There was a problem hiding this comment.
When a generic attachment's filename contains Markdown link delimiters such as ], this line injects it unescaped into the link label. The composer can create such filenames via paperclip/drop/paste, so ReactMarkdown terminates the label early and the URL no longer parses as the attachment link; the file card is lost and edit stripping also won't match. Escape Markdown label characters or use a stable non-user label in the body and keep the display name in imeta.
Useful? React with 👍 / 👎.
Comment on lines
+149
to
+153
| // Non-video path: buffer the body (bounded by the larger of the image | ||
| // and generic-file caps), then decide image-vs-generic by sniffed MIME. | ||
| // Images go through the thumbnailing pipeline; everything else (docs, | ||
| // archives, audio, text, data) takes the generic file path and is | ||
| // served as a download. |
There was a problem hiding this comment.
For API clients that upload a non-MP4 video with an accurate Content-Type such as video/webm or video/quicktime, this new generic-file fallback is never reached because the handler still routes every video/* request through process_video_upload, which rejects anything whose sniffed MIME is not video/mp4. That makes the added generic mappings for downloadable WebM/MOV/MKV unreachable for normal clients; route only video/mp4 to the streaming video path or fall back to process_file_upload after sniffing.
Useful? React with 👍 / 👎.
…g, test fidelity Review of PR #810 (Pinky + Codex) surfaced one regression and several follow-ups; this fixes all of them plus a test blind spot found while adding coverage. - Avatar regression (block): the shared upload path is now generic and no longer rejects non-images server-side, so `useAvatarUpload`'s browser-`file.type` precheck lost its backstop. Add a post-upload check that the server-detected MIME (`uploaded.type`) starts with `image/` before accepting an avatar URL. Covers both picker and drag paths. - Markdown label escaping: `formatImetaMediaLine` now escapes `[`, `]`, and `\` in the filename label so names like `a].pdf` still render as a FileCard instead of breaking the link. Widen `FILE_LINE_RE` to recognise the escaped form so edit-mode stripping still works. - Mock bridge: `resolveMockUploadDescriptors` treats an explicit `uploadDescriptors: []` as a valid override (picker cancel / no files) rather than falling back to the default PDF (`!== undefined`). Test fidelity (found while adding the escaping test): - `imetaMediaMarkdown.test.mjs` inlined stale copies of the functions under test — the inlined `formatImetaMediaLine` had no generic-file branch, so it could never catch a regression there. Root cause: the node test-loader only rewrote `@/` imports to `.ts`, not relative `./` ones, making it impossible to import real source that relative-imports a sibling. Teach the loader to resolve extensionless relative `.ts` imports, then convert the test to import the real module and add escaping cases. - Add onboarding e2e coverage for the avatar guard (non-image descriptor → rejected with error + URL stays empty; image descriptor → accepted) and the `data-testid`s needed to drive it. Tests: 351 desktop frontend unit tests (+3); smoke e2e (118) green; the new avatar e2e cases pass in the integration project. typecheck, biome, and file-size check clean. (Two pre-existing live-relay integration tests fail locally with ECONNREFUSED :3000 — they require a seeded relay and are untouched by this change.) Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co>
…images ForumComposer hand-built outgoing content/imeta, serializing every non-video attachment as `` and omitting the `filename` imeta tag. A PDF/zip/doc posted in a forum or notes channel rendered as a broken inline image and lost its label. Route forum/notes posts through the shared `buildOutgoingMessage` builder, exactly like the chat composer, so generic files become `[filename](url)` links with a `filename` imeta tag. Fixing the emit side alone was insufficient: the forum renderers (ForumPostCard and ForumThreadPanel's post + reply views) never passed `imetaByUrl` to <Markdown>, so FileCard's `resolveFileCard` lookup found no imeta entry and fell back to a plain link. Wire `parseImetaTags` into all three forum render sites, mirroring chat's MessageRow. Add an e2e regression guard that posts a generic-file attachment through the forum composer and asserts a FileCard renders (filename, href, download). It goes red without either half of the fix. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Two DRY cleanups in the media stack, no behavior change: 1. `is_safe_ext` was defined byte-identically in two places — `api/media.rs` (serve/resolve) and `handlers/imeta.rs` (URL validation). This is a security-relevant predicate, so a silent drift between the two copies is a real hazard. Promote the `api::media` definition to `pub(crate)` and have imeta.rs reference it, leaving one source of truth. 2. `process_upload` (image) and `process_file_upload` (generic file) were near-clones: same hash, Blossom auth window, content-addressed key, both-exist idempotency short-circuit, blob store, orphan-blob handling, and descriptor build. They differed only in the validator and the metadata builder. Extract a shared `process_buffered_upload` pipeline parameterized by a `validate` closure (returns mime/ext) and a `store_metadata` closure (receives the already-computed sha256/ext/mime/body via an owned `MetadataInput`, so nothing is recomputed). The streaming `process_video_upload` stays separate because it never buffers the body in RAM. Public signatures of `process_upload` / `process_file_upload` are unchanged; callers and the lib re-export are untouched. cargo test (sprout-media 37, sprout-relay 253) and clippy pass. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
tlongwell-block
pushed a commit
that referenced
this pull request
Jun 1, 2026
* origin/main: chore(release): release version 0.3.6 (#811) feat(mobile): add channel sections with relay sync (#800) feat(desktop): sync channel sections across devices via Nostr (#792) feat(media): support arbitrary file types with download cards (#810) feat(desktop): add user-defined channel sections to sidebar (#789) Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> # Conflicts: # desktop/src-tauri/src/commands/mod.rs
wesbillman
added a commit
that referenced
this pull request
Jun 3, 2026
Two regressions in message media handling: 1. Arbitrary file downloads (FileCard, added in #810) used a bare `<a href download>`. In the Tauri webview that navigates to the blob URL, escaping to the OS browser and landing on the Block CDN "browser not supported" interstitial. Add a native `download_file` Tauri command mirroring `download_image`'s SSRF `/media/`-origin validation, 50 MiB streaming cap, and MIME blocklist, but with a generic save dialog and a sanitized imeta filename. FileCard is now a `<button>` that invokes it, keeping the fetch inside the app's HTTP tunnel. 2. The image right-click "Download Image" menu flashed and vanished: the opening contextmenu/click also drove the Radix Dialog trigger and the menu's own global dismiss listeners, tearing it down immediately. Defer attaching the dismiss listeners by one tick, stop the contextmenu during capture with stopImmediatePropagation, and preventDefault non-left pointerdown on the dialog trigger so the menu survives the interaction that opened it. Refactor `download_image` to share a `fetch_blob_bytes` helper with the new command. Update the e2e mock bridge and file-attachment specs to assert clicking a file card invokes `download_file`. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Support arbitrary file types (Slack-style) wherever image upload exists today: generic file upload + a download-only
FileCardin chat, DMs, and forum/notes composers.What's in here
Backend
sprout-media: genericprocess_file_uploadpath beside image/video — broad allowlist (docs, pdf, audio, archives, text/code with no magic bytes), 100MB cap (SPROUT_MAX_FILE_BYTES).sprout-relay: serve allowlist widened; generic class served asContent-Disposition: attachment+ nosniff + CSP none. Active-content (html/svg/js) and native executables denied at upload and neutralized at serve. Ingest imeta validation widened in all three spots so file messages pass the gate.Frontend (desktop)
filename+mimecarried in the imeta tag.FileCardrenderer (icon + name + size + download), recognized via a[filename](url)link matched to its imeta entry. Decision logic extracted to a pure, unit-testedresolveFileCardhelper.ComposerAttachments.Housekeeping
media.rsintomedia_transcode.rsto stay under the desktop line-size limit.Tests
resolveFileCardtests).FileCardrenders with name, blob href, download attr. Full smoke project (113 e2e) green.-D warningsall clean.Scope notes