Skip to content

Commit

Permalink
catalog-backend: ensure subqueries are isolated from one another
Browse files Browse the repository at this point in the history
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 <mtlewis@users.noreply.github.com>
  • Loading branch information
mtlewis committed Nov 12, 2021
1 parent 0a66597 commit 06934f2
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/orange-experts-approve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-backend': patch
---

Adjust entity query construction to ensure sub-queries are always isolated from one another.
26 changes: 11 additions & 15 deletions plugins/catalog-backend/src/service/NextEntitiesCatalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 06934f2

Please sign in to comment.