From 1f1fe5bf351ad6fbf9709230b808aaa3c4c1ade9 Mon Sep 17 00:00:00 2001 From: Christos Gkoros Date: Tue, 15 Sep 2026 19:52:30 +0300 Subject: [PATCH 1/3] State the two requirements the specification relied on and never made Writing the security considerations for the IANA media type registration surfaced these. RFC 6838 section 4.6 makes that section mandatory, and the honest answer required writing, in a document that cites SPEC.md as the published specification, that the specification does not address them. Injection first. The premise of this project is that a JSON predicate compiles to a backend query, and every operand in it came from the caller -- so the one obligation that decides whether an implementation is safe or catastrophic was absent from the normative text, while the position existed everywhere else: SECURITY.md lists an injection defect in the filter-to-SQL compiler as in scope, and that compiler has bound every operand from the first commit. Section 7 now requires operands to be bound or escaped by the backend's own facility, never concatenated. Field paths get the asymmetric rule, because an identifier usually cannot be bound. A path MUST be resolved against the exposed field set and anything unrecognised rejected with unknown-field, rather than escaped and passed through. Refusing an unknown path is a whitelist; escaping one is a guess about the backend's quoting rules, and that is where identifier injection lives. Then duplicate member names. RFC 8259 says names SHOULD be unique and leaves the outcome undefined otherwise. That is tolerable for a document and not for a predicate, because this design deliberately validates with a JSON Schema and evaluates somewhere else -- two parsers see the filter, and where they resolve {"$gt": 18, "$gt": 65} differently the filter that was approved is not the filter that ran, with neither component having malfunctioned. For a predicate carrying tenancy that is an authorization bypass assembled out of conforming parts. New section 2.3 requires rejection with malformed-query rather than a precedence rule: a rule cannot make a third-party parser obey it, and a filter whose meaning depends on a behaviour the specification cannot enforce should not be accepted. The subsection also records that the section 7 limits bound evaluation rather than parsing. The grammar is unchanged: no operator moves, the schema is byte-identical and the $id still names v0.4.0. Both requirements constrain implementations rather than the grammar, which is the reasoning that left the $id alone through 0.5.0 and 0.6.0. The injection half is made checkable. Five hostile operands, each carrying a unique sentinel, go through every operator family that takes a string operand in both dialects, and each must either be refused with a QueryProblem or be bound -- never interpolated, never silently dropped. Refusal is the stronger answer, and $like legitimately refuses a lone backslash-quote. Verified by mutation: making the compiler interpolate strings fails all three tests. The duplicate-member half gains no test, because JSON Schema constrains a parsed instance and by then the duplication has collapsed -- which is the reason it has to be normative prose at the parse boundary, and is recorded as such. Co-Authored-By: Claude --- CHANGELOG.md | 55 +++++++ SPEC.md | 43 ++++++ ...ompilation-safety-and-duplicate-members.md | 140 ++++++++++++++++++ experiments/filter-to-sql/compile.test.mjs | 84 +++++++++++ 4 files changed, 322 insertions(+) create mode 100644 decisions/0002-compilation-safety-and-duplicate-members.md diff --git a/CHANGELOG.md b/CHANGELOG.md index beff949..19de636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,61 @@ minor release may break compatibility, in which case the break is spelled out be ## [Unreleased] +### Changed + +- **Operands MUST be bound, and duplicate member names MUST be rejected.** Two normative + requirements the specification relied on and never stated, argued in + [`decisions/0002`](https://github.com/OpenPredicate/open-predicate/blob/main/decisions/0002-compilation-safety-and-duplicate-members.md). + Both were surfaced by writing the security considerations for the IANA media type registration, + which RFC 6838 §4.6 makes mandatory — the honest answer required stating, in a document that cites + `SPEC.md` as the published specification, that the specification did not address them. + + [SPEC.md §7](./SPEC.md#7-safety-limits) now says that operands are caller-supplied and MUST be + passed to a backend as bound parameters or through its own escaping facility, and MUST NOT be + concatenated into a statement. The premise of this project is that a JSON predicate compiles to a + backend query, so this is the requirement that decides whether an implementation is safe — and it + appeared nowhere in the normative text. The project's position already existed everywhere else: + `SECURITY.md` lists an injection defect in the filter-to-SQL compiler as in scope, and + `experiments/filter-to-sql` has bound every operand from the start. + + The same section adds the asymmetric rule for field paths, which usually cannot be bound as + parameters: a path MUST be resolved against the exposed field set (§3.5) and anything + unrecognised rejected with `unknown-field`, rather than escaped and passed through. Refusing an + unknown path is a whitelist; escaping one is a guess about the backend's quoting rules. + + [SPEC.md §2.3](./SPEC.md#23-the-filter-document) is new and settles duplicate member names. + RFC 8259 §4 says names SHOULD be unique and leaves the outcome undefined otherwise, which is + tolerable for a document and not for a predicate: this design deliberately validates with a JSON + Schema and evaluates somewhere else, so two parsers see the filter. Where they resolve + `{"$gt": 18, "$gt": 65}` differently, the filter that was approved is not the filter that ran and + neither component has malfunctioned. An implementation MUST now reject a duplicate member name + with `malformed-query` rather than prefer an occurrence. The subsection also records that the §7 + limits bound evaluation rather than parsing, so input must be bounded independently — a parser + exhausted by depth has failed before any limit applies. + + **Migration.** Nothing changes for a caller except one case that never had a defined meaning: a + filter containing a duplicate member name was previously accepted according to whichever reading + the implementation's parser happened to take, and is now rejected. For an implementer, both are + no-ops if operands are already bound and the JSON parser already rejects duplicates; otherwise the + first describes a vulnerability rather than introducing one, and the second needs a check at the + read boundary. `JSON.parse` cannot report duplicates, so this binds the parser, not the validator + — which is also why no fixture can express it, as the record explains. + + **The grammar is unchanged.** No operator is added, removed or altered, the schema is + byte-identical and the `$id` still names `v0.4.0`. Both requirements constrain implementations + rather than the grammar, and §9 ties the `$id` to the grammar — the same reasoning that left the + `$id` alone through 0.5.0 and 0.6.0. + +### Added + +- **The injection requirement is checkable.** + `experiments/filter-to-sql/compile.test.mjs` drives five hostile operands, each carrying a unique + sentinel, through every operator family that takes a string operand, in both dialects. For each it + requires one of exactly two outcomes: the operand is refused with a `QueryProblem`, or it is bound + — never interpolated, and never silently dropped, since a dropped predicate widens the result set. + Refusal counts as the stronger answer: `$like` legitimately rejects a lone `\'` as an invalid + escape. Verified by mutation — making the compiler interpolate strings fails all three tests. + ## [0.6.2] — 2026-09-14 **A packaging release, again.** The schema is still byte-identical and the `$id` still names diff --git a/SPEC.md b/SPEC.md index a2006c2..5043677 100644 --- a/SPEC.md +++ b/SPEC.md @@ -100,6 +100,36 @@ Each member of `fields` describes one queryable path: `operators` tells a client what it may write; `type`, `format` and `values` tell it *what to write*. The grammar cannot carry that second half: field names are constrained through `propertyNames`, but every path shares one `Constraint` definition, so per-field operand domains are not expressible in the schema (§6). A filter naming a real field with a value outside that field's domain is therefore well-formed, and matches nothing — the failure surfaces as an empty result set rather than an error (§8). The capability document is the only place the domain can be stated, which is why `values` is RECOMMENDED wherever the domain is closed. +### 2.3 The filter document + +A filter arrives as JSON. Two properties of JSON itself decide what an implementation can safely +promise about it, and both have to be settled before the filter is validated rather than after. + +**Duplicate member names MUST be rejected.** [RFC 8259](https://www.rfc-editor.org/rfc/rfc8259) §4 +says names SHOULD be unique and leaves the outcome undefined when they are not: parsers variously +keep the first, keep the last, or retain both. That is tolerable for a document and not for a +predicate, because a filter is validated by one component and evaluated by another. Given + +```json +{ "age": { "$gt": 18, "$gt": 65 } } +``` + +a validator that keeps the first member approves a filter an evaluator that keeps the last then +executes, and no layer has malfunctioned. An implementation MUST reject an object containing a +duplicate member name with `malformed-query` (§8), and MUST NOT resolve the duplication by +preferring one occurrence. + +This cannot be expressed in the schema. JSON Schema constrains a parsed instance, and by then the +duplication has already collapsed — so the requirement binds the parser, not the validator. An +implementation whose JSON parser cannot report duplicates needs to detect them while reading the +document, or replace the parser. + +**Validation precedes evaluation, and parsing precedes both.** The limits of §7 bound what an +implementation will evaluate; they do not bound what it will parse, and a parser exhausted by +nesting depth has already failed before any of them applies. An implementation MUST therefore bound +the input independently of the grammar — by request body size at least, which §7 RECOMMENDS +anyway — and reject anything it cannot parse within those bounds with `malformed-query`. + ## 3. Data model and field paths ### 3.1 Records @@ -459,6 +489,19 @@ For `$regex`, a linear-time engine (RE2, Rust `regex`, Go `regexp`) is strongly Fields backed by unindexed storage are their own denial-of-service surface. An implementation SHOULD restrict expensive operators to indexed fields through its capability document rather than accepting them and timing out. +**Operands are attacker-controlled and MUST NOT be interpolated into a backend query.** A filter +compiles to a query, and every operand in it came from the caller. An implementation MUST pass +operands to its backend as bound parameters, or otherwise through the backend's own escaping +facility; it MUST NOT build a query by concatenating operand values into a statement. This is the +one safety requirement that no limit in this section mitigates: a bound operand of any length is +inert, and an interpolated one of any length is not. + +Field paths are the harder half, because an identifier usually cannot be bound as a parameter. A +path is caller-supplied too, so an implementation MUST resolve each path against the field set it +exposes (§3.5) and reject anything unrecognised with `unknown-field` (§8) — rather than escaping a +path and passing it through. Refusing an unknown path is a whitelist; escaping one is a guess about +the backend's quoting rules. + `$some` and `$every` are the expensive operators on most backends: each one is a traversal of an array, and a nested quantifier is a traversal per element. They count toward the nesting-depth and clause limits like any other clause, and — unlike the `[*]` path segment they replaced — a server that cannot afford them can decline the whole `collections` profile (§2.1) instead of having to accept them as part of the path grammar. An implementation SHOULD publish a lower `maxDepth` for filters containing quantifiers if its storage makes them disproportionately costly. ## 8. Errors diff --git a/decisions/0002-compilation-safety-and-duplicate-members.md b/decisions/0002-compilation-safety-and-duplicate-members.md new file mode 100644 index 0000000..16b8861 --- /dev/null +++ b/decisions/0002-compilation-safety-and-duplicate-members.md @@ -0,0 +1,140 @@ +# 0002 — Compilation safety, and duplicate member names + +**Status:** proposed · **Affects:** SPEC.md §2.3 (new), §7 · **Grammar version:** unchanged + +## Context + +Writing the IANA registration for `application/vnd.openpredicate+json` surfaced two requirements the +specification relies on and never states. RFC 6838 §4.6 makes a security considerations section +mandatory and requires, among other things, that a registration referencing an existing format +describe the issues specific to using *that* format. Answering it honestly meant writing, in a +document that points at `SPEC.md` as the published specification, that the specification does not +address the thing every implementation has to get right. + +Two gaps, from opposite ends of the pipeline. + +**The specification never mentions injection.** The premise of this project is that a JSON-encoded +predicate compiles to a backend query. Every operand in it is caller-supplied. `SPEC.md` contains no +occurrence of *injection*, *bound*, *parameter* in that sense, or *escape* outside the `$like` +grammar — so the one obligation that decides whether an implementation is safe or catastrophic is +absent from the normative text. The project's position exists everywhere else: `SECURITY.md` lists +"an injection or escaping defect in the filter-to-SQL compiler" as in scope for a security report, +and `experiments/filter-to-sql` binds every operand and has done from the first commit. Only the +specification is silent, which is the one place an independent implementer reads. + +**Duplicate member names are undefined, and this architecture is unusually exposed to that.** RFC +8259 §4 says object member names SHOULD be unique and declines to define the outcome when they are +not. The specification inherits that silence. It matters here more than it would elsewhere, because +the design deliberately separates validation from evaluation: a JSON Schema approves the filter, and +something else executes it. Those are two parsers. Where they resolve `{"$gt": 18, "$gt": 65}` +differently, the filter that was approved is not the filter that ran, and neither component has +malfunctioned. For a predicate that carries tenancy or visibility, that is an authorization bypass +assembled entirely out of conforming parts. + +### What this record does not do + +It does not add, remove or change an operator, and it does not touch the schema. It changes what a +conforming implementation must do with a filter before and after the grammar has its say. + +## Decision 1 — operands are bound, paths are whitelisted + +§7 gains two paragraphs. Operands MUST be passed to the backend as bound parameters or through its +own escaping facility, and MUST NOT be concatenated into a statement. + +The second paragraph is the one worth arguing. Field paths are caller-supplied in exactly the same +way, and an identifier generally cannot be bound as a parameter — so the symmetric rule is +unavailable and the temptation is to escape the path instead. The record rejects that. An +implementation MUST resolve each path against the field set it exposes (§3.5) and reject anything +unrecognised with `unknown-field`. Refusing an unknown path is a whitelist and is decidable from +information the implementation already has; escaping one is a guess about the backend's quoting +rules, and quoting rules are where identifier injection lives. §3.5 and §8 already provide both +halves of the whitelist, so this states a consequence rather than inventing a mechanism. + +Placing this in §7 is a small stretch of that section's title — *Safety limits* — but the section +already carries safety guidance that is not a limit: prefer a linear-time regex engine, restrict +expensive operators to indexed fields, publish a lower `maxDepth` for quantifiers. A separate +section would have been tidier and would have renumbered §8 through §10 and every cross-reference in +the README, the schema descriptions, the site and `decisions/0001`. Not worth it. + +## Decision 2 — duplicate member names are rejected, at the parser + +A new §2.3 requires an implementation to reject an object containing a duplicate member name with +`malformed-query`, and forbids resolving the duplication by preferring an occurrence. + +Rejection rather than a defined precedence rule. Defining "last wins" would make the outcome +predictable and would not fix anything: the two parsers in a conforming deployment are usually not +both ours, so a rule we state cannot make a third-party JSON parser obey it. A filter whose meaning +depends on a behaviour the specification cannot enforce should not be accepted at all. Rejection is +also the only option that fails loudly, which is the stance the rest of the document takes — §2.1 +forbids silently ignoring an unsupported operator, §7 forbids truncating an over-limit filter, and +both for the same reason: a filter that matches more than it says is the worst failure available. + +**This requirement cannot be tested through the schema, and that is the point.** JSON Schema +constrains a parsed instance; by then the duplication has collapsed and `JSON.parse` has already +chosen a winner. So the requirement binds the parser, and §2.3 says so explicitly. The test suite +cannot express it, and no fixture is added — a fixture would have to be a *document*, and +`tests/fixtures/` holds parsed instances. An implementer whose parser cannot report duplicates has +to detect them while reading or change parser, which §2.3 states. + +The same subsection records the adjacent ordering constraint, because it has the same shape and the +same cause: the §7 limits bound evaluation, not parsing, and a parser exhausted by nesting depth has +failed before any limit applies. An implementation must bound its input independently. + +## Migration + +Nothing to migrate for a caller. No filter that was valid becomes invalid, with one exception that +was never well-defined: a filter containing a duplicate member name was previously accepted by +whichever reading the implementation's parser happened to take, and is now rejected with +`malformed-query`. No such filter had a specified meaning, so none can be said to have worked. + +For an implementer, both decisions are conformance requirements that may already hold: + +- If operands are already bound, Decision 1 is a no-op. If they are interpolated, that is a + vulnerability being described rather than introduced. +- If the JSON parser rejects or reports duplicates, Decision 2 is a no-op. Several do not by + default, and the check has to be added at the read boundary. + +## Costs and risks + +- **§7's title is now slightly wrong.** Accepted, with the renumbering argument above. If §7 is ever + split, the title goes with it. +- **§2.3 asks for something some parsers make awkward.** `JSON.parse` offers no duplicate reporting; + detection means a streaming parser, a reviver-based trick, or a pre-scan. This is a real cost + imposed on implementers, and it buys the elimination of a divergence class that is otherwise + undetectable at run time. +- **The registration now cites sections that must not drift.** The IANA submission quotes §2.3 and + §7 by number. A future renumbering would strand it. +- **Neither decision is enforced by the test suite for its own sake.** Decision 1 gains a real + regression test in `experiments/filter-to-sql`; Decision 2 gains none, for the structural reason + above. A normative requirement no test can reach is weaker than one a test can, and that asymmetry + is recorded rather than hidden. + +## Open question + +**Should `malformed-query` be the condition for a duplicate member, or should there be a distinct +one?** Settled: `malformed-query`, whose §8 definition is "the body does not conform to the schema: +unknown operator, wrong operand type, structural error". A duplicate member is a structural error and +needs no new problem type; a new type would also mean a new URI under +`https://openpredicate.tech/problems/`, and five conditions that each mean something distinct are +worth more than six where two overlap. Reconsider if implementers report that clients cannot tell a +duplicate member from a schema violation and that the distinction changes what a client does about +it — the `pointer` of §8 already locates the clause either way. + +## Follow-ups, explicitly out of scope + +- Defining the media type itself in `SPEC.md`. The registration needs a normative §Media type + section with the parameters it declares; that is its own record, and this one only fixes what the + registration's security considerations exposed. +- The `profile` media type parameter, deliberately not registered for now. +- Whether §7's RECOMMENDED limits are the right numbers. Untouched here. +- Guidance on composing a caller's predicate with a server's authorization predicate. The + registration says it must be by conjunction; the specification still does not, because the + enclosing request body is out of scope per §1 and the right home is unclear. + +## Results + +`SPEC.md` gains §2.3 and two paragraphs in §7. The schema is byte-identical and the `$id` still +names `v0.4.0`: both decisions constrain implementations rather than the grammar, and §9 ties the +`$id` to the grammar — the same reasoning that left the `$id` alone through 0.5.0 and 0.6.0. +`experiments/filter-to-sql/compile.test.mjs` gains a test asserting that no operand appears +literally in emitted SQL, in both dialects, which is Decision 1 made checkable. diff --git a/experiments/filter-to-sql/compile.test.mjs b/experiments/filter-to-sql/compile.test.mjs index 671b742..092e9d2 100644 --- a/experiments/filter-to-sql/compile.test.mjs +++ b/experiments/filter-to-sql/compile.test.mjs @@ -130,3 +130,87 @@ test("both dialects compile the whole corpus", () => { } assert.deepEqual(failures, []); }); + +/** + * SPEC.md §7: operands are caller-supplied and MUST be bound, never + * interpolated. This is the one safety requirement no limit in §7 mitigates — + * a bound operand of any length is inert, an interpolated one is not — so it is + * asserted directly rather than inferred from the result sets above. + * + * Each operand is a hostile payload with a unique alphabetic sentinel appended. + * The sentinel is what the assertions track, for two reasons. It cannot collide + * with SQL the compiler legitimately generates — an earlier version of this test + * probed for "SELECT 1" and tripped over the compiler's own + * `EXISTS (SELECT 1 FROM json_each(...))`. And it contains no LIKE + * metacharacter, so it survives the rewriting the pattern operators properly do: + * $contains escapes a literal `%` to `\%` so it cannot act as a wildcard, while + * $like passes the pattern through because there `%` is a wildcard by design. + * Tracking the sentinel tells "escaped" from "dropped" without asserting any one + * operator's escaping scheme. + */ +const HOSTILE = [ + ["x' OR 1=1 --", "SENTINELONE"], + ['"; DROP TABLE pets; --', "SENTINELTWO"], + ["'||(SELECT name FROM pets LIMIT 1)||'", "SENTINELTHREE"], + ["\\'; SELECT 1; --", "SENTINELFOUR"], + ["%' OR name LIKE '%", "SENTINELFIVE"], +].map(([payload, sentinel]) => ({ value: payload + sentinel, probe: sentinel })); + +for (const [dialectName, binding] of [ + ["sqlite", HYBRID_BINDING], + ["postgres", { ...HYBRID_BINDING, dialect: "postgres" }], +]) { + test(`${dialectName}: hostile operands are bound, never interpolated (§7)`, () => { + for (const { value, probe } of HOSTILE) { + // One filter per operator family that carries a string operand, so an + // operator added later that forgets to bind is caught rather than only + // $eq being covered. + const filters = [ + { name: { $eq: value } }, + { name: { $ne: value } }, + { name: { $in: [value] } }, + { name: { $like: value } }, + { name: { $contains: value } }, + { name: { $startsWith: value } }, + { tags: { $some: { $eq: value } } }, + ]; + + for (const filter of filters) { + const label = JSON.stringify(filter); + + // Two outcomes are safe and no third is: the operand is refused, or it + // is bound. $like legitimately refuses a lone "\\'" as an invalid + // escape (§5.5), and refusing is a stronger answer than binding. + let compiled; + try { + compiled = compile(filter, binding); + } catch (err) { + assert.ok(err instanceof QueryProblem, `expected a QueryProblem for ${label}, got ${err}`); + continue; + } + + const { sql, params } = compiled; + assert.ok(!sql.includes(value), `operand was interpolated into the statement for ${label}:\n ${sql}`); + assert.ok(!sql.includes(probe), `part of the operand reached the statement for ${label}:\n ${sql}`); + + // It must also have been passed along rather than silently dropped: a + // dropped predicate widens the result set, which §2.1 and §7 forbid. + assert.ok( + params.some((v) => typeof v === "string" && v.includes(probe)), + `operand never reached the parameter list for ${label}: ${JSON.stringify(params)}`, + ); + } + } + }); +} + +test("a hostile operand matches nothing rather than changing the query (§7)", () => { + // The end-to-end form of the same claim: executed, against the real dataset. + for (const { value } of HOSTILE) { + const result = search(db, { name: { $eq: value } }, HYBRID_BINDING); + assert.ok(!result.problem, `unexpected ${result.problem?.type} for ${value}`); + assert.deepEqual(result.ids, [], `an injected operand must select no records: ${value}`); + } + // And the table is still there, which a successful injection would have settled. + assert.deepEqual(search(db, { status: "available" }, HYBRID_BINDING).problem, undefined); +}); From 306ad0081e5ee823687d91160e8a0b84964dc573 Mon Sep 17 00:00:00 2001 From: Christos Gkoros Date: Tue, 15 Sep 2026 19:54:25 +0300 Subject: [PATCH 2/3] Run the experiment suite in CI, so the injection rule is actually enforced The new section 7 requirement is checked in experiments/filter-to-sql/compile.test.mjs, and CI ran only `npm test`, which globs tests/*.test.mjs. So the test proving that operands are bound rather than interpolated was not running anywhere except locally, and a test that does not run enforces nothing. The compiler remains an exercise rather than a deliverable; that is a statement about what it is for, not a reason to leave its assertions unchecked. The suite takes about 300ms. Co-Authored-By: Claude --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2e85c04..ed9bde4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,6 +26,12 @@ jobs: - run: npm ci - run: npm test + # The filter-to-SQL compiler is an exercise rather than a deliverable, but + # its suite is where the §7 requirement that operands be bound and never + # interpolated is actually checked. A test that does not run enforces + # nothing, so it runs here. + - run: npm run test:experiment + # The 0.6.0 bin shipped as a silent no-op because nothing exercised the packed # artifact. Package managers disagree about how they link a bin — npm and yarn # symlink, pnpm writes a shim, Berry's PnP has no node_modules at all, and From f88e6586fd895cb50540696a8b938d0f39bb20fe Mon Sep 17 00:00:00 2001 From: Christos Gkoros Date: Wed, 16 Sep 2026 09:23:47 +0300 Subject: [PATCH 3/3] Let the experiment suite run on every Node the package supports The previous commit put `npm run test:experiment` in CI on all three matrix versions, and the Node 20 job failed at module load: harness.mjs imports node:sqlite, a builtin that does not exist before Node 22.5, so nothing in the file ran. package.json declares engines.node >=20.10.0, so Node 20 belongs in the matrix -- the assumption that needed stating was the test file's, not the matrix's. The harness is now loaded through a guarded dynamic import, and the four tests that need rows back from a database carry a shared skip option that is false whenever node:sqlite is present. Only harness.mjs needs the builtin; compile.mjs, cases.mjs and dataset.mjs do not. So Node 20 still runs the compiler-level half, including both section 7 tests asserting that hostile operands are bound rather than interpolated, which is why the step was added to CI in the first place. Node 22 and 24 keep executing the full corpus against the real database, which is where the evidence for the result sets comes from. The guard is narrow: it swallows ERR_UNKNOWN_BUILTIN_MODULE and rethrows everything else, so a harness that breaks for any other reason still fails loudly rather than reporting 149 tests as skipped. Verified on Node 26 (155 pass, 0 skipped) and with the probe pointed at a nonexistent builtin to simulate Node 20 (exit 0, 6 pass, 149 skipped with the reason attached). No real Node 20 was available locally. Co-Authored-By: Claude --- experiments/filter-to-sql/compile.test.mjs | 31 +++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/experiments/filter-to-sql/compile.test.mjs b/experiments/filter-to-sql/compile.test.mjs index 092e9d2..fe74f99 100644 --- a/experiments/filter-to-sql/compile.test.mjs +++ b/experiments/filter-to-sql/compile.test.mjs @@ -3,10 +3,29 @@ import assert from "node:assert/strict"; import { CASES, expectationFor, bindingsDiverge } from "./cases.mjs"; import { DOCUMENT_BINDING, HYBRID_BINDING, RECORDS } from "./dataset.mjs"; -import { openDatabase, search } from "./harness.mjs"; import { compile, parsePath, QueryProblem } from "./compile.mjs"; -const db = openDatabase(); +/** + * The database is a Node 22.5+ builtin, and this suite runs on every version + * package.json claims to support. So the harness is loaded conditionally: where + * `node:sqlite` is missing the executed half of the suite reports as skipped + * rather than failing, and the compiler-level half — including the §7 binding + * rule this suite exists to enforce — still runs everywhere. + */ +let harness = null; +let unavailable = null; +try { + await import("node:sqlite"); + harness = await import("./harness.mjs"); +} catch (error) { + if (error?.code !== "ERR_UNKNOWN_BUILTIN_MODULE") throw error; + unavailable = `node:sqlite is unavailable on ${process.version} (needs Node 22.5+)`; +} + +const search = (...args) => harness.search(...args); +const db = harness?.openDatabase() ?? null; +// `skip: false` runs the test normally, so one options object covers both cases. +const executed = { skip: unavailable ?? false }; const BINDINGS = { hybrid: HYBRID_BINDING, document: DOCUMENT_BINDING }; /** @@ -15,7 +34,7 @@ const BINDINGS = { hybrid: HYBRID_BINDING, document: DOCUMENT_BINDING }; */ for (const testCase of CASES) { for (const [name, binding] of Object.entries(BINDINGS)) { - test(`${testCase.id} ${name}: ${testCase.title}`, () => { + test(`${testCase.id} ${name}: ${testCase.title}`, executed, () => { const expected = expectationFor(testCase, name); const result = search(db, testCase.filter, binding); if (expected.problem) { @@ -36,7 +55,7 @@ for (const testCase of CASES) { * them to differ. Every disagreement is either a compiler bug or a case that * declares itself binding-dependent. */ -test("the two bindings agree except where the spec allows them not to", () => { +test("the two bindings agree except where the spec allows them not to", executed, () => { const divergent = []; for (const testCase of CASES) { const results = Object.entries(BINDINGS).map(([name, binding]) => { @@ -48,7 +67,7 @@ test("the two bindings agree except where the spec allows them not to", () => { assert.deepEqual(divergent, CASES.filter(bindingsDiverge).map((c) => c.id)); }); -test("every fixture record is reachable, and no case matches everything by accident", () => { +test("every fixture record is reachable, and no case matches everything by accident", executed, () => { const all = search(db, { id: { $exists: true } }, DOCUMENT_BINDING); assert.equal(all.ids.length, RECORDS.length); }); @@ -204,7 +223,7 @@ for (const [dialectName, binding] of [ }); } -test("a hostile operand matches nothing rather than changing the query (§7)", () => { +test("a hostile operand matches nothing rather than changing the query (§7)", executed, () => { // The end-to-end form of the same claim: executed, against the real dataset. for (const { value } of HOSTILE) { const result = search(db, { name: { $eq: value } }, HYBRID_BINDING);