feat(api-gateway): accept a list of API secrets for verification

[bsod90]

Adds CUBEJS_API_SECRETS (comma-separated) as a sibling of CUBEJS_API_SECRET. When set, CUBEJS_API_SECRETS takes precedence: the gateway tries each secret in the list when verifying a JWT and accepts the token if any of them matches the signature. Signing behavior is unchanged — callers still sign with whatever single secret they hold.

This enables zero-downtime rotation of the API secret. During a rotation window operators can publish the outgoing, current, and incoming secrets together in CUBEJS_API_SECRETS; in-flight tokens signed by the outgoing secret keep verifying until they expire or are reissued.

• New optional apiSecrets: string[] on ApiGatewayOptions and CreateOptions (server-core).
• createDefaultCheckAuth iterates apiSecrets when present; otherwise falls back to the singular apiSecret. An explicit options.key (used by the playground/system auth path) still wins, so playground behavior is unchanged.
• JWK auth is unaffected — when jwkUrl is configured the secret is fetched dynamically from JWKS, so iterating a static list would be meaningless.
• Cube.apiSecrets() static accessor mirrors Cube.apiSecret().

Tests cover: accept any listed secret, reject unlisted, list takes precedence over singular, empty list falls back, playground path isolated.

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

[claude]

Claude finished @bsod90's task in 2m 10s —— View job

---

Review — feat(api-gateway): accept a list of API secrets

Verdict: LGTM with minor suggestions. 0 high, 1 medium, 3 low. Implementation is clean, well-scoped, and well-tested. Iterating candidate secrets for HS-style verification is the right primitive for zero-downtime rotation, and the carve-outs (options.key wins for the playground/system path, JWK short-circuits the loop) are correct.

Details (todos, findings, suggestions)

Todos

• Read all changed files & related context (createDefaultCheckAuth, system-auth path, JWK branch)
• Analyze correctness, security, perf, tests
• Post inline findings + summary

Findings

Medium

• Error masking across candidates — gateway.ts:2551–2563. Only lastError is bubbled up. A TokenExpiredError / audience-mismatch from candidate ReferenceError: self is not defined - Issue with whatwg-fetch when using the core lib with Node.js #1 gets overwritten by the generic "invalid signature" from candidate #N, so the 403 message becomes less actionable as the rotation list grows. Prefer non-JsonWebTokenError errors over signature failures when picking the error to surface. (Posted inline.)

Low

• (string | undefined)[] + as string cast — gateway.ts:2536–2540, 2553. Behavior matches master (which also passed undefined to jwt.verify when nothing was configured), but the cast hides an unconfigured-state branch. Either narrow the type to string[] or comment the no-secret branch explicitly. (Inline.)
• Duplicated env parsing — OptsHandler.ts:471–473 and cubejs-server/src/server.ts:219–225 both implement split(',') / trim / filter(Boolean) / empty→undefined. Worth extracting a shared helper. (Inline.)
• No de-duplication of secrets — OptsHandler.ts:471–473. CUBEJS_API_SECRETS="s, s, other" silently doubles verify work on every failing token. Array.from(new Set(...)) would fix it. (Inline.)

Other observations

• JWK interaction — correctly handled: isJWK breaks after the first attempt because checkAuthFn ignores the passed secret in JWK mode. The PR description acknowledges this; the code matches.
• Playground / system auth isolation — options.key always wins (gateway.ts:2536–2540), and createCheckAuthSystemFn (line 2608) keeps using playgroundAuthSecret. The dedicated test (apiSecrets - playground secret path is unaffected) locks this in. 👍
• Perf — worst-case N HS256 verifies per invalid token; negligible. For RS256/ES256 the JWK short-circuit prevents N-fold cost.
• Security — multi-secret verification is the standard rotation pattern; no new timing-side-channel concerns beyond the single-secret case.
• Docs — CUBEJS_API_SECRETS isn't yet added to docs/content/product/configuration/reference/environment-variables.mdx (the PR's "Docs updated" box is unchecked). Worth a follow-up entry next to CUBEJS_API_SECRET documenting the rotation use case and precedence rule.
• Tests — coverage is solid: positive (any secret in list), negative (none match), precedence over singular, empty-list fallback, playground isolation. Nice. Could optionally add: behavior when both apiSecrets and jwkUrl are set (likely document-only — current behavior is "JWK wins, list ignored"), and an explicit assertion that enforceSecurityChecks=false still allows requests through when no secret in the list matches.

