diff --git a/src/agent/tools/files.ts b/src/agent/tools/files.ts index f431186..678fd47 100644 --- a/src/agent/tools/files.ts +++ b/src/agent/tools/files.ts @@ -27,7 +27,12 @@ const IMAGE_MIME: Record = { // ---- Read ---- export const readFileTool = defineTool("Read", false, async (input) => { if (typeof input.path !== "string" || !input.path) return { output: "error: path is required and must be a string" }; - const p = safePath(input.path); + let p: string; + try { + p = safePath(input.path); + } catch (e) { + return { output: `error: invalid path: ${e instanceof Error ? e.message : String(e)}` }; + } const ext = path.extname(p).toLowerCase(); // Image files: return a base64 image block so it reaches the model. @@ -83,7 +88,12 @@ export const readFileTool = defineTool("Read", false, async (input) => { // ---- ListDir ---- export const listDirTool = defineTool("ListDir", false, async (input) => { try { - const p = safePath(input.path ?? "."); + let p: string; + try { + p = safePath(input.path ?? "."); + } catch (e) { + return { output: `error: invalid path: ${e instanceof Error ? e.message : String(e)}` }; + } const entries = await fs.readdir(p, { withFileTypes: true }); const out = entries @@ -100,9 +110,14 @@ export const listDirTool = defineTool("ListDir", false, async (input) => { // ---- Glob ---- export const globTool = defineTool("Glob", false, async (input, abortSignal) => { try { - const root = input.target_directory ? safePath(input.target_directory) : getWorkspaceRoot(); + let root: string; + try { + root = input.target_directory ? safePath(input.target_directory) : getWorkspaceRoot(); + } catch (e) { + return { output: `error: invalid target_directory: ${e instanceof Error ? e.message : String(e)}` }; + } // Only walk ignored dirs when the pattern explicitly targets them - // (e.g. "**/node_modules/**") — otherwise node_modules hangs the tool. + // (e.g. "**/node_modules/**") - otherwise node_modules hangs the tool. let pattern: string = String(input.glob_pattern ?? ""); if (pattern && !pattern.startsWith("**/")) pattern = "**/" + pattern; const wantsIgnored = /node_modules|\.git|[/\\]dist[/\\]|[/\\]out[/\\]|[/\\]build[/\\]/.test(pattern); @@ -171,7 +186,12 @@ function blockedInMultitask(ctx?: ToolContext): boolean { const editExecute: Tool["execute"] = async (input, _signal, _callId, ctx) => { if (blockedInMultitask(ctx)) return MULTITASK_BLOCK; if (typeof input.path !== "string" || !input.path) return { output: "error: path is required and must be a string" }; - const p = safePath(input.path); + let p: string; + try { + p = safePath(input.path); + } catch (e) { + return { output: `error: invalid path: ${e instanceof Error ? e.message : String(e)}` }; + } let existedBefore = false; try { await fs.access(p); @@ -250,7 +270,12 @@ export const writeTool = defineTool("Write", true, editExecute); export const deleteFileTool = defineTool("Delete", true, async (input, _signal, _callId, ctx) => { if (blockedInMultitask(ctx)) return MULTITASK_BLOCK; if (typeof input.path !== "string" || !input.path) return { output: "error: path is required and must be a string" }; - const p = safePath(input.path); + let p: string; + try { + p = safePath(input.path); + } catch (e) { + return { output: `error: invalid path: ${e instanceof Error ? e.message : String(e)}` }; + } let before = ""; try { before = await fs.readFile(p, "utf8"); @@ -326,7 +351,12 @@ export const editNotebookTool = defineTool("EditNotebook", true, async (input, _ const oldString = String(input?.old_string ?? ""); const newString = String(input?.new_string ?? ""); - const abs = safePath(target); + let abs: string; + try { + abs = safePath(target); + } catch (e) { + return { output: `error: invalid path: ${e instanceof Error ? e.message : String(e)}` }; + } // Read (or scaffold) the notebook JSON. let nb: any; diff --git a/src/agent/tools/search.ts b/src/agent/tools/search.ts index 635da0e..cdf4a80 100644 --- a/src/agent/tools/search.ts +++ b/src/agent/tools/search.ts @@ -43,7 +43,14 @@ export const grepTool = defineTool("Grep", false, async (input, abortSignal) => if (abortSignal?.aborted) return { output: "(grep aborted)" }; const root = getWorkspaceRoot(); const mode: string = input.output_mode || "content"; - const target = input.path ? safePath(input.path) : "."; + let target = "."; + if (input.path) { + try { + target = safePath(input.path); // spawn arg — spaces OK without shell quoting + } catch (e) { + return { output: `error: invalid path: ${e instanceof Error ? e.message : String(e)}` }; + } + } const cap = Math.max(1, Math.min(Number(input.head_limit) || 200, 2000)); const skip = Math.max(0, Number(input.offset) || 0); const pattern = String(input.pattern ?? ""); @@ -119,7 +126,14 @@ export const grepTool = defineTool("Grep", false, async (input, abortSignal) => } // Node fallback (no ripgrep available). Honor path/glob/type/-A/-B/-C/multiline. - const scopeRoot = input.path ? safePath(input.path) : root; + let scopeRoot = root; + if (input.path) { + try { + scopeRoot = safePath(input.path); + } catch (e) { + return { output: `error: invalid path: ${e instanceof Error ? e.message : String(e)}` }; + } + } const all: string[] = []; await walk(scopeRoot, all, 0, false, abortSignal, 15_000); if (abortSignal?.aborted) return { output: "(grep aborted)" }; @@ -235,7 +249,13 @@ export const semanticSearchTool = defineTool("SemanticSearch", false, async (inp // Scope by target_directories (prefix match on workspace-relative paths). const dirs: string[] = Array.isArray(input.target_directories) ? input.target_directories : []; const prefixes = dirs - .map((d) => path.relative(root, safePath(String(d))).split(path.sep).join("/")) + .map((d) => { + try { + return path.relative(root, safePath(String(d))).split(path.sep).join("/"); + } catch { + return ""; + } + }) .filter((p) => p && !p.startsWith("..")); const filter = prefixes.length ? (rel: string) => prefixes.some((p) => rel === p || rel.startsWith(p + "/")) diff --git a/src/agent/tools/shell.ts b/src/agent/tools/shell.ts index 82c8e9d..d281151 100644 --- a/src/agent/tools/shell.ts +++ b/src/agent/tools/shell.ts @@ -47,33 +47,64 @@ function buildNotify(input: any, ctx: any): ShellNotify | undefined { }; } +/** Quote a filesystem path for the session shell (spaces, quotes, unicode). */ +function quotePath(p: string): string { + if (isWin) { + // PowerShell single-quoted literal; escape ' by doubling. Drop trailing + // backslash that would escape the closing quote if we ever used doubles. + return `'${p.replace(/'/g, "''")}'`; + } + // bash: single-quote with '\'' for embedded quotes + return `'${p.replace(/'/g, `'\\''`)}'`; +} + /** - * Frame a command so the session always prints a unique sentinel with exit code. - * PowerShell: use `if ($?)` — `$LASTEXITCODE` is often $null for native/cmdlets - * and would never emit a sentinel (classic hang). + * Frame a command so the session ALWAYS prints a sentinel with exit code. + * + * Critical: do NOT paste the user command raw into the script. Paths with + * spaces, unclosed quotes, or bad syntax leave PowerShell waiting for more + * input forever (no sentinel → tool looks "stuck"). Instead base64-encode the + * command and Invoke-Expression / bash -c it inside try/catch/finally so + * parse errors still emit the sentinel and free the session. */ function wrapCommand(command: string, cd: string): string { + const b64 = Buffer.from(command, "utf8").toString("base64"); if (isWin) { - const loc = cd - ? `Push-Location -LiteralPath '${cd.replace(/'/g, "''")}'; try { ` - : ""; - const endLoc = cd ? ` } finally { Pop-Location } ` : " "; - // Always write the sentinel, even if the command throws. - // Use [Console]::Out so the line is not stuck in PowerShell's output pipeline. + const cdBlock = cd + ? `Push-Location -LiteralPath ${quotePath(cd)}; $__oc_pop = $true; ` + : `$__oc_pop = $false; `; + // Decode → Invoke-Expression inside try; sentinel always in finally. + // $? / $LASTEXITCODE after IEX covers native cmds and cmdlets. return ( - `${loc}${command}${endLoc}\n` + - `if ($?) { [Console]::Out.WriteLine("${SENTINEL}:0") } else { ` + - `$__oc = if ($null -ne $LASTEXITCODE -and "$LASTEXITCODE" -ne "") { $LASTEXITCODE } else { 1 }; ` + - `[Console]::Out.WriteLine("${SENTINEL}:$__oc") }\n` + `${cdBlock}` + + `$__oc_ok = $false; $__oc_code = 1; ` + + `try { ` + + `$__oc_cmd = [System.Text.Encoding]::UTF8.GetString([System.Convert]::FromBase64String('${b64}')); ` + + `Invoke-Expression -Command $__oc_cmd; ` + + `if ($?) { $__oc_ok = $true; $__oc_code = 0 } ` + + `elseif ($null -ne $LASTEXITCODE -and "$LASTEXITCODE" -ne '') { $__oc_code = [int]$LASTEXITCODE } ` + + `else { $__oc_code = 1 } ` + + `} catch { ` + + `[Console]::Error.WriteLine($_.Exception.Message); $__oc_ok = $false; $__oc_code = 1 ` + + `} finally { ` + + `if ($__oc_pop) { Pop-Location -ErrorAction SilentlyContinue }; ` + + `if ($__oc_ok) { [Console]::Out.WriteLine("${SENTINEL}:0") } ` + + `else { [Console]::Out.WriteLine("${SENTINEL}:$__oc_code") } ` + + `}\n` ); } - const push = cd ? `pushd '${cd.replace(/'/g, `'\\''`)}' >/dev/null 2>&1 || true\n` : ""; + // bash: decode to a temp eval so spaces/quotes never break the outer script. + // Always print sentinel even if eval fails (set +e). + const push = cd ? `pushd ${quotePath(cd)} >/dev/null 2>&1 || true\n` : ""; const pop = cd ? `popd >/dev/null 2>&1 || true\n` : ""; return ( - `${push}${command}\n` + + `set +e\n` + + `${push}` + + `__oc_cmd=$(printf '%s' '${b64}' | base64 -d 2>/dev/null || printf '%s' '${b64}' | base64 -D 2>/dev/null)\n` + + `eval "$__oc_cmd"\n` + `__oc_rc=$?\n` + `${pop}` + - `echo "${SENTINEL}:$__oc_rc"\n` + `printf '%s\\n' "${SENTINEL}:$__oc_rc"\n` ); } @@ -171,7 +202,20 @@ export const runTerminalTool = defineTool("Shell", true, async (input, abortSign }; abortSignal?.addEventListener("abort", onAbort); - const cd = input.working_directory ? safePath(input.working_directory) : ""; + let cd = ""; + if (input.working_directory) { + try { + cd = safePath(String(input.working_directory)); + } catch (e) { + abortSignal?.removeEventListener("abort", onAbort); + sh.done = true; + sh.exitCode = 1; + releaseQueue(); + return { + output: `error: invalid working_directory: ${e instanceof Error ? e.message : String(e)}`, + }; + } + } const wrapped = wrapCommand(command, cd); try { diff --git a/src/context/workspaceUtils.ts b/src/context/workspaceUtils.ts index 406b1d0..0af9f39 100644 --- a/src/context/workspaceUtils.ts +++ b/src/context/workspaceUtils.ts @@ -35,12 +35,30 @@ export function getRecentFiles(): string[] { return out; } +/** + * Resolve a workspace path safely. Handles spaces, unicode, and mixed + * separators. Does not shell-quote — callers that inject into a shell must + * quote the result (see shell.ts quotePath). + */ export function safePath(rel: string): string { const root = getWorkspaceRoot(); - const abs = path.isAbsolute(rel) ? rel : path.join(root, rel); + // Normalize user input: trim, unify slashes, drop surrounding quotes the + // model sometimes wraps around paths that contain spaces. + let s = String(rel ?? "").trim(); + if ( + (s.startsWith('"') && s.endsWith('"')) || + (s.startsWith("'") && s.endsWith("'")) + ) { + s = s.slice(1, -1); + } + s = s.replace(/\//g, path.sep); + const abs = path.isAbsolute(s) ? s : path.join(root, s); const norm = path.resolve(abs); const ws = path.resolve(root); - if (norm !== ws && !norm.startsWith(ws + path.sep)) { + // Case-insensitive root check on Windows (C:\ vs c:\). + const normKey = process.platform === "win32" ? norm.toLowerCase() : norm; + const wsKey = process.platform === "win32" ? ws.toLowerCase() : ws; + if (normKey !== wsKey && !normKey.startsWith(wsKey + path.sep)) { throw new Error(`path outside workspace: ${rel}`); } return norm;