Fix/review hardening

README.md

@@ -31,6 +31,19 @@ codesmith                                # interactive TUI
 
 Modes — **Plan** (read-only) / **Agent** (default, gated) / **YOLO** (auto-approve): [docs/MODES.md](docs/MODES.md) · other providers: [docs/PROVIDERS.md](docs/PROVIDERS.md)
 
+## One Binary, Many Modes
+
+Don't write extensions to get the tool you want — turn dials. A *mode* is one TOML file bundling tool surface, thinking depth, memory persistence, approval posture, sub-agent cap, and model into a switchable preset:
+
+```bash
+codesmith --mode minimal   # core file+shell tools, thinking off, zero memory
+codesmith --mode maximal   # full surface, deepest thinking, 20 sub-agents
+/mode plan                 # switch mid-session, no restart
+/mode export my-setup      # snapshot your dials into a shareable file
+```
+
+Four modes ship built-in (`minimal` / `balanced` / `maximal` / `plan`); yours live in `~/.codesmith/modes/` and per-project `.codesmith/modes/` — commit the project ones, that's the sharing story. Memory is a dial like everything else: `goldfish` (no cross-session memory) / `notebook` (only what you explicitly save) / `elephant` (auto memory with budget + decay). Full schema and semantics: [docs/MODES.md](docs/MODES.md).
+
 ## Documentation
 
 Get started: [user guide](docs/GUIDE.md) · [modes & approvals](docs/MODES.md) · [keybindings](docs/KEYBINDINGS.md) · [skills](docs/SKILLS.md) · [memory](docs/MEMORY.md) · [localization](docs/LOCALIZATION.md) · [full command catalog](docs/CLI.md)

config.example.toml

@@ -173,6 +173,18 @@ allow_shell = true
 approval_policy = "on-request" # on-request | untrusted | never
 sandbox_mode = "workspace-write" # read-only | workspace-write | danger-full-access | external-sandbox
 
+# ─────────────────────────────────────────────────────────────────────────────────
+# Named Mode
+# ─────────────────────────────────────────────────────────────────────────────────
+# Active named mode: a delta bundle of dials in a single TOML file. Built-ins:
+# minimal | balanced | maximal | plan. Your own modes live in
+# ~/.codesmith/modes/*.toml and <workspace>/.codesmith/modes/*.toml (project
+# files override by name). Switch live with `/mode <name>`, list with
+# `/mode list`, snapshot your current dials with `/mode export <name>`.
+# Overridden by the `--mode` CLI flag; see docs/MODES.md.
+#
+# mode = "minimal"
+
 # ─────────────────────────────────────────────────────────────────────────────────
 # External Sandbox Backend (pluggable remote execution)
 # ─────────────────────────────────────────────────────────────────────────────────

crates/agent-runtime/src/agent_memory/paths.rs

@@ -113,9 +113,9 @@ pub fn ensure_agent_memory_dir(memory_dir: &Path) -> std::io::Result<()> {
     fs::create_dir_all(memory_dir)?;
     let entrypoint = resolve_agent_memory_entrypoint(memory_dir);
     if !entrypoint.exists() {
-        fs::write(
+        crate::utils::write_atomic(
             &entrypoint,
-            "# Agent Memory\n\nAdd links to durable memory topic files here.\n",
+            b"# Agent Memory\n\nAdd links to durable memory topic files here.\n",
         )?;
     }
     Ok(())
@@ -151,6 +151,62 @@ pub fn scoped_path_within_memory(memory_dir: &Path, raw: &str) -> Result<PathBuf
             base.display()
         ));
     }
