From e6892e876715b4d051cd7c89022ed2887b045cfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 13 May 2026 15:47:19 +0200 Subject: [PATCH] fix(filter-predicates): reject operator keys mixed with other keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, a predicate like `{ kind: 'API', $not: { 'spec.type': 'dataset' } }` was silently accepted — the `kind` check was dropped entirely because `evaluateFilterPredicate` short-circuited on the `$not` operator. This made the filter match any non-dataset entity regardless of kind, which is incorrect. The correct form requires wrapping in `$all`. Both the runtime evaluator and the zod validation schemas now reject predicates that mix `$all`/`$any`/`$not` with other keys. Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Fredrik Adelöw --- .../src/predicates/evaluate.test.ts | 22 +++++++++++++++++++ .../src/predicates/evaluate.ts | 16 ++++++++++++++ .../src/predicates/schema.test.ts | 6 +++++ .../src/predicates/schema.ts | 12 +++++----- 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/packages/filter-predicates/src/predicates/evaluate.test.ts b/packages/filter-predicates/src/predicates/evaluate.test.ts index 6eb54c3c92..946e4e0600 100644 --- a/packages/filter-predicates/src/predicates/evaluate.test.ts +++ b/packages/filter-predicates/src/predicates/evaluate.test.ts @@ -258,6 +258,28 @@ describe('evaluate', () => { }); }); + it('throws when operator keys are mixed with other keys', () => { + const entity = entities[0]; + expect(() => + evaluateFilterPredicate( + { kind: 'API', $not: { 'spec.type': 'dataset' } } as any, + entity, + ), + ).toThrow(/Operator \$not must be the only key/); + expect(() => + evaluateFilterPredicate( + { kind: 'API', $all: [{ 'spec.type': 'service' }] } as any, + entity, + ), + ).toThrow(/Operator \$all must be the only key/); + expect(() => + evaluateFilterPredicate( + { kind: 'API', $any: [{ 'spec.type': 'service' }] } as any, + entity, + ), + ).toThrow(/Operator \$any must be the only key/); + }); + it('handles unknown filter predicate operators and matchers', () => { const operator = { $unknown: 'foo' } as unknown as FilterPredicate; const value = { kind: { $unknown: 'foo' } } as unknown as FilterPredicate; diff --git a/packages/filter-predicates/src/predicates/evaluate.ts b/packages/filter-predicates/src/predicates/evaluate.ts index 2006265004..f616dec058 100644 --- a/packages/filter-predicates/src/predicates/evaluate.ts +++ b/packages/filter-predicates/src/predicates/evaluate.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { InputError } from '@backstage/errors'; import { JsonValue } from '@backstage/types'; import { FilterPredicate, FilterPredicateValue } from './types'; import { getJsonValueAtPath } from './getJsonValueAtPath'; @@ -36,12 +37,27 @@ export function evaluateFilterPredicate( } if ('$all' in predicate) { + if (Object.keys(predicate).length !== 1) { + throw new InputError( + 'Operator $all must be the only key in the predicate, wrap in $all to combine with other conditions', + ); + } return predicate.$all.every(f => evaluateFilterPredicate(f, value)); } if ('$any' in predicate) { + if (Object.keys(predicate).length !== 1) { + throw new InputError( + 'Operator $any must be the only key in the predicate, wrap in $all to combine with other conditions', + ); + } return predicate.$any.some(f => evaluateFilterPredicate(f, value)); } if ('$not' in predicate) { + if (Object.keys(predicate).length !== 1) { + throw new InputError( + 'Operator $not must be the only key in the predicate, wrap in $all to combine with other conditions', + ); + } return !evaluateFilterPredicate(predicate.$not, value); } diff --git a/packages/filter-predicates/src/predicates/schema.test.ts b/packages/filter-predicates/src/predicates/schema.test.ts index ff38dabfce..422fc42c3d 100644 --- a/packages/filter-predicates/src/predicates/schema.test.ts +++ b/packages/filter-predicates/src/predicates/schema.test.ts @@ -108,6 +108,9 @@ describe('createZodV3FilterPredicateSchema', () => { { $not: { $all: [{ x: { $unknown: true } }] } }, { $unknown: 'foo' }, { 'metadata.tags': ['foo', 'bar'] }, + { kind: 'API', $not: { 'spec.type': 'dataset' } }, + { kind: 'API', $all: [{ 'spec.type': 'service' }] }, + { kind: 'API', $any: [{ 'spec.type': 'service' }] }, ]; it.each(predicates)('should reject invalid predicate %j', predicate => { @@ -207,6 +210,9 @@ describe('parseFilterPredicate', () => { { $unknown: 'foo' }, { kind: { $hasPrefix: 1 } }, { 'metadata.tags': ['foo', 'bar'] }, + { kind: 'API', $not: { 'spec.type': 'dataset' } }, + { kind: 'API', $all: [{ 'spec.type': 'service' }] }, + { kind: 'API', $any: [{ 'spec.type': 'service' }] }, ]; it.each(predicates)( diff --git a/packages/filter-predicates/src/predicates/schema.ts b/packages/filter-predicates/src/predicates/schema.ts index 8526748472..7d30f9317a 100644 --- a/packages/filter-predicates/src/predicates/schema.ts +++ b/packages/filter-predicates/src/predicates/schema.ts @@ -53,9 +53,9 @@ export function createZodV3FilterPredicateSchema( z.union([ expressionSchema, primitiveSchema, - z.object({ $all: z.array(predicateSchema) }), - z.object({ $any: z.array(predicateSchema) }), - z.object({ $not: predicateSchema }), + z.object({ $all: z.array(predicateSchema) }).strict(), + z.object({ $any: z.array(predicateSchema) }).strict(), + z.object({ $not: predicateSchema }).strict(), ]), ) as zodV3.ZodType; @@ -104,9 +104,9 @@ export function createZodV4FilterPredicateSchema(): zodV4.ZodType< z.union([ expressionSchema, primitiveSchema, - z.object({ $all: z.array(predicateSchema) }), - z.object({ $any: z.array(predicateSchema) }), - z.object({ $not: predicateSchema }), + z.object({ $all: z.array(predicateSchema) }).strict(), + z.object({ $any: z.array(predicateSchema) }).strict(), + z.object({ $not: predicateSchema }).strict(), ]), ) as zodV4.ZodType;