· branch: `api-gateway-multi-secret`

[claude]

When verification fails for all candidates, only lastError is reported — but for a truly invalid token, every candidate produces a similar JsonWebTokenError: invalid signature and the last one is fine. The concerning case is when the token is structurally valid but failed for a non-signature reason (TokenExpiredError, audience/issuer mismatch). Today such an error from secret #1 is overwritten by an "invalid signature" from secret #N, masking the real cause and degrading the 403 message. Consider preferring a non-JsonWebTokenError (e.g. TokenExpiredError, NotBeforeError) over a generic signature failure when picking which error to bubble up.

[claude]

Claude finished @bsod90's task in 37s —— View job

---

Review — feat(api-gateway): accept a list of API secrets

Verdict: LGTM with minor suggestions. 0 high, 1 medium, 3 low. Re-affirmed 4 prior unresolved threads — the diff is unchanged since the previous review (still a single commit 4c716e9), so no new inline comments were posted.

Details (todos, findings)

Todos

• Re-fetch diff vs origin/master and confirm scope unchanged
• List existing review threads and deduplicate
• Re-affirm prior findings; skip duplicate inline posts

Findings (still applicable)

Medium

• Error masking across candidates — gateway.ts:2555. Only lastError is bubbled; a TokenExpiredError / audience-mismatch from candidate ReferenceError: self is not defined - Issue with whatwg-fetch when using the core lib with Node.js #1 gets overwritten by a generic "invalid signature" from candidate #N. Prefer non-JsonWebTokenError errors over signature failures when picking which to surface.

Low

• (string | undefined)[] + as string cast — gateway.ts:2533. Behavior matches master, but the cast hides an unconfigured-state branch. Narrow the type or document the no-secret branch.
• Duplicated env parsing — OptsHandler.ts:471–473 and cubejs-server/src/server.ts:219–225 both implement split(',') / trim / filter(Boolean) / empty→undefined. Extract a shared helper.
• No de-duplication of secrets — OptsHandler.ts:473. CUBEJS_API_SECRETS="s, s, other" silently doubles verify work. Array.from(new Set(...)) would fix it.

Other observations

• JWK interaction correctly handled (loop short-circuits since checkAuthFn ignores the passed secret in JWK mode).
• Playground / system auth isolation preserved by options.key precedence and createCheckAuthSystemFn continuing to use playgroundAuthSecret.
• Perf: worst-case N HS256 verifies per invalid token — negligible.
• Security: standard rotation pattern; no new side-channel concerns.
• Docs: CUBEJS_API_SECRETS still not added to docs/content/product/configuration/reference/environment-variables.mdx. Worth a follow-up entry next to CUBEJS_API_SECRET.
• Tests: solid coverage. Optional additions — behavior when apiSecrets and jwkUrl are both set, and enforceSecurityChecks=false fallthrough when no listed secret matches.

· branch: api-gateway-multi-secret ·

[claude]

Claude finished @bsod90's task in 1m 33s —— View job

---

Review — feat(api-gateway): accept a list of API secrets

Verdict: LGTM. 0 high, 0 medium, 1 low. New commit cd792cf addresses all four prior threads — type narrowed to string[], env parsing extracted to a shared parseApiSecretsEnv helper with de-duplication, and the verification loop now breaks early on TokenExpiredError / NotBeforeError / JWK so token-level errors aren't shadowed. Resolved the 3 corresponding stale threads.

Details

Todos

• Re-fetch diff and identify changes vs prior review
• Verify each prior finding is addressed in the new diff
• Resolve stale claude threads where the concern no longer applies
• Note any residual or new issues

What changed since the prior review

• gateway.ts:2529-2533 — candidateSecrets is now string[]; the [] branch when nothing is configured is explicit, and the as string cast is gone.
• gateway.ts:2543-2561 — loop breaks early on TokenExpiredError, NotBeforeError, or isJWK, so those errors reach the 403 instead of being overwritten by a later "invalid signature".
• packages/cubejs-server-core/src/core/apiSecretsEnv.ts (new) — shared parseApiSecretsEnv(raw): split / trim / filter / dedupe (new Set) / empty→undefined. Used by both OptsHandler.ts:472 and server.ts:216.
• apiSecretsEnv.test.ts (new) — covers unset, empty, all-blank, trim/filter, dedupe-with-order, single value.

