-
Notifications
You must be signed in to change notification settings - Fork 13
feat(convex): add Cerbos query plan adapter for Convex #140
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
Conversation
Converts Cerbos PlanResources responses into Convex-compatible filter
functions for use with ctx.db.query("table").filter(filterFn).
Supports eq, ne, lt, le, gt, ge, and, or, not, in (composed as OR of
eq), isSet, bare boolean operands, and nested field access via dot
notation. Throws for unsupported operators (contains, startsWith, etc).
Includes 27 unit tests with mock filter builder and 8 integration tests
against a self-hosted Convex backend via Docker Compose.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: Alex Olivier <[email protected]>
The integration test imports Convex generated types that only exist after codegen runs. Exclude it from tsconfig so the prepare build succeeds, and add an explicit codegen step in the integration test CI job. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Alex Olivier <[email protected]>
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e25432f8cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Convex treats null and undefined as distinct: null is a stored value, undefined means a missing field. The adapter was converting all nulls from Cerbos plans to undefined, which broke null comparisons. Now null passes through as-is. The isSet operator still correctly compares against undefined for field-existence checks. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Alex Olivier <[email protected]>
Instead of throwing for string/collection operators (contains, startsWith, endsWith, hasIntersection, exists, all, etc.), return a JS postFilter predicate alongside the DB filter. AND expressions are split: pushable children go to the DB filter, unsupported children to postFilter. OR expressions with any unsupported child go entirely to postFilter. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Alex Olivier <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Alex Olivier <[email protected]>
postFilter bypasses the DB-level authorization boundary since data is
fetched before filtering is applied. Default to throwing with a
descriptive error; callers must set { allowPostFilter: true } to opt in.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed-off-by: Alex Olivier <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Alex Olivier <[email protected]>
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e5ed1e339
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
convex/src/index.ts
Outdated
| const canPushToDb = (expression: PlanExpressionOperand): boolean => { | ||
| if (isValue(expression) || isVariable(expression)) return true; | ||
| if (!isExpression(expression)) return false; | ||
| if (!DB_PUSHABLE_OPERATORS.has(expression.operator)) return false; | ||
| return expression.operands.every(canPushToDb); |
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.
Reject non-literal operands in canPushToDb
When a policy compares two variables (e.g. request.resource.attr.owner == request.principal.id) or uses in with a variable array (e.g. request.resource.attr.group in request.principal.attr.groups), canPushToDb currently returns true because it only checks operator membership and recursively accepts any variables/values. That causes buildFilters to treat the expression as DB-pushable and call translateExpression, which then throws (requires one field and one value operand) instead of falling back to a postFilter (even when allowPostFilter is true). This makes common policy patterns fail at runtime. Consider tightening canPushToDb to validate operand shapes for eq/ne/lt/... and in (literal value/array vs variable) so non-pushable comparisons become post-filters instead of hard errors.
Useful? React with 👍 / 👎.
canPushToDb now validates that comparison operators (eq, ne, lt, le, gt, ge, in, isSet) have at least one variable and one literal value operand. Previously, expressions like `R.attr.owner == R.attr.manager` were incorrectly marked as DB-pushable, causing translateExpression to throw instead of falling back to postFilter. Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Alex Olivier <[email protected]>
Summary
@cerbos/orm-convexadapter that converts CerbosPlanResourcesresponses into Convex-compatible filter functions forctx.db.query("table").filter(filterFn)eq,ne,lt,le,gt,ge), logical (and,or,not), collection (invia composed OR),isSet, bare boolean operands, and nested field accesscontains,startsWith,endsWith, collection ops)Test plan
Closes #105