Files
gbrain/src/core/sql-query.ts
T
814258dda6 v0.42.53.0 fix(sync,db): #2339 op_checkpoints jsonb double-encode + bug-class sweep + CI guard (#2375)
* fix(sync): op_checkpoints pin write double-encodes jsonb — every sync aborts (#2339)

recordCompleted bound JSON.stringify(array) to a $3::jsonb param via postgres.js
.unsafe(), double-encoding it into a jsonb string scalar that violates the v119
op_checkpoints_completed_keys_array CHECK — aborting every multi-source sync on
real Postgres at the first checkpoint write. PGLite parses the string silently,
which is why unit tests stayed green and it shipped. Cast through $3::text::jsonb
so the text->jsonb cast parses a genuine array. Adds a DATABASE_URL-gated parity
test + a dedicated Postgres CI job so the guard can never silently skip.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* 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>

* fix(search,eval): alias-hop injected results carry page_id (contradiction-probe crash)

applyAliasHop injected synthetic SearchResults without page_id (the `as
SearchResult` cast hid the missing field), so listActiveTakesForPages bound
undefined/NaN into ANY($1::int[]) and crashed the whole contradiction probe on
real Postgres. Stamp page_id=page.id at the injection site and add a finite-id
filter in generateIntraPagePairs as a defensive backstop (mirrors hybrid.ts:63).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(engines): positional jsonb binding rule (text::jsonb vs the double-encode trap)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* v0.42.53.0 fix(sync,db): #2339 op_checkpoints jsonb double-encode + bug-class sweep + CI guard

Bumps VERSION + package.json to 0.42.53.0, adds the CHANGELOG entry, and
regenerates llms-full.txt. Ships the #2339 sync-abort hotfix, the repo-wide
positional jsonb double-encode sweep, the alias-hop contradiction-probe crash
fix, and the new positional-form CI guard.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs: post-ship sync — jsonb invariant now covers the positional form + new guard

CLAUDE.md JSONB invariant + KEY_FILES (sql-query, check-jsonb-pattern, op-checkpoint)
now describe the #2339 positional double-encode class, the $N::text::jsonb fix, and
the new check-jsonb-params.mjs guard. Regenerates llms-full.txt.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-24 06:05:16 -07:00

144 lines
6.3 KiB
TypeScript

import type { BrainEngine } from './engine.ts';
/**
* Minimal tagged SQL function used by OAuth/admin/auth infrastructure.
*
* This is deliberately narrower than postgres.js's `sql` tag: values must be
* scalar bind parameters only. It does not support nested SQL fragments,
* sql.json(), sql.unsafe(), sql.begin(), or direct JS array binding. JSONB
* writes go through the separate `executeRawJsonb` helper below.
*
* The narrow surface is the feature: every call site in auth.ts /
* serve-http.ts / mcp/http-transport.ts / files.ts answers "do you support
* X?" with "no, and that's the contract." That keeps the adapter from
* drifting into a partial postgres.js clone (codex finding #7 from the
* v0.31 plan review).
*/
export type SqlValue = string | number | bigint | boolean | Date | null;
export type SqlQuery = (strings: TemplateStringsArray, ...values: SqlValue[]) => Promise<Record<string, unknown>[]>;
/**
* Build a minimal tagged-template SQL adapter over the active BrainEngine.
*
* OAuth/admin code only needs scalar positional parameters plus returned rows.
* Using BrainEngine.executeRaw keeps the path engine-aware: Postgres goes
* through the connected postgres.js client (`unsafe(sql, params)`), while
* PGLite goes through its embedded `db.query(sql, params)`.
*
* The v0.12.0 double-encode bug class does NOT apply here because
* executeRaw uses positional binding, not the postgres.js template tag's
* auto-stringify path that caused the original silent-data-loss incident.
*/
export function sqlQueryForEngine(engine: BrainEngine): SqlQuery {
return async (strings: TemplateStringsArray, ...values: SqlValue[]) => {
for (const value of values) {
assertSqlValue(value);
}
const query = strings.reduce((acc, str, i) => {
return acc + str + (i < values.length ? `$${i + 1}` : '');
}, '');
return engine.executeRaw(query, values);
};
}
function assertSqlValue(value: unknown): asserts value is SqlValue {
if (
value === null ||
typeof value === 'string' ||
typeof value === 'number' ||
typeof value === 'bigint' ||
typeof value === 'boolean' ||
value instanceof Date
) {
return;
}
const kind = Array.isArray(value)
? 'array'
: value && typeof (value as { then?: unknown }).then === 'function'
? 'promise'
: typeof value;
throw new TypeError(
`sqlQueryForEngine only supports scalar bind values; got ${kind}. ` +
'Use fixed SQL with scalar params, or executeRawJsonb for JSONB writes.',
);
}
/**
* Cross-engine JSONB write helper. Composes a parametrized SQL string with
* explicit `$N::jsonb` casts for the JSONB positions and passes the JSONB
* values as JS objects through `engine.executeRaw`. Both the postgres.js
* `unsafe(sql, params)` path (via PostgresEngine) and PGLite's
* `db.query(sql, params)` accept objects for `$N::jsonb` positions and
* round-trip them with `jsonb_typeof = 'object'` (verified by
* test/e2e/auth-permissions.test.ts:67 on Postgres and test/sql-query.test.ts
* on PGLite).
*
* Why this exists separately from SqlQuery: the SqlQuery contract is
* deliberately scalar-only. JSONB columns are rare enough across the
* auth/admin surface that a focused helper preserves the contract without
* forcing every call site to remember which positions hold JSONB.
*
* Why this is safe vs the double-encode bug: this helper binds a JS **object**
* (not a pre-stringified string) to each `$N::jsonb` position. postgres.js
* `unsafe()` and PGLite both serialize a JS object to the jsonb wire type
* correctly, so there is no double-encode.
*
* IMPORTANT (the #2339 distinction): positional binding is NOT universally safe.
* Binding `JSON.stringify(x)` (a **string**) to a `$N::jsonb` position via
* `unsafe()`/`executeRawDirect` DOES double-encode — the text→jsonb cast wraps
* the already-JSON string into a jsonb *string scalar* (PGLite hides it; real
* Postgres exposes it, and it broke every sync in #2339). The fixes are: pass a
* raw object (this helper), or cast through `$N::text::jsonb` so the string is
* parsed, never `$N::jsonb` + JSON.stringify. The legacy grep guard
* (scripts/check-jsonb-pattern.sh) only caught the template-tag form; the
* positional `$N::jsonb` + JSON.stringify form is caught by the AST guard
* scripts/check-jsonb-params.mjs. This helper's `executeRawJsonb(...)` method-call
* shape trips neither guard because it passes objects, which is correct.
*
* Usage:
* await executeRawJsonb(
* engine,
* `INSERT INTO access_tokens (name, token_hash, permissions)
* VALUES ($1, $2, $3::jsonb)`,
* [name, hash],
* [{ takes_holders: ['world', 'garry'] }],
* );
*
* The SQL string MUST already contain the `$N::jsonb` casts; the helper
* does NOT rewrite or inject them. Scalar params come first ($1..$N), then
* JSONB params ($N+1..$N+M). Matching the call-site convention to scalars-
* before-JSONB simplifies argument order and matches how the existing
* call sites we're migrating are shaped.
*/
export async function executeRawJsonb<R = Record<string, unknown>>(
engine: BrainEngine,
sql: string,
scalarParams: SqlValue[],
jsonbParams: unknown[],
): Promise<R[]> {
for (const value of scalarParams) {
assertSqlValue(value);
}
// jsonbParams hold JS objects (or null) that postgres.js / PGLite encode as
// JSONB via the explicit `::jsonb` cast in the caller's SQL string. A
// top-level ARRAY is rejected: postgres.js can bind a bare JS array as a
// Postgres ARRAY literal rather than jsonb, which silently re-enters the
// "malformed array literal" class gbrain#1861 exists to escape. Wrap arrays
// in an object, e.g. `[{ rows: [...] }]` selected via
// `jsonb_to_recordset(($N::jsonb)->'rows')`. This enforces at the call layer
// the invariant the batch-insert methods rely on (codex #1861 P2a).
for (const value of jsonbParams) {
if (Array.isArray(value)) {
throw new TypeError(
'executeRawJsonb: a top-level array jsonb param can bind as a Postgres ' +
'array literal (not jsonb) through postgres.js. Wrap it in an object — ' +
"e.g. `[{ rows: [...] }]` with `jsonb_to_recordset(($N::jsonb)->'rows')`. " +
'(gbrain#1861)',
);
}
}
const params: unknown[] = [...scalarParams, ...jsonbParams];
return engine.executeRaw<R>(sql, params);
}