[ovr]

No description provided.

[claude]

Claude finished @ovr's task in 2m 49s —— View job

---

Security fix looks correct; 1 high (docs), 1 low.

Review details

• Gather context (diff, changed files)
• Review code, security, tests, docs, comments
• Review-threads subagent
• Post findings

Findings

Severity	Location	Issue
High	packages/cubejs-api-gateway/src/query.ts:184	/v1/load now rejects subqueryJoins (query.ts:220), but docs-mintlify/api-reference/core-data.yaml:596 still documents it on V1LoadRequestQuery. This is resolved by a docs change or a top-level comment from the PR author. A reply on the thread doesn't count.
Low	packages/cubejs-api-gateway/src/gateway.ts:1879	The startsWith('{') check is copied in 4 places. Use one shared isSerializedMemberExpression helper instead.

Security

• Closed: REST queries could pass object-form parsedMemberExpression values in measures/dimensions/segments. hasExpressionsInQuery only detected {-prefixed strings, so these objects got past the memberExpressions flag. querySchema now rejects any object or {-prefixed string in those arrays.
• Closed: REST queries could send subqueryJoins[].sql, which is raw SQL. This was accepted whenever on was a plain member path. querySchema now forbids subqueryJoins entirely.
• No REST route sets memberExpressions; only sql-server.ts:249,279 does. /v1/sql and /v1/dry-run therefore use the strict schema too, and the tests cover both.
• normalize(rewrittenQuery, true) (gateway.ts:1620) only sees output from queryRewrite or row-level security. That query was already built from one that passed the strict first check, so it doesn't reopen the hole.
• stream() (gateway.ts:2469, SQL API streaming) uses the strict schema. cubesql never sends subqueryJoins (the field is skip_serializing_if = "Option::is_none") or expressions on that path, so I see no regression.

Correctness

Normalizing before parsing is safe because:

• asTimeDimension (query.ts:494) skips serialized expressions.
• memberPaths (gateway.ts:1879) skips serialized expressions.

The test keeps a serialized member expression whose SQL has two member references a dimension covers the case where a dotted JSON string would otherwise be split as a time dimension.

Tests

• I couldn't run them: the checkout has no dependencies installed (node_modules/.bin/jest is missing).
• From reading them, coverage is good: object, serialized and array-compiled members; blending and compare-date-range queries; /v1/sql, /v1/dry-run, GET params; subqueryJoins; and expressions added by queryRewrite.

Review threads

The listing returned 0 of my own threads, so there were no prior threads to resolve or re-affirm.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated
cubed3	🟢 Ready	View Preview	Nov 5, 2026, 3:40 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

[claude]

Claude finished @ovr's task in 2m 22s —— View job

---

LGTM: no new findings. One earlier low-priority thread is still open.

Review details

• Gather context (diff, changed files)
• Review code, tests, docs
• Resolve/dedupe prior review threads
• Post findings

What I checked

• Security fix holds. querySchema (packages/cubejs-api-gateway/src/query.ts:194) now rejects member expressions in measures, dimensions and segments, whether sent as objects or {…} strings. It also rejects subqueryJoins. Before this PR, a REST /v1/load body could send parsed member expressions (PatchMeasure or an expression array) because hasExpressionsInQuery only looked at strings. Only these callers pass true, so only they reach memberExpressionsQuerySchema:
  • sqlApiLoad, via sql-server.ts:249,279
  • sql() for the SQL API
  • preAggregationMatch
• Moving parseMemberExpressionsInQuery after normalize keeps the old behaviour:
  • asTimeDimension (query.ts:494) and memberPaths (gateway.ts:1879) skip {-prefixed strings, just as they used to skip parsed objects.
  • parseInputMemberExpression still validates the parsed shape.
  • The second normalize(…, true) checks the evaluated result against memberExpression/parsedMemberExpression.
• normalize(rewrittenQuery, true) is safe for REST (gateway.ts:1620). By then the user's input has passed the strict schema, so anything the permissive schema accepts came from RLS or queryRewrite.
• Tests cover:
  • object and serialized forms
  • GET query params
  • /v1/sql and /v1/dry-run
  • blending and compareDateRange queries
  • subqueryJoins
  • member expressions added by queryRewrite
  • SQL API parsing of SqlFunction, PatchMeasure and subquery-join on
• Docs: core-data.yaml no longer lists subqueryJoins, and its RequestId/Traceparent parameters match packages/cubejs-api-gateway/openspec.yml. REST member expressions were never documented, so the new rejection brings behaviour in line with the docs. The docs requirement is met.

I didn't run the tests: this checkout has no installed dependencies, and the sandbox wouldn't allow fetching origin/master. I diffed against f2412e3, and the diff stat matches the PR (+363/−47).

Review threads

The listing returned 2 of my own unresolved threads:

• Resolved 1 (PRRT_kwDOCOH19M60IpkB): the subqueryJoins docs fix is in commit 797ea46.
• Re-affirmed 1 (PRRT_kwDOCOH19M60IppS): startsWith('{') is still checked inline at gateway.ts:1879,1942,1959 and query.ts:494, with no shared helper. It's a low-priority cleanup.

[claude]

Claude finished @ovr's task in 2m 11s —— View job

---

LGTM: no blocking issues. 0 high, 0 medium, 1 low.

Review details

• Gather context (diff, changed files)
• Review code, tests, docs
• Reconcile prior review threads
• Post findings

Findings

• Low: packages/cubejs-api-gateway/src/gateway.ts:1946: this PR edits the signature of parseMemberExpressionsInQuery, but the method is still private. CLAUDE.md asks for protected. Posted inline.

Verified

• Who can send member expressions: memberExpressions is set only by sql-server.ts:249,279. That is the SQL API calling sqlApiLoad and sql. None of the HTTP routes (/load, /sql, /dry-run, /cubesql) reads it from the request. So /load, /sql and /dry-run always validate with the strict querySchema.
  • These endpoints reject object-form expressions (the PatchMeasure expressionName injection), serialized {…} strings and subqueryJoins. This holds for single, blending and compare-date-range queries.
  • The /cubesql endpoint takes SQL only; it never takes a JSON query.
• Normalize now runs before parse. Two spots could misread a serialized expression as a member path:
  • asTimeDimension (query.ts:498): ${Foo.a} + ${Foo.b} splits into 3 parts on ., which looks like cube.dimension.granularity.
  • memberPaths (gateway.ts:1880): the same string would reach the long-path resolver.
  • Both now skip serialized strings, and the SQL API test for a two-reference SqlFunction covers this.
• Validation of parsed expressions: the parsed SQL API expressions are no longer checked against parsedMemberExpression. Nothing is lost: parseInputMemberExpression already validates the raw input against inputMemberExpression, which requires cubeName/alias strings and a known expr type.
• After RLS and queryRewrite: expressions are parsed and evaluated before normalize(rewrittenQuery, true) (gateway.ts:1615-1621), so the post-rewrite check validates evaluated objects. This works the same way as before for queryRewrite hooks on /load that add member expressions, and the new test covers it.
• Tests cover every rejected form (object, serialized, compiled-array, timeDimensions, filters, subqueryJoins) and the SQL API paths that are still allowed (SqlFunction, PatchMeasure, subquery-join on, malformed path).

Docs

The only change users can see is that the REST API now rejects input it was never documented to accept, apart from subqueryJoins. This PR removes subqueryJoins from the REST reference (core-data.yaml, via extract-core-data.js). The docs requirement is met.

Review threads

The listing returned 1 of my own unresolved threads. I resolved it: the shared isSerializedMemberExpression helper landed in fbba56a. No threads re-affirmed. I posted 1 new inline comment.

[claude]

Claude finished @ovr's task in 2m 26s —— View job

---

Looks good to merge: 0 high, 0 medium, 1 low.

Review details

• Gather context
• Review code changes
• Review threads
• Post review

Findings

Severity	Location	Issue
Low	packages/cubejs-api-gateway/src/gateway.ts:1563	Expressions are not allowed in this context is thrown as a plain Error. A serialized member expression sent over REST therefore gets a 500 and an Internal Server Error log entry, while the same expression in object form gets a 400 UserError.

What I checked

• Security fix. Before this change, querySchema accepted parsedMemberExpression objects in measures, dimensions and segments on REST, GraphQL, /sql and /dry-run. Their expression arrays reach Function.constructor in evalMemberExpression. They now fail validation through memberPath → forbidden. Object-form subqueryJoins[].on is closed off too, because REST now forbids subqueryJoins entirely. filters[].member and timeDimensions[].dimension were already id-only.
• Only the SQL API gets memberExpressions: true. That is sql-server.ts:249 (sqlApiLoad) and sql-server.ts:279 (sql), plus preAggregationMatch (gateway.ts:2065), which needs the sql scope. /v1/cubesql rejects a memberExpressions body key, and a test covers that.
• Normalize now runs before parse (gateway.ts:1570). normalizeQuery sees the serialized strings, so asTimeDimension (query.ts:498) and memberPaths (gateway.ts:1880) skip them. Without that, {"…${Foo.a} + ${Foo.b}…"} could split into three parts and be taken for a time dimension, or go to the path resolver. A test covers this.
• Rewrites. Running the final normalize(rewrittenQuery, true) after RLS and queryRewrite is safe. The user's input has already passed the strict schema in the first stage, and only trusted config can add expressions after that.
• Docs. Removing subqueryJoins from the REST reference (extract-core-data.js) matches the new rejection. Object-form member expressions were never documented for REST, so no other docs need changing.
• Tests. The new tests are thorough. They cover object-form and serialized expressions on /load (GET and POST), /sql, /dry-run, blending, compare date range, subqueryJoins, expressions added by queryRewrite, and parsing on the SQL API side.
• Comments. All within the 3-line limit, and each one carries a reason.

Review threads

The listing returned 0 of my own unresolved threads: none resolved, none re-affirmed. The earlier subqueryJoins docs thread had already been resolved.

[claude]

Claude finished @ovr's task in 2m 19s —— View job

---

Changes requested: 1 high, 0 medium, 0 low. Since the last commit, the error from mixing member expressions into a /load query has a different text, so two tests still expect the old text and will fail.

Review details

• Gather diff and context
• Review code, tests, docs
• Review threads
• Post findings

Findings

• High, packages/cubejs-api-gateway/test/index.test.ts:1194 (also :990). a5dfa27 changed gateway.ts:1563 to throw UserError. handleError now responds with e.message and status 400, so the Error: prefix is gone. Both assertions that expect 'Error: Expressions are not allowed in this context' will fail. I couldn't run jest in this sandbox because the command needs approval. The Build workflow was still running when I posted this.

Checked, no issue

• Security: REST /load, /sql and /dry-run now reject object-form member expressions and subqueryJoins at schema level. Only sql-server.ts:249,279 sets memberExpressions: true, so the permissive schema can't be reached over HTTP. The REST-side expressionName injection is closed.
• Normalize-then-parse order: asTimeDimension (query.ts:498) and memberPaths (gateway.ts:1880) both skip serialized strings, so SQL API expressions never reach split('.') or resolveMemberPath.
• Second normalize with true (gateway.ts:1621): input from users has already passed the strict schema at that point. Only RLS or queryRewrite output gets the permissive schema.
• Docs: this is a security fix that removes input the docs never described for REST, apart from subqueryJoins. The PR drops subqueryJoins from core-data.yaml and adds a guard to extract-core-data.js so the reference stays in step. The docs requirement is met.
• Comments: all of them are within 3 lines and give reasons rather than restating the code.

Review threads

The listing returned no prior threads from me, so nothing was resolved or re-affirmed. 1 new inline comment posted.

[claude]

Claude finished @ovr's task in 1m 54s —— View job

---

LGTM. 0 high, 0 medium, 1 low (posted inline).

Review details

• Gather context (diff, changed files)
• Review code, tests, docs
• Review threads (resolve stale / dedupe)
• Post findings

Findings

• Low: packages/cubejs-api-gateway/src/query.ts:239. On the SQL API's first, pre-rewrite pass, memberExpressionsQuerySchema still accepts object-form member expressions. hasExpressionsInQuery only detects serialized strings, so those objects would skip parse/eval and assertSafeCompiledMemberExpression. Only cubesql can reach this pass today, so this is a defense-in-depth fix.

What I checked

• Security fix. On /load, /sql (HTTP), /dry-run and /subscribe, querySchema now rejects object-form and serialized member expressions in measures / dimensions / segments, along with subqueryJoins. Arrays such as ['Foo', 'return ...'] fail the id string check. timeDimensions[].dimension and filter member are already id-only. Blending and compareDateRange queries go through the same per-query normalize. Tests cover all of these paths.
• Parsing after normalize. Parsing now happens after normalize (gateway.ts:1570-1573). This matches the old behaviour: asTimeDimension and memberPaths skip serialized strings the same way they used to skip parsed objects. parseMemberExpressionsInQuery keeps the remaining fields because it spreads the normalized query.
• Post-rewrite normalize(…, true). This is correct, because RLS and queryRewrite can inject expressions into any query. The new queryRewrite segment test covers it.
• Error → UserError. "Expressions are not allowed in this context" now returns 400 instead of 500.
• Docs. subqueryJoins is removed from the REST reference (core-data.yaml) without touching openspec.yml, so cubeclient codegen still has the field. As far as I can tell, object-form member expressions were never documented for REST. The docs requirement is met.
• extract-core-data.js. It now copies the referenced components.parameters and fails hard if any $ref is missing. The 3-line comment on SQL_API_ONLY_PROPERTIES is within the limit.
• Tests. I didn't run them: node_modules isn't installed in this checkout.

Review threads

The listing returned 0 of my own threads (no prior threads), so nothing was resolved or re-affirmed. The new finding was posted.

[claude]

Claude finished @ovr's task in 1m 46s —— View job

---

LGTM: 0 high, 0 medium, 1 low, a hardening suggestion only.

Review details

• Gather context (diff, changed files)
• Review code, tests, docs
• Reconcile prior review threads
• Post findings

Findings

• Low: packages/cubejs-api-gateway/src/query.ts:232. On a memberExpressions query the first validation pass still accepts object-form memberExpression / parsedMemberExpression, even though cubesql only sends serialized strings. hasExpressionsInQuery only detects serialized strings, so an object form that got past this pass would skip parse/eval. You can't reach this from REST, but the first pass could accept only path | serializedMemberExpression (inline comment).

Verified

• Against /load, /sql and /dry-run, including blending and compare-date-range queries, querySchema now rejects object-form expressions in measures / dimensions / segments, compiled array-form expressions, and subqueryJoins. That closes the PatchMeasure.expressionName injection path.
• Order of normalize and parsing: serialized strings are skipped by asTimeDimension (query.ts:498) and by memberPaths (gateway.ts:1880), so a JSON string with dots is never treated as a granularity or as a long member path.
• queryRewrite / RLS output is normalized with allowMemberExpressions = true (gateway.ts:1621). That input is already validated, so this is server-side code adding expressions and the client can't smuggle anything through it. A test covers it.
• Expressions are not allowed in this context is now a UserError, so it becomes a 400 instead of a 500. The test expectation is updated to match.
• Tests cover each endpoint, the GET query param, blending and compare queries, timeDimensions / filters, /v1/cubesql, and the SQL API parse paths (SqlFunction, PatchMeasure, subqueryJoins.on).
• Docs: subqueryJoins was SQL API-only and is now removed from the REST reference (extract-core-data.js, core-data.yaml). Object-form member expressions were never documented for REST. No more docs are needed.
• Comments: all are within 3 lines and say why, not what.

Not run

I didn't run the Jest suite: the cd … && npx jest command needed approval in this environment.

Review threads

The listing returned 0 of my own unresolved threads (no prior threads), so I resolved none and re-affirmed none. 1 new inline comment posted.

[claude]

Low (hardening): the first validation pass of a memberExpressions query still accepts object-form memberExpression / parsedMemberExpression. cubesql only ever sends serialized {...} strings in measures / dimensions / segments. RLS and queryRewrite are the only things that produce object forms, and they go through the second pass (normalize(rewrittenQuery, true), gateway.ts:1621).

There's also a gap: hasExpressionsInQuery (gateway.ts:1935) only looks for serialized strings. So an object-form parsedMemberExpression that reached the first pass would skip parseMemberExpressionsInQuery / evalMemberExpressionsInQuery and get to the compiler with no eval. You can't reach this from REST today because memberExpressions comes only from the SQL server. Still, splitting the schema closes it at the source:

• first pass: Joi.alternatives(path, serializedMemberExpression)
• second pass (after rewrite): the current alternatives, object forms included

For example, use one schema for parsing input and a separate rewrittenQuerySchema for normalize(rewrittenQuery, true).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.28%. Comparing base (9a7337c) to head (bf5b041).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #12626      +/-   ##
==========================================
+ Coverage   66.22%   66.28%   +0.05%     
==========================================
  Files         258      258              
  Lines       22023    22042      +19     
  Branches     5308     5311       +3     
==========================================
+ Hits        14585    14610      +25     
+ Misses       6640     6638       -2     
+ Partials      798      794       -4     

Flag	Coverage Δ
cube-backend	66.28% <100.00%> (+0.05%)	⬆️

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

☔ View full report in Codecov by Harness.
📢 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.