Skip to content

Commit be721ef

Browse files
committed
fix(plugin-security)!: the RLS compile seam refuses a non-number on a numeric column, as where does; pin the moved aggregate answers
The compiled policy filter runs the spec's number-comparand verdict, the one the engine's where door consults: a comparand a numeric column cannot be compared with drops the policy through the refused-comparand route (read deny sentinel, write 403), and a numeric string is narrowed to its number. The objectql pins record what formula's deleted whole-day copy moves at the registry-less, audit-opt-out, text/text and direct applyInMemoryAggregation positions. Changesets for formula and plugin-security. Claude-Session: https://claude.ai/code/session_017xfMoEjKUuSh2xYB8sCozp Co-authored-by: Claude <noreply@anthropic.com>
1 parent e4d85c5 commit be721ef

7 files changed

Lines changed: 588 additions & 18 deletions
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
"@objectstack/formula": minor
3+
---
4+
5+
fix(formula)!: `matchesFilterCondition` compares a bare-day upper bound as written; its own whole-day copy is deleted (ADR-0053 D-D1 items 5 and 9)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) a change of how one runtime evaluator answers an ordering comparison, not of anything an author writes: no spec key, spelling, export or stored shape moves. FilterConditionSchema, every RLS policy, object and query definition parse and save as before, @objectstack/formula exports the same names with the same types, and no stored row is read or rewritten. What moves is the answer for a bare-day upper bound that reaches the evaluator without the shared lowering, which the seams already apply, so there is nothing for objectstack migrate meta to rewrite. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers a filter's bound semantics and this diff adds none (not registered / already-registered); and the change is runtime behaviour, not a declaration (not runtime-interface-only / type-surface-only). -->
10+
11+
**BREAKING**: this narrows what the RLS write check admits on columns that are not `datetime`, and moves a few `engine.aggregate` answers that no seam lowers. It ships as `minor` under the launch-window convention for accept-set narrowings. No export or published type changes.
12+
13+
**What is deleted.** `matchesFilterCondition` no longer reads a bare `YYYY-MM-DD` `$lte`, or a `$between` maximum, as "through that whole day", and no longer drops the bound on `9999-12-31`. It compares the value as written, as every other ordering operator here does, and as `driver-sql` compares it on the read. The whole day is applied once, at the seams that feed this evaluator, by the shared `lowerFilterCondition` (`@objectstack/spec/data`): the RLS compile seam lowers every policy filter on the object's declared `datetime` columns, the engine lowers `having` and `aggregations[i].filter` the same way, and the RLS write check judges a declared `date`, `datetime` or `time` column in its stored form. So a `check` on a `date` or `datetime` column answers exactly as before.
14+
15+
**The RLS write check now agrees with the read on other columns.** Measured through `ObjectQL.insert` and `SecurityPlugin` on `SqlDriver` (better-sqlite3), as a member whose policy has the same `using` and `check`:
16+
17+
- a `text` column under `record.title <= '2026-01-05'`, written as `'2026-01-05T15:00:00Z'` or `'2026-01-05 noon'`: the write was admitted while the read hid the stored row. It is now refused `PERMISSION_DENIED` / 403, and the read still hides it;
18+
- two `text` columns, `record.title <= record.code`, with `code` holding `'2026-01-05'`: the same, admitted before and 403 now, with the read hiding the row;
19+
- a `number` column under `record.amount <= '9999-12-31'`: the write was admitted because an epoch number read as an instant on the last supported day. A number is not less than a day string, so it is now 403. The engine refuses the same comparison in a `where` (`INVALID_FILTER` / 400: a day string is not a number).
20+
21+
The access explanation (`explain`) judges a stored row with this evaluator, so its row verdict moves the same way: for the two `text` cells it now says hidden, as the read does.
22+
23+
**`engine.aggregate` answers that no seam lowers.** A `{ $field }` referent is per row, so no seam can lower it. These positions are now compared as written:
24+
25+
- two declared `text` columns of one class, at a per-aggregation `filter` or between two `having` group columns: `'2026-01-05 noon'` against `'2026-01-05'` is no longer counted or kept, which is what the same comparison answers in a `where`;
26+
- the pairs the class rule cannot judge because a side has no declaration: an object the registry does not declare, and an audit-opt-out object's row-carried `created_at` / `updated_at` against a `date`. An instant on the due day is no longer counted against that bare day;
27+
- a direct `applyInMemoryAggregation` call, which applies no class rule.
28+
29+
**The remedy.** Compare a `datetime` with a `datetime` and a `date` with a `date`. A `datetime` against a calendar day has no single answer across SQL and memory, and a declared pair of the two is already refused. A number compared with a day string has no answer at all: compare a number with a number. A caller that evaluates a filter on a `datetime` column without passing a seam lowers it first with `lowerFilterCondition(filter, { isDatetimeColumn })` to get the whole-day reading.
30+
31+
**Unchanged.** A `check` on a declared `date`, `datetime` or `time` column, a `{ $field }` pair of two `date` or two `datetime` columns (with or without `addDays`), a full-ISO bound, `$gte` / `$gt` / `$lt` and `$eq`.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
"@objectstack/plugin-security": minor
3+
---
4+
5+
fix(plugin-security)!: a row-level policy that compares a numeric column with a comparand that is not a number is refused at the RLS compile seam, read and write alike, as the engine's `where` refuses the same comparison
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) a refusal of a compiled policy comparand at the RLS compile seam, the same comparand the engine's where door already refuses: no authorable key, spelling, export or stored shape moves. RowLevelSecurityPolicySchema and every permission set parse and save as before, the predicate's text is untouched, @objectstack/plugin-security exports the same names, and no stored row is read or rewritten. Which number the author meant is not something a ledger entry can decide, so there is nothing for objectstack migrate meta to rewrite. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers a filter comparand's type and this diff adds none (not registered / already-registered); and the change is runtime behaviour, not a declaration (not runtime-interface-only / type-surface-only). -->
10+
11+
**BREAKING**: this narrows which row-level policies the RLS compile seam hands to its two consumers, the read and the write check. It ships as `minor` under the launch-window convention for accept-set narrowings. No export or published type changes.
12+
13+
**What was accepted before.** A policy such as `record.amount <= '9999-12-31'` on a `number` column compiled, and its `using` and `check` both reached their consumers unjudged. Measured through `ObjectQL.insert` and `SecurityPlugin` on `SqlDriver` (better-sqlite3), as a member: the write of `amount: 5` was admitted (`@objectstack/formula`'s deleted whole-day copy read the number as an instant), and the read showed the stored row, because SQLite orders an integer before any text. PostgreSQL refuses to bind such text against a numeric column. The same comparison in a caller's `where` is refused `INVALID_FILTER` / 400 by the engine's number-comparand door.
14+
15+
**What is refused now.** The seam runs the spec's number-comparand verdict (`numberComparandDoorVerdict`, `@objectstack/spec/data`), the one the engine's `where` door consults, on every compiled policy filter, after the shape door and before the comparand-type door. On a column the object declares numeric, a comparand that is not a number (a string the platform's numeric grammar does not read, such as `'9999-12-31'` or `'abc'`, a boolean, a `Date` or a list) drops the policy through the existing fail-closed route: the read is filtered by the deny sentinel and returns no rows, the write is refused `PERMISSION_DENIED` / 403, and a WARN line names the policy, the clause and the comparand. A granting sibling policy still grants.
16+
17+
**Narrowed, as in `where`.** A numeric string (`'10'`, `'1e3'`) is replaced by the number it names before either consumer runs. So `record.amount == '10'` now matches a stored `10` on the write check, which compared the text with the number and refused it, while the read showed the row.
18+
19+
**The remedy.** Compare a numeric column with a number: `record.amount <= 9999`, not `record.amount <= '9999-12-31'`.
20+
21+
**Unchanged.** A numeric literal, a column that is not numeric, a `{ $field }` reference, and an object whose declaration cannot be read (nothing is judged without one).

‎packages/objectql/src/engine-aggregate-filter.test.ts‎

Lines changed: 97 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { describe, it, expect } from 'vitest';
3232
import { normalizeFilterComparandTypes, type EngineAggregateOptions } from '@objectstack/spec/data';
3333
import { ObjectQL } from './engine.js';
3434
import { matchesAggregationFilter } from './having-filter.js';
35+
import { applyInMemoryAggregation } from './in-memory-aggregation.js';
3536

3637
// The #10413 measurement's dataset shape: opportunities with a stage and an
3738
// amount. 6 rows, 2 closed_won worth 700 total.
@@ -863,18 +864,107 @@ describe('[#21255] per-aggregation filter — a plain { $field } across two comp
863864
});
864865
}
865866