Residual (Low)

• Other JsonWebTokenError subtypes still get masked — gateway.ts:2553-2559. The early-break list covers TokenExpiredError and NotBeforeError, but jwt.verify also throws JsonWebTokenError for audience/issuer/subject mismatch ("jwt audience invalid", "jwt issuer invalid", etc.). Those still get overwritten by a later candidate's "invalid signature". Could be addressed by checking e?.message against the non-signature messages, or — simpler — by remembering the first error and surfacing it if every subsequent error is "invalid signature". Not blocking; the main expired-token case is now handled.

Other observations

• JWK correctly short-circuits via the new isJWK break.
• De-dup preserves first-seen order — the test pins this; matches the "outgoing, current, incoming" rotation mental model.
• Playground/system path still isolated (options.key wins, createCheckAuthSystemFn unchanged).
• Docs — CUBEJS_API_SECRETS still not added to docs/content/product/configuration/reference/environment-variables.mdx; the PR's "Docs updated" box remains unchecked. Worth a follow-up entry beside CUBEJS_API_SECRET documenting precedence and the rotation use case.
• Tests — coverage looks complete for the helper and the gateway flow. Optional additions still: apiSecrets + jwkUrl together (current behavior: JWK wins, list ignored — confirm with a test), and an enforceSecurityChecks=false fallthrough assertion when no listed secret matches.

· branch: `api-gateway-multi-secret`

[claude]

Claude finished @bsod90's task in 56s —— View job

---

Review — feat(api-gateway): accept a list of API secrets

Verdict: LGTM. 0 high, 0 medium, 1 low (residual). No diff changes since the prior review at 16:56:05Z — the branch is still a single commit (16d5427) and the file contents match what was reviewed there. Re-affirming the one remaining open thread; no new inline comments posted.

Details

Todos

• Fetch current diff vs origin/master and compare against prior review state
• List existing review threads and dedupe against prior claude comments
• Re-affirm the one unresolved thread; skip duplicate inline posts

State

• Branch head: 16d5427 (single commit).
• Resolved (outdated) claude threads: (string|undefined)[] cast, duplicated env parsing, no de-dup of secrets — all addressed by the parseApiSecretsEnv helper and the candidateSecrets: string[] rewrite.
• Open claude thread: error masking across candidates (gateway.ts:2566). The current code at gateway.ts:2558-2564 now breaks early on TokenExpiredError, NotBeforeError, and JWK, which covers the main case from that thread. What remains is the residual below.

Residual (Low)

• Other JsonWebTokenError subtypes still masked — gateway.ts:2554-2565. jwt.verify also throws plain JsonWebTokenError for "jwt audience invalid", "jwt issuer invalid", "jwt subject invalid", "jwt id invalid", etc. With multiple candidates, those still get overwritten by a later candidate's "invalid signature". Could be addressed by either checking e?.message against the non-signature messages, or — simpler — keeping the first non-invalid signature error seen and surfacing it if every subsequent error is a plain signature failure. Not blocking.

Other observations

• Playground / system path still isolated (options.key precedence + createCheckAuthSystemFn unchanged).
• De-dup preserves first-seen order — covered by apiSecretsEnv.test.ts.
• Docs — CUBEJS_API_SECRETS still not added to docs/content/product/configuration/reference/environment-variables.mdx; the PR's "Docs updated" box remains unchecked. Worth a follow-up next to CUBEJS_API_SECRET documenting precedence and the rotation use case.
• Tests — solid. Optional additions still: apiSecrets + jwkUrl together (current behavior: JWK wins, list ignored), and an enforceSecurityChecks=false fallthrough when no listed secret matches.

· branch: api-gateway-multi-secret ·

[codecov]

Codecov Report

❌ Patch coverage is 82.14286% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.54%. Comparing base (5f3cb69) to head (d309898).
⚠️ Report is 15 commits behind head on master.

Files with missing lines	Patch %	Lines
packages/cubejs-api-gateway/src/gateway.ts	84.21%	2 Missing and 1 partial ⚠️
packages/cubejs-server/src/server.ts	0.00%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (5f3cb69) and HEAD (d309898). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (5f3cb69)	HEAD (d309898)
cubesql	1	0

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #10985       +/-   ##
===========================================
- Coverage   78.75%   58.54%   -20.22%     
===========================================
  Files         470      216      -254     
  Lines       93902    17175    -76727     
  Branches     3466     3501       +35     
