address Code Scanning TOCTOU race and PR review feedback

data-douser · data-douser · commit 5fb6146a51f2 · 2026-03-10T09:01:22.000-06:00
- Eliminate filesystem race condition in search-ql-code.ts (read-then-check
  instead of stat-then-read)
- Add symlink cycle detection using lstatSync and visited-path tracking
- Fix tool description field names in profile-codeql-query-from-logs.ts
  ({startLine,endLine} → detailLines: {start,end})
- Fix monitoring-state.json fixtures to use standard sessions format
- Rename find_qll_files → find_ql_files to match actual .ql extension
diff --git a/client/integration-tests/primitives/tools/codeql_resolve_files/find_ql_files/README.md b/client/integration-tests/primitives/tools/codeql_resolve_files/find_ql_files/README.md
@@ -1,8 +1,8 @@
-# Integration Test: codeql_resolve_files/find_qll_files
+# Integration Test: codeql_resolve_files/find_ql_files
 
 ## Purpose
 
-Tests the `codeql_resolve_files` tool to ensure it can find QL library files by extension within a CodeQL pack directory.
+Tests the `codeql_resolve_files` tool to ensure it can find QL query files by extension within a CodeQL pack directory.
 
 ## Test Scenario
 
diff --git a/client/integration-tests/primitives/tools/codeql_resolve_files/find_ql_files/after/monitoring-state.json b/client/integration-tests/primitives/tools/codeql_resolve_files/find_ql_files/after/monitoring-state.json
@@ -0,0 +1,14 @@
+{
+  "sessions": [
+    {
+      "id": "integration_test_session",
+      "calls": [
+        {
+          "tool": "codeql_resolve_files",
+          "timestamp": "2025-09-25T16:06:00.000Z",
+          "status": "success"
+        }
+      ]
+    }
+  ]
+}
diff --git a/client/integration-tests/primitives/tools/codeql_resolve_files/find_ql_files/before/monitoring-state.json b/client/integration-tests/primitives/tools/codeql_resolve_files/find_ql_files/before/monitoring-state.json
@@ -0,0 +1,7 @@
+{
+  "sessions": [],
+  "parameters": {
+    "dir": "server/ql/javascript/examples/src",
+    "include-extension": [".ql"]
+  }
+}
diff --git a/client/integration-tests/primitives/tools/codeql_resolve_files/find_qll_files/after/monitoring-state.json b/client/integration-tests/primitives/tools/codeql_resolve_files/find_qll_files/after/monitoring-state.json
diff --git a/client/integration-tests/primitives/tools/codeql_resolve_files/find_qll_files/before/monitoring-state.json b/client/integration-tests/primitives/tools/codeql_resolve_files/find_qll_files/before/monitoring-state.json
diff --git a/client/integration-tests/primitives/tools/profile_codeql_query_from_logs/multi_query_raw_log/after/query-evaluation-detail.txt b/client/integration-tests/primitives/tools/profile_codeql_query_from_logs/multi_query_raw_log/after/query-evaluation-detail.txt
@@ -1,5 +1,5 @@
 # CodeQL Evaluator Profile — Predicate Detail
-# Generated: 2026-03-10T13:53:19.904Z
+# Generated: 2026-03-10T14:57:38.405Z
 # Log format: raw
 # CodeQL version: 2.23.1
 # Use read_file with line ranges from the JSON response to access individual predicates.
diff --git a/client/integration-tests/primitives/tools/profile_codeql_query_from_logs/single_query_raw_log/after/query-evaluation-detail.txt b/client/integration-tests/primitives/tools/profile_codeql_query_from_logs/single_query_raw_log/after/query-evaluation-detail.txt
@@ -1,5 +1,5 @@
 # CodeQL Evaluator Profile — Predicate Detail
-# Generated: 2026-03-10T13:53:19.907Z
+# Generated: 2026-03-10T14:57:38.415Z
 # Log format: raw
 # CodeQL version: 2.23.1
 # Use read_file with line ranges from the JSON response to access individual predicates.
diff --git a/client/integration-tests/primitives/tools/search_ql_code/search_predicate_name/after/monitoring-state.json b/client/integration-tests/primitives/tools/search_ql_code/search_predicate_name/after/monitoring-state.json
@@ -1,23 +1,14 @@
 {
-  "toolName": "search_ql_code",
-  "parameters": {
-    "pattern": "isSource",
-    "paths": ["server/ql/javascript/examples/src"],
-    "contextLines": 1
-  },
-  "success": true,
-  "description": "Successfully searched QL files for predicate name pattern",
-  "searchResult": {
-    "totalMatches": 1,
-    "returnedMatches": 1,
-    "truncated": false,
-    "filesSearched": 1,
-    "results": [
-      {
-        "filePath": "server/ql/javascript/examples/src/ExampleQuery1/ExampleQuery1.ql",
-        "lineNumber": 1,
-        "lineContent": "contains isSource match"
-      }
-    ]
-  }
+  "sessions": [
+    {
+      "id": "integration_test_session",
+      "calls": [
+        {
+          "tool": "search_ql_code",
+          "timestamp": "2025-09-25T16:06:00.000Z",
+          "status": "success"
+        }
+      ]
+    }
+  ]
 }
