diff --git a/src/runtime/LocalRuntime.ts b/src/runtime/LocalRuntime.ts index 4b01216e8..95818b758 100644 --- a/src/runtime/LocalRuntime.ts +++ b/src/runtime/LocalRuntime.ts @@ -303,6 +303,16 @@ export class LocalRuntime implements Runtime { } } + normalizePath(targetPath: string, basePath: string): string { + // For local runtime, use Node.js path resolution + // Handle special case: current directory + const target = targetPath.trim(); + if (target === ".") { + return path.resolve(basePath); + } + return path.resolve(basePath, target); + } + getWorkspacePath(projectPath: string, workspaceName: string): string { const projectName = getProjectName(projectPath); return path.join(this.srcBaseDir, projectName, workspaceName); diff --git a/src/runtime/Runtime.ts b/src/runtime/Runtime.ts index 0bfd1af25..28f8fd9b4 100644 --- a/src/runtime/Runtime.ts +++ b/src/runtime/Runtime.ts @@ -195,6 +195,25 @@ export interface Runtime { */ stat(path: string): Promise; + /** + * Normalize a path for comparison purposes within this runtime's context. + * Handles runtime-specific path semantics (local vs remote). + * + * @param targetPath Path to normalize (may be relative or absolute) + * @param basePath Base path to resolve relative paths against + * @returns Normalized path suitable for string comparison + * + * @example + * // LocalRuntime + * runtime.normalizePath(".", "/home/user") // => "/home/user" + * runtime.normalizePath("../other", "/home/user/project") // => "/home/user/other" + * + * // SSHRuntime + * runtime.normalizePath(".", "/home/user") // => "/home/user" + * runtime.normalizePath("~/project", "~") // => "~/project" + */ + normalizePath(targetPath: string, basePath: string): string; + /** * Compute absolute workspace path from project and workspace name. * This is the SINGLE source of truth for workspace path computation. diff --git a/src/runtime/SSHRuntime.ts b/src/runtime/SSHRuntime.ts index 7c19f3d27..4b86d916b 100644 --- a/src/runtime/SSHRuntime.ts +++ b/src/runtime/SSHRuntime.ts @@ -317,6 +317,41 @@ export class SSHRuntime implements Runtime { }; } + normalizePath(targetPath: string, basePath: string): string { + // For SSH, handle paths in a POSIX-like manner without accessing the remote filesystem + const target = targetPath.trim(); + let base = basePath.trim(); + + // Normalize base path - remove trailing slash (except for root "/") + if (base.length > 1 && base.endsWith("/")) { + base = base.slice(0, -1); + } + + // Handle special case: current directory + if (target === ".") { + return base; + } + + // Handle tilde expansion - keep as-is for comparison + let normalizedTarget = target; + if (target === "~" || target.startsWith("~/")) { + normalizedTarget = target; + } else if (target.startsWith("/")) { + // Absolute path - use as-is + normalizedTarget = target; + } else { + // Relative path - resolve against base using POSIX path joining + normalizedTarget = base.endsWith("/") ? base + target : base + "/" + target; + } + + // Remove trailing slash for comparison (except for root "/") + if (normalizedTarget.length > 1 && normalizedTarget.endsWith("/")) { + normalizedTarget = normalizedTarget.slice(0, -1); + } + + return normalizedTarget; + } + /** * Build common SSH arguments based on runtime config * @param includeHost - Whether to include the host in the args (for direct ssh commands) diff --git a/src/services/tools/bash.test.ts b/src/services/tools/bash.test.ts index fc58458e6..0542280b2 100644 --- a/src/services/tools/bash.test.ts +++ b/src/services/tools/bash.test.ts @@ -1235,3 +1235,138 @@ fi expect(remainingProcesses).toBe(0); }); }); + +describe("SSH runtime redundant cd detection", () => { + // Helper to create bash tool with SSH runtime configuration + // Note: These tests check redundant cd detection logic only - they don't actually execute via SSH + function createTestBashToolWithSSH(cwd: string) { + const tempDir = new TestTempDir("test-bash-ssh"); + const sshRuntime = createRuntime({ + type: "ssh", + host: "test-host", + srcBaseDir: "/remote/base", + }); + + const tool = createBashTool({ + cwd, + runtime: sshRuntime, + tempDir: tempDir.path, + }); + + return { + tool, + [Symbol.dispose]() { + tempDir[Symbol.dispose](); + }, + }; + } + + it("should reject redundant cd to absolute path on SSH runtime", async () => { + const remoteCwd = "/home/user/project"; + using testEnv = createTestBashToolWithSSH(remoteCwd); + const tool = testEnv.tool; + + const args: BashToolArgs = { + script: `cd ${remoteCwd} && echo test`, + timeout_secs: 5, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Redundant cd"); + expect(result.error).toContain("already runs in"); + } + }); + + it("should reject redundant cd with relative path (.) on SSH runtime", async () => { + const remoteCwd = "/home/user/project"; + using testEnv = createTestBashToolWithSSH(remoteCwd); + const tool = testEnv.tool; + + const args: BashToolArgs = { + script: "cd . && echo test", + timeout_secs: 5, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Redundant cd"); + } + }); + + it("should reject redundant cd with tilde path on SSH runtime", async () => { + const remoteCwd = "~/project"; + using testEnv = createTestBashToolWithSSH(remoteCwd); + const tool = testEnv.tool; + + const args: BashToolArgs = { + script: "cd ~/project && echo test", + timeout_secs: 5, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Redundant cd"); + } + }); + + it("should reject redundant cd with single tilde on SSH runtime", async () => { + const remoteCwd = "~"; + using testEnv = createTestBashToolWithSSH(remoteCwd); + const tool = testEnv.tool; + + const args: BashToolArgs = { + script: "cd ~ && echo test", + timeout_secs: 5, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Redundant cd"); + } + }); + + it("should handle trailing slashes in path comparison on SSH runtime", async () => { + const remoteCwd = "/home/user/project"; + using testEnv = createTestBashToolWithSSH(remoteCwd); + const tool = testEnv.tool; + + const args: BashToolArgs = { + script: "cd /home/user/project/ && echo test", + timeout_secs: 5, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Redundant cd"); + } + }); + + it("should handle cwd with trailing slash on SSH runtime", async () => { + const remoteCwd = "/home/user/project/"; + using testEnv = createTestBashToolWithSSH(remoteCwd); + const tool = testEnv.tool; + + const args: BashToolArgs = { + script: "cd /home/user/project && echo test", + timeout_secs: 5, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Redundant cd"); + } + }); +}); diff --git a/src/services/tools/bash.ts b/src/services/tools/bash.ts index 23dbcabfa..8def72db6 100644 --- a/src/services/tools/bash.ts +++ b/src/services/tools/bash.ts @@ -77,17 +77,15 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { let fileTruncated = false; // Hit 100KB file limit // Detect redundant cd to working directory - // Note: config.cwd is the actual execution path (local for LocalRuntime, remote for SSHRuntime) - // Match patterns like: "cd /path &&", "cd /path;", "cd '/path' &&", "cd "/path" &&" + // Delegate path normalization to the runtime for proper handling of local vs remote paths const cdPattern = /^\s*cd\s+['"]?([^'";\\&|]+)['"]?\s*[;&|]/; const match = cdPattern.exec(script); if (match) { const targetPath = match[1].trim(); - // For SSH runtime, config.cwd might use $HOME - need to handle this - // Normalize paths for comparison (resolve to absolute where possible) - // Note: This check is best-effort - it won't catch all cases on SSH (e.g., ~/path vs $HOME/path) - const normalizedTarget = path.resolve(config.cwd, targetPath); - const normalizedCwd = path.resolve(config.cwd); + + // Use runtime's normalizePath method to handle path comparison correctly + const normalizedTarget = config.runtime.normalizePath(targetPath, config.cwd); + const normalizedCwd = config.runtime.normalizePath(".", config.cwd); if (normalizedTarget === normalizedCwd) { return {