From 4b5a908ebcda03dc2ccf2e676f9ceddd7fe4f286 Mon Sep 17 00:00:00 2001 From: tulsi Date: Mon, 13 Apr 2026 14:21:53 -0700 Subject: [PATCH 1/2] fix: harden attachment loading --- scripts/check-file-sizes.mjs | 4 +- src-tauri/src/commands/system.rs | 78 ++++++++++++++++--- .../chat/hooks/useChatInputAttachments.ts | 8 +- src/features/chat/ui/ChatInput.tsx | 4 +- 4 files changed, 78 insertions(+), 16 deletions(-) diff --git a/scripts/check-file-sizes.mjs b/scripts/check-file-sizes.mjs index 1079a49..05cbfc3 100644 --- a/scripts/check-file-sizes.mjs +++ b/scripts/check-file-sizes.mjs @@ -66,9 +66,9 @@ const EXCEPTIONS = { "Session prepare/load/list logic, working-dir updates, wait_for_replay_drain helper with iteration cap, and composite prepared-session reuse remain colocated while ACP session ownership stabilizes.", }, "src-tauri/src/commands/system.rs": { - limit: 540, + limit: 580, justification: - "Desktop system commands still centralize file mentions, attachment inspection, image loading, and export helpers in one Tauri command surface.", + "Desktop system commands still centralize file mentions, attachment inspection, guarded image loading, and export helpers in one Tauri command surface.", }, }; diff --git a/src-tauri/src/commands/system.rs b/src-tauri/src/commands/system.rs index 9025313..91b498b 100644 --- a/src-tauri/src/commands/system.rs +++ b/src-tauri/src/commands/system.rs @@ -10,6 +10,7 @@ use std::path::{Path, PathBuf}; const DEFAULT_FILE_MENTION_LIMIT: usize = 1500; const MAX_FILE_MENTION_LIMIT: usize = 5000; const MAX_SCAN_DEPTH: usize = 8; +const MAX_IMAGE_ATTACHMENT_BYTES: u64 = 20 * 1024 * 1024; #[derive(Serialize, Clone, Debug, PartialEq, Eq)] #[serde(rename_all = "camelCase")] @@ -178,10 +179,13 @@ fn inspect_attachment_path(path: &Path) -> Result { }) } -#[tauri::command] -pub fn inspect_attachment_paths(paths: Vec) -> Result, String> { +fn normalized_path_key(path: &Path) -> String { + path.to_string_lossy().into_owned() +} + +fn normalize_attachment_paths(paths: Vec) -> Vec { let mut seen = HashSet::new(); - let mut attachments = Vec::new(); + let mut normalized = Vec::new(); for raw_path in paths { let trimmed = raw_path.trim(); @@ -190,11 +194,20 @@ pub fn inspect_attachment_paths(paths: Vec) -> Result) -> Result, String> { + let mut attachments = Vec::new(); + + for path in normalize_attachment_paths(paths) { attachments.push(inspect_attachment_path(&path)?); } @@ -212,6 +225,16 @@ pub fn read_image_attachment(path: String) -> Result MAX_IMAGE_ATTACHMENT_BYTES { + return Err(format!( + "Image attachment '{}' exceeds the {} MB limit", + attachment.path, + MAX_IMAGE_ATTACHMENT_BYTES / (1024 * 1024) + )); + } + let bytes = fs::read(&attachment.path) .map_err(|error| format!("Failed to read image '{}': {}", attachment.path, error))?; @@ -230,7 +253,7 @@ fn normalize_roots(roots: Vec) -> Vec { continue; } let path = PathBuf::from(trimmed); - let key = path.to_string_lossy().to_lowercase(); + let key = normalized_path_key(&path); if dedup.insert(key) { normalized.push(path); } @@ -291,7 +314,7 @@ fn scan_files_for_mentions(roots: Vec, max_results: Option) -> Ve continue; } let path_str = entry.path().to_string_lossy().to_string(); - let dedup_key = path_str.to_lowercase(); + let dedup_key = path_str.clone(); if seen.insert(dedup_key) { files.push(path_str); } @@ -314,13 +337,15 @@ pub async fn list_files_for_mentions( #[cfg(test)] mod tests { use super::{ - build_file_tree_entry, inspect_attachment_path, read_directory_entries, - read_image_attachment, scan_files_for_mentions, + build_file_tree_entry, inspect_attachment_path, normalize_attachment_paths, + read_directory_entries, read_image_attachment, scan_files_for_mentions, + MAX_IMAGE_ATTACHMENT_BYTES, }; use base64::Engine; use std::fs; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; + use std::path::PathBuf; use std::process::Command; use tempfile::tempdir; @@ -508,4 +533,37 @@ mod tests { assert_eq!(payload.mime_type, "image/png"); assert!(!payload.base64.is_empty()); } + + #[test] + fn preserves_case_distinctions_when_deduping_attachment_paths() { + let normalized = normalize_attachment_paths(vec![ + "/tmp/Readme.md".into(), + "/tmp/README.md".into(), + "/tmp/Readme.md".into(), + ]); + + assert_eq!( + normalized, + vec![ + PathBuf::from("/tmp/Readme.md"), + PathBuf::from("/tmp/README.md") + ] + ); + } + + #[test] + fn rejects_oversized_image_attachment_payloads() { + let dir = tempdir().expect("tempdir"); + let image = dir.path().join("huge.png"); + fs::write( + &image, + vec![0_u8; (MAX_IMAGE_ATTACHMENT_BYTES as usize) + 1], + ) + .expect("oversized image file"); + + let error = + read_image_attachment(image.to_string_lossy().into_owned()).expect_err("size limit"); + + assert!(error.contains("exceeds the 20 MB limit")); + } } diff --git a/src/features/chat/hooks/useChatInputAttachments.ts b/src/features/chat/hooks/useChatInputAttachments.ts index 2c1aaa8..50e472d 100644 --- a/src/features/chat/hooks/useChatInputAttachments.ts +++ b/src/features/chat/hooks/useChatInputAttachments.ts @@ -28,6 +28,10 @@ function pathToPreviewUrl(path: string) { : path; } +function attachmentPathKey(path?: string) { + return path ?? null; +} + async function createImageAttachmentFromFile( file: File, ): Promise { @@ -95,13 +99,13 @@ export function useChatInputAttachments() { setAttachments((previous) => { const seenPaths = new Set( previous - .map((attachment) => attachment.path?.toLowerCase()) + .map((attachment) => attachmentPathKey(attachment.path)) .filter((value): value is string => Boolean(value)), ); const next = [...previous]; for (const attachment of incoming) { - const pathKey = attachment.path?.toLowerCase(); + const pathKey = attachmentPathKey(attachment.path); if (pathKey && seenPaths.has(pathKey)) { revokeAttachmentPreview(attachment); continue; diff --git a/src/features/chat/ui/ChatInput.tsx b/src/features/chat/ui/ChatInput.tsx index 715a0fe..fdf2bea 100644 --- a/src/features/chat/ui/ChatInput.tsx +++ b/src/features/chat/ui/ChatInput.tsx @@ -280,7 +280,7 @@ export function ChatInput({ title: t("attachments.chooseFilesDialogTitle"), multiple: true, }); - void addPathAttachments(normalizeDialogSelection(selected)); + await addPathAttachments(normalizeDialogSelection(selected)); } catch { // Dialog plugin may be unavailable in some environments. } @@ -297,7 +297,7 @@ export function ChatInput({ title: t("attachments.chooseFoldersDialogTitle"), multiple: true, }); - void addPathAttachments(normalizeDialogSelection(selected)); + await addPathAttachments(normalizeDialogSelection(selected)); } catch { // Dialog plugin may be unavailable in some environments. } From c49b48807ed278499514a3bbab24f98c57c9e0d5 Mon Sep 17 00:00:00 2001 From: tulsi Date: Mon, 13 Apr 2026 14:35:24 -0700 Subject: [PATCH 2/2] fix: normalize path dedupe by platform --- scripts/check-file-sizes.mjs | 4 +- src-tauri/src/commands/system.rs | 59 +++++++++++++++---- .../chat/hooks/useChatInputAttachments.ts | 7 ++- .../__tests__/ChatInput.attachments.test.tsx | 37 ++++++++++++ 4 files changed, 93 insertions(+), 14 deletions(-) diff --git a/scripts/check-file-sizes.mjs b/scripts/check-file-sizes.mjs index 05cbfc3..d7bfa60 100644 --- a/scripts/check-file-sizes.mjs +++ b/scripts/check-file-sizes.mjs @@ -66,9 +66,9 @@ const EXCEPTIONS = { "Session prepare/load/list logic, working-dir updates, wait_for_replay_drain helper with iteration cap, and composite prepared-session reuse remain colocated while ACP session ownership stabilizes.", }, "src-tauri/src/commands/system.rs": { - limit: 580, + limit: 620, justification: - "Desktop system commands still centralize file mentions, attachment inspection, guarded image loading, and export helpers in one Tauri command surface.", + "Desktop system commands still centralize file mentions, attachment inspection, platform-aware path dedupe, guarded image loading, and export helpers in one Tauri command surface.", }, }; diff --git a/src-tauri/src/commands/system.rs b/src-tauri/src/commands/system.rs index 91b498b..46be344 100644 --- a/src-tauri/src/commands/system.rs +++ b/src-tauri/src/commands/system.rs @@ -180,7 +180,19 @@ fn inspect_attachment_path(path: &Path) -> Result { } fn normalized_path_key(path: &Path) -> String { - path.to_string_lossy().into_owned() + if let Ok(canonical) = path.canonicalize() { + return canonical.to_string_lossy().into_owned(); + } + + let raw = path.to_string_lossy().into_owned(); + #[cfg(any(target_os = "macos", target_os = "windows"))] + { + raw.to_lowercase() + } + #[cfg(not(any(target_os = "macos", target_os = "windows")))] + { + raw + } } fn normalize_attachment_paths(paths: Vec) -> Vec { @@ -314,7 +326,7 @@ fn scan_files_for_mentions(roots: Vec, max_results: Option) -> Ve continue; } let path_str = entry.path().to_string_lossy().to_string(); - let dedup_key = path_str.clone(); + let dedup_key = normalized_path_key(entry.path()); if seen.insert(dedup_key) { files.push(path_str); } @@ -338,7 +350,7 @@ pub async fn list_files_for_mentions( mod tests { use super::{ build_file_tree_entry, inspect_attachment_path, normalize_attachment_paths, - read_directory_entries, read_image_attachment, scan_files_for_mentions, + normalize_roots, read_directory_entries, read_image_attachment, scan_files_for_mentions, MAX_IMAGE_ATTACHMENT_BYTES, }; use base64::Engine; @@ -535,20 +547,45 @@ mod tests { } #[test] - fn preserves_case_distinctions_when_deduping_attachment_paths() { + fn dedupes_attachment_paths_using_platform_path_rules() { let normalized = normalize_attachment_paths(vec![ "/tmp/Readme.md".into(), "/tmp/README.md".into(), "/tmp/Readme.md".into(), ]); - assert_eq!( - normalized, - vec![ - PathBuf::from("/tmp/Readme.md"), - PathBuf::from("/tmp/README.md") - ] - ); + if cfg!(any(target_os = "macos", target_os = "windows")) { + assert_eq!(normalized, vec![PathBuf::from("/tmp/Readme.md")]); + } else { + assert_eq!( + normalized, + vec![ + PathBuf::from("/tmp/Readme.md"), + PathBuf::from("/tmp/README.md") + ] + ); + } + } + + #[test] + fn dedupes_mention_roots_using_platform_path_rules() { + let normalized = normalize_roots(vec![ + "/tmp/Workspace".into(), + "/tmp/workspace".into(), + "/tmp/Workspace".into(), + ]); + + if cfg!(any(target_os = "macos", target_os = "windows")) { + assert_eq!(normalized, vec![PathBuf::from("/tmp/Workspace")]); + } else { + assert_eq!( + normalized, + vec![ + PathBuf::from("/tmp/Workspace"), + PathBuf::from("/tmp/workspace") + ] + ); + } } #[test] diff --git a/src/features/chat/hooks/useChatInputAttachments.ts b/src/features/chat/hooks/useChatInputAttachments.ts index 50e472d..8c202c8 100644 --- a/src/features/chat/hooks/useChatInputAttachments.ts +++ b/src/features/chat/hooks/useChatInputAttachments.ts @@ -10,6 +10,7 @@ import type { ChatFileAttachmentDraft, ChatImageAttachmentDraft, } from "@/shared/types/messages"; +import { getPlatform } from "@/shared/lib/platform"; import { resizeImage } from "../lib/resizeImage"; function isBlobPreview(url: string) { @@ -29,7 +30,11 @@ function pathToPreviewUrl(path: string) { } function attachmentPathKey(path?: string) { - return path ?? null; + if (!path) { + return null; + } + + return getPlatform() === "linux" ? path : path.toLowerCase(); } async function createImageAttachmentFromFile( diff --git a/src/features/chat/ui/__tests__/ChatInput.attachments.test.tsx b/src/features/chat/ui/__tests__/ChatInput.attachments.test.tsx index ab5938d..d4df797 100644 --- a/src/features/chat/ui/__tests__/ChatInput.attachments.test.tsx +++ b/src/features/chat/ui/__tests__/ChatInput.attachments.test.tsx @@ -11,6 +11,10 @@ vi.mock("@/features/providers/hooks/useAgentProviderStatus", () => ({ }), })); +vi.mock("@/shared/lib/platform", () => ({ + getPlatform: () => "mac", +})); + const mockListFilesForMentions = vi.fn< (roots: string[], maxResults?: number) => Promise >(async () => []); @@ -176,4 +180,37 @@ describe("ChatInput attachments", () => { expect(screen.getByAltText("Attachment 2")).toBeInTheDocument(); }); }); + + it("dedupes path attachments that differ only by case on case-insensitive platforms", async () => { + const user = userEvent.setup(); + mockOpenDialog.mockResolvedValue("/Users/test/report.pdf"); + mockInspectAttachmentPaths + .mockResolvedValueOnce([ + { + name: "report.pdf", + path: "/Users/test/report.pdf", + kind: "file", + mimeType: "application/pdf", + }, + ]) + .mockResolvedValueOnce([ + { + name: "report.pdf", + path: "/users/test/REPORT.pdf", + kind: "file", + mimeType: "application/pdf", + }, + ]); + + render(); + + await user.click(screen.getByRole("button", { name: /^attach$/i })); + await user.click(screen.getByRole("menuitem", { name: /^file$/i })); + expect(await screen.findByText("report.pdf")).toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: /^attach$/i })); + await user.click(screen.getByRole("menuitem", { name: /^file$/i })); + + expect(screen.getAllByText("report.pdf")).toHaveLength(1); + }); });