Skip to content

fix: cached GraphQL schema can result in missing members if used with access policies#10590

Merged
paveltiunov merged 7 commits intomasterfrom
cursor/rust-member-validation-16e8
Apr 1, 2026
Merged

fix: cached GraphQL schema can result in missing members if used with access policies#10590
paveltiunov merged 7 commits intomasterfrom
cursor/rust-member-validation-16e8

Conversation

@paveltiunov
Copy link
Copy Markdown
Member

@paveltiunov paveltiunov commented Mar 31, 2026

Check List

  • Tests have been run in packages where changes have been made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Issue Reference this PR resolves

Addresses the GraphQL schema caching issue causing intermittent 400s when different security contexts share a CompilerApi instance.

Description of Changes Made

Problem

The GraphQL schema was previously built from RBAC-filtered metadata and cached per CompilerApi instance. When multiple users with different RBAC roles shared the same CompilerApi (via contextToAppId), the first user's RBAC-filtered schema got cached and used for ALL users — causing intermittent 400 errors for fields that should be accessible.

The branch igor/core-384-investigate-graphql-schema-caching-causing-intermittent-400s proposed adding JS-side member validation in graphql.ts before calling load(). However, this validation is redundant because the Rust-side result transform layer can perform this check.

Solution

Instead of adding JS-side validation, this PR fixes the existing Rust-side validation in query_result_transform.rs to properly handle RBAC-denied members even when query results are empty.

Root cause of the Rust-side gap: When RBAC denies access, applyRowLevelSecurity adds a 1=0 filter that returns zero rows. With zero rows, QueryResult::from_js_raw_data produces empty columns. The Rust get_members() function previously short-circuited on db_data.columns.is_empty() without checking if requested members were actually accessible in the annotation.

Changes:

  1. query_result_transform.rs (Rust):

    • Extracted ensure_member_in_annotation() to deduplicate the hidden member check that was previously copy-pasted in get_members, get_vanilla_row, and the new validation function
    • Added validate_query_members_in_annotation() that checks the query's measures, dimensions, and time dimensions against the annotation map even when db_data.columns is empty (segments are excluded since the annotation map only contains measures, dimensions, and time dimensions)
    • Used ensure_member_in_annotation() in all three check sites
  2. gateway.ts: Build GraphQL schema from unfiltered metadata (skipVisibilityPatch: true) so it can be safely cached across all security contexts sharing a CompilerApi.

  3. CompilerApi.ts: Added skipVisibilityPatch option to metaConfig() to bypass RBAC visibility patching when building the shared GraphQL schema.

  4. Integration test: Added smoke-rbac-graphql.test.ts with test fixtures verifying:

    • All roles see the same unfiltered GraphQL schema (schema is properly shared)
    • RBAC-denied members return "You requested hidden member" errors at query time
    • Different roles get appropriate access
    • Complete denial (includes: []) blocks all member access
  5. CI: Added smoke:rbac-graphql to CI pipeline.

  6. Updated existing tests: smoke-rbac.test.ts tests that previously expected silent empty results for hidden members now correctly expect "You requested hidden member" errors.

How RBAC enforcement works end-to-end

  1. load() fetches metaConfig() with RBAC visibility patching → hidden members get isVisible: false
  2. filterVisibleItemsInMeta() strips items where isVisible === false
  3. prepareAnnotation() builds annotation from filtered metadata → hidden members are not in the annotation
  4. Rust's TransformedData::transform calls get_members which:
    • For non-empty results: checks each column's member against annotation via ensure_member_in_annotation
    • For empty results (RBAC denial with 1=0): calls validate_query_members_in_annotation to check the query's requested members
  5. If a member is missing from annotation → ensure_member_in_annotation throws "You requested hidden member" error
  6. Error propagates: Rust → getFinalResult() reject → handleError → GraphQL error response
Open in Web Open in Cursor 

…at query time

Instead of adding JS-side member validation for GraphQL RBAC enforcement,
rely on the existing Rust-side validation in query_result_transform.rs.

The Rust transform layer already checks if requested members are present
in the RBAC-filtered annotation map and throws 'You requested hidden member'
errors for inaccessible members.

