Repository navigation
feat: accept the .wasm bytes or a compiled module in loadModule() - #193
Merged
Merged
Conversation
Hosts that cannot fetch or read libpg-query.wasm (a custom protocol without
fetch support, a bundle that inlines assets) can pass it once with
loadModule({ wasm }). Every later instance, including replacements after a
failed call, is created from it. Applies to v13-v18.
Co-authored-by: Gldywn <14254051+Gldywn@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Review complete. 🟠 1 high · 🟡 1 medium 💬 Inline comments (2)
The change threads a caller-provided WASM module through Emscripten's
Note: the new Reviewed commit: 4640ae9 |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds a loadModule({ wasm }) API letting callers supply pre-compiled WASM bytes or a WebAssembly.Module, copied identically across all six versioned packages, plus a subprocess-based test harness for WASM load recovery.
Key findings
- 🟠 Pass the
WebAssembly.Instance, not the result record — index.ts:155 - 🟡 Subprocess failures produce opaque test errors — load-module.test.js:27
…cs: pass only trusted wasm
devin-ai-integration Bot
pushed a commit
that referenced
this pull request
Oct 29, 2026
Provided wasm sets compiled, so the sync replaceInstance recovery path reuses it without fetching; load-module OOM test asserts the repeated-limit error.
Gldywn
added a commit
to Gldywn/gatekeeper
that referenced
this pull request
Oct 31, 2026
…rst (#73) * feat(plugin): load PostgreSQL's own grammar as WASM at startup Adds libpg-query 18.1.5 and pgsql-deparser 18.3.8 (exact pins, operator-approved) and awaits the WASM grammar once before the app starts, so later parses stay synchronous. The build copies libpg-query.wasm next to the page, where its Emscripten loader fetches it. * build(plugin): serve the PostgreSQL WASM grammar in the fixture preview One Vite plugin, shared by the build and the preview, serves libpg-query.wasm in dev and emits it in the bundle. The preview loads the grammar before rendering, and its in-memory broker lets that one same-origin asset through while every other fetch stays blocked. * build(plugin): ship the PostgreSQL parser's license notices libpg-query.wasm embeds libpg_query (BSD-3) and PostgreSQL parser sources (PostgreSQL License), whose texts the npm packages do not carry. public/ lands in dist/, which the release zip takes whole. The two upstream texts are verbatim (pganalyze/libpg_query 18.0.0, postgres REL_18_STABLE). * fix(plugin): log the parser load failure without promising a fallback Classification still runs on node-sql-parser on this branch, so the message only states that the grammar failed to load. * refactor(plugin): route SQL parsing through a per-engine registry One SqlEngine interface and one engineFor(databaseType) replace mapDialect and the parser calls scattered over the gate, the schema annotations, the receipt and the row count. Every caller now carries Beekeeper's raw databaseType. The node-sql-parser code moves as is into sql/nsp/, the parser-independent parts stay in risk.ts and schema.ts, and the dispatching functions live in classify.ts and inspect.ts. No behaviour changes: every engine, PostgreSQL included, still reads SQL through node-sql-parser with the same grammar. * feat(plugin): gate PostgreSQL on PostgreSQL's own grammar A real PostgreSQL connection is now classified on libpg_query's parse tree: one statement, reads only for SELECT and SHOW, a lock, an INTO or a modifying statement escalates wherever it sits, and EXPLAIN carries its statement's risk unless every ANALYZE option is explicitly false. Text the parser cannot vouch for is blocked rather than read as text: a NUL ends its C string, and a backslash before a quote in an ordinary string splits statements differently when the server runs with standard_conforming_strings off. Other engines, including PostgreSQL look-alikes, keep node-sql-parser. Gate tests: VACUUM and DROP DATABASE now parse (same class, approvable); a nested EXPLAIN, which PostgreSQL rejects, is destructive and approvable instead of blocked. * feat(plugin): annotate PostgreSQL from its own parse tree Tables, source columns, output names, JSON keys, sensitive literals, the decided receipt and the row-count rule now read libpg_query's tree on a real PostgreSQL connection. Literals come back as written, escapes included, so the query highlight still finds them, while their shape is checked on the decoded value. Three node-sql-parser defects the old tests pinned are gone: a CTE is no longer reported as a table, a quoted alias holding parentheses is no longer split into columns, and the value of IS DISTINCT FROM is now screened. Auto's backslash check reads PostgreSQL's scanner instead of the quote adapter. * fix(plugin): close six gaps in the PostgreSQL extraction From the cross-read of the native extraction: a CTE now hides a table only inside its own WITH scope, MERGE actions expose their columns and values, a composite field selection counts as the column compared, a join alias renames its columns, a JSON key written as a cast string is read, and a dollar tag is taken as written whatever its letters. * feat(plugin): resolve Auto reads on PostgreSQL's own parse tree The Auto walker reads libpg_query's tree with the same scoping, dependency and catalog resolution. Jev's SQL is rendered from the redacted tree by pgsql-deparser, then parsed again by PostgreSQL's grammar: any difference, beyond positions, keeps the query local, and every constant left in it must be syntax (LIMIT, ordinal, EXTRACT unit, typmod, boolean, NULL) or a screened JSON key. Now supported: NULLS FIRST/LAST, DISTINCT ON, bare USING keys merged per join input as PostgreSQL does, IS [NOT] DISTINCT FROM, set operations nested as written (mixed precedence, parenthesized branches, INTERSECT ALL and EXCEPT ALL), FETCH FIRST/NEXT, timetz and quoted type names. Over all 753 corpus cases, 644 eligible inputs keep identical dependencies and withholding, 108 local cases keep their reason, and only the formatting of Jev's SQL changes (analysis shared-read-3). The one case that becomes eligible, mixed INTERSECT and UNION precedence on three status columns, was local only because of node-sql-parser and is relabelled allow with the operator's go. * refactor(plugin): use node-sql-parser plainly for the other engines PostgreSQL no longer goes through node-sql-parser, so the layers that rewrote its input and recomposed what the parser lost have nothing left to serve. The remaining engines now get the parser's own answer: - classification walks the tree as parsed: no EXPLAIN stripping, row locks come from the parser's locking_read instead of a text regex - MySQL and MariaDB refuse any statement holding /*!, whose body the parser drops; the strict text check then decides - extraction reads names and literals as the tree gives them: no name placeholders, no E'' / dollar-quote / relation column-list readers prepareSql, pg/syntax.ts and the node-sql-parser shape contract go. The PostgreSQL name cases worth keeping move to pg/names.test.ts. * docs: describe the per-engine parsers and native PostgreSQL analysis The gate, the annotations and Auto mode now read PostgreSQL with its own grammar and every other engine with node-sql-parser as it parses. The docs still described the node-sql-parser adapter, its rewrites and the shapes it could not represent. - ARCHITECTURE and DEVELOPMENT: the engine registry and the libraries - SECURITY: which parser reads what, the blocked backslash-quote string, the /*! refusal on MySQL and MariaDB - AUTO-MODE: shared-read-3, set operations as PostgreSQL groups them, the newly supported shapes, the deparse-and-reparse rendering, the updated manual list - AGENTS: the foundation example now refers to the removed adapter * chore(server): note which plugins still send the mixed set operation hold * fix(plugin): class an unread MySQL or MariaDB statement destructive A statement holding an executable comment fell to the text check, which strips comments without seeing strings: in SELECT '/*!', '/*' INTO OUTFILE '/tmp/x' /* end */ it swallowed the INTO OUTFILE and returned read, where the parsed tree had said destructive. An unread statement now takes the strictest class, approvable like any statement Gatekeeper cannot read, and stays blocked when the parser or the text check sees more than one statement. MariaDB's /*M! form, whose body MariaDB also runs, is unread too. A harmless optimizer hint now needs Destructive mode. Also pins Auto's refusal of an explicit window frame, which the deleted node-sql-parser shape contract used to cover. * docs: state precisely which grammar reads what and what Jev sees Corrections from the documentation review: - engines without a node-sql-parser grammar use its PostgreSQL one - a failed parse goes to the text check, which can return read; a PostgreSQL grammar that fails to load sends every statement there - the analysis uses PostgreSQL 18's grammar with standard_conforming_strings on, not the server's own reading - the deparser keeps an explicit ASC; NULL and booleans stay visible to Jev - /*! is refused on MySQL, /*! and /*M! on MariaDB * fix(plugin): stop listing CTE names as tables on the card A query defining a CTE (pay, stuck) showed it among the tables it reads, and the annotations looked its columns up as a table's. - PostgreSQL: the CTE filter now scopes as PostgreSQL does. Without RECURSIVE a CTE body sees only the CTEs before it, so a real table read there under a later CTE's name was hidden. - node-sql-parser: references to a name the statement defines as a CTE leave the table list. The list has no positions, so an unqualified real table sharing a CTE's name in the same statement is hidden too. * build(plugin): move libpg-query to 18.2.1 with a loader patch Beekeeper serves plugin pages from a scheme that cannot fetch the plugin's own files, and libpg-query only loads its WASM through Emscripten's fetch, started at import. The option that fixes it is proposed upstream (constructive-io/libpg-query-node#191) but not released. Until then, patches/libpg-query@18.2.1.patch gives the published wrapper the part of that API Gatekeeper relies on: nothing loads at import, and loadModule({ instantiateWasm }) hands the hook to every load, reloads after a WASM exception included. It is narrower than the PR: a locateFile that re-enters loadModule() starts a second load, options are not validated, and a failed load is retried as 18.2.1 already does. Gatekeeper passes only instantiateWasm. pnpm applies the patch on every install, and a missing or changed patch fails a frozen install. 18.2.1 brings the reload after a WASM exception and the NUL refusal. The operator allowed it before the end of its quarantine (2026-11-01) through an exception scoped to this version, to delete after that date. * fix(plugin): load PostgreSQL's grammar from the bundle, not a fetch The plugin fetched libpg-query.wasm next to its page, which works on Vite's dev server but not from Beekeeper's plugin scheme, so a real install had no PostgreSQL parser. The WASM is now inlined in the bundle (sql/pg/wasm.ts) and handed to the patched loader through instantiateWasm, which also serves every reload. The plugin, the fixture preview and the tests load through the same hook; the server's evals keep Node's load from disk. The Vite plugin that served and emitted the file, and the preview broker's exception for it, are gone. A test fails if libpg-query stops honouring the options, which an unpatched release would silently do. * refactor(plugin): read the load check through each engine's own parser The load check (#55) read every statement with node-sql-parser through the adapters this branch removed, so it no longer compiled. - sql/load-plan.ts holds the plan in no parser's terms: a block keeps its conditions as the columns each AND part and each OR branch name, so usage() and checkLoad() never read a parse tree again. - sql/pg/load.ts walks PostgreSQL's own tree. It reads FETCH, EXPLAIN options, CREATE TABLE AS, data-modifying CTEs, subqueries in every clause and the maintenance commands (VACUUM FULL, CLUSTER, REINDEX, REFRESH) from their nodes instead of their leading words. CTE visibility follows PostgreSQL, RECURSIVE included. Aggregates are told by their syntax and PostgreSQL 18's built-in names, a heuristic kept to this check. - sql/nsp/load.ts is main's walker on node-sql-parser used plainly. MySQL and MariaDB statements holding an executable comment stay unread, as for every other answer of that engine. Sizes, locks and the catalog read now follow Beekeeper's own type: Redshift, CockroachDB, DuckDB and the other engines read with node-sql-parser's PostgreSQL grammar no longer get the PostgreSQL catalog query, which main sent them. Two expectations described node-sql-parser's limits and now take PostgreSQL's answer: a DELETE inside a CTE is reported, and a comma after JOIN ... ON is one more FROM item. * fix(plugin): no false heavy alert from the PostgreSQL load walker Codex's cross-read reproduced alerts main never raised on statements it could read, which matter because a heavy read holds Auto mode and can be auto-denied: - a constant compared with itself (1::integer = 1::integer, 1.0 = 1.00) is no WHERE again; two different untyped constants of one kind read nothing, while a cast or two kinds prove nothing - an aliased join is read as one source, a block of its own like a derived table, instead of flattened under names its alias hides - each side of a set operation keeps its own ORDER BY and LIMIT Also bounds set-operation chains by the walk's depth and block limits, and reads FETCH ... WITH TIES as no cap. * fix(plugin): bound nested derived blocks and drop string contradictions From Codex's second cross-read of the load walker: - a block a filter bounds now bounds every block read through it, so a filter on an outer alias reaches a nested aliased join or derived table, on every engine - an aliased join enters the walk's depth and block limits - only two untyped numbers or two booleans that differ read nothing: PostgreSQL types a string as the other side ('true' = true holds), and a collation may equate two spellings * fix(plugin): let an outer filter bound only what it reaches The transitive bound from 63e4498 hid full reads: filtering the rows of (SELECT * FROM r) y also silenced the count(*) r had to finish first. - each block says whether a filter on its rows reaches what it reads (passes): not past an aggregate, a window, GROUP BY, DISTINCT, its own LIMIT, or a write; both the direct bound and its propagation require it, which also lifts main's older silence on (SELECT count(*) FROM t) y WHERE y.n = 1 - a bound goes on through a block only when its sources form one joined group - on PostgreSQL a derived table or CTE exposes only its own-level blocks, not its sublinks', and a CTE PostgreSQL does not inline (MATERIALIZED, recursive, or used twice unless NOT MATERIALIZED) exposes none * fix(plugin): carry each kind of bound only where PostgreSQL does One flag could not tell the three ways an enclosing query bounds what a block reads, so 50847af alerted on bounded reads and hid unbounded ones: - a filter passes a sort but no aggregate, window, GROUP BY, DISTINCT or own LIMIT (filterable) - a small enclosing LIMIT passes a plain inner LIMIT but no sort, unless an index leads on the order, which the catalog facts decide as for an ordered read (streams, ordered) - an empty enclosing query runs nothing below it, writes aside A set operation hands its sides its own LIMIT, deduplication and sort. * fix(plugin): let a small LIMIT stop early through any join The one-group rule kept a filter on one side of a cross product from bounding the other, but it also stopped an enclosing LIMIT, so a LIMIT over a cross product of two plain derived inputs reported both tables read in full. A LIMIT now passes any join, as the preview rule already held for direct tables. Two narrower false alerts are left as noted limits: an index order is not followed below the block that sorts, and a CTE used twice stays read even when every use is empty. * fix(plugin): let a UNION ALL's own small LIMIT stop its sides The walker read a set operation's LIMIT only as a barrier to filters on its rows. Without a sort or duplicates to remove, that LIMIT also stops each side early, as main's preview rule held, so "... UNION ALL SELECT id FROM b LIMIT 10" no longer reports b read in full; a UNION or an ordered operation still does. * docs(evals): record the shared-read-3 live pass and its paired rerun * build(plugin): load the bundled grammar through libpg-query 18.2.2 Upstream released the option Gatekeeper patched in: loadModule({ wasm }) (constructive-io/libpg-query-node#193, which superseded our #191 and credits the operator) builds every instance, reloads after a WASM exception included, from the bytes it is given. 18.2.2 ships the same libpg-query.wasm and Emscripten loader as 18.2.1; only the wrapper changed. The local patch and its patchedDependencies entry go. The plugin hands the bundled bytes to loadModule, and the guard test now checks that libpg-query compiles the bytes it is given and that bad bytes leave the loaded parser intact. Unlike the patch, 18.2.2 still starts its default load at import: in Beekeeper that fetch fails and logs to the console before the bundled bytes replace it. The operator allowed 18.2.2 before the end of its quarantine (2026-11-05 19:45 UTC) through an exception scoped to this version; the main session has the cleanup. * refactor(plugin): trace PostgreSQL output with its own grammar The reviewer's flags on a PostgreSQL connection now come from libpg_query's tree, which ends the stopgap of the merge commit. - output-model.ts holds what both walkers share: slots, relations, documents, column lookup, joins and how a write fills a column. nsp/output.ts and pg/output.ts each read their own tree into it. - pg/output.ts compares names exactly, reads Alias.colnames, joins as PostgreSQL scopes them (NATURAL from known columns, an aliased join hiding its tables, USING aliases), reads PG 18 RETURNING old/new names, MERGE, multiple assignment and SQL/JSON, and names an unaliased output as FigureColname does. - Model fixes for every engine: CTEs differing only by case count as both where names compare without case; a partial write or a MERGE action keeps the column's old value; a name no known column has is untraced; a write without RETURNING still records what it reads. Tests that encoded node-sql-parser's gaps now hold PostgreSQL's behaviour, and new cases cover each fix. SECURITY.md records the known limit left: a key read inside a part of a document is not attached to that part when the statement returns it. * test(plugin): pin the load check's two documented false alerts A small LIMIT over an index-served sort, with a subquery that only copies the table in between, and a CTE used twice whose every use is empty both alert as a full read where PostgreSQL may read little. The operator chose to keep them as documented limits: the tests make lifting either a deliberate change, and SECURITY.md lists them next to the opposite limit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Some hosts can't fetch or read the package's own
libpg-query.wasm, for example an Electron custom protocol registered withoutsupportFetchAPI(Beekeeper Studio plugins) or a bundle that inlines its assets. Until now those hosts couldn't load the parser at all, becauseloadModule()ignored its arguments. With this change they hand the module over once:provided. Each later instance is created from it through Emscripten'sinstantiateWasm, using asyncWebAssembly.instantiateand no fetch. That includes the replacement instance after a failed call (createModule()ininstantiate()).wasm.parseSyncand the stale-instance recovery all behave as before.loadModule.lengthstays0, via a rest parameter.templates/index.ts(v13–17) andtemplates/full/index.ts(v18), and the generatedversions/*/src/index.tsfiles were regenerated withcopy:templates.Builds on the approach in #191 by @Gldywn, who is credited as co-author.
Tests
versions/*/test/load-module.test.jsruns the wrapper from a copy that has nolibpg-query.wasmnext to it:main: 3 of 6 fail.Every version's WASM was built locally and each full suite passed with 0 failures: v18 149/149, v13–17 78 pass / 20 skipped (the full-API tests, which are skipped in the slim builds).
I also checked it in Electron 37 on a
plugin://scheme withoutsupportFetchAPI. A parse succeeded, a deeply nested query threw, and the next parse succeeded again:{"ok":true,"first":1,"threw":true,"afterRecovery":1}.Link to Devin session: https://app.devin.ai/sessions/9bbf3e7a34e9477ea812446ccf8a5fb0
Open in Devin Desktop: https://app.devin.ai/desktop/session/9bbf3e7a34e9477ea812446ccf8a5fb0?variant=devin
Requested by: @pyramation