From f20a0048c79668cfa6940fdaa37b8ae0d0ab5ca7 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 30 Jul 2026 15:32:55 -0400 Subject: [PATCH 1/2] fix(security): contain external repo file indexing --- Dockerfile | 9 + server/src/addie/mcp/external-repos.ts | 366 ++++++++++++++++-- .../unit/external-repos-security.test.ts | 359 +++++++++++++++++ 3 files changed, 699 insertions(+), 35 deletions(-) create mode 100644 server/tests/unit/external-repos-security.test.ts diff --git a/Dockerfile b/Dockerfile index 265febd85e..d80c0cdb0e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -112,6 +112,15 @@ if [ "$remaining" -gt 0 ]; then echo "ERROR: $remaining .git dirs remain after stripping" >&2 exit 1 fi +# Git repositories can contain tracked symbolic links. Remove every link before +# the cache is copied into the runtime image; runtime indexing independently +# enforces the same no-link boundary. +find /repos -type l -exec rm -f {} + +remaining_links=$(find /repos -type l | wc -l) +if [ "$remaining_links" -gt 0 ]; then + echo "ERROR: $remaining_links symbolic links remain after sanitizing" >&2 + exit 1 +fi rm /repos/clone.sh # Only markdown/MDX files are indexed by search_repos at runtime. Keep this # in sync with EXTERNAL_REPOS indexPatterns if non-markdown sources are added. diff --git a/server/src/addie/mcp/external-repos.ts b/server/src/addie/mcp/external-repos.ts index 369313be1c..394d448b8b 100644 --- a/server/src/addie/mcp/external-repos.ts +++ b/server/src/addie/mcp/external-repos.ts @@ -342,6 +342,33 @@ let initialized = false; // Directory where repos are cached let reposDir: string; +// External repositories are untrusted input. Keep individual documents +// bounded so a repository cannot exhaust memory while the in-memory index is +// built. The descriptor reader below also enforces this limit if a file grows +// after it is opened. +const MAX_EXTERNAL_DOC_BYTES = 4 * 1024 * 1024; +const FILE_READ_CHUNK_BYTES = 64 * 1024; + +interface TrustedRepoRoot { + path: string; + realPath: string; + dev: number; + ino: number; +} + +interface InspectedRepoFile { + path: string; + realPath: string; + stats: fs.Stats; +} + +class UnsafeExternalRepoPathError extends Error { + constructor(message: string) { + super(message); + this.name = 'UnsafeExternalRepoPathError'; + } +} + /** * Get the repos cache directory */ @@ -454,59 +481,313 @@ function syncRepo(repo: ExternalRepo): string | null { } } +function isContainedPath(rootPath: string, candidatePath: string, allowRoot: boolean): boolean { + const relativePath = path.relative(rootPath, candidatePath); + if (relativePath === '') return allowRoot; + return relativePath !== '..' + && !relativePath.startsWith(`..${path.sep}`) + && !path.isAbsolute(relativePath); +} + +function createTrustedRepoRoot(repoPath: string): TrustedRepoRoot { + const absolutePath = path.resolve(repoPath); + const rootStats = fs.lstatSync(absolutePath); + if (rootStats.isSymbolicLink() || !rootStats.isDirectory()) { + throw new UnsafeExternalRepoPathError('Repository root must be a non-symlink directory'); + } + + const realPath = fs.realpathSync.native(absolutePath); + const realStats = fs.lstatSync(realPath); + if (!realStats.isDirectory() + || realStats.dev !== rootStats.dev + || realStats.ino !== rootStats.ino) { + throw new UnsafeExternalRepoPathError('Repository root changed during validation'); + } + + return { path: absolutePath, realPath, dev: rootStats.dev, ino: rootStats.ino }; +} + +function hasNestedGitBoundary(repoRoot: TrustedRepoRoot, directoryPath: string): boolean { + if (path.resolve(directoryPath) === repoRoot.path) return false; + + try { + return fs.lstatSync(path.join(directoryPath, '.git'), { throwIfNoEntry: false }) !== undefined; + } catch { + // A marker that cannot be inspected is not safe to cross. + return true; + } +} + +function inspectRepoPath( + repoRoot: TrustedRepoRoot, + candidatePath: string, + expectedType: 'directory' | 'file' +): InspectedRepoFile { + const currentRootStats = fs.lstatSync(repoRoot.path); + if (currentRootStats.isSymbolicLink() + || !currentRootStats.isDirectory() + || currentRootStats.dev !== repoRoot.dev + || currentRootStats.ino !== repoRoot.ino) { + throw new UnsafeExternalRepoPathError('Repository root changed during validation'); + } + + const absolutePath = path.resolve(candidatePath); + if (!isContainedPath(repoRoot.path, absolutePath, expectedType === 'directory')) { + throw new UnsafeExternalRepoPathError('Path escapes the repository root'); + } + + const relativePath = path.relative(repoRoot.path, absolutePath); + const components = relativePath === '' ? [] : relativePath.split(path.sep); + let currentPath = repoRoot.path; + let finalStats: fs.Stats | undefined; + + for (let index = 0; index < components.length; index++) { + const component = components[index]; + if (component.toLowerCase() === '.git') { + throw new UnsafeExternalRepoPathError('Git metadata is not indexable'); + } + + currentPath = path.join(currentPath, component); + const stats = fs.lstatSync(currentPath); + const isFinalComponent = index === components.length - 1; + + if (stats.isSymbolicLink()) { + throw new UnsafeExternalRepoPathError('Symbolic links are not indexable'); + } + + if (!isFinalComponent) { + if (!stats.isDirectory()) { + throw new UnsafeExternalRepoPathError('Path ancestor must be a directory'); + } + if (hasNestedGitBoundary(repoRoot, currentPath)) { + throw new UnsafeExternalRepoPathError('Nested Git repositories are not indexable'); + } + continue; + } + + if (expectedType === 'directory') { + if (!stats.isDirectory()) { + throw new UnsafeExternalRepoPathError('Expected a directory'); + } + if (hasNestedGitBoundary(repoRoot, currentPath)) { + throw new UnsafeExternalRepoPathError('Nested Git repositories are not indexable'); + } + } else { + if (!stats.isFile()) { + throw new UnsafeExternalRepoPathError('Expected a regular file'); + } + if (stats.nlink !== 1) { + throw new UnsafeExternalRepoPathError('Hard-linked files are not indexable'); + } + if (stats.size < 0 || stats.size > MAX_EXTERNAL_DOC_BYTES) { + throw new UnsafeExternalRepoPathError('External repository document exceeds the size limit'); + } + } + + finalStats = stats; + } + + if (components.length === 0) { + if (expectedType !== 'directory') { + throw new UnsafeExternalRepoPathError('Repository root is not an indexable file'); + } + finalStats = fs.lstatSync(repoRoot.path); + } + + const realPath = fs.realpathSync.native(absolutePath); + if (!isContainedPath(repoRoot.realPath, realPath, expectedType === 'directory')) { + throw new UnsafeExternalRepoPathError('Resolved path escapes the repository root'); + } + + return { path: absolutePath, realPath, stats: finalStats! }; +} + +function tryInspectRepoPath( + repoRoot: TrustedRepoRoot, + candidatePath: string, + expectedType: 'directory' | 'file' +): InspectedRepoFile | null { + try { + return inspectRepoPath(repoRoot, candidatePath, expectedType); + } catch { + return null; + } +} + +function readBoundedFileDescriptor(fd: number): string { + const chunks: Buffer[] = []; + let totalBytes = 0; + + while (totalBytes <= MAX_EXTERNAL_DOC_BYTES) { + const remainingBytes = MAX_EXTERNAL_DOC_BYTES + 1 - totalBytes; + const buffer = Buffer.allocUnsafe(Math.min(FILE_READ_CHUNK_BYTES, remainingBytes)); + const bytesRead = fs.readSync(fd, buffer, 0, buffer.length, null); + if (bytesRead === 0) { + return Buffer.concat(chunks, totalBytes).toString('utf8'); + } + + totalBytes += bytesRead; + if (totalBytes > MAX_EXTERNAL_DOC_BYTES) { + throw new UnsafeExternalRepoPathError('External repository document exceeds the size limit'); + } + chunks.push(buffer.subarray(0, bytesRead)); + } + + throw new UnsafeExternalRepoPathError('External repository document exceeds the size limit'); +} + +function readContainedRegularFile(repoRoot: TrustedRepoRoot, candidatePath: string): string { + const inspected = inspectRepoPath(repoRoot, candidatePath, 'file'); + const noFollowFlag = fs.constants.O_NOFOLLOW; + if (typeof noFollowFlag !== 'number') { + throw new UnsafeExternalRepoPathError('This platform cannot safely open untrusted repository files'); + } + + let fd: number | undefined; + try { + fd = fs.openSync(inspected.realPath, fs.constants.O_RDONLY | noFollowFlag); + const openedStats = fs.fstatSync(fd); + if (!openedStats.isFile() + || openedStats.nlink !== 1 + || openedStats.dev !== inspected.stats.dev + || openedStats.ino !== inspected.stats.ino + || openedStats.size < 0 + || openedStats.size > MAX_EXTERNAL_DOC_BYTES) { + throw new UnsafeExternalRepoPathError('Repository file changed during validation'); + } + + return readBoundedFileDescriptor(fd); + } finally { + if (fd !== undefined) { + fs.closeSync(fd); + } + } +} + +function isSafeRelativePattern(pattern: string): boolean { + if (!pattern + || pattern.includes('\0') + || pattern.includes('\\') + || path.posix.isAbsolute(pattern) + || /^[a-zA-Z]:\//.test(pattern)) { + return false; + } + + const components = pattern.split('/'); + if (components.some((component) => component === '' + || component === '.' + || component === '..' + || component.toLowerCase() === '.git')) { + return false; + } + + const recursiveIndexes = components + .map((component, index) => component === '**' ? index : -1) + .filter((index) => index >= 0); + if (recursiveIndexes.length > 1) return false; + + const basename = components[components.length - 1]; + const isSupportedBasename = !basename.includes('*') + || basename === '*' + || (/^\*\.[^*]+$/.test(basename)); + if (!isSupportedBasename) return false; + + return components.every((component, index) => { + if (!component.includes('*')) return true; + if (component === '**') { + return index === components.length - 2; + } + return index === components.length - 1; + }); +} + /** * Find files matching glob patterns in a directory * Simple implementation without glob library dependency */ -function findMatchingFiles(baseDir: string, patterns: string[]): string[] { - const files: string[] = []; +function findMatchingFiles(repoRoot: TrustedRepoRoot, patterns: string[]): string[] { + const files = new Set(); for (const pattern of patterns) { - if (pattern.includes('**')) { - // Recursive pattern like 'docs/**/*.md' - const [prefix, suffix] = pattern.split('**'); - const searchDir = path.join(baseDir, prefix.replace(/\/$/, '')); - if (fs.existsSync(searchDir)) { - findFilesRecursive(searchDir, suffix.replace(/^\//, ''), files); - } - } else if (pattern.includes('*')) { - // Simple glob like '*.md' - const dir = path.dirname(pattern); - const filePattern = path.basename(pattern); - const searchDir = dir === '.' ? baseDir : path.join(baseDir, dir); - if (fs.existsSync(searchDir)) { - const entries = fs.readdirSync(searchDir); - for (const entry of entries) { - if (matchesPattern(entry, filePattern)) { - files.push(path.join(searchDir, entry)); + if (!isSafeRelativePattern(pattern)) { + logger.warn({ pattern }, 'Addie External Repos: Ignoring unsafe index pattern'); + continue; + } + + try { + if (pattern.includes('**')) { + // Recursive pattern like 'docs/**/*.md' + const [prefix, suffix] = pattern.split('**'); + const relativePrefix = prefix.replace(/\/$/, ''); + const searchDir = relativePrefix === '' + ? repoRoot.path + : path.resolve(repoRoot.path, ...relativePrefix.split('/')); + if (tryInspectRepoPath(repoRoot, searchDir, 'directory')) { + findFilesRecursive(repoRoot, searchDir, suffix.replace(/^\//, ''), files); + } + } else if (pattern.includes('*')) { + // Simple glob like '*.md' + const components = pattern.split('/'); + const filePattern = components.pop()!; + const searchDir = components.length === 0 + ? repoRoot.path + : path.resolve(repoRoot.path, ...components); + if (tryInspectRepoPath(repoRoot, searchDir, 'directory')) { + const entries = fs.readdirSync(searchDir, { withFileTypes: true }); + for (const entry of entries) { + if (entry.isFile() && matchesPattern(entry.name, filePattern)) { + const filePath = path.join(searchDir, entry.name); + if (tryInspectRepoPath(repoRoot, filePath, 'file')) { + files.add(filePath); + } + } } } + } else { + // Exact file like 'README.md' + const filePath = path.resolve(repoRoot.path, ...pattern.split('/')); + if (tryInspectRepoPath(repoRoot, filePath, 'file')) { + files.add(filePath); + } } - } else { - // Exact file like 'README.md' - const filePath = path.join(baseDir, pattern); - if (fs.existsSync(filePath) && fs.statSync(filePath).isFile()) { - files.push(filePath); - } + } catch (error) { + logger.warn( + { pattern, error }, + 'Addie External Repos: Skipping index pattern after filesystem change' + ); } } - return files; + return Array.from(files); } -function findFilesRecursive(dir: string, suffix: string, files: string[]): void { - if (!fs.existsSync(dir)) return; +function findFilesRecursive( + repoRoot: TrustedRepoRoot, + dir: string, + suffix: string, + files: Set +): void { + if (!tryInspectRepoPath(repoRoot, dir, 'directory')) return; - const entries = fs.readdirSync(dir, { withFileTypes: true }); + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; + } for (const entry of entries) { const fullPath = path.join(dir, entry.name); if (entry.isDirectory()) { - if (!entry.name.startsWith('.') && entry.name !== 'node_modules') { - findFilesRecursive(fullPath, suffix, files); + if (!entry.name.startsWith('.') + && entry.name !== 'node_modules' + && entry.name.toLowerCase() !== '.git') { + findFilesRecursive(repoRoot, fullPath, suffix, files); } } else if (entry.isFile()) { if (matchesPattern(entry.name, suffix)) { - files.push(fullPath); + if (tryInspectRepoPath(repoRoot, fullPath, 'file')) { + files.add(fullPath); + } } } } @@ -725,12 +1006,20 @@ function indexRepo( const headings: IndexedExternalHeading[] = []; const patterns = repo.indexPatterns || ['README.md', 'docs/**/*.md']; - const files = findMatchingFiles(repoPath, patterns); + let repoRoot: TrustedRepoRoot; + try { + repoRoot = createTrustedRepoRoot(repoPath); + } catch (error) { + logger.warn({ repoId: repo.id, repoPath, error }, 'Addie External Repos: Unsafe repository root'); + return { docs, headings }; + } + + const files = findMatchingFiles(repoRoot, patterns); for (const filePath of files) { try { - const content = fs.readFileSync(filePath, 'utf-8'); - const relativePath = path.relative(repoPath, filePath); + const content = readContainedRegularFile(repoRoot, filePath); + const relativePath = path.relative(repoRoot.path, filePath); const filename = path.basename(filePath); const title = extractTitle(content, filename); const cleanedContent = cleanContent(content); @@ -1122,3 +1411,10 @@ export function getExternalRepoStats(): Array<{ id: string; name: string; docCou export function getConfiguredRepos(): ExternalRepo[] { return [...EXTERNAL_REPOS]; } + +/** @internal Exported for focused filesystem boundary tests only. */ +export const _testing = { + indexRepo, + isSafeRelativePattern, + MAX_EXTERNAL_DOC_BYTES, +}; diff --git a/server/tests/unit/external-repos-security.test.ts b/server/tests/unit/external-repos-security.test.ts new file mode 100644 index 0000000000..9344a3153c --- /dev/null +++ b/server/tests/unit/external-repos-security.test.ts @@ -0,0 +1,359 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { + _testing, + type ExternalRepo, + type IndexedExternalDoc, + type IndexedExternalHeading, +} from '../../src/addie/mcp/external-repos.js'; + +const SECRET_SENTINEL = 'EXTERNAL_REPO_LOCAL_SECRET_SENTINEL'; + +type IndexResult = { + docs: IndexedExternalDoc[]; + headings: IndexedExternalHeading[]; +}; + +let tempDir: string; +let repoDir: string; +let outsideDir: string; +let secretPath: string; + +function markdown(title: string, marker = 'ordinary repository content'): string { + return `# ${title}\n\n## Details\n\n${marker} ${'safe searchable text '.repeat(8)}`; +} + +function writeRepoFile(relativePath: string, content = markdown(relativePath)): string { + const filePath = path.join(repoDir, relativePath); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, content); + return filePath; +} + +function writeOutsideFile(relativePath: string, content = markdown('Secret', SECRET_SENTINEL)): string { + const filePath = path.join(outsideDir, relativePath); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, content); + return filePath; +} + +function config(indexPatterns: string[]): ExternalRepo { + return { + id: 'security-fixture', + url: 'https://github.com/example/security-fixture.git', + name: 'Security fixture', + description: 'Filesystem security fixture', + branch: 'main', + indexPatterns, + }; +} + +function index(indexPatterns: string[], root = repoDir): IndexResult { + return _testing.indexRepo(config(indexPatterns), root); +} + +function indexedText(result: IndexResult): string { + return JSON.stringify({ docs: result.docs, headings: result.headings }); +} + +function expectSecretAbsent(result: IndexResult): void { + expect(indexedText(result)).not.toContain(SECRET_SENTINEL); +} + +function relativeSymlinkTarget(linkPath: string, targetPath: string): string { + return path.relative(path.dirname(linkPath), targetPath); +} + +beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'external-repos-security-')); + repoDir = path.join(tempDir, 'repo'); + outsideDir = path.join(tempDir, 'outside'); + fs.mkdirSync(repoDir); + fs.mkdirSync(outsideDir); + secretPath = writeOutsideFile('secret.md'); +}); + +afterEach(() => { + vi.restoreAllMocks(); + fs.rmSync(tempDir, { recursive: true, force: true }); +}); + +describe('external repository filesystem boundary', () => { + describe.each([ + { name: 'exact', link: 'README.md', valid: 'VALID.md', patterns: ['README.md', 'VALID.md'] }, + { name: 'root glob', link: 'leak.md', valid: 'valid.md', patterns: ['*.md'] }, + { name: 'recursive', link: 'docs/leak.md', valid: 'docs/valid.md', patterns: ['docs/**/*.md'] }, + ])('$name discovery', ({ link, valid, patterns }) => { + it.each(['relative', 'absolute'] as const)( + 'rejects a %s leaf symlink without aborting a valid sibling', + (targetKind) => { + const linkPath = path.join(repoDir, link); + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + fs.symlinkSync( + targetKind === 'relative' ? relativeSymlinkTarget(linkPath, secretPath) : secretPath, + linkPath, + ); + writeRepoFile(valid); + + const result = index(patterns); + + expect(result.docs.map((doc) => doc.path)).toEqual([valid]); + expectSecretAbsent(result); + }, + ); + }); + + it('rejects escaping root and nested directory symlinks', () => { + writeRepoFile('valid.md'); + fs.symlinkSync(relativeSymlinkTarget(path.join(repoDir, 'docs'), outsideDir), path.join(repoDir, 'docs')); + + const rootEscape = index(['docs/**/*.md', 'valid.md']); + expect(rootEscape.docs.map((doc) => doc.path)).toEqual(['valid.md']); + expectSecretAbsent(rootEscape); + + fs.unlinkSync(path.join(repoDir, 'docs')); + fs.mkdirSync(path.join(repoDir, 'docs')); + writeRepoFile('docs/valid.md'); + fs.symlinkSync( + relativeSymlinkTarget(path.join(repoDir, 'docs', 'a'), outsideDir), + path.join(repoDir, 'docs', 'a'), + ); + + const nestedEscape = index(['docs/**/*.md']); + expect(nestedEscape.docs.map((doc) => doc.path)).toEqual(['docs/valid.md']); + expectSecretAbsent(nestedEscape); + }); + + it('rejects in-tree links and link chains while indexing the ordinary target', () => { + const ordinaryPath = writeRepoFile('real/ordinary.md'); + fs.symlinkSync(relativeSymlinkTarget(path.join(repoDir, 'alias.md'), ordinaryPath), path.join(repoDir, 'alias.md')); + fs.symlinkSync('real', path.join(repoDir, 'alias-dir')); + fs.symlinkSync('chain-b.md', path.join(repoDir, 'chain-a.md')); + fs.symlinkSync(relativeSymlinkTarget(path.join(repoDir, 'chain-b.md'), secretPath), path.join(repoDir, 'chain-b.md')); + const outsideLink = path.join(outsideDir, 'outside-link.md'); + fs.symlinkSync('secret.md', outsideLink); + fs.symlinkSync(relativeSymlinkTarget(path.join(repoDir, 'outside-chain.md'), outsideLink), path.join(repoDir, 'outside-chain.md')); + + const result = index(['**/*.md']); + + expect(result.docs.map((doc) => doc.path)).toEqual(['real/ordinary.md']); + expectSecretAbsent(result); + }); + + it('skips broken and cyclic links without throwing or hanging', () => { + fs.symlinkSync('missing.md', path.join(repoDir, 'broken.md')); + fs.symlinkSync('cycle-b.md', path.join(repoDir, 'cycle-a.md')); + fs.symlinkSync('cycle-a.md', path.join(repoDir, 'cycle-b.md')); + writeRepoFile('valid.md'); + + const result = index(['*.md']); + + expect(result.docs.map((doc) => doc.path)).toEqual(['valid.md']); + expectSecretAbsent(result); + }); + + it('rejects outside and in-tree hardlinks', () => { + fs.linkSync(secretPath, path.join(repoDir, 'README.md')); + const target = writeRepoFile('target.md'); + fs.linkSync(target, path.join(repoDir, 'alias.md')); + writeRepoFile('valid.md'); + + const result = index(['README.md', '*.md']); + + expect(result.docs.map((doc) => doc.path)).toEqual(['valid.md']); + expectSecretAbsent(result); + }); + + it('does not cross nested Git repository or submodule boundaries', () => { + fs.mkdirSync(path.join(repoDir, '.git')); + writeRepoFile('valid.md'); + writeRepoFile('nested-file/README.md', markdown('Nested file marker', SECRET_SENTINEL)); + fs.writeFileSync(path.join(repoDir, 'nested-file', '.git'), 'gitdir: ../metadata'); + writeRepoFile('nested-dir/README.md', markdown('Nested directory marker', SECRET_SENTINEL)); + fs.mkdirSync(path.join(repoDir, 'nested-dir', '.git')); + fs.mkdirSync(path.join(repoDir, 'empty-gitlink')); + + const result = index(['**/*.md']); + + expect(result.docs.map((doc) => doc.path)).toEqual(['valid.md']); + expectSecretAbsent(result); + }); + + it('rejects traversal, absolute, dot, empty, and sibling-prefix patterns', () => { + writeRepoFile('valid.md'); + const siblingDir = path.join(tempDir, 'repo-evil'); + fs.mkdirSync(siblingDir); + fs.writeFileSync(path.join(siblingDir, 'secret.md'), markdown('Sibling secret', SECRET_SENTINEL)); + + const unsafePatterns = [ + '../outside/secret.md', + '../../outside/secret.md', + '../repo-evil/secret.md', + secretPath, + '.', + '', + 'docs/./guide.md', + 'nested/.git/README.md', + ]; + for (const pattern of unsafePatterns) { + expect(_testing.isSafeRelativePattern(pattern)).toBe(false); + } + + const result = index([...unsafePatterns, 'valid.md']); + expect(result.docs.map((doc) => doc.path)).toEqual(['valid.md']); + expectSecretAbsent(result); + }); + + it('rejects a repository root that is itself a symlink', () => { + writeRepoFile('README.md', markdown('Would otherwise index', SECRET_SENTINEL)); + const linkedRoot = path.join(tempDir, 'repo-link'); + fs.symlinkSync(repoDir, linkedRoot); + + const result = index(['README.md'], linkedRoot); + + expect(result.docs).toEqual([]); + expect(result.headings).toEqual([]); + expectSecretAbsent(result); + }); + + it('preserves valid exact, glob, recursive, MDX, Unicode, and metadata behavior', () => { + writeRepoFile( + 'README.md', + `---\ntitle: Fixture frontmatter title\n---\n\n## Overview\n\n${'frontmatter content '.repeat(8)}`, + ); + writeRepoFile('CHANGELOG.md'); + writeRepoFile('Root Guide.md'); + writeRepoFile('docs/a/b/Guide ünicode.mdx'); + writeRepoFile('.hidden/secret.md', markdown('Hidden', SECRET_SENTINEL)); + writeRepoFile('node_modules/secret.md', markdown('Dependency', SECRET_SENTINEL)); + + const result = index([ + 'README.md', + 'CHANGELOG.md', + '*.md', + 'docs/**/*.md', + 'docs/**/*.mdx', + 'README.md', + ]); + + expect(result.docs.map((doc) => doc.path).sort()).toEqual([ + 'CHANGELOG.md', + 'README.md', + 'Root Guide.md', + 'docs/a/b/Guide ünicode.mdx', + ]); + expect(result.docs.find((doc) => doc.path === 'README.md')?.title).toBe('Fixture frontmatter title'); + expect(result.docs.find((doc) => doc.path === 'README.md')?.id).toBe('external:security-fixture:README'); + expect(result.docs.find((doc) => doc.path === 'Root Guide.md')?.sourceUrl) + .toBe('https://github.com/example/security-fixture/blob/main/Root Guide.md'); + expect(result.headings.some((heading) => heading.path === 'README.md')).toBe(true); + expectSecretAbsent(result); + }); + + it('rejects oversized documents without aborting valid siblings', () => { + fs.writeFileSync( + path.join(repoDir, 'oversized.md'), + Buffer.alloc(_testing.MAX_EXTERNAL_DOC_BYTES + 1, 0x61), + ); + writeRepoFile('valid.md'); + + const result = index(['*.md']); + + expect(result.docs.map((doc) => doc.path)).toEqual(['valid.md']); + }); + + it('isolates a simple-glob readdir failure and continues with later patterns', () => { + writeRepoFile('README.md'); + writeRepoFile('valid.md'); + vi.spyOn(fs, 'readdirSync').mockImplementationOnce(() => { + throw Object.assign(new Error('directory disappeared'), { code: 'ENOENT' }); + }); + + const result = index(['*.md', 'valid.md']); + + expect(result.docs.map((doc) => doc.path)).toEqual(['valid.md']); + }); + + it('isolates a transient root revalidation failure and continues safely', () => { + writeRepoFile('README.md'); + writeRepoFile('valid.md'); + const originalLstatSync = fs.lstatSync.bind(fs); + let lstatCalls = 0; + vi.spyOn(fs, 'lstatSync').mockImplementation(((...args: Parameters) => { + lstatCalls += 1; + if (lstatCalls === 3) { + throw Object.assign(new Error('repository root disappeared'), { code: 'ENOENT' }); + } + return originalLstatSync(...args); + }) as typeof fs.lstatSync); + + const result = index(['README.md', 'valid.md']); + + expect(result.docs.map((doc) => doc.path)).toEqual(['valid.md']); + }); + + it('rejects a regular file swapped to an outside symlink before open', () => { + const readmePath = writeRepoFile('README.md'); + writeRepoFile('valid.md'); + const originalOpenSync = fs.openSync.bind(fs); + vi.spyOn(fs, 'openSync').mockImplementationOnce((filePath, flags, mode) => { + fs.unlinkSync(readmePath); + fs.symlinkSync(secretPath, readmePath); + return originalOpenSync(filePath, flags, mode); + }); + + const result = index(['README.md', 'valid.md']); + + expect(result.docs.map((doc) => doc.path)).toEqual(['valid.md']); + expectSecretAbsent(result); + }); + + it('rejects and closes a different regular inode swapped in before open', () => { + const readmePath = writeRepoFile('README.md'); + writeRepoFile('valid.md'); + const originalOpenSync = fs.openSync.bind(fs); + const originalCloseSync = fs.closeSync.bind(fs); + vi.spyOn(fs, 'openSync').mockImplementationOnce((filePath, flags, mode) => { + fs.unlinkSync(readmePath); + fs.copyFileSync(secretPath, readmePath); + return originalOpenSync(filePath, flags, mode); + }); + const closeSpy = vi.spyOn(fs, 'closeSync').mockImplementation(originalCloseSync); + + const result = index(['README.md', 'valid.md']); + + expect(result.docs.map((doc) => doc.path)).toEqual(['valid.md']); + expect(closeSpy).toHaveBeenCalledTimes(2); + expectSecretAbsent(result); + }); + + it('reads the validated descriptor if the pathname is replaced after open', () => { + const validatedMarker = 'VALIDATED_DESCRIPTOR_CONTENT'; + const readmePath = writeRepoFile('README.md', markdown('Validated', validatedMarker)); + const heldPath = path.join(repoDir, 'held-original.md'); + const originalOpenSync = fs.openSync.bind(fs); + vi.spyOn(fs, 'openSync').mockImplementationOnce((filePath, flags, mode) => { + const fd = originalOpenSync(filePath, flags, mode); + fs.renameSync(readmePath, heldPath); + fs.symlinkSync(secretPath, readmePath); + return fd; + }); + + const result = index(['README.md']); + + expect(result.docs).toHaveLength(1); + expect(result.docs[0].content).toContain(validatedMarker); + expectSecretAbsent(result); + }); + + it('skips special and non-file exact candidates without aborting', () => { + fs.mkdirSync(path.join(repoDir, 'README.md')); + writeRepoFile('valid.md'); + + const result = index(['README.md', 'valid.md']); + + expect(result.docs.map((doc) => doc.path)).toEqual(['valid.md']); + }); +}); From c8c841318100aeccddd1841f161a462b3bd7c94a Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 30 Jul 2026 17:07:17 -0400 Subject: [PATCH 2/2] test: make inode swap coverage deterministic --- server/tests/unit/external-repos-security.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/tests/unit/external-repos-security.test.ts b/server/tests/unit/external-repos-security.test.ts index 9344a3153c..2e212231e4 100644 --- a/server/tests/unit/external-repos-security.test.ts +++ b/server/tests/unit/external-repos-security.test.ts @@ -312,12 +312,13 @@ describe('external repository filesystem boundary', () => { it('rejects and closes a different regular inode swapped in before open', () => { const readmePath = writeRepoFile('README.md'); + const replacementPath = path.join(repoDir, 'replacement.md'); writeRepoFile('valid.md'); + fs.copyFileSync(secretPath, replacementPath); const originalOpenSync = fs.openSync.bind(fs); const originalCloseSync = fs.closeSync.bind(fs); vi.spyOn(fs, 'openSync').mockImplementationOnce((filePath, flags, mode) => { - fs.unlinkSync(readmePath); - fs.copyFileSync(secretPath, readmePath); + fs.renameSync(replacementPath, readmePath); return originalOpenSync(filePath, flags, mode); }); const closeSpy = vi.spyOn(fs, 'closeSync').mockImplementation(originalCloseSync);