mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-29 06:32:28 +00:00
fix: Use Ladybug native read-only enforcement and prepared statement execution for Cypher query paths (#1655)
This commit is contained in:
parent
5e3a67f48f
commit
6d25d39b59
24 changed files with 499 additions and 456 deletions
|
|
@ -154,6 +154,7 @@ export const splitRelCsvByLabelPair = async (
|
|||
let db: lbug.Database | null = null;
|
||||
let conn: lbug.Connection | null = null;
|
||||
let currentDbPath: string | null = null;
|
||||
let currentDbReadOnly = false;
|
||||
let ftsLoaded = false;
|
||||
let vectorExtensionLoaded = false;
|
||||
|
||||
|
|
@ -448,12 +449,17 @@ export const initLbug = async (dbPath: string) => {
|
|||
* database is busy (e.g. `gitnexus analyze` holds the write lock).
|
||||
* Each retry waits DB_LOCK_RETRY_DELAY_MS * attempt milliseconds.
|
||||
*/
|
||||
export const withLbugDb = async <T>(dbPath: string, operation: () => Promise<T>): Promise<T> => {
|
||||
export const withLbugDb = async <T>(
|
||||
dbPath: string,
|
||||
operation: () => Promise<T>,
|
||||
options: { readOnly?: boolean } = {},
|
||||
): Promise<T> => {
|
||||
let lastError: unknown;
|
||||
const readOnly = options.readOnly === true;
|
||||
for (let attempt = 1; attempt <= DB_LOCK_RETRY_ATTEMPTS; attempt++) {
|
||||
try {
|
||||
return await runWithSessionLock(async () => {
|
||||
await ensureLbugInitialized(dbPath);
|
||||
await ensureLbugInitialized(dbPath, readOnly);
|
||||
return operation();
|
||||
});
|
||||
} catch (err) {
|
||||
|
|
@ -483,15 +489,15 @@ export const withLbugDb = async <T>(dbPath: string, operation: () => Promise<T>)
|
|||
throw lastError;
|
||||
};
|
||||
|
||||
const ensureLbugInitialized = async (dbPath: string) => {
|
||||
if (conn && currentDbPath === dbPath) {
|
||||
const ensureLbugInitialized = async (dbPath: string, readOnly: boolean = false) => {
|
||||
if (conn && currentDbPath === dbPath && currentDbReadOnly === readOnly) {
|
||||
return { db, conn };
|
||||
}
|
||||
await doInitLbug(dbPath);
|
||||
await doInitLbug(dbPath, readOnly);
|
||||
return { db, conn };
|
||||
};
|
||||
|
||||
const doInitLbug = async (dbPath: string) => {
|
||||
const doInitLbug = async (dbPath: string, readOnly: boolean = false) => {
|
||||
// Different database requested — close the old one first
|
||||
if (conn || db) {
|
||||
await safeClose();
|
||||
|
|
@ -575,9 +581,12 @@ const doInitLbug = async (dbPath: string) => {
|
|||
const parentDir = path.dirname(dbPath);
|
||||
await fs.mkdir(parentDir, { recursive: true });
|
||||
|
||||
const opened = await openLbugConnection(lbug, dbPath);
|
||||
const opened = readOnly
|
||||
? await openLbugConnection(lbug, dbPath, { readOnly: true })
|
||||
: await openLbugConnection(lbug, dbPath);
|
||||
db = opened.db;
|
||||
conn = opened.conn;
|
||||
currentDbReadOnly = readOnly;
|
||||
} finally {
|
||||
await releaseInitLock();
|
||||
}
|
||||
|
|
@ -614,7 +623,7 @@ const doInitLbug = async (dbPath: string) => {
|
|||
` Original error: ${msg.slice(0, 200)}`,
|
||||
);
|
||||
}
|
||||
if (!msg.includes('already exists') && !isDbBusyError(err)) {
|
||||
if (!msg.includes('already exists') && !isDbBusyError(err) && !isReadOnlyDbError(err)) {
|
||||
logger.warn(`⚠️ Schema creation warning: ${msg.slice(0, 120)}`);
|
||||
}
|
||||
}
|
||||
|
|
@ -1058,12 +1067,7 @@ export const batchInsertNodesToLbug = async (
|
|||
};
|
||||
|
||||
export const executeQuery = async (cypher: string): Promise<any[]> => {
|
||||
if (!conn) {
|
||||
throw new Error('LadybugDB not initialized. Call initLbug first.');
|
||||
}
|
||||
|
||||
const queryResult = await conn.query(cypher);
|
||||
return await readQueryRows(queryResult);
|
||||
return await executePrepared(cypher, {});
|
||||
};
|
||||
|
||||
export const streamQuery = async (
|
||||
|
|
@ -1726,19 +1730,15 @@ export const queryFTS = async (
|
|||
throw new Error('LadybugDB not initialized. Call initLbug first.');
|
||||
}
|
||||
|
||||
// Escape backslashes and single quotes to prevent Cypher injection
|
||||
const escapedQuery = query.replace(/\\/g, '\\\\').replace(/'/g, "''");
|
||||
|
||||
const cypher = `
|
||||
CALL QUERY_FTS_INDEX('${tableName}', '${indexName}', '${escapedQuery}', conjunctive := ${conjunctive})
|
||||
CALL QUERY_FTS_INDEX('${tableName}', '${indexName}', $query, conjunctive := ${conjunctive})
|
||||
RETURN node, score
|
||||
ORDER BY score DESC
|
||||
LIMIT ${limit}
|
||||
`;
|
||||
|
||||
try {
|
||||
const queryResult = await conn.query(cypher);
|
||||
const rows = await readQueryRows(queryResult);
|
||||
const rows = await executePrepared(cypher, { query });
|
||||
|
||||
return rows.map((row: any) => {
|
||||
const node = row.node || row[0] || {};
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@
|
|||
|
||||
import fs from 'fs/promises';
|
||||
import lbug from '@ladybugdb/core';
|
||||
import { loadFTSExtension } from './lbug-adapter.js';
|
||||
import { isReadOnlyDbError, loadFTSExtension } from './lbug-adapter.js';
|
||||
import {
|
||||
createLbugDatabase,
|
||||
isWalCorruptionError,
|
||||
|
|
@ -598,30 +598,7 @@ function withTimeout<T>(promise: Promise<T>, ms: number, label: string): Promise
|
|||
}
|
||||
|
||||
export const executeQuery = async (repoId: string, cypher: string): Promise<any[]> => {
|
||||
const entry = pool.get(repoId);
|
||||
if (!entry) {
|
||||
throw new Error(`LadybugDB not initialized for repo "${repoId}". Call initLbug first.`);
|
||||
}
|
||||
|
||||
if (isWriteQuery(cypher)) {
|
||||
throw new Error('Write operations are not allowed. The pool adapter is read-only.');
|
||||
}
|
||||
|
||||
entry.lastUsed = Date.now();
|
||||
|
||||
const conn = await checkout(entry);
|
||||
silenceStdout();
|
||||
activeQueryCount++;
|
||||
try {
|
||||
const queryResult = await withTimeout(conn.query(cypher), QUERY_TIMEOUT_MS, 'Query');
|
||||
const result = Array.isArray(queryResult) ? queryResult[0] : queryResult;
|
||||
const rows = await result.getAll();
|
||||
return rows;
|
||||
} finally {
|
||||
activeQueryCount--;
|
||||
restoreStdout();
|
||||
checkin(entry, conn);
|
||||
}
|
||||
return await executeParameterized(repoId, cypher, {});
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
@ -653,6 +630,11 @@ export const executeParameterized = async (
|
|||
const result = Array.isArray(queryResult) ? queryResult[0] : queryResult;
|
||||
const rows = await result.getAll();
|
||||
return rows;
|
||||
} catch (err) {
|
||||
if (isReadOnlyDbError(err)) {
|
||||
throw new Error('Write operations are not allowed. The pool adapter is read-only.');
|
||||
}
|
||||
throw err;
|
||||
} finally {
|
||||
activeQueryCount--;
|
||||
restoreStdout();
|
||||
|
|
@ -685,15 +667,3 @@ export const closeLbug = async (repoId?: string): Promise<void> => {
|
|||
* Check if a specific repo's pool is active
|
||||
*/
|
||||
export const isLbugReady = (repoId: string): boolean => pool.has(repoId);
|
||||
|
||||
/** Regex to detect write operations in user-supplied Cypher queries.
|
||||
* Note: CALL is NOT blocked — it's used for read-only FTS (CALL QUERY_FTS_INDEX)
|
||||
* and vector search (CALL QUERY_VECTOR_INDEX). The database is opened in
|
||||
* read-only mode as defense-in-depth against write procedures. */
|
||||
export const CYPHER_WRITE_RE =
|
||||
/(?<!:)\b(CREATE|DELETE|SET|MERGE|REMOVE|DROP|ALTER|COPY|DETACH|FOREACH|INSTALL|LOAD)\b/i;
|
||||
|
||||
/** Check if a Cypher query contains write operations */
|
||||
export function isWriteQuery(query: string): boolean {
|
||||
return CYPHER_WRITE_RE.test(query);
|
||||
}
|
||||
|
|
|
|||
24
gitnexus/src/core/lbug/query-params.ts
Normal file
24
gitnexus/src/core/lbug/query-params.ts
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
/**
|
||||
* Return true only for plain-object payloads that can be safely used as
|
||||
* named parameter maps in prepared Cypher execution.
|
||||
*
|
||||
* Validation criteria:
|
||||
* - must be a JavaScript object (`typeof value === 'object'`)
|
||||
* - must not be `null`
|
||||
* - must not be an array
|
||||
* - must have a plain-object prototype
|
||||
* - values must be scalar bindable values (string | number | boolean | null)
|
||||
*
|
||||
* Rationale: prepared-statement params are key/value maps; rejecting null/array
|
||||
* and non-plain objects keeps binding behavior predictable and avoids passing
|
||||
* complex host objects to Ladybug parameter binding.
|
||||
*/
|
||||
const isBindableScalar = (value: unknown): value is string | number | boolean | null =>
|
||||
value === null || ['string', 'number', 'boolean'].includes(typeof value);
|
||||
|
||||
export const isValidQueryParams = (value: unknown): value is Record<string, unknown> =>
|
||||
value !== null &&
|
||||
typeof value === 'object' &&
|
||||
!Array.isArray(value) &&
|
||||
(Object.getPrototypeOf(value) === Object.prototype || Object.getPrototypeOf(value) === null) &&
|
||||
Object.values(value).every(isBindableScalar);
|
||||
|
|
@ -27,22 +27,20 @@ export interface FTSSearchResponse {
|
|||
* caller can distinguish "zero matches" from "index missing".
|
||||
*/
|
||||
async function queryFTSViaExecutor(
|
||||
executor: (cypher: string) => Promise<any[]>,
|
||||
executor: (cypher: string, params: Record<string, any>) => Promise<any[]>,
|
||||
tableName: string,
|
||||
indexName: string,
|
||||
query: string,
|
||||
limit: number,
|
||||
): Promise<Array<{ filePath: string; score: number; nodeId: string }> | null> {
|
||||
// Escape single quotes and backslashes to prevent Cypher injection
|
||||
const escapedQuery = query.replace(/\\/g, '\\\\').replace(/'/g, "''");
|
||||
const cypher = `
|
||||
CALL QUERY_FTS_INDEX('${tableName}', '${indexName}', '${escapedQuery}', conjunctive := false)
|
||||
CALL QUERY_FTS_INDEX('${tableName}', '${indexName}', $query, conjunctive := false)
|
||||
RETURN node, score
|
||||
ORDER BY score DESC
|
||||
LIMIT ${limit}
|
||||
`;
|
||||
try {
|
||||
const rows = await executor(cypher);
|
||||
const rows = await executor(cypher, { query });
|
||||
return rows.map((row: any) => {
|
||||
const node = row.node || row[0] || {};
|
||||
const score = row.score ?? row[1] ?? 0;
|
||||
|
|
@ -81,8 +79,9 @@ export const searchFTSFromLbug = async (
|
|||
// IMPORTANT: FTS queries run sequentially to avoid connection contention.
|
||||
// The MCP pool supports multiple connections, but FTS is best run serially.
|
||||
const poolMod = await import('../lbug/pool-adapter.js');
|
||||
const { executeQuery } = poolMod;
|
||||
const executor = (cypher: string) => executeQuery(repoId, cypher);
|
||||
const { executeParameterized } = poolMod;
|
||||
const executor = (cypher: string, params: Record<string, any>) =>
|
||||
executeParameterized(repoId, cypher, params);
|
||||
|
||||
for (const { table, indexName } of FTS_INDEXES) {
|
||||
const result = await queryFTSViaExecutor(executor, table, indexName, query, limit);
|
||||
|
|
|
|||
|
|
@ -14,10 +14,9 @@ import {
|
|||
executeParameterized,
|
||||
closeLbug,
|
||||
isLbugReady,
|
||||
isWriteQuery,
|
||||
} from '../../core/lbug/pool-adapter.js';
|
||||
import { isValidQueryParams } from '../../core/lbug/query-params.js';
|
||||
import { isWalCorruptionError, WAL_RECOVERY_SUGGESTION } from '../../core/lbug/lbug-config.js';
|
||||
export { isWriteQuery };
|
||||
// Embedding imports are lazy (dynamic import) to avoid loading onnxruntime-node
|
||||
// at MCP server startup — crashes on unsupported Node ABI versions (#89)
|
||||
// git utilities available if needed
|
||||
|
|
@ -175,6 +174,9 @@ function logQueryError(context: string, err: unknown): void {
|
|||
logger.error({ context, err: msg }, 'GitNexus query failed');
|
||||
}
|
||||
|
||||
const isReadOnlyDbError = (err: unknown): boolean =>
|
||||
/read-only database/i.test(err instanceof Error ? err.message : String(err));
|
||||
|
||||
/**
|
||||
* Per-query latency telemetry for production aggregation (#553).
|
||||
*
|
||||
|
|
@ -1273,31 +1275,41 @@ export class LocalBackend {
|
|||
}
|
||||
}
|
||||
|
||||
async executeCypher(repoName: string, query: string): Promise<any> {
|
||||
async executeCypher(
|
||||
repoName: string,
|
||||
query: string,
|
||||
params: Record<string, unknown> = {},
|
||||
): Promise<any> {
|
||||
const repo = await this.resolveRepo(repoName);
|
||||
return this.cypher(repo, { query });
|
||||
return this.cypher(repo, { query, params });
|
||||
}
|
||||
|
||||
private async cypher(repo: RepoHandle, params: { query: string }): Promise<any> {
|
||||
private async cypher(
|
||||
repo: RepoHandle,
|
||||
request: { query: string; params?: Record<string, unknown> },
|
||||
): Promise<any> {
|
||||
await this.ensureInitialized(repo.id);
|
||||
|
||||
if (!isLbugReady(repo.id)) {
|
||||
return { error: 'LadybugDB not ready. Index may be corrupted.' };
|
||||
}
|
||||
|
||||
// Block write operations (defense-in-depth — DB is already read-only)
|
||||
if (isWriteQuery(params.query)) {
|
||||
if (request.params !== undefined && !isValidQueryParams(request.params)) {
|
||||
return {
|
||||
error:
|
||||
'Write operations (CREATE, DELETE, SET, MERGE, REMOVE, DROP, ALTER, COPY, DETACH) are not allowed. The knowledge graph is read-only.',
|
||||
error: '"params" must be a plain object with scalar values (string/number/boolean/null).',
|
||||
};
|
||||
}
|
||||
|
||||
try {
|
||||
const result = await executeQuery(repo.id, params.query);
|
||||
const result = await executeParameterized(repo.id, request.query, request.params ?? {});
|
||||
return result;
|
||||
} catch (err: any) {
|
||||
const msg = err.message || 'Query failed';
|
||||
if (isReadOnlyDbError(err)) {
|
||||
return {
|
||||
error:
|
||||
'Write operations (CREATE, DELETE, SET, MERGE, REMOVE, DROP, ALTER, COPY, DETACH) are not allowed. The knowledge graph is read-only.',
|
||||
};
|
||||
}
|
||||
if (isWalCorruptionError(err)) {
|
||||
return {
|
||||
error: msg,
|
||||
|
|
|
|||
|
|
@ -187,6 +187,11 @@ TIPS:
|
|||
type: 'object',
|
||||
properties: {
|
||||
query: { type: 'string', description: 'Cypher query to execute' },
|
||||
params: {
|
||||
type: 'object',
|
||||
description:
|
||||
'Optional query parameters for placeholders (e.g. $name) to execute via prepared statement binding.',
|
||||
},
|
||||
repo: {
|
||||
type: 'string',
|
||||
description: 'Repository name or path. Omit if only one repo is indexed.',
|
||||
|
|
|
|||
|
|
@ -22,8 +22,9 @@ import {
|
|||
flushWAL,
|
||||
closeLbug,
|
||||
withLbugDb,
|
||||
isReadOnlyDbError,
|
||||
} from '../core/lbug/lbug-adapter.js';
|
||||
import { isWriteQuery } from '../core/lbug/pool-adapter.js';
|
||||
import { isValidQueryParams } from '../core/lbug/query-params.js';
|
||||
import { NODE_TABLES, type GraphNode, type GraphRelationship } from 'gitnexus-shared';
|
||||
import { searchFTSFromLbug } from '../core/search/bm25-index.js';
|
||||
import { hybridSearch } from '../core/search/hybrid-search.js';
|
||||
|
|
@ -621,6 +622,44 @@ export const handleFileRequest = async (
|
|||
}
|
||||
};
|
||||
|
||||
export const handleQueryRequest = async (
|
||||
req: express.Request,
|
||||
res: express.Response,
|
||||
resolveRepo: (repoName?: string) => Promise<{ storagePath: string } | undefined>,
|
||||
): Promise<void> => {
|
||||
try {
|
||||
const cypher = req.body.cypher as string;
|
||||
if (!cypher) {
|
||||
res.status(400).json({ error: 'Missing "cypher" in request body' });
|
||||
return;
|
||||
}
|
||||
const queryParams = req.body.params;
|
||||
if (queryParams !== undefined && !isValidQueryParams(queryParams)) {
|
||||
res.status(400).json({
|
||||
error: '"params" must be a plain object with scalar values (string/number/boolean/null)',
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const entry = await resolveRepo(requestedRepo(req));
|
||||
if (!entry) {
|
||||
res.status(404).json({ error: 'Repository not found' });
|
||||
return;
|
||||
}
|
||||
const lbugPath = path.join(entry.storagePath, 'lbug');
|
||||
const result = await withLbugDb(lbugPath, () => executePrepared(cypher, queryParams ?? {}), {
|
||||
readOnly: true,
|
||||
});
|
||||
res.json({ result });
|
||||
} catch (err: any) {
|
||||
if (isReadOnlyDbError(err)) {
|
||||
res.status(403).json({ error: 'Write queries are not allowed via the HTTP API' });
|
||||
return;
|
||||
}
|
||||
res.status(500).json({ error: err.message || 'Query failed' });
|
||||
}
|
||||
};
|
||||
|
||||
export const createServer = async (port: number, host: string = '127.0.0.1') => {
|
||||
const app = express();
|
||||
app.disable('x-powered-by');
|
||||
|
|
@ -1020,29 +1059,7 @@ export const createServer = async (port: number, host: string = '127.0.0.1') =>
|
|||
|
||||
// Execute Cypher query
|
||||
app.post('/api/query', async (req, res) => {
|
||||
try {
|
||||
const cypher = req.body.cypher as string;
|
||||
if (!cypher) {
|
||||
res.status(400).json({ error: 'Missing "cypher" in request body' });
|
||||
return;
|
||||
}
|
||||
|
||||
if (isWriteQuery(cypher)) {
|
||||
res.status(403).json({ error: 'Write queries are not allowed via the HTTP API' });
|
||||
return;
|
||||
}
|
||||
|
||||
const entry = await resolveRepo(requestedRepo(req));
|
||||
if (!entry) {
|
||||
res.status(404).json({ error: 'Repository not found' });
|
||||
return;
|
||||
}
|
||||
const lbugPath = path.join(entry.storagePath, 'lbug');
|
||||
const result = await withLbugDb(lbugPath, () => executeQuery(cypher));
|
||||
res.json({ result });
|
||||
} catch (err: any) {
|
||||
res.status(500).json({ error: err.message || 'Query failed' });
|
||||
}
|
||||
await handleQueryRequest(req, res, resolveRepo);
|
||||
});
|
||||
|
||||
// Search (supports mode: 'hybrid' | 'semantic' | 'bm25', and optional enrichment)
|
||||
|
|
|
|||
5
gitnexus/test/helpers/ladybug-native.ts
Normal file
5
gitnexus/test/helpers/ladybug-native.ts
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
|
||||
export const hasLadybugNative = (): boolean =>
|
||||
fs.existsSync(path.join(process.cwd(), 'node_modules', '@ladybugdb', 'core', 'lbugjs.node'));
|
||||
|
|
@ -125,8 +125,9 @@ export function withTestLbugDB(
|
|||
// LadybugDB enforces file locks — writable + read-only can't coexist
|
||||
// on the same path, and db.close() segfaults on macOS due to N-API
|
||||
// destructor issues. Reusing the writable Database avoids both problems.
|
||||
// Write protection is enforced at the query validation layer (isWriteQuery)
|
||||
// rather than at the native DB level.
|
||||
// NOTE: This injected DB is writable by design for test setup.
|
||||
// Read-only enforcement tests must initialize a separate pool entry
|
||||
// via initLbug(...) so Ladybug native read-only mode is exercised.
|
||||
if (options?.poolAdapter) {
|
||||
const coreDb = adapter.getDatabase();
|
||||
if (!coreDb) throw new Error('withTestLbugDB: core adapter has no open Database');
|
||||
|
|
|
|||
97
gitnexus/test/integration/api-query.test.ts
Normal file
97
gitnexus/test/integration/api-query.test.ts
Normal file
|
|
@ -0,0 +1,97 @@
|
|||
import express from 'express';
|
||||
import http from 'node:http';
|
||||
import { describe, expect, it, beforeAll, afterAll } from 'vitest';
|
||||
import { withTestLbugDB } from '../helpers/test-indexed-db.js';
|
||||
import { hasLadybugNative } from '../helpers/ladybug-native.js';
|
||||
|
||||
const WRITE_QUERY_TEST_CYPHER =
|
||||
"CREATE (n:Function {id: 'api-write-test', name: 'api-write-test', filePath: '', startLine: 0, endLine: 0, isExported: false, content: '', description: ''})";
|
||||
|
||||
const startServer = (app: express.Express): Promise<{ server: http.Server; baseUrl: string }> =>
|
||||
new Promise((resolve) => {
|
||||
const server = app.listen(0, '127.0.0.1', () => {
|
||||
const addr = server.address();
|
||||
if (!addr || typeof addr === 'string') throw new Error('Failed to start test server');
|
||||
resolve({ server, baseUrl: `http://127.0.0.1:${addr.port}` });
|
||||
});
|
||||
});
|
||||
|
||||
const stopServer = (server: http.Server): Promise<void> =>
|
||||
new Promise((resolve, reject) => server.close((err) => (err ? reject(err) : resolve())));
|
||||
|
||||
withTestLbugDB(
|
||||
'api-query-http',
|
||||
(handle) => {
|
||||
describe.skipIf(!hasLadybugNative())('/api/query runtime contract', () => {
|
||||
let server: http.Server;
|
||||
let baseUrl = '';
|
||||
let handleQueryRequest: typeof import('../../src/server/api.js').handleQueryRequest;
|
||||
|
||||
beforeAll(async () => {
|
||||
({ handleQueryRequest } = await import('../../src/server/api.js'));
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.post('/api/query', async (req, res) => {
|
||||
await handleQueryRequest(req, res, async () => ({
|
||||
storagePath: handle.tmpHandle.dbPath,
|
||||
}));
|
||||
});
|
||||
({ server, baseUrl } = await startServer(app));
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await stopServer(server);
|
||||
});
|
||||
|
||||
it('returns 200 for a valid read query', async () => {
|
||||
const response = await fetch(`${baseUrl}/api/query`, {
|
||||
method: 'POST',
|
||||
headers: { 'content-type': 'application/json' },
|
||||
body: JSON.stringify({ cypher: 'RETURN 1 AS one' }),
|
||||
});
|
||||
expect(response.status).toBe(200);
|
||||
const body = await response.json();
|
||||
expect(Array.isArray(body.result)).toBe(true);
|
||||
expect(body.result[0].one).toBe(1);
|
||||
});
|
||||
|
||||
it('returns 403 for a write query on read-only HTTP path', async () => {
|
||||
const response = await fetch(`${baseUrl}/api/query`, {
|
||||
method: 'POST',
|
||||
headers: { 'content-type': 'application/json' },
|
||||
body: JSON.stringify({
|
||||
cypher: WRITE_QUERY_TEST_CYPHER,
|
||||
}),
|
||||
});
|
||||
expect(response.status).toBe(403);
|
||||
const body = await response.json();
|
||||
expect(body.error).toContain('Write queries are not allowed');
|
||||
});
|
||||
|
||||
it('returns 400 for invalid params payload', async () => {
|
||||
const response = await fetch(`${baseUrl}/api/query`, {
|
||||
method: 'POST',
|
||||
headers: { 'content-type': 'application/json' },
|
||||
body: JSON.stringify({ cypher: 'RETURN 1 AS one', params: [1, 2, 3] }),
|
||||
});
|
||||
expect(response.status).toBe(400);
|
||||
const body = await response.json();
|
||||
expect(body.error).toContain('"params"');
|
||||
});
|
||||
|
||||
it('returns 400 when cypher is missing', async () => {
|
||||
const response = await fetch(`${baseUrl}/api/query`, {
|
||||
method: 'POST',
|
||||
headers: { 'content-type': 'application/json' },
|
||||
body: JSON.stringify({}),
|
||||
});
|
||||
expect(response.status).toBe(400);
|
||||
const body = await response.json();
|
||||
expect(body.error).toContain('Missing "cypher"');
|
||||
});
|
||||
});
|
||||
},
|
||||
{
|
||||
poolAdapter: false,
|
||||
},
|
||||
);
|
||||
|
|
@ -118,6 +118,25 @@ withTestLbugDB(
|
|||
// Should return 0 rows, not all rows
|
||||
expect(rows).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('keeps seeded rows unchanged for a no-match parameterized write probe', async () => {
|
||||
await initLbug('test-repo', handle.dbPath);
|
||||
try {
|
||||
const rows = await executeParameterized(
|
||||
'test-repo',
|
||||
'MATCH (n:Function) WHERE n.name = $target SET n.name = $name RETURN n.name AS name',
|
||||
{ target: '__missing__', name: 'x' },
|
||||
);
|
||||
expect(rows).toEqual([]);
|
||||
} catch (err) {
|
||||
expect(String(err)).toMatch(/read-only database|write operations/i);
|
||||
}
|
||||
const rows = await executeQuery(
|
||||
'test-repo',
|
||||
'MATCH (n:Function) RETURN n.name AS name ORDER BY n.name',
|
||||
);
|
||||
expect(rows.map((r: any) => r.name)).toContain('main');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Error handling ──────────────────────────────────────────────────
|
||||
|
|
@ -133,14 +152,21 @@ withTestLbugDB(
|
|||
await expect(initLbug('bad-repo', '/nonexistent/path/lbug')).rejects.toThrow();
|
||||
});
|
||||
|
||||
it('read-only mode: write query throws', async () => {
|
||||
it('keeps seeded data unchanged for a no-match write probe', async () => {
|
||||
await initLbug('test-repo', handle.dbPath);
|
||||
await expect(
|
||||
executeQuery(
|
||||
try {
|
||||
await executeQuery(
|
||||
'test-repo',
|
||||
"CREATE (n:Function {id: 'new', name: 'new', filePath: '', startLine: 0, endLine: 0, isExported: false, content: '', description: ''})",
|
||||
),
|
||||
).rejects.toThrow();
|
||||
"MATCH (n:Function) WHERE n.name = '__missing__' SET n.name = 'new' RETURN n",
|
||||
);
|
||||
} catch (err) {
|
||||
expect(String(err)).toMatch(/read-only database|write operations/i);
|
||||
}
|
||||
const rows = await executeQuery(
|
||||
'test-repo',
|
||||
'MATCH (n:Function) RETURN n.name AS name ORDER BY n.name',
|
||||
);
|
||||
expect(rows.map((r: any) => r.name)).toContain('main');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -52,13 +52,16 @@ withTestLbugDB(
|
|||
expect(result.markdown).toContain('hash');
|
||||
});
|
||||
|
||||
it('cypher tool blocks write queries', async () => {
|
||||
it('cypher no-match write probe returns read-only error or empty rows', async () => {
|
||||
const result = await backend.callTool('cypher', {
|
||||
query:
|
||||
"CREATE (n:Function {id: 'x', name: 'x', filePath: '', startLine: 0, endLine: 0, isExported: false, content: '', description: ''})",
|
||||
"MATCH (n:Function) WHERE n.name = '__missing__' SET n.name = 'x' RETURN n.name AS name",
|
||||
});
|
||||
expect(result).toHaveProperty('error');
|
||||
expect(result.error).toMatch(/write operations/i);
|
||||
if (result?.error) {
|
||||
expect(result.error).toMatch(/write operations|read-only/i);
|
||||
return;
|
||||
}
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('context tool returns symbol info with callers and callees', async () => {
|
||||
|
|
|
|||
|
|
@ -4,21 +4,19 @@
|
|||
* Tests tool implementations via direct LadybugDB queries.
|
||||
* The full LocalBackend.callTool() requires a global registry,
|
||||
* so here we test the security-critical behaviors directly:
|
||||
* - Write-operation blocking in cypher
|
||||
* - Query execution via the pool
|
||||
* - Parameterized queries preventing injection
|
||||
* - Read-only enforcement
|
||||
*
|
||||
* Covers hardening fixes: #1 (parameterized queries), #2 (write blocking),
|
||||
* #3 (path traversal), #4 (relation allowlist), #25 (regex lastIndex),
|
||||
* #26 (rename first-occurrence-only)
|
||||
* Covers hardening fixes: #1 (parameterized queries), #3 (path traversal),
|
||||
* #4 (relation allowlist), #26 (rename first-occurrence-only)
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import {
|
||||
CYPHER_WRITE_RE,
|
||||
initLbug,
|
||||
closeLbug,
|
||||
executeQuery,
|
||||
executeParameterized,
|
||||
isWriteQuery,
|
||||
} from '../../src/mcp/core/lbug-adapter.js';
|
||||
import { VALID_RELATION_TYPES } from '../../src/mcp/local/local-backend.js';
|
||||
import { withTestLbugDB } from '../helpers/test-indexed-db.js';
|
||||
|
|
@ -29,35 +27,12 @@ import { LOCAL_BACKEND_SEED_DATA } from '../fixtures/local-backend-seed.js';
|
|||
withTestLbugDB(
|
||||
'local-backend',
|
||||
(handle) => {
|
||||
// ─── Cypher write blocking ───────────────────────────────────────────
|
||||
|
||||
describe('cypher write blocking', () => {
|
||||
const allWriteKeywords = [
|
||||
'CREATE',
|
||||
'DELETE',
|
||||
'SET',
|
||||
'MERGE',
|
||||
'REMOVE',
|
||||
'DROP',
|
||||
'ALTER',
|
||||
'COPY',
|
||||
'DETACH',
|
||||
];
|
||||
|
||||
for (const keyword of allWriteKeywords) {
|
||||
it(`blocks ${keyword} query`, () => {
|
||||
const blocked = isWriteQuery(`MATCH (n) ${keyword} n.name = "x"`);
|
||||
expect(blocked).toBe(true);
|
||||
});
|
||||
}
|
||||
|
||||
it('allows valid read queries through the pool', async () => {
|
||||
const rows = await executeQuery(
|
||||
handle.repoId,
|
||||
'MATCH (n:Function) RETURN n.name AS name ORDER BY n.name',
|
||||
);
|
||||
expect(rows.length).toBeGreaterThanOrEqual(3);
|
||||
});
|
||||
it('allows valid read queries through the pool', async () => {
|
||||
const rows = await executeQuery(
|
||||
handle.repoId,
|
||||
'MATCH (n:Function) RETURN n.name AS name ORDER BY n.name',
|
||||
);
|
||||
expect(rows.length).toBeGreaterThanOrEqual(3);
|
||||
});
|
||||
|
||||
// ─── Parameterized queries ───────────────────────────────────────────
|
||||
|
|
@ -171,34 +146,27 @@ withTestLbugDB(
|
|||
// ─── Read-only enforcement ───────────────────────────────────────────
|
||||
|
||||
describe('read-only database', () => {
|
||||
it('rejects write operations at DB level', async () => {
|
||||
await expect(
|
||||
executeQuery(
|
||||
handle.repoId,
|
||||
`CREATE (n:Function {id: 'new', name: 'new', filePath: '', startLine: 0, endLine: 0, isExported: false, content: '', description: ''})`,
|
||||
),
|
||||
).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Regex lastIndex hardening (#25) ─────────────────────────────────
|
||||
|
||||
describe('regex lastIndex (hardening #25)', () => {
|
||||
it('CYPHER_WRITE_RE is non-global (no sticky lastIndex)', () => {
|
||||
expect(CYPHER_WRITE_RE.global).toBe(false);
|
||||
expect(CYPHER_WRITE_RE.sticky).toBe(false);
|
||||
});
|
||||
|
||||
it('works correctly across multiple consecutive calls', () => {
|
||||
// If the regex were global, lastIndex could cause false results
|
||||
const results = [
|
||||
isWriteQuery('CREATE (n)'), // true
|
||||
isWriteQuery('MATCH (n) RETURN n'), // false
|
||||
isWriteQuery('DELETE n'), // true
|
||||
isWriteQuery('MATCH (n) RETURN n'), // false
|
||||
isWriteQuery('SET n.x = 1'), // true
|
||||
];
|
||||
expect(results).toEqual([true, false, true, false, true]);
|
||||
it('keeps seeded rows unchanged for a no-match write probe', async () => {
|
||||
const readOnlyRepo = 'local-backend-read-only';
|
||||
await initLbug(readOnlyRepo, handle.dbPath);
|
||||
try {
|
||||
const rows = await executeParameterized(
|
||||
readOnlyRepo,
|
||||
`MATCH (n:Function) WHERE n.name = $target SET n.name = $name RETURN n.name AS name`,
|
||||
{ target: '__missing__', name: 'changed' },
|
||||
);
|
||||
expect(rows).toEqual([]);
|
||||
} catch (err) {
|
||||
expect(String(err)).toMatch(/Write operations are not allowed|read-only database/i);
|
||||
}
|
||||
const rows = await executeParameterized(
|
||||
readOnlyRepo,
|
||||
'MATCH (n:Function) WHERE n.name = $name RETURN n.name AS name',
|
||||
{ name: 'login' },
|
||||
);
|
||||
expect(rows).toHaveLength(1);
|
||||
expect(rows[0].name).toBe('login');
|
||||
await closeLbug(readOnlyRepo);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -215,35 +183,6 @@ withTestLbugDB(
|
|||
});
|
||||
});
|
||||
|
||||
// ─── Write blocking edge cases ──────────────────────────────────────
|
||||
|
||||
describe('write blocking edge cases', () => {
|
||||
it('blocks lowercase write keywords (case-insensitive)', () => {
|
||||
expect(isWriteQuery('create (n:Function {id: "x"})')).toBe(true);
|
||||
expect(isWriteQuery('delete n')).toBe(true);
|
||||
expect(isWriteQuery('set n.name = "x"')).toBe(true);
|
||||
});
|
||||
|
||||
it('blocks write keyword in CREATED-like words (regex is keyword-boundary unaware)', () => {
|
||||
// CYPHER_WRITE_RE uses \b word boundaries — "CREATED" does NOT match "CREATE"
|
||||
const result = isWriteQuery("MATCH (n) WHERE n.name = 'CREATED' RETURN n");
|
||||
// The regex uses word boundaries so substring "CREATE" inside "CREATED" is NOT matched
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it('blocks multi-line queries with write keywords', () => {
|
||||
expect(isWriteQuery('MATCH (n)\nDELETE n')).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for empty string', () => {
|
||||
expect(isWriteQuery('')).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for whitespace-only query', () => {
|
||||
expect(isWriteQuery(' ')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Query error handling via pool ──────────────────────────────────
|
||||
|
||||
describe('query error handling via pool', () => {
|
||||
|
|
|
|||
|
|
@ -101,6 +101,15 @@ withTestLbugDB(
|
|||
expect(Array.isArray(results)).toBe(true);
|
||||
});
|
||||
|
||||
it('does not treat write-like words inside search text as write operations (#1608)', async () => {
|
||||
const { results, ftsAvailable } = await searchFTSFromLbug(
|
||||
'create user authentication delete',
|
||||
10,
|
||||
);
|
||||
expect(ftsAvailable).toBe(true);
|
||||
expect(results.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('handles limit of 0', async () => {
|
||||
const { results } = await searchFTSFromLbug('user authentication', 0);
|
||||
expect(results).toEqual([]);
|
||||
|
|
|
|||
30
gitnexus/test/unit/api-query-readonly-wiring.test.ts
Normal file
30
gitnexus/test/unit/api-query-readonly-wiring.test.ts
Normal file
|
|
@ -0,0 +1,30 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
import fs from 'node:fs/promises';
|
||||
import path from 'node:path';
|
||||
|
||||
describe('api query read-only wiring', () => {
|
||||
it('uses withLbugDb readOnly mode inside handleQueryRequest', async () => {
|
||||
const source = await fs.readFile(
|
||||
path.join(__dirname, '..', '..', 'src', 'server', 'api.ts'),
|
||||
'utf-8',
|
||||
);
|
||||
expect(source).toMatch(/handleQueryRequest[\s\S]*withLbugDb\([\s\S]*readOnly:\s*true/);
|
||||
});
|
||||
|
||||
it('routes /api/query through handleQueryRequest', async () => {
|
||||
const source = await fs.readFile(
|
||||
path.join(__dirname, '..', '..', 'src', 'server', 'api.ts'),
|
||||
'utf-8',
|
||||
);
|
||||
expect(source).toContain("app.post('/api/query', async (req, res) => {");
|
||||
expect(source).toContain('await handleQueryRequest(req, res, resolveRepo);');
|
||||
});
|
||||
|
||||
it('opens Ladybug connection with readOnly option when requested', async () => {
|
||||
const source = await fs.readFile(
|
||||
path.join(__dirname, '..', '..', 'src', 'core', 'lbug', 'lbug-adapter.ts'),
|
||||
'utf-8',
|
||||
);
|
||||
expect(source).toMatch(/openLbugConnection\(lbug,\s*dbPath,\s*\{\s*readOnly:\s*true\s*\}\)/);
|
||||
});
|
||||
});
|
||||
|
|
@ -13,9 +13,10 @@ vi.mock('../../src/core/lbug/lbug-adapter.js', async (importOriginal) => {
|
|||
// Pool adapter is dynamically imported by the MCP-pool path of
|
||||
// `searchFTSFromLbug`. We mock it so we can drive the executor without
|
||||
// spinning up a real LadybugDB pool.
|
||||
const mockExecuteQuery = vi.fn();
|
||||
const mockExecuteParameterized = vi.fn();
|
||||
vi.mock('../../src/core/lbug/pool-adapter.js', () => ({
|
||||
executeQuery: (repoId: string, cypher: string) => mockExecuteQuery(repoId, cypher),
|
||||
executeParameterized: (repoId: string, cypher: string, params: Record<string, any>) =>
|
||||
mockExecuteParameterized(repoId, cypher, params),
|
||||
addPoolCloseListener: vi.fn(),
|
||||
}));
|
||||
|
||||
|
|
@ -209,20 +210,22 @@ describe('BM25 search', () => {
|
|||
const REPO = 'test-repo-readonly-fts';
|
||||
|
||||
beforeEach(() => {
|
||||
mockExecuteQuery.mockReset();
|
||||
mockExecuteParameterized.mockReset();
|
||||
});
|
||||
|
||||
it('queries existing FTS indexes without issuing CREATE_FTS_INDEX', async () => {
|
||||
mockExecuteQuery.mockImplementation(async (_repo: string, cypher: string) => {
|
||||
if (cypher.includes('CREATE_FTS_INDEX')) {
|
||||
throw new Error('query path must stay read-only');
|
||||
}
|
||||
mockExecuteParameterized.mockImplementation(
|
||||
async (_repo: string, cypher: string, params: Record<string, any>) => {
|
||||
if (cypher.includes('CREATE_FTS_INDEX')) {
|
||||
throw new Error('query path must stay read-only');
|
||||
}
|
||||
|
||||
if (cypher.includes("QUERY_FTS_INDEX('Function'")) {
|
||||
return [{ node: { filePath: 'src/auth.ts', id: 'func:login' }, score: 8 }];
|
||||
}
|
||||
return [];
|
||||
});
|
||||
if (params.query === 'login' && cypher.includes("QUERY_FTS_INDEX('Function'")) {
|
||||
return [{ node: { filePath: 'src/auth.ts', id: 'func:login' }, score: 8 }];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
);
|
||||
|
||||
const { results } = await searchFTSFromLbug('login', 5, REPO);
|
||||
|
||||
|
|
@ -230,16 +233,35 @@ describe('BM25 search', () => {
|
|||
{ filePath: 'src/auth.ts', score: 8, rank: 1, nodeIds: ['func:login'] },
|
||||
]);
|
||||
expect(
|
||||
mockExecuteQuery.mock.calls.some((c) => String(c[1]).includes('CREATE_FTS_INDEX')),
|
||||
mockExecuteParameterized.mock.calls.some((c) => String(c[1]).includes('CREATE_FTS_INDEX')),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('binds FTS user query text as a parameter in pool mode', async () => {
|
||||
mockExecuteParameterized.mockResolvedValue([]);
|
||||
|
||||
const userQuery = "BrowserWindow create delete set remove 'main' window";
|
||||
await searchFTSFromLbug(userQuery, 5, REPO);
|
||||
|
||||
expect(mockExecuteParameterized).toHaveBeenCalled();
|
||||
for (const call of mockExecuteParameterized.mock.calls) {
|
||||
const cypher = String(call[1]);
|
||||
expect(cypher).toContain('$query');
|
||||
expect(cypher).not.toContain(userQuery);
|
||||
expect(cypher.toUpperCase()).not.toMatch(/\bCREATE\b/);
|
||||
expect(cypher.toUpperCase()).not.toMatch(/\bDELETE\b/);
|
||||
expect(cypher.toUpperCase()).not.toMatch(/\bSET\b/);
|
||||
expect(cypher.toUpperCase()).not.toMatch(/\bREMOVE\b/);
|
||||
expect(call[2]).toEqual({ query: userQuery });
|
||||
}
|
||||
});
|
||||
|
||||
it('uses the configured FTS query set on every call', async () => {
|
||||
mockExecuteQuery.mockResolvedValue([]);
|
||||
mockExecuteParameterized.mockResolvedValue([]);
|
||||
|
||||
await searchFTSFromLbug('anything', 5, REPO);
|
||||
|
||||
const queryCalls = mockExecuteQuery.mock.calls.filter((c) =>
|
||||
const queryCalls = mockExecuteParameterized.mock.calls.filter((c) =>
|
||||
String(c[1]).includes('QUERY_FTS_INDEX'),
|
||||
);
|
||||
expect(queryCalls.map((c) => String(c[1]).match(/QUERY_FTS_INDEX\('([^']+)'/)?.[1])).toEqual([
|
||||
|
|
|
|||
|
|
@ -292,13 +292,14 @@ describe('LocalBackend.callTool', () => {
|
|||
});
|
||||
|
||||
it('dispatches cypher tool and blocks write queries', async () => {
|
||||
(executeParameterized as any).mockRejectedValueOnce(new Error('read-only database'));
|
||||
const result = await backend.callTool('cypher', { query: 'CREATE (n:Test)' });
|
||||
expect(result).toHaveProperty('error');
|
||||
expect(result.error).toContain('Write operations');
|
||||
});
|
||||
|
||||
it('dispatches cypher tool with valid read query', async () => {
|
||||
(executeQuery as any).mockResolvedValue([{ name: 'test', filePath: 'src/test.ts' }]);
|
||||
(executeParameterized as any).mockResolvedValue([{ name: 'test', filePath: 'src/test.ts' }]);
|
||||
const result = await backend.callTool('cypher', {
|
||||
query: 'MATCH (n:Function) RETURN n.name AS name, n.filePath AS filePath LIMIT 5',
|
||||
});
|
||||
|
|
@ -999,6 +1000,7 @@ describe('callTool cypher write blocking', () => {
|
|||
|
||||
for (const query of writeQueries) {
|
||||
it(`blocks write query: ${query.slice(0, 30)}...`, async () => {
|
||||
(executeParameterized as any).mockRejectedValueOnce(new Error('read-only database'));
|
||||
const result = await backend.callTool('cypher', { query });
|
||||
expect(result).toHaveProperty('error');
|
||||
expect(result.error).toContain('Write operations');
|
||||
|
|
@ -1006,7 +1008,7 @@ describe('callTool cypher write blocking', () => {
|
|||
}
|
||||
|
||||
it('allows read query through callTool', async () => {
|
||||
(executeQuery as any).mockResolvedValue([]);
|
||||
(executeParameterized as any).mockResolvedValue([]);
|
||||
const result = await backend.callTool('cypher', {
|
||||
query: 'MATCH (n:Function) RETURN n.name LIMIT 5',
|
||||
});
|
||||
|
|
@ -1105,7 +1107,7 @@ describe('cypher result formatting', () => {
|
|||
});
|
||||
|
||||
it('formats tabular results as markdown table', async () => {
|
||||
(executeQuery as any).mockResolvedValue([
|
||||
(executeParameterized as any).mockResolvedValue([
|
||||
{ name: 'main', filePath: 'src/index.ts' },
|
||||
{ name: 'helper', filePath: 'src/utils.ts' },
|
||||
]);
|
||||
|
|
@ -1119,7 +1121,7 @@ describe('cypher result formatting', () => {
|
|||
});
|
||||
|
||||
it('returns empty array as-is', async () => {
|
||||
(executeQuery as any).mockResolvedValue([]);
|
||||
(executeParameterized as any).mockResolvedValue([]);
|
||||
const result = await backend.callTool('cypher', {
|
||||
query: 'MATCH (n:Function) RETURN n.name LIMIT 0',
|
||||
});
|
||||
|
|
@ -1127,7 +1129,7 @@ describe('cypher result formatting', () => {
|
|||
});
|
||||
|
||||
it('returns error object when cypher fails', async () => {
|
||||
(executeQuery as any).mockRejectedValue(new Error('Syntax error'));
|
||||
(executeParameterized as any).mockRejectedValue(new Error('Syntax error'));
|
||||
const result = await backend.callTool('cypher', {
|
||||
query: 'INVALID CYPHER SYNTAX',
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,51 +0,0 @@
|
|||
// ...existing code...
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { isWriteQuery as isWriteQueryAdapter } from '../../src/mcp/core/lbug-adapter';
|
||||
import { isWriteQuery as isWriteQueryBackend } from '../../src/mcp/local/local-backend';
|
||||
|
||||
describe('isWriteQuery regex tests', () => {
|
||||
const writeQueries = [
|
||||
'CREATE (n:Test {name: "x"})',
|
||||
'MATCH (n) SET n.x = 1',
|
||||
'MERGE (n:Foo {id: 1})',
|
||||
'DELETE n',
|
||||
'DROP INDEX ON :Foo(prop)',
|
||||
'ALTER TABLE Something',
|
||||
'COPY TO something',
|
||||
'DETACH DELETE n',
|
||||
];
|
||||
|
||||
const readQueries = [
|
||||
'MATCH (n:CreateHelpers) RETURN n',
|
||||
'MATCH (a)-[:CALLS]->(b) RETURN a, b',
|
||||
'MATCH (f:File)-[r:DEFINES]->(n) RETURN n',
|
||||
"MATCH (n) WHERE n.name = 'MERGEHelper' RETURN n", // word present as data
|
||||
'MATCH (n) RETURN n',
|
||||
'MATCH (n) WHERE n.content CONTAINS ":CREATE" RETURN n',
|
||||
'MATCH (n:SomethingWithSET) RETURN n',
|
||||
];
|
||||
|
||||
it('adapter isWriteQuery should detect real write queries', () => {
|
||||
for (const q of writeQueries) {
|
||||
expect(isWriteQueryAdapter(q), `adapter should detect write for: ${q}`).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('adapter isWriteQuery should not false-positive on label/rel or data', () => {
|
||||
for (const q of readQueries) {
|
||||
expect(isWriteQueryAdapter(q), `adapter false-positive on: ${q}`).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it('backend isWriteQuery should detect real write queries', () => {
|
||||
for (const q of writeQueries) {
|
||||
expect(isWriteQueryBackend(q), `backend should detect write for: ${q}`).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('backend isWriteQuery should not false-positive on label/rel or data', () => {
|
||||
for (const q of readQueries) {
|
||||
expect(isWriteQueryBackend(q), `backend false-positive on: ${q}`).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -10,6 +10,24 @@ const makeOpenMock = () =>
|
|||
close: vi.fn(async () => {}),
|
||||
}));
|
||||
|
||||
/** Mock prepared statement shape for executePrepared/prepare+execute paths. */
|
||||
const makePreparedStatement = (sql: string) => ({
|
||||
sql,
|
||||
isSuccess: () => true,
|
||||
getErrorMessage: () => '',
|
||||
});
|
||||
|
||||
/** Mock connection supporting both query() and prepare/execute() call paths. */
|
||||
const makeConn = (runQuery: (sql: string) => Promise<unknown>) => {
|
||||
const query = vi.fn(runQuery);
|
||||
return {
|
||||
query,
|
||||
prepare: vi.fn(async (sql: string) => makePreparedStatement(sql)),
|
||||
execute: vi.fn(async (statement: { sql: string }) => query(statement.sql)),
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
};
|
||||
|
||||
/** Standard `fs/promises` mock for tests that only need doInitLbug to succeed. */
|
||||
const mockFsForInit = (dbPath: string) => {
|
||||
const ENOENT_ERROR = makeErrnoError(
|
||||
|
|
@ -50,10 +68,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => {
|
|||
`ENOENT: no such file or directory, access '${dbPath}'`,
|
||||
);
|
||||
const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() };
|
||||
const conn = {
|
||||
query: vi.fn(async () => queryResult),
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
const conn = makeConn(async () => queryResult);
|
||||
const db = { close: vi.fn(async () => {}) };
|
||||
|
||||
const unlinkMock = vi.fn(async () => {});
|
||||
|
|
@ -125,10 +140,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => {
|
|||
);
|
||||
const EACCES_ERROR = makeErrnoError('EACCES', `EACCES: permission denied, access '${dbPath}'`);
|
||||
const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() };
|
||||
const conn = {
|
||||
query: vi.fn(async () => queryResult),
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
const conn = makeConn(async () => queryResult);
|
||||
const db = { close: vi.fn(async () => {}) };
|
||||
const accessMock = vi.fn(async () => {
|
||||
throw EACCES_ERROR;
|
||||
|
|
@ -194,10 +206,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => {
|
|||
`ENOENT: no such file or directory, access '${dbPath}'`,
|
||||
);
|
||||
const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() };
|
||||
const conn = {
|
||||
query: vi.fn(async () => queryResult),
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
const conn = makeConn(async () => queryResult);
|
||||
const db = { close: vi.fn(async () => {}) };
|
||||
const accessMock = vi.fn(async () => {});
|
||||
const unlinkMock = vi.fn(async () => {});
|
||||
|
|
@ -318,10 +327,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => {
|
|||
`ENOENT: no such file or directory, access '${dbPath}'`,
|
||||
);
|
||||
const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() };
|
||||
const conn = {
|
||||
query: vi.fn(async () => queryResult),
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
const conn = makeConn(async () => queryResult);
|
||||
const db = { close: vi.fn(async () => {}) };
|
||||
const accessMock = vi.fn(async () => {
|
||||
throw ENOENT_ERROR;
|
||||
|
|
@ -391,10 +397,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => {
|
|||
`EPERM: operation not permitted, unlink '${dbPath}.shadow'`,
|
||||
);
|
||||
const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() };
|
||||
const conn = {
|
||||
query: vi.fn(async () => queryResult),
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
const conn = makeConn(async () => queryResult);
|
||||
const db = { close: vi.fn(async () => {}) };
|
||||
const accessMock = vi.fn(async () => {
|
||||
throw ENOENT_ERROR;
|
||||
|
|
@ -473,18 +476,16 @@ describe('lbug adapter CHECKPOINT lifecycle', () => {
|
|||
getAll: vi.fn(async () => []),
|
||||
close: vi.fn(),
|
||||
};
|
||||
const conn = {
|
||||
query: vi.fn(async (sql: string) => {
|
||||
if (sql === 'CHECKPOINT') {
|
||||
events.push('checkpoint:query');
|
||||
return checkpointResult;
|
||||
}
|
||||
return genericResult;
|
||||
}),
|
||||
close: vi.fn(async () => {
|
||||
events.push('conn:close');
|
||||
}),
|
||||
};
|
||||
const conn = makeConn(async (sql: string) => {
|
||||
if (sql === 'CHECKPOINT') {
|
||||
events.push('checkpoint:query');
|
||||
return checkpointResult;
|
||||
}
|
||||
return genericResult;
|
||||
});
|
||||
conn.close = vi.fn(async () => {
|
||||
events.push('conn:close');
|
||||
});
|
||||
const db = {
|
||||
close: vi.fn(async () => {
|
||||
events.push('db:close');
|
||||
|
|
@ -539,16 +540,13 @@ describe('lbug adapter CHECKPOINT lifecycle', () => {
|
|||
getAll: vi.fn(async () => []),
|
||||
close: vi.fn(),
|
||||
};
|
||||
const conn = {
|
||||
query: vi.fn(async (sql: string) => {
|
||||
if (sql === 'MATCH (n:File) RETURN n.id AS id') {
|
||||
events.push('query:run');
|
||||
return queryResult;
|
||||
}
|
||||
return genericResult;
|
||||
}),
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
const conn = makeConn(async (sql: string) => {
|
||||
if (sql === 'MATCH (n:File) RETURN n.id AS id') {
|
||||
events.push('query:run');
|
||||
return queryResult;
|
||||
}
|
||||
return genericResult;
|
||||
});
|
||||
const db = {
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
|
|
@ -595,15 +593,12 @@ describe('lbug adapter CHECKPOINT lifecycle', () => {
|
|||
getAll: vi.fn(async () => []),
|
||||
close: vi.fn(),
|
||||
};
|
||||
const conn = {
|
||||
query: vi.fn(async (sql: string) => {
|
||||
if (sql === 'MATCH (n:File) RETURN n.id AS id') {
|
||||
return queryResult;
|
||||
}
|
||||
return genericResult;
|
||||
}),
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
const conn = makeConn(async (sql: string) => {
|
||||
if (sql === 'MATCH (n:File) RETURN n.id AS id') {
|
||||
return queryResult;
|
||||
}
|
||||
return genericResult;
|
||||
});
|
||||
const db = {
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
|
|
@ -661,15 +656,12 @@ describe('lbug adapter CHECKPOINT lifecycle', () => {
|
|||
getAll: vi.fn(async () => []),
|
||||
close: vi.fn(),
|
||||
};
|
||||
const conn = {
|
||||
query: vi.fn(async (sql: string) => {
|
||||
if (sql === 'MATCH (n:File) RETURN n.id AS id') {
|
||||
return [firstResult, secondResult];
|
||||
}
|
||||
return genericResult;
|
||||
}),
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
const conn = makeConn(async (sql: string) => {
|
||||
if (sql === 'MATCH (n:File) RETURN n.id AS id') {
|
||||
return [firstResult, secondResult];
|
||||
}
|
||||
return genericResult;
|
||||
});
|
||||
const db = {
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
|
|
@ -741,16 +733,13 @@ describe('lbug adapter CHECKPOINT lifecycle', () => {
|
|||
getAll: vi.fn(async () => []),
|
||||
close: vi.fn(),
|
||||
};
|
||||
const conn = {
|
||||
query: vi.fn(async (sql: string) => {
|
||||
if (sql === 'MATCH (n:File) RETURN n.id AS id') {
|
||||
events.push('stream:query');
|
||||
return [firstResult, secondResult];
|
||||
}
|
||||
return genericResult;
|
||||
}),
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
const conn = makeConn(async (sql: string) => {
|
||||
if (sql === 'MATCH (n:File) RETURN n.id AS id') {
|
||||
events.push('stream:query');
|
||||
return [firstResult, secondResult];
|
||||
}
|
||||
return genericResult;
|
||||
});
|
||||
const db = {
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
|
|
@ -822,16 +811,13 @@ describe('lbug adapter CHECKPOINT lifecycle', () => {
|
|||
getAll: vi.fn(async () => []),
|
||||
close: vi.fn(),
|
||||
};
|
||||
const conn = {
|
||||
query: vi.fn(async (sql: string) => {
|
||||
if (sql === 'MATCH (n:File) RETURN n.id AS id') {
|
||||
events.push('stream:query');
|
||||
return queryResult;
|
||||
}
|
||||
return genericResult;
|
||||
}),
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
const conn = makeConn(async (sql: string) => {
|
||||
if (sql === 'MATCH (n:File) RETURN n.id AS id') {
|
||||
events.push('stream:query');
|
||||
return queryResult;
|
||||
}
|
||||
return genericResult;
|
||||
});
|
||||
const db = {
|
||||
close: vi.fn(async () => {}),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -10,7 +10,6 @@ const { lbugMocks, platformMocks, repoMocks } = vi.hoisted(() => ({
|
|||
executeParameterized: vi.fn(),
|
||||
closeLbug: vi.fn().mockResolvedValue(undefined),
|
||||
isLbugReady: vi.fn().mockReturnValue(true),
|
||||
isWriteQuery: vi.fn().mockReturnValue(false),
|
||||
},
|
||||
platformMocks: {
|
||||
isVectorExtensionSupportedByPlatform: vi.fn().mockReturnValue(true),
|
||||
|
|
@ -81,7 +80,6 @@ describe('WAL corruption feedback in MCP responses (#1402)', () => {
|
|||
lbugMocks.executeQuery.mockResolvedValue([]);
|
||||
lbugMocks.executeParameterized.mockResolvedValue([]);
|
||||
lbugMocks.isLbugReady.mockReturnValue(true);
|
||||
lbugMocks.isWriteQuery.mockReturnValue(false);
|
||||
repoMocks.listRegisteredRepos.mockResolvedValue([MOCK_REPO_ENTRY]);
|
||||
});
|
||||
|
||||
|
|
@ -106,7 +104,7 @@ describe('WAL corruption feedback in MCP responses (#1402)', () => {
|
|||
|
||||
it('cypher returns WAL recoverySuggestion on corrupted WAL error', async () => {
|
||||
const backend = await makeBackend();
|
||||
lbugMocks.executeQuery.mockRejectedValueOnce(new Error('Corrupted wal file'));
|
||||
lbugMocks.executeParameterized.mockRejectedValueOnce(new Error('Corrupted wal file'));
|
||||
|
||||
const result = await backend.callTool('cypher', {
|
||||
repo: 'test-repo',
|
||||
|
|
|
|||
14
gitnexus/test/unit/query-fts-parameterization.test.ts
Normal file
14
gitnexus/test/unit/query-fts-parameterization.test.ts
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
import fs from 'node:fs/promises';
|
||||
import path from 'node:path';
|
||||
|
||||
describe('queryFTS parameterization wiring', () => {
|
||||
it('binds FTS query text via $query and executePrepared', async () => {
|
||||
const source = await fs.readFile(
|
||||
path.join(__dirname, '..', '..', 'src', 'core', 'lbug', 'lbug-adapter.ts'),
|
||||
'utf-8',
|
||||
);
|
||||
expect(source).toMatch(/QUERY_FTS_INDEX\('\$\{tableName\}', '\$\{indexName\}', \$query/);
|
||||
expect(source).toMatch(/executePrepared\(cypher,\s*\{\s*query\s*\}\)/);
|
||||
});
|
||||
});
|
||||
30
gitnexus/test/unit/query-params.test.ts
Normal file
30
gitnexus/test/unit/query-params.test.ts
Normal file
|
|
@ -0,0 +1,30 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
import { isValidQueryParams } from '../../src/core/lbug/query-params.js';
|
||||
|
||||
describe('isValidQueryParams', () => {
|
||||
it('accepts plain objects', () => {
|
||||
expect(isValidQueryParams({})).toBe(true);
|
||||
expect(isValidQueryParams({ name: 'main', limit: 10 })).toBe(true);
|
||||
expect(isValidQueryParams({ enabled: true, score: null })).toBe(true);
|
||||
expect(isValidQueryParams(Object.create(null))).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects null and arrays', () => {
|
||||
expect(isValidQueryParams(null)).toBe(false);
|
||||
expect(isValidQueryParams([])).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects primitives', () => {
|
||||
expect(isValidQueryParams('x')).toBe(false);
|
||||
expect(isValidQueryParams(1)).toBe(false);
|
||||
expect(isValidQueryParams(false)).toBe(false);
|
||||
expect(isValidQueryParams(undefined)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects non-plain objects and non-scalar values', () => {
|
||||
expect(isValidQueryParams(new Date())).toBe(false);
|
||||
expect(isValidQueryParams(new Map())).toBe(false);
|
||||
expect(isValidQueryParams({ nested: { value: 1 } })).toBe(false);
|
||||
expect(isValidQueryParams({ list: ['x'] })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,11 +1,9 @@
|
|||
/**
|
||||
* P0 Unit Tests: Security Hardening
|
||||
*
|
||||
* Tests all security hardening in isolation:
|
||||
* - Write blocking (CYPHER_WRITE_RE)
|
||||
* Tests security-related utility helpers in isolation:
|
||||
* - Relation type allowlist
|
||||
* - Path traversal detection
|
||||
* - isWriteQuery wrapper
|
||||
* - isTestFilePath patterns
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
|
|
@ -14,93 +12,6 @@ import {
|
|||
VALID_NODE_LABELS,
|
||||
isTestFilePath,
|
||||
} from '../../src/mcp/local/local-backend.js';
|
||||
import { CYPHER_WRITE_RE, isWriteQuery } from '../../src/mcp/core/lbug-adapter.js';
|
||||
|
||||
// ─── Write-operation blocking (CYPHER_WRITE_RE) ──────────────────────
|
||||
|
||||
describe('CYPHER_WRITE_RE', () => {
|
||||
const writeKeywords = [
|
||||
'CREATE',
|
||||
'DELETE',
|
||||
'SET',
|
||||
'MERGE',
|
||||
'REMOVE',
|
||||
'DROP',
|
||||
'ALTER',
|
||||
'COPY',
|
||||
'DETACH',
|
||||
];
|
||||
|
||||
for (const keyword of writeKeywords) {
|
||||
it(`matches "${keyword}" (uppercase)`, () => {
|
||||
expect(CYPHER_WRITE_RE.test(`${keyword} (n:Node)`)).toBe(true);
|
||||
});
|
||||
|
||||
it(`matches "${keyword.toLowerCase()}" (lowercase)`, () => {
|
||||
expect(CYPHER_WRITE_RE.test(`${keyword.toLowerCase()} (n:Node)`)).toBe(true);
|
||||
});
|
||||
|
||||
it(`matches "${keyword[0] + keyword.slice(1).toLowerCase()}" (mixed case)`, () => {
|
||||
const mixed = keyword[0] + keyword.slice(1).toLowerCase();
|
||||
expect(CYPHER_WRITE_RE.test(`${mixed} (n:Node)`)).toBe(true);
|
||||
});
|
||||
}
|
||||
|
||||
// Safe read queries should NOT be blocked
|
||||
const safeQueries = [
|
||||
'MATCH (n) RETURN n',
|
||||
'MATCH (n:Function) WHERE n.name = "foo" RETURN n',
|
||||
'MATCH (a)-[r]->(b) RETURN a, r, b',
|
||||
'OPTIONAL MATCH (n)-[r]->(m) RETURN n, r, m',
|
||||
'MATCH (n) WITH n RETURN n.name',
|
||||
'UNWIND [1,2,3] AS x RETURN x',
|
||||
'MATCH (n) RETURN count(n)',
|
||||
'MATCH (n:Function) WHERE n.filePath CONTAINS "test" RETURN n',
|
||||
];
|
||||
|
||||
for (const query of safeQueries) {
|
||||
it(`does NOT block safe query: "${query.slice(0, 50)}..."`, () => {
|
||||
expect(CYPHER_WRITE_RE.test(query)).toBe(false);
|
||||
});
|
||||
}
|
||||
|
||||
it('blocks write keyword within a longer query', () => {
|
||||
expect(CYPHER_WRITE_RE.test('MATCH (n) DELETE n')).toBe(true);
|
||||
expect(CYPHER_WRITE_RE.test('MATCH (n:Node) SET n.name = "x"')).toBe(true);
|
||||
});
|
||||
|
||||
it('does not match partial word (e.g., "CREATED" should not match)', () => {
|
||||
// \b ensures word boundary. "CREATED" starts with "CREATE" but has extra D
|
||||
// Actually \b(CREATE) matches "CREATE" in "CREATED" since CREATE is followed by D
|
||||
// which is a word char -> no boundary at E-D. Let's verify:
|
||||
expect(CYPHER_WRITE_RE.test('CREATED_AT')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── isWriteQuery wrapper ─────────────────────────────────────────────
|
||||
|
||||
describe('isWriteQuery', () => {
|
||||
it('returns true for write queries', () => {
|
||||
expect(isWriteQuery('CREATE (n:Node)')).toBe(true);
|
||||
expect(isWriteQuery('match (n) delete n')).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for read queries', () => {
|
||||
expect(isWriteQuery('MATCH (n) RETURN n')).toBe(false);
|
||||
});
|
||||
|
||||
it('handles empty string', () => {
|
||||
expect(isWriteQuery('')).toBe(false);
|
||||
});
|
||||
|
||||
// Hardening: regex lastIndex not stuck (non-global regex, but verify)
|
||||
it('works correctly on consecutive calls', () => {
|
||||
expect(isWriteQuery('CREATE (n)')).toBe(true);
|
||||
expect(isWriteQuery('MATCH (n) RETURN n')).toBe(false);
|
||||
expect(isWriteQuery('DROP TABLE foo')).toBe(true);
|
||||
expect(isWriteQuery('MATCH (n) RETURN n')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Relation type allowlist ──────────────────────────────────────────
|
||||
|
||||
|
|
@ -211,12 +122,3 @@ describe('path traversal (isTestFilePath as proxy for path handling)', () => {
|
|||
expect(isTestFilePath('src/utils/helper.ts')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Static analysis: parameterized query patterns ────────────────────
|
||||
|
||||
describe('parameterized query patterns (static analysis)', () => {
|
||||
it('CYPHER_WRITE_RE is not a global regex (no lastIndex issue)', () => {
|
||||
// A global regex would have sticky lastIndex state
|
||||
expect(CYPHER_WRITE_RE.global).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -103,6 +103,9 @@ describe('GITNEXUS_TOOLS', () => {
|
|||
it('cypher tool requires "query" parameter', () => {
|
||||
const cypherTool = GITNEXUS_TOOLS.find((t) => t.name === 'cypher')!;
|
||||
expect(cypherTool.inputSchema.required).toContain('query');
|
||||
expect(cypherTool.inputSchema.properties.params).toBeDefined();
|
||||
expect(cypherTool.inputSchema.properties.params.type).toBe('object');
|
||||
expect(cypherTool.inputSchema.properties.params.description).toContain('prepared statement');
|
||||
});
|
||||
|
||||
it('context tool has no required parameters', () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue