Skip to content

Commit 69a6df8

Browse files
committed
Address PR #169 review comments
- Fix TOCTOU in query-results-evaluator (openSync/fstatSync/readFileSync(fd)) - Use datetime('now') consistently for annotation timestamps - Debounce flush() with 200ms coalescing via scheduleFlush() - Fix resultIndices to inclusive [start, end] range with clamping - Fix WASM→asm.js comments in session-data-manager - Fix audit-tools header comment (no separate ENABLE_AUDIT_TOOLS flag) - Add separate maxResults parameter for SARIF in cache-tools - Use createProjectTempDir() in all test files - Fix monitoring-tools test init order (mock before initialize) - Add store.close() in session-data-manager test afterEach
1 parent 46d5b9c commit 69a6df8

10 files changed

+136
-92
lines changed

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

Lines changed: 61 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -184855,7 +184855,7 @@ import { createHash as createHash2 } from "crypto";
184855184855
// src/lib/query-results-evaluator.ts
184856184856
init_cli_executor();
184857184857
init_logger();
184858-
import { writeFileSync, readFileSync as readFileSync4, statSync as statSync3 } from "fs";
184858+
import { fstatSync, openSync, readFileSync as readFileSync4, writeFileSync } from "fs";
184859184859
import { dirname as dirname3, isAbsolute as isAbsolute3 } from "path";
184860184860
import { mkdirSync as mkdirSync4 } from "fs";
184861184861
var BUILT_IN_EVALUATORS = {
@@ -184866,13 +184866,13 @@ var BUILT_IN_EVALUATORS = {
184866184866
var metadataCache = /* @__PURE__ */ new Map();
184867184867
async function extractQueryMetadata(queryPath) {
184868184868
try {
184869-
const stat = statSync3(queryPath);
184870-
const mtime = stat.mtimeMs;
184869+
const fd = openSync(queryPath, "r");
184870+
const mtime = fstatSync(fd).mtimeMs;
184871184871
const cached2 = metadataCache.get(queryPath);
184872184872
if (cached2 && cached2.mtime === mtime) {
184873184873
return cached2.metadata;
184874184874
}
184875-
const queryContent = readFileSync4(queryPath, "utf-8");
184875+
const queryContent = readFileSync4(fd, "utf-8");
184876184876
const metadata = {};
184877184877
const kindMatch = queryContent.match(/@kind\s+([^\s]+)/);
184878184878
if (kindMatch) metadata.kind = kindMatch[1];
@@ -185131,11 +185131,14 @@ var import_sql_asm = __toESM(require_sql_asm(), 1);
185131185131
init_logger();
185132185132
import { existsSync as existsSync6, mkdirSync as mkdirSync5, readFileSync as readFileSync5, writeFileSync as writeFileSync2 } from "fs";
185133185133
import { join as join8 } from "path";
185134-
var SqliteStore = class {
185134+
var SqliteStore = class _SqliteStore {
185135185135
db = null;
185136185136
dbPath;
185137185137
storageDir;
185138185138
dirty = false;
185139+
flushTimer = null;
185140+
/** Debounce interval (ms) for automatic disk writes after mutations. */
185141+
static FLUSH_DEBOUNCE_MS = 200;
185139185142
constructor(storageDir) {
185140185143
this.storageDir = storageDir;
185141185144
this.dbPath = join8(storageDir, "ql-mcp.db");
@@ -185255,12 +185258,27 @@ var SqliteStore = class {
185255185258
* Write the in-memory database to disk.
185256185259
*/
185257185260
flush() {
185261+
if (this.flushTimer) {
185262+
globalThis.clearTimeout(this.flushTimer);
185263+
this.flushTimer = null;
185264+
}
185258185265
if (!this.db) return;
185259185266
const data = this.db.export();
185260185267
const buffer = Buffer.from(data);
185261185268
writeFileSync2(this.dbPath, buffer);
185262185269
this.dirty = false;
185263185270
}
185271+
/**
185272+
* Schedule a debounced flush. Multiple rapid writes are coalesced into
185273+
* a single disk write after `FLUSH_DEBOUNCE_MS` of inactivity.
185274+
*/
185275+
scheduleFlush() {
185276+
if (this.flushTimer) globalThis.clearTimeout(this.flushTimer);
185277+
this.flushTimer = globalThis.setTimeout(() => {
185278+
this.flushTimer = null;
185279+
this.flushIfDirty();
185280+
}, _SqliteStore.FLUSH_DEBOUNCE_MS);
185281+
}
185264185282
/**
185265185283
* Flush only if there are pending writes.
185266185284
*/
@@ -185289,7 +185307,7 @@ var SqliteStore = class {
185289185307
{ $id: sessionId, $data: json2 }
185290185308
);
185291185309
this.dirty = true;
185292-
this.flush();
185310+
this.scheduleFlush();
185293185311
}
185294185312
/**
185295185313
* Retrieve a single session by ID, or null if not found.
@@ -185325,7 +185343,7 @@ var SqliteStore = class {
185325185343
*/
185326185344
deleteSession(sessionId) {
185327185345
this.exec("DELETE FROM sessions WHERE session_id = $id", { $id: sessionId });
185328-
this.flush();
185346+
this.scheduleFlush();
185329185347
}
185330185348
/**
185331185349
* Count active sessions.
@@ -185346,24 +185364,21 @@ var SqliteStore = class {
185346185364
*/
185347185365
createAnnotation(category, entityKey, content, label, metadata) {
185348185366
const db = this.ensureDb();
185349-
const now = (/* @__PURE__ */ new Date()).toISOString();
185350185367
db.run(
185351185368
`INSERT INTO annotations (category, entity_key, label, content, metadata, created_at, updated_at)
185352-
VALUES ($category, $entity_key, $label, $content, $metadata, $created_at, $updated_at)`,
185369+
VALUES ($category, $entity_key, $label, $content, $metadata, datetime('now'), datetime('now'))`,
185353185370
{
185354185371
$category: category,
185355185372
$entity_key: entityKey,
185356185373
$label: label ?? null,
185357185374
$content: content ?? null,
185358-
$metadata: metadata ?? null,
185359-
$created_at: now,
185360-
$updated_at: now
185375+
$metadata: metadata ?? null
185361185376
}
185362185377
);
185363185378
this.dirty = true;
185364185379
const result = db.exec("SELECT last_insert_rowid() as id");
185365185380
const id = result.length > 0 ? result[0].values[0][0] : 0;
185366-
this.flush();
185381+
this.scheduleFlush();
185367185382
return id;
185368185383
}
185369185384
/**
@@ -185453,7 +185468,7 @@ var SqliteStore = class {
185453185468
);
185454185469
const changed = this.getRowsModified();
185455185470
this.dirty = true;
185456-
this.flush();
185471+
this.scheduleFlush();
185457185472
return changed > 0;
185458185473
}
185459185474
/**
@@ -185486,7 +185501,7 @@ var SqliteStore = class {
185486185501
);
185487185502
const deleted = this.getRowsModified();
185488185503
this.dirty = true;
185489-
this.flush();
185504+
this.scheduleFlush();
185490185505
return deleted;
185491185506
}
185492185507
// ---------------------------------------------------------------------------
@@ -185523,7 +185538,7 @@ var SqliteStore = class {
185523185538
}
185524185539
);
185525185540
this.dirty = true;
185526-
this.flush();
185541+
this.scheduleFlush();
185527185542
}
185528185543
/**
185529185544
* Look up a cache entry by key. Returns metadata (no content) or null.
@@ -185622,8 +185637,15 @@ var SqliteStore = class {
185622185637
});
185623185638
}
185624185639
if (options.resultIndices) {
185625-
const [start, end] = options.resultIndices;
185626-
selected = selected.slice(start, end);
185640+
const [rawStart, rawEnd] = options.resultIndices;
185641+
const total = selected.length;
185642+
if (Number.isFinite(rawStart) && Number.isFinite(rawEnd) && total > 0) {
185643+
const start = Math.max(0, Math.min(total - 1, Math.floor(rawStart)));
185644+
const end = Math.max(0, Math.min(total - 1, Math.floor(rawEnd)));
185645+
selected = end >= start ? selected.slice(start, end + 1) : [];
185646+
} else {
185647+
selected = [];
185648+
}
185627185649
}
185628185650
const truncated = selected.length > maxResults;
185629185651
if (truncated) {
@@ -185705,7 +185727,7 @@ var SqliteStore = class {
185705185727
if (filter?.all) {
185706185728
this.exec("DELETE FROM query_result_cache");
185707185729
const deleted2 = this.getRowsModified();
185708-
this.flush();
185730+
this.scheduleFlush();
185709185731
return deleted2;
185710185732
}
185711185733
const conditions = [];
@@ -185730,7 +185752,7 @@ var SqliteStore = class {
185730185752
);
185731185753
const deleted = this.getRowsModified();
185732185754
this.dirty = true;
185733-
this.flush();
185755+
this.scheduleFlush();
185734185756
return deleted;
185735185757
}
185736185758
};
@@ -185882,7 +185904,7 @@ var SessionDataManager = class {
185882185904
}
185883185905
/**
185884185906
* Initialize the database and ensure it's properly set up.
185885-
* Must be awaited before any session operations (sql.js WASM init is async).
185907+
* Must be awaited before any session operations (sql.js init is async).
185886185908
*/
185887185909
async initialize() {
185888185910
try {
@@ -190023,7 +190045,7 @@ var codeqlGenerateQueryHelpTool = {
190023190045
};
190024190046

190025190047
// src/tools/codeql/list-databases.ts
190026-
import { existsSync as existsSync9, readdirSync as readdirSync4, readFileSync as readFileSync8, statSync as statSync5 } from "fs";
190048+
import { existsSync as existsSync9, readdirSync as readdirSync4, readFileSync as readFileSync8, statSync as statSync4 } from "fs";
190027190049
import { join as join12 } from "path";
190028190050

190029190051
// src/lib/discovery-config.ts
@@ -190088,7 +190110,7 @@ async function discoverDatabases(baseDirs, language) {
190088190110
for (const entry of entries) {
190089190111
const entryPath = join12(baseDir, entry);
190090190112
try {
190091-
if (!statSync5(entryPath).isDirectory()) {
190113+
if (!statSync4(entryPath).isDirectory()) {
190092190114
continue;
190093190115
}
190094190116
} catch {
@@ -190177,7 +190199,7 @@ function registerListDatabasesTool(server) {
190177190199
}
190178190200

190179190201
// src/tools/codeql/list-mrva-run-results.ts
190180-
import { existsSync as existsSync10, readdirSync as readdirSync5, readFileSync as readFileSync9, statSync as statSync6 } from "fs";
190202+
import { existsSync as existsSync10, readdirSync as readdirSync5, readFileSync as readFileSync9, statSync as statSync5 } from "fs";
190181190203
import { join as join13 } from "path";
190182190204
init_logger();
190183190205
var NUMERIC_DIR_PATTERN = /^\d+$/;
@@ -190197,7 +190219,7 @@ async function discoverMrvaRunResults(resultsDirs, runId) {
190197190219
for (const entry of entries) {
190198190220
const entryPath = join13(dir, entry);
190199190221
try {
190200-
if (!statSync6(entryPath).isDirectory()) {
190222+
if (!statSync5(entryPath).isDirectory()) {
190201190223
continue;
190202190224
}
190203190225
} catch {
@@ -190242,7 +190264,7 @@ function discoverRepoResults(runPath) {
190242190264
}
190243190265
const ownerPath = join13(runPath, ownerEntry);
190244190266
try {
190245-
if (!statSync6(ownerPath).isDirectory()) {
190267+
if (!statSync5(ownerPath).isDirectory()) {
190246190268
continue;
190247190269
}
190248190270
} catch {
@@ -190257,7 +190279,7 @@ function discoverRepoResults(runPath) {
190257190279
for (const repoEntry of repoEntries) {
190258190280
const repoPath = join13(ownerPath, repoEntry);
190259190281
try {
190260-
if (!statSync6(repoPath).isDirectory()) {
190282+
if (!statSync5(repoPath).isDirectory()) {
190261190283
continue;
190262190284
}
190263190285
} catch {
@@ -190364,7 +190386,7 @@ function registerListMrvaRunResultsTool(server) {
190364190386
}
190365190387

190366190388
// src/tools/codeql/list-query-run-results.ts
190367-
import { existsSync as existsSync11, readdirSync as readdirSync6, readFileSync as readFileSync10, statSync as statSync7 } from "fs";
190389+
import { existsSync as existsSync11, readdirSync as readdirSync6, readFileSync as readFileSync10, statSync as statSync6 } from "fs";
190368190390
import { join as join14 } from "path";
190369190391
init_logger();
190370190392
var QUERY_RUN_DIR_PATTERN = /^(.+\.ql)-(.+)$/;
@@ -190413,7 +190435,7 @@ async function discoverQueryRunResults(resultsDirs, filter) {
190413190435
for (const entry of entries) {
190414190436
const entryPath = join14(dir, entry);
190415190437
try {
190416-
if (!statSync7(entryPath).isDirectory()) {
190438+
if (!statSync6(entryPath).isDirectory()) {
190417190439
continue;
190418190440
}
190419190441
} catch {
@@ -191553,7 +191575,7 @@ function registerQuickEvaluateTool(server) {
191553191575

191554191576
// src/tools/codeql/read-database-source.ts
191555191577
var import_adm_zip = __toESM(require_adm_zip(), 1);
191556-
import { existsSync as existsSync14, readdirSync as readdirSync7, readFileSync as readFileSync11, statSync as statSync8 } from "fs";
191578+
import { existsSync as existsSync14, readdirSync as readdirSync7, readFileSync as readFileSync11, statSync as statSync7 } from "fs";
191557191579
import { join as join18, resolve as resolve7 } from "path";
191558191580
import { fileURLToPath as fileURLToPath2 } from "url";
191559191581
init_logger();
@@ -191572,7 +191594,7 @@ function toFilesystemPath(uri) {
191572191594
function* walkDirectory(dir, base = dir) {
191573191595
for (const entry of readdirSync7(dir)) {
191574191596
const fullPath = join18(dir, entry);
191575-
if (statSync8(fullPath).isDirectory()) {
191597+
if (statSync7(fullPath).isDirectory()) {
191576191598
yield* walkDirectory(fullPath, base);
191577191599
} else {
191578191600
yield fullPath.slice(base.length).replace(/\\/g, "/").replace(/^\//, "");
@@ -192063,7 +192085,7 @@ var codeqlResolveTestsTool = {
192063192085
};
192064192086

192065192087
// src/tools/codeql/search-ql-code.ts
192066-
import { closeSync, createReadStream as createReadStream3, fstatSync, lstatSync, openSync, readdirSync as readdirSync8, readFileSync as readFileSync12, realpathSync } from "fs";
192088+
import { closeSync, createReadStream as createReadStream3, fstatSync as fstatSync2, lstatSync, openSync as openSync2, readdirSync as readdirSync8, readFileSync as readFileSync12, realpathSync } from "fs";
192067192089
import { basename as basename8, extname as extname2, join as join19, resolve as resolve9 } from "path";
192068192090
import { createInterface as createInterface3 } from "readline";
192069192091
init_logger();
@@ -192120,13 +192142,13 @@ function collectFiles(paths, extensions, fileCount) {
192120192142
async function searchFile(filePath, regex, contextLines, maxCollect) {
192121192143
let fd;
192122192144
try {
192123-
fd = openSync(filePath, "r");
192145+
fd = openSync2(filePath, "r");
192124192146
} catch {
192125192147
return { matches: [], totalCount: 0 };
192126192148
}
192127192149
let size;
192128192150
try {
192129-
size = fstatSync(fd).size;
192151+
size = fstatSync2(fd).size;
192130192152
} catch {
192131192153
try {
192132192154
closeSync(fd);
@@ -195553,12 +195575,13 @@ function registerQueryResultsCacheRetrieveTool(server) {
195553195575
{
195554195576
cacheKey: external_exports.string().describe("The cache key of the result to retrieve."),
195555195577
lineRange: external_exports.tuple([external_exports.number(), external_exports.number()]).optional().describe("Line range [start, end] (1-indexed). For graphtext/CSV output."),
195556-
resultIndices: external_exports.tuple([external_exports.number(), external_exports.number()]).optional().describe("SARIF result index range [start, end] (0-indexed)."),
195578+
resultIndices: external_exports.tuple([external_exports.number(), external_exports.number()]).optional().describe("SARIF result index range [start, end] (0-indexed, inclusive)."),
195557195579
fileFilter: external_exports.string().optional().describe("For SARIF: only include results whose file path contains this string."),
195558195580
grep: external_exports.string().optional().describe("Text search filter: only include lines/results containing this term."),
195559-
maxLines: external_exports.number().optional().describe("Maximum number of lines to return (default: 500).")
195581+
maxLines: external_exports.number().optional().describe("Maximum number of lines to return for line-based formats (default: 500)."),
195582+
maxResults: external_exports.number().optional().describe("Maximum number of SARIF results to return (default: 100).")
195560195583
},
195561-
async ({ cacheKey: cacheKey2, lineRange, resultIndices, fileFilter, grep, maxLines }) => {
195584+
async ({ cacheKey: cacheKey2, lineRange, resultIndices, fileFilter, grep, maxLines, maxResults }) => {
195562195585
const store = sessionDataManager.getStore();
195563195586
const meta = store.getCacheEntryMeta(cacheKey2);
195564195587
if (!meta) {
@@ -195569,7 +195592,7 @@ function registerQueryResultsCacheRetrieveTool(server) {
195569195592
const subset2 = store.getCacheSarifSubset(cacheKey2, {
195570195593
resultIndices,
195571195594
fileFilter,
195572-
maxResults: maxLines
195595+
maxResults
195573195596
});
195574195597
if (!subset2) {
195575195598
return { content: [{ type: "text", text: `Cached content not available for key: ${cacheKey2}` }] };

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

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

server/src/lib/query-results-evaluator.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import { executeCodeQLCommand } from './cli-executor';
66
import { logger } from '../utils/logger';
7-
import { writeFileSync, readFileSync, statSync } from 'fs';
7+
import { fstatSync, openSync, readFileSync, writeFileSync } from 'fs';
88
import { dirname, isAbsolute } from 'path';
99
import { mkdirSync } from 'fs';
1010

@@ -46,15 +46,15 @@ const metadataCache = new Map<string, { mtime: number; metadata: QueryMetadata }
4646
*/
4747
export async function extractQueryMetadata(queryPath: string): Promise<QueryMetadata> {
4848
try {
49-
// Check cache with mtime validation
50-
const stat = statSync(queryPath);
51-
const mtime = stat.mtimeMs;
49+
// Open once, then fstat + read via the fd to avoid TOCTOU race (CWE-367).
50+
const fd = openSync(queryPath, 'r');
51+
const mtime = fstatSync(fd).mtimeMs;
5252
const cached = metadataCache.get(queryPath);
5353
if (cached && cached.mtime === mtime) {
5454
return cached.metadata;
5555
}
5656

57-
const queryContent = readFileSync(queryPath, 'utf-8');
57+
const queryContent = readFileSync(fd, 'utf-8');
5858
const metadata: QueryMetadata = {};
5959

6060
// Extract metadata from comments using regex patterns

server/src/lib/session-data-manager.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* Session Data Management
3-
* Provides session lifecycle management backed by SqliteStore (sql.js WASM)
3+
* Provides session lifecycle management backed by SqliteStore (sql.js asm.js)
44
*/
55

66
import { mkdirSync, writeFileSync } from 'fs';
@@ -45,7 +45,7 @@ export class SessionDataManager {
4545

4646
/**
4747
* Initialize the database and ensure it's properly set up.
48-
* Must be awaited before any session operations (sql.js WASM init is async).
48+
* Must be awaited before any session operations (sql.js init is async).
4949
*/
5050
async initialize(): Promise<void> {
5151
try {

0 commit comments

Comments
 (0)