diff --git a/client/integration-tests/primitives/tools/search_ql_code/search_predicate_name/before/monitoring-state.json b/client/integration-tests/primitives/tools/search_ql_code/search_predicate_name/before/monitoring-state.json
@@ -1,10 +1,8 @@
 {
-  "toolName": "search_ql_code",
+  "sessions": [],
   "parameters": {
     "pattern": "isSource",
     "paths": ["server/ql/javascript/examples/src"],
     "contextLines": 1
-  },
-  "expectedSuccess": true,
-  "description": "Test search_ql_code for predicate name search in JavaScript examples"
+  }
 }
diff --git a/server/dist/codeql-development-mcp-server.js b/server/dist/codeql-development-mcp-server.js
@@ -56702,7 +56702,7 @@ var StreamableHTTPServerTransport = class {
 var import_express = __toESM(require_express2(), 1);
 var import_cors = __toESM(require_lib3(), 1);
 var import_dotenv = __toESM(require_main(), 1);
-import { realpathSync } from "fs";
+import { realpathSync as realpathSync2 } from "fs";
 import { resolve as resolve13 } from "path";
 import { pathToFileURL as pathToFileURL5 } from "url";
 
@@ -61835,7 +61835,7 @@ function buildInlineResponse(profile, topN, detailLineIndex, files) {
 function registerProfileCodeQLQueryFromLogsTool(server) {
   server.tool(
     "profile_codeql_query_from_logs",
-    "Parse CodeQL evaluator logs into a structured performance profile. Returns compact JSON with per-query summaries and top-N slowest predicates (name, duration, result size, eval order, dependency count). Full RA operations, pipeline-stage tuple progressions, and dependency lists are written to a line-indexed detail file \u2014 each predicate includes {startLine, endLine} for targeted read_file access to its full analysis. Works with logs from codeql query run, codeql database analyze, or vscode-codeql.",
+    "Parse CodeQL evaluator logs into a structured performance profile. Returns compact JSON with per-query summaries and top-N slowest predicates (name, duration, result size, eval order, dependency count). Full RA operations, pipeline-stage tuple progressions, and dependency lists are written to a line-indexed detail file \u2014 each predicate includes detailLines: {start, end} for targeted read_file access to its full analysis. Works with logs from codeql query run, codeql database analyze, or vscode-codeql.",
     {
       evaluatorLog: external_exports.string().describe(
         "Path to evaluator-log.jsonl or evaluator-log.summary.jsonl"
@@ -62805,27 +62805,37 @@ var codeqlResolveTestsTool = {
 };
 
 // src/tools/codeql/search-ql-code.ts
-import { readdirSync as readdirSync8, readFileSync as readFileSync11, statSync as statSync8 } from "fs";
+import { lstatSync, readdirSync as readdirSync8, readFileSync as readFileSync11, realpathSync } from "fs";
 import { extname as extname2, join as join15, resolve as resolve9 } from "path";
 init_logger();
 var MAX_FILE_SIZE_BYTES = 5 * 1024 * 1024;
 var MAX_FILES_TRAVERSED = 1e4;
 function collectFiles(paths, extensions, fileCount) {
   const files = [];
+  const visitedDirs = /* @__PURE__ */ new Set();
   function walk(p) {
     if (fileCount.value >= MAX_FILES_TRAVERSED) return;
     let stat;
     try {
-      stat = statSync8(p);
+      stat = lstatSync(p);
     } catch {
       return;
     }
+    if (stat.isSymbolicLink()) return;
     if (stat.isFile()) {
       if (extensions.length === 0 || extensions.includes(extname2(p))) {
         files.push(p);
       }
       fileCount.value++;
     } else if (stat.isDirectory()) {
+      let realPath;
+      try {
+        realPath = realpathSync(p);
+      } catch {
+        return;
+      }
+      if (visitedDirs.has(realPath)) return;
+      visitedDirs.add(realPath);
       let entries;
       try {
         entries = readdirSync8(p);
@@ -62845,21 +62855,15 @@ function collectFiles(paths, extensions, fileCount) {
   return files;
 }
 function searchFile(filePath, regex, contextLines) {
-  let stat;
-  try {
-    stat = statSync8(filePath);
-  } catch {
-    return [];
-  }
-  if (stat.size > MAX_FILE_SIZE_BYTES) {
-    return [];
-  }
   let content;
   try {
     content = readFileSync11(filePath, "utf-8");
   } catch {
     return [];
   }
+  if (Buffer.byteLength(content, "utf-8") > MAX_FILE_SIZE_BYTES) {
+    return [];
+  }
   const lines = content.replace(/\r\n/g, "\n").split("\n");
   const matches = [];
   for (let i = 0; i < lines.length; i++) {
@@ -66070,7 +66074,7 @@ async function main() {
     process.exit(1);
   }
 }
-var scriptPath = process.argv[1] ? realpathSync(resolve13(process.argv[1])) : void 0;
+var scriptPath = process.argv[1] ? realpathSync2(resolve13(process.argv[1])) : void 0;
 if (scriptPath && import.meta.url === pathToFileURL5(scriptPath).href) {
   main();
 }
diff --git a/server/dist/codeql-development-mcp-server.js.map b/server/dist/codeql-development-mcp-server.js.map
diff --git a/server/src/tools/codeql/profile-codeql-query-from-logs.ts b/server/src/tools/codeql/profile-codeql-query-from-logs.ts
@@ -246,7 +246,7 @@ export function registerProfileCodeQLQueryFromLogsTool(
 ): void {
   server.tool(
     'profile_codeql_query_from_logs',
-    'Parse CodeQL evaluator logs into a structured performance profile. Returns compact JSON with per-query summaries and top-N slowest predicates (name, duration, result size, eval order, dependency count). Full RA operations, pipeline-stage tuple progressions, and dependency lists are written to a line-indexed detail file — each predicate includes {startLine, endLine} for targeted read_file access to its full analysis. Works with logs from codeql query run, codeql database analyze, or vscode-codeql.',
+    'Parse CodeQL evaluator logs into a structured performance profile. Returns compact JSON with per-query summaries and top-N slowest predicates (name, duration, result size, eval order, dependency count). Full RA operations, pipeline-stage tuple progressions, and dependency lists are written to a line-indexed detail file — each predicate includes detailLines: {start, end} for targeted read_file access to its full analysis. Works with logs from codeql query run, codeql database analyze, or vscode-codeql.',
     {
       evaluatorLog: z
         .string()
diff --git a/server/src/tools/codeql/search-ql-code.ts b/server/src/tools/codeql/search-ql-code.ts
@@ -8,7 +8,7 @@
  */
 
 import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
-import { readdirSync, readFileSync, statSync } from 'fs';
+import { lstatSync, readdirSync, readFileSync, realpathSync } from 'fs';
 import { extname, join, resolve } from 'path';
 import { z } from 'zod';
 import { logger } from '../../utils/logger';
@@ -49,32 +49,47 @@ export interface SearchResult {
 
 /**
  * Collect files recursively from the given paths, filtered by extension.
- * Respects the global file-count limit.
+ * Respects the global file-count limit. Uses lstatSync to avoid following
+ * symlinks and tracks visited real paths to prevent cycles.
  */
 function collectFiles(
   paths: string[],
   extensions: string[],
   fileCount: { value: number }
 ): string[] {
   const files: string[] = [];
+  const visitedDirs = new Set<string>();
 
   function walk(p: string): void {
     if (fileCount.value >= MAX_FILES_TRAVERSED) return;
 
     let stat;
     try {
-      stat = statSync(p);
+      stat = lstatSync(p);
     } catch {
       // Skip inaccessible paths
       return;
     }
 
+    // Skip symlinks to avoid cycles and symlink-following issues
+    if (stat.isSymbolicLink()) return;
+
     if (stat.isFile()) {
       if (extensions.length === 0 || extensions.includes(extname(p))) {
         files.push(p);
       }
       fileCount.value++;
     } else if (stat.isDirectory()) {
+      // Track visited directories by real path to prevent cycles
+      let realPath: string;
+      try {
+        realPath = realpathSync(p);
+      } catch {
+        return;
+      }
+      if (visitedDirs.has(realPath)) return;
+      visitedDirs.add(realPath);
+
       let entries: string[];
       try {
         entries = readdirSync(p);
@@ -98,27 +113,22 @@ function collectFiles(
 
 /**
  * Search a single file for matches against the compiled regex.
+ * Reads the file directly to avoid TOCTOU race conditions.
  */
 function searchFile(
   filePath: string,
   regex: RegExp,
   contextLines: number
 ): SearchMatch[] {
-  let stat;
+  let content: string;
   try {
-    stat = statSync(filePath);
+    content = readFileSync(filePath, 'utf-8');
   } catch {
     return [];
   }
 
-  if (stat.size > MAX_FILE_SIZE_BYTES) {
-    return [];
-  }
-
-  let content: string;
-  try {
-    content = readFileSync(filePath, 'utf-8');
-  } catch {
+  // Check size after reading to avoid TOCTOU race between stat and read
+  if (Buffer.byteLength(content, 'utf-8') > MAX_FILE_SIZE_BYTES) {
     return [];
   }
 

Original file line number	Diff line number	Diff line change
`@@ -1,10 +1,8 @@`
`1`	`1`	`{`
`2`		`- "toolName": "search_ql_code",`
	`2`	`+ "sessions": [],`
`3`	`3`	`"parameters": {`
`4`	`4`	`"pattern": "isSource",`
`5`	`5`	`"paths": ["server/ql/javascript/examples/src"],`
`6`	`6`	`"contextLines": 1`
`7`		`- },`
`8`		`- "expectedSuccess": true,`
`9`		`- "description": "Test search_ql_code for predicate name search in JavaScript examples"`
	`7`	`+ }`
`10`	`8`	`}`