fix: remove dead try/catch in insertNode; fix SENSITIVE_PATHS case-sensitivity (#327)
Drop the no-op try/catch around insertNode.run, and lowercase the Windows SENSITIVE_PATHS entries so validateProjectPath's case-insensitive check actually blocks c:\windows. Adds a validateProjectPath test (POSIX + Windows-gated); the Windows-gated case was validated on a real Windows 11 VM. Closes #327
This commit is contained in:
@@ -12,7 +12,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
|||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
import * as os from 'os';
|
import * as os from 'os';
|
||||||
import { FileLock } from '../src/utils';
|
import { FileLock, validateProjectPath } from '../src/utils';
|
||||||
import CodeGraph from '../src/index';
|
import CodeGraph from '../src/index';
|
||||||
import { ToolHandler, tools } from '../src/mcp/tools';
|
import { ToolHandler, tools } from '../src/mcp/tools';
|
||||||
import { scanDirectory, isSourceFile } from '../src/extraction';
|
import { scanDirectory, isSourceFile } from '../src/extraction';
|
||||||
@@ -176,6 +176,36 @@ describe('Path Traversal Prevention', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('validateProjectPath — sensitive directory blocking', () => {
|
||||||
|
// POSIX-only: on Windows '/etc' resolves to C:\etc (non-existent), not a
|
||||||
|
// sensitive dir — the Windows case is covered by the win32-gated test below.
|
||||||
|
it.runIf(process.platform !== 'win32')('blocks POSIX system directories (exact match)', () => {
|
||||||
|
expect(validateProjectPath('/')).toMatch(/sensitive system directory/i);
|
||||||
|
expect(validateProjectPath('/etc')).toMatch(/sensitive system directory/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('allows a normal, existing directory', () => {
|
||||||
|
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-validate-'));
|
||||||
|
try {
|
||||||
|
expect(validateProjectPath(dir)).toBeNull();
|
||||||
|
} finally {
|
||||||
|
fs.rmSync(dir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
// SENSITIVE_PATHS stores the Windows entries lowercase and validateProjectPath
|
||||||
|
// matches via resolved.toLowerCase(), so 'C:\\Windows' and 'c:\\windows' are
|
||||||
|
// both blocked. path.resolve is platform-specific, so this only runs on Windows.
|
||||||
|
it.runIf(process.platform === 'win32')(
|
||||||
|
'blocks Windows system directories regardless of case',
|
||||||
|
() => {
|
||||||
|
expect(validateProjectPath('C:\\Windows')).toMatch(/sensitive system directory/i);
|
||||||
|
expect(validateProjectPath('c:\\windows')).toMatch(/sensitive system directory/i);
|
||||||
|
expect(validateProjectPath('C:\\WINDOWS\\System32')).toMatch(/sensitive system directory/i);
|
||||||
|
}
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
describe('MCP Input Validation', () => {
|
describe('MCP Input Validation', () => {
|
||||||
let testDir: string;
|
let testDir: string;
|
||||||
let cg: CodeGraph;
|
let cg: CodeGraph;
|
||||||
|
|||||||
+22
-26
@@ -230,32 +230,28 @@ export class QueryBuilder {
|
|||||||
// deleteNode below).
|
// deleteNode below).
|
||||||
this.nodeCache.delete(node.id);
|
this.nodeCache.delete(node.id);
|
||||||
|
|
||||||
try {
|
this.stmts.insertNode.run({
|
||||||
this.stmts.insertNode.run({
|
id: node.id,
|
||||||
id: node.id,
|
kind: node.kind,
|
||||||
kind: node.kind,
|
name: node.name,
|
||||||
name: node.name,
|
qualifiedName: node.qualifiedName ?? node.name,
|
||||||
qualifiedName: node.qualifiedName ?? node.name,
|
filePath: node.filePath,
|
||||||
filePath: node.filePath,
|
language: node.language,
|
||||||
language: node.language,
|
startLine: node.startLine ?? 0,
|
||||||
startLine: node.startLine ?? 0,
|
endLine: node.endLine ?? 0,
|
||||||
endLine: node.endLine ?? 0,
|
startColumn: node.startColumn ?? 0,
|
||||||
startColumn: node.startColumn ?? 0,
|
endColumn: node.endColumn ?? 0,
|
||||||
endColumn: node.endColumn ?? 0,
|
docstring: node.docstring ?? null,
|
||||||
docstring: node.docstring ?? null,
|
signature: node.signature ?? null,
|
||||||
signature: node.signature ?? null,
|
visibility: node.visibility ?? null,
|
||||||
visibility: node.visibility ?? null,
|
isExported: node.isExported ? 1 : 0,
|
||||||
isExported: node.isExported ? 1 : 0,
|
isAsync: node.isAsync ? 1 : 0,
|
||||||
isAsync: node.isAsync ? 1 : 0,
|
isStatic: node.isStatic ? 1 : 0,
|
||||||
isStatic: node.isStatic ? 1 : 0,
|
isAbstract: node.isAbstract ? 1 : 0,
|
||||||
isAbstract: node.isAbstract ? 1 : 0,
|
decorators: node.decorators ? JSON.stringify(node.decorators) : null,
|
||||||
decorators: node.decorators ? JSON.stringify(node.decorators) : null,
|
typeParameters: node.typeParameters ? JSON.stringify(node.typeParameters) : null,
|
||||||
typeParameters: node.typeParameters ? JSON.stringify(node.typeParameters) : null,
|
updatedAt: node.updatedAt ?? Date.now(),
|
||||||
updatedAt: node.updatedAt ?? Date.now(),
|
});
|
||||||
});
|
|
||||||
} catch (error) {
|
|
||||||
throw error;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
+1
-1
@@ -43,7 +43,7 @@ import * as path from 'path';
|
|||||||
const SENSITIVE_PATHS = new Set([
|
const SENSITIVE_PATHS = new Set([
|
||||||
'/', '/etc', '/usr', '/bin', '/sbin', '/var', '/tmp', '/dev', '/proc', '/sys',
|
'/', '/etc', '/usr', '/bin', '/sbin', '/var', '/tmp', '/dev', '/proc', '/sys',
|
||||||
'/root', '/boot', '/lib', '/lib64', '/opt',
|
'/root', '/boot', '/lib', '/lib64', '/opt',
|
||||||
'C:\\', 'C:\\Windows', 'C:\\Windows\\System32',
|
'c:\\', 'c:\\windows', 'c:\\windows\\system32',
|
||||||
]);
|
]);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user