+    // Symlink-escape hardening: the lexical prefix check above is not enough
+    // when a component under the memory root is itself a symlink pointing
+    // outside it (e.g. a planted `topics -> /etc`). Verify against the real
+    // filesystem using the canonical base: an existing candidate (read or
+    // rewrite target) canonicalizes directly — a symlinked leaf resolves to
+    // its target and then fails containment — while a write target that does
+    // not exist yet is covered by canonicalizing its deepest existing
+    // ancestor, with the not-yet-existing final component required to be a
+    // single segment (no separators). Legitimate paths are unaffected: the
+    // candidate is returned exactly as before; only the containment decision
+    // gains the canonical re-check.
+    if let Ok(canonical_base) = memory_dir.canonicalize() {
+        if let Ok(canonical) = candidate.canonicalize() {
+            if !canonical.starts_with(&canonical_base) {
+                return Err(format!(
+                    "path {} escapes agent memory directory {}",
+                    canonical.display(),
+                    canonical_base.display()
+                ));
+            }
+        } else {
+            // Write target not on disk yet: the final component must be a
+            // single safe segment — a separator inside it would mean the
+            // lexical component scan above was bypassed (e.g. a backslash
+            // smuggled on a Unix host targeting a Windows-style path).
+            let leaf_is_single_segment = candidate
+                .file_name()
+                .and_then(|name| name.to_str())
+                .is_some_and(|name| !name.contains('/') && !name.contains('\\'));
+            // Walk to the deepest existing ancestor — the parent for a
+            // simple `dir/file.md` write — and canonicalize that.
+            let mut existing = candidate.as_path();
+            while !existing.exists() && existing.file_name().is_some() {
+                match existing.parent() {
+                    Some(parent) => existing = parent,
+                    None => break,
+                }
+            }
+            if !leaf_is_single_segment {
+                return Err(format!(
+                    "path {} is not a single safe segment within agent memory directory {}",
+                    candidate.display(),
+                    canonical_base.display()
+                ));
+            }
+            if let Ok(canonical) = existing.canonicalize()
+                && !canonical.starts_with(&canonical_base)
+            {
+                return Err(format!(
+                    "path {} escapes agent memory directory {}",
+                    canonical.display(),
+                    canonical_base.display()
+                ));
+            }
+        }
+    }
     Ok(candidate)
 }
 
@@ -214,4 +270,58 @@ mod tests {
         let path = scoped_path_within_memory(tmp.path(), "topics/foo.md").unwrap();
         assert!(path.ends_with("topics/foo.md"));
     }
+
+    #[test]
+    #[cfg(unix)]
+    fn scoped_path_rejects_symlinked_directory_escape() {
+        // A symlink planted inside the memory dir pointing outside must be
+        // rejected even though the lexical component check passes — the
+        // canonicalized ancestor resolves outside the memory root.
+        let tmp = tempdir().unwrap();
+        let memory_dir = tmp.path().join("memory");
+        let outside = tmp.path().join("outside");
+        fs::create_dir_all(&memory_dir).unwrap();
+        fs::create_dir_all(&outside).unwrap();
+        std::os::unix::fs::symlink(&outside, memory_dir.join("topics")).unwrap();
+
+        let err = scoped_path_within_memory(&memory_dir, "topics/secret.md").unwrap_err();
+        assert!(
+            err.contains("escapes"),
+            "symlinked directory escape must be rejected; got: {err}"
+        );
+    }
+
+    #[test]
+    #[cfg(unix)]
+    fn scoped_path_rejects_symlinked_leaf_escape() {
+        // An existing symlinked leaf file resolves outside the memory root
+        // and must be rejected by the canonical containment re-check.
+        let tmp = tempdir().unwrap();
+        let memory_dir = tmp.path().join("memory");
+        fs::create_dir_all(&memory_dir).unwrap();
+        let outside_file = tmp.path().join("outside-secret.md");
+        fs::write(&outside_file, "secret").unwrap();
+        std::os::unix::fs::symlink(&outside_file, memory_dir.join("leaked.md")).unwrap();
+
+        let err = scoped_path_within_memory(&memory_dir, "leaked.md").unwrap_err();
+        assert!(
+            err.contains("escapes"),
+            "symlinked leaf escape must be rejected; got: {err}"
+        );
+    }
+
+    #[test]
+    #[cfg(unix)]
+    fn scoped_path_allows_real_files_after_canonical_check() {
+        // Legitimate existing nested paths keep working under the canonical
+        // containment re-check (both sides canonicalize consistently, so
+        // e.g. macOS `/var` -> `/private/var` does not cause false hits).
+        let tmp = tempdir().unwrap();
+        let memory_dir = tmp.path().join("memory");
+        fs::create_dir_all(memory_dir.join("topics")).unwrap();
+        fs::write(memory_dir.join("topics/notes.md"), "x").unwrap();
+
+        let path = scoped_path_within_memory(&memory_dir, "topics/notes.md").unwrap();
+        assert!(path.ends_with("topics/notes.md"));
+    }
 }

crates/agent-runtime/src/artifacts.rs

@@ -98,15 +98,37 @@ pub fn set_test_artifact_sessions_root(root: Option<PathBuf>) -> Option<PathBuf>
     std::mem::replace(&mut *guard, root)
 }
 
