From bc93b540cdcf70b037b9f4d4c32595297bcc359e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 19 Sep 2026 03:24:35 -0400 Subject: [PATCH] fix(clipboard): the macOS image probe writes into a private scratch dir The osascript fallback hands the pasted raster to kigi through a file. Every kigi process used the same three paths in the shared temp dir (`kigi-clipboard-probe.{png,tiff,jpg}`). Two sessions pasting at once, or a stale file from an earlier probe, could swap the file between the AppleScript's write and kigi's read: the wrong image went to the model. `ProbeTemps` owns a fresh 0700 directory per probe and removes it on drop, so the hand-written removals are gone. The script builder, the raster reader, and the read flow move out of the cfg(macos) module into `clipboard/osascript_probe.rs`, which compiles under test on every platform; the flow takes the script runner as a closure. The script text with a scratch dir is byte-identical to the old builder's (checked once against the old function, then the copy was dropped). A file paste needs no scratch file. When the directory cannot be made, the probe warns, sends the file-URL half of the script only, and answers file URLs as before; with no file URL either, the scratch error surfaces instead of an empty board. `get_image` needs the file and fails fast. Tests (all platforms): probes never share a path, the directory goes away with the probe, mode 0700 on unix (Linux under WSL), the reported class is read from this probe's file, file URLs survive a missing scratch dir, and no dir plus no file URL is an error. The remaining cfg(macos) callers were type-checked in a stubbed scratch crate; the warm-cache workflow builds both darwin targets before the tag. Ported from upstream (grok-build shared/clipboard.rs ProbeTemps and osascript_attachments, 1.0.33). --- crates/codegen/kigi-shared/src/clipboard.rs | 187 +------------ .../src/clipboard/osascript_probe.rs | 246 ++++++++++++++++++ 2 files changed, 258 insertions(+), 175 deletions(-) create mode 100644 crates/codegen/kigi-shared/src/clipboard/osascript_probe.rs diff --git a/crates/codegen/kigi-shared/src/clipboard.rs b/crates/codegen/kigi-shared/src/clipboard.rs index 818ebfa..5ea7c26 100644 --- a/crates/codegen/kigi-shared/src/clipboard.rs +++ b/crates/codegen/kigi-shared/src/clipboard.rs @@ -544,13 +544,16 @@ mod attachments_protocol { } } +#[cfg(any(target_os = "macos", test))] +mod osascript_probe; + // macOS: subprocess-based clipboard (no AppKit linkage) #[cfg(target_os = "macos")] mod platform { use std::process::{Command, Stdio}; use std::sync::OnceLock; - use super::attachments_protocol::{FURL_MARKER, IMAGE_MARKER, parse_attachments_output}; + use super::osascript_probe::{ProbeTemps, osascript_attachments, read_probe_raster}; use super::{ClipboardAttachments, ImageData}; // Fast pasteboard probes (NSPasteboard via lazy dlopen) @@ -759,153 +762,15 @@ mod platform { Ok(output.stdout) } - fn attachments_probe_temp_paths() -> (std::path::PathBuf, std::path::PathBuf, std::path::PathBuf) - { - let temp_dir = std::env::temp_dir(); - ( - temp_dir.join("kigi-clipboard-probe.png"), - temp_dir.join("kigi-clipboard-probe.tiff"), - temp_dir.join("kigi-clipboard-probe.jpg"), - ) - } - - fn read_clipboard_image_from_class( - class: &str, - path_png: &std::path::Path, - path_tiff: &std::path::Path, - path_jpg: &std::path::Path, - ) -> anyhow::Result> { - let (temp_path, mime) = match class { - "PNGf" => (path_png, "image/png"), - "TIFF" => (path_tiff, "image/tiff"), - "JPEG" => (path_jpg, "image/jpeg"), - _ => return Ok(None), - }; - - let data = match std::fs::read(temp_path) { - Ok(bytes) if !bytes.is_empty() => bytes, - Ok(_) => { - let _ = std::fs::remove_file(temp_path); - return Ok(None); - } - Err(e) => { - let _ = std::fs::remove_file(temp_path); - return Err(anyhow::anyhow!("failed to read clipboard temp file: {e}")); - } - }; - - let _ = std::fs::remove_file(temp_path); - Ok(Some(ImageData { - data, - mime_type: mime.to_owned(), - })) - } - - fn remove_attachment_probe_temps( - path_png: &std::path::Path, - path_tiff: &std::path::Path, - path_jpg: &std::path::Path, - ) { - let _ = std::fs::remove_file(path_png); - let _ = std::fs::remove_file(path_tiff); - let _ = std::fs::remove_file(path_jpg); - } - - fn attachments_osascript( - path_png: &std::path::Path, - path_tiff: &std::path::Path, - path_jpg: &std::path::Path, - ) -> String { - format!( - "set furlOut to \"none\"\n\ - try\n\ - set urlList to the clipboard as list\n\ - set out to \"\"\n\ - repeat with u in urlList\n\ - try\n\ - set itemRef to contents of u as \u{00AB}class furl\u{00BB}\n\ - set out to out & POSIX path of itemRef & \"\\n\"\n\ - end try\n\ - end repeat\n\ - if out is not \"\" then\n\ - set furlOut to out\n\ - else\n\ - try\n\ - set urlRef to the clipboard as \u{00AB}class furl\u{00BB}\n\ - set furlOut to POSIX path of urlRef\n\ - on error\n\ - end try\n\ - end if\n\ - on error\n\ - try\n\ - set urlRef to the clipboard as \u{00AB}class furl\u{00BB}\n\ - set furlOut to POSIX path of urlRef\n\ - on error\n\ - end try\n\ - end try\n\ - set imageOut to \"NONE\"\n\ - if furlOut is \"none\" then\n\ - try\n\ - set imgData to the clipboard as \u{00AB}class PNGf\u{00BB}\n\ - set filePath to POSIX file \"{png}\" as text\n\ - set fRef to open for access file filePath with write permission\n\ - set eof of fRef to 0\n\ - write imgData to fRef\n\ - close access fRef\n\ - set imageOut to \"PNGf\"\n\ - on error\n\ - try\n\ - set imgData to the clipboard as \u{00AB}class TIFF\u{00BB}\n\ - set filePath to POSIX file \"{tiff}\" as text\n\ - set fRef to open for access file filePath with write permission\n\ - set eof of fRef to 0\n\ - write imgData to fRef\n\ - close access fRef\n\ - set imageOut to \"TIFF\"\n\ - on error\n\ - try\n\ - set imgData to the clipboard as \u{00AB}class JPEG\u{00BB}\n\ - set filePath to POSIX file \"{jpg}\" as text\n\ - set fRef to open for access file filePath with write permission\n\ - set eof of fRef to 0\n\ - write imgData to fRef\n\ - close access fRef\n\ - set imageOut to \"JPEG\"\n\ - on error\n\ - end try\n\ - end try\n\ - end try\n\ - end if\n\ - return \"{furl_marker}\" & linefeed & furlOut & linefeed & \"{image_marker}\" & linefeed & \"IMAGE:\" & imageOut", - png = path_png.display(), - tiff = path_tiff.display(), - jpg = path_jpg.display(), - furl_marker = FURL_MARKER, - image_marker = IMAGE_MARKER, - ) - } - - fn run_attachments_osascript() -> anyhow::Result { - let (path_png, path_tiff, path_jpg) = attachments_probe_temp_paths(); - remove_attachment_probe_temps(&path_png, &path_tiff, &path_jpg); - - let script = attachments_osascript(&path_png, &path_tiff, &path_jpg); - + fn run_osascript(script: &str) -> anyhow::Result> { let mut cmd = Command::new("osascript"); cmd.arg("-e") - .arg(&script) + .arg(script) .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::piped()); kigi_tools::util::detach_std_command(&mut cmd); - let stdout = match checked_command_stdout("osascript", cmd.output()) { - Ok(stdout) => stdout, - Err(error) => { - remove_attachment_probe_temps(&path_png, &path_tiff, &path_jpg); - return Err(error); - } - }; - Ok(String::from_utf8_lossy(&stdout).into_owned()) + checked_command_stdout("osascript", cmd.output()) } /// Unified furl-then-image pasteboard probe. @@ -923,28 +788,7 @@ mod platform { image: Some(image), }); } - let raw = run_attachments_osascript()?; - if raw.trim().is_empty() { - return Ok(ClipboardAttachments::default()); - } - - let (path_png, path_tiff, path_jpg) = attachments_probe_temp_paths(); - let (file_urls, image_class) = parse_attachments_output(&raw); - let image = if file_urls.is_some() { - remove_attachment_probe_temps(&path_png, &path_tiff, &path_jpg); - None - } else { - match image_class { - Some(class) => { - read_clipboard_image_from_class(class, &path_png, &path_tiff, &path_jpg)? - } - None => { - remove_attachment_probe_temps(&path_png, &path_tiff, &path_jpg); - None - } - } - }; - Ok(ClipboardAttachments { file_urls, image }) + osascript_attachments(ProbeTemps::new(), run_osascript) } /// Read text via `pbpaste -Prefer txt`. @@ -1020,8 +864,8 @@ mod platform { return Ok(Some(image)); } - let (path_png, path_tiff, path_jpg) = attachments_probe_temp_paths(); - remove_attachment_probe_temps(&path_png, &path_tiff, &path_jpg); + let temps = ProbeTemps::new()?; + let (path_png, path_tiff, path_jpg) = temps.paths(); // Image-only AppleScript (ImageOnly paste route). Unicode guillemets // (\u{AB}/\u{BB}) are required for `«class …»` in `osascript -e`. @@ -1069,20 +913,13 @@ mod platform { .stdout(Stdio::piped()) .stderr(Stdio::null()); kigi_tools::util::detach_std_command(&mut cmd); - let stdout = match checked_command_stdout("osascript", cmd.output()) { - Ok(stdout) => stdout, - Err(error) => { - remove_attachment_probe_temps(&path_png, &path_tiff, &path_jpg); - return Err(error); - } - }; + let stdout = checked_command_stdout("osascript", cmd.output())?; let class = String::from_utf8_lossy(&stdout); let class = class.trim(); if class == "none" { - remove_attachment_probe_temps(&path_png, &path_tiff, &path_jpg); return Ok(None); } - read_clipboard_image_from_class(class, &path_png, &path_tiff, &path_jpg) + read_probe_raster(class, &temps) } /// Read file URLs from the macOS pasteboard via `osascript`. diff --git a/crates/codegen/kigi-shared/src/clipboard/osascript_probe.rs b/crates/codegen/kigi-shared/src/clipboard/osascript_probe.rs new file mode 100644 index 0000000..b39b874 --- /dev/null +++ b/crates/codegen/kigi-shared/src/clipboard/osascript_probe.rs @@ -0,0 +1,246 @@ +//! The macOS `osascript` attachment probe: scratch files, script text, and the read flow. + +use std::path::PathBuf; + +use super::attachments_protocol::{FURL_MARKER, IMAGE_MARKER, parse_attachments_output}; +use super::{ClipboardAttachments, ImageData}; + +/// Private 0700 dir, removed on drop: a shared path let a concurrent kigi swap the raster. +pub(crate) struct ProbeTemps(tempfile::TempDir); + +impl ProbeTemps { + pub(crate) fn new() -> anyhow::Result { + let mut builder = tempfile::Builder::new(); + builder.prefix("kigi-clipboard-probe-"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + builder.permissions(std::fs::Permissions::from_mode(0o700)); + } + builder + .tempdir() + .map(Self) + .map_err(|e| anyhow::anyhow!("failed to create clipboard probe dir: {e}")) + } + + /// PNG, TIFF, JPEG: the AppleScript writes the first class that coerces. + pub(crate) fn paths(&self) -> (PathBuf, PathBuf, PathBuf) { + ( + self.0.path().join("probe.png"), + self.0.path().join("probe.tiff"), + self.0.path().join("probe.jpg"), + ) + } +} + +/// File URLs first, a raster only when none are present; without `temps` the raster half is skipped. +pub(crate) fn attachments_osascript(temps: Option<&ProbeTemps>) -> String { + let image_probe = temps.map_or_else(String::new, raster_half); + format!( + "set furlOut to \"none\"\n\ + try\n\ + set urlList to the clipboard as list\n\ + set out to \"\"\n\ + repeat with u in urlList\n\ + try\n\ + set itemRef to contents of u as \u{00AB}class furl\u{00BB}\n\ + set out to out & POSIX path of itemRef & \"\\n\"\n\ + end try\n\ + end repeat\n\ + if out is not \"\" then\n\ + set furlOut to out\n\ + else\n\ + try\n\ + set urlRef to the clipboard as \u{00AB}class furl\u{00BB}\n\ + set furlOut to POSIX path of urlRef\n\ + on error\n\ + end try\n\ + end if\n\ + on error\n\ + try\n\ + set urlRef to the clipboard as \u{00AB}class furl\u{00BB}\n\ + set furlOut to POSIX path of urlRef\n\ + on error\n\ + end try\n\ + end try\n\ + set imageOut to \"NONE\"\n\ + {image_probe}\ + return \"{furl_marker}\" & linefeed & furlOut & linefeed & \"{image_marker}\" & linefeed & \"IMAGE:\" & imageOut", + furl_marker = FURL_MARKER, + image_marker = IMAGE_MARKER, + ) +} + +fn raster_half(temps: &ProbeTemps) -> String { + let (path_png, path_tiff, path_jpg) = temps.paths(); + format!( + "if furlOut is \"none\" then\n\ + try\n\ + set imgData to the clipboard as \u{00AB}class PNGf\u{00BB}\n\ + set filePath to POSIX file \"{png}\" as text\n\ + set fRef to open for access file filePath with write permission\n\ + set eof of fRef to 0\n\ + write imgData to fRef\n\ + close access fRef\n\ + set imageOut to \"PNGf\"\n\ + on error\n\ + try\n\ + set imgData to the clipboard as \u{00AB}class TIFF\u{00BB}\n\ + set filePath to POSIX file \"{tiff}\" as text\n\ + set fRef to open for access file filePath with write permission\n\ + set eof of fRef to 0\n\ + write imgData to fRef\n\ + close access fRef\n\ + set imageOut to \"TIFF\"\n\ + on error\n\ + try\n\ + set imgData to the clipboard as \u{00AB}class JPEG\u{00BB}\n\ + set filePath to POSIX file \"{jpg}\" as text\n\ + set fRef to open for access file filePath with write permission\n\ + set eof of fRef to 0\n\ + write imgData to fRef\n\ + close access fRef\n\ + set imageOut to \"JPEG\"\n\ + on error\n\ + end try\n\ + end try\n\ + end try\n\ + end if\n", + png = path_png.display(), + tiff = path_tiff.display(), + jpg = path_jpg.display(), + ) +} + +/// The class on stdout picks the file and MIME; an empty file means no raster. +pub(crate) fn read_probe_raster( + class: &str, + temps: &ProbeTemps, +) -> anyhow::Result> { + let (path_png, path_tiff, path_jpg) = temps.paths(); + let (temp_path, mime) = match class { + "PNGf" => (path_png, "image/png"), + "TIFF" => (path_tiff, "image/tiff"), + "JPEG" => (path_jpg, "image/jpeg"), + _ => return Ok(None), + }; + let data = std::fs::read(&temp_path) + .map_err(|e| anyhow::anyhow!("failed to read clipboard temp file: {e}"))?; + if data.is_empty() { + return Ok(None); + } + Ok(Some(ImageData { + data, + mime_type: mime.to_owned(), + })) +} + +/// `run` executes a script and returns its stdout; a file paste needs no scratch dir. +pub(crate) fn osascript_attachments( + temps: anyhow::Result, + run: impl FnOnce(&str) -> anyhow::Result>, +) -> anyhow::Result { + if let Err(error) = &temps { + tracing::warn!(%error, "clipboard probe reads file URLs only"); + } + let stdout = run(&attachments_osascript(temps.as_ref().ok()))?; + let (file_urls, image_class) = parse_attachments_output(&String::from_utf8_lossy(&stdout)); + if file_urls.is_some() { + return Ok(ClipboardAttachments { + file_urls, + image: None, + }); + } + let temps = temps?; + let image = match image_class { + Some(class) => read_probe_raster(class, &temps)?, + None => None, + }; + Ok(ClipboardAttachments { + file_urls: None, + image, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn attachments_stdout(furl: &str, image: &str) -> Vec { + format!("{FURL_MARKER}\n{furl}\n{IMAGE_MARKER}\nIMAGE:{image}").into_bytes() + } + + #[test] + fn two_probes_never_share_a_path() { + let first = ProbeTemps::new().unwrap(); + let second = ProbeTemps::new().unwrap(); + assert_ne!(first.paths().0, second.paths().0); + } + + #[test] + fn scratch_dir_goes_away_with_the_probe() { + let temps = ProbeTemps::new().unwrap(); + let (png, tiff, jpg) = temps.paths(); + let dir = png.parent().unwrap().to_path_buf(); + assert_eq!(tiff.parent().unwrap(), dir); + assert_eq!(jpg.parent().unwrap(), dir); + std::fs::write(&png, b"raster").unwrap(); + + drop(temps); + assert!(!dir.exists(), "{} must be removed", dir.display()); + } + + #[cfg(unix)] + #[test] + fn scratch_dir_is_private() { + use std::os::unix::fs::PermissionsExt; + let temps = ProbeTemps::new().unwrap(); + let dir = temps.paths().0.parent().unwrap().to_path_buf(); + let mode = std::fs::metadata(dir).unwrap().permissions().mode(); + assert_eq!(mode & 0o777, 0o700); + } + + #[test] + fn reported_class_is_read_from_this_probes_file() { + let temps = ProbeTemps::new().unwrap(); + let (_png, tiff, _jpg) = temps.paths(); + std::fs::write(&tiff, b"MM\x00\x2a").unwrap(); + let script_names_the_file = tiff.display().to_string(); + + let read = osascript_attachments(Ok(temps), |script| { + assert!(script.contains(&script_names_the_file), "{script}"); + Ok(attachments_stdout("none", "TIFF")) + }) + .unwrap(); + + let image = read.image.expect("raster attached"); + assert_eq!(image.mime_type, "image/tiff"); + assert_eq!(image.data, b"MM\x00\x2a".to_vec()); + assert_eq!(read.file_urls, None); + } + + #[test] + fn file_urls_survive_a_missing_scratch_dir() { + let read = osascript_attachments(Err(anyhow::anyhow!("no scratch dir")), |script| { + assert!( + !script.contains("open for access"), + "raster half must be skipped" + ); + Ok(attachments_stdout("/tmp/a.txt\n", "NONE")) + }) + .unwrap(); + + assert_eq!(read.file_urls.as_deref(), Some("/tmp/a.txt")); + assert_eq!(read.image, None); + } + + #[test] + fn no_scratch_dir_and_no_file_url_is_an_error_not_an_empty_board() { + let err = osascript_attachments(Err(anyhow::anyhow!("no scratch dir")), |_| { + Ok(attachments_stdout("none", "NONE")) + }) + .unwrap_err(); + + assert!(err.to_string().contains("no scratch dir"), "{err}"); + } +}