From b4d2817e67d7fbc252557dd3f31575b8d0178a58 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 14 May 2026 10:21:07 -0400 Subject: [PATCH] fix(tests): require trailing slash in path-replacement homedir check (#3503) The check at tests/path-replacement.test.cjs:163 used a naive content.includes(normalizedHomedir) to detect resolved homedir leaks in installed .md files. When os.homedir() is short (e.g. /root inside a Docker container), the substring false-matches inside ordinary tokens such as `` in agents/gsd-debug-session-manager.md, producing spurious failures with no actual path leak. Real path leaks are always followed by a path separator, so require `normalizedHomedir + '/'` instead. Extracted the predicate into a testable `containsResolvedHomedir` helper and added regression tests covering the /root case, a genuine /home/alice leak, /root followed by an actual separator, and the $HOME placeholder short-circuit. Fixes #3503 Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/path-replacement.test.cjs | 37 ++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/path-replacement.test.cjs b/tests/path-replacement.test.cjs index 4c0bebc62..95eb2622e 100644 --- a/tests/path-replacement.test.cjs +++ b/tests/path-replacement.test.cjs @@ -30,6 +30,17 @@ function computePathPrefix(homedir, targetDir) { return resolvedTarget + '/'; } +// Detect whether `content` leaks a resolved absolute homedir path (e.g. +// /home/alice or /root). A bare substring match false-positives when homedir +// is short and happens to appear inside ordinary words or tags — for example +// `` when os.homedir() === '/root' (Docker). Real path +// leaks are followed by a path separator, so we require a trailing '/'. +// See #3503. +function containsResolvedHomedir(content, normalizedHomedir) { + if (!normalizedHomedir || normalizedHomedir === '$HOME') return false; + return content.includes(normalizedHomedir + '/'); +} + describe('pathPrefix computation', () => { test('default Claude global install uses $HOME/', () => { const homedir = os.homedir(); @@ -160,10 +171,34 @@ describe('installed .md files contain no resolved absolute paths', () => { let content = fs.readFileSync(file, 'utf8'); content = content.replace(claudeDirRegex, pathPrefix); content = content.replace(claudeHomeRegex, pathPrefix); - if (content.includes(normalizedHomedir) && normalizedHomedir !== '$HOME') { + if (containsResolvedHomedir(content, normalizedHomedir)) { failures.push(path.relative(repoRoot, file)); } } assert.deepStrictEqual(failures, [], `Files with resolved absolute paths: ${failures.join(', ')}`); }); }); + +describe('containsResolvedHomedir predicate (#3503)', () => { + test('flags a real homedir path leak with trailing slash', () => { + const content = 'see /home/alice/.claude/config for details'; + assert.strictEqual(containsResolvedHomedir(content, '/home/alice'), true); + }); + + test('does NOT flag short homedir appearing as substring of an identifier (#3503)', () => { + // Regression: in Docker, os.homedir() === '/root'. Agent markdown contains + // `` / `` tags. The old naive + // substring check false-fired on these. The trailing-slash rule fixes it. + const content = '\nfoo\n'; + assert.strictEqual(containsResolvedHomedir(content, '/root'), false); + }); + + test('still flags /root when followed by a real path separator', () => { + const content = 'cat /root/.claude/agents.md'; + assert.strictEqual(containsResolvedHomedir(content, '/root'), true); + }); + + test('returns false for $HOME placeholder', () => { + assert.strictEqual(containsResolvedHomedir('$HOME/.claude/', '$HOME'), false); + }); +});