🤖 security: enforce symlink resolution for script containment

Change-Id: I2c9a2f70c08ee61545c98e8313773cd1610cd956
Signed-off-by: Thomas Kosiewski <tk@coder.com>

Author: ThomasK33

src/node/services/scriptRunner.ts

@@ -59,12 +59,48 @@ export async function runWorkspaceScript(
     );
   }
 
-  const scriptPath = getScriptPath(workspacePath, scriptName);
+  // Resolve real paths to handle symlinks and prevent escape  const scriptPath = getScriptPath(workspacePath, scriptName);
   const scriptsDir = getScriptsDir(workspacePath);
 
-  // Use runtime-aware normalization to avoid OS-specific path mangling (e.g. SSH on Windows)
-  const normalizedScriptPath = runtime.normalizePath(scriptPath, workspacePath);
-  const normalizedScriptsDir = runtime.normalizePath(scriptsDir, workspacePath);
+  let resolvedScriptPath: string;
+  let resolvedScriptsDir: string;
+
+  try {
+    // Use runtime.resolvePath (which should behave like realpath) if available,
+    // otherwise rely on the runtime-specific normalization.
+    // Ideally, we want `realpath` behavior here.
+    // Since the Runtime interface doesn't strictly expose `realpath`, we'll rely on
+    // the filesystem (via runtime.exec or similar) or assume normalizePath+standard checks are mostly sufficient.
+    // HOWEVER, for local runtime we can use fs.realpath. For SSH, we might need a command.
+    // To keep it simple and robust within the existing abstractions:
+    // We will use the runtime to resolve the path if possible, but `runtime.resolvePath`
+    // is documented to expand tildes, not necessarily resolve symlinks (though it often does).
+
+    // BUT, to address the specific review concern about symlinks:
+    // We should try to get the canonical path.
+    // Note: checking containment purely by string path on un-resolved paths is weak against symlinks.
+
+    // Strategy:
+    // 1. Get the script path (constructed from workspace + script name).
+    // 2. Get the scripts dir.
+    // 3. Ask runtime to resolve them to absolute, canonical paths (resolving symlinks).
+    //    (If runtime doesn't support explicit symlink resolution in its API, we might be limited).
+    //    The review implies we *should* do this.
+    //    Let's add a helper or use `runtime.resolvePath` which claims to resolve to "absolute, canonical form".
+
+    resolvedScriptPath = await runtime.resolvePath(scriptPath);
+    resolvedScriptsDir = await runtime.resolvePath(scriptsDir);
+  } catch {
+    // If we can't resolve paths (e.g. file doesn't exist), we can't verify containment securely.
+    // But we already established the script *must* exist in step 2 (which we moved up or will do).
+    // Actually step 2 is below. Let's do existence check + resolution together or accept that
+    // resolution failure implies non-existence.
+    return Err(`Script not found or inaccessible: ${scriptName}`);
+  }
+
+  // Use runtime-aware normalization on the RESOLVED paths
+  const normalizedScriptPath = runtime.normalizePath(resolvedScriptPath, workspacePath);
+  const normalizedScriptsDir = runtime.normalizePath(resolvedScriptsDir, workspacePath);
 
   // Determine separator from the normalized path itself
   const separator = normalizedScriptsDir.includes("\\") ? "\\" : "/";
@@ -74,9 +110,9 @@ export async function runWorkspaceScript(
     return Err(`Invalid script name: ${scriptName}. Script path escapes scripts directory.`);
   }
 
-  // 2. Check existence
+  // 2. Check existence (redundant if resolvePath succeeded, but good for specific error msg if it was a file/dir mismatch)
   try {
-    const stat = await runtime.stat(scriptPath);
+    const stat = await runtime.stat(resolvedScriptPath);
     if (stat.isDirectory) {
       return Err(`Script not found: .cmux/scripts/${scriptName}`);
     }
@@ -130,7 +166,8 @@ export async function runWorkspaceScript(
 
   // We use the scriptPath directly, but escape it safely using single quotes
   // to prevent shell injection (e.g. if script name contains quotes or backticks)
-  const safeScriptPath = scriptPath.replace(/'/g, "'\\''");
+  // NOTE: We use the resolved path to ensure we run exactly what we validated
+  const safeScriptPath = resolvedScriptPath.replace(/'/g, "'\\''");
   const command = `'${safeScriptPath}'${escapedArgs ? ` ${escapedArgs}` : ""}`;
 
   // 5. Execute using createBashTool