+/// Reject Windows-only absolute forms that std only parses into
+/// `Component::Prefix`/`Component::RootDir` when running *on* Windows:
+/// `C:evil.txt` (drive-relative prefix) and `\evil.txt` (root-relative).
+/// On other hosts these arrive as plain `Normal` components, so the text
+/// has to be scanned as well as the components.
+fn is_windows_absolute_form(relative_path: &Path) -> bool {
+    let Some(text) = relative_path.to_str() else {
+        // Artifact relative paths are always generated as UTF-8; anything
+        // else is not a form we produced and is rejected.
+        return true;
+    };
+    if text.starts_with('\\') || text.starts_with('/') {
+        return true;
+    }
+    let bytes = text.as_bytes();
+    bytes.len() >= 2 && bytes[0].is_ascii_alphabetic() && bytes[1] == b':'
+}
+
 #[must_use]
 pub fn session_artifact_absolute_path(session_id: &str, relative_path: &Path) -> Option<PathBuf> {
     if !is_valid_session_id(session_id) {
         return None;
     }
     if relative_path.is_absolute()
-        || relative_path
-            .components()
-            .any(|component| matches!(component, Component::ParentDir))
+        || is_windows_absolute_form(relative_path)
+        || relative_path.components().any(|component| {
+            matches!(
+                component,
+                Component::ParentDir | Component::Prefix(_) | Component::RootDir
+            )
+        })
     {
         return None;
     }
@@ -314,4 +336,44 @@ mod tests {
                 .join("art_call-big.txt")
         );
     }
+
+    #[test]
+    fn session_artifact_absolute_path_rejects_windows_prefix_forms() {
+        // `C:evil.txt` (drive-relative) and `\evil.txt` (root-relative) are
+        // absolute on Windows but parse as plain `Normal` components
+        // elsewhere — both must reject on every host.
+        let _guard = TEST_ARTIFACT_SESSIONS_GUARD
+            .lock()
+            .unwrap_or_else(|err| err.into_inner());
+        let tmp = tempfile::tempdir().unwrap();
+        let _root = set_test_sessions_root(tmp.path().join("sessions"));
+
+        assert!(
+            session_artifact_absolute_path("session-123", Path::new("C:evil.txt")).is_none(),
+            "drive-relative prefix must reject"
+        );
+        assert!(
+            session_artifact_absolute_path("session-123", Path::new("\\evil.txt")).is_none(),
+            "root-relative backslash form must reject"
+        );
+        assert!(
+            session_artifact_absolute_path("session-123", Path::new(r"C:\Windows\system32"))
+                .is_none(),
+            "full drive-absolute form must reject"
+        );
+    }
+
+    #[test]
+    fn session_artifact_absolute_path_rejects_parent_traversal() {
+        let _guard = TEST_ARTIFACT_SESSIONS_GUARD
+            .lock()
+            .unwrap_or_else(|err| err.into_inner());
+        let tmp = tempfile::tempdir().unwrap();
+        let _root = set_test_sessions_root(tmp.path().join("sessions"));
+
+        assert!(
+            session_artifact_absolute_path("session-123", Path::new("../../etc/passwd")).is_none(),
+            "`..` traversal must reject"
+        );
+    }
 }

crates/agent-runtime/src/compaction/compact.rs

