Skip to content

ObjectQL silently drops unsupported predicate keys; findOne then returns the first row #4419

Description

@yinlianghui

Package: @objectstack/objectql Version observed: 16.1.0
Found while: auditing L2 hook reads in objectstack-ai/hotcrm
App-side workaround: objectstack-ai/hotcrm#573 (does not patch the kernel)

Summary

A read query carrying an unrecognised predicate key is not rejected and not
reported. The key is dropped and the query executes without a predicate.

On findOne the consequences are severe, because the engine also applies
limit: 1. The caller does not get an error, and does not get null. They get
the object's first row — a real, plausible-looking record that has nothing
to do with what they asked for.

This is the worst available failure mode. A throw would be caught in
development; a null would be caught by the null-check that essentially every
call site already has. Returning a valid-looking wrong record defeats both, and
propagates into whatever the caller computes next.

Reproduction

Standalone, no app metadata required. Real engine, real ScopedContext — the
object the kernel injects as ctx.api in an L2 hook.

import { ObjectQL } from '@objectstack/objectql';
import { InMemoryDriver } from '@objectstack/driver-memory';

const ql = await ObjectQL.create({
  datasources: { default: new InMemoryDriver({ persistence: false }) },
  objects: {
    crm_account: {
      name: 'crm_account',
      fields: { id: { type: 'text' }, name: { type: 'text' }, annual_revenue: { type: 'number' } },
    },
  },
});

const api = ql.createContext({ isSystem: true });
const o = () => api.object('crm_account');

const one = await o().insert({ name: 'One',   annual_revenue: 100 });
const two = await o().insert({ name: 'Two',   annual_revenue: 200 });
await o().insert({ name: 'Three', annual_revenue: 200 });

await o().findOne({ where:  { id: two.id } });   // -> Two   ✅
await o().findOne({ filter: { id: two.id } });   // -> One   ❌ first row

await o().findOne({ where:  { id: 'nope' } });   // -> null  ✅
await o().findOne({ filter: { id: 'nope' } });   // -> One   ❌ a record, not null

await o().count({ where:  { annual_revenue: 100 } });  // -> 1  ✅
await o().count({ filter: { annual_revenue: 100 } });  // -> 3  ❌ whole object

await o().find({ where:  { annual_revenue: 200 } });   // -> [Two, Three]  ✅
await o().find({ filter: { annual_revenue: 200 } });   // -> [Two, Three]  ✅ aliased

Note: persistence: false matters when reproducing. InMemoryDriver defaults
to persistence: 'auto', which in Node writes .objectstack/data/memory-driver.json
and carries rows across processes — so "the first row" silently becomes a row
from an earlier run.

Root cause — the three read paths disagree

From @objectstack/objectql@16.1.0, dist/index.js:

find (~line 3952) — aliases, so it works:

const ast = { object, ...query };
delete ast.context;
if (ast.filter != null && ast.where == null) {
  ast.where = ast.filter;
}
delete ast.filter;

findOne (~line 4039) — no alias, and limit: 1:

const ast = { object: objectName, ...query, limit: 1 };
delete ast.context;
delete ast.top;

filter is spread into the AST as an unknown key, the driver ignores it, and
what remains is an unpredicated query truncated to one row.

count (~line 4563) — reads where explicitly:

ast: { object, where: query?.where },

Anything not named where is dropped, so the call counts the whole object.

The inconsistency is what makes this expensive in practice. filter appears
to be supported, because the path most people try first (find) accepts it.
The failure only shows up on the paths where being wrong is silent.

Why it stayed hidden in our codebase

Worth flagging, because the same shape will hide it in any consumer:

  1. find working made filter look like a legitimate synonym, so it spread by
    copy-paste — eighteen call sites across eight files in our app.
  2. Our in-memory test harness resolved its predicate as
    q.filter ?? q.where ?? {}, accepting both spellings. A stand-in more
    permissive than the real engine turns a whole test suite into a green light
    for broken code. This is a general hazard of hand-written kernel fakes.
  3. Nothing in the runtime logs anything. No warning, no debug line, nothing to
    grep for after the fact.

The documentation points the wrong way

ObjectQL.createContext's own doc comment demonstrates the broken spelling:

/**
 * Usage:
 *   const ctx = engine.createContext({ userId: '...', tenantId: '...' });
 *   const users = ctx.object('user');
 *   await users.find({ filter: { status: 'active' } });   // <-- here
 */

It happens to be a find, so the example itself works — which is precisely how
a reader concludes filter is the API and reaches for it on findOne. This
should be corrected regardless of which fix below is chosen.

Suggested fixes, in preference order

  1. Reject unknown query keys. Validate the read-query shape and throw on
    anything unrecognised. Turns a silent wrong answer into an immediate, local,
    obvious failure. Best outcome; a breaking change for anyone currently
    relying on the find alias, so it likely wants a major or a deprecation
    window.

  2. If a key must be tolerated, tolerate it consistently. Hoist the
    filterwhere normalization out of find into one shared query
    normalizer used by find, findOne, count, and aggregate. Removes the
    footgun without breaking existing callers. Please also emit a deprecation
    warning so the alias can eventually be dropped.

  3. Independently, make findOne fail safe. Even with (1) or (2), a
    findOne whose predicate resolved to empty is nearly always a caller bug
    rather than a genuine "give me any row" request. Consider warning — or at
    minimum documenting loudly — when findOne executes with no predicate. The
    limit: 1-over-no-predicate combination is what converts a dropped key into
    a confidently wrong record.

  4. Fix the createContext doc comment to use where.

Impact we observed downstream

For calibration on how this lands in a real app — all from a single wrong key,
none of it surfaced by tests or logs:

  • Quote and opportunity line items defaulted list_price/unit_price from the
    first product in the catalog rather than the selected one — wrong monetary
    values written to records, then rolled up into deal amounts and quote totals.
  • Delete guards built on count counted entire objects, blocking deletes that
    had no real references and reporting fabricated reference counts to users.
  • Campaign ROI snapshots recorded whole-object opportunity and lead counts as
    per-campaign attribution.
  • Stage/status gates (is this deal already closed?, is this account already a customer?) were evaluated against an unrelated record, while the subsequent
    write correctly targeted the intended id — so the guard protected the wrong
    row.

Relationship to the #2737 hole referenced in-source

dist/index.js carries this comment on aggregate:

On the opCtx so middleware-injected read filters (RLS/sharing) land in ast.where — same #2737 hole as count(): a locally-built AST aggregated over rows the caller may not see.

That is adjacent but distinct. #2737 (as described there) is about middleware-injected read filters not reaching a locally-built AST — a visibility problem. This report is about a caller-supplied predicate being dropped because the three read paths disagree on the key name. Both live in the same locally-built-AST area of count/aggregate, so a single shared query normalizer would plausibly address both — flagging it so this is not triaged as a duplicate.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions