fix(app-shell): an inherited member is not a URL filter operator suffix

.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[<field>][<op>]=<value>` 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
+  `= <value>` 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<string, string>` 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.

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
+ *     `= <value>` 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');

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<string, string> = { 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<string, string>` 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<string, string> = 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. */