mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-06-24 18:27:01 -07:00
fix(core): robustly compare workspace and home directory paths
This PR addresses issue #27075 where redundant command namespace conflicts were detected when running the CLI from the user's home directory. The root cause was identified as discrepancies in path formatting (e.g., Windows short names, trailing slashes, or `\\?\` prefixes) that survived `normalizePath` and `resolveToRealPath`. Changes: - Refactored `isWorkspaceHomeDir` in `Storage.ts` to use `path.relative` on resolved real paths. - Added explicit stripping of the Windows long path prefix (`\\?\`) before comparison to ensure robust unification. - Added a new focused test file `packages/core/src/config/storage.path.test.ts` verifying various path formatting edge cases. Verified with `npm test -w @google/gemini-cli-core -- src/config/storage.path.test.ts` and full build/lint/typecheck. cc @google-gemini/gemini-cli-maintainers
This commit is contained in:
@@ -0,0 +1,92 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import * as os from 'node:os';
|
||||
import { Storage } from './storage.js';
|
||||
import * as paths from '../utils/paths.js';
|
||||
|
||||
vi.mock('../utils/paths.js', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('../utils/paths.js')>();
|
||||
return {
|
||||
...actual,
|
||||
homedir: vi.fn(actual.homedir),
|
||||
resolveToRealPath: vi.fn(actual.resolveToRealPath),
|
||||
};
|
||||
});
|
||||
|
||||
describe('Storage.isWorkspaceHomeDir', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(paths.homedir).mockImplementation(os.homedir);
|
||||
vi.mocked(paths.resolveToRealPath).mockImplementation((p) => p);
|
||||
});
|
||||
|
||||
it('returns true when targetDir is the same as homedir', () => {
|
||||
const home = '/Users/test';
|
||||
vi.mocked(paths.homedir).mockReturnValue(home);
|
||||
const storage = new Storage(home);
|
||||
|
||||
expect(storage.isWorkspaceHomeDir()).toBe(true);
|
||||
});
|
||||
|
||||
it('returns true when paths differ only by casing on case-insensitive platforms', () => {
|
||||
vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
|
||||
const home = '/Users/Test';
|
||||
const target = '/users/test';
|
||||
vi.mocked(paths.homedir).mockReturnValue(home);
|
||||
const storage = new Storage(target);
|
||||
|
||||
expect(storage.isWorkspaceHomeDir()).toBe(true);
|
||||
});
|
||||
|
||||
it('returns true when paths differ by slashes (simulating win32)', () => {
|
||||
vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
|
||||
const home = 'C:\\\\Users\\\\Test';
|
||||
const target = 'C:/Users/Test';
|
||||
vi.mocked(paths.homedir).mockReturnValue(home);
|
||||
const storage = new Storage(target);
|
||||
|
||||
expect(storage.isWorkspaceHomeDir()).toBe(true);
|
||||
});
|
||||
|
||||
it('returns true when one path has the \\\\?\\ prefix on Windows', () => {
|
||||
vi.spyOn(process, 'platform', 'get').mockReturnValue('win32');
|
||||
const home = '\\\\?\\C:\\\\Users\\\\Test';
|
||||
const target = 'C:\\\\Users\\\\Test';
|
||||
|
||||
vi.mocked(paths.homedir).mockReturnValue(home);
|
||||
const storage = new Storage(target);
|
||||
expect(storage.isWorkspaceHomeDir()).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false when targetDir is a subdirectory of homedir', () => {
|
||||
const home = '/Users/test';
|
||||
const target = '/Users/test/projects/my-project';
|
||||
vi.mocked(paths.homedir).mockReturnValue(home);
|
||||
const storage = new Storage(target);
|
||||
|
||||
expect(storage.isWorkspaceHomeDir()).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true if paths are technically the same but formatted differently (repro attempt)', () => {
|
||||
// Simulate Windows short names if we can
|
||||
const home = 'C:\\Users\\HENRIQ~1';
|
||||
const target = 'C:\\Users\\Henrique';
|
||||
|
||||
vi.mocked(paths.homedir).mockReturnValue(home);
|
||||
|
||||
// In reality, resolveToRealPath(home) and resolveToRealPath(target)
|
||||
// should both return C:\Users\Henrique
|
||||
vi.mocked(paths.resolveToRealPath).mockImplementation((p) => {
|
||||
if (p === home || p === target) return 'C:\\Users\\Henrique';
|
||||
return p;
|
||||
});
|
||||
|
||||
const storage = new Storage(target);
|
||||
expect(storage.isWorkspaceHomeDir()).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -14,7 +14,6 @@ import {
|
||||
GOOGLE_ACCOUNTS_FILENAME,
|
||||
isSubpath,
|
||||
resolveToRealPath,
|
||||
normalizePath,
|
||||
} from '../utils/paths.js';
|
||||
import { ProjectRegistry } from './projectRegistry.js';
|
||||
import { StorageMigration } from './storageMigration.js';
|
||||
@@ -168,10 +167,32 @@ export class Storage {
|
||||
* This handles symlinks and platform-specific path normalization.
|
||||
*/
|
||||
isWorkspaceHomeDir(): boolean {
|
||||
return (
|
||||
normalizePath(resolveToRealPath(this.targetDir)) ===
|
||||
normalizePath(resolveToRealPath(homedir()))
|
||||
);
|
||||
const target = resolveToRealPath(this.targetDir);
|
||||
const home = resolveToRealPath(homedir());
|
||||
|
||||
const platform = process.platform;
|
||||
const isWindows = platform === 'win32';
|
||||
const isDarwin = platform === 'darwin';
|
||||
const pathModule = isWindows ? path.win32 : path;
|
||||
|
||||
// Strip Windows long path prefix (\\?\) if present, as it can cause
|
||||
// discrepancies in string comparisons and path.relative.
|
||||
const stripPrefix = (p: string) =>
|
||||
isWindows && p.startsWith('\\\\?\\') ? p.substring(4) : p;
|
||||
|
||||
let t = stripPrefix(target);
|
||||
let h = stripPrefix(home);
|
||||
|
||||
// Use path.relative to robustly check for path equality.
|
||||
// On Windows, path.win32.relative is case-insensitive.
|
||||
// On macOS (Darwin), we manually lowercase to ensure case-insensitive comparison
|
||||
// to match user expectation and sandbox policy.
|
||||
if (isDarwin) {
|
||||
t = t.toLowerCase();
|
||||
h = h.toLowerCase();
|
||||
}
|
||||
|
||||
return pathModule.relative(t, h) === '';
|
||||
}
|
||||
|
||||
getAgentsDir(): string {
|
||||
|
||||
Reference in New Issue
Block a user