fix(memory): match nested directories on path boundaries, not name prefixes (#1974)

* fix(memory): match nested directories on path boundaries, not name prefixes

getDirectoriesToProcess documents nestedDirs as "Directories between CWD and
targetPath", but tested containment with currentDir.startsWith(originalCwd).
A sibling whose name merely begins with the CWD's name satisfies that: with cwd
/work/myapp, reading /work/myapp-backend/src/a.ts collected /work/myapp-backend
and its subdirectory, so their CLAUDE.md loaded as Project memory. Renaming the
directory to /work/backend loads nothing — same layout and same permission
grant, different behavior purely because of how the name is spelled.

Route the check through pathInWorkingPath, the helper already used for path
containment elsewhere in this file's module graph, so the comparison happens on
path boundaries.

* test(memory): build nested-dir fixtures with path helpers for Windows

getDirectoriesToProcess resolves its inputs, so on Windows the outputs carry a
drive letter and backslashes; hardcoded POSIX fixture strings would never match
and the walk-to-CWD comparison could not terminate. Construct every fixture and
expectation with resolve/join so they follow the platform.

* fix(memory): use native case semantics for the nested-directory check

pathInWorkingPath case-folds both operands on every platform so that
case-variant spellings cannot slip past a permission check. That is the
wrong direction for memory traversal: on a case-sensitive filesystem
/work/MyApp and /work/myapp are two unrelated projects, and folding them
together made getDirectoriesToProcess('/work/myapp/src/a.ts', '/work/MyApp')
return the /work/myapp ancestors, loading the other project's
CLAUDE.md/AGENTS.md as nested project memory.

Use a local boundary check built on relative(), which keeps the platform's
native case semantics while still comparing on path boundaries rather than
string prefixes.

* fix(memory): compare the relative path on segment boundaries

The containment check used rel.startsWith('..'), which is the same
string-prefix mistake this PR set out to fix: a directory legitimately named
'..hello' yields the relative path '..hello', so a genuinely nested
directory was dropped and its CLAUDE.md never loaded.

Match '..' exactly or followed by a separator instead, and add a regression
for the dotted-name case.

Also skip the case-variant assertion on Windows: path comparison there is
case-insensitive, so /work/MyApp and /work/myapp really are the same
directory and treating them as nested is correct.

* fix(memory): keep directory containment case-faithful on Windows

path.win32.relative() compares components case-insensitively, so it
returns "src" for C:\\work\\MyApp -> C:\\work\\myapp\\src. NTFS supports
per-directory case sensitivity, so those can be distinct project trees,
and the containment check would load the other project's CLAUDE.md and
rules as nested memory for a session rooted at the first.

Rebuild the child from the parent and compare exactly: the boundary logic
relative() provides is kept, the lexical case distinction is restored. The
path implementation is injectable so the Windows semantics are covered on
every host rather than skipped outside Windows.
This commit is contained in:
0xfandom
2026-07-23 07:17:47 +08:00
committed by GitHub
parent df85369b60
commit 0ff1d1cb7b
2 changed files with 175 additions and 3 deletions
+116
View File
@@ -0,0 +1,116 @@
import { describe, expect, test } from 'bun:test'
import { join, posix, resolve, win32 } from 'path'
import { getDirectoriesToProcess, isPathUnder } from './attachments.js'
// Build every fixture with node:path so drive letters and separators match what
// the implementation's own resolve()/dirname() produce on Windows as well.
const WORK = resolve('/work')
const CWD = join(WORK, 'myapp')
const PREFIXED_SIBLING = join(WORK, 'myapp-backend')
const PLAIN_SIBLING = join(WORK, 'backend')
describe('getDirectoriesToProcess', () => {
test('does not treat a name-prefixed sibling as nested under the CWD', () => {
// `/work/myapp-backend` is a sibling of the CWD, not a directory "between
// CWD and targetPath". A string-prefix test accepted it because the name
// starts with "myapp", so its CLAUDE.md was loaded as Project memory —
// e.g. `cd /work/myapp && claude --add-dir ../myapp-backend`.
const { nestedDirs } = getDirectoriesToProcess(
join(PREFIXED_SIBLING, 'src', 'a.ts'),
CWD,
)
expect(nestedDirs).toEqual([])
})
test('treats sibling directories the same regardless of their name', () => {
// The only difference here is spelling: `backend` shares no prefix with the
// CWD's name while `myapp-backend` does. Both are siblings, so both must
// behave identically.
const prefixed = getDirectoriesToProcess(
join(PREFIXED_SIBLING, 'src', 'a.ts'),
CWD,
).nestedDirs
const unprefixed = getDirectoriesToProcess(
join(PLAIN_SIBLING, 'src', 'a.ts'),
CWD,
).nestedDirs
expect(prefixed).toEqual(unprefixed)
})
test('still collects directories genuinely nested under the CWD', () => {
const { nestedDirs } = getDirectoriesToProcess(
join(CWD, 'src', 'deep', 'a.ts'),
CWD,
)
expect(nestedDirs).toEqual([join(CWD, 'src'), join(CWD, 'src', 'deep')])
})
test('does not treat a case-variant sibling as nested', () => {
// On a case-sensitive filesystem /work/MyApp and /work/myapp are two
// unrelated projects. A case-folding containment check would merge them and
// load the other project's CLAUDE.md/AGENTS.md as nested project memory.
const caseVariant = join(WORK, 'MyApp')
const { nestedDirs } = getDirectoriesToProcess(
join(CWD, 'src', 'a.ts'),
caseVariant,
)
expect(nestedDirs).not.toContain(CWD)
expect(nestedDirs).not.toContain(join(CWD, 'src'))
})
test('collects a nested directory whose name begins with dots', () => {
// `relative()` returns "..hello" here, which begins with ".." without being
// an upward traversal — a string-prefix check would drop it.
const dotted = join(CWD, '..hello')
const { nestedDirs } = getDirectoriesToProcess(join(dotted, 'a.ts'), CWD)
expect(nestedDirs).toContain(dotted)
})
test('reports directories from the root down to the CWD', () => {
const { cwdLevelDirs } = getDirectoriesToProcess(
join(CWD, 'src', 'a.ts'),
CWD,
)
expect(cwdLevelDirs).toEqual([WORK, CWD])
})
})
// The Windows semantics are driven through the injected path implementation so
// they run on every host: path.win32.relative() compares components
// case-insensitively, and NTFS supports per-directory case sensitivity, so a
// case-variant spelling there can be a genuinely different project tree.
describe('isPathUnder', () => {
test('keeps case distinct under Windows path semantics', () => {
expect(isPathUnder('C:\\work\\myapp\\src', 'C:\\work\\MyApp', win32)).toBe(
false,
)
expect(isPathUnder('C:\\work\\MyApp\\src', 'C:\\work\\MyApp', win32)).toBe(
true,
)
})
test('keeps case distinct under POSIX path semantics', () => {
expect(isPathUnder('/work/myapp/src', '/work/MyApp', posix)).toBe(false)
expect(isPathUnder('/work/myapp/src', '/work/myapp', posix)).toBe(true)
})
test('matches on boundaries, not string prefixes, on both platforms', () => {
for (const [api, child, parent, dotted, dottedParent] of [
[posix, '/work/myapp-backend/src', '/work/myapp', '/work/myapp/..hello', '/work/myapp'],
[
win32,
'C:\\work\\myapp-backend\\src',
'C:\\work\\myapp',
'C:\\work\\myapp\\..hello',
'C:\\work\\myapp',
],
] as const) {
expect(isPathUnder(child, parent, api)).toBe(false)
// `..hello` yields a relative path starting with ".." without being an
// upward traversal.
expect(isPathUnder(dotted, dottedParent, api)).toBe(true)
expect(isPathUnder(parent, parent, api)).toBe(false)
expect(isPathUnder(parent, child, api)).toBe(false)
}
})
})
+59 -3
View File
@@ -45,7 +45,7 @@ import {
getConditionalRulesForCwdLevelDirectory,
type MemoryFileInfo,
} from './claudemd.js'
import { dirname, parse, relative, resolve } from 'path'
import nodePath, { dirname, parse, relative, resolve } from 'path'
import { getCwd } from 'src/utils/cwd.js'
import { getViewedTeammateTask } from '../state/selectors.js'
import { logError } from './log.js'
@@ -1756,6 +1756,57 @@ async function getSelectedLinesFromIDE(
* @param originalCwd The original current working directory
* @returns Object with nestedDirs and cwdLevelDirs arrays, both ordered from parent to child
*/
/**
* True when `child` sits strictly beneath `parent`, compared on path
* boundaries.
*
* Deliberately not the permissions predicate (`pathInWorkingPath`): that one
* case-folds both operands on every platform to stop case-variant spellings
* from slipping past a security check. Applying it here would go the wrong way
* — on a case-sensitive filesystem `/work/MyApp` and `/work/myapp` are two
* unrelated projects, and folding them together would load the other project's
* CLAUDE.md/AGENTS.md as nested memory.
*
* `relative()` gets the boundary right but is not case-faithful on Windows: it
* compares components case-insensitively, so `C:\work\MyApp` -> `C:\work\myapp\src`
* returns `src` as though it were nested. NTFS supports per-directory case
* sensitivity, so those can be distinct trees there too. Rebuilding the child
* from the parent and comparing exactly keeps the boundary logic while
* restoring the lexical case distinction.
*
* `pathApi` is injectable so the Windows semantics can be covered from any
* host. Exported for testing.
*/
export function isPathUnder(
child: string,
parent: string,
pathApi: Pick<
typeof nodePath,
'relative' | 'join' | 'normalize' | 'isAbsolute' | 'sep'
> = nodePath,
): boolean {
const rel = pathApi.relative(parent, child)
// Compare on segment boundaries, not a string prefix: a directory legitimately
// named `..hello` yields the relative path `..hello`, which starts with `..`
// without being an upward traversal.
if (
rel === '' ||
rel === '..' ||
rel.startsWith('..' + pathApi.sep) ||
pathApi.isAbsolute(rel)
) {
return false
}
const stripTrailingSep = (value: string): string =>
value.length > 1 && value.endsWith(pathApi.sep)
? value.slice(0, -pathApi.sep.length)
: value
return (
stripTrailingSep(pathApi.join(parent, rel)) ===
stripTrailingSep(pathApi.normalize(child))
)
}
export function getDirectoriesToProcess(
targetPath: string,
originalCwd: string,
@@ -1765,9 +1816,14 @@ export function getDirectoriesToProcess(
const nestedDirs: string[] = []
let currentDir = targetDir
// Walk up from target directory to original CWD
// Walk up from target directory to original CWD.
// Containment must be tested on path boundaries, not string prefixes: a
// sibling whose name merely starts with the CWD's name (cwd `/work/myapp`,
// target in `/work/myapp-backend`) is not "between CWD and targetPath", but
// startsWith accepts it — so its CLAUDE.md loaded as Project memory purely
// because of how the directory happened to be spelled.
while (currentDir !== originalCwd && currentDir !== parse(currentDir).root) {
if (currentDir.startsWith(originalCwd)) {
if (isPathUnder(currentDir, originalCwd)) {
nestedDirs.push(currentDir)
}
currentDir = dirname(currentDir)