feat: implement secure file system access and path validation

- Introduced a restricted file system wrapper to ensure all file operations are confined to the script's directory, enhancing security. - Updated various modules to utilize the new secure file system methods, replacing direct fs calls with validated operations. - Enhanced path validation in the server routes and context loaders to prevent unauthorized access to the file system. - Adjusted environment variable handling to use centralized methods for reading and writing API keys, ensuring consistent security practices. This change improves the overall security posture of the application by enforcing strict file access controls and validating paths before any operations are performed.
2026-02-01 08:13:37 +00:00 · 2025-12-31 18:03:01 -05:00
parent 847a8ff327
commit 2b89b0606c
21 changed files with 1577 additions and 2198 deletions
--- a/apps/server/src/lib/auth.ts
+++ b/apps/server/src/lib/auth.ts
@@ -10,8 +10,8 @@

 import type { Request, Response, NextFunction } from 'express';
 import crypto from 'crypto';
-import fs from 'fs';
 import path from 'path';
+import * as secureFs from './secure-fs.js';

 const DATA_DIR = process.env.DATA_DIR || './data';
 const API_KEY_FILE = path.join(DATA_DIR, '.api-key');
@@ -41,8 +41,8 @@ setInterval(() => {
 */
 function loadSessions(): void {
  try {
-    if (fs.existsSync(SESSIONS_FILE)) {
-      const data = fs.readFileSync(SESSIONS_FILE, 'utf-8');
+    if (secureFs.existsSync(SESSIONS_FILE)) {
+      const data = secureFs.readFileSync(SESSIONS_FILE, 'utf-8') as string;
      const sessions = JSON.parse(data) as Array<
        [string, { createdAt: number; expiresAt: number }]
      >;
@@ -74,12 +74,9 @@ function loadSessions(): void {
 */
 async function saveSessions(): Promise<void> {
  try {
-    await fs.promises.mkdir(path.dirname(SESSIONS_FILE), { recursive: true });
+    await secureFs.mkdir(path.dirname(SESSIONS_FILE), { recursive: true });
    const sessions = Array.from(validSessions.entries());
-    await fs.promises.writeFile(SESSIONS_FILE, JSON.stringify(sessions), {
-      encoding: 'utf-8',
-      mode: 0o600,
-    });
+    await secureFs.writeFile(SESSIONS_FILE, JSON.stringify(sessions), 'utf-8');
  } catch (error) {
    console.error('[Auth] Failed to save sessions:', error);
  }
@@ -101,8 +98,8 @@ function ensureApiKey(): string {

  // Try to read from file
  try {
-    if (fs.existsSync(API_KEY_FILE)) {
-      const key = fs.readFileSync(API_KEY_FILE, 'utf-8').trim();
+    if (secureFs.existsSync(API_KEY_FILE)) {
+      const key = (secureFs.readFileSync(API_KEY_FILE, 'utf-8') as string).trim();
      if (key) {
        console.log('[Auth] Loaded API key from file');
        return key;
@@ -115,8 +112,8 @@ function ensureApiKey(): string {
  // Generate new key
  const newKey = crypto.randomUUID();
  try {
-    fs.mkdirSync(path.dirname(API_KEY_FILE), { recursive: true });
-    fs.writeFileSync(API_KEY_FILE, newKey, { encoding: 'utf-8', mode: 0o600 });
+    secureFs.mkdirSync(path.dirname(API_KEY_FILE), { recursive: true });
+    secureFs.writeFileSync(API_KEY_FILE, newKey, { encoding: 'utf-8' });
    console.log('[Auth] Generated new API key');
  } catch (error) {
    console.error('[Auth] Failed to save API key:', error);
--- a/apps/server/src/lib/secure-fs.ts
+++ b/apps/server/src/lib/secure-fs.ts
@@ -6,6 +6,7 @@
 import { secureFs } from '@automaker/platform';

 export const {
+  // Async methods
  access,
  readFile,
  writeFile,
@@ -20,6 +21,16 @@ export const {
  lstat,
  joinPath,
  resolvePath,
+  // Sync methods
+  existsSync,
+  readFileSync,
+  writeFileSync,
+  mkdirSync,
+  readdirSync,
+  statSync,
+  accessSync,
+  unlinkSync,
+  rmSync,
  // Throttling configuration and monitoring
  configureThrottling,
  getThrottlingConfig,
--- a/apps/server/src/routes/context/routes/describe-image.ts
+++ b/apps/server/src/routes/context/routes/describe-image.ts
@@ -15,7 +15,7 @@ import { query } from '@anthropic-ai/claude-agent-sdk';
 import { createLogger, readImageAsBase64 } from '@automaker/utils';
 import { CLAUDE_MODEL_MAP } from '@automaker/types';
 import { createCustomOptions } from '../../../lib/sdk-options.js';
-import * as fs from 'fs';
+import * as secureFs from '../../../lib/secure-fs.js';
 import * as path from 'path';
 import type { SettingsService } from '../../../services/settings-service.js';
 import { getAutoLoadClaudeMdSetting } from '../../../lib/settings-helpers.js';
@@ -57,13 +57,13 @@ function filterSafeHeaders(headers: Record<string, unknown>): Record<string, unk
 */
 function findActualFilePath(requestedPath: string): string | null {
  // First, try the exact path
-  if (fs.existsSync(requestedPath)) {
+  if (secureFs.existsSync(requestedPath)) {
    return requestedPath;
  }

  // Try with Unicode normalization
  const normalizedPath = requestedPath.normalize('NFC');
-  if (fs.existsSync(normalizedPath)) {
+  if (secureFs.existsSync(normalizedPath)) {
    return normalizedPath;
  }

@@ -72,12 +72,12 @@ function findActualFilePath(requestedPath: string): string | null {
  const dir = path.dirname(requestedPath);
  const baseName = path.basename(requestedPath);

-  if (!fs.existsSync(dir)) {
+  if (!secureFs.existsSync(dir)) {
    return null;
  }

  try {
-    const files = fs.readdirSync(dir);
+    const files = secureFs.readdirSync(dir);

    // Normalize the requested basename for comparison
    // Replace various space-like characters with regular space for comparison
@@ -281,9 +281,9 @@ export function createDescribeImageHandler(
      }

      // Log path + stats (this is often where issues start: missing file, perms, size)
-      let stat: fs.Stats | null = null;
+      let stat: ReturnType<typeof secureFs.statSync> | null = null;
      try {
-        stat = fs.statSync(actualPath);
+        stat = secureFs.statSync(actualPath);
        logger.info(
          `[${requestId}] fileStats size=${stat.size} bytes mtime=${stat.mtime.toISOString()}`
        );
--- a/apps/server/src/routes/fs/routes/browse.ts
+++ b/apps/server/src/routes/fs/routes/browse.ts
@@ -6,7 +6,7 @@ import type { Request, Response } from 'express';
 import * as secureFs from '../../../lib/secure-fs.js';
 import os from 'os';
 import path from 'path';
-import { getAllowedRootDirectory, PathNotAllowedError } from '@automaker/platform';
+import { getAllowedRootDirectory, PathNotAllowedError, isPathAllowed } from '@automaker/platform';
 import { getErrorMessage, logError } from '../common.js';

 export function createBrowseHandler() {
@@ -40,9 +40,16 @@ export function createBrowseHandler() {
        return drives;
      };

-      // Get parent directory
+      // Get parent directory - only if it's within the allowed root
      const parentPath = path.dirname(targetPath);
-      const hasParent = parentPath !== targetPath;
+
+      // Determine if parent navigation should be allowed:
+      // 1. Must have a different parent (not at filesystem root)
+      // 2. If ALLOWED_ROOT_DIRECTORY is set, parent must be within it
+      const hasParent = parentPath !== targetPath && isPathAllowed(parentPath);
+
+      // Security: Don't expose parent path outside allowed root
+      const safeParentPath = hasParent ? parentPath : null;

      // Get available drives
      const drives = await detectDrives();
@@ -70,7 +77,7 @@ export function createBrowseHandler() {
        res.json({
          success: true,
          currentPath: targetPath,
-          parentPath: hasParent ? parentPath : null,
+          parentPath: safeParentPath,
          directories,
          drives,
        });
@@ -84,7 +91,7 @@ export function createBrowseHandler() {
          res.json({
            success: true,
            currentPath: targetPath,
-            parentPath: hasParent ? parentPath : null,
+            parentPath: safeParentPath,
            directories: [],
            drives,
            warning:
--- a/apps/server/src/routes/fs/routes/validate-path.ts
+++ b/apps/server/src/routes/fs/routes/validate-path.ts
@@ -5,7 +5,7 @@
 import type { Request, Response } from 'express';
 import * as secureFs from '../../../lib/secure-fs.js';
 import path from 'path';
-import { isPathAllowed } from '@automaker/platform';
+import { isPathAllowed, PathNotAllowedError, getAllowedRootDirectory } from '@automaker/platform';
 import { getErrorMessage, logError } from '../common.js';

 export function createValidatePathHandler() {
@@ -20,6 +20,20 @@ export function createValidatePathHandler() {

      const resolvedPath = path.resolve(filePath);

+      // Validate path against ALLOWED_ROOT_DIRECTORY before checking if it exists
+      if (!isPathAllowed(resolvedPath)) {
+        const allowedRoot = getAllowedRootDirectory();
+        const errorMessage = allowedRoot
+          ? `Path not allowed: ${filePath}. Must be within ALLOWED_ROOT_DIRECTORY: ${allowedRoot}`
+          : `Path not allowed: ${filePath}`;
+        res.status(403).json({
+          success: false,
+          error: errorMessage,
+          isAllowed: false,
+        });
+        return;
+      }
+
      // Check if path exists
      try {
        const stats = await secureFs.stat(resolvedPath);
@@ -32,7 +46,7 @@ export function createValidatePathHandler() {
        res.json({
          success: true,
          path: resolvedPath,
-          isAllowed: isPathAllowed(resolvedPath),
+          isAllowed: true,
        });
      } catch {
        res.status(400).json({ success: false, error: 'Path does not exist' });
--- a/apps/server/src/routes/setup/common.ts
+++ b/apps/server/src/routes/setup/common.ts
@@ -4,7 +4,7 @@

 import { createLogger } from '@automaker/utils';
 import path from 'path';
-import fs from 'fs/promises';
+import { secureFs } from '@automaker/platform';
 import { getErrorMessage as getErrorMessageShared, createLogError } from '../common.js';

 const logger = createLogger('Setup');
@@ -35,36 +35,13 @@ export function getAllApiKeys(): Record<string, string> {

 /**
 * Helper to persist API keys to .env file
+ * Uses centralized secureFs.writeEnvKey for path validation
 */
 export async function persistApiKeyToEnv(key: string, value: string): Promise<void> {
  const envPath = path.join(process.cwd(), '.env');

  try {
-    let envContent = '';
-    try {
-      envContent = await fs.readFile(envPath, 'utf-8');
-    } catch {
-      // .env file doesn't exist, we'll create it
-    }
-
-    // Parse existing env content
-    const lines = envContent.split('\n');
-    const keyRegex = new RegExp(`^${key}=`);
-    let found = false;
-    const newLines = lines.map((line) => {
-      if (keyRegex.test(line)) {
-        found = true;
-        return `${key}=${value}`;
-      }
-      return line;
-    });
-
-    if (!found) {
-      // Add the key at the end
-      newLines.push(`${key}=${value}`);
-    }
-
-    await fs.writeFile(envPath, newLines.join('\n'));
+    await secureFs.writeEnvKey(envPath, key, value);
    logger.info(`[Setup] Persisted ${key} to .env file`);
  } catch (error) {
    logger.error(`[Setup] Failed to persist ${key} to .env:`, error);
--- a/apps/server/src/routes/setup/get-claude-status.ts
+++ b/apps/server/src/routes/setup/get-claude-status.ts
@@ -4,9 +4,7 @@

 import { exec } from 'child_process';
 import { promisify } from 'util';
-import os from 'os';
-import path from 'path';
-import fs from 'fs/promises';
+import { getClaudeCliPaths, getClaudeAuthIndicators, systemPathAccess } from '@automaker/platform';
 import { getApiKey } from './common.js';

 const execAsync = promisify(exec);
@@ -37,42 +35,25 @@ export async function getClaudeStatus() {
      // Version command might not be available
    }
  } catch {
-    // Not in PATH, try common locations based on platform
-    const commonPaths = isWindows
-      ? (() => {
-          const appData = process.env.APPDATA || path.join(os.homedir(), 'AppData', 'Roaming');
-          return [
-            // Windows-specific paths
-            path.join(os.homedir(), '.local', 'bin', 'claude.exe'),
-            path.join(appData, 'npm', 'claude.cmd'),
-            path.join(appData, 'npm', 'claude'),
-            path.join(appData, '.npm-global', 'bin', 'claude.cmd'),
-            path.join(appData, '.npm-global', 'bin', 'claude'),
-          ];
-        })()
-      : [
-          // Unix (Linux/macOS) paths
-          path.join(os.homedir(), '.local', 'bin', 'claude'),
-          path.join(os.homedir(), '.claude', 'local', 'claude'),
-          '/usr/local/bin/claude',
-          path.join(os.homedir(), '.npm-global', 'bin', 'claude'),
-        ];
+    // Not in PATH, try common locations from centralized system paths
+    const commonPaths = getClaudeCliPaths();

    for (const p of commonPaths) {
      try {
-        await fs.access(p);
-        cliPath = p;
-        installed = true;
-        method = 'local';
+        if (await systemPathAccess(p)) {
+          cliPath = p;
+          installed = true;
+          method = 'local';

-        // Get version from this path
-        try {
-          const { stdout: versionOut } = await execAsync(`"${p}" --version`);
-          version = versionOut.trim();
-        } catch {
-          // Version command might not be available
+          // Get version from this path
+          try {
+            const { stdout: versionOut } = await execAsync(`"${p}" --version`);
+            version = versionOut.trim();
+          } catch {
+            // Version command might not be available
+          }
+          break;
        }
-        break;
      } catch {
        // Not found at this path
      }
@@ -82,7 +63,7 @@ export async function getClaudeStatus() {
  // Check authentication - detect all possible auth methods
  // Note: apiKeys.anthropic_oauth_token stores OAuth tokens from subscription auth
  //       apiKeys.anthropic stores direct API keys for pay-per-use
-  let auth = {
+  const auth = {
    authenticated: false,
    method: 'none' as string,
    hasCredentialsFile: false,
@@ -97,76 +78,36 @@ export async function getClaudeStatus() {
    hasRecentActivity: false,
  };

-  const claudeDir = path.join(os.homedir(), '.claude');
+  // Use centralized system paths to check Claude authentication indicators
+  const indicators = await getClaudeAuthIndicators();

-  // Check for recent Claude CLI activity - indicates working authentication
-  // The stats-cache.json file is only populated when the CLI is working properly
-  const statsCachePath = path.join(claudeDir, 'stats-cache.json');
-  try {
-    const statsContent = await fs.readFile(statsCachePath, 'utf-8');
-    const stats = JSON.parse(statsContent);
+  // Check for recent activity (indicates working authentication)
+  if (indicators.hasStatsCacheWithActivity) {
+    auth.hasRecentActivity = true;
+    auth.hasCliAuth = true;
+    auth.authenticated = true;
+    auth.method = 'cli_authenticated';
+  }

-    // Check if there's any activity (which means the CLI is authenticated and working)
-    if (stats.dailyActivity && stats.dailyActivity.length > 0) {
-      auth.hasRecentActivity = true;
-      auth.hasCliAuth = true;
+  // Check for settings + sessions (indicates CLI is set up)
+  if (!auth.hasCliAuth && indicators.hasSettingsFile && indicators.hasProjectsSessions) {
+    auth.hasCliAuth = true;
+    auth.authenticated = true;
+    auth.method = 'cli_authenticated';
+  }
+
+  // Check credentials file
+  if (indicators.hasCredentialsFile && indicators.credentials) {
+    auth.hasCredentialsFile = true;
+    if (indicators.credentials.hasOAuthToken) {
+      auth.hasStoredOAuthToken = true;
+      auth.oauthTokenValid = true;
      auth.authenticated = true;
-      auth.method = 'cli_authenticated';
-    }
-  } catch {
-    // Stats file doesn't exist or is invalid
-  }
-
-  // Check for settings.json - indicates CLI has been set up
-  const settingsPath = path.join(claudeDir, 'settings.json');
-  try {
-    await fs.access(settingsPath);
-    // If settings exist but no activity, CLI might be set up but not authenticated
-    if (!auth.hasCliAuth) {
-      // Try to check for other indicators of auth
-      const sessionsDir = path.join(claudeDir, 'projects');
-      try {
-        const sessions = await fs.readdir(sessionsDir);
-        if (sessions.length > 0) {
-          auth.hasCliAuth = true;
-          auth.authenticated = true;
-          auth.method = 'cli_authenticated';
-        }
-      } catch {
-        // Sessions directory doesn't exist
-      }
-    }
-  } catch {
-    // Settings file doesn't exist
-  }
-
-  // Check for credentials file (OAuth tokens from claude login)
-  // Note: Claude CLI may use ".credentials.json" (hidden) or "credentials.json" depending on version/platform
-  const credentialsPaths = [
-    path.join(claudeDir, '.credentials.json'),
-    path.join(claudeDir, 'credentials.json'),
-  ];
-
-  for (const credentialsPath of credentialsPaths) {
-    try {
-      const credentialsContent = await fs.readFile(credentialsPath, 'utf-8');
-      const credentials = JSON.parse(credentialsContent);
-      auth.hasCredentialsFile = true;
-
-      // Check what type of token is in credentials
-      if (credentials.oauth_token || credentials.access_token) {
-        auth.hasStoredOAuthToken = true;
-        auth.oauthTokenValid = true;
-        auth.authenticated = true;
-        auth.method = 'oauth_token'; // Stored OAuth token from credentials file
-      } else if (credentials.api_key) {
-        auth.apiKeyValid = true;
-        auth.authenticated = true;
-        auth.method = 'api_key'; // Stored API key in credentials file
-      }
-      break; // Found and processed credentials file
-    } catch {
-      // No credentials file at this path or invalid format
+      auth.method = 'oauth_token';
+    } else if (indicators.credentials.hasApiKey) {
+      auth.apiKeyValid = true;
+      auth.authenticated = true;
+      auth.method = 'api_key';
    }
  }

@@ -174,21 +115,21 @@ export async function getClaudeStatus() {
  if (auth.hasEnvApiKey) {
    auth.authenticated = true;
    auth.apiKeyValid = true;
-    auth.method = 'api_key_env'; // API key from ANTHROPIC_API_KEY env var
+    auth.method = 'api_key_env';
  }

  // In-memory stored OAuth token (from setup wizard - subscription auth)
  if (!auth.authenticated && getApiKey('anthropic_oauth_token')) {
    auth.authenticated = true;
    auth.oauthTokenValid = true;
-    auth.method = 'oauth_token'; // Stored OAuth token from setup wizard
+    auth.method = 'oauth_token';
  }

  // In-memory stored API key (from settings UI - pay-per-use)
  if (!auth.authenticated && getApiKey('anthropic')) {
    auth.authenticated = true;
    auth.apiKeyValid = true;
-    auth.method = 'api_key'; // Manually stored API key
+    auth.method = 'api_key';
  }

  return {
--- a/apps/server/src/routes/setup/routes/delete-api-key.ts
+++ b/apps/server/src/routes/setup/routes/delete-api-key.ts
@@ -5,40 +5,22 @@
 import type { Request, Response } from 'express';
 import { createLogger } from '@automaker/utils';
 import path from 'path';
-import fs from 'fs/promises';
+import { secureFs } from '@automaker/platform';

 const logger = createLogger('Setup');

 // In-memory storage reference (imported from common.ts pattern)
-// We need to modify common.ts to export a deleteApiKey function
 import { setApiKey } from '../common.js';

 /**
 * Remove an API key from the .env file
+ * Uses centralized secureFs.removeEnvKey for path validation
 */
 async function removeApiKeyFromEnv(key: string): Promise<void> {
  const envPath = path.join(process.cwd(), '.env');

  try {
-    let envContent = '';
-    try {
-      envContent = await fs.readFile(envPath, 'utf-8');
-    } catch {
-      // .env file doesn't exist, nothing to delete
-      return;
-    }
-
-    // Parse existing env content and remove the key
-    const lines = envContent.split('\n');
-    const keyRegex = new RegExp(`^${key}=`);
-    const newLines = lines.filter((line) => !keyRegex.test(line));
-
-    // Remove empty lines at the end
-    while (newLines.length > 0 && newLines[newLines.length - 1].trim() === '') {
-      newLines.pop();
-    }
-
-    await fs.writeFile(envPath, newLines.join('\n') + (newLines.length > 0 ? '\n' : ''));
+    await secureFs.removeEnvKey(envPath, key);
    logger.info(`[Setup] Removed ${key} from .env file`);
  } catch (error) {
    logger.error(`[Setup] Failed to remove ${key} from .env:`, error);
--- a/apps/server/src/routes/setup/routes/gh-status.ts
+++ b/apps/server/src/routes/setup/routes/gh-status.ts
@@ -5,27 +5,14 @@
 import type { Request, Response } from 'express';
 import { exec } from 'child_process';
 import { promisify } from 'util';
-import os from 'os';
-import path from 'path';
-import fs from 'fs/promises';
+import { getGitHubCliPaths, getExtendedPath, systemPathAccess } from '@automaker/platform';
 import { getErrorMessage, logError } from '../common.js';

 const execAsync = promisify(exec);

-// Extended PATH to include common tool installation locations
-const extendedPath = [
-  process.env.PATH,
-  '/opt/homebrew/bin',
-  '/usr/local/bin',
-  '/home/linuxbrew/.linuxbrew/bin',
-  `${process.env.HOME}/.local/bin`,
-]
-  .filter(Boolean)
-  .join(':');
-
 const execEnv = {
  ...process.env,
-  PATH: extendedPath,
+  PATH: getExtendedPath(),
 };

 export interface GhStatus {
@@ -55,25 +42,16 @@ async function getGhStatus(): Promise<GhStatus> {
    status.path = stdout.trim().split(/\r?\n/)[0];
    status.installed = true;
  } catch {
-    // gh not in PATH, try common locations
-    const commonPaths = isWindows
-      ? [
-          path.join(process.env.LOCALAPPDATA || '', 'Programs', 'gh', 'bin', 'gh.exe'),
-          path.join(process.env.ProgramFiles || '', 'GitHub CLI', 'gh.exe'),
-        ]
-      : [
-          '/opt/homebrew/bin/gh',
-          '/usr/local/bin/gh',
-          path.join(os.homedir(), '.local', 'bin', 'gh'),
-          '/home/linuxbrew/.linuxbrew/bin/gh',
-        ];
+    // gh not in PATH, try common locations from centralized system paths
+    const commonPaths = getGitHubCliPaths();

    for (const p of commonPaths) {
      try {
-        await fs.access(p);
-        status.path = p;
-        status.installed = true;
-        break;
+        if (await systemPathAccess(p)) {
+          status.path = p;
+          status.installed = true;
+          break;
+        }
      } catch {
        // Not found at this path
      }
--- a/apps/server/src/routes/terminal/routes/sessions.ts
+++ b/apps/server/src/routes/terminal/routes/sessions.ts
@@ -22,12 +22,12 @@ export function createSessionsListHandler() {
 }

 export function createSessionsCreateHandler() {
-  return (req: Request, res: Response): void => {
+  return async (req: Request, res: Response): Promise<void> => {
    try {
      const terminalService = getTerminalService();
      const { cwd, cols, rows, shell } = req.body;

-      const session = terminalService.createSession({
+      const session = await terminalService.createSession({
        cwd,
        cols: cols || 80,
        rows: rows || 24,
--- a/apps/server/src/services/terminal-service.ts
+++ b/apps/server/src/services/terminal-service.ts
@@ -8,8 +8,13 @@
 import * as pty from 'node-pty';
 import { EventEmitter } from 'events';
 import * as os from 'os';
-import * as fs from 'fs';
 import * as path from 'path';
+// secureFs is used for user-controllable paths (working directory validation)
+// to enforce ALLOWED_ROOT_DIRECTORY security boundary
+import * as secureFs from '../lib/secure-fs.js';
+// System paths module handles shell binary checks and WSL detection
+// These are system paths outside ALLOWED_ROOT_DIRECTORY, centralized for security auditing
+import { systemPathExists, systemPathReadFileSync, getWslVersionPath } from '@automaker/platform';

 // Maximum scrollback buffer size (characters)
 const MAX_SCROLLBACK_SIZE = 50000; // ~50KB per terminal
@@ -68,7 +73,7 @@ export class TerminalService extends EventEmitter {
    if (platform === 'linux' && this.isWSL()) {
      // In WSL, prefer the user's configured shell or bash
      const userShell = process.env.SHELL || '/bin/bash';
-      if (fs.existsSync(userShell)) {
+      if (systemPathExists(userShell)) {
        return { shell: userShell, args: ['--login'] };
      }
      return { shell: '/bin/bash', args: ['--login'] };
@@ -80,10 +85,10 @@ export class TerminalService extends EventEmitter {
        const pwsh = 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe';
        const pwshCore = 'C:\\Program Files\\PowerShell\\7\\pwsh.exe';

-        if (fs.existsSync(pwshCore)) {
+        if (systemPathExists(pwshCore)) {
          return { shell: pwshCore, args: [] };
        }
-        if (fs.existsSync(pwsh)) {
+        if (systemPathExists(pwsh)) {
          return { shell: pwsh, args: [] };
        }
        return { shell: 'cmd.exe', args: [] };
@@ -92,10 +97,10 @@ export class TerminalService extends EventEmitter {
      case 'darwin': {
        // macOS: prefer user's shell, then zsh, then bash
        const userShell = process.env.SHELL;
-        if (userShell && fs.existsSync(userShell)) {
+        if (userShell && systemPathExists(userShell)) {
          return { shell: userShell, args: ['--login'] };
        }
-        if (fs.existsSync('/bin/zsh')) {
+        if (systemPathExists('/bin/zsh')) {
          return { shell: '/bin/zsh', args: ['--login'] };
        }
        return { shell: '/bin/bash', args: ['--login'] };
@@ -105,10 +110,10 @@ export class TerminalService extends EventEmitter {
      default: {
        // Linux: prefer user's shell, then bash, then sh
        const userShell = process.env.SHELL;
-        if (userShell && fs.existsSync(userShell)) {
+        if (userShell && systemPathExists(userShell)) {
          return { shell: userShell, args: ['--login'] };
        }
-        if (fs.existsSync('/bin/bash')) {
+        if (systemPathExists('/bin/bash')) {
          return { shell: '/bin/bash', args: ['--login'] };
        }
        return { shell: '/bin/sh', args: [] };
@@ -122,8 +127,9 @@ export class TerminalService extends EventEmitter {
  isWSL(): boolean {
    try {
      // Check /proc/version for Microsoft/WSL indicators
-      if (fs.existsSync('/proc/version')) {
-        const version = fs.readFileSync('/proc/version', 'utf-8').toLowerCase();
+      const wslVersionPath = getWslVersionPath();
+      if (systemPathExists(wslVersionPath)) {
+        const version = systemPathReadFileSync(wslVersionPath, 'utf-8').toLowerCase();
        return version.includes('microsoft') || version.includes('wsl');
      }
      // Check for WSL environment variable
@@ -157,8 +163,9 @@ export class TerminalService extends EventEmitter {
  /**
   * Validate and resolve a working directory path
   * Includes basic sanitization against null bytes and path normalization
+   * Uses secureFs to enforce ALLOWED_ROOT_DIRECTORY for user-provided paths
   */
-  private resolveWorkingDirectory(requestedCwd?: string): string {
+  private async resolveWorkingDirectory(requestedCwd?: string): Promise<string> {
    const homeDir = os.homedir();

    // If no cwd requested, use home
@@ -187,15 +194,19 @@ export class TerminalService extends EventEmitter {
    }

    // Check if path exists and is a directory
+    // Using secureFs.stat to enforce ALLOWED_ROOT_DIRECTORY security boundary
+    // This prevents terminals from being opened in directories outside the allowed workspace
    try {
-      const stat = fs.statSync(cwd);
-      if (stat.isDirectory()) {
+      const statResult = await secureFs.stat(cwd);
+      if (statResult.isDirectory()) {
        return cwd;
      }
      console.warn(`[Terminal] Path exists but is not a directory: ${cwd}, falling back to home`);
      return homeDir;
    } catch {
-      console.warn(`[Terminal] Working directory does not exist: ${cwd}, falling back to home`);
+      console.warn(
+        `[Terminal] Working directory does not exist or not allowed: ${cwd}, falling back to home`
+      );
      return homeDir;
    }
  }
@@ -228,7 +239,7 @@ export class TerminalService extends EventEmitter {
   * Create a new terminal session
   * Returns null if the maximum session limit has been reached
   */
-  createSession(options: TerminalOptions = {}): TerminalSession | null {
+  async createSession(options: TerminalOptions = {}): Promise<TerminalSession | null> {
    // Check session limit
    if (this.sessions.size >= maxSessions) {
      console.error(`[Terminal] Max sessions (${maxSessions}) reached, refusing new session`);
@@ -241,7 +252,8 @@ export class TerminalService extends EventEmitter {
    const shell = options.shell || detectedShell;

    // Validate and resolve working directory
-    const cwd = this.resolveWorkingDirectory(options.cwd);
+    // Uses secureFs internally to enforce ALLOWED_ROOT_DIRECTORY
+    const cwd = await this.resolveWorkingDirectory(options.cwd);

    // Build environment with some useful defaults
    // These settings ensure consistent terminal behavior across platforms