test: add unit tests for resolveAtCommandPath and path validator edge cases

packages/cli/src/utils/atCommandUtils.test.ts

@@ -0,0 +1,178 @@
+/**
+ * @license
+ * Copyright 2026 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest';
+import * as path from 'node:path';
+import * as fs from 'node:fs/promises';
+import { resolveAtCommandPath } from './atCommandUtils.js';
+import { type Config } from '@google/gemini-cli-core';
+
+vi.mock('node:fs/promises');
+
+describe('atCommandUtils', () => {
+  let mockConfig: Record<string, unknown>;
+  let mockWorkspaceContext: Record<string, unknown>;
+
+  beforeEach(() => {
+    vi.resetAllMocks();
+
+    mockWorkspaceContext = {
+      getDirectories: vi.fn().mockReturnValue(['/mock/root']),
+      isPathReadable: vi.fn().mockReturnValue(true),
+    };
+
+    mockConfig = {
+      getTargetDir: vi.fn().mockReturnValue('/mock/root'),
+      getWorkspaceContext: vi.fn().mockReturnValue(mockWorkspaceContext),
+      validatePathAccess: vi.fn().mockReturnValue(null),
+    };
+  });
+
+  it('should resolve a valid path', async () => {
+    const mockStats = {
+      isDirectory: () => false,
+      isFile: () => true,
+    };
+    vi.mocked(fs.stat).mockResolvedValue(mockStats as unknown as fs.Stats);
+
+    const result = await resolveAtCommandPath(
+      'file.ts',
+      mockConfig as unknown as Config,
+    );
+
+    expect(result).not.toBeNull();
+    expect(result?.absolutePath).toBe(path.resolve('/mock/root', 'file.ts'));
+    expect(result?.relativePath).toBe('file.ts');
+  });
+
+  it('should resolve an absolute path', async () => {
+    const mockStats = {
+      isDirectory: () => false,
+      isFile: () => true,
+    };
+    vi.mocked(fs.stat).mockResolvedValue(mockStats as unknown as fs.Stats);
+
+    const absolutePath = path.resolve('/mock/root', 'src/index.ts');
+    const result = await resolveAtCommandPath(
+      absolutePath,
+      mockConfig as unknown as Config,
+    );
+
+    expect(result).not.toBeNull();
+    expect(result?.absolutePath).toBe(absolutePath);
+    expect(result?.relativePath).toBe('src/index.ts');
+  });
+
+  it('should handle multiple directories in workspace context', async () => {
+    (mockWorkspaceContext.getDirectories as Mock).mockReturnValue([
+      '/dir1',
+      '/dir2',
+    ]);
+    const mockStats = {
+      isDirectory: () => false,
+      isFile: () => true,
+    };
+
+    vi.mocked(fs.stat).mockImplementation(async (p) => {
+      if (p === path.resolve('/dir2', 'file.txt')) {
+        return mockStats as unknown as fs.Stats;
+      }
+      throw new Error('ENOENT');
+    });
+
+    const result = await resolveAtCommandPath(
+      'file.txt',
+      mockConfig as unknown as Config,
+    );
+
+    expect(result?.absolutePath).toBe(path.resolve('/dir2', 'file.txt'));
+    expect(result?.relativePath).toBe('file.txt');
+  });
+
+  it('should return null for invalid path (too long)', async () => {
+    const longPath = 'a'.repeat(5000);
+    const result = await resolveAtCommandPath(
+      longPath,
+      mockConfig as unknown as Config,
+    );
+    expect(result).toBeNull();
+  });
+
+  it('should return null for path with log markers', async () => {
+    const onDebug = vi.fn();
+    const result = await resolveAtCommandPath(
+      'FAIL tests/my.test.ts',
+      mockConfig as unknown as Config,
+      onDebug,
+    );
+    expect(result).toBeNull();
+    expect(onDebug).toHaveBeenCalledWith(
+      expect.stringContaining('Skipping invalid path'),
+    );
+  });
+
+  it('should return null if path does not exist in any workspace directory', async () => {
+    vi.mocked(fs.stat).mockRejectedValue(new Error('ENOENT'));
+
+    const result = await resolveAtCommandPath(
+      'nonexistent.ts',
+      mockConfig as unknown as Config,
+    );
+
+    expect(result).toBeNull();
+  });
+
+  it('should resolve directory paths correctly', async () => {
+    const mockStats = {
+      isDirectory: () => true,
+      isFile: () => false,
+    };
+    vi.mocked(fs.stat).mockResolvedValue(mockStats as unknown as fs.Stats);
+
+    const result = await resolveAtCommandPath(
+      'src',
+      mockConfig as unknown as Config,
+    );
+
+    expect(result?.stats.isDirectory()).toBe(true);
+  });
+
+  it('should respect validatePathAccess for paths within root', async () => {
+    (mockConfig.validatePathAccess as Mock).mockReturnValue(
+      'Unauthorized access',
+    );
+    // Mock getTargetDir to match the resolved path so it's considered "within root"
+    (mockConfig.getTargetDir as Mock).mockReturnValue('/mock/root');
+
+    const result = await resolveAtCommandPath(
+      'secret.txt',
+      mockConfig as unknown as Config,
+    );
+    expect(result).toBeNull();
+  });
+
+  it('should return null for unauthorized paths (letting calling site handle it, e.g. acpSession permission dialog)', async () => {
+    (mockConfig.validatePathAccess as Mock).mockReturnValue(
+      'Outside workspace',
+    );
+    (mockConfig.getTargetDir as Mock).mockReturnValue('/mock/workspace');
+
+    const mockStats = {
+      isDirectory: () => false,
+      isFile: () => true,
+    };
+    vi.mocked(fs.stat).mockResolvedValue(mockStats as unknown as fs.Stats);
+
+    // Path resolve will use /mock/root as base from mockWorkspaceContext
+    const result = await resolveAtCommandPath(
+      'outside.txt',
+      mockConfig as unknown as Config,
+    );
+
+    // Should now return null so acpSession can trigger its own permission flow
+    expect(result).toBeNull();
+  });
+});

packages/cli/src/utils/atCommandUtils.ts

@@ -6,11 +6,7 @@
 
 import * as path from 'node:path';
 import * as fs from 'node:fs/promises';
-import {
-  validatePath,
-  isWithinRoot,
-  type Config,
-} from '@google/gemini-cli-core';
+import { validatePath, type Config } from '@google/gemini-cli-core';
 
 export interface ResolvedAtCommandPath {
   absolutePath: string;
@@ -52,16 +48,10 @@ export async function resolveAtCommandPath(
       // Final workspace boundary check using centralized logic
       const validationError = config.validatePathAccess(absolutePath, 'read');
       if (validationError) {
-        // If it's outside root, we might still allow it with explicit user permission in acpSession,
-        // but for now, we follow the general rule.
-        if (!isWithinRoot(absolutePath, config.getTargetDir())) {
-          // Proceed to stat check, calling sites will handle permission dialogs if needed
-        } else {
-          onDebugMessage(
-            `Skipping unauthorized path: ${absolutePath}. Reason: ${validationError}`,
-          );
-          continue;
-        }
+        onDebugMessage(
+          `Skipping unauthorized path: ${absolutePath}. Reason: ${validationError}`,
+        );
+        continue;
       }
 
       const stats = await fs.stat(absolutePath);

packages/core/src/config/path-validation.consolidated.test.ts

@@ -0,0 +1,61 @@
+/**
+ * @license
+ * Copyright 2026 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import path from 'node:path';
+import { Config, type ConfigParameters } from './config.js';
+
+describe('Config Path Validation Integration', () => {
+  let config: Config;
+  const projectRoot = process.cwd();
+
+  beforeEach(() => {
+    const params: ConfigParameters = {
+      sessionId: 'test-session',
+      targetDir: projectRoot,
+      debugMode: false,
+      model: 'test-model',
+      cwd: projectRoot,
+    };
+    config = new Config(params);
+  });
+
+  it('should reject pathologically long paths in validatePathAccess', () => {
+    const longPath = path.join(projectRoot, 'a'.repeat(5000));
+    const result = config.validatePathAccess(longPath, 'read');
+    expect(result).toContain('Invalid path: Path is too long');
+  });
+
+  it('should reject paths with log markers in validatePathAccess', () => {
+    const logPath = path.join(
+      projectRoot,
+      'AssertionError: expected true to be false',
+    );
+    const result = config.validatePathAccess(logPath, 'read');
+    expect(result).toContain(
+      'Invalid path: Path appears to be a misinterpreted log fragment',
+    );
+  });
+
+  it('should reject paths with control characters in validatePathAccess', () => {
+    const malformedPath = path.join(projectRoot, 'file\nwith\nnewline.txt');
+    const result = config.validatePathAccess(malformedPath, 'read');
+    expect(result).toContain('Invalid path: Path contains invalid characters');
+  });
+
+  it('should allow normal paths in validatePathAccess', () => {
+    const normalPath = path.resolve(projectRoot, 'src/index.ts');
+    const result = config.validatePathAccess(normalPath, 'read');
+
+    // It might return "outside workspace" if not fully initialized,
+    // but it should NOT return the "Invalid path" prefix from PathValidator.
+    if (result) {
+      expect(result).not.toContain('Invalid path:');
+    } else {
+      expect(result).toBeNull();
+    }
+  });
+});

packages/core/src/utils/path-validator.test.ts

@@ -68,10 +68,25 @@ describe('PathValidator', () => {
     ).toBe(false);
     expect(validatePath('✓ test passed').isValid).toBe(false);
     expect(validatePath('× test failed').isValid).toBe(false);
+    expect(
+      validatePath('TestingLibraryElementError: Unable to find an element')
+        .isValid,
+    ).toBe(false);
   });
 
   it('should allow short paths with quotes (even if unusual)', () => {
     // Some systems might technically allow this, and we only want to block long/obvious log fragments
     expect(validatePath('file"with"quote.txt').isValid).toBe(true);
   });
+
+  it('should reject long paths with ellipses', () => {
+    expect(
+      validatePath('this/is/a/very/long/path/with/ellipses/.../and/more')
+        .isValid,
+    ).toBe(false);
+  });
+
+  it('should allow short paths with braces', () => {
+    expect(validatePath('{a,b}.ts').isValid).toBe(true);
+  });
 });