[pull] master from cube-js:master#450
Merged
Merged
Conversation
…ies (#10803) * feat: conditional data masking with row-level filters in access policies When an access policy grants full member_level access with row_level filters, and another policy provides masked access, the masking should be conditional on the row filter. Previously, masking was skipped entirely when any policy granted full access, even if that access was restricted by row-level filters. Now, masked members with conditional full access generate: CASE WHEN {rowFilter} THEN {originalValue} ELSE {maskedValue} END This ensures that rows matching the row filter see unmasked values, while rows outside the filter range see masked values. Changes: - CompilerApi: Distinguish unconditional vs conditional full access when determining masking. Track row filters for conditionally accessible members as memberMaskFilters. - BaseQuery: Add conditionalMemberMaskSql() and maskFilterToSql() methods to generate CASE WHEN SQL for masked members with associated row filters. - NormalizedQuery type: Add memberMaskFilters field. - Tests: Add 4 new tests for conditional masking behavior. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * refactor: extend maskedMembers type to carry filter info inline Instead of a separate memberMaskFilters field, extend maskedMembers to support both string and {member, filter} object formats: maskedMembers: (string | { member: string, filter: FilterItem })[] This ensures Tesseract/native SQL planner also supports conditional masking. The Rust MaskedSqlNode now generates CASE WHEN SQL when a member has an associated mask filter. Changes: - CompilerApi: Emit maskedMembers with inline filter objects - BaseQuery.js: Parse both formats from maskedMembers array - Rust (base_query_options.rs): Add MaskedMemberItem enum (untagged) - Rust (query_tools.rs): Store mask filters alongside masked members - Rust (masked.rs): Generate CASE WHEN for conditional masks - Rust test fixtures: Update to use MaskedMemberItem type - NormalizedQuery type: Update maskedMembers type signature - query.js: Update Joi validation for new format - smoke-rbac.test.ts: Add conditional masking test case - conditional_masking_test.yaml: Add fixture for smoke test Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * fix: resolve lint, fmt, and stack overflow issues - Fix infinite recursion in conditionalMemberMaskSql by adding skipMaskFor context guard to prevent re-entrance - Fix CompilerApi logic: only apply conditional masking when a memberMasking policy actually exists (prevents false positive masking for cubes that only have row_level filters without memberMasking) - Fix lint: object-property-newline and object-curly-spacing in tests - Run cargo fmt on all Rust files Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * refactor: use standard filter pipeline for mask filter SQL generation BaseQuery.js: Replace custom and/or rendering in maskFilterToSql with the standard extractFiltersAsTree + initFilter + filterToWhere() pipeline that all other filters use. Rust MaskedSqlNode: Replace custom render_native_filter + render_filter_condition with FilterCompiler::add_item + FilterItem::to_sql using a standard VisitorContext - the same approach used by QueryProperties and Select for all other filter rendering. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * fix: resolve lint no-continue errors and Rust RefCell borrow panic - CompilerApi.ts: Replace continue statements with nested if/else to satisfy the no-continue lint rule - Rust MaskedSqlNode: Fix RefCell already borrowed panic by using VisitorContext::new_with_node_processor with self.input (the underlying EvaluateSqlNode) instead of SqlNodesFactory::new() which creates a node processor chain that includes MaskedSqlNode, causing re-entrant borrow when evaluating filter member SQL - Add VisitorContext::new_with_node_processor constructor for creating contexts with custom node processors Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * fix: prevent infinite recursion when filter member is also masked The stack overflow occurred when a mask filter referenced a member that was itself masked (e.g., product_id has a mask filter on product_id). The evaluateSymbolSql for the filter member would re-enter the masking code, creating infinite recursion. Fix: Use skipMasking context flag to disable all masking during CASE WHEN evaluation (both the filter SQL and the original value SQL). Also set currentMember to null to prevent memberChildren cycle tracking that caused hasMultiStageMembers infinite loop. Added test case that verifies no recursion when filter member is masked. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * refactor: use { member, filter? } struct for maskedMembers, remove validation - maskedMembers now always uses { member: string, filter?: FilterItem } objects instead of string | object alternatives - Remove maskedMembers from Joi query validation schema since users should never provide masking params in the query directly - Rust MaskedMemberItem: replace untagged enum with simple struct - Update all consumers and tests to use object format Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * fix: keep maskedMembers in Joi schema for internal query validation The normalizeQuery validation runs after applyRowLevelSecurity sets maskedMembers on the query, so the field must be allowed in the schema. Use Joi.array().items(Joi.object()) to accept the internal format without exposing strict typing to users. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * fix: prevent maskedMembers injection from user queries and rewrites - Strip maskedMembers from user query before first normalizeQuery validation so users cannot inject masking params - After queryRewrite, always restore maskedMembers from the post-RLS query, ensuring rewrites cannot override RLS-determined masking Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * fix: throw error if maskedMembers is provided in user query Instead of silently stripping maskedMembers, throw a UserError if the user attempts to pass it in their query. The Joi schema keeps the correct type definition for internal validation after RLS sets it. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * refactor: reuse standard CASE WHEN templates for conditional masking BaseQuery.js: Use caseWhenStatement() method (same template used by measure filters and dimension case types) instead of inline template string. Rust MaskedSqlNode: Use templates.case() from PlanSqlTemplates (same template used by CaseSqlNode for dimension case rendering) instead of inline format! string. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * refactor: use BaseGroupFilter for multiple mask filter clauses Instead of manually joining clauses with ' AND ', create a proper and-group filter via newGroupFilter when there are multiple filter items, reusing the standard BaseGroupFilter.filterToWhere() rendering. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * docs: add TODO for FILTER_PARAMS support in mask filter SQL Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * fix: OR across policies, AND within policy for mask filters When multiple policies grant conditional full access, the member should be unmasked when ANY policy's filter matches (OR), not when ALL match (AND). Within a single policy, multiple filters are still AND'd. Example with two policies: Policy A: filters = [region = 'RESEARCH', lock = 0] Policy B: filters = [region = 'DEMO'] Before: region = 'RESEARCH' AND lock = 0 AND region = 'DEMO' (always false) After: (region = 'RESEARCH' AND lock = 0) OR (region = 'DEMO') This is consistent with row-level security which uses union (OR) across policies for row visibility. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * test: add smoke test for OR across multiple conditional mask policies Adds a test that verifies when a user has two conditional mask roles: - conditional_mask_role: product_id <= 3 - conditional_mask_role_extra: product_id = 5 The price is unmasked when EITHER filter matches (OR across policies), confirming the fix for the AND vs OR bug. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> * refactor: remove unused memberPathArray parameter from conditionalMemberMaskSql Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )