diff --git a/CHANGELOG.md b/CHANGELOG.md index b256add..896e50f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ _primitives/_rust/target/release/kei-changelog \ ### Fixed - Placeholder: hook-bypass edge case follow-up to v0.15.1. +- **primitives/keisei (v0.19 audit hardening):** close 3 Security HIGH + 3 Critic HIGH + 2 Critic MEDIUM findings. Path-escape guard on `mcp_server` + `memory/artifacts/manifests` (absolute / `..` / canonical-mismatch → `PathEscape`); brain-name regex `^[a-z][a-z0-9_-]{0,63}$` (`InvalidName`); symlink-rooted brain inputs rejected (`BrainIsSymlink` — closes USB → `$HOME` pivot); MCP-entry collision check across all 4 adapters (`NameConflict` instead of silent clobber); dropped unused `rusqlite` dep (no C toolchain tail); `BrainPaths.{memory,artifacts,manifests}` relaxed to `Option`; `$KEISEI_HOME`/`$HOME` resolver deduped into `paths.rs` SSoT; `fsx::write_atomic` rewritten on `tempfile::NamedTempFile` for Windows + cross-fs correctness; 5 adversarial integration tests added (16 total pass). ## [0.15.0] — 2026-04-22 diff --git a/_primitives/MANIFEST.toml b/_primitives/MANIFEST.toml index a8ed621..a38c21b 100644 --- a/_primitives/MANIFEST.toml +++ b/_primitives/MANIFEST.toml @@ -259,5 +259,5 @@ desc = "Typed artifact handoff pipeline — schema-validated content pass-betwee [primitive.keisei] kind = "rust" crate = "keisei" -deps = ["rusqlite bundled (no system sqlite required)"] +deps = ["regex", "tempfile (runtime)"] desc = "Exobrain attach/status CLI — mounts a portable brain into an AI client (MVP: Claude Code)" diff --git a/_primitives/_rust/Cargo.lock b/_primitives/_rust/Cargo.lock index 8fb1b62..e7b40f8 100644 --- a/_primitives/_rust/Cargo.lock +++ b/_primitives/_rust/Cargo.lock @@ -1149,7 +1149,7 @@ name = "keisei" version = "0.1.0" dependencies = [ "clap", - "rusqlite", + "regex", "serde", "serde_json", "serde_yaml", diff --git a/_primitives/_rust/Cargo.toml b/_primitives/_rust/Cargo.toml index 4c7f8cd..ecba924 100644 --- a/_primitives/_rust/Cargo.toml +++ b/_primitives/_rust/Cargo.toml @@ -42,6 +42,7 @@ serde_json = "1" serde_yaml = "0.9" sha2 = "0.10" image = { version = "0.25", default-features = false, features = ["png"] } +regex = "1.10" [profile.release] opt-level = "z" diff --git a/_primitives/_rust/keisei/Cargo.toml b/_primitives/_rust/keisei/Cargo.toml index 165221b..00e9a1b 100644 --- a/_primitives/_rust/keisei/Cargo.toml +++ b/_primitives/_rust/keisei/Cargo.toml @@ -16,7 +16,8 @@ serde_json = "1" serde_yaml = "0.9" toml = "0.8" thiserror = "2" -rusqlite = { version = "0.31", features = ["bundled"] } +regex = { workspace = true } +tempfile = "3" [dev-dependencies] tempfile = "3" diff --git a/_primitives/_rust/keisei/src/adapters/claude_code.rs b/_primitives/_rust/keisei/src/adapters/claude_code.rs index bc08638..840ed37 100644 --- a/_primitives/_rust/keisei/src/adapters/claude_code.rs +++ b/_primitives/_rust/keisei/src/adapters/claude_code.rs @@ -5,15 +5,21 @@ //! Detection: `$CWD/.claude/settings.json` exists OR //! `$KEISEI_HOME/.claude` (or `$HOME/.claude`) is a directory. //! `$KEISEI_HOME` overrides `$HOME` for tests. +//! +//! Security (v0.19 audit): if an entry at `mcpServers["keisei"]` already +//! exists and doesn't match what keisei would write, attach fails with +//! `NameConflict` instead of silently clobbering the user's config. use crate::adapter::ClientAdapter; use crate::brain::Brain; -use crate::error::Result; -use crate::fsx::write_atomic; +use crate::error::{Error, Result}; +use crate::fsx::write_atomic_json; +use crate::paths; use serde_json::{json, Map, Value}; use std::path::PathBuf; pub const MCP_ENTRY_KEY: &str = "keisei"; +pub const CLIENT_NAME: &str = "claude-code"; pub struct ClaudeCodeAdapter; @@ -23,12 +29,7 @@ impl ClaudeCodeAdapter { } fn user_config_dir(&self) -> PathBuf { - let base = std::env::var("KEISEI_HOME") - .ok() - .map(PathBuf::from) - .or_else(|| std::env::var("HOME").ok().map(PathBuf::from)) - .unwrap_or_else(|| PathBuf::from(".")); - base.join(".claude") + paths::resolve_home().join(".claude") } } @@ -40,7 +41,7 @@ impl Default for ClaudeCodeAdapter { impl ClientAdapter for ClaudeCodeAdapter { fn name(&self) -> &str { - "claude-code" + CLIENT_NAME } fn detect(&self) -> bool { @@ -56,8 +57,8 @@ impl ClientAdapter for ClaudeCodeAdapter { std::fs::create_dir_all(parent)?; } let mut doc = load_json_or_empty(&cfg)?; - merge_mcp_entry(&mut doc, brain); - write_atomic(&cfg, &serde_json::to_string_pretty(&doc)?) + merge_mcp_entry(&mut doc, brain)?; + write_atomic_json(&cfg, &doc) } fn detach(&self) -> Result<()> { @@ -67,7 +68,7 @@ impl ClientAdapter for ClaudeCodeAdapter { } let mut doc = load_json_or_empty(&cfg)?; remove_mcp_entry(&mut doc); - write_atomic(&cfg, &serde_json::to_string_pretty(&doc)?) + write_atomic_json(&cfg, &doc) } fn config_path(&self) -> PathBuf { @@ -86,7 +87,18 @@ fn load_json_or_empty(cfg: &std::path::Path) -> Result { Ok(serde_json::from_str(&raw)?) } -fn merge_mcp_entry(doc: &mut Value, brain: &Brain) { +fn build_entry(brain: &Brain) -> Value { + json!({ + "command": brain.mcp_server_path().to_string_lossy(), + "args": [], + "env": { + "KEISEI_BRAIN_ROOT": brain.root.to_string_lossy(), + "KEISEI_BRAIN_NAME": brain.name(), + } + }) +} + +fn merge_mcp_entry(doc: &mut Value, brain: &Brain) -> Result<()> { if !doc.is_object() { *doc = json!({}); } @@ -97,18 +109,18 @@ fn merge_mcp_entry(doc: &mut Value, brain: &Brain) { if !servers.is_object() { *servers = Value::Object(Map::new()); } - let entry = json!({ - "command": brain.mcp_server_path().to_string_lossy(), - "args": [], - "env": { - "KEISEI_BRAIN_ROOT": brain.root.to_string_lossy(), - "KEISEI_BRAIN_NAME": brain.name(), + let entry = build_entry(brain); + let map = servers.as_object_mut().expect("servers is object"); + if let Some(existing) = map.get(MCP_ENTRY_KEY) { + if existing != &entry { + return Err(Error::NameConflict { + name: MCP_ENTRY_KEY.to_string(), + existing_client: CLIENT_NAME.to_string(), + }); } - }); - servers - .as_object_mut() - .expect("servers is object") - .insert(MCP_ENTRY_KEY.to_string(), entry); + } + map.insert(MCP_ENTRY_KEY.to_string(), entry); + Ok(()) } fn remove_mcp_entry(doc: &mut Value) { diff --git a/_primitives/_rust/keisei/src/adapters/continue_adapter.rs b/_primitives/_rust/keisei/src/adapters/continue_adapter.rs index 31805c4..44cf9e7 100644 --- a/_primitives/_rust/keisei/src/adapters/continue_adapter.rs +++ b/_primitives/_rust/keisei/src/adapters/continue_adapter.rs @@ -22,15 +22,21 @@ //! in the public docs. If the live schema diverges, update this module. //! //! Detach preserves all other config keys and all non-keisei servers. +//! +//! Security (v0.19 audit): if an existing `name: keisei` entry has +//! different content than we'd write, attach fails with `NameConflict` +//! instead of silent overwrite. use crate::adapter::ClientAdapter; use crate::brain::Brain; use crate::error::{Error, Result}; use crate::fsx::write_atomic; +use crate::paths; use serde_json::{json, Value}; use std::path::PathBuf; pub const SERVER_NAME: &str = "keisei"; +pub const CLIENT_NAME: &str = "continue"; #[derive(Clone, Copy, PartialEq, Eq)] enum Form { @@ -46,12 +52,7 @@ impl ContinueAdapter { } fn continue_dir(&self) -> PathBuf { - let base = std::env::var("KEISEI_HOME") - .ok() - .map(PathBuf::from) - .or_else(|| std::env::var("HOME").ok().map(PathBuf::from)) - .unwrap_or_else(|| PathBuf::from(".")); - base.join(".continue") + paths::resolve_home().join(".continue") } fn pick_form_and_path(&self) -> (Form, PathBuf) { @@ -77,7 +78,7 @@ impl Default for ContinueAdapter { impl ClientAdapter for ContinueAdapter { fn name(&self) -> &str { - "continue" + CLIENT_NAME } fn detect(&self) -> bool { @@ -90,7 +91,7 @@ impl ClientAdapter for ContinueAdapter { std::fs::create_dir_all(parent)?; } let mut doc = load_doc(&cfg, form)?; - merge_entry(&mut doc, brain); + merge_entry(&mut doc, brain)?; write_doc(&cfg, form, &doc) } @@ -138,12 +139,8 @@ fn write_doc(cfg: &std::path::Path, form: Form, doc: &Value) -> Result<()> { write_atomic(cfg, &text) } -fn merge_entry(doc: &mut Value, brain: &Brain) { - if !doc.is_object() { - *doc = json!({}); - } - let obj = doc.as_object_mut().expect("doc is object after guard"); - let entry = json!({ +fn build_entry(brain: &Brain) -> Value { + json!({ "name": SERVER_NAME, "command": brain.mcp_server_path().to_string_lossy(), "args": [], @@ -151,7 +148,15 @@ fn merge_entry(doc: &mut Value, brain: &Brain) { "KEISEI_BRAIN_ROOT": brain.root.to_string_lossy(), "KEISEI_BRAIN_NAME": brain.name(), } - }); + }) +} + +fn merge_entry(doc: &mut Value, brain: &Brain) -> Result<()> { + if !doc.is_object() { + *doc = json!({}); + } + let obj = doc.as_object_mut().expect("doc is object after guard"); + let entry = build_entry(brain); let servers = obj .entry("mcpServers".to_string()) .or_insert_with(|| Value::Array(Vec::new())); @@ -159,8 +164,20 @@ fn merge_entry(doc: &mut Value, brain: &Brain) { *servers = Value::Array(Vec::new()); } let arr = servers.as_array_mut().expect("array after guard"); + if let Some(existing) = arr + .iter() + .find(|v| v.get("name").and_then(|n| n.as_str()) == Some(SERVER_NAME)) + { + if existing != &entry { + return Err(Error::NameConflict { + name: SERVER_NAME.to_string(), + existing_client: CLIENT_NAME.to_string(), + }); + } + } arr.retain(|v| v.get("name").and_then(|n| n.as_str()) != Some(SERVER_NAME)); arr.push(entry); + Ok(()) } fn remove_entry(doc: &mut Value) { diff --git a/_primitives/_rust/keisei/src/adapters/cursor.rs b/_primitives/_rust/keisei/src/adapters/cursor.rs index 29ffcaf..cda8b5f 100644 --- a/_primitives/_rust/keisei/src/adapters/cursor.rs +++ b/_primitives/_rust/keisei/src/adapters/cursor.rs @@ -4,15 +4,21 @@ //! dir exists, else `~/.cursor/mcp.json`. Detection fires if either dir //! exists. Schema [UNVERIFIED — matches Claude Desktop MCP convention]: //! `{ "mcpServers": { "keisei": { "command": "...", "args": [] } } }`. +//! +//! Security (v0.19 audit): collision-safe — if `mcpServers["keisei"]` +//! already exists with different content, attach fails with +//! `NameConflict` rather than silently clobbering. use crate::adapter::ClientAdapter; use crate::brain::Brain; -use crate::error::Result; -use crate::fsx::write_atomic; +use crate::error::{Error, Result}; +use crate::fsx::write_atomic_json; +use crate::paths; use serde_json::{json, Map, Value}; use std::path::PathBuf; pub const MCP_ENTRY_KEY: &str = "keisei"; +pub const CLIENT_NAME: &str = "cursor"; pub struct CursorAdapter; @@ -22,12 +28,7 @@ impl CursorAdapter { } fn home_cursor_dir(&self) -> PathBuf { - let base = std::env::var("KEISEI_HOME") - .ok() - .map(PathBuf::from) - .or_else(|| std::env::var("HOME").ok().map(PathBuf::from)) - .unwrap_or_else(|| PathBuf::from(".")); - base.join(".cursor") + paths::resolve_home().join(".cursor") } fn project_cursor_dir(&self) -> Option { @@ -52,7 +53,7 @@ impl Default for CursorAdapter { impl ClientAdapter for CursorAdapter { fn name(&self) -> &str { - "cursor" + CLIENT_NAME } fn detect(&self) -> bool { @@ -69,8 +70,8 @@ impl ClientAdapter for CursorAdapter { std::fs::create_dir_all(parent)?; } let mut doc = load_json_or_empty(&cfg)?; - merge_entry(&mut doc, brain); - write_atomic(&cfg, &serde_json::to_string_pretty(&doc)?) + merge_entry(&mut doc, brain)?; + write_atomic_json(&cfg, &doc) } fn detach(&self) -> Result<()> { @@ -80,7 +81,7 @@ impl ClientAdapter for CursorAdapter { } let mut doc = load_json_or_empty(&cfg)?; remove_entry(&mut doc); - write_atomic(&cfg, &serde_json::to_string_pretty(&doc)?) + write_atomic_json(&cfg, &doc) } fn config_path(&self) -> PathBuf { @@ -99,7 +100,18 @@ fn load_json_or_empty(cfg: &std::path::Path) -> Result { Ok(serde_json::from_str(&raw)?) } -fn merge_entry(doc: &mut Value, brain: &Brain) { +fn build_entry(brain: &Brain) -> Value { + json!({ + "command": brain.mcp_server_path().to_string_lossy(), + "args": [], + "env": { + "KEISEI_BRAIN_ROOT": brain.root.to_string_lossy(), + "KEISEI_BRAIN_NAME": brain.name(), + } + }) +} + +fn merge_entry(doc: &mut Value, brain: &Brain) -> Result<()> { if !doc.is_object() { *doc = json!({}); } @@ -110,18 +122,18 @@ fn merge_entry(doc: &mut Value, brain: &Brain) { if !servers.is_object() { *servers = Value::Object(Map::new()); } - let entry = json!({ - "command": brain.mcp_server_path().to_string_lossy(), - "args": [], - "env": { - "KEISEI_BRAIN_ROOT": brain.root.to_string_lossy(), - "KEISEI_BRAIN_NAME": brain.name(), + let entry = build_entry(brain); + let map = servers.as_object_mut().expect("servers is object"); + if let Some(existing) = map.get(MCP_ENTRY_KEY) { + if existing != &entry { + return Err(Error::NameConflict { + name: MCP_ENTRY_KEY.to_string(), + existing_client: CLIENT_NAME.to_string(), + }); } - }); - servers - .as_object_mut() - .expect("servers is object") - .insert(MCP_ENTRY_KEY.to_string(), entry); + } + map.insert(MCP_ENTRY_KEY.to_string(), entry); + Ok(()) } fn remove_entry(doc: &mut Value) { diff --git a/_primitives/_rust/keisei/src/adapters/zed.rs b/_primitives/_rust/keisei/src/adapters/zed.rs index 8a16db5..e0942a7 100644 --- a/_primitives/_rust/keisei/src/adapters/zed.rs +++ b/_primitives/_rust/keisei/src/adapters/zed.rs @@ -23,19 +23,20 @@ //! environment) is [UNVERIFIED] in this session. If a future Zed release //! diverges, update this module. //! -//! Detach preserves all other settings and all non-keisei context servers. -//! If the settings file is not valid JSON (Zed historically permitted -//! JSONC / comments), attach degrades gracefully by returning -//! `ConfigParseError` — the user can then switch to manual config. +//! Security (v0.19 audit): collision-safe — if `context_servers["keisei"]` +//! already exists with different content, attach fails with +//! `NameConflict` rather than silently clobbering. use crate::adapter::ClientAdapter; use crate::brain::Brain; use crate::error::{Error, Result}; -use crate::fsx::write_atomic; +use crate::fsx::write_atomic_json; +use crate::paths; use serde_json::{json, Map, Value}; use std::path::PathBuf; pub const ENTRY_KEY: &str = "keisei"; +pub const CLIENT_NAME: &str = "zed"; pub struct ZedAdapter; @@ -45,11 +46,7 @@ impl ZedAdapter { } fn settings_dir(&self) -> PathBuf { - let base = std::env::var("KEISEI_HOME") - .ok() - .map(PathBuf::from) - .or_else(|| std::env::var("HOME").ok().map(PathBuf::from)) - .unwrap_or_else(|| PathBuf::from(".")); + let base = paths::resolve_home(); if cfg!(target_os = "macos") { base.join("Library/Application Support/Zed") } else { @@ -70,7 +67,7 @@ impl Default for ZedAdapter { impl ClientAdapter for ZedAdapter { fn name(&self) -> &str { - "zed" + CLIENT_NAME } fn detect(&self) -> bool { @@ -83,8 +80,8 @@ impl ClientAdapter for ZedAdapter { std::fs::create_dir_all(parent)?; } let mut doc = load_json_or_empty(&cfg)?; - merge_entry(&mut doc, brain); - write_atomic(&cfg, &serde_json::to_string_pretty(&doc)?)?; + merge_entry(&mut doc, brain)?; + write_atomic_json(&cfg, &doc)?; Ok(()) } @@ -95,7 +92,7 @@ impl ClientAdapter for ZedAdapter { } let mut doc = load_json_or_empty(&cfg)?; remove_entry(&mut doc); - write_atomic(&cfg, &serde_json::to_string_pretty(&doc)?)?; + write_atomic_json(&cfg, &doc)?; Ok(()) } @@ -118,7 +115,18 @@ fn load_json_or_empty(cfg: &std::path::Path) -> Result { }) } -fn merge_entry(doc: &mut Value, brain: &Brain) { +fn build_entry(brain: &Brain) -> Value { + json!({ + "command": brain.mcp_server_path().to_string_lossy(), + "args": [], + "env": { + "KEISEI_BRAIN_ROOT": brain.root.to_string_lossy(), + "KEISEI_BRAIN_NAME": brain.name(), + } + }) +} + +fn merge_entry(doc: &mut Value, brain: &Brain) -> Result<()> { if !doc.is_object() { *doc = json!({}); } @@ -129,18 +137,18 @@ fn merge_entry(doc: &mut Value, brain: &Brain) { if !servers.is_object() { *servers = Value::Object(Map::new()); } - let entry = json!({ - "command": brain.mcp_server_path().to_string_lossy(), - "args": [], - "env": { - "KEISEI_BRAIN_ROOT": brain.root.to_string_lossy(), - "KEISEI_BRAIN_NAME": brain.name(), + let entry = build_entry(brain); + let map = servers.as_object_mut().expect("servers is object"); + if let Some(existing) = map.get(ENTRY_KEY) { + if existing != &entry { + return Err(Error::NameConflict { + name: ENTRY_KEY.to_string(), + existing_client: CLIENT_NAME.to_string(), + }); } - }); - servers - .as_object_mut() - .expect("servers is object") - .insert(ENTRY_KEY.to_string(), entry); + } + map.insert(ENTRY_KEY.to_string(), entry); + Ok(()) } fn remove_entry(doc: &mut Value) { diff --git a/_primitives/_rust/keisei/src/brain.rs b/_primitives/_rust/keisei/src/brain.rs index 8abad1b..a2756a5 100644 --- a/_primitives/_rust/keisei/src/brain.rs +++ b/_primitives/_rust/keisei/src/brain.rs @@ -8,24 +8,27 @@ //! ```toml //! [brain] //! schema_version = 1 -//! name = "my-ai-brain" +//! name = "my-ai-brain" # ^[a-z][a-z0-9_-]{0,63}$ //! created = "2026-04-22T00:00:00Z" //! //! [paths] -//! memory = "memory/" -//! artifacts = "artifacts/" -//! manifests = "manifests/" -//! mcp_server = "bin/kei-mcp-server-darwin-arm64" +//! mcp_server = "bin/kei-mcp-server-darwin-arm64" # REQUIRED, relative, in-root +//! memory = "memory/" # optional +//! artifacts = "artifacts/" # optional +//! manifests = "manifests/" # optional //! ``` //! -//! Paths under `[paths]` are interpreted relative to the brain root. The -//! loader canonicalizes them on construction so downstream code can treat -//! them as absolute. +//! Every path under `[paths]` MUST be relative AND resolve (after +//! canonicalization) inside the brain root. Absolute paths or `..` +//! traversal are rejected with `Error::PathEscape`. Symlink roots are +//! rejected with `Error::BrainIsSymlink` (user must pass the canonical +//! path explicitly to avoid USB→host pivot). //! -//! Constructor Pattern: single responsibility — parse and validate the -//! brain manifest. No I/O beyond the initial read. No client coupling. +//! Constructor Pattern: single responsibility — parse + compose the five +//! validation primitives from `brain_validate.rs` into the load pipeline. -use crate::error::{Error, Result}; +use crate::brain_validate as v; +use crate::error::Result; use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; @@ -42,10 +45,17 @@ pub struct BrainMeta { #[derive(Debug, Serialize, Deserialize, Clone)] pub struct BrainPaths { - pub memory: String, - pub artifacts: String, - pub manifests: String, + /// Required. Path to the MCP server binary, relative to the brain root. pub mcp_server: String, + /// Optional. If present, must be relative + in-root. + #[serde(default)] + pub memory: Option, + /// Optional. If present, must be relative + in-root. + #[serde(default)] + pub artifacts: Option, + /// Optional. If present, must be relative + in-root. + #[serde(default)] + pub manifests: Option, } #[derive(Debug, Serialize, Deserialize, Clone)] @@ -58,46 +68,55 @@ pub struct BrainManifest { pub struct Brain { pub root: PathBuf, pub manifest: BrainManifest, + /// Canonical absolute path to the mcp_server binary, pre-validated + /// to live under `root`. + canonical_mcp_server: PathBuf, } impl Brain { /// Load a brain from `/manifest.toml`. - /// Canonicalizes `root` to an absolute path and validates schema_version. - pub fn load(root: &Path) -> Result { - let root = root - .canonicalize() - .map_err(|_| Error::BrainNotFound(root.to_path_buf()))?; - let mpath = root.join(MANIFEST_FILENAME); - if !mpath.is_file() { - return Err(Error::BrainNotFound(mpath)); - } - let raw = std::fs::read_to_string(&mpath)?; - let manifest: BrainManifest = toml::from_str(&raw)?; - if manifest.brain.schema_version != SUPPORTED_SCHEMA { - return Err(Error::UnsupportedSchema { - found: manifest.brain.schema_version, - }); - } - Ok(Self { root, manifest }) - } - - /// Resolve a manifest-declared path (relative or absolute) to an - /// absolute path under the brain root. Trailing slashes are preserved - /// by `PathBuf` normalization. - pub fn resolve(&self, rel: &str) -> PathBuf { - let p = Path::new(rel); - if p.is_absolute() { - p.to_path_buf() - } else { - self.root.join(p) - } + /// + /// Order matters (security-critical): + /// 1. Reject symlink-rooted inputs (SEC-H3 — USB/host pivot). + /// 2. Canonicalize `root`. + /// 3. Parse manifest, validate schema_version. + /// 4. Validate `brain.name` against regex. + /// 5. Validate + canonicalize every [paths] field; assert in-root. + pub fn load(input: &Path) -> Result { + v::reject_symlink_root(input)?; + let root = v::canonicalize_root(input)?; + let manifest = v::read_manifest(&root)?; + v::validate_schema(&manifest)?; + v::validate_name(&manifest.brain.name)?; + check_all_paths(&manifest)?; + let canonical_mcp_server = v::canonicalize_in_root(&root, &manifest.paths.mcp_server)?; + Ok(Self { + root, + manifest, + canonical_mcp_server, + }) } + /// Pre-validated, canonicalized absolute path to the mcp_server binary. pub fn mcp_server_path(&self) -> PathBuf { - self.resolve(&self.manifest.paths.mcp_server) + self.canonical_mcp_server.clone() } pub fn name(&self) -> &str { &self.manifest.brain.name } } + +fn check_all_paths(manifest: &BrainManifest) -> Result<()> { + v::check_relative_in_root(&manifest.paths.mcp_server)?; + if let Some(p) = manifest.paths.memory.as_deref() { + v::check_relative_in_root(p)?; + } + if let Some(p) = manifest.paths.artifacts.as_deref() { + v::check_relative_in_root(p)?; + } + if let Some(p) = manifest.paths.manifests.as_deref() { + v::check_relative_in_root(p)?; + } + Ok(()) +} diff --git a/_primitives/_rust/keisei/src/brain_validate.rs b/_primitives/_rust/keisei/src/brain_validate.rs new file mode 100644 index 0000000..818f568 --- /dev/null +++ b/_primitives/_rust/keisei/src/brain_validate.rs @@ -0,0 +1,108 @@ +//! Validation primitives for `Brain::load`. +//! +//! Constructor Pattern: single responsibility — own the five mechanical +//! checks (symlink reject / root canonicalize / manifest read / name +//! regex / in-root path guard). `brain.rs` composes them into the load +//! pipeline. No cross-module state; every fn is pure w.r.t. filesystem. + +use crate::brain::{BrainManifest, MANIFEST_FILENAME, SUPPORTED_SCHEMA}; +use crate::error::{Error, Result}; +use regex::Regex; +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; + +fn name_regex() -> &'static Regex { + static RE: OnceLock = OnceLock::new(); + RE.get_or_init(|| Regex::new(r"^[a-z][a-z0-9_-]{0,63}$").expect("valid regex")) +} + +pub fn reject_symlink_root(input: &Path) -> Result<()> { + match std::fs::symlink_metadata(input) { + Ok(md) if md.file_type().is_symlink() => { + let target = std::fs::read_link(input).unwrap_or_else(|_| PathBuf::from("?")); + Err(Error::BrainIsSymlink { + input: input.to_path_buf(), + target, + }) + } + // Missing-path → let canonicalize produce the final error message. + _ => Ok(()), + } +} + +pub fn canonicalize_root(input: &Path) -> Result { + input.canonicalize().map_err(|e| { + if e.kind() == std::io::ErrorKind::NotFound { + Error::BrainNotFound(input.to_path_buf()) + } else { + Error::BrainLoad { + path: input.to_path_buf(), + source: e, + } + } + }) +} + +pub fn read_manifest(root: &Path) -> Result { + let mpath = root.join(MANIFEST_FILENAME); + if !mpath.is_file() { + return Err(Error::BrainNotFound(mpath)); + } + let raw = std::fs::read_to_string(&mpath)?; + let manifest: BrainManifest = toml::from_str(&raw)?; + Ok(manifest) +} + +pub fn validate_schema(manifest: &BrainManifest) -> Result<()> { + if manifest.brain.schema_version != SUPPORTED_SCHEMA { + return Err(Error::UnsupportedSchema { + found: manifest.brain.schema_version, + }); + } + Ok(()) +} + +pub fn validate_name(name: &str) -> Result<()> { + if name_regex().is_match(name) { + Ok(()) + } else { + Err(Error::InvalidName(name.to_string())) + } +} + +/// Syntactic check before touching disk: absolute path or `..` component +/// → `PathEscape`. Filters obvious attacks without requiring the target +/// to exist. +pub fn check_relative_in_root(rel: &str) -> Result<()> { + let p = Path::new(rel); + if p.is_absolute() { + return Err(Error::PathEscape(p.to_path_buf())); + } + for comp in p.components() { + if matches!(comp, std::path::Component::ParentDir) { + return Err(Error::PathEscape(p.to_path_buf())); + } + } + Ok(()) +} + +/// Resolve + canonicalize a manifest-declared relative path and assert it +/// lives under `root`. Called only on paths that already passed +/// `check_relative_in_root`. +pub fn canonicalize_in_root(root: &Path, rel: &str) -> Result { + let joined = root.join(rel); + let canonical = joined.canonicalize().map_err(|e| { + if e.kind() == std::io::ErrorKind::NotFound { + Error::BrainNotFound(joined.clone()) + } else { + Error::BrainLoad { + path: joined.clone(), + source: e, + } + } + })?; + if !canonical.starts_with(root) { + return Err(Error::PathEscape(canonical)); + } + Ok(canonical) +} diff --git a/_primitives/_rust/keisei/src/config.rs b/_primitives/_rust/keisei/src/config.rs index de96126..ac0be1c 100644 --- a/_primitives/_rust/keisei/src/config.rs +++ b/_primitives/_rust/keisei/src/config.rs @@ -99,12 +99,7 @@ impl WireRecord { } pub fn home_root() -> PathBuf { - let base = std::env::var("KEISEI_HOME") - .ok() - .map(PathBuf::from) - .or_else(|| std::env::var("HOME").ok().map(PathBuf::from)) - .unwrap_or_else(|| PathBuf::from(".")); - base.join(".claude") + crate::paths::resolve_home().join(".claude") } pub fn attached_path() -> PathBuf { diff --git a/_primitives/_rust/keisei/src/error.rs b/_primitives/_rust/keisei/src/error.rs index 071b6c5..28dacb0 100644 --- a/_primitives/_rust/keisei/src/error.rs +++ b/_primitives/_rust/keisei/src/error.rs @@ -28,6 +28,40 @@ pub enum Error { #[error("config parse error at {path}: {reason}")] ConfigParseError { path: PathBuf, reason: String }, + #[error( + "brain manifest path '{0}' escapes the brain root \ + (absolute path, parent traversal, or canonical mismatch)" + )] + PathEscape(PathBuf), + + #[error( + "invalid brain name '{0}' — must match ^[a-z][a-z0-9_-]{{0,63}}$ \ + (lowercase, letter-start, 1-64 chars, word chars + hyphen)" + )] + InvalidName(String), + + #[error( + "MCP entry '{name}' already exists in {existing_client} config with \ + different content; resolve manually (keisei will not clobber)" + )] + NameConflict { + name: String, + existing_client: String, + }, + + #[error( + "brain path '{input}' is a symlink to '{target}'; \ + pass the canonical path explicitly to avoid USB/host pivot ambiguity" + )] + BrainIsSymlink { input: PathBuf, target: PathBuf }, + + #[error("failed to load brain at {path}: {source}")] + BrainLoad { + path: PathBuf, + #[source] + source: std::io::Error, + }, + #[error("i/o error: {0}")] Io(#[from] std::io::Error), diff --git a/_primitives/_rust/keisei/src/fsx.rs b/_primitives/_rust/keisei/src/fsx.rs index ec27757..e3c0a7a 100644 --- a/_primitives/_rust/keisei/src/fsx.rs +++ b/_primitives/_rust/keisei/src/fsx.rs @@ -1,30 +1,40 @@ //! Filesystem helpers shared across adapters. //! //! Constructor Pattern: single responsibility — own the write-then-rename -//! pattern. Kept in a dedicated module so every adapter shares the exact -//! same crash-safe write, regardless of extension. +//! pattern. Every adapter shares the exact same crash-safe write, +//! regardless of extension. +//! +//! Uses `tempfile::NamedTempFile::persist` so: +//! - on Windows, a locked target no longer leaks a stale `.tmp` file +//! (the temp file is cleaned up on drop if `persist` failed); +//! - on crash mid-write, the original target is preserved intact; +//! - cross-filesystem persist gracefully falls back to copy-then-remove +//! via `tempfile`'s own logic. use crate::error::Result; -use std::ffi::OsString; +use std::io::Write; use std::path::Path; -/// Write-then-rename to avoid truncating the target on crash. -/// -/// The temp file lives in the same directory as the target (required for -/// a POSIX-atomic rename). Its name is `.tmp` — no randomness -/// since only one writer touches an adapter's config at a time. +/// Atomic write. Temp file lives in the target's parent dir, then is +/// persisted (renamed) onto the target. Uses `tempfile::NamedTempFile` +/// under the hood. pub fn write_atomic(target: &Path, content: &str) -> Result<()> { - let tmp = tmp_path(target); - std::fs::write(&tmp, content)?; - std::fs::rename(&tmp, target)?; + let parent = target + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + std::fs::create_dir_all(parent)?; + let mut tmp = tempfile::NamedTempFile::new_in(parent)?; + tmp.write_all(content.as_bytes())?; + tmp.flush()?; + tmp.persist(target).map_err(|e| e.error)?; Ok(()) } -fn tmp_path(target: &Path) -> std::path::PathBuf { - let mut name: OsString = target.file_name().map(OsString::from).unwrap_or_default(); - name.push(".tmp"); - match target.parent() { - Some(p) if !p.as_os_str().is_empty() => p.join(name), - _ => std::path::PathBuf::from(name), - } +/// Convenience: serialize a `serde_json::Value` as pretty JSON and +/// atomically write it. Every adapter that targets a JSON file uses +/// this — keeps the serialization shape identical across adapters. +pub fn write_atomic_json(target: &Path, doc: &serde_json::Value) -> Result<()> { + let text = serde_json::to_string_pretty(doc)?; + write_atomic(target, &text) } diff --git a/_primitives/_rust/keisei/src/main.rs b/_primitives/_rust/keisei/src/main.rs index ab3aacc..9481911 100644 --- a/_primitives/_rust/keisei/src/main.rs +++ b/_primitives/_rust/keisei/src/main.rs @@ -8,12 +8,14 @@ mod adapter; mod adapters; mod attach; mod brain; +mod brain_validate; mod config; mod detach; mod error; mod fsx; mod list; mod mount; +mod paths; mod status; use clap::{Parser, Subcommand}; diff --git a/_primitives/_rust/keisei/src/paths.rs b/_primitives/_rust/keisei/src/paths.rs new file mode 100644 index 0000000..54a00cb --- /dev/null +++ b/_primitives/_rust/keisei/src/paths.rs @@ -0,0 +1,23 @@ +//! Host path resolution — SSoT for `$KEISEI_HOME` / `$HOME` fallback. +//! +//! Constructor Pattern: single responsibility — resolve the user's home +//! directory for every adapter + the attach marker. `$KEISEI_HOME` +//! overrides `$HOME` so integration tests can isolate state per tmpdir. +//! Adapters compose on top of this SSoT; no duplication of the env-var +//! chain across the codebase. + +use std::path::PathBuf; + +/// Resolve the user's home directory. +/// +/// Precedence: +/// 1. `$KEISEI_HOME` (test isolation knob) +/// 2. `$HOME` (standard POSIX) +/// 3. `"."` (degenerate; only hit if both env vars are absent) +pub fn resolve_home() -> PathBuf { + std::env::var("KEISEI_HOME") + .ok() + .map(PathBuf::from) + .or_else(|| std::env::var("HOME").ok().map(PathBuf::from)) + .unwrap_or_else(|| PathBuf::from(".")) +} diff --git a/_primitives/_rust/keisei/tests/integration.rs b/_primitives/_rust/keisei/tests/integration.rs index 96077e5..479461c 100644 --- a/_primitives/_rust/keisei/tests/integration.rs +++ b/_primitives/_rust/keisei/tests/integration.rs @@ -8,8 +8,12 @@ #[path = "../src/error.rs"] mod error; +#[path = "../src/paths.rs"] +mod paths; #[path = "../src/brain.rs"] mod brain; +#[path = "../src/brain_validate.rs"] +mod brain_validate; #[path = "../src/config.rs"] mod config; #[path = "../src/fsx.rs"] @@ -297,6 +301,156 @@ fn list_adapters_prints_expected_rows() { list::run().expect("list-adapters ok"); } +// ----------------------------------------------------------------------- +// v0.19 audit-hardening tests (SEC-H1 / H2 / H3 path + name + symlink). +// ----------------------------------------------------------------------- + +/// Write a brain manifest with a caller-chosen `mcp_server` string. Used +/// to exercise the path-escape rejection paths. Does NOT create the +/// `mcp_server` file itself — the manifest-time rejection fires before +/// canonicalization would ever run. +fn write_brain_raw_mcp(root: &Path, mcp_server: &str) -> PathBuf { + let manifest = format!( + r#"[brain] +schema_version = 1 +name = "test-brain" +created = "2026-04-22T00:00:00Z" + +[paths] +mcp_server = "{mcp_server}" +"# + ); + fs::write(root.join("manifest.toml"), manifest).unwrap(); + root.to_path_buf() +} + +/// Write a brain manifest with a caller-chosen `name`. Used to exercise +/// the name-regex rejection path. `mcp_server` still points at a real +/// file so name-validation runs before path-canonicalization. +fn write_brain_raw_name(root: &Path, name: &str) -> PathBuf { + fs::create_dir_all(root.join("bin")).unwrap(); + fs::write(root.join("bin/kei-mcp-server-test"), b"#!/bin/sh\n").unwrap(); + let manifest = format!( + r#"[brain] +schema_version = 1 +name = "{name}" +created = "2026-04-22T00:00:00Z" + +[paths] +mcp_server = "bin/kei-mcp-server-test" +"# + ); + fs::write(root.join("manifest.toml"), manifest).unwrap(); + root.to_path_buf() +} + +#[test] +fn manifest_with_absolute_mcp_server_rejected() { + let _g = setup_home(); + let brain_dir = tempfile::tempdir().unwrap(); + // Malicious manifest: absolute path to arbitrary host binary. + write_brain_raw_mcp(brain_dir.path(), "/usr/bin/curl"); + let err = attach::run(brain_dir.path()).unwrap_err(); + assert!( + matches!(err, error::Error::PathEscape(_)), + "expected PathEscape, got {err:?}" + ); + // Containment: marker MUST NOT be written. + assert!(config::read().unwrap().is_none()); +} + +#[test] +fn manifest_with_parent_traversal_rejected() { + let _g = setup_home(); + let brain_dir = tempfile::tempdir().unwrap(); + write_brain_raw_mcp(brain_dir.path(), "../../etc/passwd"); + let err = attach::run(brain_dir.path()).unwrap_err(); + assert!( + matches!(err, error::Error::PathEscape(_)), + "expected PathEscape, got {err:?}" + ); + assert!(config::read().unwrap().is_none()); +} + +#[test] +fn manifest_with_invalid_name_rejected() { + let _g = setup_home(); + let brain_dir = tempfile::tempdir().unwrap(); + // "claude-ide!" contains `!` — forbidden by ^[a-z][a-z0-9_-]{0,63}$. + write_brain_raw_name(brain_dir.path(), "claude-ide!"); + let err = attach::run(brain_dir.path()).unwrap_err(); + assert!( + matches!(err, error::Error::InvalidName(ref s) if s == "claude-ide!"), + "expected InvalidName(\"claude-ide!\"), got {err:?}" + ); +} + +#[test] +fn brain_path_is_symlink_rejected() { + let _g = setup_home(); + // Target brain is legitimate... + let target = tempfile::tempdir().unwrap(); + write_brain(target.path(), 1); + // ...but caller passes a symlink pointing at it (USB/host pivot). + let link_parent = tempfile::tempdir().unwrap(); + let link = link_parent.path().join("brain-link"); + #[cfg(unix)] + std::os::unix::fs::symlink(target.path(), &link).unwrap(); + #[cfg(windows)] + std::os::windows::fs::symlink_dir(target.path(), &link).unwrap(); + + let err = attach::run(&link).unwrap_err(); + assert!( + matches!(err, error::Error::BrainIsSymlink { .. }), + "expected BrainIsSymlink, got {err:?}" + ); + // No marker, no config pollution. + assert!(config::read().unwrap().is_none()); +} + +#[test] +fn attach_refuses_to_clobber_existing_mcp_entry() { + let _g = setup_home(); + // Pre-populate settings.json with a DIFFERENT `keisei` entry. + let settings = config::home_root().join("settings.json"); + fs::create_dir_all(settings.parent().unwrap()).unwrap(); + fs::write( + &settings, + r#"{ + "mcpServers": { + "keisei": { + "command": "/tmp/not-our-binary", + "args": ["--evil"] + } + } +}"#, + ) + .unwrap(); + + let brain_dir = tempfile::tempdir().unwrap(); + write_brain(brain_dir.path(), 1); + let err = attach::run(brain_dir.path()).unwrap_err(); + assert!( + matches!(err, error::Error::NameConflict { .. }), + "expected NameConflict, got {err:?}" + ); + + // User's pre-existing entry survives intact. + let after: Value = serde_json::from_str(&fs::read_to_string(&settings).unwrap()).unwrap(); + let cmd = after + .get("mcpServers") + .and_then(|s| s.get("keisei")) + .and_then(|k| k.get("command")) + .and_then(|c| c.as_str()) + .unwrap_or(""); + assert_eq!( + cmd, "/tmp/not-our-binary", + "pre-existing keisei entry was mutated; attach should have been a no-op on conflict" + ); + // Marker MUST NOT be written on conflict. + assert!(config::read().unwrap().is_none()); +} + #[test] fn schema_v1_to_v2_migration() { let _g = setup_home();