diff --git a/app/src-tauri/src/commands/file_commands.rs b/app/src-tauri/src/commands/file_commands.rs index 6b8f476..a301e40 100644 --- a/app/src-tauri/src/commands/file_commands.rs +++ b/app/src-tauri/src/commands/file_commands.rs @@ -12,8 +12,8 @@ use tauri::State; use crate::docker::client::get_docker; use crate::docker::exec::{ - build_single_file_tar, container_user_ids, exec_oneshot_as, exec_oneshot_streams_as, - now_epoch_secs, OUTPUT_LIMIT_MARKER, + build_single_file_tar, container_user_ids, exec_oneshot_as, exec_oneshot_as_within, + exec_oneshot_streams_as, now_epoch_secs, OUTPUT_LIMIT_MARKER, }; use crate::AppState; @@ -399,7 +399,12 @@ fn validate_container_write_path(what: &str, path: &str) -> Result<(), String> { async fn resolve_container_dir(container_id: &str, what: &str, dir: &str) -> Result<(), String> { validate_container_write_path(what, dir)?; - let (output, code) = exec_oneshot_as( + // Split streams, not the combined buffer: `realpath`'s answer is a *path* + // and its diagnostics are not, so parsing the two together is the same + // hazard the listing above took apart for `find`. A warning on stderr — + // and there is one whenever a component is unreadable — used to be spliced + // into the string this then compared against the write roots. + let (stdout, diagnostics, code) = exec_oneshot_streams_as( container_id, "claude", vec![ @@ -412,12 +417,17 @@ async fn resolve_container_dir(container_id: &str, what: &str, dir: &str) -> Res ) .await?; - let resolved = output.trim(); + let resolved = stdout.trim(); if code != 0 || resolved.is_empty() { log::warn!( - "Could not resolve {} in the container (exit {}); using the literal path", + "Could not resolve {} in the container (exit {}{}); using the literal path", dir, - code + code, + if diagnostics.trim().is_empty() { + String::new() + } else { + format!(": {}", diagnostics.trim()) + } ); return Ok(()); } @@ -495,6 +505,37 @@ const HOST_AUTORUN_DIRS: &[&[&str]] = &[ &["start menu", "programs", "startup"], ]; +/// Directory *sequences* that hold credentials or authority, judged on the +/// path a symlink chain really leads to. +/// +/// This is the part of the hidden-component rule that has to survive +/// resolution. The rule itself cannot: see [`validate_resolved_host_path`] for +/// why "the real location passes through a dot directory" describes ordinary +/// software far more often than it describes an attack — `node_modules/.pnpm`, +/// `~/.local/share`, `~/.cache`, `~/.var/app`, `~/.nvm`, `~/.cargo`. What +/// *is* worth refusing after resolution is the small set of directories whose +/// contents are keys, tokens and startup entries, and those can be named. +/// +/// Matched as a contiguous run of components anywhere in the path, so +/// `~/.ssh/keys/id_rsa` is as refused as `~/.ssh/id_rsa`. Same defence-in-depth +/// footing as [`HOST_AUTORUN_DIRS`], and the same honest caveat: it is a list +/// of the places that are known, not of the ones that exist. The boundary is +/// the file dialog; this is what stops a *planted symlink* aiming an otherwise +/// ordinary-looking path at the one directory the attack wants. +const HOST_CREDENTIAL_DIRS: &[&[&str]] = &[ + &[".ssh"], + &[".gnupg"], + &[".aws"], + &[".azure"], + &[".kube"], + &[".docker"], + &[".claude"], + &[".config", "gcloud"], + &[".config", "autostart"], + &[".config", "systemd", "user"], + &[".local", "share", "keyrings"], +]; + /// Length of a `C:` drive prefix at the head of `path`, or 0. fn drive_prefix_len(path: &str) -> usize { let b = path.as_bytes(); @@ -614,6 +655,23 @@ fn is_autorun_dir(names: &[String]) -> bool { }) } +/// The [`HOST_CREDENTIAL_DIRS`] entry `names` passes through, spelled the way +/// the list spells it so the refusal can name it. +/// +/// Anywhere in the path rather than at the end: what matters is that the path +/// goes *through* `~/.ssh`, not how much further it goes. +fn credential_dir_in(names: &[String]) -> Option { + HOST_CREDENTIAL_DIRS.iter().find_map(|seq| { + (0..names.len().saturating_sub(seq.len() - 1)).find_map(|start| { + names[start..start + seq.len()] + .iter() + .zip(seq.iter()) + .all(|(have, want)| have.eq_ignore_ascii_case(want)) + .then(|| seq.join("/")) + }) + }) +} + /// Structural and policy checks on a host path *as written*, returning it as a /// [`PathBuf`]. /// @@ -635,12 +693,16 @@ fn is_autorun_dir(names: &[String]) -> bool { /// dragging a project's own `.env` into the container is an ordinary thing /// to do and its parent is not hidden. /// -/// **This is a lexical check on a string, and lexical is not enough on its own.** -/// A path whose components are all visible can still lead somewhere hidden, so -/// nothing calls this directly any more: [`resolve_host_path`] resolves the -/// symlinks first and then applies this to the answer. Keeping the two apart is -/// what lets the policy stay pure and testable while the thing it judges is the -/// path that will really be opened. +/// **This is a lexical check on the string the user chose, and it judges only +/// that.** A path whose components are all visible can still *lead* somewhere +/// that deserves a second opinion, which is why [`resolve_host_path`] follows +/// this with [`validate_resolved_host_path`] over the canonical form. The two +/// ask different questions and must not be confused: this one is "did the user +/// point at a hidden place", that one is "where do these bytes actually land". +/// Re-running *this* function over a canonical path is the H6 regression — it +/// refuses `node_modules/pkg` under pnpm, and every visible directory that +/// symlinks into `~/.local/share`, `~/.cache`, `~/.var/app`, `~/.nvm` or +/// `~/.cargo`, none of which is anybody's attack. /// /// **And the policy itself is a denylist, which is losing by construction.** /// `~/Library/LaunchAgents`, `%AppData%\…\Startup`, `~/bin` and `/opt` are only @@ -666,6 +728,21 @@ fn validate_host_path(path: &str, use_for: HostPathUse) -> Result Result