-
-
Notifications
You must be signed in to change notification settings - Fork 7
fix(core): refuse to prune a sealed filter condition #879
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| */ | ||
|
|
||
| import { | ||
| ErrorCode, | ||
| Field, | ||
| Fields, | ||
| Filter, | ||
|
|
@@ -24,6 +25,7 @@ import { | |
| pruneFiltersByRelations, | ||
| pruneRelationsByRelations, | ||
| pruneSortsByRelations, | ||
| seal, | ||
| } from '../../../src'; | ||
| import type { IFilters } from '../../../src'; | ||
|
|
||
|
|
@@ -197,4 +199,136 @@ describe('src/parser/relation-prune.ts', () => { | |
| expect(pruneFiltersByRelations(filters, ['items.owner']).value).toEqual([]); | ||
| }); | ||
| }); | ||
|
|
||
| describe('pruneFiltersByRelations (sealed conditions)', () => { | ||
| const eq = (field: string) => new Filter(FilterFieldOperator.EQUAL, field, 'x'); | ||
|
|
||
| it('throws instead of dropping a sealed leaf', () => { | ||
| const filters = new Filters(FilterCompoundOperator.AND, [ | ||
| seal(eq('user.name')), | ||
| ]); | ||
|
|
||
| expect(() => pruneFiltersByRelations(filters, ['user'])) | ||
| .toThrowError(expect.objectContaining({ code: ErrorCode.SCHEMA_SEALED_CONDITION_PRUNED })); | ||
| }); | ||
|
|
||
| it('names the rejected relation and the sealed field', () => { | ||
| const filters = new Filters(FilterCompoundOperator.AND, [ | ||
| seal(eq('realm.id')), | ||
| ]); | ||
|
|
||
| expect(() => pruneFiltersByRelations(filters, ['realm'])) | ||
| .toThrowError(/"realm".+"realm\.id"/); | ||
| }); | ||
|
|
||
| it('throws instead of dropping a policy residual out of a sealed group', () => { | ||
| // the shape a filters validate hook produces: | ||
| // seal(and(<client leaf>, <policy residual>)) | ||
| const filters = new Filters(FilterCompoundOperator.AND, [ | ||
| seal(new Filters(FilterCompoundOperator.AND, [ | ||
| eq('name'), | ||
| eq('realm.id'), | ||
| ])), | ||
| eq('realm.name'), | ||
| ]); | ||
|
|
||
| expect(() => pruneFiltersByRelations(filters, ['realm'])) | ||
| .toThrowError(expect.objectContaining({ code: ErrorCode.SCHEMA_SEALED_CONDITION_PRUNED })); | ||
| }); | ||
|
|
||
| it('throws for a sealed condition nested below an unsealed group', () => { | ||
| const filters = new Filters(FilterCompoundOperator.AND, [ | ||
| new Filters(FilterCompoundOperator.OR, [ | ||
| eq('id'), | ||
| seal(eq('user.name')), | ||
| ]), | ||
| ]); | ||
|
|
||
| expect(() => pruneFiltersByRelations(filters, ['user'])) | ||
| .toThrowError(expect.objectContaining({ code: ErrorCode.SCHEMA_SEALED_CONDITION_PRUNED })); | ||
| }); | ||
|
|
||
| it('throws instead of dropping a sealed elemMatch', () => { | ||
| const filters = new Filters(FilterCompoundOperator.AND, [ | ||
| seal(new Filter( | ||
| FilterFieldOperator.ELEM_MATCH, | ||
| 'items', | ||
| new Filter(FilterFieldOperator.EQUAL, 'id', 1), | ||
| )), | ||
| ]); | ||
|
|
||
| expect(() => pruneFiltersByRelations(filters, ['items'])) | ||
| .toThrowError(expect.objectContaining({ code: ErrorCode.SCHEMA_SEALED_CONDITION_PRUNED })); | ||
| }); | ||
|
|
||
| it('throws instead of pruning the interior of a sealed elemMatch', () => { | ||
| const filters = new Filters(FilterCompoundOperator.AND, [ | ||
| seal(new Filter( | ||
| FilterFieldOperator.ELEM_MATCH, | ||
| 'items', | ||
| new Filters(FilterCompoundOperator.AND, [ | ||
| eq('owner.name'), | ||
| new Filter(FilterFieldOperator.EQUAL, 'id', 1), | ||
| ]), | ||
| )), | ||
| ]); | ||
|
|
||
| expect(() => pruneFiltersByRelations(filters, ['items.owner'])) | ||
| .toThrowError(expect.objectContaining({ code: ErrorCode.SCHEMA_SEALED_CONDITION_PRUNED })); | ||
| }); | ||
|
|
||
| // The two shapes below cannot fail open: dropping an OR arm narrows, | ||
| // and dropping the interior of a NOT removes a restriction the seal | ||
| // put there. Pruning still refuses, because the seal is a per-node | ||
| // marker and not a per-operator judgement call. | ||
| it('throws for a sealed OR arm, where a drop would narrow rather than widen', () => { | ||
| const filters = new Filters(FilterCompoundOperator.AND, [ | ||
| seal(new Filters(FilterCompoundOperator.OR, [eq('id'), eq('user.name')])), | ||
| ]); | ||
|
|
||
| expect(() => pruneFiltersByRelations(filters, ['user'])) | ||
| .toThrowError(expect.objectContaining({ code: ErrorCode.SCHEMA_SEALED_CONDITION_PRUNED })); | ||
| }); | ||
|
|
||
| it('throws for a sealed condition below a NOT', () => { | ||
| const filters = new Filters(FilterCompoundOperator.AND, [ | ||
| new Filters(FilterCompoundOperator.NOT, [seal(eq('user.name'))]), | ||
| ]); | ||
|
|
||
| expect(() => pruneFiltersByRelations(filters, ['user'])) | ||
| .toThrowError(expect.objectContaining({ code: ErrorCode.SCHEMA_SEALED_CONDITION_PRUNED })); | ||
| }); | ||
|
|
||
| it('keeps pruning around a sealed condition it does not touch', () => { | ||
| const filters = new Filters(FilterCompoundOperator.AND, [ | ||
| seal(eq('realm_id')), | ||
| eq('user.name'), | ||
| eq('id'), | ||
| ]); | ||
|
|
||
| const output = pruneFiltersByRelations(filters, ['user']); | ||
| expect(filterFields(output)).toEqual(['realm_id', 'id']); | ||
| expect(output.value[0].sealed).toBe(true); | ||
| }); | ||
|
|
||
| it('re-applies an UNSEALED default naming a rejected relation', () => { | ||
| // the server-authored baseline is exempt from the gate, which is | ||
| // why the default fallback is not pruned. | ||
| const filters = new Filters(FilterCompoundOperator.AND, [eq('user.a')]); | ||
| const schema = defineFiltersSchema({ default: eq('user.b') }); | ||
|
|
||
| expect(filterFields(pruneFiltersByRelations(filters, ['user'], schema))).toEqual(['user.b']); | ||
| }); | ||
|
|
||
| it('throws for a SEALED default naming a rejected relation', () => { | ||
| // otherwise the same default would throw when it is materialized | ||
| // before this pass (client sent no filters) and survive when it is | ||
| // materialized after it (client sent filters that all pruned away). | ||
| const filters = new Filters(FilterCompoundOperator.AND, [eq('user.a')]); | ||
| const schema = defineFiltersSchema({ default: seal(eq('user.b')) }); | ||
|
|
||
| expect(() => pruneFiltersByRelations(filters, ['user'], schema)) | ||
| .toThrowError(expect.objectContaining({ code: ErrorCode.SCHEMA_SEALED_CONDITION_PRUNED })); | ||
| }); | ||
|
Comment on lines
+323
to
+332
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 12 \
'SCHEMA_SEALED_CONDITION_PRUNED|default: seal|defineFiltersSchema' \
packages/core/test/unit/parser/relation-prune.spec.tsRepository: tada5hi/rapiq Length of output: 8732 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== locate source definitions =="
rg -n "function pruneFiltersByRelations|export .*pruneFiltersByRelations|SCHEMA_SEALED_CONDITION_PRUNED|defineFiltersSchema" packages/core/src packages/core/test -g '*.ts' | sed -n '1,220p'
echo
echo "== relevant source snippets =="
for file in $(rg -l "pruneFiltersByRelations|SCHEMA_SEALED_CONDITION_PRUNED|defineFiltersSchema" packages/core/src packages/core/test -g '*.ts' | head -20); do
echo "--- $file ---"
rg -n -C 20 "pruneFiltersByRelations|SCHEMA_SEALED_CONDITION_PRUNED|defineFiltersSchema" "$file" | sed -n '1,220p'
doneRepository: tada5hi/rapiq Length of output: 44603 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
path = Path('packages/core/src/parser/relation-prune.ts')
text = path.read_text()
m = re.search(r'export function pruneFiltersByRelations[\s\S]+?^}', text, re.M)
print(m.group(0) if m else 'function not found')
print()
print('uses buildFiltersDefaults:')
print('buildFiltersDefaults' in (m.group(0) if m else ''))
print('check all calls to pruneFiltersByRelations in tracked TS:')
for p in Path('packages/core').glob('**/*.ts'):
if any(part.endswith(('.test', '.spec')) for part in p.parts):
lines = p.read_text(errors='ignore').splitlines()
for i, line in enumerate(lines, 1):
if 'pruneFiltersByRelations(' in line or 'buildFiltersDefaults(' in line:
print(f'{p}:{i}:{line.strip()}')
PYRepository: tada5hi/rapiq Length of output: 1448 Cover the no-client-filter sealed-default path. The existing case only prunes a client leaf and then checks the default fallback. Add the opposite call shape with an empty client filters tree, or cover that exact sealed default in another sealed-default test. [loweffort_and_high_reward] 🤖 Prompt for AI Agents |
||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exercise a sealed OR arm, not only a sealed OR group.
Line 286 calls
seal()on the entireORnode. The test does not verify that a sealeduser.namearm is rejected while the surroundingORremains unsealed. Moveseal()to the rejected arm, or rename the test and comments to describe a sealed group.Proposed test shape
📝 Committable suggestion
🤖 Prompt for AI Agents