Skip to content

fix(data): gate unknown fields on the explicit filter axes (#7534) - #7587

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-7534-explicit-filter-field-gate
Aug 11, 2026
Merged

fix(data): gate unknown fields on the explicit filter axes (#7534)#7587
os-zhuang merged 2 commits into
mainfrom
claude/issue-7534-explicit-filter-field-gate

Conversation

@claude

@claudeclaudeBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes#7534

What was wrong

POST /api/v1/data/:object/query with {"where":{"not_a_field":"x"}} answered 200 {"records":[],"total":0} — no code, no mention of the unknown name — and identically through the $filter door and the filter-AST door. The bare-key door, on the same object and the same field name, answered 400 INVALID_FIELD.

One endpoint family, two verdicts for one mistake, chosen by which door the caller used — and the losing verdict is indistinguishable from "no data".

This is not a regression of #4134 — verified at the branch point

Measured on the branch point (9051802) before writing any fix, all four doors on one object and one field name:

CONTROL bare-key => REJECTED status=400 code=INVALID_FIELD field=not_a_field
DOOR where-object => RESOLVED total=0 records=0
DOOR $filter-string => RESOLVED total=0 records=0
DOOR filter-AST => RESOLVED total=0 records=0
DOOR AST single-node=> RESOLVED total=0 records=0
DOOR where $and => RESOLVED total=0 records=0
SANITY real field => RESOLVED total=2 records=2

The control holds, so #4134's gate is intact on the door it covers. This is the sibling door its fix never reached: assertQueryParamsAreFields gated only the implicit filters findData derives from leftover query params, while the explicit axes reached the driver ungated — even though resolveQueryFields documents itself as "ONE resolution shared by all four read axes".

What landed

assertFilterFieldsExist calls the existing resolveQueryFields on the normalized where. resolveQueryFields itself is unchanged — the call is purely additive, per the serialization constraint with #7532 on the projection axis.

One call covers all three doors because they are not three code paths: where / filter / filters / $filter resolve to one slot at the #3795 fold, and a filter AST is lowered by parseFilterAST — the single sink for that sugar — before the gate runs. The gate therefore reads the same FilterCondition the driver will read.

Rejections carry the envelope the write path and the bare-key door already produce — 400 INVALID_FIELD + field + fields + object — plus param naming the caller's own wire spelling ($filter, not where), and a message stating the zero-row consequence.

File surface

FileChange
packages/metadata-protocol/src/protocol.tsFILTER_LOGICAL_KEYS (+1 const) and collectFilterFieldKeys (module scope, +~30 LoC) — walk a FilterCondition for field-naming keys
packages/metadata-protocol/src/protocol.tsassertFilterFieldsExist (new private method, beside its #4134 sibling) — the gate, calling the unchangedresolveQueryFields
packages/metadata-protocol/src/protocol.tsfindDataone added call, after the #4134 param gate, before the #4164 merge
packages/objectql/src/protocol-explicit-filter-field-gate.test.tsNew, 22 tests (11 fix-dependent, 11 GUARD)
.changeset/rest-list-explicit-filter-unknown-field.mdpatch → @objectstack/metadata-protocol

No other file changed. resolveQueryFields, assertQueryParamsAreFields and assertProjectionFieldsExist are byte-identical to main.

Ordering, deliberately unmoved

Reach, deliberately bounded

Reverse verification

Predictions were written before running. The revert was performed by neutralising the gate body (not the call site — removing the call fails the build with TS6133, which would have made the whole revert run read against a stale dist; the first attempt did exactly that and its greens were discarded).

TestPredictedObserved on revert
where object door refuses unknown fieldREDRED
$filter string door refuses unknown fieldREDRED
filter AST door refuses unknown fieldREDRED
AST single-comparison + and-group spellingsREDRED
unknown field under nested $and/$or/$notREDRED
unknown field carrying an operator bagREDRED
all four doors give ONE verdictREDRED
param names caller's own wire spellingREDRED
zero-row consequence + typo suggestionREDRED
first unknown reported, rest disclosedREDRED
dotted path on UNKNOWN head refusedREDRED ✓ (see miss below)
GUARD baseline — no filter returns every rowgreen bothgreen
GUARD every explicit door applies a REAL filtergreen bothgreen
GUARD bare-key control untouched (#4134)green bothgreen
GUARD real field matching nothing = honest 200/0green bothgreen
GUARD registry-injected system fields filterablegreen bothgreen
GUARD dotted path on REAL head passes throughgreen bothgreen
GUARD nested-relation value not descended intogreen bothgreen
GUARD unknown object stays 404green bothgreen
GUARD unrunnable filter still INVALID_FILTERgreen bothgreen
GUARD#4164 composition unchangedgreen bothgreen
GUARD param gate reports first when both wronggreen bothgreen

Missed prediction (disclosed). One test — the dotted-path case — was labelled GUARD and went RED on revert. Cause: I bundled two assertions into one case, a genuinely both-directions-green half (owner_id.name passes through) and a fix-dependent half (not_a_field.name → 400). The label was wrong, not the test. Split into two cases with the correct labels; the table above reflects the corrected split, and the re-run confirms 11 RED / 11 green.

Fixture bug caught by a failed green prediction.GUARD every explicit door applies a REAL filter failed on the first run. The door table built its "real field" variant by JSON.stringify(…).replace(/"x"/g, …), which silently misses on the $filter door because that door nests JSON inside JSON and its quotes are escaped. Per the working rule, I suspected the fixture rather than the gate — the door table now takes the value as a parameter instead of patching it in by string surgery, and the reason is written into the file so it is not reintroduced.

Anti-vacuous. The positive identity is pinned first: every door is shown to apply a real filter and return the two expected rows by id before any refusal is asserted. The four-door agreement test builds an explicit verdict list and asserts toEqual on all four strings, so a door answering something new — including a different wrong answer — fails. Both loop-based assertions carry a toHaveLength scale guard.

Consumption radius

Grepped every findData caller repo-wide and ran the suites, since grep alone misses fragment- and regex-shaped pins.

SuiteResult
@objectstack/metadata-protocol1051 passed (71 files)
@objectstack/objectql3175 passed (179 files)
@objectstack/rest1341 passed (82 files)
@objectstack/runtime1976 passed (122 files)
@objectstack/client279 passed (21 files)
turbo run typecheck (both changed packages)15 tasks, clean
eslint --no-inline-config on changed filesclean
full-monorepo turbo run test --concurrency=3see below

One downstream behaviour change, disclosed

import-runner.ts has two speculative findData probes:

  • resolveRef probes candidate display fields (name, title, label, …) that may not exist on the target object. It already wraps them in catch { /* field absent on target object — try the next candidate */ }, so a 400 lands exactly where the 200/0 did. No change.
  • findExisting filters by the caller's configured matchFields. A matchField naming a nonexistent field previously returned 'none' and silently degraded an upsert into an insert; it now produces a per-row failed result carrying INVALID_FIELD (the row loop's own try/catch contains it). This is contained and, I'd argue, the correct direction — but it is a behaviour change and no test pinned either way.

Not owed

No spec migration entry: #4134 / #4226 / #4254 — the sibling gates at this same REST-ingress layer — have none, and the one semantic entry in this family (#7095) exists because it changed the engine code-path API, which this does not. content/docs/releases/ untouched.

What I did not measure

  • No live showcase boot. The issue's repro boots showcase on a fresh file DB (SqlDriver / better-sqlite3) over HTTP. I measured at the findData ingress with a real ObjectQL engine and registry — the layer the gate lives at, and the layer all four doors converge on — not through an HTTP socket. The @objectstack/rest suite covers the route→mapDataError→envelope leg, but I did not personally observe 400 INVALID_FIELD on the wire against the showcase invoice object.
  • Other drivers. The gate is driver-independent by construction (it runs before engine.find), but I exercised it only against the in-suite stub driver and whatever the package suites use.
  • $nor and other unrecognised combinators are documented as an intentional hole, not tested as one.

Generated by Claude Code

`POST /data/:object/query` with `{"where":{"not_a_field":"x"}}` answered
`200 {records:[],total:0}` — and identically through the `$filter` door and
the filter-AST door — while the bare-key door on the same object and the same
field name answered `400 INVALID_FIELD`. One endpoint family, two verdicts for
one mistake, and the losing one is indistinguishable from "no data".
Not a regression of #4134: the bare-key control still passes at the branch
point (measured alongside the three failures). It is the sibling door that
fix never reached — `assertQueryParamsAreFields` gated only the implicit
filters derived from leftover query params, while the explicit axes reached
the driver ungated.
`assertFilterFieldsExist` calls the existing `resolveQueryFields` — additively;
that shared helper is unchanged — on the normalized `where`. One call covers
all three doors because they fold to one slot (#3795) and the AST is lowered
by `parseFilterAST` before the gate runs, so it reads the same
`FilterCondition` the driver reads.
Ordering is deliberately unmoved: after the #4134 param gate (so existing
precedence holds) and before the #4164 merge (so the rejection can name the
axis the caller used).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NxE7c6qf7Bi9ZQ7HtYrNUj
@vercel

vercelBot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 11, 2026 6:51am

Request Review

@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/metadata-protocol)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/metadata-protocol)
  • content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/metadata-protocol)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/v9.mdx(via @objectstack/metadata-protocol)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

… gate
The gate sits at the `findData` ingress, so it also reaches the record-matching
lookup the import runner performs for `update` / `upsert` writes. That is a
user-visible behaviour change on a second surface, and someone who hits it must
be able to find out why from the release notes rather than from a support
thread.
Records the old behaviour (a `matchField` naming no field silently degraded an
upsert into an insert, returning `'none'`), the new one (that row fails with
`400 INVALID_FIELD`, contained by the row loop's own try/catch so the rest of
the import proceeds), the remedy, and that `resolveRef`'s speculative probes are
unaffected because they already catch the absent-field case deliberately.
Ruled on #7534: the failure stays. Exempting the import path would have meant
ADDING code to preserve a silent data-correctness bug of the same family this
change closes. Changeset only — no code changed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NxE7c6qf7Bi9ZQ7HtYrNUj
@os-zhuang
os-zhuang marked this pull request as ready for review August 11, 2026 07:02
@os-zhuang
os-zhuang added this pull request to the merge queueAug 11, 2026
Merged via the queue into main with commit 7f1d4d0Aug 11, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-7534-explicit-filter-field-gate branch August 11, 2026 07:19
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-zhuang@claude