866-
it('an object the registry does not declare is not judged — the card\'s query is answered as before', async () => {
867+
it('an object the registry does not declare is not judged — the card\'s query is answered, as written', async () => {
867868
// The fail-open direction an `addDays` pair already takes for a
868869
// registry-less host: no declaration, no class, no verdict.
870+
//
871+
// [#21242] What answers it is `@objectstack/formula`'s matcher, which no
872+
// longer keeps a whole-day copy of the bare-day upper bound: a pair that
873+
// reaches it without a seam is compared as written (ADR-0053 D-D1 item 5).
874+
// None of the six ORDERS closes on its due day, so they count 3 either
875+
// way. `o7` does, at 15:00: the deleted copy read its due day as "through
876+
// that day" and counted it (4 of 7); as written, the instant's text sorts
877+
// above the bare day, and it is not counted (3 of 7).
878+
const ON_THE_DUE_DAY = {
879+
id: 'o7', customer_id: 'c3', amount: 10, cap: 1, placed_on: '2026-01-05', due_on: '2026-01-05', grace: 0,
880+
opened_at: '2026-01-05T08:00:00.000Z', closed_at: '2026-01-05T15:00:00.000Z', slot: '15:00:00', created_at: '2026-09-01T00:00:00.000Z',
881+
};
869882
for (const native of [true, false]) {
870-
const { driver } = makeCountingDriver(ORDERS, native);
871-
const engine = new ObjectQL();
872-
engine.registerDriver(driver, true);
873-
await engine.init();
874-
expect(await engine.aggregate('crm_order', withFilter({ closed_at: { $lte: { $field: 'due_on' } } })))
875-
.toEqual([{ opp_count: 6, picked: 3 }]);
883+
for (const [rows, expected] of [
884+
[ORDERS, { opp_count: 6, picked: 3 }],
885+
[[...ORDERS, ON_THE_DUE_DAY], { opp_count: 7, picked: 3 }],
886+
] as const) {
887+
const { driver } = makeCountingDriver(rows, native);
888+
const engine = new ObjectQL();
889+
engine.registerDriver(driver, true);
890+
await engine.init();
891+
expect(await engine.aggregate('crm_order', withFilter({ closed_at: { $lte: { $field: 'due_on' } } })))
892+
.toEqual([expected]);
893+
}
894+
}
895+
});
896+
});
897+
898+
// ───────────────────────────────────────────────────────────────────────────
899+
// [#21242] formula's whole-day copy is deleted: what reaches the matcher
900+
// unlowered is compared as written
901+
// ───────────────────────────────────────────────────────────────────────────
902+
903+
describe('[#21242] per-aggregation filter — a { $field } pair no seam lowers is compared as written', () => {
904+
// `@objectstack/formula`'s `matchesFilterCondition`, which this position
905+
// calls for every `{ $field }` comparison, read a bare-day referent as
906+
// "through that day" until its copy of the rule was deleted (ADR-0053 D-D1
907+
// items 5 and 9). The engine's seam lowers a literal bound; a referent is
908+
// per row and no seam can lower it, so each pair below is now compared as
909+
// written. Measured through `engine.aggregate` before the deletion (counts
910+
// in the names), and on `SqlDriver` over better-sqlite3 for the `where`
911+
// twin of the text pair, which keeps only `b2` there.
912+
const DAY_ROWS: ReadonlyArray<Record<string, unknown>> = [
913+
{ id: 'b1', code: '2026-01-05 noon', label: '2026-01-05', due_on: '2026-01-05', created_at: '2026-01-05T15:00:00.000Z', updated_at: '2026-01-05T15:00:00.000Z', closed_at: '2026-01-05T15:00:00.000Z' },
914+
{ id: 'b2', code: '2026-01-04', label: '2026-01-05', due_on: '2026-01-05', created_at: '2026-01-05T00:00:00.000Z', updated_at: '2026-01-05T00:00:00.000Z', closed_at: '2026-01-05T00:00:00.000Z' },
915+
{ id: 'b3', code: '2026-01-06', label: '2026-01-05', due_on: '2026-01-05', created_at: '2026-01-04T10:00:00.000Z', updated_at: '2026-01-04T10:00:00.000Z', closed_at: '2026-01-04T10:00:00.000Z' },
916+
{ id: 'b4', code: '2026-01-05', label: '2026-01-05', due_on: '2026-01-05', created_at: '2026-01-06T01:00:00.000Z', updated_at: '2026-01-06T01:00:00.000Z', closed_at: '2026-01-06T01:00:00.000Z' },
917+
];
918+
const FIELDS = { code: { type: 'text' }, label: { type: 'text' }, due_on: { type: 'date' }, closed_at: { type: 'datetime' } };
919+
920+
async function dayEngine(native: boolean, object: Record<string, unknown> | null) {
921+
const { driver } = makeCountingDriver(DAY_ROWS, native);
922+
const engine = new ObjectQL();
923+
engine.registerDriver(driver, true);
924+
await engine.init();
925+
if (object) (engine.registry as any).registerObject(object);
926+
return engine;
927+
}
928+
929+
it('two declared text columns (one class): "2026-01-05 noon" is not <= "2026-01-05" — 2 of 4, was 3, as the where twin keeps', async () => {
930+
for (const native of [true, false]) {
931+
const engine = await dayEngine(native, { name: 'qa_day', fields: FIELDS });
932+
expect(await engine.aggregate('qa_day', withFilter({ code: { $lte: { $field: 'label' } } })))
933+
.toEqual([{ opp_count: 4, picked: 2 }]);
934+
}
935+
});
936+
937+
it('an audit-opt-out object\'s row-carried created_at / updated_at against a date — 1 of 4, was 3 (the residual fail-open, not judged)', async () => {
938+
// The field map carries no `created_at` / `updated_at` when the object
939+
// opts out of the audit columns, so the class rule has no declaration to
940+
// judge; `declaredReferenceNames` still admits them as referents. The same
941+
// pair is refused 400 on an object that keeps its audit columns (below)
942+
// and by `driver-sql` in a `where`.
943+
for (const native of [true, false]) {
944+
const engine = await dayEngine(native, { name: 'qa_day', fields: FIELDS, systemFields: { audit: false } });
945+
for (const column of ['created_at', 'updated_at']) {
946+
expect(await engine.aggregate('qa_day', withFilter({ [column]: { $lte: { $field: 'due_on' } } })), column)
947+
.toEqual([{ opp_count: 4, picked: 1 }]);
948+
}
876949
}
877950
});
951+
952+
it('…the same pair on an object that keeps its audit columns is refused before any read (the control)', async () => {
953+
for (const native of [true, false]) {
954+
const engine = await dayEngine(native, { name: 'qa_day', fields: FIELDS });
955+
const err = await refusalOf(() => engine.aggregate('qa_day', withFilter({ created_at: { $lte: { $field: 'due_on' } } })));
956+
expect(err.code).toBe('INVALID_FILTER');
957+
expect(err.status).toBe(400);
958+
}
959+
});
960+
961+
it('a direct applyInMemoryAggregation call — no seam, no class rule — 1 of 4, was 3, with or without a field map', () => {
962+
// The class rule is `engine.aggregate`'s; this published function applies
963+
// the JSON-column rule alone, so a cross-class pair reaches the matcher.
964+
const ast = withFilter({ closed_at: { $lte: { $field: 'due_on' } } }) as never;
965+
expect(applyInMemoryAggregation([...DAY_ROWS], ast)).toEqual([{ opp_count: 4, picked: 1 }]);
966+
expect(applyInMemoryAggregation([...DAY_ROWS], ast, undefined, FIELDS)).toEqual([{ opp_count: 4, picked: 1 }]);
967+
});
878968
});
879969