Changes:
- Build GraphQL schema from unfiltered metadata (skipVisibilityPatch) so it
  can be safely cached across security contexts sharing a CompilerApi
- Add skipVisibilityPatch option to CompilerApi.metaConfig() to bypass
  RBAC visibility patching when building the shared GraphQL schema
- Add integration test verifying RBAC enforcement through GraphQL

Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
@github-actions github-actions bot added the javascript Pull requests that update Javascript code label Mar 31, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

❌ Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.88%. Comparing base (515eeae) to head (a8f6707).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...ackages/cubejs-server-core/src/core/CompilerApi.ts 16.66% 5 Missing ⚠️
packages/cubejs-api-gateway/src/gateway.ts 0.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (515eeae) and HEAD (a8f6707). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (515eeae) HEAD (a8f6707)
cubesql 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #10590       +/-   ##
===========================================
- Coverage   78.58%   57.88%   -20.71%     
===========================================
  Files         475      225      -250     
  Lines       92822    17581    -75241     
  Branches     3612     3614        +2     
===========================================
- Hits        72945    10176    -62769     
+ Misses      19333     6861    -12472     
  Partials      544      544               
Flag Coverage Δ
cube-backend 57.88% <14.28%> (-0.01%) ⬇️
cubesql ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@paveltiunov paveltiunov marked this pull request as ready for review March 31, 2026 23:32
@paveltiunov paveltiunov requested a review from a team as a code owner March 31, 2026 23:32
cursoragent and others added 2 commits March 31, 2026 23:46
Add the new RBAC GraphQL integration test to:
- packages/cubejs-testing/package.json as smoke:rbac-graphql script
- .github/actions/smoke.sh to run in integration-smoke CI job

Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
… with empty results

When RBAC denies access to a member, the query gets a '1=0' filter that
returns zero rows. Previously, the Rust get_members() function would
short-circuit on empty columns (derived from zero rows) without checking
if the requested members were actually accessible.

Now validate_query_members_in_annotation() checks the query's measures,
dimensions, segments, and time dimensions against the annotation map
even when db_data.columns is empty. This ensures 'You requested hidden
member' errors are returned for RBAC-denied members regardless of
whether the query returns data.

Also adds a test for the hidden member + empty dataset scenario.

Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
@github-actions github-actions bot added the rust Pull requests that update Rust code label Apr 1, 2026
cursoragent and others added 4 commits April 1, 2026 01:12
…dation

Segments are not included in the annotation map (which only contains
measures, dimensions, and time dimensions). The rlsAccessDenied
synthetic segment added by RBAC denial would incorrectly trigger the
hidden member error when validated against the annotation.

Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
Now that the Rust transform validates query members against the annotation
even with empty results, RBAC-denied members return 'You requested hidden
member' errors instead of silently returning empty data.

Updated tests:
- line_items hidden price_dim: expect error instead of empty result
- orders_view and cube with default policy: expect error for orders_view.count
  when user has no matching access policy on the view

Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
The orders_view_rest snapshot is no longer used after updating the test
to expect a hidden member error instead of empty results.

Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
…duplicate hidden member checks

The hidden member error was duplicated in three places:
- get_members (column-based check for non-empty results)
- get_vanilla_row (per-row alias check)
- validate_query_members_in_annotation (query member check for empty results)

Extract ensure_member_in_annotation() and use it in all three places.

Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
@paveltiunov paveltiunov changed the title fix(api-gateway): cache unfiltered GraphQL schema with RBAC enforced at query time via Rust fix(api-gateway): cached GraphQL schema can result in missing members if used with access policies Apr 1, 2026
@paveltiunov paveltiunov changed the title fix(api-gateway): cached GraphQL schema can result in missing members if used with access policies fix: cached GraphQL schema can result in missing members if used with access policies Apr 1, 2026
@paveltiunov paveltiunov merged commit c95317b into master Apr 1, 2026
74 of 76 checks passed
@paveltiunov paveltiunov deleted the cursor/rust-member-validation-16e8 branch April 1, 2026 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javascript Pull requests that update Javascript code rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants