fix(analytics): refuse an undefined comparand in the where door instead of reading it seven ways (#6386) (#6445)

`buildNode`'s first line was `if (raw === undefined) continue;` — the entry gate
doing the one thing the rest of the module forbids: dropping a key without a
trace, which does not narrow a query, it WIDENS it. Measured on origin/main
(5faa23c), one shape had seven readings; the first three widen:

  {d: undefined}              -> null                      (the WHOLE where dropped)
  {stage:'won', d: undefined} -> stage equals 'won'         (the conjunct vanished)
  {$not: {d: undefined}}      -> NOT (d set) = d IS NULL    (an invented predicate)
  {d: {$eq: undefined}}       -> d equals [null]            (value comparison)
  {d: {$gt: undefined}}       -> d gt [null]
  {d: {$in: [undefined]}}     -> d in [null]
  {d: {$ne: undefined}}       -> d notSet OR d notEquals [null]

Row three is the strangest: #5146's null-safe rewrite splits the leaf into
`{d: {$null: false}} AND {d: undefined}`, the entry gate dropped the second
half, and the surviving guard was negated — so a predicate grew out of a
discarded leaf.

#6050 ruling B (undefined in a comparand position is refused) is pushed down to
this door, in this module's existing envelope: INVALID_FILTER / 400, because
unlike `read-scope-sql`'s platform-compiled artifact this door receives caller
input. `assertDefinedComparands` sweeps the four comparand positions in
`fieldLeaves` — the only leaf producer, so one gate covers all three consumers of
the tree — and deleting the entry line lets each combinator reach the refusal it
already had. The direction is silently wrong RESULTS, not a permission bypass:
read scope is compiled by the other door and never passes through here.

`comparand()` (#5526) and the strict `wrapper[opKey] === null` branch (#5332) are
byte-identical: this refuses an INPUT and reopens neither. The normalisation is
now unreachable from this door, which is recorded rather than acted on.


Claude-Session: https://claude.ai/code/session_015a5qkLzpGXhLL2F5gvJ7dD

Co-authored-by: Claude <noreply@anthropic.com>

Author: hotlong

.changeset/lucky-pandas-repeat.md

@@ -0,0 +1,50 @@
+---
+'@objectstack/service-analytics': patch
+---
+
+fix(analytics): an `undefined` comparand in an analytics `where` is refused (400 `INVALID_FILTER`), not read seven different ways
+
+**Observable behaviour change.** A `where` key whose value is `undefined` used to
+compile — in seven different ways, depending on where it sat. It is now refused
+with `INVALID_FILTER` / 400, the envelope every other refusal at this door
+already carries.
+
+The three that mattered WIDENED the query, which is the failure mode
+`filter-normalizer.ts` forbids in its own body ("NEVER drop: a missing predicate
+does not narrow the query, it WIDENS it"), while its entry line did exactly that:
+
+| `where` | used to normalize to | reading |
+|---|---|---|
+| `{d: undefined}` | `null` | the WHOLE filter dropped — the query ran **unfiltered** |
+| `{stage: 'won', d: undefined}` | `stage equals 'won'` | the `d` conjunct vanished in silence |
+| `{$not: {d: undefined}}` | `NOT (d set)` | `d IS NULL` — a predicate the author never wrote |
+| `{d: {$eq: undefined}}` | `d equals [null]` | a value comparison, **not** `$eq: null`'s null predicate |
+| `{d: {$gt: undefined}}` | `d gt [null]` | ditto |
+| `{d: {$in: [undefined]}}` | `d in [null]` | ditto |
+| `{d: {$ne: undefined}}` | `d notSet OR d notEquals [null]` | ditto |
+
+The direction is silently **wrong results** — an analytics figure, a report
+total, an aggregate, wrong with nothing to read — **not** a permission bypass:
+read scope is compiled by a different door (`read-scope-sql.ts`) and never passed
+through here, so a caller still saw only rows it was entitled to, just more of
+them than it asked for.
+
+**What to change if this refuses your filter.** `undefined` cannot cross JSON, so
+neither REST door can carry it — this only reaches in-process callers of
+`AnalyticsService.query({ where })` that spread a possibly-absent value into the
+filter object (`{ owner_id: ctx.user?.id }`). Two repairs, both stated by the
+error message:
+
+- meant the null predicate → write `{ field: null }` or `{ field: { $null: true } }`;
+- the value is genuinely absent → **omit the key**, which is the same "no
+  constraint" without the ambiguity.
+
+Inside stored metadata, the platform's own answer to "scope this to the current
+user" is unaffected and was already fail-closed: a `{current_user_id}`
+placeholder resolves through `resolveFilterTokens`, which raises
+`FILTER_TOKEN_UNRESOLVED` / 400 rather than emitting `undefined`.
+
+⛔ **`null` does not move.** `{d: null}`, `{$eq: null}`, `{$ne: null}`,
+`{$null: …}`, `{$exists: …}` and `$contains: null` keep their exact lowering —
+`null` is a declared comparand and is the null predicate. `$null` / `$exists`
+carry a declared boolean flag rather than a comparand and are likewise untouched.

packages/services/service-analytics/src/__tests__/filter-normalizer-not-null-safe.test.ts

@@ -722,9 +722,34 @@ describe('[#5325] analytics `where` — NULL-safe `$not` and the boolean identit
       expect(sql).toContain('"account"."region" = $1');
     });
 
-    it('an undefined value is still skipped, as it always was', async () => {
-      expect(await ids({ stage: undefined, owner: 'u1' })).toEqual(['1', '3']);
-      expect(await ids({ $not: undefined, owner: 'u1' })).toEqual(['1', '3']);
+    it('an undefined value is REFUSED at this seam (#6386), not skipped', async () => {
+      // ⚠️ REPLACED, not re-spelled. This case read "an undefined value is still
+      // skipped, as it always was" and asserted `ids({stage: undefined, owner:
+      // 'u1'}) === ['1','3']` — i.e. it PINNED the skip as a guarantee. #6386
+      // measured what the skip actually bought: `{stage: undefined}` alone
+      // normalised to `null`, so a single-key `where` ran with NO filter and the
+      // chart was drawn over every row — the #3650 widening this file's own
+      // subject (`$not`) exists to prevent, arriving through the entry gate
+      // instead. The two-key spelling above hid that: `owner: 'u1'` survived, so
+      // the row set still looked filtered.
+      //
+      // Kept at THIS seam deliberately — `ids` executes end to end through
+      // `NativeSQLStrategy`, which is where a compiled-to-nothing filter turns
+      // into a statement with no `WHERE` (#5297's lesson, one call above the
+      // compiler). So this asserts the refusal reaches the executing seam, not
+      // merely that the normalizer throws in isolation.
+      await expect(ids({ stage: undefined, owner: 'u1' })).rejects.toThrowError(
+        /comparand at "stage" is undefined/,
+      );
+      // `$not: undefined` is NOT the same condition and must not borrow the same
+      // message: it is a combinator with a missing operand, and the branch that
+      // already owned that shape gives the truer diagnosis.
+      await expect(ids({ $not: undefined, owner: 'u1' })).rejects.toThrowError(
+        /"\$not" requires a filter object/,
+      );
+      // The row set the old assertion recorded is still reachable — by writing
+      // the filter the author meant, with the unknowable key simply omitted.
+      expect(await ids({ owner: 'u1' })).toEqual(['1', '3']);
     });
 
     it('an ordinary filter compiles to exactly the SQL it always did', async () => {