fix: address Cube chart review feedback#7956
Conversation
WalkthroughThis PR synchronizes Cube.js configuration and query security across Helm chart and Docker environments. Changes expand tenant-level query filtering to cover multiple Cube dimensions (FeedbackRecords.tenantId and TopicsUnnested.tenantId), refine NPS score calculation to enforce value bounds, and enhance audit logging when security context validation fails. The Helm deployment template now validates that in-memory cache driver is only used with single replica, and pod configuration gains conditional support for security contexts and health probes. These updates consolidate tenant enforcement across the data schema and query layer. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/formbricks/templates/cube-deployment.yaml`:
- Around line 1-3: The check uses $cubeCacheDriver to compare against "memory"
but doesn't normalize casing/whitespace; update the assignment of
$cubeCacheDriver (the variable derived from get (.Values.cube.env)
"CUBEJS_CACHE_AND_QUEUE_DRIVER") to trim and lowercase the string before
comparison (e.g., pipe through trim and lower after toString) so the conditional
that enforces the memory/replica guard (the if using .Values.cube.enabled,
.Values.cube.replicas and eq $cubeCacheDriver "memory") correctly detects values
with accidental spaces or different casing, and leave the fail message
unchanged.
In `@docker/cube/cube.js`:
- Around line 145-157: The audit schema is inconsistent:
logCubeQuerySecurityContextFailure emits both errorName and errorMessage but
logCubeQueryAuditEvent only spreads errorName on error; update
logCubeQueryAuditEvent (the function that builds the audit event for cube.query)
to also include errorMessage when an Error (or other) is present so both fields
are emitted consistently — check where error is spread (errorName currently) and
add errorMessage similar to logCubeQuerySecurityContextFailure, ensuring you
stringify non-Error values the same way.
- Around line 181-185: The current unconditional injection of TENANT_MEMBERS
filters causes TopicsUnnested filters to be applied even on pure FeedbackRecords
queries; change the filter injection so each TENANT_MEMBERS entry is only added
when the target cube is actually present in the incoming query. Constrain the
mapping that produces the { member, operator, values } objects (the
TENANT_MEMBERS usage) to check the query's cubes (e.g., the list of requested
cubes—identify via the request/query context available where TENANT_MEMBERS is
used) and only return the TopicsUnnested filter when TopicsUnnested is included,
similarly only return FeedbackRecords filters when FeedbackRecords is included;
keep using context.tenantId for values. Ensure you reference TENANT_MEMBERS,
context.tenantId, TopicsUnnested and FeedbackRecords so the condition is
implemented at the source of the injected filters.
In `@docker/cube/schema/FeedbackRecords.js`:
- Around line 160-163: TopicsUnnested.tenantId is missing a description which
breaks docs consistency; add a descriptive description property to the tenantId
dimension in the TopicsUnnested cube (the tenantId definition) so it matches
other dimensions—e.g., set a clear one-line description like "Tenant identifier"
or similar that explains the field's meaning and usage in the
TopicsUnnested.tenantId dimension.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2f6ce3ce-5088-4564-9c86-6d66deaaa110
📒 Files selected for processing (7)
charts/formbricks/README.mdcharts/formbricks/cube/cube.jscharts/formbricks/cube/schema/FeedbackRecords.jscharts/formbricks/templates/cube-deployment.yamlcharts/formbricks/values.yamldocker/cube/cube.jsdocker/cube/schema/FeedbackRecords.js
|



Addresses CodeRabbit comments from #7955.
Changes:
Validation: