From bbcaee7797aad49dd65935bd605073e2a0c7a4f2 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Tue, 22 Sep 2026 21:09:33 -0700 Subject: [PATCH] feat(viewer): poll and save scripts run as the container user poll.rs: one exec per tick that tests existence then hashes+stats the file (sha256sum/stat), so the 2 s poll costs one exec instead of re-downloading up to 1 MiB of archive per window per tick. write.rs: sha256_hex/is_sha256_hex, MAX_WRITE_BYTES, and the save script. Saving stages the payload in /tmp via the existing write_file_to_container (owned by the container user, since the Docker archive API writes as root), then an exec as `claude` checks the base hash, swaps the file in with a same-directory rename when the directory is writable (falling back to an in-place `cat >` when it is not), and always cleans up the staged temp file via `trap`. classify_write maps exit 0/3/4 to Saved/Conflict/Gone. Applies three pre-flight rulings against the brief's literal text: - P8: pulled the write script's argv shape and the size/hash checks into pure `write_command`/`check_write_input` helpers with their own unit tests, since both lived only inside the untested async `write_file` otherwise. - P9: the brief's manual Docker smoke-test invocation (`sh w.sh save target tmp hash`) makes `$1` become "save" instead of the target, which is not what the script or the Rust caller expect. Verified in a throwaway container that invoking the file directly without the dummy "save" arg reproduces the Rust convention's `$1/$2/$3` correctly: exit 0 with the new hash and a removed payload on a clean save, exit 3 with the file untouched on a stale base hash, and exit 4 when the target is gone. - P15: GNU sha256sum prefixes its output line with `\` when the path contains a backslash or newline. Without a fix that turns into a permanent false conflict (write.rs) and a blinded poll (poll.rs, hash: null forever). Both parsers now strip a leading `\`, and the script itself strips it from $actual before comparing to $expect. Verified against real sha256sum output in a container with a backslash-named file: the save no longer false-conflicts and the reported hash matches. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/src-tauri/src/file_viewer/poll.rs | 106 ++++++++++++- app/src-tauri/src/file_viewer/write.rs | 209 ++++++++++++++++++++++++- 2 files changed, 313 insertions(+), 2 deletions(-) diff --git a/app/src-tauri/src/file_viewer/poll.rs b/app/src-tauri/src/file_viewer/poll.rs index 9482e55..6621d55 100644 --- a/app/src-tauri/src/file_viewer/poll.rs +++ b/app/src-tauri/src/file_viewer/poll.rs @@ -1 +1,105 @@ -//! Filled in by Task N. +//! One cheap exec per tick: the file's full hash and size, or "gone". +//! +//! This is what the 2 s poll asks, instead of re-downloading up to 1 MiB of archive per +//! window per tick. The hash is coreutils `sha256sum`, which equals `write::sha256_hex` +//! of the bytes whenever the read was not truncated — the only case in which the +//! editor uses a hash as its save base. + +use serde::Serialize; + +use crate::docker::exec::exec_oneshot_streams_as; + +#[derive(Clone, Debug, Serialize, PartialEq, Eq)] +pub struct ViewerPoll { + pub exists: bool, + pub hash: Option, + pub size: Option, +} + +pub const POLL_SCRIPT: &str = + r#"test -f "$1" || exit 4; sha256sum -- "$1" && stat -c %s -- "$1""#; + +pub fn parse_poll_output(code: i64, stdout: &str) -> ViewerPoll { + if code == 4 { + return ViewerPoll { exists: false, hash: None, size: None }; + } + let mut lines = stdout.lines(); + let hash = lines + .next() + .and_then(|l| l.split_whitespace().next()) + // GNU `sha256sum` prefixes the line with `\` when the name contains a + // backslash or a newline; strip it before validating the hex (P15). + .map(|h| h.trim_start_matches('\\')) + .filter(|h| super::write::is_sha256_hex(h)) + .map(str::to_string); + let size = lines.next().and_then(|l| l.trim().parse::().ok()); + ViewerPoll { exists: true, hash, size } +} + +pub async fn poll_file(container_id: &str, container_path: &str) -> Result { + let cmd = vec![ + "sh".to_string(), + "-c".to_string(), + POLL_SCRIPT.to_string(), + "poll".to_string(), + container_path.to_string(), + ]; + let (stdout, stderr, code) = + exec_oneshot_streams_as(container_id, "claude", cmd, Vec::new()).await?; + if code != 0 && code != 4 { + return Err(format!( + "Could not check the file: {}", + crate::commands::file_commands::clip_container_text(&stderr) + )); + } + Ok(parse_poll_output(code, &stdout)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_present_file_yields_hash_and_size() { + let out = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 /workspace/x\n42\n"; + assert_eq!( + parse_poll_output(0, out), + ViewerPoll { + exists: true, + hash: Some("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855".into()), + size: Some(42) + } + ); + } + + #[test] + fn exit_four_means_gone() { + assert_eq!(parse_poll_output(4, ""), ViewerPoll { exists: false, hash: None, size: None }); + } + + #[test] + fn garbage_is_not_a_hash() { + let p = parse_poll_output(0, "not a hash /x\nabc\n"); + assert_eq!(p, ViewerPoll { exists: true, hash: None, size: None }); + } + + #[test] + fn the_script_tests_existence_before_hashing() { + assert!(POLL_SCRIPT.contains("test -f \"$1\" || exit 4")); + assert!(POLL_SCRIPT.contains("sha256sum -- \"$1\"")); + assert!(POLL_SCRIPT.contains("stat -c %s -- \"$1\"")); + } + + /// P15: a path containing a backslash makes GNU `sha256sum` prefix the whole + /// line with `\`; that must not blind change detection by yielding `hash: None`. + #[test] + fn a_backslash_prefixed_hash_is_still_recognised() { + let out = "\\e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 /workspace/x\\y\n7\n"; + let p = parse_poll_output(0, out); + assert_eq!( + p.hash.as_deref(), + Some("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") + ); + assert_eq!(p.size, Some(7)); + } +} diff --git a/app/src-tauri/src/file_viewer/write.rs b/app/src-tauri/src/file_viewer/write.rs index 9482e55..8697911 100644 --- a/app/src-tauri/src/file_viewer/write.rs +++ b/app/src-tauri/src/file_viewer/write.rs @@ -1 +1,208 @@ -//! Filled in by Task N. +//! Saving: stage in `/tmp`, then swap in as the container user. +//! +//! The Docker archive API writes as root, so it is used for exactly one thing — landing +//! the payload at `/tmp/triple-c-viewer-`, owned by the container user (the +//! existing `write_file_to_container`). Everything that touches the *target directory* +//! runs in an exec as `claude`, so a save can do nothing the user's own shell could not. +//! A non-root process cannot `chown`, so the saved file is owned by the container user, +//! as it would be after Claude Code edited it; mode is kept with `chmod --reference`. + +use sha2::{Digest, Sha256}; + +use crate::commands::file_commands::clip_container_text; +use crate::docker::exec::{exec_oneshot_streams_as, ExecSessionManager}; + +/// Spec §4/§5: only untruncated (≤ 1 MiB) text is editable, so nothing larger is saved. +pub const MAX_WRITE_BYTES: usize = 1024 * 1024; + +pub fn sha256_hex(bytes: &[u8]) -> String { + let digest = Sha256::digest(bytes); + digest.iter().map(|b| format!("{:02x}", b)).collect() +} + +pub fn is_sha256_hex(s: &str) -> bool { + s.len() == 64 && s.bytes().all(|b| matches!(b, b'0'..=b'9' | b'a'..=b'f')) +} + +/// `$1` target, `$2` staged payload in /tmp, `$3` the hash the editor loaded from. +/// Exit 3 = changed on disk, 4 = gone; stdout on success is `sha256sum` of the target. +/// +/// P15: `sha256sum -- "$target"` prefixes its whole line with `\` when the path +/// contains a backslash or a newline, so `$actual` has that prefix stripped before +/// it is compared with `$expect` (which never carries one) — otherwise such a path +/// would conflict forever. +pub const WRITE_SCRIPT: &str = r#"target=$1; tmp=$2; expect=$3 +trap 'rm -f -- "$tmp"' EXIT +test -f "$target" || exit 4 +actual=$(sha256sum -- "$target" | cut -d' ' -f1) || exit 1 +actual=${actual#\\} +[ "$actual" = "$expect" ] || exit 3 +dir=$(dirname -- "$target"); name=$(basename -- "$target") +if [ -w "$dir" ]; then + staged="$dir/.$name.triple-c-$$" + cp -- "$tmp" "$staged" || exit 1 + chmod --reference="$target" "$staged" 2>/dev/null + mv -f -- "$staged" "$target" || { rm -f -- "$staged"; exit 1; } +else + cat -- "$tmp" > "$target" || exit 1 +fi +sha256sum -- "$target""#; + +pub enum WriteOutcome { + Saved(String), + Conflict, + Gone, + Failed(String), +} + +pub fn classify_write(code: i64, stdout: &str, stderr: &str) -> WriteOutcome { + match code { + 3 => WriteOutcome::Conflict, + 4 => WriteOutcome::Gone, + 0 => match stdout + .split_whitespace() + .next() + .map(|h| h.trim_start_matches('\\')) + .filter(|h| is_sha256_hex(h)) + { + Some(h) => WriteOutcome::Saved(h.to_string()), + None => WriteOutcome::Failed( + "The container did not report the saved file's hash.".into(), + ), + }, + _ => WriteOutcome::Failed(clip_container_text(stderr)), + } +} + +/// The write script's argv beyond `sh -c SCRIPT`: `$0=save`, `$1=target`, `$2=tmp`, +/// `$3=base_hash` — pulled out pure so the argument shape has a unit test (P8). +fn write_command(target: &str, tmp: &str, base_hash: &str) -> Vec { + vec![ + "sh".to_string(), + "-c".to_string(), + WRITE_SCRIPT.to_string(), + "save".to_string(), + target.to_string(), + tmp.to_string(), + base_hash.to_string(), + ] +} + +/// Refuses a payload too large to be editable, or a malformed base hash, before +/// anything is staged in the container (P8). +fn check_write_input(len: usize, base_hash: &str) -> Result<(), String> { + if len > MAX_WRITE_BYTES { + return Err("Files over 1 MiB are read-only in the viewer.".into()); + } + if !is_sha256_hex(base_hash) { + return Err("The editor's base hash is malformed; reload the file.".into()); + } + Ok(()) +} + +pub async fn write_file( + container_id: &str, + exec_manager: &ExecSessionManager, + target: &str, + bytes: &[u8], + base_hash: &str, +) -> Result { + check_write_input(bytes.len(), base_hash)?; + let tmp_name = format!("triple-c-viewer-{}", uuid::Uuid::new_v4().simple()); + let tmp_path = exec_manager + .write_file_to_container(container_id, &tmp_name, bytes) + .await?; + let cmd = write_command(target, &tmp_path, base_hash); + let (stdout, stderr, code) = + exec_oneshot_streams_as(container_id, "claude", cmd, Vec::new()).await?; + match classify_write(code, &stdout, &stderr) { + WriteOutcome::Saved(hash) => Ok(hash), + WriteOutcome::Conflict => { + Err("conflict: the file changed on disk since it was loaded.".into()) + } + WriteOutcome::Gone => Err("gone: the file no longer exists.".into()), + WriteOutcome::Failed(msg) => Err(format!("Could not save the file: {}", msg)), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn sha256_matches_coreutils() { + // `printf 'hello\n' | sha256sum` + assert_eq!( + sha256_hex(b"hello\n"), + "5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03" + ); + assert!(is_sha256_hex(&sha256_hex(b""))); + assert!(!is_sha256_hex("ABC")); + assert!(!is_sha256_hex(&"g".repeat(64))); + } + + #[test] + fn exit_codes_map_to_outcomes() { + let h = "5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03"; + assert!(matches!(classify_write(0, &format!("{} /x\n", h), ""), WriteOutcome::Saved(s) if s == h)); + assert!(matches!(classify_write(3, "", ""), WriteOutcome::Conflict)); + assert!(matches!(classify_write(4, "", ""), WriteOutcome::Gone)); + assert!(matches!(classify_write(1, "", "cp: Permission denied"), WriteOutcome::Failed(m) if m.contains("Permission denied"))); + // Success without a parseable hash is still a failure: the editor's base would be wrong. + assert!(matches!(classify_write(0, "junk", ""), WriteOutcome::Failed(_))); + } + + /// P15: a target path with a backslash makes `sha256sum` prefix the line; + /// the parsed hash must still be recognised as the saved hash. + #[test] + fn a_backslash_prefixed_saved_hash_is_still_recognised() { + let h = "5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03"; + assert!(matches!( + classify_write(0, &format!("\\{} /x\\y\n", h), ""), + WriteOutcome::Saved(s) if s == h + )); + } + + #[test] + fn the_write_script_checks_then_swaps_and_always_cleans_up() { + for needle in [ + "test -f \"$target\" || exit 4", + "exit 3", + "chmod --reference=\"$target\"", + "mv -f --", + "cat -- \"$tmp\" > \"$target\"", + "trap 'rm -f -- \"$tmp\"' EXIT", + ] { + assert!(WRITE_SCRIPT.contains(needle), "missing: {}", needle); + } + } + + /// P8: the write script's test list is binding, and the argument order is + /// exactly what a later edit could silently break. + #[test] + fn write_command_has_the_expected_argv_shape() { + let cmd = write_command("/w/t.txt", "/tmp/x", "abc123"); + assert_eq!( + cmd, + vec![ + "sh".to_string(), + "-c".to_string(), + WRITE_SCRIPT.to_string(), + "save".to_string(), + "/w/t.txt".to_string(), + "/tmp/x".to_string(), + "abc123".to_string(), + ] + ); + } + + /// P8: the size cap and base-hash checks are unit-testable in isolation from + /// the async `write_file`. + #[test] + fn check_write_input_refuses_oversized_payload_and_malformed_hash() { + let h = "5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03"; + assert!(check_write_input(MAX_WRITE_BYTES, h).is_ok()); + assert!(check_write_input(MAX_WRITE_BYTES + 1, h).is_err()); + assert!(check_write_input(0, "not-a-hash").is_err()); + } +}