mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-30 03:12:32 +00:00
* fix(engine): batch inserts use jsonb_to_recordset, not text[] array literals (#1861) addLinksBatch/addTimelineEntriesBatch/addTakesBatch passed free text through unnest(${arr}::text[]); postgres.js serialized it to a Postgres text[] literal that array_in rejected ("malformed array literal") on calendar/Zoom context, aborting the whole `extract links --stale` sweep. Bind the batch as one JSONB doc via jsonb_to_recordset(($1::jsonb)->'rows') through the audited executeRawJsonb contract instead. Shared row builders (src/core/batch-rows.ts) keep both engines byte-identical; NUL is stripped only from free-text body fields (context/summary/detail/claim), while identity/security fields (slugs/source_ids/holder/kind/dates) still reject NUL. addTakesBatch is now batchRetry-wrapped ('addTakesBatch' audit site) and its BrainEngine signature takes BatchOpts. Scalar addLink context is NUL-stripped too. Regression tests on both engines: PGLite always-on poison/NUL/parity suite + DATABASE_URL-gated Postgres lane (the engine that actually crashed). * test: make "no Anthropic key" tests hermetic via withoutAnthropicKey hasAnthropicKey() reads both ANTHROPIC_API_KEY and ~/.gbrain config; tests that only deleted the env var fired a real LLM call on configured machines (warning flipped NO_ANTHROPIC_API_KEY -> LLM_OUTPUT_NOT_JSON). New test/helpers/no-anthropic-key.ts neutralizes both sources (env + GBRAIN_HOME temp dir) for the duration of the call. Refactors the five no-key tests in think-pipeline + takes-mcp-allowlist to use it, including two that previously passed only by luck of the live LLM output. * chore: docs + version bump (v0.42.28.0) KEY_FILES.md/RETRIEVAL.md describe the jsonb_to_recordset batch path; TODOS.md files the #1861 follow-ups (element-isolation, remaining ::text[] sites, shared SQL-string hoist, batch-insert edge-case tests). CHANGELOG + VERSION + package.json to 0.42.28.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: sync TESTING.md batch-insert references for v0.42.28.0 The #1861 fix migrated links/timeline/takes batch inserts from unnest(::text[]) to jsonb_to_recordset. Update the stale "postgres-js unnest() binding" note and add the two new poison-regression test files (test/links-timeline-jsonb-poison.test.ts PGLite half, test/e2e/jsonb-batch-poison-postgres.test.ts Postgres lane) to the inventory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sql-query): reject top-level array jsonb params in executeRawJsonb (#1861 P2a) The "no top-level array" rule was only a comment. A bare JS array bound to a $N::jsonb position can serialize as a Postgres array literal (not jsonb) through postgres.js, silently re-entering the "malformed array literal" class #1861 just escaped. executeRawJsonb now throws a clear error steering callers to the { rows: [...] } object wrapper. Verified breaks zero call sites (all pass objects or null). Codex adversarial P2a; batch-size enforcement (P2b) filed as a TODO. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
137 lines
5.7 KiB
TypeScript
137 lines
5.7 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 v0.12.0 double-encode bug: the bug was specific
|
|
* to postgres.js's template-tag auto-stringify path interacting with
|
|
* sql.json() — not to positional binding through `unsafe()`. JS objects
|
|
* passed as positional params reach the wire protocol with the correct
|
|
* type oid (jsonb when cast in the SQL string), so there is no double-
|
|
* encode. The CI guard (scripts/check-jsonb-pattern.sh) doesn't fire
|
|
* because the source pattern is a method call (`executeRawJsonb(...)`),
|
|
* not the banned literal-template-tag interpolation pattern with
|
|
* JSON.stringify cast to jsonb.
|
|
*
|
|
* 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);
|
|
}
|