Skip to content

fix(metadata)!: a $filter array that is not a filter AST is rejected, not passed through (#4121) - #4200

Merged
os-zhuang merged 1 commit into
mainfrom
claude/protocol-reject-malformed-filter
Jul 30, 2026
Merged

fix(metadata)!: a $filter array that is not a filter AST is rejected, not passed through (#4121)#4200
os-zhuang merged 1 commit into
mainfrom
claude/protocol-reject-malformed-filter

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes #4121. Filed as item 2 of #3948error locality. The investigation found it is more than that.

It closes the last silently-unfiltered shape

#3948 made the drivers throw on a bare triple with an unknown operator, and on any element that is neither a join keyword nor a condition array. What it could not reach is a lone ['and'] / ['or']:

['and']  →  isFilterAST false   (a logical node needs length >= 2)
         →  assigned to `where` unconverted
         →  driver sets nextJoin, matches no element, emits NO predicate
         →  every row returned, silently

No driver-side check applies, because there is no offending element to reject — the array is well-formed and empty of conditions. It is the last survivor of the class #3948 closed, and it is now a 400.

For every other shape this is not a narrowing

That was the open question in #4121 — whether an opaque array where is a supported input. Traced through every driver:

Backend array where that is not an AST
driver-sql throws — applyFilters rejects any element that is neither and/or nor a condition array (#3948)
driver-memory throws (7 sites)
driver-mongodb its own parser, then a server error on the raw $${op}

So rejecting at the protocol changes which error the caller sees, not whether there is one. The gain is that the message is in the request's own vocabulary — unrecognised operator "not in", element 1 is number, plus the recognised operator list — instead of a driver's internal builder state, several layers from the query parameter that carried it.

Scoped narrowly, because the regression to fear is rejecting something valid

  • Only arrays are in scope. A where object is untouched: {status:'active'}, {$and:[…]}, {amount:{$gte:100}} all pass through exactly as before.
  • An empty [] is left alone. It means "no filter", and every path already treats it that way.
  • isFilterAST accepts nested arrays (filter.zod.ts"Legacy flat array: [[cond],[cond]]"), so [[a,'=',1],[b,'=',2]] and ['and', […], ['or', …]] keep converting. A naive "arrays are suspect" rule would have broken exactly those — which is why the accepted shapes are pinned by as many tests as the rejected ones.
  • before / after, which A filter with an operator outside VALID_AST_OPERATORS is silently dropped, not rejected — single-condition views return unfiltered results #3948 added to AST_OPERATOR_MAP, convert normally — verified, not assumed.

Errors carry status: 400 and code: 'INVALID_FILTER', matching the UNSUPPORTED_QUERY_PARAM convention immediately alongside.

Verification

  • 12 new tests driving the real findData normalisation — not a re-implementation of its rule. My first draft extracted the predicate into the test, which would have verified the rule while proving nothing about the wiring or the message; this drives ObjectStackProtocolImplementation.findData with a stub engine and asserts on the options handed to engine.find.
  • Six cover shapes that must keep converting, six the shapes that must be rejected — including exact message text and the status/code.
  • Confirmed the tests have teeth: disabling the new branch fails 6 of 12.
  • Full @objectstack/metadata-protocol suite: 122 tests across 19 files, green. tsc --noEmit: 0 errors in the touched files. eslint --no-inline-config: clean.

Refs #3948, objectstack-ai/objectui#2945

🤖 Generated with Claude Code

…4121)

isFilterAST was read as a CONVERSION gate: an array it refused was assigned
to options.where unconverted, leaving each backend to make sense of a value
the protocol had already decided it could not parse.

Filed as item 2 of #3948 — error locality. The investigation found more.

It closes the last silently-unfiltered shape. #3948 made the drivers throw
on a bare triple with an unknown operator and on any element that is neither
a join keyword nor a condition array. What it could not reach is a lone
['and'] / ['or']: the driver sets its join mode, matches no element, emits
NO predicate, and returns every row. isFilterAST refuses it (a logical node
needs length >= 2), so it arrived as an opaque `where` and no driver-side
check applied.

For every other shape this is not a narrowing: driver-sql throws,
driver-memory throws, driver-mongodb fails at the server. This changes WHICH
error the caller sees, not whether there is one — and puts the request's own
vocabulary in the message instead of a driver's builder state.

Scoped narrowly, because the regression to fear is rejecting something
valid:
- only arrays are in scope; a `where` OBJECT is untouched ($and/$or/$gte);
- an empty [] is left alone — it means "no filter";
- isFilterAST accepts nested arrays, so [[a,'=',1],[b,'=',2]] and
  ['and', […], ['or', …]] keep converting. A naive "arrays are suspect"
  rule would have broken those, so the accepted shapes are pinned by more
  tests than the rejected ones.

status 400 + code INVALID_FILTER, matching UNSUPPORTED_QUERY_PARAM alongside.

12 new tests driving the real findData normalisation rather than a
re-implementation of its rule; reverting the change fails six. Package suite
122 tests / 19 files green.

Closes #4121. Refs #3948, objectstack-ai/objectui#2945

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 30, 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 Jul 30, 2026 3:04pm

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Jul 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

1 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)

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.

@os-zhuang
os-zhuang merged commit 4475c59 into main Jul 30, 2026
17 checks passed
@os-zhuang
os-zhuang deleted the claude/protocol-reject-malformed-filter branch July 30, 2026 15:14
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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A filter array that isn't a valid AST reaches the driver as an opaque where — reject it at the protocol instead

1 participant