fix(entities): stop pushing field projection into DB query (#73)#78
Merged
Conversation
`entity list --fields ...` (and `entity query ... --fields`) returned `entities: []` / `count: 0` while `total_count` stayed non-zero whenever a record-access policy was active. `execute_entity_query` pushed the client `fields` projection down into the backend query as a column selection, so rows came back carrying only the requested columns. Each row is then reconstructed as a Cedar resource entity and validated in strict mode inside `filter_visible`. Required DSL fields are emitted as required Cedar attributes, so a projected row missing them (or missing any optional field a custom policy references) fails strict-mode validation; `authorize` returns Err and the row is silently dropped. Every row drops, but the independent COUNT(*) is unaffected — hence the mismatch. Remove the DB-level push-down. Authorization now sees complete entities, and the projection is applied purely as a presentation concern after filtering via the existing `fields.retain` step. `query.projection` stays available for internal sub-queries (display resolution, derived collections) that don't run user-facing record authorization. Adds a regression test that lists a row with a projection omitting a required field and asserts the row is still returned.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #73.
schemaforge entity list <Schema> --fields ...(andentity query ... --fields) returnedentities: []/count: 0whiletotal_countstayed non-zero whenever a record-access policy was active (the defaultCedarRecordPolicy).Root cause
execute_entity_querypushed the clientfieldsprojection down into the backend query as a SQL column selection. Rows came back carrying only the requested columns. Each row is then reconstructed as a Cedar resource entity and validated in strict mode insidefilter_visible(Entities::from_entities(.., Some(&schema))).Required DSL fields are emitted as required Cedar attributes:
A projected row missing a required field (or missing any optional field a custom Cedar policy references) fails strict-mode entity validation →
authorizereturnsErr→filter_visiblesilently drops the row via its catch-all arm. Every row drops, but the independentCOUNT(*)query is unaffected — producing the reportedtotal_count/entitiesmismatch with a 200 status and no error.Fix
Stop pushing the user
fieldsprojection into the DB query. Authorization now sees complete entities; the projection is applied purely as a presentation concern after filtering, via the already-presentfields.retain(...)step.query.projectionremains available for internal sub-queries (relation-display resolution, derived collections) that don't run user-facing record authorization.This also covers
entity query ... --fields(issue's open question), since both endpoints shareexecute_entity_query.Testing
list_field_projection_omitting_required_field_returns_rowsinauth_demo.rs: lists a row with?fields=name,notesthat omits a requiredcategoryfield and asserts the row is still returned (count == 1,total_count == 1). Red before the fix, green after.cargo clippy -p schema-forge-acton --all-targets: clean.Closes #73.