Skip to content

Commit e540330

Browse files
committed
Address second round of PR #169 review comments
- Bound metadataCache to 256 entries with oldest-first eviction - Make SqliteStore.initialize() idempotent (close existing db first) - Fix TOCTOU in initialize(): try readFileSync directly instead of existsSync - Atomic flush: write to temp file + renameSync to prevent corruption - Clarify annotation_search uses substring LIKE matching, not FTS - Close store in monitoring-tools test afterEach to prevent timer leaks
1 parent 9c1d456 commit e540330

File tree

6 files changed

+88
-59
lines changed

6 files changed

+88
-59
lines changed

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

Lines changed: 52 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -184863,6 +184863,7 @@ var BUILT_IN_EVALUATORS = {
184863184863
"csv-decode": "CSV format decoder for query results",
184864184864
"mermaid-graph": "Mermaid diagram generator for @kind graph queries (like PrintAST)"
184865184865
};
184866+
var METADATA_CACHE_MAX = 256;
184866184867
var metadataCache = /* @__PURE__ */ new Map();
184867184868
async function extractQueryMetadata(queryPath) {
184868184869
try {
@@ -184886,6 +184887,10 @@ async function extractQueryMetadata(queryPath) {
184886184887
if (tagsMatch) {
184887184888
metadata.tags = tagsMatch[1].split(/\s+/).filter((t) => t.length > 0);
184888184889
}
184890+
if (metadataCache.size >= METADATA_CACHE_MAX) {
184891+
const firstKey = metadataCache.keys().next().value;
184892+
if (firstKey !== void 0) metadataCache.delete(firstKey);
184893+
}
184889184894
metadataCache.set(queryPath, { mtime, metadata });
184890184895
return metadata;
184891184896
} catch (error2) {
@@ -185129,7 +185134,7 @@ import { randomUUID as randomUUID2 } from "crypto";
185129185134
// src/lib/sqlite-store.ts
185130185135
var import_sql_asm = __toESM(require_sql_asm(), 1);
185131185136
init_logger();
185132-
import { existsSync as existsSync6, mkdirSync as mkdirSync5, readFileSync as readFileSync5, writeFileSync as writeFileSync2 } from "fs";
185137+
import { mkdirSync as mkdirSync5, readFileSync as readFileSync5, renameSync, writeFileSync as writeFileSync2 } from "fs";
185133185138
import { join as join8 } from "path";
185134185139
var SqliteStore = class _SqliteStore {
185135185140
db = null;
@@ -185145,19 +185150,18 @@ var SqliteStore = class _SqliteStore {
185145185150
}
185146185151
/**
185147185152
* Initialize the database. Must be called (and awaited) before any other method.
185153+
* Safe to call more than once — an already-open database is closed first.
185148185154
*/
185149185155
async initialize() {
185156+
if (this.db) {
185157+
this.close();
185158+
}
185150185159
mkdirSync5(this.storageDir, { recursive: true });
185151185160
const SQL = await (0, import_sql_asm.default)();
185152-
if (existsSync6(this.dbPath)) {
185153-
try {
185154-
const fileBuffer = readFileSync5(this.dbPath);
185155-
this.db = new SQL.Database(fileBuffer);
185156-
} catch {
185157-
logger.warn("Failed to read existing database, creating new one");
185158-
this.db = new SQL.Database();
185159-
}
185160-
} else {
185161+
try {
185162+
const fileBuffer = readFileSync5(this.dbPath);
185163+
this.db = new SQL.Database(fileBuffer);
185164+
} catch {
185161185165
this.db = new SQL.Database();
185162185166
}
185163185167
this.createSchema();
@@ -185256,6 +185260,9 @@ var SqliteStore = class _SqliteStore {
185256185260
}
185257185261
/**
185258185262
* Write the in-memory database to disk.
185263+
*
185264+
* Uses write-to-temp + atomic rename so a crash mid-write cannot
185265+
* corrupt the database file.
185259185266
*/
185260185267
flush() {
185261185268
if (this.flushTimer) {
@@ -185265,7 +185272,9 @@ var SqliteStore = class _SqliteStore {
185265185272
if (!this.db) return;
185266185273
const data = this.db.export();
185267185274
const buffer = Buffer.from(data);
185268-
writeFileSync2(this.dbPath, buffer);
185275+
const tmpPath = this.dbPath + ".tmp";
185276+
writeFileSync2(tmpPath, buffer);
185277+
renameSync(tmpPath, this.dbPath);
185269185278
this.dirty = false;
185270185279
}
185271185280
/**
@@ -186391,7 +186400,7 @@ Warning: Query processing error - ${error2}`
186391186400
// src/lib/cli-tool-registry.ts
186392186401
init_package_paths();
186393186402
init_temp_dir();
186394-
import { writeFileSync as writeFileSync4, rmSync, existsSync as existsSync7, mkdirSync as mkdirSync8 } from "fs";
186403+
import { writeFileSync as writeFileSync4, rmSync, existsSync as existsSync6, mkdirSync as mkdirSync8 } from "fs";
186395186404
import { dirname as dirname5, isAbsolute as isAbsolute4, join as join10, resolve as resolve4 } from "path";
186396186405
var defaultCLIResultProcessor = (result, _params) => {
186397186406
if (!result.success) {
@@ -186718,7 +186727,7 @@ function registerCLITool(server, definition) {
186718186727
cwd = isAbsolute4(rawCwd) ? rawCwd : resolve4(getUserWorkspaceDir(), rawCwd);
186719186728
}
186720186729
const defaultExamplesPath = resolve4(packageRootDir, "ql", "javascript", "examples");
186721-
const additionalPacksPath = process.env.CODEQL_ADDITIONAL_PACKS || (existsSync7(defaultExamplesPath) ? defaultExamplesPath : void 0);
186730+
const additionalPacksPath = process.env.CODEQL_ADDITIONAL_PACKS || (existsSync6(defaultExamplesPath) ? defaultExamplesPath : void 0);
186722186731
if (additionalPacksPath && (name === "codeql_test_run" || name === "codeql_query_run" || name === "codeql_query_compile" || name === "codeql_database_analyze")) {
186723186732
options["additional-packs"] = additionalPacksPath;
186724186733
}
@@ -186739,7 +186748,7 @@ function registerCLITool(server, definition) {
186739186748
}
186740186749
if ((name === "codeql_query_run" || name === "codeql_database_analyze") && result.success && queryLogDir) {
186741186750
const evalLogPath = options["evaluator-log"];
186742-
if (evalLogPath && existsSync7(evalLogPath)) {
186751+
if (evalLogPath && existsSync6(evalLogPath)) {
186743186752
try {
186744186753
const summaryPath = evalLogPath.replace(/\.jsonl$/, ".summary.jsonl");
186745186754
const summaryResult = await executeCodeQLCommand(
@@ -190045,7 +190054,7 @@ var codeqlGenerateQueryHelpTool = {
190045190054
};
190046190055

190047190056
// src/tools/codeql/list-databases.ts
190048-
import { existsSync as existsSync9, readdirSync as readdirSync4, readFileSync as readFileSync8, statSync as statSync4 } from "fs";
190057+
import { existsSync as existsSync8, readdirSync as readdirSync4, readFileSync as readFileSync8, statSync as statSync4 } from "fs";
190049190058
import { join as join12 } from "path";
190050190059

190051190060
// src/lib/discovery-config.ts
@@ -190098,7 +190107,7 @@ function parseDatabaseYml(ymlPath) {
190098190107
async function discoverDatabases(baseDirs, language) {
190099190108
const databases = [];
190100190109
for (const baseDir of baseDirs) {
190101-
if (!existsSync9(baseDir)) {
190110+
if (!existsSync8(baseDir)) {
190102190111
continue;
190103190112
}
190104190113
let entries;
@@ -190117,7 +190126,7 @@ async function discoverDatabases(baseDirs, language) {
190117190126
continue;
190118190127
}
190119190128
const ymlPath = join12(entryPath, "codeql-database.yml");
190120-
if (!existsSync9(ymlPath)) {
190129+
if (!existsSync8(ymlPath)) {
190121190130
continue;
190122190131
}
190123190132
const metadata = parseDatabaseYml(ymlPath);
@@ -190199,15 +190208,15 @@ function registerListDatabasesTool(server) {
190199190208
}
190200190209

190201190210
// src/tools/codeql/list-mrva-run-results.ts
190202-
import { existsSync as existsSync10, readdirSync as readdirSync5, readFileSync as readFileSync9, statSync as statSync5 } from "fs";
190211+
import { existsSync as existsSync9, readdirSync as readdirSync5, readFileSync as readFileSync9, statSync as statSync5 } from "fs";
190203190212
import { join as join13 } from "path";
190204190213
init_logger();
190205190214
var NUMERIC_DIR_PATTERN = /^\d+$/;
190206190215
var SKIP_DIRS = /* @__PURE__ */ new Set([".DS_Store", "exported-results"]);
190207190216
async function discoverMrvaRunResults(resultsDirs, runId) {
190208190217
const results = [];
190209190218
for (const dir of resultsDirs) {
190210-
if (!existsSync10(dir)) {
190219+
if (!existsSync9(dir)) {
190211190220
continue;
190212190221
}
190213190222
let entries;
@@ -190233,7 +190242,7 @@ async function discoverMrvaRunResults(resultsDirs, runId) {
190233190242
}
190234190243
let timestamp2;
190235190244
const timestampPath = join13(entryPath, "timestamp");
190236-
if (existsSync10(timestampPath)) {
190245+
if (existsSync9(timestampPath)) {
190237190246
try {
190238190247
timestamp2 = readFileSync9(timestampPath, "utf-8").trim();
190239190248
} catch {
@@ -190289,7 +190298,7 @@ function discoverRepoResults(runPath) {
190289190298
let analysisStatus;
190290190299
let resultCount;
190291190300
const repoTaskPath = join13(repoPath, "repo_task.json");
190292-
if (existsSync10(repoTaskPath)) {
190301+
if (existsSync9(repoTaskPath)) {
190293190302
try {
190294190303
const raw = readFileSync9(repoTaskPath, "utf-8");
190295190304
const task = JSON.parse(raw);
@@ -190302,8 +190311,8 @@ function discoverRepoResults(runPath) {
190302190311
} catch {
190303190312
}
190304190313
}
190305-
const hasSarif = existsSync10(join13(repoPath, "results", "results.sarif"));
190306-
const hasBqrs = existsSync10(join13(repoPath, "results", "results.bqrs"));
190314+
const hasSarif = existsSync9(join13(repoPath, "results", "results.sarif"));
190315+
const hasBqrs = existsSync9(join13(repoPath, "results", "results.bqrs"));
190307190316
repos.push({
190308190317
analysisStatus,
190309190318
fullName,
@@ -190386,7 +190395,7 @@ function registerListMrvaRunResultsTool(server) {
190386190395
}
190387190396

190388190397
// src/tools/codeql/list-query-run-results.ts
190389-
import { existsSync as existsSync11, readdirSync as readdirSync6, readFileSync as readFileSync10, statSync as statSync6 } from "fs";
190398+
import { existsSync as existsSync10, readdirSync as readdirSync6, readFileSync as readFileSync10, statSync as statSync6 } from "fs";
190390190399
import { join as join14 } from "path";
190391190400
init_logger();
190392190401
var QUERY_RUN_DIR_PATTERN = /^(.+\.ql)-(.+)$/;
@@ -190423,7 +190432,7 @@ async function discoverQueryRunResults(resultsDirs, filter) {
190423190432
const normalizedFilter = typeof filter === "string" ? { queryName: filter } : filter;
190424190433
const results = [];
190425190434
for (const dir of resultsDirs) {
190426-
if (!existsSync11(dir)) {
190435+
if (!existsSync10(dir)) {
190427190436
continue;
190428190437
}
190429190438
let entries;
@@ -190449,14 +190458,14 @@ async function discoverQueryRunResults(resultsDirs, filter) {
190449190458
if (normalizedFilter?.queryName && name !== normalizedFilter.queryName) {
190450190459
continue;
190451190460
}
190452-
const hasEvaluatorLog = existsSync11(join14(entryPath, "evaluator-log.jsonl"));
190453-
const hasBqrs = existsSync11(join14(entryPath, "results.bqrs"));
190454-
const hasSarif = existsSync11(join14(entryPath, "results-interpreted.sarif"));
190455-
const hasQueryLog = existsSync11(join14(entryPath, "query.log"));
190456-
const hasSummaryLog = existsSync11(join14(entryPath, "evaluator-log.summary.jsonl"));
190461+
const hasEvaluatorLog = existsSync10(join14(entryPath, "evaluator-log.jsonl"));
190462+
const hasBqrs = existsSync10(join14(entryPath, "results.bqrs"));
190463+
const hasSarif = existsSync10(join14(entryPath, "results-interpreted.sarif"));
190464+
const hasQueryLog = existsSync10(join14(entryPath, "query.log"));
190465+
const hasSummaryLog = existsSync10(join14(entryPath, "evaluator-log.summary.jsonl"));
190457190466
let timestamp2;
190458190467
const timestampPath = join14(entryPath, "timestamp");
190459-
if (existsSync11(timestampPath)) {
190468+
if (existsSync10(timestampPath)) {
190460190469
try {
190461190470
timestamp2 = readFileSync10(timestampPath, "utf-8").trim();
190462190471
} catch {
@@ -190638,7 +190647,7 @@ var codeqlPackLsTool = {
190638190647
};
190639190648

190640190649
// src/tools/codeql/profile-codeql-query-from-logs.ts
190641-
import { existsSync as existsSync12, mkdirSync as mkdirSync9, writeFileSync as writeFileSync5 } from "fs";
190650+
import { existsSync as existsSync11, mkdirSync as mkdirSync9, writeFileSync as writeFileSync5 } from "fs";
190642190651
import { basename as basename6, dirname as dirname7, join as join15 } from "path";
190643190652

190644190653
// src/lib/evaluator-log-parser.ts
@@ -191075,7 +191084,7 @@ function registerProfileCodeQLQueryFromLogsTool(server) {
191075191084
try {
191076191085
const { evaluatorLog, outputDir, topN } = params;
191077191086
const effectiveTopN = topN ?? 20;
191078-
if (!existsSync12(evaluatorLog)) {
191087+
if (!existsSync11(evaluatorLog)) {
191079191088
return {
191080191089
content: [
191081191090
{
@@ -191128,7 +191137,7 @@ function registerProfileCodeQLQueryFromLogsTool(server) {
191128191137
// src/tools/codeql/profile-codeql-query.ts
191129191138
init_cli_executor();
191130191139
init_logger();
191131-
import { writeFileSync as writeFileSync6, existsSync as existsSync13 } from "fs";
191140+
import { writeFileSync as writeFileSync6, existsSync as existsSync12 } from "fs";
191132191141
import { createReadStream as createReadStream2 } from "fs";
191133191142
import { join as join16, dirname as dirname8, basename as basename7 } from "path";
191134191143
import { mkdirSync as mkdirSync10 } from "fs";
@@ -191331,7 +191340,7 @@ function registerProfileCodeQLQueryTool(server) {
191331191340
isError: true
191332191341
};
191333191342
}
191334-
if (existsSync13(bqrsPath)) {
191343+
if (existsSync12(bqrsPath)) {
191335191344
try {
191336191345
const sarifResult = await executeCodeQLCommand(
191337191346
"bqrs interpret",
@@ -191346,7 +191355,7 @@ function registerProfileCodeQLQueryTool(server) {
191346191355
}
191347191356
}
191348191357
}
191349-
if (!existsSync13(logPath)) {
191358+
if (!existsSync12(logPath)) {
191350191359
return {
191351191360
content: [
191352191361
{
@@ -191377,7 +191386,7 @@ function registerProfileCodeQLQueryTool(server) {
191377191386
if (bqrsPath) {
191378191387
outputFiles.push(`Query Results (BQRS): ${bqrsPath}`);
191379191388
}
191380-
if (sarifPath && existsSync13(sarifPath)) {
191389+
if (sarifPath && existsSync12(sarifPath)) {
191381191390
outputFiles.push(`Query Results (SARIF): ${sarifPath}`);
191382191391
}
191383191392
const responseText = [
@@ -191575,7 +191584,7 @@ function registerQuickEvaluateTool(server) {
191575191584

191576191585
// src/tools/codeql/read-database-source.ts
191577191586
var import_adm_zip = __toESM(require_adm_zip(), 1);
191578-
import { existsSync as existsSync14, readdirSync as readdirSync7, readFileSync as readFileSync11, statSync as statSync7 } from "fs";
191587+
import { existsSync as existsSync13, readdirSync as readdirSync7, readFileSync as readFileSync11, statSync as statSync7 } from "fs";
191579191588
import { join as join18, resolve as resolve7 } from "path";
191580191589
import { fileURLToPath as fileURLToPath2 } from "url";
191581191590
init_logger();
@@ -191657,13 +191666,13 @@ function applyLineRange(content, startLine, endLine) {
191657191666
async function readDatabaseSource(params) {
191658191667
const { databasePath, endLine, filePath, maxEntries, prefix, startLine } = params;
191659191668
const resolvedDbPath = resolve7(databasePath);
191660-
if (!existsSync14(resolvedDbPath)) {
191669+
if (!existsSync13(resolvedDbPath)) {
191661191670
throw new Error(`Database path does not exist: ${databasePath}`);
191662191671
}
191663191672
const srcZipPath = join18(resolvedDbPath, "src.zip");
191664191673
const srcDirPath = join18(resolvedDbPath, "src");
191665-
const hasSrcZip = existsSync14(srcZipPath);
191666-
const hasSrcDir = existsSync14(srcDirPath);
191674+
const hasSrcZip = existsSync13(srcZipPath);
191675+
const hasSrcDir = existsSync13(srcDirPath);
191667191676
if (!hasSrcZip && !hasSrcDir) {
191668191677
throw new Error(
191669191678
`No source archive found in database: expected src.zip or src/ in ${databasePath}`
@@ -195346,9 +195355,9 @@ function registerAnnotationDeleteTool(server) {
195346195355
function registerAnnotationSearchTool(server) {
195347195356
server.tool(
195348195357
"annotation_search",
195349-
"Full-text search across annotation content, metadata, and labels.",
195358+
"Substring search across annotation content, metadata, and labels (case-insensitive SQL LIKE matching).",
195350195359
{
195351-
query: external_exports.string().describe("Search term (matched against content, metadata, and label)."),
195360+
query: external_exports.string().describe("Search term \u2014 matched as a substring against content, metadata, and label."),
195352195361
category: external_exports.string().optional().describe("Restrict search to a specific category."),
195353195362
limit: external_exports.number().optional().describe("Maximum number of results (default: 50).")
195354195363
},

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: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ export type BuiltInEvaluator = keyof typeof BUILT_IN_EVALUATORS;
3737
/**
3838
* In-memory cache for extracted query metadata, keyed by file path.
3939
* Stores the file modification time to invalidate when the file changes.
40+
* Bounded to {@link METADATA_CACHE_MAX} entries; oldest entries are evicted
41+
* when the limit is reached (Map iteration order = insertion order).
4042
*/
43+
const METADATA_CACHE_MAX = 256;
4144
const metadataCache = new Map<string, { mtime: number; metadata: QueryMetadata }>();
4245

4346
/**
@@ -75,6 +78,11 @@ export async function extractQueryMetadata(queryPath: string): Promise<QueryMeta
7578
metadata.tags = tagsMatch[1].split(/\s+/).filter(t => t.length > 0);
7679
}
7780

81+
// Evict oldest entries when the cache exceeds the size limit.
82+
if (metadataCache.size >= METADATA_CACHE_MAX) {
83+
const firstKey = metadataCache.keys().next().value;
84+
if (firstKey !== undefined) metadataCache.delete(firstKey);
85+
}
7886
metadataCache.set(queryPath, { mtime, metadata });
7987
return metadata;
8088
} catch (error) {

server/src/lib/sqlite-store.ts

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
import initSqlJs from 'sql.js/dist/sql-asm.js';
1515
import type { Database as SqlJsDatabase } from 'sql.js';
16-
import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'fs';
16+
import { mkdirSync, readFileSync, renameSync, writeFileSync } from 'fs';
1717
import { join } from 'path';
1818
import { logger } from '../utils/logger';
1919

@@ -65,21 +65,22 @@ export class SqliteStore {
6565

6666
/**
6767
* Initialize the database. Must be called (and awaited) before any other method.
68+
* Safe to call more than once — an already-open database is closed first.
6869
*/
6970
async initialize(): Promise<void> {
71+
if (this.db) {
72+
this.close();
73+
}
74+
7075
mkdirSync(this.storageDir, { recursive: true });
7176

7277
const SQL = await initSqlJs();
7378

74-
if (existsSync(this.dbPath)) {
75-
try {
76-
const fileBuffer = readFileSync(this.dbPath);
77-
this.db = new SQL.Database(fileBuffer);
78-
} catch {
79-
logger.warn('Failed to read existing database, creating new one');
80-
this.db = new SQL.Database();
81-
}
82-
} else {
79+
try {
80+
const fileBuffer = readFileSync(this.dbPath);
81+
this.db = new SQL.Database(fileBuffer);
82+
} catch {
83+
// File doesn't exist or is unreadable — start with a fresh database.
8384
this.db = new SQL.Database();
8485
}
8586

@@ -194,6 +195,9 @@ export class SqliteStore {
194195

195196
/**
196197
* Write the in-memory database to disk.
198+
*
199+
* Uses write-to-temp + atomic rename so a crash mid-write cannot
200+
* corrupt the database file.
197201
*/
198202
flush(): void {
199203
if (this.flushTimer) {
@@ -203,7 +207,9 @@ export class SqliteStore {
203207
if (!this.db) return;
204208
const data = this.db.export();
205209
const buffer = Buffer.from(data);
206-
writeFileSync(this.dbPath, buffer);
210+
const tmpPath = this.dbPath + '.tmp';
211+
writeFileSync(tmpPath, buffer);
212+
renameSync(tmpPath, this.dbPath);
207213
this.dirty = false;
208214
}
209215

0 commit comments

Comments
 (0)