From 2c974ed88d45525e4d38cd1ee6ccde647937206d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 09:07:53 +0000 Subject: [PATCH] fix(app-shell): an inherited member is not a URL filter operator suffix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `parseUrlFilterTriples` resolved a `filter[][]` suffix by indexing `URL_FILTER_OPS` — a plain object literal — and testing the result for truthiness. The suffix comes from the address bar, so `Object.prototype` answered that question too: `[constructor]`, `[toString]`, `[hasOwnProperty]` each emitted a triple whose OPERATOR WAS A FUNCTION, and `[__proto__]` one whose operator was `Object.prototype` itself. The function's own contract says an unknown suffix is ignored, never silently downgraded to equality; for these it was neither. The map now has no prototype, so an own entry is the only thing a lookup can find. No denylist: that is a spelling-level patch the next member of a prototype this module does not own walks straight past. The exported face is unchanged — same name, same `Record`, same four entries, same behaviour under spread and `Object.entries`. The accompanying sweep enumerates `Object.prototype` at run time rather than listing today's members, and is paired with an assertion that the four declared operators still resolve so it cannot pass vacuously. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Xm4WFhEe5mwcgyqHjxR2hn --- .../9507-url-filter-op-prototype-chain.md | 50 +++++++++++++ .../src/views/drillUrlFilters.test.ts | 72 +++++++++++++++++++ .../app-shell/src/views/drillUrlFilters.ts | 39 +++++++++- 3 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 .changeset/9507-url-filter-op-prototype-chain.md diff --git a/.changeset/9507-url-filter-op-prototype-chain.md b/.changeset/9507-url-filter-op-prototype-chain.md new file mode 100644 index 0000000000..382f940e9e --- /dev/null +++ b/.changeset/9507-url-filter-op-prototype-chain.md @@ -0,0 +1,50 @@ +--- +'@object-ui/app-shell': patch +--- + +A URL filter operator suffix naming an inherited member is no longer an operator +(objectui#9507). + +`parseUrlFilterTriples` — the reader behind the ADR-0055 bare data surface's +`filter[][]=` URLs — decided "is this suffix an operator" by +indexing `URL_FILTER_OPS` with the suffix and testing the result for truthiness. +The map was a plain object literal and the suffix comes from the address bar, so +that question was also answered by `Object.prototype`: `filter[amount][constructor]=1` +resolved to `Object.prototype.constructor`, passed the guard, and emitted a filter +condition **whose operator was a JavaScript function**. `[toString]` and +`[hasOwnProperty]` did the same; `__proto__` was the same defect in a second shape, +its inherited accessor yielding `Object.prototype` itself, so that suffix produced +an operator that was an object. `filter[amount][nope]=1` was always correct — it +produced nothing, which is the documented behaviour this repair restores for the +inherited names: the function's own contract says an unknown operator suffix is +ignored, never silently downgraded to equality, and for these suffixes it was +neither. + +**What a reader saw before the repair**, driven once per consumer of these triples +and recorded here because none of the three was a crash: + +- the **filter-chip row** fell through the range arms of `groupFilterChips` to the + `= ` default and drew a confident `amount = 1` chip — the silent downgrade + to equality the contract rules out, rendered as if the user had asked for it; +- **"Save as view"** dropped the condition rather than persisting it (a function is + not a string, so it survives `normalizeFilterOperator` unchanged and + `ViewFilterRuleSchema` refuses it) and saved a view carrying no `filter` key at + all — so the saved view silently disagreed with the chip the user had just read. + A malformed operator could **not** reach stored view metadata; +- the **list query** passed the triples to `toFilterNode` untouched and + `JSON.stringify` turned the function into `null` on the wire, sending the data + layer a condition with no operator in it. + +The repair removes the construction rather than naming the members: `URL_FILTER_OPS` +has no prototype, so an own entry is the only thing a lookup in it can find. A +denylist of `constructor` / `toString` / `hasOwnProperty` was considered and refused +— it is a spelling-level patch that the next member of a prototype this module does +not own walks straight past. + +**Nothing on the exported face moves.** `URL_FILTER_OPS` keeps its name, its +`Record` type and its four entries (`gte` `lte` `gt` `lt`), and +behaves identically under spread, `Object.keys` and `Object.entries` — which is how +`ObjectDataPage` inverts it to bridge a triple's operator to the spec's alias +spelling. The four declared operators are asserted to still resolve, beside a sweep +that enumerates `Object.prototype` at run time rather than listing today's members, +so a member added to the language is covered without anyone remembering to. diff --git a/packages/app-shell/src/views/drillUrlFilters.test.ts b/packages/app-shell/src/views/drillUrlFilters.test.ts index 282c62cfdc..94c703d336 100644 --- a/packages/app-shell/src/views/drillUrlFilters.test.ts +++ b/packages/app-shell/src/views/drillUrlFilters.test.ts @@ -9,6 +9,7 @@ import { describe, it, expect } from 'vitest'; import { parseUrlFilterTriples, + URL_FILTER_OPS, serializeDrillFilterParams, deleteFieldFilterParams, groupFilterChips, @@ -42,6 +43,77 @@ describe('parseUrlFilterTriples', () => { }); }); +/** + * A URL operator suffix naming an inherited member is NOT an operator + * (objectui#9507). + * + * The map was a plain object literal indexed straight by the suffix, so the + * truthiness test that decides "is this an operator" was answered by the + * PROTOTYPE CHAIN: `filter[amount][constructor]=1` resolved to + * `Object.prototype.constructor`, passed the guard, and emitted a triple whose + * OPERATOR WAS A JS FUNCTION. Three consequences were driven before the repair, + * one per consumer of these triples, and none of them was a crash — which is + * why this is pinned at the parser rather than at any of them: + * + * - the filter-chip row fell through `groupFilterChips`' range arms to the + * `= ` default and drew `amount = 1` — the "silently downgraded to + * equality" outcome this module's own contract says it never produces, + * rendered as a confident chip; + * - "Save as view" DROPPED the condition (a function is not a string, so it + * survives `normalizeFilterOperator` unchanged and `ViewFilterRuleSchema` + * refuses it) and persisted a view with no `filter` key — so the saved view + * silently disagreed with the chip the user had just read; + * - the list query passed the triples through `toFilterNode` untouched and + * `JSON.stringify` turned the function into `null` on the wire, sending an + * operator-less node the data layer refuses. + * + * The repair removes the construction rather than naming the members: the map + * has no prototype, so there is nothing to inherit and no denylist to keep in + * step with `Object.prototype`. The sweep below is written the same way — it + * ENUMERATES that prototype at run time instead of listing today's members, so + * a member added to the language is covered without anyone remembering to. + */ +describe('an inherited member is not an operator suffix (objectui#9507)', () => { + /** The card's own repro, kept literal as executable evidence of the defect. */ + it.each(['constructor', 'toString', 'hasOwnProperty'])( + 'emits nothing for `filter[amount][%s]=1`', + (suffix) => { + expect(parse(`filter[amount][${suffix}]=1`)).toEqual([]); + }, + ); + + it('emits nothing for ANY member of Object.prototype, enumerated not listed', () => { + // `__proto__` is in here and is a second shape, not a fourth spelling: its + // inherited accessor yielded `Object.prototype` itself, so that suffix + // produced a triple whose operator was an OBJECT rather than a function. + const leaking = Object.getOwnPropertyNames(Object.prototype).filter( + (name) => parse(`filter[amount][${name}]=1`).length > 0, + ); + expect(leaking).toEqual([]); + }); + + it('still resolves all four declared operators — the sweep above is not vacuous', () => { + // Without this, a parser that stopped emitting anything at all would pass + // every assertion above. The four are read from the exported map so the + // pair stays honest if the vocabulary grows. + expect(Object.keys(URL_FILTER_OPS).map((suffix) => parse(`filter[amount][${suffix}]=1`))) + .toEqual(Object.values(URL_FILTER_OPS).map((op) => [['amount', op, '1']])); + }); + + it('keeps the exported map a four-entry Record of suffix → ObjectQL symbol', () => { + // The repair may not move a published face: same name, same four entries, + // same spread/enumeration behaviour. Only the prototype is gone. + expect({ ...URL_FILTER_OPS }).toEqual({ gte: '>=', lte: '<=', gt: '>', lt: '<' }); + }); + + it('leaves the unknown-suffix control answering exactly as before', () => { + // The documented behaviour, and the control the card measured the defect + // against: a suffix that names nothing produces nothing, and this repair + // must not have reached it. + expect(parse('filter[amount][nope]=1')).toEqual([]); + }); +}); + describe('serializeDrillFilterParams', () => { it('serializes an equality value', () => { expect(serializeDrillFilterParams({ status: 'open' }).toString()).toBe('filter%5Bstatus%5D=open'); diff --git a/packages/app-shell/src/views/drillUrlFilters.ts b/packages/app-shell/src/views/drillUrlFilters.ts index 15fadd4ddb..f11dcbfb59 100644 --- a/packages/app-shell/src/views/drillUrlFilters.ts +++ b/packages/app-shell/src/views/drillUrlFilters.ts @@ -24,8 +24,43 @@ /** Filter triple shape shared with view metadata: [field, operator, value]. */ export type FilterTriple = [string, string, unknown]; -/** URL range/comparison operator suffix → ObjectQL operator (READ side). */ -export const URL_FILTER_OPS: Record = { gte: '>=', lte: '<=', gt: '>', lt: '<' }; +/** + * URL range/comparison operator suffix → ObjectQL operator (READ side). + * + * ## No prototype, because the URL chooses the key (objectui#9507) + * + * `parseUrlFilterTriples` decides "is this suffix an operator" by looking the + * suffix up here and testing the result for truthiness — and the suffix comes + * from the address bar. While this was a plain object literal that question was + * also answered by `Object.prototype`: `filter[amount][constructor]` resolved to + * `Object.prototype.constructor`, passed the guard, and emitted a triple whose + * OPERATOR WAS A JS FUNCTION — neither ignored nor downgraded, the two outcomes + * `parseUrlFilterTriples` promises are the only ones. `__proto__` was the same + * defect in a second shape: its inherited accessor yielded `Object.prototype` + * itself, so that suffix produced an operator that was an OBJECT. + * + * ⛔ The repair is deliberately NOT a list of member names to refuse. A denylist + * is a spelling-level patch that the next member of `Object.prototype` walks + * straight past, and it would have to be kept in step with a prototype this + * module does not own. Removing the prototype removes the construction that + * permitted the answer at all, so an own entry is the only thing a lookup here + * can ever find. The sweep in `drillUrlFilters.test.ts` enumerates + * `Object.prototype` at run time rather than naming members, for the same + * reason. + * + * ⚠️ The exported face is unchanged and must stay unchanged: same name, same + * four entries, same `Record` type, same behaviour under + * spread, `Object.entries` and `Object.keys` — `ObjectDataPage` inverts this + * map to bridge a triple's operator to the spec's alias spelling, and + * `drillEmptyBucketNavHost-9085.test.ts` pins its key list. ⛔ Do not "simplify" + * it back to an object literal. + */ +export const URL_FILTER_OPS: Record = Object.assign(Object.create(null), { + gte: '>=', + lte: '<=', + gt: '>', + lt: '<', +}); /** ObjectQL range operator key → URL param suffix (WRITE side). Inverse of the * relevant `URL_FILTER_OPS` entries. */