@@ -904,9 +904,15 @@ pub async fn compact_messages_safe(
     // is available on every return path (local-prune early return,
     // session-memory early return, and the LLM summary). Hooks are
     // non-blocking: failures log a warning and contribute nothing (#485).
-    let preserve_context: Option<String> = enhancements
-        .and_then(|e| e.hooks.as_ref())
-        .and_then(|(executor, context)| executor.execute_pre_compact_hook(context));
+    let preserve_context: Option<String> = match enhancements.and_then(|e| e.hooks.clone()) {
+        Some((executor, context)) => tokio::task::spawn_blocking(move || {
+            executor.execute_pre_compact_hook(&context)
+        })
+        .await
+        .ok()
+        .flatten(),
+        None => None,
+    };
 
     let mut pruned_messages = messages.to_vec();
     let mut now_under_threshold = false;

crates/agent-runtime/src/compaction/mod.rs

@@ -99,7 +99,7 @@ pub const MINIMUM_AUTO_COMPACTION_TOKENS: usize = 0;
 //
 // Pure functions over API message types. Kept `pub` so the TUI's heavy
 // compaction engine can re-export and call them unqualified. All counting
-// routes through [`crate::tokenizer`] — the historical chars/3 heuristic by
+// routes through [`crate::tokenizer`] — the CJK-aware heuristic by
 // default, exact counts when a tokenizer.json was installed at startup.
 
 pub fn estimate_tokens_for_message(message: &Message, include_thinking: bool) -> usize {

crates/agent-runtime/src/compaction/partial_compact.rs

@@ -250,17 +250,26 @@ pub async fn partial_compact(
 }
 
 /// Conservative token estimate for a single message.
+///
+/// Routes through the shared [`crate::tokenizer`] counter, mirroring the
+/// estimation helpers in `compaction::mod` (the historical bytes/4
+/// arithmetic under-counted CJK text by ~3x).
 fn estimate_tokens_for_message(msg: &Message) -> usize {
+    let counter = crate::tokenizer::default_counter();
     msg.content
         .iter()
         .map(|block| match block {
-            ContentBlock::Text { text, .. } => text.len() / 4,
-            ContentBlock::Thinking { thinking } => thinking.len() / 4,
+            ContentBlock::Text { text, .. } => counter.count_text(text),
+            ContentBlock::Thinking { thinking } => counter.count_text(thinking),
             ContentBlock::ToolUse { input, .. } => serde_json::to_string(input)
-                .map(|s| s.len() / 4)
+                .map(|s| counter.count_text(&s))
                 .unwrap_or(100),
-            ContentBlock::ToolResult { content, .. } => content.len() / 4,
+            ContentBlock::ToolResult { content, .. } => counter.count_text(content),
             ContentBlock::Image { .. } => crate::models::IMAGE_BLOCK_ESTIMATED_TOKENS,
+            // These blocks are dropped at the wire layer
+            // (crates/providers/src/rig_adapter/convert.rs) before the
+            // request is serialized, so 0 is deliberate for request-size
+            // estimation.
             ContentBlock::ServerToolUse { .. }
             | ContentBlock::ToolSearchToolResult { .. }
             | ContentBlock::CodeExecutionToolResult { .. } => 0,

crates/agent-runtime/src/cycle_manager.rs

@@ -319,6 +319,13 @@ pub struct CycleArchiveHeader {
 
 /// Resolve the on-disk archive directory: `~/.codesmith/sessions/<id>/cycles`.
 fn archive_dir_for(session_id: &str) -> Result<PathBuf> {
+    // The session id reaches `Path::join`; require a single safe path
+    // segment so a malformed id cannot traverse outside the sessions root.
+    if !crate::utils::is_safe_path_component(session_id) {
+        return Err(anyhow::anyhow!(
+            "invalid session id '{session_id}': must be a single safe path segment"
+        ));
+    }
     let sessions = codesmith_config::resolve_state_dir("sessions").unwrap_or_else(|_| {
         dirs::home_dir()
             .unwrap_or_else(|| PathBuf::from("."))
@@ -371,6 +378,19 @@ fn write_archive_file(
 ) -> Result<()> {
     let tmp_path = path.with_extension("jsonl.tmp");
     {
+        // Archives contain full conversation transcripts — create the tmp
+        // file owner-only from the start instead of chmod-after-write.
+        #[cfg(unix)]
+        let file = {
+            use std::os::unix::fs::OpenOptionsExt;
+            OpenOptions::new()
+                .create(true)
+                .truncate(true)
+                .write(true)
+                .mode(0o600)
+                .open(&tmp_path)?
+        };
+        #[cfg(not(unix))]
         let file = OpenOptions::new()
             .create(true)
             .truncate(true)
@@ -388,7 +408,9 @@ fn write_archive_file(
         // BufWriter flushes on drop, but we want any error surfaced now —
         // not silently into the void.
         buf.flush()?;
-        // File handle drops with `buf`.
+        // Durability before the rename: without fsync, a crash after rename
+        // can leave a zero-length archive that reads as data loss.
+        buf.get_ref().sync_all()?;
     }
     std::fs::rename(&tmp_path, path)?;
     Ok(())
@@ -825,6 +847,25 @@ mod tests {
         assert_eq!(seeds[1].role, "assistant");
     }
 
+    #[test]
+    fn archive_dir_for_rejects_unsafe_session_ids() {
+        // The session id reaches `Path::join` under the sessions root —
+        // traversal / separators / dot tricks must be rejected outright.
+        for bad_id in ["../../etc", "a/b", "", ".", "..", ".hidden"] {
+            let err = archive_dir_for(bad_id).expect_err("unsafe session id must reject");
+            assert!(
+                format!("{err:#}").contains("invalid session id"),
+                "id {bad_id:?}: got {err:#}"
+            );
+        }
+    }
+
+    #[test]
+    fn archive_dir_for_accepts_safe_session_ids() {
+        let dir = archive_dir_for("session-123_abc").expect("safe session id");
+        assert!(dir.ends_with(std::path::Path::new("session-123_abc").join("cycles")));
+    }
+
     #[test]
     fn archive_cycle_writes_jsonl_with_header_and_messages() {
         let dir = tempdir().expect("tempdir");