===========================================
- Hits        73953    10055    -63898     
+ Misses      19446     6610    -12836     
- Partials      503      510        +7     

Flag	Coverage Δ
cube-backend	58.54% <82.14%> (+0.11%)	⬆️
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.

[claude]

Claude finished @bsod90's task in 1m 11s —— View job

---

Review — feat(api-gateway): accept a list of API secrets

Verdict: LGTM. 0 high, 0 medium, 1 low (residual). Branch was squashed/force-pushed to a single commit 1c0118c; content matches the prior LGTM state and now also adds the CUBEJS_API_SECRETS docs entry. Re-affirmed the one open thread; no new inline comments.

Details

Todos

• Re-fetch diff vs origin/master and identify changes since prior review
• List existing review threads and dedupe against prior claude comments
• Re-affirm the one unresolved thread; skip duplicate inline posts

State

• Branch head: 1c0118c (single commit, force-pushed since the previous review at 16d5427).
• Diff scope is the same 12 files / +266/-8 reviewed previously: gateway loop, parseApiSecretsEnv helper, OptsHandler + server.ts wiring, types, Joi schema, Cube.apiSecrets() accessor, tests.
• Docs now added — docs/content/product/configuration/reference/environment-variables.mdx includes a CUBEJS_API_SECRETS entry next to CUBEJS_API_SECRET documenting precedence, trim/dedupe, and the rotation use case. This closes the long-standing "Docs updated" gap.
• Resolved (outdated) claude threads from earlier rounds: (string|undefined)[] cast, duplicated env parsing, no de-dup of secrets — all addressed by parseApiSecretsEnv and the candidateSecrets: string[] rewrite.
• Open claude thread: error masking across candidates (gateway.ts:2566). The current loop at gateway.ts:2543-2570 breaks early on TokenExpiredError, NotBeforeError, and JWK, which covers the main case. What remains is the residual below.

Residual (Low)

• Other JsonWebTokenError subtypes still masked — gateway.ts:2553-2566. jwt.verify also throws plain JsonWebTokenError for "jwt audience invalid", "jwt issuer invalid", "jwt subject invalid", "jwt id invalid". With multiple candidates, those still get overwritten by a later candidate's "invalid signature". Could be addressed by either checking e?.message against the non-signature messages, or — simpler — by keeping the first non-invalid signature error seen and surfacing it if every subsequent error is a plain signature failure. Not blocking.

Other observations

• JWK correctly short-circuits via the isJWK break.
• De-dup preserves first-seen order — pinned by apiSecretsEnv.test.ts and matches the "outgoing, current, incoming" rotation mental model.
• Playground / system path still isolated (options.key precedence + createCheckAuthSystemFn unchanged).
• Tests — solid coverage on the helper and the gateway flow (accept any listed, reject unlisted, list-over-singular, empty-list fallback, playground isolation). Optional additions still: apiSecrets + jwkUrl together (current behavior: JWK wins, list ignored — worth a confirming test), and an enforceSecurityChecks=false fallthrough assertion when no listed secret matches.
• Codecov flags 3 uncovered lines (1 in gateway.ts, the Cube.apiSecrets() static in server.ts). The static accessor is trivial; consider a one-liner test if you want clean coverage.

· branch: api-gateway-multi-secret ·

[claude]

Claude finished @bsod90's task in 1m 27s —— View job

---

Review — feat(api-gateway): accept a list of API secrets

Verdict: LGTM. 0 high, 0 medium, 1 low (residual, unchanged). New commit 16f3041 refactors the multi-secret loop into the default checkAuthFn itself, simplifying the returned handler to a plain try/catch. JWK path naturally short-circuits because it overwrites checkAuthFn. No new inline comments — the one open thread still applies as the residual below.

Details

What changed since the prior review (1c0118c → 16f3041)

• gateway.ts:2463-2510 — candidateSecrets resolution and the iteration loop moved from the returned async handler into the default checkAuthFn. Early-break on TokenExpiredError / NotBeforeError preserved.
• gateway.ts:2530-2561 — JWK branch unchanged in spirit but now cleanly replaces checkAuthFn, so the candidate loop is bypassed without needing an isJWK flag in the outer handler.
• gateway.ts:2564-2577 — outer handler shrunk to a simple try { await checkAuthFn(auth) } catch { 403 if enforceSecurityChecks }. VerifyTokenFn signature simplified to (auth) => ... since the secret is now captured by closure.
• No behavior change in tests; no other files touched.

