Skip to content

Commit 0d4908c

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 d26fe4f commit 0d4908c

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
@@ -184843,7 +184843,7 @@ import { createHash as createHash2 } from "crypto";
184843184843
// src/lib/query-results-evaluator.ts
184844184844
init_cli_executor();
184845184845
init_logger();
184846-
import { writeFileSync, readFileSync as readFileSync4, statSync as statSync3 } from "fs";
184846+
import { fstatSync, openSync, readFileSync as readFileSync4, writeFileSync } from "fs";
184847184847
import { dirname as dirname3, isAbsolute as isAbsolute3 } from "path";
184848184848
import { mkdirSync as mkdirSync4 } from "fs";
184849184849
var BUILT_IN_EVALUATORS = {
@@ -184854,13 +184854,13 @@ var BUILT_IN_EVALUATORS = {
184854184854
var metadataCache = /* @__PURE__ */ new Map();
184855184855
async function extractQueryMetadata(queryPath) {
184856184856
try {
184857-
const stat = statSync3(queryPath);
184858-
const mtime = stat.mtimeMs;
184857+
const fd = openSync(queryPath, "r");
184858+
const mtime = fstatSync(fd).mtimeMs;
184859184859
const cached2 = metadataCache.get(queryPath);
184860184860
if (cached2 && cached2.mtime === mtime) {
184861184861
return cached2.metadata;
184862184862
}
184863-
const queryContent = readFileSync4(queryPath, "utf-8");
184863+
const queryContent = readFileSync4(fd, "utf-8");
184864184864
const metadata = {};
184865184865
const kindMatch = queryContent.match(/@kind\s+([^\s]+)/);
184866184866
if (kindMatch) metadata.kind = kindMatch[1];
@@ -185119,11 +185119,14 @@ var import_sql_asm = __toESM(require_sql_asm(), 1);
185119185119
init_logger();
185120185120
import { existsSync as existsSync6, mkdirSync as mkdirSync5, readFileSync as readFileSync5, writeFileSync as writeFileSync2 } from "fs";
185121185121
import { join as join8 } from "path";
185122-
var SqliteStore = class {
185122+
var SqliteStore = class _SqliteStore {
185123185123
db = null;
185124185124
dbPath;
185125185125
storageDir;
185126185126
dirty = false;
185127+
flushTimer = null;
185128+
/** Debounce interval (ms) for automatic disk writes after mutations. */
185129+
static FLUSH_DEBOUNCE_MS = 200;
185127185130
constructor(storageDir) {
185128185131
this.storageDir = storageDir;
185129185132
this.dbPath = join8(storageDir, "ql-mcp.db");
@@ -185243,12 +185246,27 @@ var SqliteStore = class {
185243185246
* Write the in-memory database to disk.
185244185247
*/
185245185248
flush() {
185249+
if (this.flushTimer) {
185250+
globalThis.clearTimeout(this.flushTimer);
185251+
this.flushTimer = null;
185252+
}
185246185253
if (!this.db) return;
185247185254
const data = this.db.export();
185248185255
const buffer = Buffer.from(data);
185249185256
writeFileSync2(this.dbPath, buffer);
185250185257
this.dirty = false;
185251185258
}
185259+
/**
185260+
* Schedule a debounced flush. Multiple rapid writes are coalesced into
185261+
* a single disk write after `FLUSH_DEBOUNCE_MS` of inactivity.
185262+
*/
185263+
scheduleFlush() {
185264+
if (this.flushTimer) globalThis.clearTimeout(this.flushTimer);
185265+
this.flushTimer = globalThis.setTimeout(() => {
185266+
this.flushTimer = null;
185267+
this.flushIfDirty();
185268+
}, _SqliteStore.FLUSH_DEBOUNCE_MS);
185269+
}
185252185270
/**
185253185271
* Flush only if there are pending writes.
185254185272
*/
@@ -185277,7 +185295,7 @@ var SqliteStore = class {
185277185295
{ $id: sessionId, $data: json2 }
185278185296
);
185279185297
this.dirty = true;
185280-
this.flush();
185298+
this.scheduleFlush();
185281185299
}
185282185300
/**
185283185301
* Retrieve a single session by ID, or null if not found.
@@ -185313,7 +185331,7 @@ var SqliteStore = class {
185313185331
*/
185314185332
deleteSession(sessionId) {
185315185333
this.exec("DELETE FROM sessions WHERE session_id = $id", { $id: sessionId });
185316-
this.flush();
185334+
this.scheduleFlush();
185317185335
}
185318185336
/**
185319185337
* Count active sessions.
@@ -185334,24 +185352,21 @@ var SqliteStore = class {
185334185352
*/
185335185353
createAnnotation(category, entityKey, content, label, metadata) {
185336185354
const db = this.ensureDb();
185337-
const now = (/* @__PURE__ */ new Date()).toISOString();
185338185355
db.run(
185339185356
`INSERT INTO annotations (category, entity_key, label, content, metadata, created_at, updated_at)
185340-
VALUES ($category, $entity_key, $label, $content, $metadata, $created_at, $updated_at)`,
185357+
VALUES ($category, $entity_key, $label, $content, $metadata, datetime('now'), datetime('now'))`,
185341185358
{
185342185359
$category: category,
185343185360
$entity_key: entityKey,
185344185361
$label: label ?? null,
185345185362
$content: content ?? null,
185346-
$metadata: metadata ?? null,
185347-
$created_at: now,
185348-
$updated_at: now
185363+
$metadata: metadata ?? null
185349185364
}
185350185365
);
185351185366
this.dirty = true;
185352185367
const result = db.exec("SELECT last_insert_rowid() as id");
185353185368
const id = result.length > 0 ? result[0].values[0][0] : 0;
185354-
this.flush();
185369+
this.scheduleFlush();
185355185370
return id;
185356185371
}
185357185372
/**
@@ -185441,7 +185456,7 @@ var SqliteStore = class {
185441185456
);
185442185457
const changed = this.getRowsModified();
185443185458
this.dirty = true;
185444-
this.flush();
185459+
this.scheduleFlush();
185445185460
return changed > 0;
185446185461
}
185447185462
/**
@@ -185474,7 +185489,7 @@ var SqliteStore = class {
185474185489
);
185475185490
const deleted = this.getRowsModified();
185476185491
this.dirty = true;
185477-
this.flush();
185492+
this.scheduleFlush();
185478185493
return deleted;
185479185494
}
185480185495
// ---------------------------------------------------------------------------
@@ -185511,7 +185526,7 @@ var SqliteStore = class {
185511185526
}
185512185527
);
185513185528
this.dirty = true;
185514-
this.flush();
185529+
this.scheduleFlush();
185515185530
}
185516185531
/**
185517185532
* Look up a cache entry by key. Returns metadata (no content) or null.
@@ -185610,8 +185625,15 @@ var SqliteStore = class {
185610185625
});
185611185626
}
185612185627
if (options.resultIndices) {
185613-
const [start, end] = options.resultIndices;
185614-
selected = selected.slice(start, end);
185628+
const [rawStart, rawEnd] = options.resultIndices;
185629+
const total = selected.length;
185630+
if (Number.isFinite(rawStart) && Number.isFinite(rawEnd) && total > 0) {
185631+
const start = Math.max(0, Math.min(total - 1, Math.floor(rawStart)));
185632+
const end = Math.max(0, Math.min(total - 1, Math.floor(rawEnd)));
185633+
selected = end >= start ? selected.slice(start, end + 1) : [];
185634+
} else {
185635+
selected = [];
185636+
}
185615185637
}
185616185638
const truncated = selected.length > maxResults;
185617185639
if (truncated) {
@@ -185693,7 +185715,7 @@ var SqliteStore = class {
185693185715
if (filter?.all) {
185694185716
this.exec("DELETE FROM query_result_cache");
185695185717
const deleted2 = this.getRowsModified();
185696-
this.flush();
185718+
this.scheduleFlush();
185697185719
return deleted2;
185698185720
}
185699185721
const conditions = [];
@@ -185718,7 +185740,7 @@ var SqliteStore = class {
185718185740
);
185719185741
const deleted = this.getRowsModified();
185720185742
this.dirty = true;
185721-
this.flush();
185743+
this.scheduleFlush();
185722185744
return deleted;
185723185745
}
185724185746
};
@@ -185870,7 +185892,7 @@ var SessionDataManager = class {
185870185892
}
185871185893
/**
185872185894
* Initialize the database and ensure it's properly set up.
185873-
* Must be awaited before any session operations (sql.js WASM init is async).
185895+
* Must be awaited before any session operations (sql.js init is async).
185874185896
*/
185875185897
async initialize() {
185876185898
try {
@@ -190011,7 +190033,7 @@ var codeqlGenerateQueryHelpTool = {
190011190033
};
190012190034

190013190035
// src/tools/codeql/list-databases.ts
190014-
import { existsSync as existsSync9, readdirSync as readdirSync4, readFileSync as readFileSync8, statSync as statSync5 } from "fs";
190036+
import { existsSync as existsSync9, readdirSync as readdirSync4, readFileSync as readFileSync8, statSync as statSync4 } from "fs";
190015190037
import { join as join12 } from "path";
190016190038

190017190039
// src/lib/discovery-config.ts
@@ -190076,7 +190098,7 @@ async function discoverDatabases(baseDirs, language) {
190076190098
for (const entry of entries) {
190077190099
const entryPath = join12(baseDir, entry);
190078190100
try {
190079-
if (!statSync5(entryPath).isDirectory()) {
190101+
if (!statSync4(entryPath).isDirectory()) {
190080190102
continue;
190081190103
}
190082190104
} catch {
@@ -190165,7 +190187,7 @@ function registerListDatabasesTool(server) {
190165190187
}
190166190188

190167190189
// src/tools/codeql/list-mrva-run-results.ts
190168-
import { existsSync as existsSync10, readdirSync as readdirSync5, readFileSync as readFileSync9, statSync as statSync6 } from "fs";
190190+
import { existsSync as existsSync10, readdirSync as readdirSync5, readFileSync as readFileSync9, statSync as statSync5 } from "fs";
190169190191
import { join as join13 } from "path";
190170190192
init_logger();
190171190193
var NUMERIC_DIR_PATTERN = /^\d+$/;
@@ -190185,7 +190207,7 @@ async function discoverMrvaRunResults(resultsDirs, runId) {
190185190207
for (const entry of entries) {
190186190208
const entryPath = join13(dir, entry);
190187190209
try {
190188-
if (!statSync6(entryPath).isDirectory()) {
190210+
if (!statSync5(entryPath).isDirectory()) {
190189190211
continue;
190190190212
}
190191190213
} catch {
@@ -190230,7 +190252,7 @@ function discoverRepoResults(runPath) {
190230190252
}
190231190253
const ownerPath = join13(runPath, ownerEntry);
190232190254
try {
190233-
if (!statSync6(ownerPath).isDirectory()) {
190255+
if (!statSync5(ownerPath).isDirectory()) {
190234190256
continue;
190235190257
}
190236190258
} catch {
@@ -190245,7 +190267,7 @@ function discoverRepoResults(runPath) {
190245190267
for (const repoEntry of repoEntries) {
190246190268
const repoPath = join13(ownerPath, repoEntry);
190247190269
try {
190248-
if (!statSync6(repoPath).isDirectory()) {
190270+
if (!statSync5(repoPath).isDirectory()) {
190249190271
continue;
190250190272
}
190251190273
} catch {
@@ -190352,7 +190374,7 @@ function registerListMrvaRunResultsTool(server) {
190352190374
}
190353190375

190354190376
// src/tools/codeql/list-query-run-results.ts
190355-
import { existsSync as existsSync11, readdirSync as readdirSync6, readFileSync as readFileSync10, statSync as statSync7 } from "fs";
190377+
import { existsSync as existsSync11, readdirSync as readdirSync6, readFileSync as readFileSync10, statSync as statSync6 } from "fs";
190356190378
import { join as join14 } from "path";
190357190379
init_logger();
190358190380
var QUERY_RUN_DIR_PATTERN = /^(.+\.ql)-(.+)$/;
@@ -190401,7 +190423,7 @@ async function discoverQueryRunResults(resultsDirs, filter) {
190401190423
for (const entry of entries) {
190402190424
const entryPath = join14(dir, entry);
190403190425
try {
190404-
if (!statSync7(entryPath).isDirectory()) {
190426+
if (!statSync6(entryPath).isDirectory()) {
190405190427
continue;
190406190428
}
190407190429
} catch {
@@ -191541,7 +191563,7 @@ function registerQuickEvaluateTool(server) {
191541191563

191542191564
// src/tools/codeql/read-database-source.ts
191543191565
var import_adm_zip = __toESM(require_adm_zip(), 1);
191544-
import { existsSync as existsSync14, readdirSync as readdirSync7, readFileSync as readFileSync11, statSync as statSync8 } from "fs";
191566+
import { existsSync as existsSync14, readdirSync as readdirSync7, readFileSync as readFileSync11, statSync as statSync7 } from "fs";
191545191567
import { join as join18, resolve as resolve7 } from "path";
191546191568
import { fileURLToPath as fileURLToPath2 } from "url";
191547191569
init_logger();
@@ -191560,7 +191582,7 @@ function toFilesystemPath(uri) {
191560191582
function* walkDirectory(dir, base = dir) {
191561191583
for (const entry of readdirSync7(dir)) {
191562191584
const fullPath = join18(dir, entry);
191563-
if (statSync8(fullPath).isDirectory()) {
191585+
if (statSync7(fullPath).isDirectory()) {
191564191586
yield* walkDirectory(fullPath, base);
191565191587
} else {
191566191588
yield fullPath.slice(base.length).replace(/\\/g, "/").replace(/^\//, "");
@@ -192051,7 +192073,7 @@ var codeqlResolveTestsTool = {
192051192073
};
192052192074

192053192075
// src/tools/codeql/search-ql-code.ts
192054-
import { closeSync, createReadStream as createReadStream3, fstatSync, lstatSync, openSync, readdirSync as readdirSync8, readFileSync as readFileSync12, realpathSync } from "fs";
192076+
import { closeSync, createReadStream as createReadStream3, fstatSync as fstatSync2, lstatSync, openSync as openSync2, readdirSync as readdirSync8, readFileSync as readFileSync12, realpathSync } from "fs";
192055192077
import { basename as basename8, extname as extname2, join as join19, resolve as resolve9 } from "path";
192056192078
import { createInterface as createInterface3 } from "readline";
192057192079
init_logger();
@@ -192108,13 +192130,13 @@ function collectFiles(paths, extensions, fileCount) {
192108192130
async function searchFile(filePath, regex, contextLines, maxCollect) {
192109192131
let fd;
192110192132
try {
192111-
fd = openSync(filePath, "r");
192133+
fd = openSync2(filePath, "r");
192112192134
} catch {
192113192135
return { matches: [], totalCount: 0 };
192114192136
}
192115192137
let size;
192116192138
try {
192117-
size = fstatSync(fd).size;
192139+
size = fstatSync2(fd).size;
192118192140
} catch {
192119192141
try {
192120192142
closeSync(fd);
@@ -195541,12 +195563,13 @@ function registerQueryResultsCacheRetrieveTool(server) {
195541195563
{
195542195564
cacheKey: external_exports.string().describe("The cache key of the result to retrieve."),
195543195565
lineRange: external_exports.tuple([external_exports.number(), external_exports.number()]).optional().describe("Line range [start, end] (1-indexed). For graphtext/CSV output."),
195544-
resultIndices: external_exports.tuple([external_exports.number(), external_exports.number()]).optional().describe("SARIF result index range [start, end] (0-indexed)."),
195566+
resultIndices: external_exports.tuple([external_exports.number(), external_exports.number()]).optional().describe("SARIF result index range [start, end] (0-indexed, inclusive)."),
195545195567
fileFilter: external_exports.string().optional().describe("For SARIF: only include results whose file path contains this string."),
195546195568
grep: external_exports.string().optional().describe("Text search filter: only include lines/results containing this term."),
195547-
maxLines: external_exports.number().optional().describe("Maximum number of lines to return (default: 500).")
195569+
maxLines: external_exports.number().optional().describe("Maximum number of lines to return for line-based formats (default: 500)."),
195570+
maxResults: external_exports.number().optional().describe("Maximum number of SARIF results to return (default: 100).")
195548195571
},
195549-
async ({ cacheKey: cacheKey2, lineRange, resultIndices, fileFilter, grep, maxLines }) => {
195572+
async ({ cacheKey: cacheKey2, lineRange, resultIndices, fileFilter, grep, maxLines, maxResults }) => {
195550195573
const store = sessionDataManager.getStore();
195551195574
const meta = store.getCacheEntryMeta(cacheKey2);
195552195575
if (!meta) {
@@ -195557,7 +195580,7 @@ function registerQueryResultsCacheRetrieveTool(server) {
195557195580
const subset2 = store.getCacheSarifSubset(cacheKey2, {
195558195581
resultIndices,
195559195582
fileFilter,
195560-
maxResults: maxLines
195583+
maxResults
195561195584
});
195562195585
if (!subset2) {
195563195586
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)