fix(sed): fall back to direct write when hidden temp file is refused

crates/bashkit/src/builtins/atomic_write.rs

@@ -8,6 +8,16 @@
 //! finding E): the previous `sed -i` wrote straight through
 //! `FileSystem::write_file`, which recreates the entry with mode 0644 and loses
 //! the original content if the write fails part-way.
+//!
+//! Decision: when the hidden sibling temporary cannot be created or written
+//! (a filesystem or embedder policy that refuses dot paths, a file-count
+//! limit with no room for one extra entry), fall back to a direct
+//! `write_file` of the target and restore its mode. Every in-tree backend
+//! makes `write_file` all-or-nothing (TM-FS-014 for `RealFs`, in-memory
+//! replacement elsewhere), so the fallback still never leaves a half-written
+//! target; it only gives up the rename step. Chmod and rename failures do not
+//! fall back: by then the backend has accepted the temporary, so the failure
+//! is real and is reported with the source untouched.
 
 use std::path::Path;
 
@@ -44,19 +54,20 @@ pub(crate) async fn atomic_replace(
             .map(|byte| format!("{byte:02x}"))
             .collect::<String>();
         let candidate = vfs_join(parent, format!(".bashkit-{tool}-{suffix}.tmp"));
-        if !fs
-            .exists(&candidate)
-            .await
-            .map_err(|error| error.to_string())?
-        {
-            temp = Some(candidate);
-            break;
+        match fs.exists(&candidate).await {
+            Ok(false) => {
+                temp = Some(candidate);
+                break;
+            }
+            Ok(true) => {}
+            // The backend refuses to even look at the hidden path.
+            Err(_) => return direct_replace(fs, target, content).await,
         }
     }
     let temp = temp.ok_or_else(|| "cannot allocate temporary file".to_string())?;
-    if let Err(error) = fs.write_file(&temp, content).await {
+    if fs.write_file(&temp, content).await.is_err() {
         let _ = fs.remove(&temp, false).await;
-        return Err(format!("cannot write temporary file: {error}"));
+        return direct_replace(fs, target, content).await;
     }
     #[cfg(feature = "failpoints")]
     if injected_failure(failpoints.chmod, "error") {
@@ -86,6 +97,27 @@ pub(crate) async fn atomic_replace(
     Ok(())
 }
 
+/// Fallback when the hidden temporary is refused: replace the target with one
+/// `write_file` (all-or-nothing on every in-tree backend) and restore its mode.
+async fn direct_replace(
+    fs: &dyn crate::fs::FileSystem,
+    target: &Path,
+    content: &[u8],
+) -> std::result::Result<(), String> {
+    let mode = fs.stat(target).await.ok().map(|metadata| metadata.mode);
+    fs.write_file(target, content)
+        .await
+        .map_err(|error| format!("cannot write '{}': {error}", target.display()))?;
+    if let Some(mode) = mode
+        && fs.stat(target).await.map(|m| m.mode).ok() != Some(mode)
+    {
+        fs.chmod(target, mode)
+            .await
+            .map_err(|error| format!("cannot preserve file mode: {error}"))?;
+    }
+    Ok(())
+}
+
 #[cfg(feature = "failpoints")]
 fn injected_failure(name: &str, expected: &str) -> bool {
     fail::eval(name, |action| action.as_deref() == Some(expected)).unwrap_or(false)

crates/bashkit/tests/integration/main.rs

@@ -129,6 +129,7 @@ pub mod script_analysis;
 pub mod script_execution_tests;
 pub mod security_audit_pocs;
 mod sed_fuzz_scaffold_tests;
+mod sed_inplace_policy_tests;
 mod sed_resource_tests;
 pub mod set_e_and_or_tests;
 pub mod shlvl_tests;

crates/bashkit/tests/integration/sed_inplace_policy_tests.rs

@@ -0,0 +1,170 @@
+//! `sed -i` / `yq -i` under a filesystem that refuses dot-path writes.
+//!
+//! Embedders (e.g. a read/write workspace policy) may deny any path segment
+//! starting with `.`. The atomic-replace helper stages a hidden
+//! `.bashkit-<tool>-<hex>.tmp` sibling; when that is refused it must fall back
+//! to a direct write of the target instead of failing with the file unchanged.
+
+use bashkit::{
+    Bash, DirEntry, FileSystem, FileSystemExt, InMemoryFs, Metadata, Result, async_trait,
+};
+use std::io::{Error as IoError, ErrorKind};
+use std::path::{Path, PathBuf};
+use std::sync::Arc;
+use std::time::SystemTime;
+
+/// Refuses every operation whose final path component starts with `.`.
+struct NoDotFs {
+    inner: Arc<InMemoryFs>,
+}
+
+fn guard(path: &Path) -> Result<()> {
+    let hidden = path
+        .file_name()
+        .and_then(|n| n.to_str())
+        .is_some_and(|n| n.starts_with('.'));
+    if hidden {
+        return Err(IoError::new(ErrorKind::PermissionDenied, "dot paths are not allowed").into());
+    }
+    Ok(())
+}
+
+#[async_trait]
+impl FileSystemExt for NoDotFs {}
+
+#[async_trait]
+impl FileSystem for NoDotFs {
+    async fn read_file(&self, path: &Path) -> Result<Vec<u8>> {
+        guard(path)?;
+        self.inner.read_file(path).await
+    }
+    async fn write_file(&self, path: &Path, content: &[u8]) -> Result<()> {
+        guard(path)?;
+        self.inner.write_file(path, content).await
+    }
+    async fn append_file(&self, path: &Path, content: &[u8]) -> Result<()> {
+        guard(path)?;
+        self.inner.append_file(path, content).await
+    }
+    async fn mkdir(&self, path: &Path, recursive: bool) -> Result<()> {
+        guard(path)?;
+        self.inner.mkdir(path, recursive).await
+    }
+    async fn remove(&self, path: &Path, recursive: bool) -> Result<()> {
+        guard(path)?;
+        self.inner.remove(path, recursive).await
+    }
+    async fn stat(&self, path: &Path) -> Result<Metadata> {
+        guard(path)?;
+        self.inner.stat(path).await
+    }
+    async fn read_dir(&self, path: &Path) -> Result<Vec<DirEntry>> {
+        self.inner.read_dir(path).await
+    }
+    async fn exists(&self, path: &Path) -> Result<bool> {
+        self.inner.exists(path).await
+    }
+    async fn rename(&self, from: &Path, to: &Path) -> Result<()> {
+        guard(from)?;
+        guard(to)?;
+        self.inner.rename(from, to).await
+    }
+    async fn copy(&self, from: &Path, to: &Path) -> Result<()> {
+        guard(from)?;
+        guard(to)?;
+        self.inner.copy(from, to).await
+    }
+    async fn symlink(&self, target: &Path, link: &Path) -> Result<()> {
+        guard(link)?;
+        self.inner.symlink(target, link).await
+    }
+    async fn read_link(&self, path: &Path) -> Result<PathBuf> {
+        guard(path)?;
+        self.inner.read_link(path).await
+    }
+    async fn chmod(&self, path: &Path, mode: u32) -> Result<()> {
+        guard(path)?;
+        self.inner.chmod(path, mode).await
+    }
+    async fn set_modified_time(&self, path: &Path, time: SystemTime) -> Result<()> {
+        guard(path)?;
+        self.inner.set_modified_time(path, time).await
+    }
+}
+
+async fn setup(path: &str, content: &[u8], mode: u32) -> (Arc<InMemoryFs>, Bash) {
+    let inner = Arc::new(InMemoryFs::new());
+    inner.mkdir(Path::new("/workspace"), true).await.unwrap();
+    inner.write_file(Path::new(path), content).await.unwrap();
+    inner.chmod(Path::new(path), mode).await.unwrap();
+    let fs = Arc::new(NoDotFs {
+        inner: inner.clone(),
+    });
+    let bash = Bash::builder()
+        .fs(fs as Arc<dyn FileSystem>)
+        .cwd("/workspace")
+        .build();
+    (inner, bash)
+}
+
+async fn no_temp_left(inner: &InMemoryFs) -> bool {
+    inner
+        .read_dir(Path::new("/workspace"))
+        .await
+        .unwrap()
+        .iter()
+        .all(|e| !e.name.starts_with(".bashkit-"))
+}
+
+#[tokio::test]
+async fn sed_in_place_falls_back_when_dot_paths_are_refused() {
+    let (inner, mut bash) = setup("/workspace/notes.txt", b"version = 1\n", 0o640).await;
+    let r = bash
+        .exec("sed -i 's/1/2/' notes.txt && cat notes.txt")
+        .await
+        .unwrap();
+    assert_eq!(r.exit_code, 0, "stderr: {}", r.stderr);
+    assert_eq!(r.stdout, "version = 2\n");
+    let path = Path::new("/workspace/notes.txt");
+    assert_eq!(inner.stat(path).await.unwrap().mode, 0o640);
+    assert!(no_temp_left(&inner).await);
+}
+
+#[tokio::test]
+async fn sed_in_place_with_backup_falls_back_when_dot_paths_are_refused() {
+    let (inner, mut bash) = setup("/workspace/a.txt", b"x\n", 0o644).await;
+    let r = bash.exec("sed -i.bak 's/x/y/' a.txt").await.unwrap();
+    assert_eq!(r.exit_code, 0, "stderr: {}", r.stderr);
+    let read = |p: &'static str| {
+        let inner = inner.clone();
+        async move { inner.read_file(Path::new(p)).await.unwrap() }
+    };
+    assert_eq!(read("/workspace/a.txt").await, b"y\n");
+    assert_eq!(read("/workspace/a.txt.bak").await, b"x\n");
+}
+
+#[tokio::test]
+async fn sed_in_place_on_refused_target_still_fails_cleanly() {
+    // The fallback must not bypass the policy for the target itself.
+    let (inner, mut bash) = setup("/workspace/.env", b"K=1\n", 0o600).await;
+    let r = bash.exec("sed -i 's/1/2/' .env").await.unwrap();
+    assert_ne!(r.exit_code, 0);
+    assert_eq!(
+        inner.read_file(Path::new("/workspace/.env")).await.unwrap(),
+        b"K=1\n"
+    );
+}
+
+#[cfg(feature = "jq")]
+#[tokio::test]
+async fn yq_in_place_falls_back_when_dot_paths_are_refused() {
+    let (inner, mut bash) = setup("/workspace/c.yml", b"name: old\n", 0o644).await;
+    let r = bash.exec("yq -i '.name = \"new\"' c.yml").await.unwrap();
+    assert_eq!(r.exit_code, 0, "stderr: {}", r.stderr);
+    let out = inner
+        .read_file(Path::new("/workspace/c.yml"))
+        .await
+        .unwrap();
+    assert!(String::from_utf8_lossy(&out).contains("new"));
+    assert!(no_temp_left(&inner).await);
+}

knowledge/security/threat-model.md

@@ -346,7 +346,7 @@ panicked. Resolved with `wrapping_*` ops, masked shift amounts, clamped exponent
 | TM-FS-013 | Permissive RealFs mount default | `mount_real_readonly_at("/", …)` exposes whole host without `allowed_mount_paths` | Allowlist-first: `/`, `/etc`, `/root`, `/Users`, `/home`, `/dev`, `/proc`, `/sys`, `/run`, `/var/run`, `/boot`, `/private`, Windows drive/share-qualified `Users` trees, and any path component matching `.ssh`, `.aws`, `.kube`, `.docker`, `.gnupg`, `.gcloud` are refused unless explicitly allowlisted. Secret components match case-insensitively on Windows. The C ABI enforces the same denylist (`bashkit::is_sensitive_mount_path`) for config-time and runtime mounts, where a sensitive root additionally requires an exact allowlist entry — a broad parent entry (e.g. the home directory) is not consent to expose `.ssh` etc. | **MITIGATED** |
 | TM-FS-014 | Partial filesystem mutation | Failed write/copy or copy-delete move corrupts/replaces a destination, duplicates a source, or consumes retained quota | `FileSystem` failure-atomicity contract; locked in-memory rename; MountableFs restores cross-mount destinations while NamespaceFs rejects cross-mount rename; RealFs stages and flushes sibling files before rename; failpoint and conformance regressions | **MITIGATED** |
 | TM-FS-015 | Partial archive extraction | A late traversal, malformed header, or size failure leaves earlier attacker-controlled files behind | Tar validates the complete archive and per-file limits before its first VFS mutation; conformance regression uses a valid entry followed by traversal | **MITIGATED** |
-| TM-FS-016 | yq in-place partial or destructive update | Parse, evaluation, serialization, or write failure truncates the source; predictable temporary names permit collisions | Complete evaluation and bounded serialization first; write a random sibling temporary file, preserve mode, and rename only after success; failpoint regressions cover allocation, all backend-write classes, chmod, rename, original-byte retention, and temporary cleanup | **MITIGATED** |
+| TM-FS-016 | yq in-place partial or destructive update | Parse, evaluation, serialization, or write failure truncates the source; predictable temporary names permit collisions | Complete evaluation and bounded serialization first; write a random sibling temporary file, preserve mode, and rename only after success; if the backend refuses the hidden temporary (dot-path policies, file-count limits), fall back to one all-or-nothing `write_file` of the target plus mode restore (`sed_inplace_policy_tests`); failpoint regressions cover allocation, all backend-write classes, chmod, rename, original-byte retention, and temporary cleanup | **MITIGATED** |
 | TM-FS-017 | JS host filesystem widens the sandbox to embedder storage | `new Bash({ fs })` in the wasm bindings routes every VFS operation into embedder-supplied JS, so a script reaches whatever that object exposes | Paths are normalized by `PosixFs` before any host call, and `read_file` rejects host-reported symlinks before delegating to a potentially symlink-following host read. The host object remains the security boundary and is the embedder's to scope (mount root, allowlist, read-only). Reads and writes bypass the in-memory quotas, see the FS-quota scope note in §1 | **ACCEPTED** (embedder-scoped, opt-in) |
 
 **Current Risk**: MEDIUM - Two open escape vectors (TM-ESC-012, TM-ESC-013) need remediation