From 2760cdd3af1cc0b8cca62d3be606ed10e44717d4 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 14 Oct 2026 16:00:06 +1100 Subject: [PATCH 01/10] feat(staged): accept dropped, pasted, or picked PDFs as PNG image attachments A PDF dropped into Staged never reached the image store: the frontend extension allowlist routed it down the text-file path (a UTF-8 error toast on the branch card, or the literal path inserted into the chat textarea), paste and the file picker filtered on `image/*`, and the backend sniffer had no PDF branch. Backend: `store_sniffed_image`, the single choke point for chat and timeline attachments, now detects `%PDF-` and rasterizes page one with CoreGraphics in-process (new `note_media::pdf`, via the already-locked `objc2-core-graphics`/`objc2-core-foundation` crates) before the usual PNG sniff, renaming `.pdf` to `.png`. The page is drawn on an opaque white background at `min(2048 / long_edge_pt, 4.0)` px/pt, so the long edge never exceeds 2048 px and tiny pages are not blown up past 4x; `/Rotate` is honoured; if the PNG exceeds MAX_IMAGE_SIZE it is re-rendered once at half scale before failing with the existing size message. Malformed input is rejected by `CGPDFDocument` immediately (no subprocess, no hang). The renderer reports the page count for a later one-attachment-per-page follow-up; the PDF bytes are not kept. `create_image` and `create_image_from_data` are now async and run on `spawn_blocking` so rendering stays off the main thread; the web dispatcher keeps calling the synchronous impl. Non-macOS targets compile a stub returning "PDF attachments require macOS" and the CoreGraphics tests are cfg-gated, so the ubuntu CI build and clippy stay green. Note ingest (`validate_media`) still rejects `.pdf`. Frontend: `isImageAttachmentFile` (images + `.pdf`) drives the three native drop handlers and `isMaybeTextFile`; paste accepts `application/pdf` alongside `image/*`; both pickers list it via a shared ATTACHMENT_ACCEPT; local base64 previews are used only when the browser type matches the stored MIME, otherwise the existing `getImageData` effect fetches the rendered PNG; branch-card `createImage` failures now toast like the text path instead of failing silently. Verified: a 3150x1800 pt one-page fixture renders to a 2048x1170 opaque PNG (~206 KB) that matches the source; `cargo test` (1061 passed), `cargo clippy -D warnings`, `cargo fmt --check`, vitest (1317 passed, --maxWorkers=3), `pnpm run check` (0 errors/warnings), prettier. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- apps/staged/src-tauri/Cargo.lock | 2 + apps/staged/src-tauri/Cargo.toml | 20 ++ apps/staged/src-tauri/src/image_commands.rs | 77 +++- apps/staged/src-tauri/src/note_media/files.rs | 20 ++ apps/staged/src-tauri/src/note_media/mod.rs | 1 + apps/staged/src-tauri/src/note_media/pdf.rs | 335 ++++++++++++++++++ apps/staged/src-tauri/src/note_media/tests.rs | 8 + .../lib/features/branches/BranchCard.svelte | 11 +- .../branches/branchCardHelpers.test.ts | 11 + .../features/branches/branchCardHelpers.ts | 10 +- .../lib/features/sessions/ChatComposer.svelte | 3 +- .../features/sessions/ImageAttachment.svelte | 26 +- .../features/sessions/NewSessionModal.svelte | 4 +- .../features/sessions/SessionChatPane.svelte | 17 +- .../features/sessions/attachErrors.test.ts | 27 +- .../src/lib/features/sessions/attachErrors.ts | 11 + 16 files changed, 557 insertions(+), 26 deletions(-) create mode 100644 apps/staged/src-tauri/src/note_media/pdf.rs diff --git a/apps/staged/src-tauri/Cargo.lock b/apps/staged/src-tauri/Cargo.lock index 687682319..b16abe411 100644 --- a/apps/staged/src-tauri/Cargo.lock +++ b/apps/staged/src-tauri/Cargo.lock @@ -23,6 +23,8 @@ dependencies = [ "include_dir", "libc", "log", + "objc2-core-foundation", + "objc2-core-graphics", "pikchr", "rand 0.10.3", "regex", diff --git a/apps/staged/src-tauri/Cargo.toml b/apps/staged/src-tauri/Cargo.toml index b33d73b8c..1b120f56b 100644 --- a/apps/staged/src-tauri/Cargo.toml +++ b/apps/staged/src-tauri/Cargo.toml @@ -85,6 +85,26 @@ tiny-skia = "0.12" tar = "0.4" flate2 = "1" +# PDF attachments: rasterize page 1 with CoreGraphics (note_media::pdf). Both +# crates are already in the lock via arboard/tray-icon; the feature list is +# the minimum for CGPDFDocument + a bitmap CGContext + CFData input. +[target.'cfg(target_os = "macos")'.dependencies] +objc2-core-graphics = { version = "0.3.2", default-features = false, features = [ + "CGAffineTransform", + "CGBitmapContext", + "CGColorSpace", + "CGContext", + "CGDataProvider", + "CGGeometry", + "CGImage", + "CGPDFDocument", + "CGPDFPage", +] } +objc2-core-foundation = { version = "0.3.2", default-features = false, features = [ + "CFCGTypes", + "CFData", +] } + [dev-dependencies] tower = { version = "0.5", features = ["util"] } # test-util: `#[tokio::test(start_paused = true)]` for the store-events diff --git a/apps/staged/src-tauri/src/image_commands.rs b/apps/staged/src-tauri/src/image_commands.rs index 921d98d73..220f30e78 100644 --- a/apps/staged/src-tauri/src/image_commands.rs +++ b/apps/staged/src-tauri/src/image_commands.rs @@ -14,7 +14,7 @@ use std::sync::{Arc, Mutex}; /// real session ID). Pass `false` for images that should appear in the /// timeline immediately (e.g. direct branch-card drops). #[tauri::command(rename_all = "camelCase")] -pub fn create_image( +pub async fn create_image( store: tauri::State<'_, Mutex>>>, branch_id: Option, project_id: String, @@ -22,7 +22,21 @@ pub fn create_image( pending: Option, ) -> Result { let store = crate::get_store(&store)?; + // Storing may rasterize a PDF page; keep that off the main thread. + tauri::async_runtime::spawn_blocking(move || { + create_image_impl(store, branch_id, project_id, file_path, pending) + }) + .await + .map_err(|e| format!("Failed to store image: {e}"))? +} +fn create_image_impl( + store: Arc, + branch_id: Option, + project_id: String, + file_path: String, + pending: Option, +) -> Result { let src = Path::new(&file_path); let filename = src .file_name() @@ -121,7 +135,7 @@ pub fn get_image_data( /// the stored format follows the file signature. See [`create_image`] for the /// meaning of the `pending` flag. #[tauri::command(rename_all = "camelCase")] -pub fn create_image_from_data( +pub async fn create_image_from_data( store: tauri::State<'_, Mutex>>>, branch_id: Option, project_id: String, @@ -131,9 +145,14 @@ pub fn create_image_from_data( pending: Option, ) -> Result { let store = crate::get_store(&store)?; - create_image_from_data_impl( - store, branch_id, project_id, filename, mime_type, data, pending, - ) + // See `create_image`: PDF rendering must not block the main thread. + tauri::async_runtime::spawn_blocking(move || { + create_image_from_data_impl( + store, branch_id, project_id, filename, mime_type, data, pending, + ) + }) + .await + .map_err(|e| format!("Failed to store image: {e}"))? } pub(crate) fn create_image_from_data_impl( @@ -219,4 +238,52 @@ mod tests { let err = attach(&f, "clip.png", "", b"\0\0\0\x18ftypisom\0\0\0\0").unwrap_err(); assert!(err.contains("only images"), "{err}"); } + + #[cfg(target_os = "macos")] + #[test] + fn pdf_attachments_are_rasterized_to_png() { + use crate::note_media::pdf::tests::{tiny_pdf, PAGE_ONE}; + let f = Fixture::new(); + let image = attach(&f, "doc.pdf", "application/pdf", &tiny_pdf(&[PAGE_ONE])).unwrap(); + assert_eq!(image.mime_type, "image/png"); + assert_eq!(image.filename, "doc.png"); + let path = + crate::store::images::image_file_path(&image.project_id, &image.id, &image.filename) + .unwrap(); + let png = std::fs::read(path).unwrap(); + assert_eq!(image.size_bytes, png.len() as i64); + assert!(png.starts_with(b"\x89PNG\r\n\x1a\n")); + let pixmap = tiny_skia::Pixmap::decode_png(&png).unwrap(); + assert!(pixmap.width().max(pixmap.height()) <= 2048); + let corner = pixmap.pixel(0, 0).unwrap(); + assert_eq!( + (corner.red(), corner.green(), corner.blue(), corner.alpha()), + (255, 255, 255, 255) + ); + + // Pasted PDFs arrive without a usable name, like pasted images. + assert_eq!( + attach(&f, "pasted", "application/pdf", &tiny_pdf(&[PAGE_ONE])) + .unwrap() + .filename, + "pasted.png" + ); + } + + #[cfg(target_os = "macos")] + #[test] + fn malformed_pdf_is_rejected_without_hanging() { + let f = Fixture::new(); + let err = attach(&f, "doc.pdf", "application/pdf", b"%PDF-1.4\ngarbage").unwrap_err(); + assert!(err.contains("PDF"), "{err}"); + } + + #[cfg(not(target_os = "macos"))] + #[test] + fn pdf_attachments_report_platform_support() { + use crate::note_media::pdf::tests::{tiny_pdf, PAGE_ONE}; + let f = Fixture::new(); + let err = attach(&f, "doc.pdf", "application/pdf", &tiny_pdf(&[PAGE_ONE])).unwrap_err(); + assert!(err.contains("macOS"), "{err}"); + } } diff --git a/apps/staged/src-tauri/src/note_media/files.rs b/apps/staged/src-tauri/src/note_media/files.rs index dfffbc57b..98090c454 100644 --- a/apps/staged/src-tauri/src/note_media/files.rs +++ b/apps/staged/src-tauri/src/note_media/files.rs @@ -113,6 +113,26 @@ pub(crate) fn store_sniffed_image( MAX_IMAGE_SIZE / 1024 / 1024 )); } + // A PDF becomes an ordinary PNG attachment of its first page; nothing + // downstream (store, viewer, agent prompt) ever sees PDF bytes. Only this + // chat/timeline path converts: note ingest (`validate_media`) still + // rejects `.pdf` references. + let rendered; + let renamed; + let (filename, bytes) = if super::pdf::is_pdf(bytes) { + rendered = super::pdf::render_first_page(bytes, super::pdf::MAX_EDGE_PX)?; + log::debug!( + "Rendered page 1 of {} from PDF attachment {filename}", + rendered.page_count + ); + renamed = Path::new(filename) + .with_extension("png") + .to_string_lossy() + .into_owned(); + (renamed.as_str(), rendered.png.as_slice()) + } else { + (filename, bytes) + }; let sniffed = sniff_format(bytes).ok_or("unsupported image format")?; if mime_for_extension(sniffed).unwrap().starts_with("video/") { return Err("only images can be uploaded here".into()); diff --git a/apps/staged/src-tauri/src/note_media/mod.rs b/apps/staged/src-tauri/src/note_media/mod.rs index 83d9b2915..b1530ed13 100644 --- a/apps/staged/src-tauri/src/note_media/mod.rs +++ b/apps/staged/src-tauri/src/note_media/mod.rs @@ -5,6 +5,7 @@ mod blocks; pub(crate) mod files; mod materialize; pub(crate) use materialize::materialize_note_media; +pub(crate) mod pdf; #[cfg(test)] pub(crate) use materialize::{materialize_note_media_with, RemoteMedia}; pub(crate) mod refs; diff --git a/apps/staged/src-tauri/src/note_media/pdf.rs b/apps/staged/src-tauri/src/note_media/pdf.rs new file mode 100644 index 000000000..f5ea76d3c --- /dev/null +++ b/apps/staged/src-tauri/src/note_media/pdf.rs @@ -0,0 +1,335 @@ +//! PDF attachments: rasterize the first page to an opaque PNG so a dropped +//! PDF flows through the image store, viewer, and agent prompt as an ordinary +//! `image/png` row. Rendering uses CoreGraphics in-process (no subprocess, no +//! temp files, malformed input fails fast instead of hanging). +//! +//! macOS only — the shipped target. Other platforms compile a stub so the +//! Linux CI build stays green; they report that PDF attachments need macOS. + +#[cfg(target_os = "macos")] +use super::files::MAX_IMAGE_SIZE; + +/// Long edge of the rendered page in pixels. A letter page lands at ~186 DPI, +/// which keeps body text legible to a vision model without approaching the +/// attachment size cap. +pub(crate) const MAX_EDGE_PX: u32 = 2048; + +/// Pages are never blown up beyond 4 px per PDF point, so a tiny page does +/// not become a huge blurry bitmap just to reach the long-edge target. +#[cfg(target_os = "macos")] +const MAX_SCALE: f64 = 4.0; + +pub(crate) fn is_pdf(bytes: &[u8]) -> bool { + bytes.starts_with(b"%PDF-") +} + +pub(crate) struct RenderedPage { + pub png: Vec, + /// Pages in the document. Only the first is rendered today; a follow-up + /// can emit one attachment per page. + pub page_count: usize, +} + +impl std::fmt::Debug for RenderedPage { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("RenderedPage") + .field("png_bytes", &self.png.len()) + .field("page_count", &self.page_count) + .finish() + } +} + +#[cfg(target_os = "macos")] +fn size_limit_error() -> String { + format!("media exceeds {} MB limit", MAX_IMAGE_SIZE / 1024 / 1024) +} + +/// Render page 1 to a white-backed PNG whose long edge is `max_edge_px` +/// (less when that would exceed [`MAX_SCALE`]). If the PNG is still over +/// [`MAX_IMAGE_SIZE`] — photographic pages — it is re-rendered once at half +/// scale before failing with the usual size message. +#[cfg(target_os = "macos")] +pub(crate) fn render_first_page(bytes: &[u8], max_edge_px: u32) -> Result { + use objc2_core_foundation::CFData; + use objc2_core_graphics::{CGDataProvider, CGPDFBox, CGPDFDocument, CGPDFPage}; + + let data = CFData::from_bytes(bytes); + let provider = CGDataProvider::with_cf_data(Some(&data)).ok_or("could not read PDF data")?; + // CoreGraphics returns no document for anything it cannot parse, so a + // `%PDF-` prefix on garbage is rejected here rather than drawn as blank. + let document = CGPDFDocument::with_provider(Some(&provider)).ok_or("could not open PDF")?; + let page_count = CGPDFDocument::number_of_pages(Some(&document)); + if page_count == 0 { + return Err("PDF has no pages".into()); + } + let page = CGPDFDocument::page(Some(&document), 1).ok_or("could not read PDF page 1")?; + + // MediaBox is the page; fall back to CropBox only when it is unusable. + let mut page_box = CGPDFBox::MediaBox; + let mut rect = CGPDFPage::box_rect(Some(&page), page_box); + if !is_drawable(rect.size.width, rect.size.height) { + page_box = CGPDFBox::CropBox; + rect = CGPDFPage::box_rect(Some(&page), page_box); + if !is_drawable(rect.size.width, rect.size.height) { + return Err("PDF page has no drawable area".into()); + } + } + // `drawing_transform` honours the page's own /Rotate, so the bitmap must + // take the rotated orientation for the page to fill it. + let rotated = CGPDFPage::rotation_angle(Some(&page)).rem_euclid(180) == 90; + let (w_pt, h_pt) = if rotated { + (rect.size.height, rect.size.width) + } else { + (rect.size.width, rect.size.height) + }; + let scale = (f64::from(max_edge_px) / w_pt.max(h_pt)).min(MAX_SCALE); + + let png = render(&page, page_box, (w_pt, h_pt), scale)?; + let png = if png.len() as u64 > MAX_IMAGE_SIZE { + render(&page, page_box, (w_pt, h_pt), scale / 2.0)? + } else { + png + }; + if png.len() as u64 > MAX_IMAGE_SIZE { + return Err(size_limit_error()); + } + Ok(RenderedPage { png, page_count }) +} + +#[cfg(target_os = "macos")] +fn is_drawable(width: f64, height: f64) -> bool { + width.is_finite() && height.is_finite() && width >= 1.0 && height >= 1.0 +} + +#[cfg(target_os = "macos")] +fn pixel_size(w_pt: f64, h_pt: f64, scale: f64) -> (u32, u32) { + let px = |pt: f64| (pt * scale).round().max(1.0) as u32; + (px(w_pt), px(h_pt)) +} + +/// Draw the page, displayed at `(w_pt, h_pt)` points, into a fresh RGBA +/// bitmap at `scale` px/pt and PNG-encode it with tiny-skia (its pixmaps are +/// premultiplied RGBA8, the same layout CoreGraphics writes for +/// `PremultipliedLast`). +#[cfg(target_os = "macos")] +fn render( + page: &objc2_core_graphics::CGPDFPage, + page_box: objc2_core_graphics::CGPDFBox, + (w_pt, h_pt): (f64, f64), + scale: f64, +) -> Result, String> { + use objc2_core_foundation::{CGPoint, CGRect, CGSize}; + use objc2_core_graphics::{ + CGBitmapContextCreate, CGColorSpace, CGContext, CGImageAlphaInfo, CGPDFPage, + }; + + let (width, height) = pixel_size(w_pt, h_pt, scale); + let (w, h) = (width as usize, height as usize); + let mut buf = vec![0u8; w * h * 4]; + let space = CGColorSpace::new_device_rgb().ok_or("could not create color space")?; + let bitmap = CGRect { + origin: CGPoint { x: 0.0, y: 0.0 }, + size: CGSize { + width: f64::from(width), + height: f64::from(height), + }, + }; + // In points, after the CTM is scaled. `drawing_transform` only ever + // scales a page *down* to fit its rect (otherwise it centres the page at + // 1:1), so the magnification has to come from the CTM, and the rect is + // the page's own displayed size so the box lands flush at the origin. + let page_rect = CGRect { + origin: CGPoint { x: 0.0, y: 0.0 }, + size: CGSize { + width: w_pt, + height: h_pt, + }, + }; + { + // SAFETY: the context draws into `buf`, which outlives it — the + // context is released at the end of this block, before `buf` moves + // into the pixmap — and the width, height, and row stride describe + // exactly the RGBA8 buffer allocated above. + let context = unsafe { + CGBitmapContextCreate( + buf.as_mut_ptr().cast(), + w, + h, + 8, + w * 4, + Some(&space), + CGImageAlphaInfo::PremultipliedLast.0, + ) + } + .ok_or("could not create bitmap context")?; + let ctx = Some(&*context); + // White behind the page: PDFs have no background, and a transparent + // PNG would lose black text on the dark theme and for the agent. + CGContext::set_rgb_fill_color(ctx, 1.0, 1.0, 1.0, 1.0); + CGContext::fill_rect(ctx, bitmap); + CGContext::scale_ctm(ctx, scale, scale); + CGContext::concat_ctm( + ctx, + CGPDFPage::drawing_transform(Some(page), page_box, page_rect, 0, true), + ); + CGContext::draw_pdf_page(ctx, Some(page)); + } + let size = tiny_skia::IntSize::from_wh(width, height).ok_or("invalid PDF page size")?; + let pixmap = tiny_skia::Pixmap::from_vec(buf, size).ok_or("invalid PDF page bitmap")?; + pixmap + .encode_png() + .map_err(|e| format!("could not encode PDF page: {e}")) +} + +#[cfg(not(target_os = "macos"))] +pub(crate) fn render_first_page(_bytes: &[u8], _max_edge_px: u32) -> Result { + Err("PDF attachments require macOS".into()) +} + +#[cfg(test)] +pub(crate) mod tests { + use super::*; + + /// A minimal PDF with one letter-size page per content stream, using the + /// built-in Helvetica. Offsets in the xref table are computed, so the + /// output is well-formed rather than relying on reader repair. + pub(crate) fn tiny_pdf(pages: &[&str]) -> Vec { + tiny_pdf_with(pages, "[0 0 612 792]", None) + } + + pub(crate) fn tiny_pdf_with(pages: &[&str], media_box: &str, rotate: Option) -> Vec { + let font_id = 3 + pages.len() * 2; + let mut objects = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".to_owned(), + format!( + "<< /Type /Pages /Kids [{}] /Count {} >>", + (0..pages.len()) + .map(|i| format!("{} 0 R", 3 + i * 2)) + .collect::>() + .join(" "), + pages.len() + ), + ]; + for (i, content) in pages.iter().enumerate() { + let rotate = rotate.map(|r| format!(" /Rotate {r}")).unwrap_or_default(); + objects.push(format!( + "<< /Type /Page /Parent 2 0 R /MediaBox {media_box}{rotate} \ + /Resources << /Font << /F1 {font_id} 0 R >> >> /Contents {} 0 R >>", + 4 + i * 2 + )); + objects.push(format!( + "<< /Length {} >>\nstream\n{content}\nendstream", + content.len() + )); + } + objects.push("<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".to_owned()); + + let mut out = b"%PDF-1.4\n".to_vec(); + let mut offsets = Vec::new(); + for (i, body) in objects.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{body}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice( + format!("xref\n0 {}\n0000000000 65535 f \n", objects.len() + 1).as_bytes(), + ); + for offset in offsets { + out.extend_from_slice(format!("{offset:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice( + format!( + "trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", + objects.len() + 1 + ) + .as_bytes(), + ); + out + } + + pub(crate) const PAGE_ONE: &str = + "BT /F1 24 Tf 72 700 Td (PAGE ONE Staged PDF probe) Tj ET\n4 w 72 650 m 540 650 l S"; + const PAGE_TWO: &str = "BT /F1 24 Tf 72 700 Td (PAGE TWO should not appear) Tj ET"; + + #[test] + fn detects_the_pdf_signature_only() { + assert!(is_pdf(&tiny_pdf(&[PAGE_ONE]))); + assert!(is_pdf(b"%PDF-1.7 junk")); + assert!(!is_pdf(b"\x89PNG\r\n\x1a\n")); + assert!(!is_pdf(b" %PDF-1.4")); + assert!(!is_pdf(b"")); + } + + #[cfg(target_os = "macos")] + mod macos { + use super::*; + + fn decode(png: &[u8]) -> tiny_skia::Pixmap { + assert!(png.starts_with(b"\x89PNG\r\n\x1a\n")); + tiny_skia::Pixmap::decode_png(png).unwrap() + } + + fn is_opaque_white(pixmap: &tiny_skia::Pixmap, x: u32, y: u32) -> bool { + let p = pixmap.pixel(x, y).unwrap(); + (p.red(), p.green(), p.blue(), p.alpha()) == (255, 255, 255, 255) + } + + #[test] + fn renders_first_page_to_opaque_png_at_the_long_edge() { + let rendered = render_first_page(&tiny_pdf(&[PAGE_ONE, PAGE_TWO]), 2048).unwrap(); + assert_eq!(rendered.page_count, 2); + let pixmap = decode(&rendered.png); + // Letter: 2048 / 792 pt ≈ 2.59x, under the 4x ceiling. + assert_eq!((pixmap.width(), pixmap.height()), (1583, 2048)); + for (x, y) in [(0, 0), (1582, 0), (0, 2047), (1582, 2047)] { + assert!(is_opaque_white(&pixmap, x, y), "corner {x},{y}"); + } + // The rule at y=650pt from the bottom is drawn; everything is opaque. + let rule_y = ((792.0 - 650.0) * 2048.0 / 792.0) as u32; + let p = pixmap.pixel(800, rule_y).unwrap(); + assert!(p.red() < 128 && p.alpha() == 255, "rule pixel {p:?}"); + assert!(pixmap.pixels().iter().all(|p| p.alpha() == 255)); + // Page two's text sits where page one has nothing but white. + assert!(is_opaque_white(&pixmap, 1400, 2048 - rule_y - 300)); + } + + #[test] + fn wide_pages_are_downscaled_and_tiny_pages_capped_at_4x() { + // Same box as the onboarding flow chart fixture: 3150×1800 pt. + let wide = tiny_pdf_with(&[PAGE_ONE], "[0 0 3150 1800]", None); + let pixmap = decode(&render_first_page(&wide, 2048).unwrap().png); + assert_eq!((pixmap.width(), pixmap.height()), (2048, 1170)); + + let tiny = tiny_pdf_with(&[PAGE_ONE], "[0 0 100 50]", None); + let pixmap = decode(&render_first_page(&tiny, 2048).unwrap().png); + assert_eq!((pixmap.width(), pixmap.height()), (400, 200)); + } + + #[test] + fn rotated_pages_take_their_displayed_orientation() { + let landscape = tiny_pdf_with(&[PAGE_ONE], "[0 0 612 792]", Some(90)); + let pixmap = decode(&render_first_page(&landscape, 2048).unwrap().png); + assert_eq!((pixmap.width(), pixmap.height()), (2048, 1583)); + assert!(pixmap.pixels().iter().any(|p| p.red() < 128)); + } + + #[test] + fn malformed_pdfs_are_rejected_without_hanging() { + for bytes in [ + &b"%PDF-1.4\nnot really a pdf"[..], + b"%PDF-", + &tiny_pdf(&[])[..], + ] { + let err = render_first_page(bytes, 2048).unwrap_err(); + assert!(err.contains("PDF"), "{err}"); + } + } + } + + #[cfg(not(target_os = "macos"))] + #[test] + fn reports_platform_support() { + let err = render_first_page(&tiny_pdf(&[PAGE_ONE]), 2048).unwrap_err(); + assert!(err.contains("macOS"), "{err}"); + } +} diff --git a/apps/staged/src-tauri/src/note_media/tests.rs b/apps/staged/src-tauri/src/note_media/tests.rs index 311d6327f..83e2d6283 100644 --- a/apps/staged/src-tauri/src/note_media/tests.rs +++ b/apps/staged/src-tauri/src/note_media/tests.rs @@ -550,6 +550,14 @@ fn validates_magic_and_caps_by_kind_without_reading_oversized_files() { } assert!(validate_media("fake.mp4", b"\0\0\0\x18ftypavif\0\0\0\0", true).is_err()); assert!(validate_media("fake.webm", b"\x1a\x45\xdf\xa3matroska", true).is_err()); + // PDFs are converted only on the chat attachment path; note ingest does + // not accept them. + assert!(validate_media( + "a.pdf", + &pdf::tests::tiny_pdf(&[pdf::tests::PAGE_ONE]), + true + ) + .is_err()); assert!(size_limit("a.mp4", false).is_err()); assert_eq!(size_limit("a.png", true).unwrap(), MAX_IMAGE_SIZE); assert_eq!(size_limit("a.mov", true).unwrap(), MAX_VIDEO_SIZE); diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index 6ac06c662..243c34b95 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -72,7 +72,7 @@ formatBaseBranch, isGitActionInFlight, isMaybeTextFile, - isImageFile, + isImageAttachmentFile, } from './branchCardHelpers'; import BranchCardHeaderInfo from './BranchCardHeaderInfo.svelte'; import BranchCardActionsBar from './BranchCardActionsBar.svelte'; @@ -1737,7 +1737,7 @@ function handleFileDrop(paths: string[]) { const textPaths = paths.filter(isMaybeTextFile); - const imagePaths = paths.filter(isImageFile); + const imagePaths = paths.filter(isImageAttachmentFile); if (textPaths.length > 0) { const placeholders = textPaths.map((filePath) => ({ @@ -1778,6 +1778,13 @@ await commands.createImage(branch.id, branch.projectId, filePath); } catch (e) { console.error('Failed to create image from drop:', e); + // Same shape as the text path, so a rejected PDF or oversized + // image ("could not open PDF", "media exceeds 10 MB limit") is + // not a silent no-op. + const reason = e instanceof Error ? e.message : typeof e === 'string' ? e : null; + toast.error('Error', { + description: `Could not attach "${fileNameFromPath(filePath)}"${reason ? ` \u2014 ${reason}` : ''}`, + }); } }) ).then(() => { diff --git a/apps/staged/src/lib/features/branches/branchCardHelpers.test.ts b/apps/staged/src/lib/features/branches/branchCardHelpers.test.ts index a5a0129cf..6a253da91 100644 --- a/apps/staged/src/lib/features/branches/branchCardHelpers.test.ts +++ b/apps/staged/src/lib/features/branches/branchCardHelpers.test.ts @@ -8,6 +8,7 @@ import { extractPrNumber, extractPrUrl, isGitActionInFlight, + isImageAttachmentFile, isImageFile, isMaybeTextFile, isPushRejectedNonFastForward, @@ -62,6 +63,15 @@ describe('file type helpers', () => { const filePath = `/tmp/image.${extension}`; expect(isImageFile(filePath)).toBe(true); + expect(isImageAttachmentFile(filePath)).toBe(true); + expect(isMaybeTextFile(filePath)).toBe(false); + } + }); + + it('attaches PDFs as images rather than reading them as text', () => { + for (const filePath of ['/tmp/report.pdf', '/tmp/Scan.PDF']) { + expect(isImageFile(filePath)).toBe(false); + expect(isImageAttachmentFile(filePath)).toBe(true); expect(isMaybeTextFile(filePath)).toBe(false); } }); @@ -71,6 +81,7 @@ describe('file type helpers', () => { const filePath = `/tmp/image.${extension}`; expect(isImageFile(filePath)).toBe(false); + expect(isImageAttachmentFile(filePath)).toBe(false); expect(isMaybeTextFile(filePath)).toBe(true); } }); diff --git a/apps/staged/src/lib/features/branches/branchCardHelpers.ts b/apps/staged/src/lib/features/branches/branchCardHelpers.ts index e6a739e65..5ba602b3a 100644 --- a/apps/staged/src/lib/features/branches/branchCardHelpers.ts +++ b/apps/staged/src/lib/features/branches/branchCardHelpers.ts @@ -3,6 +3,8 @@ import type { PushState } from '../../stores/pushState.svelte'; import type { PipelineExecution } from '../../types'; const IMAGE_EXTENSIONS = ['.png', '.jpg', '.jpeg', '.gif', '.webp']; +// Stored as an image attachment too: the backend rasterizes page one to PNG. +const PDF_EXTENSIONS = ['.pdf']; /** * Whether a push or pull of the branch's own is running or waiting on the branch @@ -245,8 +247,14 @@ export function isImageFile(filePath: string): boolean { return IMAGE_EXTENSIONS.some((ext) => lower.endsWith(ext)); } +/** Files `createImage` accepts: images plus PDFs, which it converts to PNG. */ +export function isImageAttachmentFile(filePath: string): boolean { + const lower = filePath.toLowerCase(); + return isImageFile(filePath) || PDF_EXTENSIONS.some((ext) => lower.endsWith(ext)); +} + export function isMaybeTextFile(filePath: string): boolean { - return !isImageFile(filePath); + return !isImageAttachmentFile(filePath); } /** diff --git a/apps/staged/src/lib/features/sessions/ChatComposer.svelte b/apps/staged/src/lib/features/sessions/ChatComposer.svelte index e00dc4ed1..776a2cbf2 100644 --- a/apps/staged/src/lib/features/sessions/ChatComposer.svelte +++ b/apps/staged/src/lib/features/sessions/ChatComposer.svelte @@ -28,6 +28,7 @@ import ImagePlus from '@lucide/svelte/icons/image-plus'; import Spinner from '../../shared/Spinner.svelte'; import { Button } from '$lib/components/ui/button'; + import { ATTACHMENT_ACCEPT } from './attachErrors'; import HashtagInput from './HashtagInput.svelte'; import type { HashtagItem } from '../../types'; @@ -136,7 +137,7 @@ string[]); @@ -127,11 +133,15 @@ true ); onImageIdsChange((current) => [...current, image.id]); - // Set preview immediately from the local data. The stored MIME follows - // the file signature, which may differ from the browser's label. - const dataUrl = `data:${image.mimeType};base64,${base64}`; - previews = new Map(previews); - previews.set(image.id, dataUrl); + // Set preview immediately from the local data when it is what got + // stored. The stored MIME follows the file signature, which may differ + // from the browser's label; a PDF becomes a rendered PNG, which the + // preview effect above fetches instead. + if (file.type === image.mimeType) { + const dataUrl = `data:${image.mimeType};base64,${base64}`; + previews = new Map(previews); + previews.set(image.id, dataUrl); + } return null; } catch (err) { console.error('Failed to attach image:', err); @@ -145,7 +155,7 @@ if (!items || disabled) return; const files: File[] = []; for (const item of Array.from(items)) { - if (item.type.startsWith('image/')) { + if (isAttachableClipboardType(item.type)) { e.preventDefault(); const file = item.getAsFile(); if (file) files.push(file); @@ -169,7 +179,7 @@ isImageFile(p)); + const imagePaths = paths.filter((p) => isImageAttachmentFile(p)); const textPaths = paths.filter((p) => isMaybeTextFile(p)); const newIds: string[] = []; for (const path of imagePaths) { diff --git a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte index 6201e87ae..fd1d78d54 100644 --- a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte +++ b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte @@ -124,7 +124,7 @@ import { Button } from '$lib/components/ui/button'; import { subscribeDragDrop } from '../branches/dragDrop'; import { - isImageFile, + isImageAttachmentFile, isMaybeTextFile, insertFilePathsAtCursor, } from '../branches/branchCardHelpers'; @@ -133,6 +133,7 @@ attachBasename, attachErrorReason, formatAttachErrors, + isAttachableClipboardType, type AttachRejection, } from './attachErrors'; import { @@ -580,9 +581,13 @@ true ); replyImageIds = [...replyImageIds, image.id]; - const dataUrl = `data:${image.mimeType};base64,${base64}`; - imagePreviews = new Map(imagePreviews); - imagePreviews.set(image.id, dataUrl); + // Preview from the local bytes only when they are what got stored; a + // PDF is stored as a rendered PNG, which the preview effect fetches. + if (file.type === image.mimeType) { + const dataUrl = `data:${image.mimeType};base64,${base64}`; + imagePreviews = new Map(imagePreviews); + imagePreviews.set(image.id, dataUrl); + } return null; } catch (err) { console.error('Failed to attach image:', err); @@ -596,7 +601,7 @@ if (!items) return; const files: File[] = []; for (const item of Array.from(items)) { - if (item.type.startsWith('image/')) { + if (isAttachableClipboardType(item.type)) { e.preventDefault(); const file = item.getAsFile(); if (file) files.push(file); @@ -618,7 +623,7 @@ // Drag-and-drop files (via Tauri native drag-drop events) async function handleFileDrop(paths: string[]) { if (!projectId) return; - const imagePaths = paths.filter((p) => isImageFile(p)); + const imagePaths = paths.filter((p) => isImageAttachmentFile(p)); const textPaths = paths.filter((p) => isMaybeTextFile(p)); const bid = branchId ?? null; const pid = projectId; diff --git a/apps/staged/src/lib/features/sessions/attachErrors.test.ts b/apps/staged/src/lib/features/sessions/attachErrors.test.ts index dc04d52c8..5ed2d6879 100644 --- a/apps/staged/src/lib/features/sessions/attachErrors.test.ts +++ b/apps/staged/src/lib/features/sessions/attachErrors.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it } from 'vitest'; -import { attachBasename, attachErrorReason, formatAttachErrors } from './attachErrors'; +import { + ATTACHMENT_ACCEPT, + attachBasename, + attachErrorReason, + formatAttachErrors, + isAttachableClipboardType, +} from './attachErrors'; describe('formatAttachErrors', () => { it('returns null when nothing was rejected', () => { @@ -47,3 +53,22 @@ describe('attachBasename', () => { expect(attachBasename('three.png')).toBe('three.png'); }); }); + +describe('isAttachableClipboardType', () => { + it('accepts images and PDFs, which the backend renders to PNG', () => { + expect(isAttachableClipboardType('image/png')).toBe(true); + expect(isAttachableClipboardType('image/webp')).toBe(true); + expect(isAttachableClipboardType('application/pdf')).toBe(true); + }); + + it('ignores other clipboard payloads', () => { + expect(isAttachableClipboardType('text/plain')).toBe(false); + expect(isAttachableClipboardType('text/html')).toBe(false); + expect(isAttachableClipboardType('application/octet-stream')).toBe(false); + }); + + it('offers the same types in the file picker', () => { + expect(ATTACHMENT_ACCEPT.split(',')).toContain('application/pdf'); + expect(ATTACHMENT_ACCEPT.split(',')).toContain('image/png'); + }); +}); diff --git a/apps/staged/src/lib/features/sessions/attachErrors.ts b/apps/staged/src/lib/features/sessions/attachErrors.ts index 60785b143..6cc0d4a7c 100644 --- a/apps/staged/src/lib/features/sessions/attachErrors.ts +++ b/apps/staged/src/lib/features/sessions/attachErrors.ts @@ -4,6 +4,17 @@ export interface AttachRejection { reason: string; } +/** + * File-picker `accept` list for the image store. A PDF is accepted alongside + * images because the backend stores page one of it as a rendered PNG. + */ +export const ATTACHMENT_ACCEPT = 'image/png,image/jpeg,image/gif,image/webp,application/pdf'; + +/** Whether a pasted clipboard item can be attached through the image store. */ +export function isAttachableClipboardType(type: string): boolean { + return type.startsWith('image/') || type === 'application/pdf'; +} + export function attachErrorReason(err: unknown): string { return err instanceof Error ? err.message : String(err); } From 2bdff3542b4bb9194569b34d929b0349015c73c3 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 14 Oct 2026 16:10:20 +1100 Subject: [PATCH 02/10] fix(staged): prefer the PDF CropBox and render web-mode attachments off the runtime thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups from review of 2760cdd3 (PDF attachments as PNG images). `render_first_page` chose the MediaBox and only fell back to the CropBox when the MediaBox was unusable. Preview, Quick Look and most viewers display the CropBox, so a print-ready PDF with bleed and crop marks rendered a different image from what the user sees when they open the file. The order is now CropBox first — CoreGraphics already clips it to the MediaBox and returns the MediaBox when none is set — with the MediaBox as the fallback for a degenerate CropBox. New test: a letter MediaBox with a 300x400 pt CropBox renders at 1200x1600 (4x cap) with only the in-crop content, and an empty CropBox still falls back to the full page. `tiny_pdf_with` gained a `crop_box` argument for it. The test-only `PAGE_TWO` constant moved inside the macOS test module so the Linux `cargo test` step no longer warns that it is never used. The web dispatcher's `create_image_from_data` arm now runs the impl under `tauri::async_runtime::spawn_blocking` like the other heavy arms, so PDF rasterization no longer blocks the async runtime in web mode; the `?`/`??` error shape is unchanged with a "task failed" join error. Verified: the 3150x1800 pt onboarding flow chart fixture (CropBox equal to MediaBox) still renders to 2048x1170, 205,763 bytes; `cargo test` (1062 passed), `cargo clippy -D warnings`, `cargo fmt --check`. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- apps/staged/src-tauri/src/note_media/pdf.rs | 59 +++++++++++++++++---- apps/staged/src-tauri/src/web_server.rs | 10 ++-- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/apps/staged/src-tauri/src/note_media/pdf.rs b/apps/staged/src-tauri/src/note_media/pdf.rs index f5ea76d3c..a449a5d71 100644 --- a/apps/staged/src-tauri/src/note_media/pdf.rs +++ b/apps/staged/src-tauri/src/note_media/pdf.rs @@ -64,11 +64,14 @@ pub(crate) fn render_first_page(bytes: &[u8], max_edge_px: u32) -> Result Vec { - tiny_pdf_with(pages, "[0 0 612 792]", None) + tiny_pdf_with(pages, "[0 0 612 792]", None, None) } - pub(crate) fn tiny_pdf_with(pages: &[&str], media_box: &str, rotate: Option) -> Vec { + pub(crate) fn tiny_pdf_with( + pages: &[&str], + media_box: &str, + crop_box: Option<&str>, + rotate: Option, + ) -> Vec { let font_id = 3 + pages.len() * 2; let mut objects = vec![ "<< /Type /Catalog /Pages 2 0 R >>".to_owned(), @@ -211,9 +219,12 @@ pub(crate) mod tests { ), ]; for (i, content) in pages.iter().enumerate() { + let crop_box = crop_box + .map(|b| format!(" /CropBox {b}")) + .unwrap_or_default(); let rotate = rotate.map(|r| format!(" /Rotate {r}")).unwrap_or_default(); objects.push(format!( - "<< /Type /Page /Parent 2 0 R /MediaBox {media_box}{rotate} \ + "<< /Type /Page /Parent 2 0 R /MediaBox {media_box}{crop_box}{rotate} \ /Resources << /Font << /F1 {font_id} 0 R >> >> /Contents {} 0 R >>", 4 + i * 2 )); @@ -249,7 +260,6 @@ pub(crate) mod tests { pub(crate) const PAGE_ONE: &str = "BT /F1 24 Tf 72 700 Td (PAGE ONE Staged PDF probe) Tj ET\n4 w 72 650 m 540 650 l S"; - const PAGE_TWO: &str = "BT /F1 24 Tf 72 700 Td (PAGE TWO should not appear) Tj ET"; #[test] fn detects_the_pdf_signature_only() { @@ -264,6 +274,8 @@ pub(crate) mod tests { mod macos { use super::*; + const PAGE_TWO: &str = "BT /F1 24 Tf 72 700 Td (PAGE TWO should not appear) Tj ET"; + fn decode(png: &[u8]) -> tiny_skia::Pixmap { assert!(png.starts_with(b"\x89PNG\r\n\x1a\n")); tiny_skia::Pixmap::decode_png(png).unwrap() @@ -296,23 +308,50 @@ pub(crate) mod tests { #[test] fn wide_pages_are_downscaled_and_tiny_pages_capped_at_4x() { // Same box as the onboarding flow chart fixture: 3150×1800 pt. - let wide = tiny_pdf_with(&[PAGE_ONE], "[0 0 3150 1800]", None); + let wide = tiny_pdf_with(&[PAGE_ONE], "[0 0 3150 1800]", None, None); let pixmap = decode(&render_first_page(&wide, 2048).unwrap().png); assert_eq!((pixmap.width(), pixmap.height()), (2048, 1170)); - let tiny = tiny_pdf_with(&[PAGE_ONE], "[0 0 100 50]", None); + let tiny = tiny_pdf_with(&[PAGE_ONE], "[0 0 100 50]", None, None); let pixmap = decode(&render_first_page(&tiny, 2048).unwrap().png); assert_eq!((pixmap.width(), pixmap.height()), (400, 200)); } #[test] fn rotated_pages_take_their_displayed_orientation() { - let landscape = tiny_pdf_with(&[PAGE_ONE], "[0 0 612 792]", Some(90)); + let landscape = tiny_pdf_with(&[PAGE_ONE], "[0 0 612 792]", None, Some(90)); let pixmap = decode(&render_first_page(&landscape, 2048).unwrap().png); assert_eq!((pixmap.width(), pixmap.height()), (2048, 1583)); assert!(pixmap.pixels().iter().any(|p| p.red() < 128)); } + #[test] + fn crop_box_wins_over_media_box() { + // Letter MediaBox with bleed trimmed to a 300×400 pt CropBox; the + // rule is drawn inside the crop, the page-one text outside it. + const INSIDE_CROP: &str = "4 w 100 300 m 400 300 l S"; + let cropped = tiny_pdf_with( + &[&format!("{PAGE_ONE}\n{INSIDE_CROP}")], + "[0 0 612 792]", + Some("[100 100 400 500]"), + None, + ); + let pixmap = decode(&render_first_page(&cropped, 2048).unwrap().png); + // 2048 / 400 pt would be 5.12x, so the 4x ceiling applies. + assert_eq!((pixmap.width(), pixmap.height()), (1200, 1600)); + let rule_y = ((500.0 - 300.0) * 4.0) as u32; + let p = pixmap.pixel(600, rule_y).unwrap(); + assert!(p.red() < 128 && p.alpha() == 255, "rule pixel {p:?}"); + for (x, y) in [(0, 0), (1199, 0), (0, 1599), (1199, 1599)] { + assert!(is_opaque_white(&pixmap, x, y), "corner {x},{y}"); + } + + // An empty CropBox is not drawable, so the MediaBox is used. + let degenerate = tiny_pdf_with(&[PAGE_ONE], "[0 0 612 792]", Some("[0 0 0 0]"), None); + let pixmap = decode(&render_first_page(°enerate, 2048).unwrap().png); + assert_eq!((pixmap.width(), pixmap.height()), (1583, 2048)); + } + #[test] fn malformed_pdfs_are_rejected_without_hanging() { for bytes in [ diff --git a/apps/staged/src-tauri/src/web_server.rs b/apps/staged/src-tauri/src/web_server.rs index c2dab66be..02fc0ffcc 100644 --- a/apps/staged/src-tauri/src/web_server.rs +++ b/apps/staged/src-tauri/src/web_server.rs @@ -2851,9 +2851,13 @@ async fn dispatch(command: &str, args: Value, state: &WebAppState) -> Result = opt_arg(&args, "pending")?; - let image = crate::image_commands::create_image_from_data_impl( - store, branch_id, project_id, filename, mime_type, data, pending, - )?; + let image = tauri::async_runtime::spawn_blocking(move || { + crate::image_commands::create_image_from_data_impl( + store, branch_id, project_id, filename, mime_type, data, pending, + ) + }) + .await + .map_err(|e| format!("Image task failed: {e}"))??; Ok(serde_json::to_value(image).unwrap()) } "get_image_path" => { From 7251de3a1b4f99139464b53fd10dbb51ff96e189 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 14 Oct 2026 16:24:39 +1100 Subject: [PATCH 03/10] fix(staged): give PDF attachments a 50 MB input cap and name PDFs in the attach UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups from review of 2bdff354 (PDF attachments as PNG images). Only the rendered PNG of a PDF's first page is stored, but the 10 MB MAX_IMAGE_SIZE was applied to the PDF bytes themselves, so scans and print-ready files (routinely 10–50 MB) were refused with "media exceeds 10 MB limit" even though their first page rasterizes to a few hundred KB. `%PDF-` payloads now get MAX_PDF_INPUT_SIZE (50 MiB), chosen by a shared `attachment_limit(header)` on every route: `store_sniffed_image` for in-memory bytes, the new `read_local_attachment` for the path route (it reads the 5-byte signature first and refuses an oversized non-PDF from its metadata, so a 10 MB+ image is never buffered towards 50 MiB), and `create_image_from_data_impl`, which decodes the first two base64 quanta to pick the cap before decoding the payload. An oversized PDF now fails with "PDF exceeds 50 MB limit"; images keep "media exceeds 10 MB limit" (the base64 route previously said "Image too large", and now matches the message the same bytes get elsewhere). The rendered PNG still has to satisfy MAX_IMAGE_SIZE via the existing half-scale retry. `read_local` is split into `open_regular` + `read_capped` so both readers share the FIFO-safe open and the grew-past-cap check, and the "exceeds N MB limit" strings come from one `size_error` helper. The frontend read paths (SessionChatPane, ImageAttachment) impose no cap of their own, so nothing changes there. Tests: a well-formed PDF padded past 10 MiB with comment lines (`padded_pdf`, xref offsets skip the padding so it still renders instantly) is accepted on both routes and stored as a PNG under 10 MB; a >50 MiB PDF is refused with the new message on both routes (sparse file on the path route, a 67 MB base64 string on the data route); a >10 MiB PNG is still refused with the old message on both; and `read_local_attachment` is covered directly for the cap decision, sub-signature files, directories, and missing files. UI copy: the picker buttons now say "Attach image or PDF" / "Add image or PDF" / "Attach images or PDFs", and docs/inline-note-media.md states that a dropped, pasted, or picked PDF (up to 50 MiB) is stored as a PNG of its first page and is still refused as inline note media. BranchCard's createImage failure toast uses `attachBasename`, like the composer paths, so it reads `Could not attach "report.pdf" — …` rather than dropping the extension that explains the failure. The pdf.rs "page two was not rendered" assertion sampled a pixel that is white on either page. PAGE_TWO now draws a rule at y=400 pt that PAGE_ONE lacks; the test asserts that row is white in the rendered first page and dark when PAGE_TWO's content is rendered as page one, so the check is self-validating. Verified: `cargo test` (1064 passed), `cargo clippy -D warnings`, `cargo fmt --check`, vitest (1317 passed, --maxWorkers=3), `pnpm run check` (0 errors/warnings), prettier on the changed Svelte files (the docs file's only prettier complaint is a pre-existing one on line 18). Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- apps/staged/docs/inline-note-media.md | 11 ++- apps/staged/src-tauri/src/image_commands.rs | 98 ++++++++++++++++++- apps/staged/src-tauri/src/note_media/files.rs | 72 +++++++++++--- apps/staged/src-tauri/src/note_media/pdf.rs | 58 ++++++++--- apps/staged/src-tauri/src/note_media/tests.rs | 49 ++++++++++ .../lib/features/branches/BranchCard.svelte | 6 +- .../lib/features/sessions/ChatComposer.svelte | 4 +- .../features/sessions/ImageAttachment.svelte | 8 +- 8 files changed, 264 insertions(+), 42 deletions(-) diff --git a/apps/staged/docs/inline-note-media.md b/apps/staged/docs/inline-note-media.md index e3d30ae6e..0a5ec8c52 100644 --- a/apps/staged/docs/inline-note-media.md +++ b/apps/staged/docs/inline-note-media.md @@ -41,10 +41,13 @@ the current webview. Older QuickTime files without an `ftyp` header are rejected Chat attachments (file picker, paste, drop) share the validation code but are stored under the format their bytes actually are: browsers label `file.type` from the filename, so a WebP saved as `shot.png` is stored as `shot.webp` with -an `image/webp` MIME rather than rejected. Only unrecognized signatures, videos, -and oversized files are refused, and the composer shows the reason for pasted, -picked, and natively dropped files alike, clearing it when the message is sent -or the attachment removed. +an `image/webp` MIME rather than rejected. A dropped, pasted, or picked PDF (up +to 50 MiB) is stored as a PNG of its first page, rendered with CoreGraphics on +macOS; the PDF bytes themselves are not kept, and a `.pdf` reference in a note +is still refused. Only unrecognized signatures, videos, and oversized files are +refused, and the composer shows the reason for pasted, picked, and natively +dropped files alike, clearing it when the message is sent or the attachment +removed. On session completion, files are copied into the existing project `images/` store and destinations become `staged-media://.`. Captions and titles diff --git a/apps/staged/src-tauri/src/image_commands.rs b/apps/staged/src-tauri/src/image_commands.rs index 220f30e78..d50e79bc0 100644 --- a/apps/staged/src-tauri/src/image_commands.rs +++ b/apps/staged/src-tauri/src/image_commands.rs @@ -42,8 +42,7 @@ fn create_image_impl( .file_name() .and_then(|n| n.to_str()) .ok_or("Invalid filename")?; - let bytes = - crate::note_media::files::read_local(src, crate::note_media::files::MAX_IMAGE_SIZE)?; + let bytes = crate::note_media::files::read_local_attachment(src)?; crate::note_media::files::store_sniffed_image( &store, branch_id.as_deref(), @@ -165,10 +164,19 @@ pub(crate) fn create_image_from_data_impl( pending: Option, ) -> Result { use base64::Engine; - if data.len() as u64 > crate::note_media::files::MAX_IMAGE_SIZE.div_ceil(3) * 4 { - return Err("Image too large (max 10 MB)".into()); + let engine = base64::engine::general_purpose::STANDARD; + // The cap depends on the payload (a PDF may be larger than an image), so + // peek at the first six bytes — two base64 quanta — before decoding a + // payload that may turn out to be oversized. + let header = data + .get(..8) + .and_then(|quanta| engine.decode(quanta).ok()) + .unwrap_or_default(); + let (cap, what) = crate::note_media::files::attachment_limit(&header); + if data.len() as u64 > cap.div_ceil(3) * 4 { + return Err(crate::note_media::files::size_error(what, cap)); } - let bytes = base64::engine::general_purpose::STANDARD + let bytes = engine .decode(&data) .map_err(|e| format!("Invalid base64 data: {e}"))?; @@ -270,6 +278,86 @@ mod tests { ); } + /// PDFs get the larger input cap on both routes; images keep theirs. The + /// padded PDFs here are well-formed, so acceptance means the PNG of page + /// one was stored, not that the bytes merely got past the size check. + #[cfg(target_os = "macos")] + #[test] + fn pdf_input_cap_is_separate_from_the_image_cap() { + use crate::note_media::files::{MAX_IMAGE_SIZE, MAX_PDF_INPUT_SIZE}; + use crate::note_media::pdf::tests::{padded_pdf, PAGE_ONE}; + use crate::note_media::tests::PNG; + let f = Fixture::new(); + let large_pdf = padded_pdf(&[PAGE_ONE], MAX_IMAGE_SIZE as usize + 1); + + let image = attach(&f, "scan.pdf", "application/pdf", &large_pdf).unwrap(); + assert_eq!(image.mime_type, "image/png"); + assert!(image.size_bytes < MAX_IMAGE_SIZE as i64); + + let path = f.dir.path().join("dropped.pdf"); + std::fs::write(&path, &large_pdf).unwrap(); + let image = create_image_impl( + f.store.clone(), + Some(f.branch.id.clone()), + f.project.id.clone(), + path.to_string_lossy().into_owned(), + None, + ) + .unwrap(); + assert_eq!( + (image.mime_type.as_str(), image.filename.as_str()), + ("image/png", "dropped.png") + ); + + // Over the PDF cap: refused before decoding (base64) or reading (path). + let mut oversized = "JVBERi0x".to_owned(); // "%PDF-1" — a PDF signature + oversized.push_str(&"A".repeat((MAX_PDF_INPUT_SIZE.div_ceil(3) * 4) as usize)); + let err = create_image_from_data_impl( + f.store.clone(), + None, + f.project.id.clone(), + "huge.pdf".into(), + "application/pdf".into(), + oversized, + None, + ) + .unwrap_err(); + assert_eq!(err, "PDF exceeds 50 MB limit"); + let sparse = f.dir.path().join("huge.pdf"); + let mut file = std::fs::File::create(&sparse).unwrap(); + std::io::Write::write_all(&mut file, b"%PDF-1.4\n").unwrap(); + file.set_len(MAX_PDF_INPUT_SIZE + 1).unwrap(); + let err = create_image_impl( + f.store.clone(), + None, + f.project.id.clone(), + sparse.to_string_lossy().into_owned(), + None, + ) + .unwrap_err(); + assert_eq!(err, "PDF exceeds 50 MB limit"); + + // An image over its own cap is still refused with the image message, + // on both routes, and from metadata alone on the path route. + let mut png = PNG.to_vec(); + png.resize(MAX_IMAGE_SIZE as usize + 1, 0); + let err = attach(&f, "big.png", "image/png", &png).unwrap_err(); + assert_eq!(err, "media exceeds 10 MB limit"); + let sparse = f.dir.path().join("big.png"); + let mut file = std::fs::File::create(&sparse).unwrap(); + std::io::Write::write_all(&mut file, PNG).unwrap(); + file.set_len(MAX_IMAGE_SIZE + 1).unwrap(); + let err = create_image_impl( + f.store.clone(), + None, + f.project.id.clone(), + sparse.to_string_lossy().into_owned(), + None, + ) + .unwrap_err(); + assert_eq!(err, "media exceeds 10 MB limit"); + } + #[cfg(target_os = "macos")] #[test] fn malformed_pdf_is_rejected_without_hanging() { diff --git a/apps/staged/src-tauri/src/note_media/files.rs b/apps/staged/src-tauri/src/note_media/files.rs index 98090c454..7a52cde28 100644 --- a/apps/staged/src-tauri/src/note_media/files.rs +++ b/apps/staged/src-tauri/src/note_media/files.rs @@ -6,7 +6,26 @@ use std::io::Read; use std::path::Path; pub(crate) const MAX_IMAGE_SIZE: u64 = 10 * 1024 * 1024; +/// A PDF attachment may be far larger than the PNG of its first page that is +/// actually stored (scans and print-ready files routinely run 10–50 MB), so +/// `%PDF-` payloads get their own input cap. Only the rendered PNG has to +/// satisfy [`MAX_IMAGE_SIZE`]. +pub(crate) const MAX_PDF_INPUT_SIZE: u64 = 50 * 1024 * 1024; pub(crate) const MAX_VIDEO_SIZE: u64 = 100 * 1024 * 1024; + +pub(crate) fn size_error(what: &str, cap: u64) -> String { + format!("{what} exceeds {} MB limit", cap / 1024 / 1024) +} + +/// Input cap for a chat attachment, decided from its leading bytes, with the +/// noun its size error names so an oversized PDF is reported as such. +pub(crate) fn attachment_limit(header: &[u8]) -> (u64, &'static str) { + if super::pdf::is_pdf(header) { + (MAX_PDF_INPUT_SIZE, "PDF") + } else { + (MAX_IMAGE_SIZE, "media") + } +} pub(crate) const MAX_REMOTE_VIDEO_SIZE: i64 = 20 * 1024 * 1024; pub(crate) fn extension(filename: &str) -> String { @@ -79,7 +98,7 @@ pub(crate) fn validate_media( ) -> Result<&'static str, String> { let cap = size_limit(filename, allow_video)?; if bytes.len() as u64 > cap { - return Err(format!("media exceeds {} MB limit", cap / 1024 / 1024)); + return Err(size_error("media", cap)); } let ext = extension(filename); let valid = match (ext.as_str(), sniff_format(bytes)) { @@ -107,11 +126,9 @@ pub(crate) fn store_sniffed_image( filename: &str, bytes: &[u8], ) -> Result { - if bytes.len() as u64 > MAX_IMAGE_SIZE { - return Err(format!( - "media exceeds {} MB limit", - MAX_IMAGE_SIZE / 1024 / 1024 - )); + let (cap, what) = attachment_limit(bytes); + if bytes.len() as u64 > cap { + return Err(size_error(what, cap)); } // A PDF becomes an ordinary PNG attachment of its first page; nothing // downstream (store, viewer, agent prompt) ever sees PDF bytes. Only this @@ -181,6 +198,35 @@ impl From<&str> for ReadError { } pub(crate) fn read_local(path: &Path, cap: u64) -> Result, ReadError> { + let (file, len) = open_regular(path)?; + if len > cap { + return Err(size_error("media", cap).into()); + } + read_capped(file, Vec::new(), cap) +} + +/// [`read_local`] for a chat attachment, whose cap depends on what the file +/// is (see [`attachment_limit`]). The signature is read before the body so an +/// oversized image is still refused from its metadata, without buffering +/// anything like [`MAX_PDF_INPUT_SIZE`] first. +pub(crate) fn read_local_attachment(path: &Path) -> Result, ReadError> { + let (mut file, len) = open_regular(path)?; + let mut bytes = Vec::new(); + (&mut file) + .take(PDF_SIGNATURE_LEN) + .read_to_end(&mut bytes) + .map_err(|_| "cannot read media file")?; + let (cap, what) = attachment_limit(&bytes); + if len > cap { + return Err(size_error(what, cap).into()); + } + read_capped(file, bytes, cap) +} + +const PDF_SIGNATURE_LEN: u64 = b"%PDF-".len() as u64; + +/// Open a regular file for reading and return it with its current length. +fn open_regular(path: &Path) -> Result<(std::fs::File, u64), ReadError> { let mut options = std::fs::OpenOptions::new(); options.read(true); #[cfg(unix)] @@ -197,11 +243,13 @@ pub(crate) fn read_local(path: &Path, cap: u64) -> Result, ReadError> { if !metadata.is_file() { return Err("media must be a regular file".into()); } - if metadata.len() > cap { - return Err(format!("media exceeds {} MB limit", cap / 1024 / 1024).into()); - } - let mut bytes = Vec::new(); - file.take(cap + 1) + Ok((file, metadata.len())) +} + +/// Read the rest of `file` after `bytes` (already read from it), refusing a +/// file that grew past `cap` since its metadata was checked. +fn read_capped(file: std::fs::File, mut bytes: Vec, cap: u64) -> Result, ReadError> { + file.take(cap + 1 - bytes.len() as u64) .read_to_end(&mut bytes) .map_err(|_| "cannot read media file")?; if bytes.len() as u64 > cap { @@ -312,7 +360,7 @@ pub(crate) fn read_candidate( } let bytes = output.stdout; if bytes.len() as u64 > cap { - return Err(format!("media exceeds {} MB limit", cap / 1024 / 1024).into()); + return Err(size_error("media", cap).into()); } Ok(bytes) } else { diff --git a/apps/staged/src-tauri/src/note_media/pdf.rs b/apps/staged/src-tauri/src/note_media/pdf.rs index a449a5d71..05abfa877 100644 --- a/apps/staged/src-tauri/src/note_media/pdf.rs +++ b/apps/staged/src-tauri/src/note_media/pdf.rs @@ -7,7 +7,7 @@ //! Linux CI build stays green; they report that PDF attachments need macOS. #[cfg(target_os = "macos")] -use super::files::MAX_IMAGE_SIZE; +use super::files::{size_error, MAX_IMAGE_SIZE}; /// Long edge of the rendered page in pixels. A letter page lands at ~186 DPI, /// which keeps body text legible to a vision model without approaching the @@ -39,11 +39,6 @@ impl std::fmt::Debug for RenderedPage { } } -#[cfg(target_os = "macos")] -fn size_limit_error() -> String { - format!("media exceeds {} MB limit", MAX_IMAGE_SIZE / 1024 / 1024) -} - /// Render page 1 to a white-backed PNG whose long edge is `max_edge_px` /// (less when that would exceed [`MAX_SCALE`]). If the PNG is still over /// [`MAX_IMAGE_SIZE`] — photographic pages — it is re-rendered once at half @@ -94,7 +89,7 @@ pub(crate) fn render_first_page(bytes: &[u8], max_edge_px: u32) -> Result MAX_IMAGE_SIZE { - return Err(size_limit_error()); + return Err(size_error("media", MAX_IMAGE_SIZE)); } Ok(RenderedPage { png, page_count }) } @@ -205,6 +200,25 @@ pub(crate) mod tests { media_box: &str, crop_box: Option<&str>, rotate: Option, + ) -> Vec { + build_pdf(pages, media_box, crop_box, rotate, 0) + } + + /// [`tiny_pdf`] grown past `min_len` bytes with comment lines after the + /// header, so a large file is still a well-formed, instantly rendered PDF + /// (the xref offsets skip the comments) — for exercising the size caps. + pub(crate) fn padded_pdf(pages: &[&str], min_len: usize) -> Vec { + let pdf = build_pdf(pages, "[0 0 612 792]", None, None, min_len); + assert!(pdf.len() >= min_len); + pdf + } + + fn build_pdf( + pages: &[&str], + media_box: &str, + crop_box: Option<&str>, + rotate: Option, + padding: usize, ) -> Vec { let font_id = 3 + pages.len() * 2; let mut objects = vec![ @@ -236,6 +250,10 @@ pub(crate) mod tests { objects.push("<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".to_owned()); let mut out = b"%PDF-1.4\n".to_vec(); + let comment = format!("%{}\n", "x".repeat(4095)); + while out.len() < padding { + out.extend_from_slice(comment.as_bytes()); + } let mut offsets = Vec::new(); for (i, body) in objects.iter().enumerate() { offsets.push(out.len()); @@ -261,6 +279,10 @@ pub(crate) mod tests { pub(crate) const PAGE_ONE: &str = "BT /F1 24 Tf 72 700 Td (PAGE ONE Staged PDF probe) Tj ET\n4 w 72 650 m 540 650 l S"; + /// Page one's text plus a second rule, at y=400 pt, that page one lacks. + pub(crate) const PAGE_TWO: &str = + "BT /F1 24 Tf 72 700 Td (PAGE TWO should not appear) Tj ET\n4 w 72 400 m 540 400 l S"; + #[test] fn detects_the_pdf_signature_only() { assert!(is_pdf(&tiny_pdf(&[PAGE_ONE]))); @@ -274,8 +296,6 @@ pub(crate) mod tests { mod macos { use super::*; - const PAGE_TWO: &str = "BT /F1 24 Tf 72 700 Td (PAGE TWO should not appear) Tj ET"; - fn decode(png: &[u8]) -> tiny_skia::Pixmap { assert!(png.starts_with(b"\x89PNG\r\n\x1a\n")); tiny_skia::Pixmap::decode_png(png).unwrap() @@ -286,6 +306,12 @@ pub(crate) mod tests { (p.red(), p.green(), p.blue(), p.alpha()) == (255, 255, 255, 255) } + /// Bitmap row through the middle of a 4 pt rule drawn at `y_pt` on a + /// letter page rendered 2048 px tall. + fn letter_rule_row(y_pt: f64) -> u32 { + ((792.0 - y_pt) * 2048.0 / 792.0) as u32 + } + #[test] fn renders_first_page_to_opaque_png_at_the_long_edge() { let rendered = render_first_page(&tiny_pdf(&[PAGE_ONE, PAGE_TWO]), 2048).unwrap(); @@ -297,12 +323,18 @@ pub(crate) mod tests { assert!(is_opaque_white(&pixmap, x, y), "corner {x},{y}"); } // The rule at y=650pt from the bottom is drawn; everything is opaque. - let rule_y = ((792.0 - 650.0) * 2048.0 / 792.0) as u32; - let p = pixmap.pixel(800, rule_y).unwrap(); + let p = pixmap.pixel(800, letter_rule_row(650.0)).unwrap(); assert!(p.red() < 128 && p.alpha() == 255, "rule pixel {p:?}"); assert!(pixmap.pixels().iter().all(|p| p.alpha() == 255)); - // Page two's text sits where page one has nothing but white. - assert!(is_opaque_white(&pixmap, 1400, 2048 - rule_y - 300)); + // Page two's own rule, at y=400pt, is not drawn... + assert!(is_opaque_white(&pixmap, 800, letter_rule_row(400.0))); + // ...and would have been, had page two been the one rendered. + let swapped = decode(&render_first_page(&tiny_pdf(&[PAGE_TWO]), 2048).unwrap().png); + let p = swapped.pixel(800, letter_rule_row(400.0)).unwrap(); + assert!( + p.red() < 128 && p.alpha() == 255, + "page-two rule pixel {p:?}" + ); } #[test] diff --git a/apps/staged/src-tauri/src/note_media/tests.rs b/apps/staged/src-tauri/src/note_media/tests.rs index 83e2d6283..e3de9889e 100644 --- a/apps/staged/src-tauri/src/note_media/tests.rs +++ b/apps/staged/src-tauri/src/note_media/tests.rs @@ -573,6 +573,55 @@ fn validates_magic_and_caps_by_kind_without_reading_oversized_files() { } } +#[test] +fn attachment_reads_pick_their_cap_from_the_file_signature() { + let f = Fixture::new(); + let sparse = |name: &str, header: &[u8], len: u64| { + let path = f.dir.path().join(name); + let mut file = std::fs::File::create(&path).unwrap(); + std::io::Write::write_all(&mut file, header).unwrap(); + file.set_len(len).unwrap(); + path + }; + // A PDF between the image and PDF caps is read in full. + let path = sparse("a.pdf", b"%PDF-1.4\n", files::MAX_IMAGE_SIZE + 1); + let bytes = files::read_local_attachment(&path).unwrap(); + assert_eq!(bytes.len() as u64, files::MAX_IMAGE_SIZE + 1); + assert!(bytes.starts_with(b"%PDF-1.4\n")); + assert_eq!( + files::read_local_attachment(&sparse("b.pdf", b"%PDF-", files::MAX_PDF_INPUT_SIZE + 1)), + Err("PDF exceeds 50 MB limit".into()) + ); + // Anything else keeps the image cap, judged from metadata. + assert_eq!( + files::read_local_attachment(&sparse("c.png", PNG, files::MAX_IMAGE_SIZE + 1)), + Err("media exceeds 10 MB limit".into()) + ); + // Files shorter than the signature are read, not mistaken for PDFs. + assert_eq!( + files::read_local_attachment(&sparse("d.png", b"%PD", 3)).unwrap(), + b"%PD" + ); + assert_eq!( + files::read_local_attachment(f.dir.path()), + Err("media must be a regular file".into()) + ); + assert_eq!( + files::read_local_attachment(&f.dir.path().join("missing.pdf")), + Err(files::ReadError::NotFound("media file is unavailable")) + ); + + // The same decision for bytes already in memory. + let mut pdf = b"%PDF-1.4\n".to_vec(); + pdf.resize(files::MAX_PDF_INPUT_SIZE as usize + 1, b' '); + let err = files::store_sniffed_image(&f.store, None, &f.project.id, None, "a.pdf", &pdf); + assert_eq!(err.unwrap_err(), "PDF exceeds 50 MB limit"); + let mut png = PNG.to_vec(); + png.resize(files::MAX_IMAGE_SIZE as usize + 1, 0); + let err = files::store_sniffed_image(&f.store, None, &f.project.id, None, "a.png", &png); + assert_eq!(err.unwrap_err(), "media exceeds 10 MB limit"); +} + #[test] fn failed_note_save_rolls_back_new_files_but_keeps_previous_media() { let f = Fixture::new(); diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index 243c34b95..998f6116c 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -62,6 +62,7 @@ import NoteModal from '../notes/NoteModal.svelte'; import WriteNoteModal from '../notes/WriteNoteModal.svelte'; import { splitNoteMarkdown } from '../notes/noteMarkdown'; + import { attachBasename } from '../sessions/attachErrors'; import * as AlertDialog from '$lib/components/ui/alert-dialog'; import { Button } from '$lib/components/ui/button'; import { @@ -1780,10 +1781,11 @@ console.error('Failed to create image from drop:', e); // Same shape as the text path, so a rejected PDF or oversized // image ("could not open PDF", "media exceeds 10 MB limit") is - // not a silent no-op. + // not a silent no-op. The extension stays in the name, as in the + // composer's attach errors: it is what explains a PDF failure. const reason = e instanceof Error ? e.message : typeof e === 'string' ? e : null; toast.error('Error', { - description: `Could not attach "${fileNameFromPath(filePath)}"${reason ? ` \u2014 ${reason}` : ''}`, + description: `Could not attach "${attachBasename(filePath)}"${reason ? ` \u2014 ${reason}` : ''}`, }); } }) diff --git a/apps/staged/src/lib/features/sessions/ChatComposer.svelte b/apps/staged/src/lib/features/sessions/ChatComposer.svelte index 776a2cbf2..c92680bf1 100644 --- a/apps/staged/src/lib/features/sessions/ChatComposer.svelte +++ b/apps/staged/src/lib/features/sessions/ChatComposer.svelte @@ -142,11 +142,11 @@ class="file-input-hidden" onchange={handleImageFileSelect} /> - +