fix: write the trace html export through the user-output primitive (#113)

The export hand-rolled temp+rename with a predictable {pid}.{counter}.tmp
name and, on non-unix, a bare fs::write that follows reparse points. The unix
arm opened the temp with create+truncate rather than create_new, so a
pre-created temp at the guessable name kept its own mode while receiving the
trace bytes. Routing through write_user_file adopts the hashed-nonce
create_new temp, destination validation, and cleanup that screenshot and
clipboard output already use.

Normalizing an empty parent in write_atomic_with is required for the swap:
Path::parent yields Some("") for a bare relative filename and metadata("")
fails NotFound, so --out report.html would otherwise be rejected. That bug was
already latent for screenshot --out and clipboard get --out.

Unifies the trace-export test modules on one lock so the process-global
JSON-size clamp cannot race across them.

crates/core/src/private_file.rs

@@ -98,6 +98,11 @@ pub(crate) fn write_user_atomic(path: &Path, bytes: &[u8]) -> std::io::Result<()
     )
 }
 
+/// `Path::parent` yields `Some("")` for a bare relative filename, and both
+/// `std::fs::metadata("")` and opening `""` fail with `NotFound`, so the empty
+/// parent is normalized to the current directory before `ensure_parent` and
+/// `sync_parent` see it; without that, a legitimate relative output path such
+/// as `--out report.html` is rejected.
 fn write_atomic_with(
     path: &Path,
     bytes: &[u8],
@@ -108,6 +113,11 @@ fn write_atomic_with(
     let parent = path
         .parent()
         .ok_or_else(|| invalid_input("private file path has no parent"))?;
+    let parent = if parent.as_os_str().is_empty() {
+        Path::new(".")
+    } else {
+        parent
+    };
     ensure_parent(parent)?;
     validate_destination(path)?;
     let (temporary, mut file) = create_temporary(path)?;

crates/core/src/private_file_tests.rs

@@ -7,6 +7,34 @@ use std::os::fd::AsRawFd;
 use std::os::unix::fs::OpenOptionsExt;
 #[cfg(unix)]
 use std::os::unix::fs::PermissionsExt;
+use std::sync::{Mutex, MutexGuard};
+
+static CURRENT_DIRECTORY_LOCK: Mutex<()> = Mutex::new(());
+
+struct CurrentDirectoryGuard {
+    previous: PathBuf,
+    _lock: MutexGuard<'static, ()>,
+}
+
+impl CurrentDirectoryGuard {
+    fn enter(target: &Path) -> Self {
+        let lock = CURRENT_DIRECTORY_LOCK
+            .lock()
+            .unwrap_or_else(|poisoned| poisoned.into_inner());
+        let previous = std::env::current_dir().unwrap();
+        std::env::set_current_dir(target).unwrap();
+        Self {
+            previous,
+            _lock: lock,
+        }
+    }
+}
+
+impl Drop for CurrentDirectoryGuard {
+    fn drop(&mut self) {
+        let _ = std::env::set_current_dir(&self.previous);
+    }
+}
 
 fn directory(label: &str) -> PathBuf {
     let path = std::env::temp_dir().join(format!(
@@ -241,6 +269,19 @@ fn private_lock_creates_and_reopens_the_lock_file() {
     std::fs::remove_dir_all(directory).unwrap();
 }
 
+#[test]
+fn user_write_accepts_a_bare_relative_filename() {
+    let directory = directory("bare-relative");
+    let guard = CurrentDirectoryGuard::enter(&directory);
+
+    write_user_atomic(Path::new("bare-name.txt"), b"relative").unwrap();
+
+    assert_eq!(std::fs::read("bare-name.txt").unwrap(), b"relative");
+    assert!(directory.join("bare-name.txt").is_file());
+    drop(guard);
+    std::fs::remove_dir_all(directory).unwrap();
+}
+
 #[test]
 fn ensure_private_creates_the_nested_parent_chain() {
     let directory = directory("nested-parents");

crates/core/src/trace_read/html.rs

@@ -5,6 +5,7 @@ use base64::Engine;
 use serde_json::{Value, json};
 use std::collections::HashSet;
 use std::path::{Path, PathBuf};
+#[cfg(test)]
 use std::sync::atomic::{AtomicU64, Ordering};
 
 const VIEWER_HTML: &str = include_str!("viewer.html");
@@ -15,8 +16,6 @@ pub const TRACE_EXPORT_DEFAULT_LIMIT: usize = 5000;
 const MAX_EMBED_SCREENSHOT_BYTES: u64 = 100 * 1024 * 1024;
 const MAX_JSON_BYTES: u64 = 200 * 1024 * 1024;
 
-static EXPORT_TMP_COUNTER: AtomicU64 = AtomicU64::new(0);
-
 #[cfg(test)]
 static TEST_MAX_JSON_BYTES: AtomicU64 = AtomicU64::new(0);
 
@@ -118,7 +117,7 @@ pub fn export_html(
             .join(format!("trace-{session_id}.html"))
     });
 
-    write_export_file(&path, html.as_bytes())?;
+    crate::refs::write_user_file(&path, html.as_bytes())?;
     let bytes = html.len();
 
     let warnings: Vec<Value> = merged
@@ -211,49 +210,6 @@ fn escape_for_json_island(json: &str) -> String {
     out
 }
 
-fn write_export_file(path: &Path, bytes: &[u8]) -> Result<(), AppError> {
-    if path.is_symlink() {
-        return Err(AppError::invalid_input_with_suggestion(
-            "Refusing to write trace export through a symlink",
-            "Choose a different --out path.",
-        ));
-    }
-    if let Some(parent) = path.parent() {
-        if !parent.as_os_str().is_empty() {
-            std::fs::create_dir_all(parent)?;
-        }
-    }
-    let unique = EXPORT_TMP_COUNTER.fetch_add(1, Ordering::Relaxed);
-    let tmp = path.with_extension(format!("{}.{unique}.tmp", std::process::id()));
-    let result = write_export_tmp_then_rename(&tmp, path, bytes);
-    if result.is_err() {
-        let _ = std::fs::remove_file(&tmp);
-    }
-    result
-}
-
-fn write_export_tmp_then_rename(tmp: &Path, path: &Path, bytes: &[u8]) -> Result<(), AppError> {
-    #[cfg(unix)]
-    {
-        use std::io::Write;
-        use std::os::unix::fs::OpenOptionsExt;
-        let mut file = std::fs::OpenOptions::new()
-            .write(true)
-            .create(true)
-            .truncate(true)
-            .mode(0o600)
-            .custom_flags(libc::O_NOFOLLOW)
-            .open(tmp)?;
-        file.write_all(bytes)?;
-        file.flush()?;
-    }
-    #[cfg(not(unix))]
-    std::fs::write(tmp, bytes)?;
-
-    std::fs::rename(tmp, path)?;
-    Ok(())
-}
-
 #[cfg(test)]
 #[path = "html_tests.rs"]
 mod tests;
@@ -265,3 +221,7 @@ mod screenshot_tests;
 #[cfg(test)]
 #[path = "html_export_stats_tests.rs"]
 mod export_stats_tests;
+
+#[cfg(test)]
+#[path = "html_write_tests.rs"]
+mod write_tests;

crates/core/src/trace_read/html_export_stats_tests.rs

@@ -4,9 +4,6 @@ use crate::trace_read::{ExportOptions, export_html};
 use std::fs;
 use std::io::Write;
 use std::path::{Path, PathBuf};
-use std::sync::Mutex;
-
-static HTML_EXPORT_STATS_TEST_LOCK: Mutex<()> = Mutex::new(());
 
 fn write_segment(trace_dir: &Path, name: &str, lines: &[&str]) {
     fs::create_dir_all(trace_dir).unwrap();
@@ -22,9 +19,7 @@ fn setup_trace_session() -> (
     String,
     PathBuf,
 ) {
-    let lock = HTML_EXPORT_STATS_TEST_LOCK
-        .lock()
-        .unwrap_or_else(|poisoned| poisoned.into_inner());
+    let lock = super::tests::html_test_guard();
     super::clear_test_max_json_bytes();
     let home = HomeGuard::new();
     let manifest = start_session(StartSessionOptions {

crates/core/src/trace_read/html_screenshot_tests.rs

@@ -5,9 +5,6 @@ use serde_json::Value;
 use std::fs;
 use std::io::Write;
 use std::path::{Path, PathBuf};
-use std::sync::Mutex;
-
-static HTML_SCREENSHOT_TEST_LOCK: Mutex<()> = Mutex::new(());
 
 fn write_segment(trace_dir: &Path, name: &str, lines: &[&str]) {
     fs::create_dir_all(trace_dir).unwrap();
@@ -23,9 +20,7 @@ fn setup_trace_session() -> (
     String,
     PathBuf,
 ) {
-    let lock = HTML_SCREENSHOT_TEST_LOCK
-        .lock()
-        .unwrap_or_else(|poisoned| poisoned.into_inner());
+    let lock = super::tests::html_test_guard();
     super::clear_test_max_json_bytes();
     let home = HomeGuard::new();
     let manifest = start_session(StartSessionOptions {

crates/core/src/trace_read/html_tests.rs

@@ -10,7 +10,7 @@ use std::sync::Mutex;
 
 static HTML_TEST_LOCK: Mutex<()> = Mutex::new(());
 
-fn html_test_guard() -> std::sync::MutexGuard<'static, ()> {
+pub(super) fn html_test_guard() -> std::sync::MutexGuard<'static, ()> {
     HTML_TEST_LOCK
         .lock()
         .unwrap_or_else(|poisoned| poisoned.into_inner())

crates/core/src/trace_read/html_write_tests.rs

@@ -0,0 +1,166 @@
+use crate::trace_read::{ExportOptions, export_html};
+use std::fs;
+use std::io::Write;
+use std::path::{Path, PathBuf};
+
+const SEGMENT_LINE: &str = r#"{"event":"command.start","command":"snapshot","ts_ms":1,"seq":1}"#;
+const SESSION_ID: &str = "s-export-write";
+const SENTINEL: &[u8] = b"planted-sentinel";
+const LEGACY_COUNTER_RANGE: u64 = 16;
+
+fn export_test_guard() -> std::sync::MutexGuard<'static, ()> {
+    let lock = super::tests::html_test_guard();
+    super::clear_test_max_json_bytes();
+    lock
+}
+
+fn export_scratch(label: &str) -> PathBuf {
+    let path = std::env::temp_dir().join(format!(
+        "agent-desktop-export-{label}-{}",
+        crate::refs::new_snapshot_id()
+    ));
+    fs::create_dir_all(&path).unwrap();
+    path
+}
+
+fn trace_dir_with_one_event(root: &Path) -> PathBuf {
+    let trace_dir = root.join("trace");
+    fs::create_dir_all(&trace_dir).unwrap();
+    let mut file = fs::File::create(trace_dir.join("100-1000.jsonl")).unwrap();
+    writeln!(file, "{SEGMENT_LINE}").unwrap();
+    trace_dir
+}
+
+fn export_to(trace_dir: &Path, out: &Path) -> Result<(), crate::AppError> {
+    export_html(
+        trace_dir,
+        SESSION_ID,
+        &ExportOptions {
+            limit: 0,
+            out: Some(out.to_path_buf()),
+        },
+    )
+    .map(|_| ())
+}
+
+fn legacy_temporary_path(out: &Path, counter: u64) -> PathBuf {
+    out.with_extension(format!("{}.{counter}.tmp", std::process::id()))
+}
+
+#[test]
+fn export_refuses_a_directory_destination() {
+    let _guard = export_test_guard();
+    let root = export_scratch("directory-destination");
+    let trace_dir = trace_dir_with_one_event(&root);
+    let out = root.join("report.html");
+    fs::create_dir(&out).unwrap();
+
+    let error = export_to(&trace_dir, &out).unwrap_err();
+
+    assert_eq!(error.code(), "INVALID_ARGS");
+    assert!(out.is_dir());
+    fs::remove_dir_all(root).unwrap();
+}
+
+#[cfg(unix)]
+#[test]
+fn export_refuses_a_symlink_destination() {
+    let _guard = export_test_guard();
+    let root = export_scratch("symlink-destination");
+    let trace_dir = trace_dir_with_one_event(&root);
+    let target = root.join("target.html");
+    fs::write(&target, b"kept").unwrap();
+    let out = root.join("report.html");
+    std::os::unix::fs::symlink(&target, &out).unwrap();
+
+    let error = export_to(&trace_dir, &out).unwrap_err();
+
+    assert_eq!(error.code(), "INVALID_ARGS");
+    assert_eq!(fs::read(&target).unwrap(), b"kept");
+    assert!(out.is_symlink());
+    fs::remove_dir_all(root).unwrap();
+}
+
+#[test]
+fn export_does_not_write_through_the_legacy_predictable_temporary_name() {
+    let _guard = export_test_guard();
+    let root = export_scratch("legacy-temporary");
+    let trace_dir = trace_dir_with_one_event(&root);
+    let out = root.join("report.html");
+    let planted: Vec<PathBuf> = (0..LEGACY_COUNTER_RANGE)
+        .map(|counter| legacy_temporary_path(&out, counter))
+        .collect();
+    for path in &planted {
+        fs::write(path, SENTINEL).unwrap();
+        #[cfg(unix)]
+        {
+            use std::os::unix::fs::PermissionsExt;
+            fs::set_permissions(path, fs::Permissions::from_mode(0o666)).unwrap();
+        }
+    }
+
+    export_to(&trace_dir, &out).unwrap();
+
+    assert!(out.is_file());
+    for path in &planted {
+        assert!(
+            path.is_file(),
+            "the export consumed the predictable temporary name {}",
+            path.display()
+        );
+        assert_eq!(
+            fs::read(path).unwrap(),
+            SENTINEL,
+            "the export overwrote the predictable temporary name {}",
+            path.display()
+        );
+        #[cfg(unix)]
+        {
+            use std::os::unix::fs::PermissionsExt;
+            assert_eq!(
+                fs::metadata(path).unwrap().permissions().mode() & 0o777,
+                0o666,
+                "the export re-permissioned the predictable temporary name {}",
+                path.display()
+            );
+        }
+    }
+    fs::remove_dir_all(root).unwrap();
+}
+
+#[test]
+fn a_successful_export_leaves_no_temporary_residue() {
+    let _guard = export_test_guard();
+    let root = export_scratch("no-residue");
+    let trace_dir = trace_dir_with_one_event(&root);
+    let out = root.join("report.html");
+
+    export_to(&trace_dir, &out).unwrap();
+
+    let mut names: Vec<String> = fs::read_dir(&root)
+        .unwrap()
+        .map(|entry| entry.unwrap().file_name().to_string_lossy().into_owned())
+        .collect();
+    names.sort();
+    assert_eq!(names, vec!["report.html".to_string(), "trace".to_string()]);
+    fs::remove_dir_all(root).unwrap();
+}
+
+#[cfg(unix)]
+#[test]
+fn an_exported_file_lands_owner_only_under_an_explicit_out_path() {
+    use std::os::unix::fs::PermissionsExt;
+    let _guard = export_test_guard();
+    let root = export_scratch("owner-only");
+    fs::set_permissions(&root, fs::Permissions::from_mode(0o777)).unwrap();
+    let trace_dir = trace_dir_with_one_event(&root);
+    let out = root.join("report.html");
+
+    export_to(&trace_dir, &out).unwrap();
+
+    assert_eq!(
+        fs::metadata(&out).unwrap().permissions().mode() & 0o777,
+        0o600
+    );
+    fs::remove_dir_all(root).unwrap();
+}