880970
describe('[#20148] a Date bound is compared as an instant — as the same bound in a where is', () => {

‎packages/objectql/src/engine-aggregate-having-comparand-shape.test.ts‎

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -942,17 +942,54 @@ describe('[#20127] having — a { $field, addDays } pair is judged by each aggre
942942
});
943943
}
944944

945-
it('a registry-less host is not judged — the pair is answered as before, as an addDays pair is', async () => {
945+
it('a registry-less host is not judged — the pair is answered, as written, as an addDays pair is', async () => {
946946
// No declaration, so no column has a type or a class: the fail-open
947947
// direction #20127 took for an `addDays` pair, kept for a plain one.
948+
//
949+
// [#21242] What answers it is `@objectstack/formula`'s matcher, which no
950+
// longer keeps a whole-day copy of the bare-day upper bound: a pair that
951+
// reaches it without a seam is compared as written (ADR-0053 D-D1 item
952+
// 5). No group of DT_ROWS closes on its first due day, so they keep
953+
// `c1` either way. `c4` does, at 15:00: the deleted copy read the day as
954+
// "through that day" and kept `c4` beside `c1`; as written, the
955+
// instant's text sorts above the bare day, and `c4` is dropped.
956+
const ON_THE_DUE_DAY = {
957+
customer_id: 'c4', amount: 10, cap: 1, placed_on: '2026-01-05', due_on: '2026-01-05', grace: 0,
958+
opened_at: '2026-01-05T08:00:00.000Z', closed_at: '2026-01-05T15:00:00.000Z',
959+
};
948960
for (const [door, native] of DOORS) {
949-
const { driver } = makeDriver(DT_ROWS, native);
950-
const engine = new ObjectQL();
951-
engine.registerDriver(driver, true);
952-
await engine.init();
953-
const rows = await engine.aggregate(OBJECT, { ...DT_QUERY, having: { last_closed: { $lte: { $field: 'first_due' } } } });
954-
expect(groups(rows), door).toEqual(['c1']);
961+
for (const rows of [DT_ROWS, [...DT_ROWS, ON_THE_DUE_DAY]]) {
962+
const { driver } = makeDriver(rows, native);
963+
const engine = new ObjectQL();
964+
engine.registerDriver(driver, true);
965+
await engine.init();
966+
const out = await engine.aggregate(OBJECT, { ...DT_QUERY, having: { last_closed: { $lte: { $field: 'first_due' } } } });
967+
expect(groups(out), door).toEqual(['c1']);
968+
}
955969
}
956970
});
971+
972+
it('[#21242] two text group columns (one class): "2026-01-05 noon" is not <= "2026-01-05" — the group is dropped, as where compares them', async () => {
973+
// `having` compares two groupBy projections with the matcher. A bare-day
974+
// label was read as "through that day" by its deleted whole-day copy,
975+
// which kept the `2026-01-05 noon` group; compared as written, as
976+
// `driver-sql` compares two text columns in a `where`, it is dropped.
977+
const LABEL_ROWS = [
978+
{ customer_id: 'c1', code: '2026-01-05 noon', label: '2026-01-05' },
979+
{ customer_id: 'c2', code: '2026-01-04', label: '2026-01-05' },
980+
{ customer_id: 'c3', code: '2026-01-06', label: '2026-01-05' },
981+
];
982+
const { driver } = makeDriver(LABEL_ROWS, false);
983+
const engine = new ObjectQL();
984+
engine.registerDriver(driver, true);
985+
await engine.init();
986+
engine.registry.registerObject({ name: OBJECT, fields: { customer_id: { type: 'text' }, code: { type: 'text' }, label: { type: 'text' } } } as any);
987+
const out = await engine.aggregate(OBJECT, {
988+
groupBy: ['code', 'label'],
989+
aggregations: [{ function: 'count', alias: 'n' }],
990+
having: { code: { $lte: { $field: 'label' } } },
991+
});
992+
expect(out.map((r: any) => r.code)).toEqual(['2026-01-04']);
993+
});
957994
});
958995
});

0 commit comments

Comments
 (0)