From 36b7758a6f1988f15dcdfc43eb9030eb995e4f3a Mon Sep 17 00:00:00 2001 From: ajianaz Date: Tue, 22 Sep 2026 23:26:40 +0700 Subject: [PATCH] =?UTF-8?q?fix:=20scan-3=20security/spec=20surfaces=20?= =?UTF-8?q?=E2=80=94=20hook=20env=20scrub,=20cora=5Fsearch=20dash=20guard,?= =?UTF-8?q?=20uteke=20spec=20truth,=20mcp=20plan-mode,=20chat=20flag=20sco?= =?UTF-8?q?pe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - hooks: hook subprocesses scrub secret-shaped env like every other child spawn (scan-3 #23). - cora_search: leading-dash query guard, mirroring uteke_recall (scan-3 #32). - uteke_document spec no longer advertises title/tags that execute() silently drops (scan-3 #33). - tole mcp honors --plan-mode (read-only served registry) and loudly rejects --on-pretool/--on-posttool (scan-3 #9). - chat REPL dropped_message scoped per message (scan-3 #8). --- crates/tole-cli/src/main.rs | 66 ++++++++++++++++++++++++++--- crates/tole-core/src/cora_search.rs | 5 +++ crates/tole-core/src/hooks.rs | 4 ++ crates/tole-core/src/uteke.rs | 11 +++-- 4 files changed, 76 insertions(+), 10 deletions(-) diff --git a/crates/tole-cli/src/main.rs b/crates/tole-cli/src/main.rs index d4c1043..ac7bed1 100644 --- a/crates/tole-cli/src/main.rs +++ b/crates/tole-cli/src/main.rs @@ -231,7 +231,7 @@ fn dispatch(cli: Cli) -> Result<()> { #[cfg(feature = "shell-tools")] memory: resolve_memory(cli.memory.as_ref())?, #[cfg(not(feature = "shell-tools"))] - memory: None, + memory: (), }; match cli.command { Command::Run { @@ -264,7 +264,29 @@ fn dispatch(cli: Cli) -> Result<()> { Command::Mcp { allow_patterns, workspace, - } => mcp_server_command(workspace.as_ref(), &allow_patterns), + } => { + // Global flags must not SILENTLY no-op on this subcommand + // (cora scan-3 #9): plan-mode filters the served registry to + // read-only; hooks are not wired in server mode (no local + // approver boundary — server mode pre-authorizes via + // --allow), so --on-pretool/--on-posttool error out loudly + // instead of being ignored. + if host.on_pretool_non_empty() || host.on_posttool_non_empty() { + anyhow::bail!( + "--on-pretool/--on-posttool are not supported by `tole mcp` \ + (server mode pre-authorizes Write tools with --allow instead)" + ); + } + if host.plan_mode { + eprintln!("tole mcp: --plan-mode is active — serving read-only tools only"); + } + mcp_server_command( + workspace.as_ref(), + &allow_patterns, + #[cfg(feature = "mcp")] + host.plan_mode, + ) + } Command::Sessions => sessions_command(&sessions_dir), Command::Status { id } => status_command(&sessions_dir, &id), Command::Chat { @@ -302,10 +324,26 @@ struct HostConfig { /// Tool-boundary hook command lines (issue #110), default empty. on_pretool: Vec, on_posttool: Vec, + /// shell-tools-only host knob. `not(feature = "shell-tools")` builds + /// still assign `memory: None` in dispatch — the field stays so the + /// assignments and helper signatures never fork per profile. + #[cfg(not(feature = "shell-tools"))] + memory: (), #[cfg(feature = "shell-tools")] memory: Option, } +impl HostConfig { + #[cfg(any(feature = "shell-tools", feature = "mcp"))] + fn on_pretool_non_empty(&self) -> bool { + !self.on_pretool.is_empty() + } + #[cfg(any(feature = "shell-tools", feature = "mcp"))] + fn on_posttool_non_empty(&self) -> bool { + !self.on_posttool.is_empty() + } +} + #[cfg(feature = "shell-tools")] impl HostConfig { /// Pre-turn recall injection (memory loop): the returned prompt is @@ -674,8 +712,23 @@ fn build_server_registry( /// D1 (issue #94): serve the registry over MCP stdio. Blocks until the /// client disconnects. #[cfg(all(feature = "mcp", feature = "shell-tools"))] -fn mcp_server_command(workspace: Option<&String>, allow_patterns: &[String]) -> Result<()> { +#[cfg_attr(not(feature = "mcp"), allow(unused_variables))] +fn mcp_server_command( + workspace: Option<&String>, + allow_patterns: &[String], + #[cfg(feature = "mcp")] plan_mode: bool, +) -> Result<()> { let registry = build_server_registry(workspace, allow_patterns)?; + // Plan mode (issue #109) applies to server mode too (cora scan-3 + // #9): serve read-only tools only when the operator asked for it. + #[cfg(feature = "mcp")] + let mut registry = registry; + #[cfg(feature = "mcp")] + if plan_mode { + let mut reg = registry; + reg.retain_read_only(); + registry = reg; + } tokio::runtime::Runtime::new() .context("creating tokio runtime")? .block_on(tole_core::mcp_server::serve_stdio(registry)) @@ -1133,11 +1186,12 @@ fn chat_command( // Set when the typed message could not run because the session was // stuck mid-flight and the bounded resolve retries ran out — the // message is NOT in the durable log, so the operator must resend it. - let mut dropped_message = false; - + // SCOPED PER MESSAGE (cora scan-3 #8): declared inside the loop — + // an outer flag never reset, so one drop warned forever after. let stdin = std::io::stdin(); loop { - print!("you> "); + let mut dropped_message = false; + print!("you>"); let _ = std::io::stdout().flush(); let mut line = String::new(); match stdin.lock().read_line(&mut line) { diff --git a/crates/tole-core/src/cora_search.rs b/crates/tole-core/src/cora_search.rs index b1493c7..ee43810 100644 --- a/crates/tole-core/src/cora_search.rs +++ b/crates/tole-core/src/cora_search.rs @@ -82,6 +82,11 @@ impl Tool for CoraSearchTool { if query.trim().is_empty() { return Err("query must not be empty".into()); } + // Flag-injection guard (cora scan-3 #32): a leading '-' would + // reach the cora CLI as a flag — same guard uteke_recall has. + if query.starts_with('-') { + return Err("query must not start with '-'".into()); + } let mut cmd = Command::new("cora"); crate::subprocess::scrub_env_for_child(&mut cmd); cmd.arg("brain") diff --git a/crates/tole-core/src/hooks.rs b/crates/tole-core/src/hooks.rs index d114ee0..8797670 100644 --- a/crates/tole-core/src/hooks.rs +++ b/crates/tole-core/src/hooks.rs @@ -62,6 +62,10 @@ impl ProcessHook { } let mut cmd = Command::new(&self.program); cmd.args(&self.args); + // Hook processes are external programs: strip secret-shaped env + // (API keys etc.) exactly like every other child spawn (cora + // scan-3 #23 — same contract as run_command/memory). + crate::subprocess::scrub_env_for_child(&mut cmd); let out = run_with_timeout_stdin(&mut cmd, self.timeout, payload.to_string().as_bytes())?; // Output cap: a runaway hook cannot flood the durable log. let mut stdout = out.stdout; diff --git a/crates/tole-core/src/uteke.rs b/crates/tole-core/src/uteke.rs index fdaef8e..7aff0dd 100644 --- a/crates/tole-core/src/uteke.rs +++ b/crates/tole-core/src/uteke.rs @@ -174,10 +174,13 @@ impl Tool for UtekeDocumentTool { "type": "object", "properties": { "slug": { "type": "string", "description": "URL-friendly document identifier (kebab-case)" }, - "title": { "type": "string", "description": "Document title (defaults to the first '# ' heading)" }, - "markdown": { "type": "string", "description": "Full markdown content of the document" }, - "room": { "type": "string", "description": "Optional room id to link the document into" }, - "tags": { "type": "array", "items": { "type": "string" }, "description": "Optional tags" } + // NO advertised `title`/`tags`: execute() passes slug + + // markdown only (the uteke doc-create CLI derives the + // title from the first '# ' heading). Advertising inputs + // the tool silently drops is a spec lie (cora scan-3 + // #33) — models waste turns filling dead fields. + "markdown": { "type": "string", "description": "Full markdown content of the document (title = first '# ' heading)" }, + "room": { "type": "string", "description": "Optional room id to link the document into" } }, "required": ["slug", "markdown"] }))