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

@claude claude Bot 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

File Change
packages/metadata-protocol/src/protocol.ts FILTER_LOGICAL_KEYS (+1 const) and collectFilterFieldKeys (module scope, +~30 LoC) — walk a FilterCondition for field-naming keys
packages/metadata-protocol/src/protocol.ts assertFilterFieldsExist (new private method, beside its #4134 sibling) — the gate, calling the unchanged resolveQueryFields
packages/metadata-protocol/src/protocol.ts findDataone added call, after the #4134 param gate, before the #4164 merge
packages/objectql/src/protocol-explicit-filter-field-gate.test.ts New, 22 tests (11 fix-dependent, 11 GUARD)
.changeset/rest-list-explicit-filter-unknown-field.md patch → @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).

Test Predicted Observed on revert
where object door refuses unknown field RED RED
$filter string door refuses unknown field RED RED
filter AST door refuses unknown field RED RED
AST single-comparison + and-group spellings RED RED
unknown field under nested $and/$or/$not RED RED
unknown field carrying an operator bag RED RED
all four doors give ONE verdict RED RED
param names caller's own wire spelling RED RED
zero-row consequence + typo suggestion RED RED
first unknown reported, rest disclosed RED RED
dotted path on UNKNOWN head refused RED RED ✓ (see miss below)
GUARD baseline — no filter returns every row green both green
GUARD every explicit door applies a REAL filter green both green
GUARD bare-key control untouched (#4134) green both green
GUARD real field matching nothing = honest 200/0 green both green
GUARD registry-injected system fields filterable green both green
GUARD dotted path on REAL head passes through green both green
GUARD nested-relation value not descended into green both green
GUARD unknown object stays 404 green both green
GUARD unrunnable filter still INVALID_FILTER green both green
GUARD #4164 composition unchanged green both green
GUARD param gate reports first when both wrong green both green

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.

Suite Result
@objectstack/metadata-protocol 1051 passed (71 files)
@objectstack/objectql 3175 passed (179 files)
@objectstack/rest 1341 passed (82 files)
@objectstack/runtime 1976 passed (122 files)
@objectstack/client 279 passed (21 files)
turbo run typecheck (both changed packages) 15 tasks, clean
eslint --no-inline-config on changed files clean
full-monorepo turbo run test --concurrency=3 see 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

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 11, 2026 6:51am

Request Review

@github-actions github-actions Bot 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 queue Aug 11, 2026
Merged via the queue into main with commit 7f1d4d0 Aug 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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants