fix(db): sweep positional jsonb double-encode sites + AST CI guard (#2324)

Every executeRaw/.unsafe site that bound JSON.stringify(x) to a bare positional
jsonb cast double-encodes on real Postgres (same class as #2339). Sweep them all
to the text::jsonb form across query-cache, sources-ops, llm-base,
calibration-profile, impact-capture, subagent, receipt-write, traversal-cache,
symbol-resolver, and the agent/sources commands. Adds scripts/check-jsonb-params.mjs
(AST-lite scanner for the positional form the legacy template grep misses, incl.
generic-typed calls), wired into check-jsonb-pattern.sh, with a self-test. PGLite's
native db.query is not scanned — it parses text to jsonb natively, so the bug can't
occur there.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-06-23 15:04:41 -07:00
co-authored by Claude Opus 4.8
parent 95acb2f360
commit 51de806d99
14 changed files with 255 additions and 28 deletions
+128
View File
@@ -0,0 +1,128 @@
#!/usr/bin/env node
/**
* CI guard for the POSITIONAL jsonb double-encode footgun (#2339 / #2324 class).
*
* The legacy scripts/check-jsonb-pattern.sh only catches the template-tag form
* (`${JSON.stringify(x)}::jsonb`). It MISSES the positional-param form:
*
* engine.executeRaw(`... $3::jsonb ...`, [a, b, JSON.stringify(x)])
*
* Under postgres.js `.unsafe(sql, params)` a JS STRING bound to a `$N::jsonb`
* param double-encodes — the text→jsonb cast wraps the already-JSON string into a
* jsonb *string scalar*. PGLite parses it silently, so the bug is invisible in
* unit tests and only bites on real Postgres (it aborted every sync in #2339).
*
* This scanner flags any executeRaw / executeRawDirect / .unsafe(...) call whose
* balanced argument span contains BOTH a positional `$N::jsonb` cast
* (NOT `$N::text::jsonb`, NOT `$N::text[]`) AND a `JSON.stringify(` — the exact
* double-encode shape. It is heuristic by design (whole-span correlation); the
* real backstop is the DATABASE_URL-gated e2e parity test. Keep both.
*
* Allowed forms (NOT flagged):
* - `$N::text::jsonb` + JSON.stringify (the fix: binds as text, cast parses it)
* - `$N::text[]` (the unnest path — arrays bind fine)
* - executeRawJsonb(...) (passes raw objects, not strings)
* - sql.json(x) (postgres.js native jsonb serializer)
* - a `jsonb-guard-ok` comment anywhere in the call span (explicit opt-out)
*
* Exit 0 = clean, 1 = violations found. Runs under node or bun.
*/
import { readdirSync, readFileSync, statSync } from 'node:fs';
import { join } from 'node:path';
// Default scan roots; overridable via argv so the guard's own test can point it
// at a fixture dir (e.g. `node check-jsonb-params.mjs /tmp/fixtures`).
const ROOTS = process.argv.slice(2).length > 0 ? process.argv.slice(2) : ['src', 'scripts'];
// executeRawDirect must precede executeRaw in the alternation so the longer name
// wins; executeRawJsonb is deliberately excluded (it passes objects). The
// optional `<...>` handles generic type args, e.g. `executeRaw<{ id: string }>(`.
//
// Only the postgres.js raw path is scanned (executeRaw/executeRawDirect/.unsafe).
// PGLite's native `this.db.query(...)` is intentionally NOT matched: its driver
// parses a text→jsonb cast natively, so the double-encode that bites postgres.js
// `.unsafe()` does not occur there (the `pglite-masks` invariant). The engine
// parity test pins that the resulting jsonb_typeof agrees across both engines.
const CALL_RE = /\b(executeRawDirect|executeRaw|unsafe)\s*(?:<[^>;]*>)?\s*\(/g;
/** Walk from the '(' at openIdx and return [start,end) of the balanced span,
* respecting strings, template literals, and comments. */
function findSpan(src, openIdx) {
let depth = 0;
let mode = 'code'; // code | line | block | sq | dq | tpl
for (let i = openIdx; i < src.length; i++) {
const c = src[i];
const n = src[i + 1];
if (mode === 'line') { if (c === '\n') mode = 'code'; continue; }
if (mode === 'block') { if (c === '*' && n === '/') { mode = 'code'; i++; } continue; }
if (mode === 'sq') { if (c === '\\') { i++; continue; } if (c === "'") mode = 'code'; continue; }
if (mode === 'dq') { if (c === '\\') { i++; continue; } if (c === '"') mode = 'code'; continue; }
if (mode === 'tpl') { if (c === '\\') { i++; continue; } if (c === '`') mode = 'code'; continue; }
// mode === 'code'
if (c === '/' && n === '/') { mode = 'line'; i++; continue; }
if (c === '/' && n === '*') { mode = 'block'; i++; continue; }
if (c === "'") { mode = 'sq'; continue; }
if (c === '"') { mode = 'dq'; continue; }
if (c === '`') { mode = 'tpl'; continue; }
if (c === '(') depth++;
else if (c === ')') { depth--; if (depth === 0) return [openIdx + 1, i]; }
}
return [openIdx + 1, src.length];
}
/** Blank out comments so a commented-out example doesn't trip the JSON.stringify probe. */
function stripComments(s) {
return s.replace(/\/\/[^\n]*/g, '').replace(/\/\*[\s\S]*?\*\//g, '');
}
const violations = [];
function scanFile(file) {
const src = readFileSync(file, 'utf8');
CALL_RE.lastIndex = 0;
let m;
while ((m = CALL_RE.exec(src))) {
const method = m[1];
const openIdx = m.index + m[0].length - 1; // index of the '('
const [s, e] = findSpan(src, openIdx);
const span = src.slice(s, e);
if (/jsonb-guard-ok/.test(span)) continue;
if (!/JSON\.stringify\s*\(/.test(stripComments(span))) continue;
// A positional `$N::jsonb` that is NOT `$N::text::jsonb`.
const jsonbRe = /\$\d+\s*::\s*jsonb\b/g;
let j;
let badText = '';
while ((j = jsonbRe.exec(span))) {
const pre = span.slice(Math.max(0, j.index - 12), j.index);
if (/::\s*text\s*$/.test(pre)) continue; // $N::text::jsonb is the fix — allowed
badText = j[0].replace(/\s+/g, '');
break;
}
if (!badText) continue;
const line = src.slice(0, s).split('\n').length;
violations.push(
`${file}:${line} ${method}(...) binds JSON.stringify into ${badText} — use $N::text::jsonb or pass a raw object (executeRawJsonb / sql.json)`,
);
}
}
function walk(dir) {
let ents;
try { ents = readdirSync(dir); } catch { return; }
for (const ent of ents) {
if (ent === 'node_modules') continue;
const p = join(dir, ent);
const st = statSync(p);
if (st.isDirectory()) walk(p);
else if (p.endsWith('.ts') && !p.endsWith('.test.ts')) scanFile(p);
}
}
for (const root of ROOTS) walk(root);
if (violations.length) {
console.error('JSONB positional double-encode violations (#2339 class):\n');
for (const v of violations) console.error(' ' + v);
console.error(`\n${violations.length} violation(s). Fix: bind through $N::text::jsonb (keeping JSON.stringify), or pass a raw object via executeRawJsonb / sql.json. See docs/ENGINES.md.`);
process.exit(1);
}
console.log('check-jsonb-params: clean (no positional $N::jsonb + JSON.stringify double-encodes)');
+14
View File
@@ -44,3 +44,17 @@ if grep -rEn "$MAX_STALLED_PATTERN" src/schema.sql src/core/migrate.ts src/core/
fi
echo "OK: max_stalled defaults are 5 in all schema sources"
# v0.42.x (#2339 / #2324): positional `$N::jsonb` + JSON.stringify double-encode.
# The template-string grep above only catches `${JSON.stringify(x)}::jsonb`. It
# MISSES the positional-param form — executeRaw(`... $N::jsonb ...`,
# [JSON.stringify(x)]) — which is the exact shape that double-encoded the
# op_checkpoints pin and aborted every sync in #2339. The AST-lite scanner below
# catches it. `set -e` propagates its non-zero exit.
if command -v node >/dev/null 2>&1; then
node scripts/check-jsonb-params.mjs
elif command -v bun >/dev/null 2>&1; then
bun scripts/check-jsonb-params.mjs
else
echo "WARN: neither node nor bun on PATH; skipping check-jsonb-params.mjs" >&2
fi
+1 -1
View File
@@ -251,7 +251,7 @@ async function runFanout(engine: BrainEngine, queue: MinionQueue, flags: RunFlag
// do this after submission because each add() returns the committed
// row's id; the aggregator's seed started with an empty array.
await engine.executeRaw(
`UPDATE minion_jobs SET data = jsonb_set(data, '{children_ids}', $1::jsonb) WHERE id = $2`,
`UPDATE minion_jobs SET data = jsonb_set(data, '{children_ids}', $1::text::jsonb) WHERE id = $2`,
[JSON.stringify(childIds), aggregator.id],
);
+6 -6
View File
@@ -692,7 +692,7 @@ async function runFederate(engine: BrainEngine, args: string[], value: boolean):
const config = parseConfig(src.config);
config.federated = value;
await engine.executeRaw(
`UPDATE sources SET config = $1::jsonb WHERE id = $2`,
`UPDATE sources SET config = $1::text::jsonb WHERE id = $2`,
[JSON.stringify(config), id],
);
console.log(`Source "${id}" is now ${value ? 'federated (appears in cross-source default search)' : 'isolated (only searched when explicitly named)'}.`);
@@ -854,7 +854,7 @@ async function runWebhookSet(engine: BrainEngine, args: string[]): Promise<void>
cfg.webhook_secret = secret;
cfg.github_repo = githubRepo;
await engine.executeRaw(
`UPDATE sources SET config = $1::jsonb WHERE id = $2`,
`UPDATE sources SET config = $1::text::jsonb WHERE id = $2`,
[JSON.stringify(cfg), id],
);
@@ -910,7 +910,7 @@ async function runWebhookRotate(engine: BrainEngine, args: string[]): Promise<vo
const cfg = parseConfig(src.config);
cfg.webhook_secret = secret;
await engine.executeRaw(
`UPDATE sources SET config = $1::jsonb WHERE id = $2`,
`UPDATE sources SET config = $1::text::jsonb WHERE id = $2`,
[JSON.stringify(cfg), id],
);
console.log(`New webhook secret for source "${id}":`);
@@ -934,7 +934,7 @@ async function runWebhookClear(engine: BrainEngine, args: string[]): Promise<voi
delete cfg.webhook_secret;
delete cfg.github_repo;
await engine.executeRaw(
`UPDATE sources SET config = $1::jsonb WHERE id = $2`,
`UPDATE sources SET config = $1::text::jsonb WHERE id = $2`,
[JSON.stringify(cfg), id],
);
console.log(`Webhook configuration cleared for source "${id}".`);
@@ -959,7 +959,7 @@ async function runTrackedBranch(engine: BrainEngine, args: string[]): Promise<vo
if (setArg) {
cfg.tracked_branch = setArg;
await engine.executeRaw(
`UPDATE sources SET config = $1::jsonb WHERE id = $2`,
`UPDATE sources SET config = $1::text::jsonb WHERE id = $2`,
[JSON.stringify(cfg), id],
);
console.log(`Tracked branch for source "${id}" set to "${setArg}".`);
@@ -975,7 +975,7 @@ async function runTrackedBranch(engine: BrainEngine, args: string[]): Promise<vo
const branch = execFileSync('git', ['-C', src.local_path, 'rev-parse', '--abbrev-ref', 'HEAD'], { encoding: 'utf8' }).trim();
cfg.tracked_branch = branch;
await engine.executeRaw(
`UPDATE sources SET config = $1::jsonb WHERE id = $2`,
`UPDATE sources SET config = $1::text::jsonb WHERE id = $2`,
[JSON.stringify(cfg), id],
);
console.log(`Detected branch "${branch}" for source "${id}"; persisted to config.tracked_branch.`);
Binary file not shown.
+1 -1
View File
@@ -123,7 +123,7 @@ export async function putCachedTraversal<T>(
`INSERT INTO code_traversal_cache
(symbol_qualified, depth, source_id, response_json,
max_chunk_updated_at, xmin_max, cluster_generation)
VALUES ($1, $2, $3, $4::jsonb, $5::timestamptz, $6, $7)
VALUES ($1, $2, $3, $4::text::jsonb, $5::timestamptz, $6, $7)
ON CONFLICT (symbol_qualified, depth, source_id)
DO UPDATE SET
response_json = EXCLUDED.response_json,
+4 -4
View File
@@ -266,13 +266,13 @@ async function writeDbCache<T>(
): Promise<void> {
const [, , contentSha] = splitCacheKey(key);
if (!contentSha) return;
// executeRaw with positional binding for JSONB. Per the sql-query.ts
// contract: object values passed via positional params reach the
// wire as proper jsonb when cast.
// #2339 class: this binds JSON.stringify(value) (a STRING) positionally, so it
// must cast through $4::text::jsonb — a bare $4::jsonb double-encodes under
// postgres.js .unsafe() (PGLite hides it). Pass a raw object to use $N::jsonb.
await engine.executeRaw(
`INSERT INTO conversation_parser_llm_cache
(content_sha256, model_id, call_shape, value_json)
VALUES ($1, $2, $3, $4::jsonb)
VALUES ($1, $2, $3, $4::text::jsonb)
ON CONFLICT (content_sha256, model_id, call_shape) DO NOTHING`,
[contentSha, modelStr, shape, JSON.stringify(value)],
);
+1 -1
View File
@@ -356,7 +356,7 @@ class CalibrationProfilePhase extends BaseCyclePhase {
active_bias_tags, model_id, cost_usd, judge_model_agreement
) VALUES ($1, $2, now(), false,
$3, $4, $5, $6, $7,
$8::jsonb, $9::text[],
$8::text::jsonb, $9::text[],
$10, $11,
$12::text[], $13, NULL, NULL)`,
[
+11 -9
View File
@@ -878,7 +878,7 @@ async function runSubagentViaGateway(args: GatewayRunArgs): Promise<SubagentResu
const rows = await engine.executeRaw<{ gbrain_tool_use_id: string }>(
`INSERT INTO subagent_tool_executions
(job_id, message_idx, tool_use_id, tool_name, input, status, schema_version, ordinal, gbrain_tool_use_id, provider_id)
VALUES ($1, $2, $3, $4, $5::jsonb, 'pending', 2, $6, $7, $8)
VALUES ($1, $2, $3, $4, $5::text::jsonb, 'pending', 2, $6, $7, $8)
ON CONFLICT (job_id, message_idx, ordinal) DO UPDATE
SET status = subagent_tool_executions.status
RETURNING gbrain_tool_use_id::text AS gbrain_tool_use_id`,
@@ -891,7 +891,7 @@ async function runSubagentViaGateway(args: GatewayRunArgs): Promise<SubagentResu
onToolCallComplete: async (gbrainToolUseId, output) => {
await engine.executeRaw(
`UPDATE subagent_tool_executions
SET status = 'complete', output = $1::jsonb, ended_at = now()
SET status = 'complete', output = $1::text::jsonb, ended_at = now()
WHERE gbrain_tool_use_id::text = $2`,
[JSON.stringify(output ?? null), gbrainToolUseId],
);
@@ -1107,7 +1107,7 @@ async function persistMessage(engine: BrainEngine, jobId: number, msg: Persisted
await engine.executeRaw(
`INSERT INTO subagent_messages (job_id, message_idx, role, content_blocks,
tokens_in, tokens_out, tokens_cache_read, tokens_cache_create, model)
VALUES ($1, $2, $3, $4::jsonb, $5, $6, $7, $8, $9)
VALUES ($1, $2, $3, $4::text::jsonb, $5, $6, $7, $8, $9)
ON CONFLICT (job_id, message_idx) DO NOTHING`,
[
jobId,
@@ -1131,13 +1131,15 @@ async function persistToolExecPending(
toolName: string,
input: unknown,
): Promise<void> {
// Serialize to JSON string for the ::jsonb cast. When `input` is already a
// string (e.g. pre-serialized), avoid double-encoding which produces a jsonb
// scalar string instead of a jsonb object — breaking `input->>'key'` lookups.
// Serialize to a JSON string, then bind through $5::text::jsonb. The value is
// ALWAYS a string here (pre-serialized input, or JSON.stringify) — binding a
// string to a bare $5::jsonb double-encodes it into a jsonb scalar string under
// postgres.js .unsafe() (#2339 class; PGLite hides it). The ::text cast makes
// the text→jsonb parse produce a real jsonb object.
const jsonStr = typeof input === 'string' ? input : JSON.stringify(input);
await engine.executeRaw(
`INSERT INTO subagent_tool_executions (job_id, message_idx, tool_use_id, tool_name, input, status)
VALUES ($1, $2, $3, $4, $5::jsonb, 'pending')
VALUES ($1, $2, $3, $4, $5::text::jsonb, 'pending')
ON CONFLICT (job_id, tool_use_id) DO NOTHING`,
[jobId, messageIdx, toolUseId, toolName, jsonStr],
);
@@ -1151,7 +1153,7 @@ async function persistToolExecComplete(
): Promise<void> {
await engine.executeRaw(
`UPDATE subagent_tool_executions
SET status = 'complete', output = $3::jsonb, ended_at = now()
SET status = 'complete', output = $3::text::jsonb, ended_at = now()
WHERE job_id = $1 AND tool_use_id = $2`,
[jobId, toolUseId, typeof output === 'string' ? output : JSON.stringify(output)],
);
@@ -1170,7 +1172,7 @@ async function persistToolExecFailed(
// rejected upfront) and "pending row exists" (tool threw mid-execute).
await engine.executeRaw(
`INSERT INTO subagent_tool_executions (job_id, message_idx, tool_use_id, tool_name, input, status, error, ended_at)
VALUES ($1, $2, $3, $4, $5::jsonb, 'failed', $6, now())
VALUES ($1, $2, $3, $4, $5::text::jsonb, 'failed', $6, now())
ON CONFLICT (job_id, tool_use_id) DO UPDATE
SET status = 'failed', error = EXCLUDED.error, ended_at = now()`,
[jobId, messageIdx, toolUseId, toolName, typeof input === 'string' ? input : JSON.stringify(input), error],
+1 -1
View File
@@ -120,7 +120,7 @@ export async function writeImpactLogRow(
remediation_id, metric_name, metric_before, metric_after,
job_id, source_id, brain_id, started_at, idempotency_key,
applied_by, details
) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11::jsonb)`,
) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11::text::jsonb)`,
[
attribution.remediation_id,
metricName,
+1 -1
View File
@@ -236,7 +236,7 @@ export class SemanticQueryCache {
// the v0.40.3.0 IRON-RULE).
await this.engine.executeRaw(
`INSERT INTO query_cache (id, query_text, source_id, knobs_hash, embedding, results, meta, ttl_seconds, page_generations, max_generation_at_store, created_at)
VALUES ($1, $2, $3, $4, $5::vector, $6::jsonb, $7::jsonb, $8, $9::jsonb, $10, now())
VALUES ($1, $2, $3, $4, $5::vector, $6::text::jsonb, $7::text::jsonb, $8, $9::text::jsonb, $10, now())
ON CONFLICT (id) DO UPDATE SET
query_text = EXCLUDED.query_text,
knobs_hash = EXCLUDED.knobs_hash,
+2 -2
View File
@@ -408,7 +408,7 @@ export async function addSource(
try {
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, config)
VALUES ($1, $2, $3, $4::jsonb)`,
VALUES ($1, $2, $3, $4::text::jsonb)`,
[opts.id, displayName, finalPath, JSON.stringify(config)],
);
} catch (e) {
@@ -454,7 +454,7 @@ export async function addSource(
const displayName = opts.name ?? opts.id;
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, config)
VALUES ($1, $2, $3, $4::jsonb)`,
VALUES ($1, $2, $3, $4::text::jsonb)`,
[opts.id, displayName, finalPath, JSON.stringify(config)],
);
}
+2 -2
View File
@@ -32,8 +32,8 @@ export async function writeReceiptToDb(engine: BrainEngine, receipt: TakesQualit
receipt_json, receipt_disk_path, created_at
) VALUES (
$1, $2, $3, $4,
$5, $6, $7, $8::jsonb, $9,
$10::jsonb, $11, $12::timestamptz
$5, $6, $7, $8::text::jsonb, $9,
$10::text::jsonb, $11, $12::timestamptz
)
ON CONFLICT (receipt_sha8_corpus, receipt_sha8_prompt, receipt_sha8_models, receipt_sha8_rubric)
DO NOTHING`,
+83
View File
@@ -0,0 +1,83 @@
/**
* Self-test for scripts/check-jsonb-params.mjs — the positional jsonb
* double-encode guard (#2339 / #2324 class). Verifies it catches the bug shape
* (including generic-typed calls and the `jsonStr` variable case is acknowledged
* as out of scope) and does NOT false-positive on the sanctioned forms.
*
* Fixtures are written to a temp dir and the scanner is pointed at it via argv,
* so this never touches the real src/ tree.
*/
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
const SCRIPT = join(import.meta.dir, '..', 'scripts', 'check-jsonb-params.mjs');
let root: string;
let badDir: string;
let goodDir: string;
function runGuard(dir: string): { code: number; err: string } {
const res = Bun.spawnSync([process.execPath, SCRIPT, dir]);
return { code: res.exitCode, err: res.stderr.toString() + res.stdout.toString() };
}
beforeAll(() => {
root = mkdtempSync(join(tmpdir(), 'jsonb-guard-'));
badDir = join(root, 'bad');
goodDir = join(root, 'good');
mkdirSync(badDir, { recursive: true });
mkdirSync(goodDir, { recursive: true });
// BAD: positional $1::jsonb bound to a JSON.stringify'd value.
writeFileSync(
join(badDir, 'bad.ts'),
"await engine.executeRaw(`INSERT INTO t (a) VALUES ($1::jsonb)`, [JSON.stringify(x)]);\n",
);
// BAD: generic-typed executeRaw<T>(...) must still be caught.
writeFileSync(
join(badDir, 'bad_generic.ts'),
"await engine.executeRaw<{ id: string }>(`UPDATE t SET a = $2::jsonb WHERE id = $1`, [id, JSON.stringify(x)]);\n",
);
// GOOD: the fix — $1::text::jsonb.
writeFileSync(
join(goodDir, 'good_text.ts'),
"await engine.executeRaw(`INSERT INTO t (a) VALUES ($1::text::jsonb)`, [JSON.stringify(x)]);\n",
);
// GOOD: text[] array path (the appendCompleted unnest shape).
writeFileSync(
join(goodDir, 'good_array.ts'),
"await engine.executeRaw(`INSERT INTO t (a) SELECT unnest($1::text[])`, [JSON.stringify(arr)]);\n",
);
// GOOD: executeRawJsonb passes a raw object, not a string — excluded.
writeFileSync(
join(goodDir, 'good_helper.ts'),
"await executeRawJsonb(engine, `INSERT INTO t (a) VALUES ($1::jsonb)`, [], [JSON.stringify(x)]);\n",
);
// GOOD: explicit opt-out for a rare legitimate object-binding case.
writeFileSync(
join(goodDir, 'good_optout.ts'),
"await engine.executeRaw(`INSERT INTO t (a) VALUES ($1::jsonb)` /* jsonb-guard-ok */, [JSON.stringify(x)]);\n",
);
});
afterAll(() => {
rmSync(root, { recursive: true, force: true });
});
describe('check-jsonb-params guard', () => {
test('flags positional $N::jsonb + JSON.stringify (incl. generic-typed calls)', () => {
const { code, err } = runGuard(badDir);
expect(code).toBe(1);
expect(err).toContain('bad.ts');
expect(err).toContain('bad_generic.ts');
});
test('passes the sanctioned forms (::text::jsonb, ::text[], executeRawJsonb, opt-out)', () => {
const { code, err } = runGuard(goodDir);
expect(code).toBe(0);
expect(err).toContain('clean');
});
});