diff --git a/.changeset/windows-path-case-sensitivity.md b/.changeset/windows-path-case-sensitivity.md new file mode 100644 index 000000000..9c29a9d2b --- /dev/null +++ b/.changeset/windows-path-case-sensitivity.md @@ -0,0 +1,5 @@ +--- +"@nanocollective/nanocoder": patch +--- + +Normalize path comparisons to lowercase on Windows (`win32`) in `isPathInside` and directory trust checks under standard Win32 case-insensitive filesystem semantics. Windows filesystems are case-insensitive by default, so paths differing only in case (e.g. `c:\work\proj` vs `C:\Work\Proj`, or uppercase drive letters) are now correctly recognized as contained within the workspace root and match existing entries in `trustedDirectories` without re-prompting or rejecting valid absolute paths. Closes #1689. diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index c93899524..b66f1b8bd 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -55,6 +55,13 @@ jobs: - name: Verify execute_bash cmd.exe quoting run: pnpm run test:ava source/services/bash-executor.spec.ts --match="cmd.exe quoting*" + - name: Verify Windows path validation and preferences case normalization + run: >- + pnpm run test:ava + source/utils/path-validation.spec.ts + source/config/preferences.spec.ts + --match="*case on Windows*" + # pnpm runs package.json scripts through cmd.exe on Windows, so POSIX-only # commands in the build script (mkdir -p, cp, chmod) break there. The shared # job only builds on Linux, so build on Windows here with no Unix shell. diff --git a/source/config/preferences.spec.ts b/source/config/preferences.spec.ts index a3040bb56..b9ca7c9b1 100644 --- a/source/config/preferences.spec.ts +++ b/source/config/preferences.spec.ts @@ -2182,6 +2182,63 @@ test('isDirectoryTrusted does not match an unrelated directory', t => { ); }); +if (process.platform === 'win32') { + test('isDirectoryTrusted matches a directory regardless of case on Windows', t => { + const dir = process.cwd(); + const altCaseDir = + dir[0] === dir[0].toLowerCase() + ? dir[0].toUpperCase() + dir.slice(1) + : dir[0].toLowerCase() + dir.slice(1); + const fingerprint = describeProjectTrust(dir).fingerprint; + + t.true( + isDirectoryTrusted(altCaseDir, { + trustedDirectories: [{path: dir, fingerprint}], + }), + ); + t.true( + isDirectoryTrusted(dir, { + trustedDirectories: [{path: altCaseDir, fingerprint}], + }), + ); + }); + + test('trustRecheck finds matching entry regardless of case on Windows', t => { + const dir = process.cwd(); + const altCaseDir = + dir[0] === dir[0].toLowerCase() + ? dir[0].toUpperCase() + dir.slice(1) + : dir[0].toLowerCase() + dir.slice(1); + + t.is( + trustRecheck(altCaseDir, { + trustedDirectories: [{path: dir, fingerprint: 'stale'}], + }), + 'changed', + ); + t.is( + trustRecheck(altCaseDir, { + trustedDirectories: [dir], + }), + 'upgrade', + ); + }); + + test('grantDirectoryTrust deduplicates entries differing only in case on Windows', t => { + const dir = process.cwd(); + const altCaseDir = + dir[0] === dir[0].toLowerCase() + ? dir[0].toUpperCase() + dir.slice(1) + : dir[0].toLowerCase() + dir.slice(1); + const prefs: UserPreferences = { + trustedDirectories: [{path: dir, fingerprint: 'old'}], + }; + const updated = grantDirectoryTrust(prefs, altCaseDir, 'new'); + t.is(updated.trustedDirectories?.length, 1); + t.is(updated.trustedDirectories?.[0]?.fingerprint, 'new'); + }); +} + test('ensureDirectoryTrust trusts without persisting when bypass is true', t => { let saveCalled = false; const result = ensureDirectoryTrust('/untrusted/project', true, { diff --git a/source/config/preferences.ts b/source/config/preferences.ts index c80135b90..98da28f08 100644 --- a/source/config/preferences.ts +++ b/source/config/preferences.ts @@ -22,6 +22,7 @@ import type {UserPreferences} from '@/types/index'; import type {NanocoderShape, ThemePreset} from '@/types/ui'; import {atomicWriteFileSync} from '@/utils/atomic-write'; import {logError} from '@/utils/message-queue'; +import {normalizeForCompare} from '@/utils/path-validation'; let PREFERENCES_PATH: string | null = null; let CACHED_CONFIG_DIR: string | undefined = undefined; @@ -55,7 +56,8 @@ function getGlobalPreferencesPath(): string { function isProjectPreferencesPath(preferencesPath: string): boolean { return ( - path.resolve(preferencesPath) !== path.resolve(getGlobalPreferencesPath()) + normalizeForCompare(preferencesPath) !== // nosemgrep + normalizeForCompare(getGlobalPreferencesPath()) // nosemgrep ); } @@ -217,14 +219,14 @@ export function isDirectoryTrusted( directory: string, preferences: UserPreferences, ): boolean { - const resolved = path.resolve(directory); // nosemgrep - const fingerprint = describeProjectTrust(resolved).fingerprint; + const resolved = normalizeForCompare(directory); // nosemgrep + const fingerprint = describeProjectTrust(path.resolve(directory)).fingerprint; return (preferences.trustedDirectories ?? []).some(entry => { const entryPath = trustPath(entry); return ( typeof entry === 'object' && entryPath !== null && - path.resolve(entryPath) === resolved && // nosemgrep + normalizeForCompare(entryPath) === resolved && // nosemgrep entry.fingerprint === fingerprint ); }); @@ -241,11 +243,12 @@ export function trustRecheck( directory: string, preferences: UserPreferences, ): TrustRecheck | null { - const resolved = path.resolve(directory); // nosemgrep + const resolved = normalizeForCompare(directory); // nosemgrep let recheck: TrustRecheck | null = null; for (const entry of preferences.trustedDirectories ?? []) { const entryPath = trustPath(entry); - if (entryPath === null || path.resolve(entryPath) !== resolved) continue; // nosemgrep + if (entryPath === null || normalizeForCompare(entryPath) !== resolved) + continue; // nosemgrep if (typeof entry !== 'string') return 'changed'; recheck = 'upgrade'; } @@ -288,10 +291,13 @@ export function grantDirectoryTrust( fingerprint = describeProjectTrust(directory).fingerprint, ): UserPreferences { const resolved = path.resolve(directory); // nosemgrep + const normalized = normalizeForCompare(directory); const trustedDirectories = (preferences.trustedDirectories ?? []).filter( entry => { const entryPath = trustPath(entry); - return entryPath === null || path.resolve(entryPath) !== resolved; // nosemgrep + return ( + entryPath === null || normalizeForCompare(entryPath) !== normalized // nosemgrep + ); }, ); trustedDirectories.push({path: resolved, fingerprint}); diff --git a/source/daemon/full-commit-trust.ts b/source/daemon/full-commit-trust.ts index d92b1d3e8..d529d95dc 100644 --- a/source/daemon/full-commit-trust.ts +++ b/source/daemon/full-commit-trust.ts @@ -9,28 +9,20 @@ import path from 'node:path'; import {loadPreferences, savePreferences} from '@/config/preferences'; - -// Windows filesystems are case-insensitive (NTFS preserves case but doesn't -// distinguish it), so a resolved path can be re-encountered with different -// casing (a different shell, a case-differing junction) and must still -// match. path.resolve() alone doesn't normalize this. -function normalize(projectRoot: string): string { - const resolved = path.resolve(projectRoot); // nosemgrep - return process.platform === 'win32' ? resolved.toLowerCase() : resolved; -} +import {normalizeForCompare} from '@/utils/path-validation'; export function isFullCommitTrustConfirmed(projectRoot: string): boolean { const preferences = loadPreferences(); const confirmed = preferences.fullCommitConfirmedProjects ?? []; - const normalizedRoot = normalize(projectRoot); - return confirmed.some(p => normalize(p) === normalizedRoot); + const normalizedRoot = normalizeForCompare(projectRoot); + return confirmed.some(p => normalizeForCompare(p) === normalizedRoot); } export function recordFullCommitTrustConfirmed(projectRoot: string): void { const preferences = loadPreferences(); const confirmed = preferences.fullCommitConfirmedProjects ?? []; - const normalizedRoot = normalize(projectRoot); - if (!confirmed.some(p => normalize(p) === normalizedRoot)) { + const normalizedRoot = normalizeForCompare(projectRoot); + if (!confirmed.some(p => normalizeForCompare(p) === normalizedRoot)) { confirmed.push(path.resolve(projectRoot)); // nosemgrep preferences.fullCommitConfirmedProjects = confirmed; savePreferences(preferences); diff --git a/source/utils/path-validation.spec.ts b/source/utils/path-validation.spec.ts index 83effcdce..22bd83e49 100644 --- a/source/utils/path-validation.spec.ts +++ b/source/utils/path-validation.spec.ts @@ -12,6 +12,7 @@ import { isPathInside, isRealPathInside, isValidFilePath, + normalizeForCompare, resolveFilePath, } from './path-validation'; @@ -427,3 +428,47 @@ test.serial('resolveFilePath: a symlinked root still rejects paths really outsid rmSync(base, {recursive: true, force: true}); } }); + +test('normalizeForCompare: normalizes case on win32 and preserves case on posix', t => { + t.not( + normalizeForCompare('/Proj/App', 'linux'), + normalizeForCompare('/proj/app', 'linux'), + ); + t.is( + normalizeForCompare('C:\\Proj\\App', 'win32'), + normalizeForCompare('c:\\proj\\app', 'win32'), + ); +}); + +test('isPathInside: treats paths case-insensitively when platform is win32', t => { + t.true(isPathInside('C:\\Work\\Proj\\file.ts', 'c:\\work\\proj', 'win32')); + t.true(isPathInside('c:\\work\\proj', 'C:\\Work\\Proj', 'win32')); + t.true(isPathInside('c:\\work\\proj\\sub\\a.txt', 'C:\\Work\\Proj', 'win32')); + t.false(isPathInside('c:\\work\\proj-evil\\a.txt', 'C:\\Work\\Proj', 'win32')); + t.false(isPathInside('d:\\work\\proj\\a.txt', 'c:\\work\\proj', 'win32')); +}); + +if (process.platform === 'win32') { + test('isValidFilePath & resolveFilePath: accepts valid absolute paths differing in case on Windows', t => { + const base = mkdtempSync(join(tmpdir(), 'pathval-win-case-')); + try { + const lowerBase = base.toLowerCase(); + const upperBase = base.toUpperCase(); + const testFile = join(base, 'test.txt'); + writeFileSync(testFile, 'hello'); + + t.true(isValidFilePath(testFile, lowerBase)); + t.true(isValidFilePath(testFile, upperBase)); + t.is( + resolveFilePath(testFile, lowerBase).toLowerCase(), + testFile.toLowerCase(), + ); + t.is( + resolveFilePath(testFile, upperBase).toLowerCase(), + testFile.toLowerCase(), + ); + } finally { + rmSync(base, {recursive: true, force: true}); + } + }); +} diff --git a/source/utils/path-validation.ts b/source/utils/path-validation.ts index 7171fb306..bcb5d9f01 100644 --- a/source/utils/path-validation.ts +++ b/source/utils/path-validation.ts @@ -189,19 +189,59 @@ export function resolveFilePath( * fail when the workspace root is a drive or `/` (containers, root-owned * checkouts). See #1240. * + * Windows paths are case-insensitive, so path comparisons are normalized to + * lowercase on win32 to prevent false containment failures when drive letters + * or directories differ only in case. Note that purely lexical comparisons + * intentionally assume standard Windows case-insensitivity and do not account + * for per-directory case sensitivity (e.g. NTFS case-sensitive flag via WSL). + * * This is purely lexical — it never touches the filesystem, so a symlink * inside the root can still point outside it. Use `isRealPathInside` when the * path may be attacker- or config-controlled. */ -export function isPathInside(target: string, root: string): boolean { - const normalizedRoot = path.resolve(root); - const normalizedTarget = path.resolve(target); +/** + * Normalizes a path for comparison: resolves it to an absolute path, + * and on Windows (win32) lowercases it because Windows filesystems + * are case-insensitive by default. + * + * Note: Lexical normalization explicitly excludes NTFS per-directory + * case-sensitivity modes (e.g. `fsutil file setCaseSensitiveInfo`). For + * filesystem-authoritative trust boundaries where case-sensitive directories + * may be present on Windows, callers must use `isRealPathInside`. + * + * @param p - The path to normalize. + * @param platform - Test seam; production code must leave this as the default (`process.platform`). + */ +export function normalizeForCompare( + p: string, + platform: NodeJS.Platform = process.platform, +): string { + const resolved = path.resolve(p); // nosemgrep + return platform === 'win32' ? resolved.toLowerCase() : resolved; +} + +/** + * Lexical containment check with platform-aware case sensitivity. + * + * @param target - The path being checked. + * @param root - The containing root directory. + * @param platform - Test seam; production callers must leave this as the default (`process.platform`). + */ +export function isPathInside( + target: string, + root: string, + platform: NodeJS.Platform = process.platform, +): boolean { + const normalizedRoot = normalizeForCompare(root, platform); + const normalizedTarget = normalizeForCompare(target, platform); if (normalizedTarget === normalizedRoot) { return true; } - const prefix = normalizedRoot.endsWith(path.sep) - ? normalizedRoot - : normalizedRoot + path.sep; + const sep = platform === 'win32' ? '\\' : path.sep; + const prefix = + normalizedRoot.endsWith(sep) || normalizedRoot.endsWith(path.sep) + ? normalizedRoot + : normalizedRoot + sep; return normalizedTarget.startsWith(prefix); }