Skip to content

Commit d6656ca

Browse files
committed
Address latest PR review comments
1 parent 88cb3b7 commit d6656ca

5 files changed

Lines changed: 130 additions & 193 deletions

File tree

.github/instructions/server_src_ts.instructions.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,4 @@ This file contains instructions for working with TypeScript source code files in
3333

3434
- NEVER leave any trailing whitespace on any line.
3535
- NEVER guess at what a `codeql` or `qlt` CLI subcommand does; ALWAYS verify against the official `codeql <subcommand> -h -vv` or `qlt <subcommand> -h` documentation, respectively.
36+
- **NEVER use stat/lstat followed by a separate read/open on the same path** — this is a TOCTOU (Time-of-Check-Time-of-Use) race condition (CWE-367). Instead, attempt the operation directly (e.g., `readFileSync`) within a try/catch block. If you need to know the file size before reading, read first and then check the buffer size — do NOT stat then read. For directory traversal, `lstatSync` is acceptable since it is the operation itself (checking entry type), not a precursor to a separate operation.

extensions/vscode/test/suite/copydb-e2e.integration.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ suite('copyDatabases E2E — query against copied database', () => {
128128

129129
const stagedDb = path.join(stagingDir, 'ExampleQuery1.testproj');
130130
console.log(`[copydb-e2e] Copying real database → staging: ${stagedDb}`);
131+
fs.mkdirSync(stagingDir, { recursive: true });
131132
copyDirSync(realDbPath, stagedDb);
132133

133134
// Inject a .lock file into the cache directory (mimicking query server).

server/dist/codeql-development-mcp-server.js

Lines changed: 55 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -62967,42 +62967,55 @@ function collectFiles(paths, extensions, fileCount) {
6296762967
}
6296862968
return files;
6296962969
}
62970-
function searchFileSmall(filePath, regex, contextLines) {
62970+
async function searchFile(filePath, regex, contextLines, maxCollect) {
6297162971
let content;
6297262972
try {
6297362973
content = readFileSync9(filePath, "utf-8");
6297462974
} catch {
62975-
return [];
62975+
return { matches: [], totalCount: 0 };
62976+
}
62977+
if (Buffer.byteLength(content, "utf-8") > MAX_FILE_SIZE_BYTES) {
62978+
return searchFileStreaming(filePath, regex, contextLines, maxCollect);
6297662979
}
6297762980
const lines = content.replace(/\r\n/g, "\n").split("\n");
6297862981
const matches = [];
62982+
let totalCount = 0;
6297962983
for (let i = 0; i < lines.length; i++) {
6298062984
if (regex.test(lines[i])) {
62981-
const match = {
62982-
filePath,
62983-
lineNumber: i + 1,
62984-
lineContent: lines[i]
62985-
};
62986-
if (contextLines > 0) {
62987-
const beforeStart = Math.max(0, i - contextLines);
62988-
const afterEnd = Math.min(lines.length - 1, i + contextLines);
62989-
match.contextBefore = lines.slice(beforeStart, i);
62990-
match.contextAfter = lines.slice(i + 1, afterEnd + 1);
62985+
totalCount++;
62986+
if (matches.length < maxCollect) {
62987+
const match = {
62988+
filePath,
62989+
lineNumber: i + 1,
62990+
lineContent: lines[i]
62991+
};
62992+
if (contextLines > 0) {
62993+
const beforeStart = Math.max(0, i - contextLines);
62994+
const afterEnd = Math.min(lines.length - 1, i + contextLines);
62995+
match.contextBefore = lines.slice(beforeStart, i);
62996+
match.contextAfter = lines.slice(i + 1, afterEnd + 1);
62997+
}
62998+
matches.push(match);
6299162999
}
62992-
matches.push(match);
6299363000
}
6299463001
}
62995-
return matches;
63002+
return { matches, totalCount };
6299663003
}
62997-
async function searchFileLarge(filePath, regex, contextLines) {
63004+
async function searchFileStreaming(filePath, regex, contextLines, maxCollect) {
6299863005
const matches = [];
6299963006
const recentLines = [];
6300063007
const pending = [];
6300163008
let lineNumber = 0;
63002-
const rl = createInterface3({
63003-
input: createReadStream3(filePath, { encoding: "utf-8" }),
63004-
crlfDelay: Infinity
63005-
});
63009+
let totalCount = 0;
63010+
let rl;
63011+
try {
63012+
rl = createInterface3({
63013+
input: createReadStream3(filePath, { encoding: "utf-8" }),
63014+
crlfDelay: Infinity
63015+
});
63016+
} catch {
63017+
return { matches: [], totalCount: 0 };
63018+
}
6300663019
for await (const line of rl) {
6300763020
lineNumber++;
6300863021
for (const p of pending) {
@@ -63015,17 +63028,20 @@ async function searchFileLarge(filePath, regex, contextLines) {
6301563028
pending.shift();
6301663029
}
6301763030
if (regex.test(line)) {
63018-
const match = {
63019-
filePath,
63020-
lineNumber,
63021-
lineContent: line
63022-
};
63023-
if (contextLines > 0) {
63024-
match.contextBefore = recentLines.slice(-contextLines);
63025-
match.contextAfter = [];
63026-
pending.push({ match, afterNeeded: contextLines });
63031+
totalCount++;
63032+
if (matches.length < maxCollect) {
63033+
const match = {
63034+
filePath,
63035+
lineNumber,
63036+
lineContent: line
63037+
};
63038+
if (contextLines > 0) {
63039+
match.contextBefore = recentLines.slice(-contextLines);
63040+
match.contextAfter = [];
63041+
pending.push({ match, afterNeeded: contextLines });
63042+
}
63043+
matches.push(match);
6302763044
}
63028-
matches.push(match);
6302963045
}
6303063046
if (contextLines > 0) {
6303163047
recentLines.push(line);
@@ -63034,21 +63050,7 @@ async function searchFileLarge(filePath, regex, contextLines) {
6303463050
}
6303563051
}
6303663052
}
63037-
return matches;
63038-
}
63039-
function searchFile(filePath, regex, contextLines) {
63040-
try {
63041-
const st = lstatSync(filePath);
63042-
if (!st.isFile()) {
63043-
return [];
63044-
}
63045-
if (st.size > MAX_FILE_SIZE_BYTES) {
63046-
return searchFileLarge(filePath, regex, contextLines);
63047-
}
63048-
} catch {
63049-
return [];
63050-
}
63051-
return searchFileSmall(filePath, regex, contextLines);
63053+
return { matches, totalCount };
6305263054
}
6305363055
async function searchQlCode(params) {
6305463056
const {
@@ -63067,40 +63069,16 @@ async function searchQlCode(params) {
6306763069
const filesToSearch = collectFiles(paths, includeExtensions, fileCount);
6306863070
const allMatches = [];
6306963071
let totalMatches = 0;
63070-
let collectedEnough = false;
6307163072
for (const file of filesToSearch) {
63072-
if (collectedEnough) {
63073-
try {
63074-
const st = lstatSync(file);
63075-
if (!st.isFile()) continue;
63076-
if (st.size > MAX_FILE_SIZE_BYTES) {
63077-
const rl = createInterface3({
63078-
input: createReadStream3(file, { encoding: "utf-8" }),
63079-
crlfDelay: Infinity
63080-
});
63081-
for await (const line of rl) {
63082-
if (regex.test(line)) totalMatches++;
63083-
}
63084-
} else {
63085-
const content = readFileSync9(file, "utf-8");
63086-
for (const line of content.replace(/\r\n/g, "\n").split("\n")) {
63087-
if (regex.test(line)) totalMatches++;
63088-
}
63089-
}
63090-
} catch {
63091-
}
63092-
continue;
63093-
}
63094-
const fileMatches = await searchFile(file, regex, contextLines);
63095-
totalMatches += fileMatches.length;
63096-
for (const m of fileMatches) {
63097-
if (allMatches.length < maxResults) {
63098-
allMatches.push(m);
63099-
}
63100-
}
63101-
if (allMatches.length >= maxResults) {
63102-
collectedEnough = true;
63103-
}
63073+
const remainingSlots = Math.max(0, maxResults - allMatches.length);
63074+
const { matches: fileMatches, totalCount } = await searchFile(
63075+
file,
63076+
regex,
63077+
contextLines,
63078+
remainingSlots
63079+
);
63080+
totalMatches += totalCount;
63081+
allMatches.push(...fileMatches);
6310463082
}
6310563083
return {
6310663084
results: allMatches,

server/dist/codeql-development-mcp-server.js.map

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)