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) <noreply@anthropic.com>
This commit is contained in:
@@ -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<String>,
|
||||||
|
pub size: Option<u64>,
|
||||||
|
}
|
||||||
|
|
||||||
|
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::<u64>().ok());
|
||||||
|
ViewerPoll { exists: true, hash, size }
|
||||||
|
}
|
||||||
|
|
||||||
|
pub async fn poll_file(container_id: &str, container_path: &str) -> Result<ViewerPoll, String> {
|
||||||
|
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));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -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-<uuid>`, 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<String> {
|
||||||
|
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<String, String> {
|
||||||
|
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());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user