From 06934f2f5275ad11882d1b060c3da874ff6df17e Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Fri, 12 Nov 2021 15:10:26 +0000 Subject: [PATCH] catalog-backend: ensure subqueries are isolated from one another Ensures that independent calls to `parseFilter` are isolated from one another, by always wrapping additional clauses to the query in `andWhere`. I think that the bug in the previous incarnation is strictly theoretical, as long as parseFilters is only called once for a given filter (which today, it is). When authorization lands in catalog-backend though, we'll need to call it multiple times - once to add the authz filters, and once to add the filters requested by the caller, which would expose this bug. Signed-off-by: Mike Lewis --- .changeset/orange-experts-approve.md | 5 ++++ .../src/service/NextEntitiesCatalog.ts | 26 ++++++++----------- 2 files changed, 16 insertions(+), 15 deletions(-) create mode 100644 .changeset/orange-experts-approve.md diff --git a/.changeset/orange-experts-approve.md b/.changeset/orange-experts-approve.md new file mode 100644 index 0000000000..13d1b022da --- /dev/null +++ b/.changeset/orange-experts-approve.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Adjust entity query construction to ensure sub-queries are always isolated from one another. diff --git a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts index 841e57c921..1c615f862a 100644 --- a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts @@ -131,29 +131,25 @@ function parseFilter( db: Knex, ): Knex.QueryBuilder { if (isEntitiesSearchFilter(filter)) { - return query.where(function filterFunction() { + return query.andWhere(function filterFunction() { addCondition(this, db, filter); }); } if (isOrEntityFilter(filter)) { - let cumulativeQuery = query; - for (const subFilter of filter.anyOf ?? []) { - cumulativeQuery = cumulativeQuery.orWhere(subQuery => - parseFilter(subFilter, subQuery, db), - ); - } - return cumulativeQuery; + return query.andWhere(function filterFunction() { + for (const subFilter of filter.anyOf ?? []) { + this.orWhere(subQuery => parseFilter(subFilter, subQuery, db)); + } + }); } if (isAndEntityFilter(filter)) { - let cumulativeQuery = query; - for (const subFilter of filter.allOf ?? []) { - cumulativeQuery = cumulativeQuery.andWhere(subQuery => - parseFilter(subFilter, subQuery, db), - ); - } - return cumulativeQuery; + return query.andWhere(function filterFunction() { + for (const subFilter of filter.allOf ?? []) { + this.andWhere(subQuery => parseFilter(subFilter, subQuery, db)); + } + }); } return query;