Prevent writing outside of the workspace roots (#6178)

Co-authored-by: Jacob Richman <[email protected]>

1 files changed, 51 insertions, 68 deletions

diff --git a/packages/core/src/utils/workspaceContext.ts b/packages/core/src/utils/workspaceContext.ts
index efbc8a4c..be99a83c 100644
--- a/packages/core/src/utils/workspaceContext.ts
+++ b/packages/core/src/utils/workspaceContext.ts
@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { isNodeError } from '../utils/errors.js';
 import * as fs from 'fs';
 import * as path from 'path';
 
@@ -13,26 +14,21 @@ import * as path from 'path';
  * in a single session.
  */
 export class WorkspaceContext {
-  private directories: Set<string>;
-
+  private directories = new Set<string>();
   private initialDirectories: Set<string>;
 
   /**
    * Creates a new WorkspaceContext with the given initial directory and optional additional directories.
-   * @param initialDirectory The initial working directory (usually cwd)
+   * @param directory The initial working directory (usually cwd)
    * @param additionalDirectories Optional array of additional directories to include
    */
-  constructor(initialDirectory: string, additionalDirectories: string[] = []) {
-    this.directories = new Set<string>();
-    this.initialDirectories = new Set<string>();
-
-    this.addDirectoryInternal(initialDirectory);
-    this.addInitialDirectoryInternal(initialDirectory);
-
-    for (const dir of additionalDirectories) {
-      this.addDirectoryInternal(dir);
-      this.addInitialDirectoryInternal(dir);
+  constructor(directory: string, additionalDirectories: string[] = []) {
+    this.addDirectory(directory);
+    for (const additionalDirectory of additionalDirectories) {
+      this.addDirectory(additionalDirectory);
     }
+
+    this.initialDirectories = new Set(this.directories);
   }
 
   /**
@@ -41,16 +37,13 @@ export class WorkspaceContext {
    * @param basePath Optional base path for resolving relative paths (defaults to cwd)
    */
   addDirectory(directory: string, basePath: string = process.cwd()): void {
-    this.addDirectoryInternal(directory, basePath);
+    this.directories.add(this.resolveAndValidateDir(directory, basePath));
   }
 
-  /**
-   * Internal method to add a directory with validation.
-   */
-  private addDirectoryInternal(
+  private resolveAndValidateDir(
     directory: string,
     basePath: string = process.cwd(),
-  ): void {
+  ): string {
     const absolutePath = path.isAbsolute(directory)
       ? directory
       : path.resolve(basePath, directory);
@@ -58,47 +51,12 @@ export class WorkspaceContext {
     if (!fs.existsSync(absolutePath)) {
       throw new Error(`Directory does not exist: ${absolutePath}`);
     }
-
     const stats = fs.statSync(absolutePath);
     if (!stats.isDirectory()) {
       throw new Error(`Path is not a directory: ${absolutePath}`);
     }
 
-    let realPath: string;
-    try {
-      realPath = fs.realpathSync(absolutePath);
-    } catch (_error) {
-      throw new Error(`Failed to resolve path: ${absolutePath}`);
-    }
-
-    this.directories.add(realPath);
-  }
-
-  private addInitialDirectoryInternal(
-    directory: string,
-    basePath: string = process.cwd(),
-  ): void {
-    const absolutePath = path.isAbsolute(directory)
-      ? directory
-      : path.resolve(basePath, directory);
-
-    if (!fs.existsSync(absolutePath)) {
-      throw new Error(`Directory does not exist: ${absolutePath}`);
-    }
-
-    const stats = fs.statSync(absolutePath);
-    if (!stats.isDirectory()) {
-      throw new Error(`Path is not a directory: ${absolutePath}`);
-    }
-
-    let realPath: string;
-    try {
-      realPath = fs.realpathSync(absolutePath);
-    } catch (_error) {
-      throw new Error(`Failed to resolve path: ${absolutePath}`);
-    }
-
-    this.initialDirectories.add(realPath);
+    return fs.realpathSync(absolutePath);
   }
 
   /**
@@ -116,7 +74,7 @@ export class WorkspaceContext {
   setDirectories(directories: readonly string[]): void {
     this.directories.clear();
     for (const dir of directories) {
-      this.addDirectoryInternal(dir);
+      this.addDirectory(dir);
     }
   }
 
@@ -127,23 +85,13 @@ export class WorkspaceContext {
    */
   isPathWithinWorkspace(pathToCheck: string): boolean {
     try {
-      const absolutePath = path.resolve(pathToCheck);
-
-      let resolvedPath = absolutePath;
-      if (fs.existsSync(absolutePath)) {
-        try {
-          resolvedPath = fs.realpathSync(absolutePath);
-        } catch (_error) {
-          return false;
-        }
-      }
+      const fullyResolvedPath = this.fullyResolvedPath(pathToCheck);
 
       for (const dir of this.directories) {
-        if (this.isPathWithinRoot(resolvedPath, dir)) {
+        if (this.isPathWithinRoot(fullyResolvedPath, dir)) {
           return true;
         }
       }
-
       return false;
     } catch (_error) {
       return false;
@@ -151,6 +99,30 @@ export class WorkspaceContext {
   }
 
   /**
+   * Fully resolves a path, including symbolic links.
+   * If the path does not exist, it returns the fully resolved path as it would be
+   * if it did exist.
+   */
+  private fullyResolvedPath(pathToCheck: string): string {
+    try {
+      return fs.realpathSync(pathToCheck);
+    } catch (e: unknown) {
+      if (
+        isNodeError(e) &&
+        e.code === 'ENOENT' &&
+        e.path &&
+        // realpathSync does not set e.path correctly for symlinks to
+        // non-existent files.
+        !this.isFileSymlink(e.path)
+      ) {
+        // If it doesn't exist, e.path contains the fully resolved path.
+        return e.path;
+      }
+      throw e;
+    }
+  }
+
+  /**
    * Checks if a path is within a given root directory.
    * @param pathToCheck The absolute path to check
    * @param rootDirectory The absolute root directory
@@ -167,4 +139,15 @@ export class WorkspaceContext {
       !path.isAbsolute(relative)
     );
   }
+
+  /**
+   * Checks if a file path is a symbolic link that points to a file.
+   */
+  private isFileSymlink(filePath: string): boolean {
+    try {
+      return !fs.readlinkSync(filePath).endsWith('/');
+    } catch (_error) {
+      return false;
+    }
+  }
 }