fix(table): reject hybrid predicate nodes at the wire instead of sile…

…ntly narrowing Found by running the HTTP suite: a node carrying BOTH a group key and a leaf's `field`/`op`/`value` returned 200, not 400. Zod strips unrecognized keys by default, so `{ all: [...], field, op, value }` parsed clean against the group branch with the leaf half quietly deleted. The hybrid guard added in eaf4179 could never fire — the keys were gone before `validatePredicate` ran. On the bulk paths that turns "delete archived rows for tenant acme" into "delete EVERY row for tenant acme". Both node shapes are now `strictObject`. Strict on the group alone would be worse than the bug: the union would fall through to the leaf branch, which is the more dangerous reading of the two. The bulk schemas are unaffected by design — their legacy `$`-object branch accepts any non-empty object, so it absorbs the hybrid WITHOUT stripping, the route's `isTablePredicate` check routes it back to `validatePredicate`, and the runtime guard rejects it there. Tests now pin both layers so removing either one fails loudly. Verified against a running server on the `hello` table: 18/18 HTTP checks pass, including the previously-failing hybrid case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011M563cbFy2S74GvSDf2C3R
TheodoreSpeaks · Jul 30, 2026 · Jun 27, 2026 · Jun 27, 2026 · Jul 3, 2026 · Jul 3, 2026
commit 705ea6f4d07fd3c352c4c81ab18c8109ae945b49
diff --git a/apps/sim/lib/api/contracts/tables-predicate.test.ts b/apps/sim/lib/api/contracts/tables-predicate.test.ts
@@ -12,6 +12,7 @@ import {
   rowQueryBodySchema,
   updateRowsByFilterBodySchema,
 } from '@/lib/api/contracts/tables'
+import { validatePredicate } from '@/lib/table/query-builder/validate'
 
 describe('rowQueryBodySchema', () => {
   it('accepts a predicate/sort object, leaves limit unbounded, has no offset', () => {
@@ -149,3 +150,76 @@ describe('predicate depth / size guard', () => {
     ).toBe(true)
   })
 })
+
+/**
+ * Zod strips unrecognized keys by default, so before the schemas were made
+ * strict a hybrid node parsed clean against the group branch with its leaf half
+ * silently deleted — turning "delete archived rows for tenant acme" into
+ * "delete EVERY row for tenant acme". `validatePredicate`'s hybrid guard could
+ * not catch it: the keys were gone before it ran.
+ */
+describe('hybrid group+leaf nodes are rejected, not silently narrowed', () => {
+  const hybrid = {
+    all: [{ field: 'tenant_id', op: 'eq', value: 'acme' }],
+    field: 'status',
+    op: 'eq',
+    value: 'archived',
+  }
+
+  it('rejects rather than dropping the leaf half', () => {
+    const result = predicateSchema.safeParse(hybrid)
+    expect(result.success).toBe(false)
+    // The dangerous outcome: parsing succeeds having quietly widened the filter.
+    expect(result.success ? result.data : null).not.toEqual({ all: hybrid.all })
+  })
+
+  /**
+   * The bulk schemas union the predicate tree with the legacy `$`-object, and
+   * that legacy branch accepts any non-empty object — so it absorbs the hybrid
+   * and the SCHEMA cannot reject it. Crucially the legacy branch does NOT strip,
+   * so `all` survives, the route's `isTablePredicate` check routes it back to
+   * `validatePredicate`, and the hybrid guard there rejects it (→ 400 via
+   * `route.ts:325`). Asserted here so a future change to either layer that
+   * removes one of them fails loudly.
+   */
+  it('keeps the hybrid intact through the bulk schemas so the runtime guard can see it', () => {
+    for (const parsed of [
+      deleteTableRowsBodySchema.safeParse({ workspaceId: 'ws-1', filter: hybrid }),
+      updateRowsByFilterBodySchema.safeParse({
+        workspaceId: 'ws-1',
+        filter: hybrid,
+        data: { active: false },
+      }),
+    ]) {
+      expect(parsed.success).toBe(true)
+      // The leaf half must NOT have been silently dropped on the way through.
+      expect(parsed.success && parsed.data.filter).toMatchObject({
+        all: hybrid.all,
+        field: 'status',
+      })
+    }
+  })
+
+  it('and validatePredicate then rejects it', () => {
+    expect(() =>
+      validatePredicate(hybrid as never, [
+        { name: 'tenant_id', type: 'string' },
+        { name: 'status', type: 'string' },
+      ])
+    ).toThrow(/not both/)
+  })
+
+  it('rejects an unknown key on a leaf (a typo must not be dropped)', () => {
+    expect(
+      predicateSchema.safeParse({ all: [{ field: 'a', op: 'eq', vlaue: 'typo' }] }).success
+    ).toBe(false)
+  })
+
+  it('still accepts well-formed nodes', () => {
+    expect(
+      predicateSchema.safeParse({
+        all: [{ field: 'a', op: 'eq', value: 1 }, { any: [{ field: 'b', op: 'isNull' }] }],
+      }).success
+    ).toBe(true)
+  })
+})
diff --git a/apps/sim/lib/api/contracts/tables.ts b/apps/sim/lib/api/contracts/tables.ts
@@ -319,7 +319,17 @@ function predicateTreeTooLarge(root: unknown): string | null {
  * (unknown column, json-op rejection) is enforced server-side by
  * `validatePredicate` once the table's columns are known.
  */
-const predicateLeafSchema = z.object({
+/**
+ * Both node shapes are `strictObject`, and that is load-bearing rather than
+ * fussiness. Zod strips unrecognized keys by default, so a hybrid node carrying
+ * BOTH a group key and a leaf's `field`/`op`/`value` parsed clean against the
+ * group branch with the leaf half silently deleted. On the bulk paths that turns
+ * "delete archived rows for tenant acme" into "delete every row for tenant acme".
+ * `validatePredicate`'s hybrid guard could never catch it — the keys were gone
+ * before it ran. Strict on BOTH branches is required: strict on the group alone
+ * would just fall through to the leaf branch, which is the more dangerous reading.
+ */
+const predicateLeafSchema = z.strictObject({
   field: z.string().min(1, 'field is required').max(128),
   op: z.enum(FILTER_OPS),
   value: z.unknown().optional(),
@@ -333,13 +343,13 @@ const predicateTreeSchema: z.ZodType<TablePredicate> = z.lazy(() =>
   z.union([
     // `.min(1)`: an empty group compiles to no WHERE clause, which on the bulk
     // delete/update paths reads as "match everything" rather than "match nothing".
-    z.object({
+    z.strictObject({
       all: z
         .array(predicateNodeSchema)
         .min(1, 'A filter group must contain at least one condition')
         .max(MAX_PREDICATE_GROUP_SIZE),
     }),
-    z.object({
+    z.strictObject({
       any: z
         .array(predicateNodeSchema)
         .min(1, 'A filter group must contain at least one condition')