diff --git a/chat.el b/chat.el index d78a579..e76e245 100644 --- a/chat.el +++ b/chat.el @@ -1535,6 +1535,134 @@ fn resolve_in_root(path: String, root: String) -> String { return root + "/" + path } +// --------------------------------------------------------------------------- +// BUG-8: server-side risk tiers + a real fence for run_command. +// +// Before this block, the ONLY thing deciding whether a tool call paused for +// user consent was is_builtin_tool() — a destructive shell command and a +// read-only file read were treated identically (both auto-ran), and the +// client's approval UI was the sole line of defense. Enforcement now lives +// where the tools execute: +// +// "read" observes only — runs silently. +// "reversible" workspace-confined writes with a client undo path — runs, +// lands on the run receipt. +// "escalate" irreversible / outward / shell — NEVER auto-runs. The loop +// suspends to the client's consent flow; the /approve +// round-trip IS the approval token, because the engine only +// executes an escalated tool inside handle_session_approve. +// +// "Always allow" can never bypass the escalate tier (irreversible actions +// always confirm — the value line). Unknown tools default to escalate. +// The run_command fence refuses parent traversal, ~, command substitution, +// and absolute paths outside the workspace — refusal, not a cwd suggestion. +// Still lexical underneath (symlinks; see the LIMITATION note above): tiered +// consent + the fence raise the floor a second and third rung; OS-level +// confinement in el_runtime.c remains the ceiling, flagged for Will. +// --------------------------------------------------------------------------- + +// Read-only shell commands may auto-run (still fenced); anything with shell +// plumbing (pipes, redirects, chaining) or an unknown head word escalates. +fn run_command_is_readonly(cmd: String) -> Bool { + if str_contains(cmd, "|") || str_contains(cmd, ">") || str_contains(cmd, "<") { + return false + } + if str_contains(cmd, ";") || str_contains(cmd, "&") { + return false + } + let sp: Int = str_index_of(cmd, " ") + let first: String = if sp < 0 { cmd } else { str_slice(cmd, 0, sp) } + if str_eq(first, "ls") || str_eq(first, "cat") || str_eq(first, "head") || str_eq(first, "tail") { + return true + } + if str_eq(first, "grep") || str_eq(first, "wc") || str_eq(first, "find") || str_eq(first, "pwd") { + return true + } + if str_eq(first, "echo") || str_eq(first, "date") || str_eq(first, "which") || str_eq(first, "file") || str_eq(first, "stat") { + return true + } + return false +} + +// True if the command references an absolute path (introduced by `needle`, +// whose last char is the "/") that does NOT stay inside the workspace root. +fn cmd_abs_escape_at(cmd: String, root: String, needle: String) -> Bool { + let rest: String = cmd + let found: Bool = false + while !found && str_contains(rest, needle) { + let idx: Int = str_index_of(rest, needle) + let slash_at: Int = idx + str_len(needle) - 1 + let after: String = str_slice(rest, slash_at, str_len(rest)) + let ok: Bool = str_starts_with(after, root + "/") || str_starts_with(after, root + " ") || str_eq(after, root) + let found = if !ok { true } else { found } + let rest = str_slice(rest, slash_at + 1, str_len(rest)) + } + return found +} + +// The run_command fence. Returns "" when the command may run, else the denial +// message (sent back to the model as the tool result, same pattern as the +// path tools). Root is REQUIRED for shell: no workspace, no commands. +fn run_command_guard(cmd: String, root: String) -> String { + if str_eq(root, "") { + return "denied: no workspace folder is set — the user must choose a workspace folder in the Agent panel before shell commands can run" + } + if str_contains(cmd, "..") { + return "denied: parent-directory traversal ('..') is not allowed" + } + if str_contains(cmd, "~") { + return "denied: home-directory references ('~') are not allowed" + } + if str_contains(cmd, "$(") || str_contains(cmd, "`") { + return "denied: command substitution is not allowed" + } + if str_starts_with(cmd, "/") && !str_starts_with(cmd, root + "/") { + return "denied: absolute paths outside the workspace are not allowed" + } + if cmd_abs_escape_at(cmd, root, " /") || cmd_abs_escape_at(cmd, root, "\"/") || cmd_abs_escape_at(cmd, root, "'/") { + return "denied: absolute paths outside the workspace are not allowed" + } + if cmd_abs_escape_at(cmd, root, "=/") || cmd_abs_escape_at(cmd, root, ">/") || cmd_abs_escape_at(cmd, root, " String { + if str_eq(tool_name, "read_file") || str_eq(tool_name, "list_files") || str_eq(tool_name, "grep") { + return "read" + } + if str_eq(tool_name, "search_memory") || str_eq(tool_name, "recall") || str_eq(tool_name, "web_get") { + return "read" + } + if str_eq(tool_name, "remember") || str_eq(tool_name, "neuron_remember") { + return "reversible" + } + if str_starts_with(tool_name, "neuron_") { + return "read" + } + if str_eq(tool_name, "write_file") || str_eq(tool_name, "edit_file") { + let root: String = agent_workspace_root() + // Unscoped writes (no workspace chosen) are not "reversible" — escalate. + if str_eq(root, "") { + return "escalate" + } + return "reversible" + } + if str_eq(tool_name, "run_command") { + let cmd: String = json_get(tool_input, "command") + let root: String = agent_workspace_root() + if !str_eq(root, "") && run_command_is_readonly(cmd) { + return "read" + } + return "escalate" + } + // Unknown tool = escalate. Default-deny, never default-allow. + return "escalate" +} + fn dispatch_tool(tool_name: String, tool_input: String) -> String { if str_eq(tool_name, "read_file") { let path: String = json_get(tool_input, "path") @@ -1557,6 +1685,10 @@ fn dispatch_tool(tool_name: String, tool_input: String) -> String { } if str_eq(tool_name, "web_get") { let url: String = json_get(tool_input, "url") + // BUG-8: scheme guard — web_get had no guard at all (file:// etc). + if !str_starts_with(url, "http://") && !str_starts_with(url, "https://") { + return json_safe("denied: only http(s) URLs can be fetched") + } let result: String = http_get(url) return json_safe(result) } @@ -1568,7 +1700,14 @@ fn dispatch_tool(tool_name: String, tool_input: String) -> String { if str_eq(tool_name, "run_command") { let cmd: String = json_get(tool_input, "command") let root: String = agent_workspace_root() - let scoped: String = if str_eq(root, "") { cmd } else { "cd " + root + " && ( " + cmd + " )" } + // BUG-8(B): the fence — refusal, not a cwd suggestion. Applies on EVERY + // execution path (auto-run in the loop AND post-consent dispatch from + // handle_session_approve), because both land here. + let denial: String = run_command_guard(cmd, root) + if !str_eq(denial, "") { + return json_safe(denial) + } + let scoped: String = "cd " + root + " && ( " + cmd + " )" let result: String = exec_capture(scoped) return json_safe(result) } @@ -1998,6 +2137,7 @@ fn agentic_loop(session_id: String, model: String, safe_sys: String, tools_json: let pend_tool_id: String = "" let pend_tool_name: String = "" let pend_tool_input: String = "" + let pend_tool_tier: String = "" while keep_going && iteration < 8 { let req_body: String = "{\"model\":\"" + model + "\"" @@ -2054,7 +2194,13 @@ fn agentic_loop(session_id: String, model: String, safe_sys: String, tools_json: let always_key: String = "always_allow_" + session_id let always_list: String = if !str_eq(session_id, "") { state_get(always_key) } else { "" } let is_always_allowed: Bool = !str_eq(tool_name, "") && !str_eq(always_list, "") && str_contains(always_list, tool_name) - let needs_bridge: Bool = is_tool_turn && !is_builtin_tool(tool_name) && !is_always_allowed + // BUG-8(A): the engine classifies every tool call and REFUSES to auto-run + // the escalate tier — being a builtin is no longer a free pass, and + // "always allow" can never bypass escalate (irreversible actions always + // confirm). Escalated calls suspend to the client's consent flow; the + // /approve round-trip is the only path that executes them. + let risk_tier: String = if is_tool_turn { classify_tool_risk(tool_name, tool_input) } else { "" } + let needs_bridge: Bool = is_tool_turn && (str_eq(risk_tier, "escalate") || (!is_builtin_tool(tool_name) && !is_always_allowed)) // Built-in tools dispatch locally; bridged tools yield "" (never sent upstream). let tool_result_raw: String = if is_tool_turn && !needs_bridge { dispatch_tool(tool_name, tool_input) } else { "" } @@ -2090,6 +2236,7 @@ fn agentic_loop(session_id: String, model: String, safe_sys: String, tools_json: let pend_tool_id = if needs_bridge { tool_id } else { pend_tool_id } let pend_tool_name = if needs_bridge { tool_name } else { pend_tool_name } let pend_tool_input = if needs_bridge { tool_input } else { pend_tool_input } + let pend_tool_tier = if needs_bridge { risk_tier } else { pend_tool_tier } // Stash messages-with-the-assistant-request so resume only needs to append the // client's tool_result block. messages_with_assistant is only meaningful when a // tool was requested, so guard on needs_bridge before persisting. @@ -2110,6 +2257,7 @@ fn agentic_loop(session_id: String, model: String, safe_sys: String, tools_json: + ",\"call_id\":\"" + pend_tool_id + "\"" + ",\"tool_name\":\"" + pend_tool_name + "\"" + ",\"tool_input\":" + safe_in + + ",\"risk_tier\":\"" + pend_tool_tier + "\"" + ",\"model\":\"" + model + "\"" + ",\"agentic\":true" + ",\"tools_used\":" + tools_arr + "}"