Skip to content

Commit 79888b0

Browse files
fix(db): keep raw_json inline for in-memory DBs (stop tests polluting prod raw tree; fixes cross-DB blob collision) + path-integer hardening
1 parent f954277 commit 79888b0

2 files changed

Lines changed: 81 additions & 5 deletions

File tree

core/db.js

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,35 @@ import { RAW_DIR, DEFAULT_BATCH_RETENTION } from './constants.js';
2121
// going forward; readers transparently rehydrate from the file so the REST
2222
// contract (er.raw_json as a JSON string) stays byte-identical.
2323

24+
/**
25+
* True when both ids are safe integer keys. result_id may arrive as a BigInt
26+
* (better-sqlite3 lastInsertRowid), so coerce via Number() for the check —
27+
* `Number.isInteger(Number(5n))` is true — while callers keep the original
28+
* value for the path string (BigInt stringifies cleanly). Guards against any
29+
* non-integer ever being interpolated into a filesystem path (defense-in-depth:
30+
* no `../` traversal if a stray string ever reaches these helpers).
31+
*/
32+
function _validRawJsonIds(run_id, result_id) {
33+
return Number.isInteger(Number(run_id)) && Number.isInteger(Number(result_id));
34+
}
35+
36+
/**
37+
* True when the handle is an in-memory better-sqlite3 DB. `db.memory` is the
38+
* documented boolean (verified true for `new Database(':memory:')`, false for a
39+
* file path in this better-sqlite3 version); we fall back to the `:memory:`
40+
* name only if `db.memory` is somehow undefined on a future build.
41+
*
42+
* Why it matters: the raw_json file offload writes to a FIXED RAW_DIR that is
43+
* independent of which handle is active. An in-memory test DB issuing
44+
* (run_id, result_id) keys that collide with a real DB's rowids would overwrite
45+
* (or be served in place of) prod diagnostic blobs. In-memory handles therefore
46+
* keep raw_json inline and write no file at all.
47+
*/
48+
function _isMemoryDb(db) {
49+
if (db && typeof db.memory === 'boolean') return db.memory;
50+
return !!(db && db.name === ':memory:');
51+
}
52+
2453
/** Deterministic path to a result's raw-json file. */
2554
function rawJsonPath(run_id, result_id) {
2655
return path.join(RAW_DIR, `run-${run_id}`, `${result_id}.json`);
@@ -43,6 +72,10 @@ function rawJsonPath(run_id, result_id) {
4372
* @param {string} json - the JSON.stringify(result) string
4473
*/
4574
function writeRawJson(run_id, result_id, json) {
75+
if (!_validRawJsonIds(run_id, result_id)) {
76+
console.error(`[db] raw_json file write skipped: non-integer key (run_id=${run_id}, result_id=${result_id})`);
77+
return;
78+
}
4679
try {
4780
const dir = path.join(RAW_DIR, `run-${run_id}`);
4881
mkdirSync(dir, { recursive: true });
@@ -61,6 +94,7 @@ function writeRawJson(run_id, result_id, json) {
6194
*/
6295
export function readRawJson(run_id, result_id) {
6396
if (run_id == null || result_id == null) return null;
97+
if (!_validRawJsonIds(run_id, result_id)) return null;
6498
try {
6599
return readFileSync(rawJsonPath(run_id, result_id), 'utf8');
66100
} catch (e) {
@@ -606,9 +640,15 @@ function mapResultToRow(run_id, r) {
606640
error_code: r.errorCode || null,
607641
error_message: r.error || null,
608642
tested_at,
609-
// raw_json is offloaded to a per-run file after insert; the column is
610-
// bound NULL going forward. _rawJson carries the blob to the writer but is
611-
// NOT a column param (SQLite ignores extra named props it doesn't bind).
643+
// The writer decides how raw_json is persisted based on the active handle:
644+
// - file-backed DB → column bound NULL, blob offloaded to a per-run file
645+
// (shrinks on-disk audit.db; readers rehydrate from the file).
646+
// - in-memory DB → blob bound to the column inline, NO file written
647+
// (offload is pointless for :memory:, and a FIXED RAW_DIR shared across
648+
// handles would let test fixtures collide with / overwrite prod blobs).
649+
// _rawJson carries the blob to the writer but is NEVER a column param —
650+
// only the resolved `raw_json` value is bound (extra named props are
651+
// ignored by better-sqlite3, but we strip it explicitly below).
612652
raw_json: null,
613653
_rawJson: JSON.stringify(r),
614654
pass,
@@ -640,10 +680,16 @@ const _insertResultSql = `
640680
*/
641681
export function insertResult(run_id, result, which) {
642682
const db = getDb(which);
683+
const memory = _isMemoryDb(db);
643684
const row = mapResultToRow(run_id, result);
644685
const { _rawJson, ...bind } = row;
686+
// In-memory DB: keep the blob inline in the column, write no file. The RAW_DIR
687+
// offload exists only to shrink the on-DISK audit.db, which is moot for a
688+
// :memory: handle — and a fixed RAW_DIR shared across handles would let test
689+
// fixtures collide with prod blobs under the same (run_id, result_id) key.
690+
if (memory) bind.raw_json = _rawJson;
645691
const info = db.prepare(_insertResultSql).run(bind);
646-
writeRawJson(run_id, info.lastInsertRowid, _rawJson);
692+
if (!memory) writeRawJson(run_id, info.lastInsertRowid, _rawJson);
647693
return info.lastInsertRowid;
648694
}
649695

@@ -656,12 +702,16 @@ export function insertResult(run_id, result, which) {
656702
*/
657703
export function insertResultsBatch(run_id, results, which) {
658704
const db = getDb(which);
705+
const memory = _isMemoryDb(db);
659706
const stmt = db.prepare(_insertResultSql);
660707
const insert = db.transaction((rows) => {
661708
for (const row of rows) {
662709
const { _rawJson, ...bind } = row;
710+
// See insertResult: in-memory keeps the blob inline (no file); file-backed
711+
// NULLs the column and offloads to a per-run file.
712+
if (memory) bind.raw_json = _rawJson;
663713
const info = stmt.run(bind);
664-
writeRawJson(run_id, info.lastInsertRowid, _rawJson);
714+
if (!memory) writeRawJson(run_id, info.lastInsertRowid, _rawJson);
665715
}
666716
});
667717
insert(results.map(r => mapResultToRow(run_id, r)));

test/db.smoke.test.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,14 @@
66
* Exit 0 = all pass, exit 1 = failures.
77
*/
88

9+
import path from 'path';
10+
import { existsSync } from 'fs';
911
import { getDb, useDb, insertRun, updateRunOnFinish, insertResult, insertResultsBatch,
1012
getRun, findRunByKey, listRuns, getLatestResultPerNode,
1113
getNodeHistory, getNetworkStats, insertErrorLog, getNodeErrors, closeDb,
1214
insertBatch, insertBatchResult, updateBatchOnFinish, getBatchResults,
1315
getActiveBatch, getLastBatch, pruneBatchResults } from '../core/db.js';
16+
import { RAW_DIR } from '../core/constants.js';
1417

1518
// ─── Use a fresh in-memory DB for this test run ───────────────────────────────
1619
// `useDb` sets the module singleton so all exported helpers target this handle.
@@ -268,6 +271,29 @@ assert(db.prepare('PRAGMA foreign_key_check').all().length === 0, 'foreign_key_c
268271
// Active batch never deleted as a parent even if outside window — already
269272
// asserted via b6 survival above.
270273

274+
// 15. in-memory DB keeps raw_json INLINE in the column and writes NO file.
275+
// Regression guard: the per-run file offload (results/raw/run-<id>/<id>.json)
276+
// uses a FIXED RAW_DIR independent of the active handle. An in-memory test
277+
// DB must NOT write there (cross-DB blob collision / prod pollution). The
278+
// fix binds raw_json inline for :memory: and skips the file entirely.
279+
console.log('15. in-memory raw_json stays inline (no file)...');
280+
{
281+
const memRunId = Number(insertRun({ started_at: NOW - 3_000, mode: 'p2p' }));
282+
const memResId = Number(insertResult(memRunId, { ...SAMPLE_PASS, address: 'sentnode1mem999' }));
283+
assert(memResId > 0, 'in-memory insertResult returns id');
284+
// Column populated (readers return it as-is; no rehydrate-from-file needed).
285+
const memHist = getNodeHistory('sentnode1mem999', { limit: 1 });
286+
assert(memHist.length === 1, 'in-memory node history has 1 row');
287+
assert(memHist[0].raw_json != null, 'in-memory raw_json column is populated (not NULL)');
288+
const memParsed = JSON.parse(memHist[0].raw_json);
289+
eq(memParsed.address, 'sentnode1mem999', 'in-memory raw_json round-trips via the column');
290+
// No file written for this (run_id, result_id) under RAW_DIR.
291+
const rawFile = path.join(RAW_DIR, `run-${memRunId}`, `${memResId}.json`);
292+
assert(!existsSync(rawFile), `in-memory insert wrote NO file (${rawFile})`);
293+
// And the RAW_DIR root itself was not created by any :memory: insert so far.
294+
assert(!existsSync(RAW_DIR), `RAW_DIR not created by :memory: tests (${RAW_DIR})`);
295+
}
296+
271297
// ─── Results ──────────────────────────────────────────────────────────────────
272298

273299
console.log(`\n${'='.repeat(50)}`);

0 commit comments

Comments
 (0)