feat: move Electric proxy to Cloudflare Worker with JWT auth#1457
Conversation
Move Electric SQL shape proxy from Next.js API route to a Cloudflare Worker for edge auth via JWKS, request collapsing, and reduced API server load. - Create apps/electric-worker with JWT verification (jose + JWKS) - Re-enable /token endpoint in better-auth with definePayload containing organizationIds claim - Desktop client fetches + caches JWT for Electric, refreshes on 55-minute interval - Collections use new NEXT_PUBLIC_ELECTRIC_URL + JWT token - Hardcoded WHERE clauses in worker (no Drizzle dependency) - Existing API route preserved for backward compatibility
📝 WalkthroughWalkthroughAdds a Cloudflare Worker (electric-proxy) that validates JWTs via JWKS and proxies organization-scoped Electric shape requests; includes new JWT payload with organizationIds on the auth server, desktop app env/CSP and auth-client updates, and per-organization apiKeys support in the desktop collections provider. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Desktop Client
participant Proxy as Electric Proxy (Worker)
participant JWKS as JWKS Provider
participant Electric as Electric DB
Client->>Proxy: GET /v1/shape?table=T&organizationId=O\nAuthorization: Bearer <jwt>
Proxy->>JWKS: GET JWKS (jwks_url)
JWKS-->>Proxy: JWKS keys
Proxy->>Proxy: jwtVerify(token, keys)\nextract organizationIds
Proxy->>Proxy: buildWhereClause(table=T, organizationId=O, organizationIds)
Proxy->>Electric: GET /v1/shape?table=T&where=...&secret=ELECTRIC_SECRET
Electric-->>Proxy: 200 + body
Proxy-->>Client: Response (CORS + Vary: Authorization)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts (1)
348-351:⚠️ Potential issue | 🟡 MinorStale doc comment — references
getAuthToken()but Electric shapes now usegetElectricToken().Line 350 says "Auth token is read dynamically via getAuthToken()" which is no longer accurate for Electric shape requests.
📝 Suggested fix
- * Auth token is read dynamically via getAuthToken() - no need to pass it. + * Auth tokens are read dynamically via getElectricToken() (for Electric shapes) + * and getAuthToken() (for tRPC API calls) - no need to pass them.
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts`:
- Around line 64-68: Update organizationsCollection, apiKeysCollection, and the
collections created in createOrgCollections to include an onError handler that
detects a 401 Unauthorized error, fetches or refreshes the JWT (via
getElectricToken or the appropriate refresh helper), and returns an object like
{ headers: { Authorization: `Bearer ${newToken}` } } so the client will retry
the request with the new token; attach this onError alongside the existing
headers function that currently returns "" when getElectricToken() is null so
that 401s are recovered and the sync stream can resume.
In `@apps/electric-worker/src/auth.ts`:
- Around line 40-45: The code currently only checks
Array.isArray(payload.organizationIds) but doesn't ensure each element is a
string; update the validation in the auth.ts logic that reads
payload.organizationIds to verify every element is typeof "string" (e.g., using
Array.prototype.every) and throw a clear Error like "organizationIds must be an
array of strings" if any element is not a string; return organizationIds as
string[] only after this check so downstream includes() usage in index.ts is
safe from tampered/malformed JWT data.
In `@apps/electric-worker/src/index.ts`:
- Around line 55-58: The current check using
url.pathname.startsWith("/v1/shape") is too permissive and will match unintended
routes; update the condition in index.ts to require an exact match (use
url.pathname === "/v1/shape") so only the exact /v1/shape path is handled and
all other paths return corsResponse(404, "Not found"); locate the check that
references url.pathname.startsWith and replace it with an equality comparison
(or explicit validation if you need to allow a single optional trailing slash)
inside the same request handling block.
- Around line 131-152: The upstream fetch to Electric (the line calling
fetch(originUrl.toString())) should be wrapped in a try/catch so network errors
are logged with context before being propagated or handled; update the block
around fetch(originUrl.toString()) to catch any thrown error, log the error
along with the originUrl (using the existing logger, e.g., processLogger or the
module's logger), and then either rethrow or return a controlled error Response
(e.g., 502) while still applying the same CORS_HEADERS and header cleanup logic
(headers set/Delete and Vary header) so callers receive consistent headers;
ensure you reference originUrl, the fetch call, CORS_HEADERS, and the Response
creation (new Response(...)) when making the change.
In `@packages/auth/src/server.ts`:
- Around line 134-139: The definePayload callback signature is incorrect for
better-auth v1.4.18 — change the parameter from a destructured top-level shape
to a single object parameter that contains both user and session (i.e., accept
({ user, session }) instead of ({ user }: { user... })), and update or remove
the explicit type annotation so it matches the plugin API (use types like User &
Record<string, any> and Session & Record<string, any> for user and session
respectively, or drop the annotation to rely on inference); modify the
definePayload function declaration accordingly so the runtime signature and
types align with the JWT plugin.
🧹 Nitpick comments (6)
apps/electric-worker/wrangler.jsonc (1)
4-10: Consider adding environment overrides for staging/dev.The
varsblock hardcodes production URLs, meaningwrangler devwill validate JWTs against the production JWKS endpoint. If you have (or plan to have) a staging environment, consider adding[env.staging]or[env.dev]overrides so local development and preview deployments can use non-production auth endpoints.apps/electric-worker/src/where-clauses.ts (1)
1-5: Consider exportingWhereClausefor downstream type safety.The
WhereClauseinterface is used as the return type of the exportedbuildWhereClausefunction but isn't itself exported. Ifindex.tsor tests ever need to explicitly type a variable holding the result, they'd have to rely on type inference or duplicate the type.Proposed fix
-interface WhereClause { +export interface WhereClause {apps/electric-worker/src/index.ts (1)
30-36: AddAccess-Control-Max-Ageto reduce preflight overhead.Without
Access-Control-Max-Age, browsers will send a CORS preflight on every request. Since the desktop client makes frequent shape requests, caching the preflight result avoids unnecessary round-trips.Proposed fix
const CORS_HEADERS: Record<string, string> = { "Access-Control-Allow-Origin": "*", "Access-Control-Allow-Methods": "GET, OPTIONS", "Access-Control-Allow-Headers": "Authorization, Content-Type", + "Access-Control-Max-Age": "86400", "Access-Control-Expose-Headers": "electric-handle, electric-offset, electric-cursor, electric-schema, electric-chunk-last-offset, electric-up-to-date", };apps/desktop/src/renderer/providers/AuthProvider/AuthProvider.tsx (2)
66-68: Validate the/api/auth/tokenresponse shape instead of a bare cast.The response from an external endpoint is cast with
as { token?: string }without validation. If the endpoint ever returns an unexpected shape,data.tokenwould silently beundefinedand the electric token would never be set.♻️ Suggested fix using Zod
+const electricTokenResponseSchema = z.object({ token: z.string() }); + const fetchElectricToken = useCallback(async () => { ... - const data = (await response.json()) as { token?: string }; - if (data.token) { - setElectricToken(data.token); - } + const parsed = electricTokenResponseSchema.safeParse(await response.json()); + if (parsed.success) { + setElectricToken(parsed.data.token); + } else { + console.error("[auth/electric-token] Unexpected response shape"); + }As per coding guidelines: "Validate external API data as untrusted by handling missing fields, unknown enums, and unexpected shapes with tolerant parsing and explicit fallbacks" and "Use Zod schemas for validating tRPC inputs and API route bodies at boundaries".
43-73: Electric token fetch failure during hydration is silent — user may run without sync indefinitely.If
fetchElectricTokenfails during hydration (line 100), the interval still starts (good), but there's no feedback mechanism if repeated refreshes also fail. Consider adding a retry-with-backoff or at minimum a warning log after consecutive failures so the issue is observable.Also applies to: 92-112
packages/auth/src/server.ts (1)
134-148: Duplicate membership-query logic withcustomSession(line 448).Both
definePayloadand thecustomSessioncallback independently querymembersbyuserIdand deduplicateorganizationIdsviaSet. Consider extracting a shared helper (e.g.,getOrganizationIdsForUser(userId)) to keep this in one place.Also a couple of secondary points:
- PII in JWT —
- No error handling — if the DB query on line 140 throws, the token endpoint returns an opaque 500. A
try/catchwith aconsole.error("[auth/definePayload] …")would improve debuggability.♻️ Example shared helper extraction
+// Near top of file or in a utils module +async function getOrganizationIdsForUser(userId: string): Promise<string[]> { + const userMemberships = await db.query.members.findMany({ + where: eq(members.userId, userId), + columns: { organizationId: true }, + }); + return [...new Set(userMemberships.map((m) => m.organizationId))]; +}Then use it in
definePayload:definePayload: async ({ user, }: { user: { id: string; email: string }; session: Record<string, unknown>; }) => { - const userMemberships = await db.query.members.findMany({ - where: eq(members.userId, user.id), - columns: { organizationId: true }, - }); - const organizationIds = [ - ...new Set(userMemberships.map((m) => m.organizationId)), - ]; + const organizationIds = await getOrganizationIdsForUser(user.id); return { sub: user.id, email: user.email, organizationIds }; },
| headers: { | ||
| Authorization: () => { | ||
| const token = getAuthToken(); | ||
| const token = getElectricToken(); | ||
| return token ? `Bearer ${token}` : ""; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the collections.ts file to understand the context
cat -n apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts | head -120Repository: superset-sh/superset
Length of output: 4170
🏁 Script executed:
# Search for Electric client initialization and configuration
rg -n "electric\|Electric" apps/desktop/src/renderer --type=ts -C3 | head -100Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for AuthProvider implementation and token loading
fd -H "AuthProvider" apps/desktop --type=ts --type=tsxRepository: superset-sh/superset
Length of output: 233
🏁 Script executed:
# Look for retry, backoff, or error handling patterns in Electric shape usage
rg -n "shapes\|sync\|retry\|error.*electric|Electric.*error" apps/desktop/src --type=ts -C2Repository: superset-sh/superset
Length of output: 477
🏁 Script executed:
# Check the package.json or lock file for Electric version to understand what features are available
rg -n "electric-sql|@electric-sql" apps/desktop/package.jsonRepository: superset-sh/superset
Length of output: 101
🏁 Script executed:
# Find and read the auth-client implementation
find apps/desktop -name "auth-client*" -type fRepository: superset-sh/superset
Length of output: 108
🏁 Script executed:
# Search for AuthProvider implementation
find apps/desktop -name "*AuthProvider*" -type fRepository: superset-sh/superset
Length of output: 129
🏁 Script executed:
# Search for getElectricToken and getAuthToken implementations
rg -n "getElectricToken|getAuthToken|fetchElectricToken" apps/desktop/src/renderer --type=ts --type=tsx -B2 -A10Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Look for Electric client error handling or onError handlers
rg -n "onError|error.*handler|retry|backoff" apps/desktop/src/renderer -A3 | head -80Repository: superset-sh/superset
Length of output: 9387
🏁 Script executed:
# Check for Electric Shape or collection error handling patterns
rg -n "shapes?\.|sync|error" apps/desktop/src/renderer/lib --type=ts --type=tsx -C2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Read auth-client implementation
cat -n apps/desktop/src/renderer/lib/auth-client.tsRepository: superset-sh/superset
Length of output: 1734
🏁 Script executed:
# Read AuthProvider implementation
cat -n apps/desktop/src/renderer/providers/AuthProvider/AuthProvider.tsx | head -150Repository: superset-sh/superset
Length of output: 5105
🏁 Script executed:
# Search for where electricCollectionOptions comes from and how it handles errors
rg -n "electricCollectionOptions" apps/desktop/src --type=js -A5 -B2Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look for any Electric-related configuration or error handling
rg -n "@tanstack/electric-db-collection\|Electric\|collection.*error" apps/desktop/src -A3 -B2Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there's any retry logic in the shapes or collection setup
rg -n "shape.*retry\|shape.*error\|collection.*retry\|collection.*error" apps/desktop/src -A3Repository: superset-sh/superset
Length of output: 46
🌐 Web query:
Electric SQL v1.5.2 client 401 unauthorized retry behavior backoff
💡 Result:
In ElectricSQL TypeScript client (@electric-sql/client), a 401 Unauthorized is treated as a non-retryable 4xx error:
- No automatic retry/backoff is applied for 401s. Automatic retries with exponential backoff are only for 5xx, network errors, and 429 rate limits. [1]
- On a 401, the stream will stop/throw unless you provide a stream-level
onErrorhandler. InonError, you typically refresh your token and return{ headers: ... }(returning any object, even{}, tells the client to retry; returningundefinedstops permanently). [1] - If you do retry after a 401 by returning new headers from
onError, recent client versions include a fix so the retry preserves the current offset (rather than resetting and re-emitting duplicates). [2]
Sources:
[1] @electric-sql/client README (UNPKG) — error handling and retry/backoff behavior, including 401 via onError. (app.unpkg.com)
[2] Electric repo release note describing the offset-preservation fix when retrying after onError (e.g., 401). (newreleases.io)
Citations:
- 1: https://app.unpkg.com/%40electric-sql/client%401.1.3/files/README.md?utm_source=openai
- 2: https://newreleases.io/project/github/electric-sql/electric/release/%40electric-sql%2Fclient%401.1.2?utm_source=openai
🌐 Web query:
@electric-sql/client shape sync error handling retry
💡 Result:
In @electric-sql/client, shape sync errors are handled at the ShapeStream level, with built-in automatic retries and an optional onError hook to control whether syncing continues.
- Automatic retry (built-in): the client automatically retries network errors, 5xx, and 429 responses using exponential backoff (with jitter). Your
onErrorhandler is called only after these retries are exhausted, or immediately for non-retryable errors (typically 4xx). [1] onErrorcontrols “retry vs stop”:- Return an object to keep syncing / retry (can also update request inputs):
{}retry with same params/headers{ headers: ... }retry with new headers (e.g., refreshed auth token){ params: ... }retry with new shape params{ params, headers }retry with both
- Return
undefined/ no return to stop the stream permanently. [1]
- Return an object to keep syncing / retry (can also update request inputs):
- Per-subscriber error callback:
stream.subscribe(onMessages, onError)can observe errors, but cannot control retry behavior (use stream-levelonErrorfor that). [1] - Server-directed backoff (
Retry-After) note: the Electric sync service has had fixes related to ensuring clients can readRetry-Afterduring overload (503) via CORS exposure; without it, browsers could force clients to fall back to purely exponential backoff. [2]
Sources:
[1] @electric-sql/client README (ShapeStream “Error Handling”) via unpkg. (app.unpkg.com)
[2] Electric sync-service release notes mentioning Retry-After CORS exposure fix. (github.com)
Citations:
- 1: https://app.unpkg.com/%40electric-sql/client%401.1.3/files/README.md?utm_source=openai
- 2: https://github.com/electric-sql/electric/releases?utm_source=openai
🏁 Script executed:
# Check if there are any useEffect guards or error handling around Electric collection usage in the collections provider
rg -n "organizationsCollection\|apiKeysCollection\|useEffect" apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider -A5 -B2Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check for any error boundaries or recovery mechanisms for Electric shapes
rg -n "catch\|error\|try\|fallback\|retry" apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider -A3Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look at how collections are used - check if they're accessed conditionally based on token availability
rg -n "getElectricToken\|electricToken" apps/desktop/src/renderer/routes/_authenticated -A3 -B3Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Final check: does `@tanstack/electric-db-collection` or electricCollectionOptions add any default error handling?
rg -n "@tanstack/electric-db-collection\|electricCollectionOptions" apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts -B5 -A15 | head -50Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there's a parent component that guards Electric collection access
cat -n apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/*.tsx | head -100Repository: superset-sh/superset
Length of output: 3079
Add onError handler to recover from 401 Unauthorized when JWT is not yet available.
The authorization header function returns "" when getElectricToken() is null, causing a 401 response. The Electric client (@electric-sql/client v1.5.2) does not automatically retry 401 errors—only 5xx, network errors, and 429 responses trigger automatic backoff. Without an onError handler, the sync stream stops permanently and does not recover once the token is fetched.
Add an onError handler to each shape that detects 401 errors, refreshes the token, and returns { headers: { Authorization: ... } } to trigger a retry with the updated token. This applies to all three instances: organizationsCollection, apiKeysCollection, and the collections in createOrgCollections.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts`
around lines 64 - 68, Update organizationsCollection, apiKeysCollection, and the
collections created in createOrgCollections to include an onError handler that
detects a 401 Unauthorized error, fetches or refreshes the JWT (via
getElectricToken or the appropriate refresh helper), and returns an object like
{ headers: { Authorization: `Bearer ${newToken}` } } so the client will retry
the request with the new token; attach this onError alongside the existing
headers function that currently returns "" when getElectricToken() is null so
that 401s are recovered and the sync stream can resume.
| const organizationIds = payload.organizationIds; | ||
| if (!Array.isArray(organizationIds)) { | ||
| throw new Error("Missing organizationIds claim"); | ||
| } | ||
|
|
||
| return { userId, organizationIds: organizationIds as string[] }; |
There was a problem hiding this comment.
Validate that organizationIds elements are strings.
Array.isArray confirms the value is an array but doesn't validate element types. A tampered or misconfigured JWT could carry non-string elements, making the downstream includes() check in index.ts silently fail or behave unexpectedly. As per coding guidelines, external API data should be treated as untrusted.
🛡️ Proposed fix
const organizationIds = payload.organizationIds;
- if (!Array.isArray(organizationIds)) {
+ if (
+ !Array.isArray(organizationIds) ||
+ !organizationIds.every((id): id is string => typeof id === "string")
+ ) {
throw new Error("Missing organizationIds claim");
}
- return { userId, organizationIds: organizationIds as string[] };
+ return { userId, organizationIds };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const organizationIds = payload.organizationIds; | |
| if (!Array.isArray(organizationIds)) { | |
| throw new Error("Missing organizationIds claim"); | |
| } | |
| return { userId, organizationIds: organizationIds as string[] }; | |
| const organizationIds = payload.organizationIds; | |
| if ( | |
| !Array.isArray(organizationIds) || | |
| !organizationIds.every((id): id is string => typeof id === "string") | |
| ) { | |
| throw new Error("Missing organizationIds claim"); | |
| } | |
| return { userId, organizationIds }; |
🤖 Prompt for AI Agents
In `@apps/electric-worker/src/auth.ts` around lines 40 - 45, The code currently
only checks Array.isArray(payload.organizationIds) but doesn't ensure each
element is a string; update the validation in the auth.ts logic that reads
payload.organizationIds to verify every element is typeof "string" (e.g., using
Array.prototype.every) and throw a clear Error like "organizationIds must be an
array of strings" if any element is not a string; return organizationIds as
string[] only after this check so downstream includes() usage in index.ts is
safe from tampered/malformed JWT data.
| // Only handle /v1/shape | ||
| if (!url.pathname.startsWith("/v1/shape")) { | ||
| return corsResponse(404, "Not found"); | ||
| } |
There was a problem hiding this comment.
Path matching with startsWith is overly broad.
url.pathname.startsWith("/v1/shape") will also match paths like /v1/shapeXYZ or /v1/shape/anything. Electric's API endpoint is exactly /v1/shape.
Proposed fix
- if (!url.pathname.startsWith("/v1/shape")) {
+ if (url.pathname !== "/v1/shape") {
return corsResponse(404, "Not found");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Only handle /v1/shape | |
| if (!url.pathname.startsWith("/v1/shape")) { | |
| return corsResponse(404, "Not found"); | |
| } | |
| // Only handle /v1/shape | |
| if (url.pathname !== "/v1/shape") { | |
| return corsResponse(404, "Not found"); | |
| } |
🤖 Prompt for AI Agents
In `@apps/electric-worker/src/index.ts` around lines 55 - 58, The current check
using url.pathname.startsWith("/v1/shape") is too permissive and will match
unintended routes; update the condition in index.ts to require an exact match
(use url.pathname === "/v1/shape") so only the exact /v1/shape path is handled
and all other paths return corsResponse(404, "Not found"); locate the check that
references url.pathname.startsWith and replace it with an equality comparison
(or explicit validation if you need to allow a single optional trailing slash)
inside the same request handling block.
| // Proxy to Electric | ||
| const response = await fetch(originUrl.toString()); | ||
|
|
||
| const headers = new Headers(response.headers); | ||
| headers.set("Vary", "Authorization"); | ||
|
|
||
| // Strip content-encoding since we're not passing through compressed body | ||
| if (headers.has("content-encoding")) { | ||
| headers.delete("content-encoding"); | ||
| headers.delete("content-length"); | ||
| } | ||
|
|
||
| // Add CORS headers | ||
| for (const [key, value] of Object.entries(CORS_HEADERS)) { | ||
| headers.set(key, value); | ||
| } | ||
|
|
||
| return new Response(response.body, { | ||
| status: response.status, | ||
| statusText: response.statusText, | ||
| headers, | ||
| }); |
There was a problem hiding this comment.
Wrap upstream fetch in try/catch for observability on Electric failures.
If the upstream Electric service is unreachable or returns a network error, the fetch will throw an unhandled exception, producing a generic 500 with no logged context. As per coding guidelines, errors should be logged with context before rethrowing or handling.
Proposed fix
// Proxy to Electric
- const response = await fetch(originUrl.toString());
+ let response: Response;
+ try {
+ response = await fetch(originUrl.toString());
+ } catch (error) {
+ console.error("[electric/proxy] Upstream request failed:", error);
+ return corsResponse(502, "Upstream service unavailable");
+ }🤖 Prompt for AI Agents
In `@apps/electric-worker/src/index.ts` around lines 131 - 152, The upstream fetch
to Electric (the line calling fetch(originUrl.toString())) should be wrapped in
a try/catch so network errors are logged with context before being propagated or
handled; update the block around fetch(originUrl.toString()) to catch any thrown
error, log the error along with the originUrl (using the existing logger, e.g.,
processLogger or the module's logger), and then either rethrow or return a
controlled error Response (e.g., 502) while still applying the same CORS_HEADERS
and header cleanup logic (headers set/Delete and Vary header) so callers receive
consistent headers; ensure you reference originUrl, the fetch call,
CORS_HEADERS, and the Response creation (new Response(...)) when making the
change.
| definePayload: async ({ | ||
| user, | ||
| }: { | ||
| user: { id: string; email: string }; | ||
| session: Record<string, unknown>; | ||
| }) => { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
better-auth 1.4.18 JWT plugin definePayload callback signature
💡 Result:
In better-auth v1.4.18, the JWT plugin option is typed as:
import type { Awaitable } from "@better-auth/core";
import type { JWTPayload } from "jose";
definePayload?: (
session: {
user: User & Record<string, any>;
session: Session & Record<string, any>;
}
) => Awaitable<JWTPayload>;It’s invoked with the full Better Auth session object (ctx.context.session!) when /token (or the middleware header) is generated. [1] [2]
🌐 Web query:
better-auth JWT plugin documentation definePayload
💡 Result:
In Better Auth’s JWT plugin, definePayload is the option used to customize what goes into the JWT payload (by default, the full user object is included). Example from the docs: [1]
import { betterAuth } from "better-auth";
import { jwt } from "better-auth/plugins";
export const auth = betterAuth({
plugins: [
jwt({
jwt: {
definePayload: ({ user }) => ({
id: user.id,
email: user.email,
role: user.role,
}),
},
}),
],
});Source: Better Auth JWT plugin docs (see “Modify JWT payload”). [1]
References
- [1] (better-auth.com)
Citations:
🌐 Web query:
better-auth GitHub source code JWT plugin definePayload
💡 Result:
In better-auth, the JWT plugin’s definePayload hook is used inside getJwtToken().
- Source file (GitHub):
packages/better-auth/src/plugins/jwt/sign.ts - Where it’s applied:
getJwtToken()setspayloadto either the fullsession.user(default) or the result ofoptions.jwt.definePayload(session), then passes it intosignJWT(). (raw.githubusercontent.com)
The corresponding docs section (“Modify JWT payload”) shows how to configure it via:
jwt({
jwt: {
definePayload: ({ user }) => ({ id: user.id, email: user.email, role: user.role })
}
})
``` ([better-auth.com](https://www.better-auth.com/docs/plugins/jwt))
Citations:
- 1: https://raw.githubusercontent.com/better-auth/better-auth/cd567659b8b6c78bf90843fd7218446a4ca6aa9b/packages/better-auth/src/plugins/jwt/sign.ts
- 2: https://www.better-auth.com/docs/plugins/jwt
---
</details>
**Fix `definePayload` callback signature to match better-auth 1.4.18 API.**
The type annotation is incorrect. better-auth v1.4.18's JWT plugin passes a single `session` object with shape `{ user: User & Record<string, any>; session: Session & Record<string, any> }`, but the code annotates as if the destructured parameters are at the top level.
Either remove the explicit type annotation and rely on inference, or correct it to match the actual signature:
```ts
definePayload: async ({ user, session: sessionData }: { user: User & Record<string, any>; session: Session & Record<string, any> }) => {🤖 Prompt for AI Agents
In `@packages/auth/src/server.ts` around lines 134 - 139, The definePayload
callback signature is incorrect for better-auth v1.4.18 — change the parameter
from a destructured top-level shape to a single object parameter that contains
both user and session (i.e., accept ({ user, session }) instead of ({ user }: {
user... })), and update or remove the explicit type annotation so it matches the
plugin API (use types like User & Record<string, any> and Session &
Record<string, any> for user and session respectively, or drop the annotation to
rely on inference); modify the definePayload function declaration accordingly so
the runtime signature and types align with the JWT plugin.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/electric-proxy/src/index.ts`:
- Around line 134-136: The Vary header is being overwritten by
headers.set("Vary", "Authorization") which drops upstream values like
"Accept-Encoding"; update the logic that builds the Headers object (the local
variable headers created from response.headers) to read any existing Vary value,
append "Authorization" if not already present (comma-separating and trimming),
and then set the combined, de-duplicated string back on headers (use
headers.get("Vary") to read and headers.set("Vary", combined) to write) so
existing cache directives are preserved.
🧹 Nitpick comments (3)
apps/electric-proxy/src/auth.ts (1)
8-15: JWKS cache doesn't account for URL changes.
getJWKScaches the firstcreateRemoteJWKSetresult and returns it for all subsequent calls regardless of thejwksUrlargument. If the envJWKS_URLever differs across deployments sharing the same isolate (unlikely but possible during config rollouts), stale keys would be used.A simple fix is to key the cache on the URL:
Proposed fix
-let jwks: ReturnType<typeof createRemoteJWKSet> | null = null; +let jwks: { url: string; keySet: ReturnType<typeof createRemoteJWKSet> } | null = null; function getJWKS(jwksUrl: string): ReturnType<typeof createRemoteJWKSet> { - if (!jwks) { - jwks = createRemoteJWKSet(new URL(jwksUrl)); + if (!jwks || jwks.url !== jwksUrl) { + jwks = { url: jwksUrl, keySet: createRemoteJWKSet(new URL(jwksUrl)) }; } - return jwks; + return jwks.keySet; }apps/electric-proxy/src/where-clauses.ts (1)
1-5: Consider exportingWhereClausefor consumers.The
WhereClauseinterface isn't exported, soindex.tsrelies on type inference for the return value. Exporting it would allow callers to explicitly type variables if needed and improve discoverability.apps/electric-proxy/src/index.ts (1)
106-108: User-suppliedtablevalue reflected in error response.
Unknown table: ${table}echoes untrusted input directly into the response body. While this isn't an HTML context (no XSS risk in a plain-text API response), it's still good practice to avoid reflecting raw input. Consider a generic message instead.Proposed fix
- return corsResponse(400, `Unknown table: ${table}`); + return corsResponse(400, "Unsupported table");
| const headers = new Headers(response.headers); | ||
| headers.set("Vary", "Authorization"); | ||
|
|
There was a problem hiding this comment.
Vary header is overwritten, not appended.
headers.set("Vary", "Authorization") replaces any existing Vary header from the Electric upstream (e.g., Vary: Accept-Encoding). This can break intermediate cache behavior, causing incorrect shared-cache hits.
Proposed fix
- headers.set("Vary", "Authorization");
+ headers.append("Vary", "Authorization");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const headers = new Headers(response.headers); | |
| headers.set("Vary", "Authorization"); | |
| const headers = new Headers(response.headers); | |
| headers.append("Vary", "Authorization"); |
🤖 Prompt for AI Agents
In `@apps/electric-proxy/src/index.ts` around lines 134 - 136, The Vary header is
being overwritten by headers.set("Vary", "Authorization") which drops upstream
values like "Accept-Encoding"; update the logic that builds the Headers object
(the local variable headers created from response.headers) to read any existing
Vary value, append "Authorization" if not already present (comma-separating and
trimming), and then set the combined, de-duplicated string back on headers (use
headers.get("Vary") to read and headers.set("Vary", combined) to write) so
existing cache directives are preserved.
- Remove getElectricToken wrapper, call authClient.token() directly in Electric async headers - Move apiKeys from singleton to per-org collection (filter by metadata->>'organizationId' instead of userId) - Remove userId from Worker auth since it's no longer needed - Clean up AuthProvider (remove Container/token-caching leftovers)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts`:
- Around line 76-79: The async Authorization header function currently calls
authClient.token() without error handling so a rejection will throw and kill the
Electric stream; wrap the call to authClient.token() in try/catch and on error
return a safe fallback (either an empty string "" or the stale token from
getAuthToken()) instead of letting the exception propagate; update the same
pattern in the other Authorization header occurrence (the second async header
block) so both places use the try/catch fallback to allow the stream to hit its
onError/retry path.
🧹 Nitpick comments (2)
apps/electric-proxy/src/auth.ts (1)
7-14: JWKS cache ignores URL changes across requests.The module-scoped
jwkssingleton is created for the firstjwksUrlseen and never invalidated. In a Cloudflare Worker isolate (which persists across requests), ifjwksUrlever differs between invocations — e.g., during a config migration or in a multi-tenant setup — stale keys will be served silently.A simple fix is to key the cache on the URL:
Proposed fix
-let jwks: ReturnType<typeof createRemoteJWKSet> | null = null; +let jwksCache: { url: string; keySet: ReturnType<typeof createRemoteJWKSet> } | null = null; function getJWKS(jwksUrl: string): ReturnType<typeof createRemoteJWKSet> { - if (!jwks) { - jwks = createRemoteJWKSet(new URL(jwksUrl)); + if (!jwksCache || jwksCache.url !== jwksUrl) { + jwksCache = { url: jwksUrl, keySet: createRemoteJWKSet(new URL(jwksUrl)) }; } - return jwks; + return jwksCache.keySet; }apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts (1)
28-36:apiKeyDisplaySchemais defined but only used for type inference — consider if runtime validation is needed.If Electric returns unexpected shapes (schema drift, missing columns), runtime parsing via
apiKeyDisplaySchema.parse()would catch it early. Currently the schema is only used asz.infer<...>. This is fine if you trust the data source, but flagging since the guidelines recommend validating external data.
| Authorization: async () => { | ||
| const { data } = await authClient.token(); | ||
| return data?.token ? `Bearer ${data.token}` : ""; | ||
| }, |
There was a problem hiding this comment.
Unhandled rejection from authClient.token() will permanently kill the shape stream.
If authClient.token() rejects (e.g., network failure, auth server unreachable), the async header function throws and the Electric stream has no way to recover. Wrap in try/catch and return "" (or the stale token via getAuthToken()) as a fallback so the stream can still attempt the request and hit the onError retry path.
♻️ Suggested helper extraction (also reduces duplication)
+async function getElectricAuthHeader(): Promise<string> {
+ try {
+ const { data } = await authClient.token();
+ if (data?.token) return `Bearer ${data.token}`;
+ } catch (e) {
+ console.error("[electric/auth] token fetch failed", e);
+ }
+ // Fall back to session bearer token so the request still fires
+ const fallback = getAuthToken();
+ return fallback ? `Bearer ${fallback}` : "";
+}
+
const organizationsCollection = createCollection(
electricCollectionOptions<SelectOrganization>({
id: "organizations",
shapeOptions: {
url: electricUrl,
params: { table: "auth.organizations" },
headers: {
- Authorization: async () => {
- const { data } = await authClient.token();
- return data?.token ? `Bearer ${data.token}` : "";
- },
+ Authorization: getElectricAuthHeader,
},
columnMapper,
},
getKey: (item) => item.id,
}),
);
function createOrgCollections(organizationId: string): OrgCollections {
const headers = {
- Authorization: async () => {
- const { data } = await authClient.token();
- return data?.token ? `Bearer ${data.token}` : "";
- },
+ Authorization: getElectricAuthHeader,
};Also applies to: 89-92
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts`
around lines 76 - 79, The async Authorization header function currently calls
authClient.token() without error handling so a rejection will throw and kill the
Electric stream; wrap the call to authClient.token() in try/catch and on error
return a safe fallback (either an empty string "" or the stale token from
getAuthToken()) instead of letting the exception propagate; update the same
pattern in the other Authorization header occurrence (the second async header
block) so both places use the try/catch fallback to allow the stream to hit its
onError/retry path.
1 participant
Summary
apps/electric-worker/) — proxies Electric shape requests at the edge with JWT verification via JWKS (zero DB calls per request)/tokenendpoint withorganizationIdsclaim; desktop client fetches + caches JWT, refreshes every 55 minNEXT_PUBLIC_ELECTRIC_URL+ JWT token instead of session token through API routeapps/api/src/app/api/electric/) untouched for older clientsArchitecture
Files changed
apps/electric-worker/(new) — auth.ts, index.ts, where-clauses.ts, wrangler.jsoncpackages/auth/src/server.ts— re-enable/token, adddefinePayloadwith org claims.gitignore— add.wranglerFollow-up
referenceIdsupport)Test plan
POST /api/auth/tokenreturns JWT withorganizationIdsclaimSummary by CodeRabbit
New Features
Authentication
Infrastructure
Chores