fix(cubejs): include team.settings and member.properties in members fragment#43
Merged
fix(cubejs): include team.settings and member.properties in members fragment#43
Conversation
Cube.js v1.6 invokes checkSqlAuth as (request, user, password) — three
positional args — see
@cubejs-backend/api-gateway/dist/src/sql-server.js:291,105.
Our implementation declared (_, user) and did:
password = typeof user === "string" ? user : user?.password
username = typeof user === "string" ? _ : user?.username
With the v1.6 wire server, user arrives as a plain string (the Postgres
username), so the code took the username as the password AND used the
request metadata object as the username. findSqlCredentials then
received the {protocol, method, apiType} object, and Hasura rejected
the query with:
parsing Text failed, expected String, but encountered Object
path: $.selectionSet.sql_credentials.args.where.username._eq
Every SQL API login failed before any password comparison ran
(reproduced via `psql -U <valid> -h <cubejs>` → 28P01).
Fix: match the documented v1.6 signature and keep a defensive branch
for the legacy object-shape call. Also reject non-string username
early so the Hasura GraphQL layer cannot receive a non-string variable.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gins
queryRewrite's rule-based row filtering relies on
`securityContext.userScope.teamProperties` and `.memberProperties` to
look up per-rule property values (e.g. `partition` from team settings).
defineUserScope populates both from the member's team settings and
member properties. buildSqlSecurityContext (the SQL API path) never
did, so userScope for SQL logins had no team/member properties. Every
rule whose property lookup returned undefined blocked the whole query
and queryRewrite replaced query.filters with:
[{ member: allMembers[0], operator: "equals",
values: ["__blocked_by_access_control__"] }]
When the first member was a numeric measure (e.g. `count`), ClickHouse
tried to cast the sentinel to Float64:
Cannot parse string '__blocked_by_access_control__' as Float64
Fix: buildSqlSecurityContext now resolves the member for the
datasource's team and passes the team settings + member properties
into the scope (matching defineUserScope). Team settings also flow
into buildSecurityContext so the content hash includes them, keeping
cache isolation consistent between REST and SQL paths.
Reproduced via `SELECT count(*) FROM stockout_event` over the Postgres
wire — rule `semantic_events.partition` couldn't resolve
team.partition, blocked fired, sentinel filter crashed ClickHouse.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…agment Follow-up to #42. buildSqlSecurityContext now resolves teamProperties and memberProperties from sqlCredentials.user.members, but the GraphQL query used to load sql_credentials (sqlCredentialsQuery → membersFragment) never selected those fields. At runtime teamMember was found but team and properties were undefined, so teamProperties stayed empty and queryRewrite rules that look up teamProperties.<key> still blocked every query. Observed via: SELECT count(*) FROM stockout_event → still rewritten to: HAVING count(*) = toFloat64('__blocked_by_access_control__') Add team { id settings } and properties to membersFragment so the SQL API path has the same shape defineUserScope consumes on the REST path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
firsi
approved these changes
Apr 20, 2026
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
2 participants
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
Why #42 wasn't enough
#42 taught `buildSqlSecurityContext` to forward `teamMember.team.settings` and `teamMember.properties` into the scope — matching `defineUserScope` on the REST path. But the upstream GraphQL `membersFragment` consumed by `sqlCredentialsQuery` never selected `team` or `properties`, so at runtime:
```js
const teamMember = allMembers.find((m) => m.team_id === teamId); // ← found
teamMember.team // undefined (not in query)
teamMember.properties // undefined (not in query)
```
`teamProperties` stayed empty, `queryRewrite` couldn't resolve `partition` from team settings, `blocked = true` fired, and `SELECT count(*) FROM stockout_event` got rewritten to:
```
HAVING count(*) = toFloat64('blocked_by_access_control')
```
— same Float64 cast crash we saw before the #42 merge.
Fix
Add `team { id settings }` and `properties` to `membersFragment`. The REST path already fetches these via the `currentUser` query; the SQL path was the odd one out.
Test plan
🤖 Generated with Claude Code