Fix macOS attachment picker lifecycle and allow inert HTML downloads (#5569)

## Problem

Canceling the native macOS file chooser leaves the composer's temporary,
detached `<input type="file">` without a `change` event or an explicit
cleanup path. Opening Finder again immediately creates a second detached
input while WebKit may still be unwinding the first picker. The newly
selected files can therefore fail to reach the upload pipeline. Drag and
drop is unaffected because it bypasses this picker lifecycle.

This does **not** add an automatic retry mechanism. “Retry” means the
user's next attachment attempt after canceling or after a prior
selection.

## Fix

- give each composer hook one hidden, body-mounted file input for its
lifetime instead of creating a detached one per click
- reset and reconfigure that input before every open, replace its
handler rather than stacking handlers, and remove it cleanly on unmount
- preserve normal selection, cancel then reopen, selecting the same file
again, and multi-select behavior
- accept canonical `text/html` attachments while continuing to serve and
render them strictly as inert downloads
- keep XHTML, SVG, JavaScript, and executable MIME types blocked

The picker change fixes the ownership/lifecycle bug at its source; it
does not retry failed uploads, add delays, or mask errors.

## Testing

- mandatory pre-push gate: branch-skew, desktop typecheck/tests/check,
Rust tests, and desktop Tauri checks passed on
`ea5a97adf957803935b28d63d32f9f332cf65287`
- `cargo test -p buzz-media --lib` (110 passed)
- `pnpm --dir desktop typecheck`
- focused Biome check for the three picker files
- picker Playwright regression: cancel/no selection then reopen, select
the same file again, and multiple selection (run on the source commit
before integration)
- HTML live-relay response regression added as ignored E2E because it
requires the S3-backed relay harness

## Manual verification

Playwright models cancellation with Chromium's
`FileChooser.setFiles([])`; it cannot exercise the native macOS Finder
panel/WebKit presentation lifecycle. Before merge, manually verify in
the built macOS app:

1. select a PNG normally
2. cancel, then immediately reopen and select a PNG
3. select the same PNG on a subsequent attempt
4. multi-select two PNGs

---------

Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: Princess Donut <68157ebd23b3897c1991015c3038658ea916200c67d3a54620b0754d1b92f6e0@buzz.block.builderlab.xyz>
Co-authored-by: Mongo <5c25403eab7271f9f94ddd4f2b270e8cac2c92e2c830c51877cca6ec974ffb3f@buzz.block.builderlab.xyz>
Co-authored-by: Carl <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
This commit is contained in:
Wes
2026-08-11 09:51:42 -07:00
committed by GitHub
co-authored by Princess Donut Mongo Carl
parent d3ec831e0c
commit bba3e06386
6 changed files with 276 additions and 26 deletions
+73 -7
View File
@@ -69,12 +69,23 @@ pub(crate) fn looks_like_mp4_iso_bmff(bytes: &[u8]) -> bool {
/// neutralises them — this allowlist-of-denials is defence in depth, so a future
/// header regression can't turn an uploaded blob into a stored-XSS vector.
///
/// HTML, JS, and SVG are the classic stored-XSS carriers. Native executables are
/// JS and SVG are the classic stored-XSS carriers. Native executables are
/// blocked because there's no legitimate reason to host them inline in chat and
/// they're a malware-distribution risk.
///
/// HTML is intentionally *not* blocked: it is accepted as an inert download
/// (`serve_inline` returns false for `text/html`, so it is served with
/// `Content-Disposition: attachment` + `nosniff` + `CSP: default-src 'none'`,
/// and the desktop renderer never navigates a webview to a generic
/// attachment). The old sniff-based block only caught the well-formed HTML
/// `infer` recognises anyway — HTML that evades the sniff already uploaded as
/// `application/octet-stream` and served as a download, so blocking canonical
/// HTML was inconsistent rather than a real control. `application/xhtml+xml`
/// stays listed as dormant defence in depth: `infer` has no XHTML matcher, so
/// it is unreachable through sniffing, but the entry costs nothing and guards
/// against a future detector that does classify it.
const BLOCKED_FILE_MIME_TYPES: &[&str] = &[
// Active web content — stored-XSS vectors.
"text/html",
"application/xhtml+xml",
"image/svg+xml",
"application/javascript",
@@ -135,6 +146,7 @@ fn file_mime_to_ext(mime: &str) -> Option<&'static str> {
// Data / text
"application/json" => "json",
"text/csv" => "csv",
"text/html" => "html",
"text/plain" => "txt",
_ => return None,
};
@@ -2586,17 +2598,71 @@ mod tests {
}
#[test]
fn test_validate_file_html_rejected() {
// HTML is a stored-XSS carrier — blocked even though headers neutralise it.
fn test_validate_file_html_accepted_as_inert_download() {
// HTML is accepted on the generic file path as an inert attachment.
// `infer` recognises canonical HTML as `text/html`; it must map to the
// `html` extension and, crucially, NOT be served inline — the serve
// layer relies on `serve_inline("text/html") == false` to attach a
// `Content-Disposition: attachment` + `nosniff` + restrictive CSP,
// which is what keeps the payload from ever executing.
let config = test_config();
let html = b"<!DOCTYPE html><html><body><script>alert(1)</script></body></html>";
let result = validate_file_content(html, &config);
// Sanity: this fixture is exactly the shape `infer` classifies as HTML.
assert_eq!(infer::get(html).map(|k| k.mime_type()), Some("text/html"));
let (mime, ext) = validate_file_content(html, &config).unwrap();
assert_eq!(mime, "text/html");
assert_eq!(ext, "html");
assert!(
matches!(result, Err(MediaError::DisallowedContentType(ref m)) if m == "text/html"),
"expected DisallowedContentType(text/html), got {result:?}"
!serve_inline(&mime),
"text/html must never be served inline — it must force download"
);
}
#[test]
fn test_validate_file_executable_still_rejected() {
// Removing HTML from the deny-list must not weaken the executable
// block. `infer` classifies an ELF header as `application/x-executable`,
// which the generic path must still reject via the deny-list.
let config = test_config();
// `infer`'s ELF matcher requires the magic plus >52 bytes of header.
let mut elf = b"\x7fELF".to_vec();
elf.extend_from_slice(&[0u8; 60]);
assert_eq!(
infer::get(&elf).map(|k| k.mime_type()),
Some("application/x-executable")
);
assert!(
matches!(validate_file_content(&elf, &config), Err(MediaError::DisallowedContentType(ref m)) if m == "application/x-executable"),
"ELF executable must still be rejected by the generic file path"
);
}
#[test]
fn test_generic_deny_list_keeps_active_content_and_executables() {
// Static guard on the deny-list itself: HTML is intentionally gone, but
// SVG, JavaScript, XHTML, and the native-executable types remain. These
// are the entries that keep the inert-download boundary honest even if a
// future `infer` upgrade starts classifying more of them by content.
assert!(!BLOCKED_FILE_MIME_TYPES.contains(&"text/html"));
for kept in [
"image/svg+xml",
"application/xhtml+xml",
"application/javascript",
"text/javascript",
"application/x-msdownload",
"application/x-executable",
"application/vnd.microsoft.portable-executable",
"application/x-mach-binary",
"application/x-msi",
"application/x-apple-diskimage",
] {
assert!(
BLOCKED_FILE_MIME_TYPES.contains(&kept),
"{kept} must remain in the generic-file deny-list"
);
}
}
#[test]
fn test_validate_file_too_large_rejected() {
let mut config = test_config();
@@ -423,6 +423,72 @@ async fn test_upload_svg_accepted_as_text_xml() {
println!("✅ SVG (XML declaration) → 200 as text/xml");
}
#[tokio::test]
#[ignore]
async fn test_upload_html_served_as_inert_attachment() {
// HTML is accepted on the generic file path and MUST be served as an inert
// download: the security property the whole feature relies on is that the
// relay returns `Content-Disposition: attachment` + `X-Content-Type-Options:
// nosniff` + `Content-Security-Policy: default-src 'none'` so the payload can
// never execute or render as active content. This response-level regression
// pins that end to end (upload → GET), not just the deny-list membership.
let client = http_client();
let keys = Keys::generate();
// Exactly the shape `infer` classifies as text/html (leading recognised tag).
let html = b"<!DOCTYPE html><html><body><script>alert(1)</script></body></html>";
let resp = upload(&client, &keys, html).await;
let status = resp.status().as_u16();
assert_eq!(
status, 200,
"HTML should upload via file path, got {status}"
);
let desc: serde_json::Value = resp.json().await.unwrap();
assert_eq!(desc["type"].as_str().unwrap(), "text/html");
let url = desc["url"].as_str().unwrap();
assert!(
url.ends_with(".html"),
"served URL must carry the .html extension, got {url}"
);
let sha256 = desc["sha256"].as_str().unwrap();
let get_resp = client
.get(url)
.header(
"Authorization",
blossom_auth_header(&sign_blossom_get_auth(&keys, sha256)),
)
.send()
.await
.expect("GET request");
assert_eq!(get_resp.status(), 200, "HTML GET roundtrip should succeed");
let header = |name: &str| {
get_resp
.headers()
.get(name)
.and_then(|v| v.to_str().ok())
.unwrap_or("")
.to_string()
};
assert_eq!(header("content-type"), "text/html");
assert_eq!(
header("content-disposition"),
"attachment",
"HTML must be forced to download, never rendered inline"
);
assert_eq!(
header("x-content-type-options"),
"nosniff",
"nosniff must prevent MIME re-sniffing to an executable type"
);
assert_eq!(
header("content-security-policy"),
"default-src 'none'",
"restrictive CSP must neutralise any active content"
);
println!("✅ HTML → 200, served as inert attachment (disposition+nosniff+CSP)");
}
#[tokio::test]
#[ignore]
async fn test_upload_pdf_accepted() {
+25 -6
View File
@@ -115,11 +115,10 @@ fn fd_real_path(_file: &std::fs::File) -> Result<std::path::PathBuf, String> {
/// MIME types blocked from upload — mirrors the server's generic-file deny-list.
///
/// Active-content XSS carriers and native executables. Everything else (images,
/// video, documents, archives, audio, text, data) is accepted; un-sniffable
/// files fall back to `application/octet-stream` and are served as downloads.
/// Active-content XSS carriers (JS, SVG) and native executables. Other types,
/// including HTML, are accepted as downloads; un-sniffable files fall back to
/// `application/octet-stream`. XHTML remains blocked in lockstep with the relay.
const BLOCKED_MIME: &[&str] = &[
"text/html",
"application/xhtml+xml",
"image/svg+xml",
"application/javascript",
@@ -895,9 +894,29 @@ mod tests {
}
#[test]
fn test_detect_and_validate_mime_rejects_html() {
fn test_detect_and_validate_mime_accepts_html_as_inert_download() {
let html = b"<!DOCTYPE html><html><body><script>alert(1)</script></body></html>";
assert!(detect_and_validate_mime(html).is_err());
assert_eq!(detect_and_validate_mime(html).unwrap(), "text/html");
}
#[test]
fn test_detect_and_validate_mime_still_rejects_executable() {
let elf = [b"\x7fELF".as_slice(), &[0u8; 60]].concat();
assert!(detect_and_validate_mime(&elf).is_err());
}
#[test]
fn test_blocked_mime_keeps_active_content_and_executables() {
for kept in [
"image/svg+xml",
"application/xhtml+xml",
"application/javascript",
"text/javascript",
"application/x-executable",
"application/x-mach-binary",
] {
assert!(BLOCKED_MIME.contains(&kept), "{kept} must stay blocked");
}
}
#[test]
@@ -0,0 +1,54 @@
import * as React from "react";
type FilePickerOptions = {
accept?: string;
multiple?: boolean;
};
/**
* Owns one mounted file input for the hook lifetime. Reusing the node avoids
* detached-input presentation races when a native picker is canceled and
* immediately reopened.
*/
export function useFilePicker() {
const inputRef = React.useRef<HTMLInputElement | null>(null);
React.useEffect(
() => () => {
const input = inputRef.current;
if (input) {
input.onchange = null;
input.remove();
}
inputRef.current = null;
},
[],
);
return React.useCallback(
(options: FilePickerOptions, onFiles: (files: File[]) => void) => {
let input = inputRef.current;
if (!input) {
input = document.createElement("input");
input.type = "file";
input.hidden = true;
document.body.append(input);
inputRef.current = input;
}
// Cancel emits no `change`, so replace rather than stack callbacks. Reset
// before opening (and after selection) to permit choosing the same file.
input.accept = options.accept ?? "";
input.multiple = options.multiple ?? false;
input.value = "";
input.onchange = (event) => {
const currentInput = event.currentTarget as HTMLInputElement;
const files = Array.from(currentInput.files ?? []);
currentInput.value = "";
onFiles(files);
};
input.click();
},
[],
);
}
@@ -8,6 +8,7 @@ import {
import { uploadMediaFile } from "@/shared/api/tauriMedia";
import type { QueuedMediaAttachment } from "./backgroundMediaUploadStore";
import { applyImetaUpdate, compactImetaSlots } from "./imetaSlots";
import { useFilePicker } from "./useFilePicker";
import { isVideoFile, videoMimeForFile } from "./videoFileType";
/**
@@ -617,21 +618,14 @@ export function useMediaUpload({
[fillSlot, onUploadError, reserveSlots, reserveUploadingPreview],
);
const openFilePicker = useFilePicker();
const handlePaperclip = React.useCallback(async () => {
if (queueUntilSend) {
const input = document.createElement("input");
input.type = "file";
input.multiple = true;
input.addEventListener(
"change",
() => {
const files = Array.from(input.files ?? []);
queueFiles(files.filter(shouldQueueFile));
uploadFiles(files.filter((file) => !shouldQueueFile(file)));
},
{ once: true },
);
input.click();
openFilePicker({ multiple: true }, (files) => {
queueFiles(files.filter(shouldQueueFile));
uploadFiles(files.filter((file) => !shouldQueueFile(file)));
});
return;
}
@@ -661,6 +655,7 @@ export function useMediaUpload({
isUploadCanceled,
isUploadStale,
onUploadError,
openFilePicker,
queueFiles,
reserveUploadingPreview,
shouldQueueFile,
+50
View File
@@ -72,6 +72,56 @@ async function choosePhoto(page: Page) {
});
}
const PHOTO_FILE = {
buffer: Buffer.from("photo"),
mimeType: "image/png",
name: "photo.png",
};
async function uploadCommandCount(page: Page) {
return page.evaluate(
() =>
(
(window as Window & { __BUZZ_E2E_COMMANDS__?: string[] })
.__BUZZ_E2E_COMMANDS__ ?? []
).filter((command) => command === "upload_media_bytes_raw").length,
);
}
test("picker survives cancel, same-file retry, and multiple selection", async ({
page,
}) => {
await page.goto("/");
await page.getByTestId("channel-general").click();
const attach = page.getByRole("button", { name: "Attach file" });
// Model cancel/no selection, then immediately reopen. The composer must
// reuse its one mounted input rather than creating competing detached ones.
const [canceledChooser] = await Promise.all([
page.waitForEvent("filechooser"),
attach.click(),
]);
await canceledChooser.setFiles([]);
await choosePhoto(page);
await expect.poll(() => uploadCommandCount(page)).toBe(1);
// Reset-before-open is load-bearing: without it browsers suppress `change`
// when the same path remains selected.
await choosePhoto(page);
await expect.poll(() => uploadCommandCount(page)).toBe(2);
const [multipleChooser] = await Promise.all([
page.waitForEvent("filechooser"),
attach.click(),
]);
await multipleChooser.setFiles([
PHOTO_FILE,
{ ...PHOTO_FILE, buffer: Buffer.from("second photo"), name: "other.png" },
]);
await expect.poll(() => uploadCommandCount(page)).toBe(4);
});
test("photos upload before Send without a queued spoiler control", async ({
page,
}) => {