Correctness check

• User-provided options.checkAuth still routes through createCheckAuthFn / wrapCheckAuth (gateway.ts:2585-2588), not through the refactored default — unaffected.
• Playground / system path still wins via options.key precedence (gateway.ts:2476-2477); createCheckAuthSystemFn unchanged.
• No-secret branch preserved: when candidateSecrets is empty, falls through to verifyToken(auth, undefined) — matches prior master behavior of letting jwt.verify raise.
• JWK correctly bypasses the loop by overwriting checkAuthFn after it's defined.

Residual (Low, unchanged from prior review)

• Other JsonWebTokenError subtypes still masked — gateway.ts:2493-2502. The early-break catches TokenExpiredError and NotBeforeError, but plain JsonWebTokenError for "jwt audience invalid", "jwt issuer invalid", "jwt subject invalid", "jwt id invalid" still gets overwritten by a later candidate's "invalid signature". Simpler fix: keep the first non-"invalid signature" error and prefer it over later signature failures. Not blocking.

Other observations

• Refactor is a net readability win — the outer handler now reads at one level of indirection; the multi-secret behavior lives where the verification logic lives.
• Tests — existing auth.test.ts coverage still applies (accept any listed, reject unlisted, list-over-singular, empty-list fallback, playground isolation); behavior preserved.
• Docs — CUBEJS_API_SECRETS entry in environment-variables.mdx from the previous commit is intact.

· branch: api-gateway-multi-secret ·

[paveltiunov]

getEnv

[claude]

Claude finished @bsod90's task in 55s —— View job

---

Review — feat(api-gateway): accept a list of API secrets

Verdict: LGTM. 0 high, 0 medium, 1 low (residual, unchanged). Already approved by @paveltiunov. New commit d309898 addresses the getEnv feedback cleanly — env parsing now lives in packages/cubejs-backend-shared/src/env.ts next to apiSecret/playgroundAuthSecret, and the ad-hoc parseApiSecretsEnv helper is gone. No new inline comments.

Details

What changed since the prior review (16f3041 → d309898)

• packages/cubejs-backend-shared/src/env.ts:1936-1946 — adds apiSecret and apiSecrets getters to the canonical variables map. apiSecrets returns string[] | undefined with split / trim / filter / dedupe in one place; comment documents precedence and empty→undefined.
• OptsHandler.ts:470-471 — replaces process.env.CUBEJS_API_SECRET with getEnv('apiSecret') (addresses @paveltiunov's getEnv note) and reads apiSecrets via getEnv('apiSecrets').
• cubejs-server/src/server.ts:210-216 — Cube.apiSecret() switched to getEnv('apiSecret'); Cube.apiSecrets() reads via getEnv('apiSecrets').
• packages/cubejs-backend-shared/test/env.test.ts (+35) — covers unset, empty, whitespace-only, trim/filter, dedupe-with-order, single value.
• Old apiSecretsEnv.ts / apiSecretsEnv.test.ts removed.

Correctness check

• Single source of truth for env parsing now matches the rest of the codebase's getEnv pattern.
• De-dup still preserves first-seen order (test pins this).
• gateway.ts candidate loop, JWK short-circuit, playground/system isolation, and early-break on TokenExpiredError/NotBeforeError all unchanged.

Residual (Low, unchanged)

• Other JsonWebTokenError subtypes still masked — gateway.ts:2493-2502. Plain JsonWebTokenError for "jwt audience invalid", "jwt issuer invalid", "jwt subject invalid", "jwt id invalid" still gets overwritten by a later candidate's "invalid signature". Simpler fix: keep the first non-"invalid signature" error and prefer it over later signature failures. Not blocking.

Other observations

• Docs — CUBEJS_API_SECRETS entry in environment-variables.mdx intact.
• Tests — coverage now spans the env layer, gateway flow (accept any listed, reject unlisted, list-over-singular, empty-list fallback, playground isolation), and dedupe semantics.
• Resolved prior threads (cast / duplicated parsing / no-dedup / getEnv) are all addressed by this commit.

· branch: `api-gateway-multi-secret`