Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/windows-path-case-sensitivity.md
Original file line number Diff line number Diff line change
@@ -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.
7 changes: 7 additions & 0 deletions .github/workflows/pr-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
57 changes: 57 additions & 0 deletions source/config/preferences.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down
20 changes: 13 additions & 7 deletions source/config/preferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
);
}

Expand Down Expand Up @@ -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
);
});
Expand All @@ -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';
}
Expand Down Expand Up @@ -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});
Expand Down
18 changes: 5 additions & 13 deletions source/daemon/full-commit-trust.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
45 changes: 45 additions & 0 deletions source/utils/path-validation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
isPathInside,
isRealPathInside,
isValidFilePath,
normalizeForCompare,
resolveFilePath,
} from './path-validation';

Expand Down Expand Up @@ -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});
}
});
}
52 changes: 46 additions & 6 deletions source/utils/path-validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L213: 🟡 risk: Lowercasing every Win32 path collapses distinct entries in case-sensitive NTFS directories (Windows supports per-directory case sensitivity: https://learn.microsoft.com/en-us/windows/wsl/case-sensitivity), so a sibling can pass this trusted-root check. Does Nanocoder explicitly exclude that filesystem mode? If so, please document it; otherwise add a guard/regression so the trust boundary preserves actual filesystem semantics. Codex-assisted review; checked exact head 2318183.

}

/**
* 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);
}

Expand Down
Loading