Repository navigation
feat: relay service for remote host-service access - #3250
Conversation
Add apps/relay — a Hono server deployed to Fly.io that proxies traffic between clients and remote host-services via WebSocket tunnels. - Tunnel endpoint: host-services connect via /tunnel?hostId=<uuid> with PSK auth, maintaining a persistent multiplexed WS connection - HTTP proxy: /v2-hosts/:hostId/trpc/* forwards tRPC calls over tunnel - WS proxy: /v2-hosts/:hostId/events and /terminal/* forward WebSocket connections as multiplexed channels over the tunnel - JWT auth via JWKS (same pattern as electric-proxy) - Access control via v2_users_hosts with 5min TTL cache - Host-service tunnel client: connects to relay when RELAY_URL is set, forwards tunnel messages to local HTTP/WS server, reconnects on drop - Fly.io deployment: always-on, iad region, health check at /health
- Replace shared PSK tunnel secret with JWT auth — relay verifies tunnel connections via JWKS, same as client auth - Add tunnel handshake (hello:ok) and keepalive (ping/pong with 90s timeout after 3 missed pings) - Relay updates v2_hosts.lastSeenAt on tunnel connect - Tunnel client uses getAuthToken() callback instead of static secret - Add host status column to v2_hosts schema (online/offline) - Remove RELAY_TUNNEL_SECRET from relay env
Host-service calls ensureV2Host on startup (not per workspace creation) to register itself with the cloud. If RELAY_URL is set, it then connects the tunnel client with the registered hostId. Exposes api, deviceClientId, deviceName from createApp for the startup flow.
- Add RELAY_PORT to setup.sh port allocation (BASE + 14) - Add RELAY_URL to workspace .env - Use @t3-oss/env-core for relay env validation (matches monorepo pattern) - Host self-registration on startup with relay auto-connect
- Add tunnel setup to desktop host-service entry (the actual bundled entry point, not just packages/host-service/src/serve.ts) - Host-service self-registers via ensureV2Host on startup, then connects tunnel to relay - Export CreateAppResult, TunnelClient from host-service package - Add device-info.ts to host-service for self-generating machineId - Relay tunnel auth accepts session tokens (JWT verification fallback) - Fix CSP to allow http://localhost:* for relay connections in dev - Relay uses RELAY_PORT env var (avoids PORT conflict with other services) - Add RELAY_URL to renderer env schema + Vite define - getRemoteHostUrl uses RELAY_URL env var, returns null if not set - Fall back to JWT in getHostServiceHeaders/getHostServiceWsToken for relay connections (no PSK available) - Rename relay routes from /v2-hosts/ to /hosts/ - Fix event bus URL construction to preserve path prefix Known issues to fix: - Event bus WS connection loop (connecting/disconnecting rapidly) - tRPC calls through relay getting 401 (JWT timing) - useWorkspaceHostUrl local service check still commented out
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Relay service and client tunnel system, plus host/device identification and desktop integration: new relay server and tunnel manager, host-side TunnelClient, host-service device identity and registration, Electron/renderer build/env updates, deployment configs, and supporting type/protocol definitions. Changes
Sequence DiagramssequenceDiagram
participant Host as Host Service
participant Relay as Relay Server
participant AuthAPI as Auth Service
Host->>Host: Derive machineId & deviceName
Host->>Relay: Open /tunnel (WebSocket) with token & hostId
Relay->>AuthAPI: Verify JWT
AuthAPI->>Relay: Return AuthContext
Relay->>Relay: checkHostAccess(userId, hostId)
Relay->>Host: Accept connection (register tunnel)
Host->>Relay: Start keepalive (ping every 30s)
Relay->>Host: Pong responses
Note over Host,Relay: Tunnel registered and active
sequenceDiagram
participant Client as Web Client
participant Relay as Relay Server
participant TunnelMgr as TunnelManager
participant Host as Host Service
participant LocalApp as Local Service
Client->>Relay: GET /hosts/{hostId}/trpc/* (HTTP)
Relay->>Relay: Verify JWT & checkHostAccess
Relay->>TunnelMgr: sendHttpRequest(hostId, method/path/headers/body)
TunnelMgr->>Host: Tunnel message type:"http" (id,...)
Host->>LocalApp: HTTP request to http://127.0.0.1:{port}{path}
LocalApp->>Host: Response (status/headers/body)
Host->>TunnelMgr: Tunnel message type:"http:response" (id,...)
TunnelMgr->>Relay: Resolve pending request
Relay->>Client: Return proxied response
sequenceDiagram
participant Client as Web Client
participant Relay as Relay Server
participant TunnelMgr as TunnelManager
participant Host as Host Service
participant LocalWS as Local WebSocket
Client->>Relay: WS upgrade /hosts/{hostId}/path
Relay->>Relay: Verify JWT & checkHostAccess
Relay->>TunnelMgr: openWsChannel(hostId, path, query)
TunnelMgr->>Host: Tunnel message type:"ws:open" (id,path)
Host->>LocalWS: Open WS to local service
Client->>Relay: Send frame
Relay->>TunnelMgr: sendWsFrame(hostId, channelId, data)
TunnelMgr->>Host: Tunnel message type:"ws:frame"
Host->>LocalWS: Forward frame to local WS
LocalWS->>Host: Response frame
Host->>TunnelMgr: Tunnel message type:"ws:frame" (from host)
TunnelMgr->>Relay: Forward to client channel
Relay->>Client: Deliver frame
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR introduces Key items to address before merging:
Confidence Score: 2/5Not safe to merge — the session-token tunnel auth bypass is a deployable security hole that must be resolved first. The architecture and happy-path implementation are solid, but the P0 security issue in tunnel.ts means the relay as deployed accepts any bearer string to register a tunnel for any host. The PR is explicitly framed as work-in-progress with known issues, but the open auth bypass must be gated before the relay is reachable in any shared environment. apps/relay/src/tunnel.ts (session token bypass), packages/host-service/src/tunnel/tunnel-client.ts (credential in URL), apps/relay/src/access.ts (unbounded cache)
|
| Filename | Overview |
|---|---|
| apps/relay/src/tunnel.ts | Core tunnel registry and route handler — critical security gap: session tokens are accepted for any hostId without verification (TODO left in place) |
| apps/relay/src/access.ts | JWT-path access control with 5-min TTL cache; cache entries are never evicted, causing unbounded memory growth over time |
| packages/host-service/src/tunnel/tunnel-client.ts | Reconnecting WebSocket tunnel client — auth credential placed in URL query string (log-exposure risk); pong type cast bypasses type safety |
| apps/relay/src/proxy.ts | HTTP and WebSocket proxy routes with JWT auth middleware; one misleading console.error on the success path |
| apps/relay/src/auth.ts | JWKS-based JWT verification helper — straightforward and correct; JWKS instance is lazily initialized and reused |
| apps/relay/src/index.ts | Relay entry point wiring Hono app, WebSocket upgrade, tunnel manager, and proxy routes — clean and minimal |
| packages/host-service/src/device-info.ts | Cross-platform machine ID derivation with HMAC hashing for cloud-safe transmission; solid fallback chain |
| apps/desktop/src/renderer/hooks/host-service/useWorkspaceHostUrl/useWorkspaceHostUrl.ts | Local-service fast-path deliberately commented out for relay testing; needs to be re-enabled before production |
| apps/desktop/src/renderer/lib/host-service-auth.ts | Auth header helper updated to fall back to user JWT when no PSK secret is present — clean relay auth integration |
| packages/host-service/src/tunnel/types.ts | Tunnel protocol types duplicated from relay — acknowledged with a TODO; missing ping/pong types forces unsafe casts |
Reviews (1): Last reviewed commit: "feat: wire relay into desktop host-servi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } else { | ||
| // Session token — verify host exists in DB as a basic check | ||
| // TODO: verify session token against Better Auth API | ||
| console.log( | ||
| "[relay:tunnel] accepting session token for host", | ||
| hostId, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The fallback branch for non-JWT tokens (lines 319–326) sets authorized = true and calls tunnelManager.register(hostId, ws) after only logging a message. There is no check that the provided session token belongs to a user who actually owns hostId.
The explicit TODO comment confirms this is incomplete:
// TODO: verify session token against Better Auth API
As written, any client that sends a non-JWT string as the token can register a tunnel for any hostId, including one belonging to another user. An attacker who knows (or can enumerate) a valid hostId could hijack that host's tunnel slot, effectively intercepting or blocking the legitimate host's connections.
This branch must not set authorized = true until the session token has been positively validated against the Better Auth API and the owner of that session is confirmed to have access to hostId.
| console.log( | ||
| `[host-service:tunnel] connected to relay for host ${this.hostId}`, | ||
| ); |
There was a problem hiding this comment.
The auth token is appended as a URL query parameter:
url.searchParams.set("token", token);WebSocket upgrade requests include the full URL in HTTP server access logs, Fly.io log streams, and any reverse proxy logs. The token will therefore appear in plaintext in logs, making it trivially discoverable by anyone with log access.
Prefer sending the token in the Authorization header. The relay already supports reading from the Authorization header (line 294 of tunnel.ts), so the client-side change would be minimal:
// Remove token from URL query string
// url.searchParams.set("token", token);
const socket = new WebSocket(url.toString(), [], {
headers: { Authorization: `Bearer ${token}` },
});Note: the token query-parameter fallback in the proxy client path (proxy.ts, extractToken) has the same exposure.
| const cache = new Map<string, { allowed: boolean; expiresAt: number }>(); | ||
|
|
||
| export async function checkHostAccess( | ||
| userId: string, | ||
| hostId: string, | ||
| ): Promise<boolean> { | ||
| const key = `${userId}:${hostId}`; | ||
| const cached = cache.get(key); | ||
| if (cached && cached.expiresAt > Date.now()) { | ||
| return cached.allowed; | ||
| } | ||
|
|
||
| const row = await db.query.v2UsersHosts.findFirst({ | ||
| where: and( | ||
| eq(v2UsersHosts.userId, userId), | ||
| eq(v2UsersHosts.hostId, hostId), | ||
| ), | ||
| columns: { id: true }, | ||
| }); | ||
|
|
||
| const allowed = !!row; | ||
| cache.set(key, { allowed, expiresAt: Date.now() + TTL_MS }); |
There was a problem hiding this comment.
The cache Map retains every userId:hostId pair that has ever been checked. Stale entries are only overwritten if the same key is rechecked after expiry; entries for users or hosts that are long gone are never removed.
For a relay that handles traffic from many distinct users and hosts over its lifetime, this is a slow but real memory leak. Consider sweeping expired entries periodically:
setInterval(() => {
const now = Date.now();
for (const [key, entry] of cache) {
if (entry.expiresAt <= now) cache.delete(key);
}
}, TTL_MS);|
|
||
| // Handle keepalive | ||
| if (message.type === "ping") { | ||
| this.send({ type: "pong" } as unknown as TunnelResponse); |
There was a problem hiding this comment.
{ type: "pong" } as unknown as TunnelResponse silences the compiler because TunnelResponse (TunnelHttpResponse | TunnelWsFrame | TunnelWsClose) has no pong member. The same issue exists on the relay side.
Adding keepalive types to both types.ts files removes the cast:
export interface TunnelPing { type: "ping" }
export interface TunnelPong { type: "pong" }
export type TunnelResponse = TunnelHttpResponse | TunnelWsFrame | TunnelWsClose | TunnelPong;
export type TunnelRequest = TunnelHttpRequest | TunnelWsOpen | TunnelWsFrame | TunnelWsClose | TunnelPing;Then:
| this.send({ type: "pong" } as unknown as TunnelResponse); | |
| this.send({ type: "pong" }); |
| console.error("[relay:auth] authorized", { | ||
| userId: auth.sub, | ||
| hostId, | ||
| path: c.req.path, | ||
| }); |
There was a problem hiding this comment.
console.error used to log a successful authorization
A successful auth check is logged at the error level, which will fire error-monitoring alerts and pollute error dashboards.
| console.error("[relay:auth] authorized", { | |
| userId: auth.sub, | |
| hostId, | |
| path: c.req.path, | |
| }); | |
| console.log("[relay:auth] authorized", { | |
| userId: auth.sub, | |
| hostId, | |
| path: c.req.path, | |
| }); |
| return useMemo(() => { | ||
| if (!match) return null; | ||
| const localService = services.get(match.hostOrgId); | ||
| if (localService) return localService.url; | ||
| // TODO: uncomment for production — commented out to test relay routing | ||
| // const localService = services.get(match.hostOrgId); | ||
| // if (localService) return localService.url; | ||
| return getRemoteHostUrl(match.hostId); |
There was a problem hiding this comment.
The local services.get(match.hostOrgId) fast-path is commented out, meaning all workspace connections route through the relay even when the host-service is running on the same machine. The TODO note is easy to miss and the comment doesn't reference an issue tracker ticket.
Consider adding a ticket reference (e.g., // TODO(#XXXX): uncomment...) so it's tracked and doesn't get merged silently.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
13 issues found across 29 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/host-service/src/tunnel/tunnel-client.ts">
<violation number="1" location="packages/host-service/src/tunnel/tunnel-client.ts:105">
P2: The `catch` block swallows the fetch error without logging, making it hard to diagnose why HTTP proxying fails. Log the error with context (e.g., method, path) before sending the 502 response.
(Based on your team's feedback about handling async errors explicitly and avoiding silent catch blocks.) [FEEDBACK_USED]</violation>
<violation number="2" location="packages/host-service/src/tunnel/tunnel-client.ts:198">
P2: When the local WebSocket errors, both `onerror` and `onclose` fire (per spec, `onclose` always follows `onerror`), sending duplicate `ws:close` messages to the relay. Remove the channel cleanup from `onerror` and let `onclose` handle it, or guard `onclose` against an already-removed channel.</violation>
<violation number="3" location="packages/host-service/src/tunnel/tunnel-client.ts:210">
P1: WebSocket frames arriving between `ws:open` and the local connection completing its handshake are silently dropped. `handleWsFrame` skips sends when `readyState !== OPEN`, but there's no buffering. This will cause data loss on initial terminal input or event-bus messages.
Buffer incoming frames per channel until the local WebSocket fires `onopen`, then flush.</violation>
</file>
<file name="apps/relay/src/auth.ts">
<violation number="1" location="apps/relay/src/auth.ts:37">
P2: Empty `catch` silently swallows all JWT verification errors (network failures, clock skew, malformed tokens). Given the PR already mentions a JWT timing issue causing 401s, logging the error here would make debugging significantly easier.
(Based on your team's feedback about handling async errors explicitly and not silently swallowing failures.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/relay/src/proxy.ts">
<violation number="1" location="apps/relay/src/proxy.ts:85">
P1: Query string is dropped when proxying HTTP requests. `c.req.path` returns only the pathname — tRPC GET requests that pass `input` and `batch` via query parameters will be forwarded without them, causing broken or incorrect responses from the host-service.</violation>
<violation number="2" location="apps/relay/src/proxy.ts:133">
P2: Silent `catch` swallows the error from `openWsChannel`. Log the error with context (hostId, path) so tunnel-open failures are diagnosable.
(Based on your team's feedback about handling async calls and errors explicitly.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/relay/src/tunnel.ts">
<violation number="1" location="apps/relay/src/tunnel.ts:319">
P0: Any token that fails JWT verification is silently accepted as a "session token" with no actual validation, allowing unauthenticated tunnel registration. An attacker with a known `hostId` can register a tunnel using a garbage token, intercept all proxied traffic, or block the legitimate host from connecting.
At minimum, reject the connection when the session token cannot be verified, rather than falling through to `authorized = true`.</violation>
</file>
<file name="apps/desktop/src/main/host-service/index.ts">
<violation number="1" location="apps/desktop/src/main/host-service/index.ts:160">
P2: `connectRelay` has no retry path when `ensureV2Host` fails, so a transient startup/auth error can permanently prevent tunnel connection until restart.</violation>
</file>
<file name="apps/relay/src/access.ts">
<violation number="1" location="apps/relay/src/access.ts:7">
P2: The access cache can grow unbounded because expired keys are never proactively evicted.</violation>
</file>
<file name="apps/desktop/src/renderer/env.renderer.ts">
<violation number="1" location="apps/desktop/src/renderer/env.renderer.ts:27">
P2: Validate `RELAY_URL` as a URL, not a free-form string, to prevent malformed relay endpoints from passing env parsing.</violation>
</file>
<file name="packages/host-service/src/device-info.ts">
<violation number="1" location="packages/host-service/src/device-info.ts:23">
P2: Avoid silently swallowing machine ID detection errors; log a warning in the catch so fallback usage is observable.
(Based on your team's feedback about handling errors explicitly and avoiding empty catch blocks.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/host-service/src/serve.ts">
<violation number="1" location="packages/host-service/src/serve.ts:70">
P2: Registration failure permanently prevents tunnel connection. If `ensureV2Host.mutate` fails (transient network error, API downtime), no retry is attempted and the tunnel is never created. Consider adding retry-with-backoff around registration, similar to the reconnect logic already in `TunnelClient`.</violation>
</file>
<file name="apps/relay/Dockerfile">
<violation number="1" location="apps/relay/Dockerfile:12">
P1: `bun install` can fail because root `postinstall` script is referenced before `scripts/` is copied into the image.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| this.localChannels.set(req.id, localWs); | ||
| } | ||
|
|
||
| private handleWsFrame(msg: TunnelWsFrame): void { |
There was a problem hiding this comment.
P1: WebSocket frames arriving between ws:open and the local connection completing its handshake are silently dropped. handleWsFrame skips sends when readyState !== OPEN, but there's no buffering. This will cause data loss on initial terminal input or event-bus messages.
Buffer incoming frames per channel until the local WebSocket fires onopen, then flush.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/tunnel/tunnel-client.ts, line 210:
<comment>WebSocket frames arriving between `ws:open` and the local connection completing its handshake are silently dropped. `handleWsFrame` skips sends when `readyState !== OPEN`, but there's no buffering. This will cause data loss on initial terminal input or event-bus messages.
Buffer incoming frames per channel until the local WebSocket fires `onopen`, then flush.</comment>
<file context>
@@ -0,0 +1,250 @@
+ this.localChannels.set(req.id, localWs);
+ }
+
+ private handleWsFrame(msg: TunnelWsFrame): void {
+ const localWs = this.localChannels.get(msg.id);
+ if (localWs?.readyState === WebSocket.OPEN) {
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.superset/lib/setup/steps.sh (1)
443-473:⚠️ Potential issue | 🟡 MinorKeep the generated port map in sync with the new relay port.
This function now allocates and writes
RELAY_PORT/RELAY_URL, but the.superset/ports.jsonblock later in the same function never adds a relay entry. The desktop port labels will drift as soon as this lands.Suggested follow-up in the later `ports.json` block
- { "port": $WRANGLER_PORT, "label": "Electric Proxy (Wrangler)" } + { "port": $WRANGLER_PORT, "label": "Electric Proxy (Wrangler)" }, + { "port": $RELAY_PORT, "label": "Relay" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.superset/lib/setup/steps.sh around lines 443 - 473, The code writes RELAY_PORT/RELAY_URL but the later ports.json generation block is missing a relay entry, causing downstream port label drift; locate the ports.json creation section in the same function and add a Relay entry using the same RELAY_PORT symbol (e.g., include "relay": "$RELAY_PORT" in the JSON object or the same key pattern used for other apps) so the generated .superset/ports.json stays in sync with write_env_var calls for RELAY_PORT/RELAY_URL.
🧹 Nitpick comments (9)
packages/host-service/src/tunnel/types.ts (1)
40-46: MissingTunnelMessageunion type present in relay types.The relay's
apps/relay/src/types.tsalso exportsTunnelMessage = TunnelRequest | TunnelResponse. If this type is needed for message parsing on the host side, consider adding it here for parity.Add TunnelMessage type for completeness
export type TunnelResponse = TunnelHttpResponse | TunnelWsFrame | TunnelWsClose; + +export type TunnelMessage = TunnelRequest | TunnelResponse;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/tunnel/types.ts` around lines 40 - 46, Add the missing union type TunnelMessage that mirrors the relay types by combining the existing TunnelRequest and TunnelResponse types so host-side code can use a single discriminated type for parsing; specifically, declare TunnelMessage = TunnelRequest | TunnelResponse alongside the existing TunnelRequest and TunnelResponse declarations (referencing the TunnelRequest and TunnelResponse type names to locate where to add it).apps/relay/src/index.ts (1)
14-15: Consider restricting CORS origins in production.
cors()with no options allows all origins. While this may be intentional for a relay service, consider whether specific origins should be allowlisted for production deployment to reduce attack surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/relay/src/index.ts` around lines 14 - 15, The app currently mounts open CORS via app.use("*", cors()), allowing all origins; change this to use a configurable allowlist in production by reading an environment variable (e.g., CORS_ORIGINS or NODE_ENV) and passing an { origin } option to cors() instead of the default. Update the setup where app.use("*", cors()) is called so that in non-production (or when a special flag is set) you keep the permissive behavior, but in production construct an origin checker from a comma-separated allowlist and pass it into cors({ origin: ... }) (or a function that validates against the list) and ensure app.use("*", cors(...)) is used with that configured options.apps/desktop/src/main/host-service/index.ts (1)
138-162: Consider extracting shared relay connection logic.The
connectRelayfunction is nearly identical toregisterAndConnectinpackages/host-service/src/serve.ts. Both perform host registration viaensureV2Host.mutateand create aTunnelClient. Consider extracting this to a shared utility in@superset/host-serviceto reduce duplication.Example shared utility
// packages/host-service/src/tunnel/connect.ts export async function connectToRelay(options: { api: { device: { ensureV2Host: { mutate: (input: { machineId: string; name: string }) => Promise<{ id: string }> } } }; machineId: string; name: string; relayUrl: string; localPort: number; getAuthToken: () => string | null; }): Promise<void> { const host = await options.api.device.ensureV2Host.mutate({ machineId: options.machineId, name: options.name, }); console.log(`[host-service] registered as host ${host.id}`); const tunnel = new TunnelClient({ relayUrl: options.relayUrl, hostId: host.id, getAuthToken: options.getAuthToken, localPort: options.localPort, }); tunnel.connect(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/host-service/index.ts` around lines 138 - 162, connectRelay duplicates logic in registerAndConnect: both call api.device.ensureV2Host.mutate and instantiate TunnelClient; extract this into a shared utility (e.g., connectToRelay) in `@superset/host-service` that accepts { api, machineId, name, relayUrl, localPort, getAuthToken } and performs the ensureV2Host.mutate call, logs the host id, creates the TunnelClient and calls connect(); then replace the body of connectRelay and registerAndConnect to call this new connectToRelay and preserve existing try/catch/error logging behavior (or let the caller handle errors) so callers still pass a getAuthToken function to TunnelClient.apps/relay/src/auth.ts (2)
9-16: JWKS cache doesn't handle multiple auth URLs.The module-level
jwkscache is set on first call and never updated. IfverifyJWTis called with differentauthUrlvalues, subsequent calls will use the cached JWKS from the first URL. In this codebase,authUrlappears to always beenv.NEXT_PUBLIC_API_URL, so this is likely fine, but the function signature suggests flexibility that the implementation doesn't support.Consider either:
- Removing
authUrlas a parameter and using the env directly, or- Using a
Map<string, JWKS>to cache per URLOption 1: Use env directly (simpler if only one auth URL)
+import { env } from "./env"; + -let jwks: ReturnType<typeof createRemoteJWKSet> | null = null; +const jwks = createRemoteJWKSet(new URL("/api/auth/jwks", env.NEXT_PUBLIC_API_URL)); -function getJWKS(authUrl: string): ReturnType<typeof createRemoteJWKSet> { - if (!jwks) { - jwks = createRemoteJWKSet(new URL("/api/auth/jwks", authUrl)); - } - return jwks; -} export async function verifyJWT( token: string, - authUrl: string, ): Promise<AuthContext | null> { try { - const { payload } = await jwtVerify(token, getJWKS(authUrl), { - issuer: authUrl, - audience: authUrl, + const { payload } = await jwtVerify(token, jwks, { + issuer: env.NEXT_PUBLIC_API_URL, + audience: env.NEXT_PUBLIC_API_URL, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/relay/src/auth.ts` around lines 9 - 16, The module-level jwks cache only holds a single entry and breaks when getJWKS(authUrl) is called with different URLs; change the cache to a Map keyed by authUrl (e.g., const jwksCache = new Map<string, ReturnType<typeof createRemoteJWKSet>>()) and update getJWKS to look up the Map by authUrl, create and store a new createRemoteJWKSet(new URL("/api/auth/jwks", authUrl)) if missing, then return the stored value; update any uses (e.g., verifyJWT) to call getJWKS(authUrl) as before.
28-36: Consider logging JWT verification failures for debugging.While silently returning
nullon errors is appropriate for production security, logging at debug/trace level could help diagnose authentication issues during development. The current catch block swallows all errors, making it harder to debug token issues.Add debug logging for verification failures
} catch { + // Uncomment for debugging: console.debug("[auth] JWT verification failed:", error); return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/relay/src/auth.ts` around lines 28 - 36, The catch block that currently swallows JWT verification errors and returns null should log the failure at debug/trace level to aid development debugging: in the function that extracts payload and returns { sub, email, organizationIds } (the catch block around the code that reads sub, email, organizationIds), call the existing logger (e.g., logger.debug or processLogger.debug) to record the verification error and minimal context (error.message and the attempted sub if available) but do not log sensitive token contents; then continue returning null as before.apps/relay/src/proxy.ts (1)
133-135: Consider logging the error before closing the WebSocket.The empty catch blocks silently swallow errors from
openWsChannel. Per the context snippet,openWsChannelthrows"Host not connected"if the tunnel is missing or handshake incomplete. Logging the error would help diagnose connection issues.Proposed fix
onOpen: (_event, ws) => { try { channelId = tunnelManager.openWsChannel( hostId, "/events", undefined, ws, ); - } catch { + } catch (err) { + console.error("[relay:proxy] failed to open WS channel", err); ws.close(1011, "Failed to open channel"); } },Also applies to: 175-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/relay/src/proxy.ts` around lines 133 - 135, The catch blocks around the openWsChannel call silently swallow errors; update both catch handlers (the one that calls ws.close(1011, "Failed to open channel") and the similar block at the other location) to log the caught error before closing the WebSocket: catch (err) { processLogger.error(...) } or use the existing logger to record the error message and stack including context (e.g., mention openWsChannel and the peer/connection id) and then call ws.close(1011, "Failed to open channel") so the failure is visible in logs for debugging.packages/host-service/src/tunnel/tunnel-client.ts (3)
109-113: Type cast for pong response is incorrect but functionally safe.The
{ type: "pong" }message is cast toTunnelResponse, butpongis not part of theTunnelResponseunion type (which only includesTunnelHttpResponse | TunnelWsFrame | TunnelWsClose). The relay expects{ type: "pong" }(per context snippet from tunnel.ts:213-217), so this works at runtime. Consider either:
- Adding a
TunnelPongtype to the protocol, or- Using a separate raw send method for control messages
Proposed fix — add sendRaw for control messages
+ private sendRaw(message: Record<string, unknown>): void { + if (this.socket?.readyState === WebSocket.OPEN) { + this.socket.send(JSON.stringify(message)); + } + } + private send(message: TunnelResponse): void { if (this.socket?.readyState === WebSocket.OPEN) { this.socket.send(JSON.stringify(message)); } }Then in handleMessage:
if (message.type === "ping") { - this.send({ type: "pong" } as unknown as TunnelResponse); + this.sendRaw({ type: "pong" }); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/tunnel/tunnel-client.ts` around lines 109 - 113, The code casts a `{ type: "pong" }` object to TunnelResponse in handleMessage which is incorrect because TunnelResponse excludes ping/pong control messages; fix by adding a proper control path instead of lying with a cast: either add a new TunnelPong type to the protocol union (e.g., export type TunnelPong and include it where TunnelResponse is defined) and use this new type when sending the pong, or add a dedicated sendRaw/sendControl method on the TunnelClient and call that from handleMessage to send `{ type: "pong" }` without forcing it into TunnelResponse; update references to send/sendRaw in tunnel-client.ts and any protocol type definitions accordingly.
170-208: Race condition: local WS may receive messages before being added to localChannels.The
onmessagehandler is attached before the WebSocket is added tolocalChannelson line 207. If the local service sends a message immediately upon connection (before line 207 executes), the flow works correctly sincethis.send()doesn't depend onlocalChannels. However, if the local WS closes or errors before line 207, thelocalChannels.delete(req.id)on lines 190/199 will be a no-op and the channel won't be tracked.This is a minor edge case, but moving
localChannels.setbefore attaching handlers would be more robust.Proposed fix
private handleWsOpen(req: TunnelWsOpen): void { const wsUrl = new URL(req.path, `ws://127.0.0.1:${this.localPort}`); if (req.query) { for (const param of req.query.split("&")) { const [key, value] = param.split("="); if (key) wsUrl.searchParams.set(key, decodeURIComponent(value ?? "")); } } const localWs = new WebSocket(wsUrl.toString()); + this.localChannels.set(req.id, localWs); localWs.onmessage = (event) => { // ... }; localWs.onclose = (event) => { this.localChannels.delete(req.id); // ... }; localWs.onerror = () => { this.localChannels.delete(req.id); // ... }; - - this.localChannels.set(req.id, localWs); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/tunnel/tunnel-client.ts` around lines 170 - 208, In handleWsOpen, avoid the race by registering the local WebSocket in localChannels before attaching its event handlers: call this.localChannels.set(req.id, localWs) immediately after creating localWs and before assigning localWs.onmessage, onclose, and onerror; keep the existing delete logic in the onclose/onerror handlers so cleanup still removes the entry if those events fire later.
137-168: Potential issue: sending body with GET/HEAD requests.When
req.methodis GET or HEAD, passingbodytofetchmay cause issues with some runtimes (e.g., throws in browsers, ignored in Node). The relay strips the body from incoming requests for such methods, but defensively handling this here would be safer.Proposed fix
private async handleHttpRequest(req: TunnelHttpRequest): Promise<void> { try { const url = `http://127.0.0.1:${this.localPort}${req.path}`; + const method = req.method.toUpperCase(); + const hasBody = method !== "GET" && method !== "HEAD"; const response = await fetch(url, { - method: req.method, + method, headers: req.headers, - body: req.body ?? undefined, + body: hasBody ? (req.body ?? undefined) : undefined, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/tunnel/tunnel-client.ts` around lines 137 - 168, In handleHttpRequest, avoid passing a body to fetch for GET or HEAD requests: detect req.method (from TunnelHttpRequest) and only include the body property in the fetch options when req.method is not "GET" or "HEAD" and req.body is present; otherwise omit body (use undefined or simply don't set the option) so fetch won't receive a body for GET/HEAD and you still forward headers and method as before and send the response via this.send.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/renderer/env.renderer.ts`:
- Line 27: Replace the loose validator for RELAY_URL with a URL validator so
malformed base URLs are rejected: change the schema entry for RELAY_URL from
z.string().optional() to z.url().optional() (or z.string().url().optional() to
match your zod version) in the env schema that defines RELAY_URL, and run a
quick type/check to ensure call sites that use env.RELAY_URL (e.g., new
URL("/tunnel", this.relayUrl) and `${env.RELAY_URL}/hosts/${hostId}`) still
handle the optional case (add a guard or fallback if necessary).
In
`@apps/desktop/src/renderer/hooks/host-service/useWorkspaceHostUrl/useWorkspaceHostUrl.ts`:
- Around line 35-39: The hook useWorkspaceHostUrl currently always returns
getRemoteHostUrl(match.hostId) because the local-service fallback is commented
out; restore the original local lookup so local host-backed features work when
relay is unavailable by re-enabling the services.get(match.hostOrgId) lookup and
returning localService.url when present before falling back to
getRemoteHostUrl(match.hostId); ensure you reference the same symbols
(useWorkspaceHostUrl, services.get, match.hostOrgId, localService.url,
getRemoteHostUrl, match.hostId) and keep the early return behavior so downstream
consumers receive the local hostUrl when available.
In `@apps/desktop/src/renderer/lib/host-service-auth.ts`:
- Around line 17-18: getHostServiceHeaders and getHostServiceWsToken currently
return empty headers/null when getJwt() is not ready, causing unauthenticated
requests; change them to signal "auth not ready" explicitly (e.g., throw a
specific AuthNotReadyError or return a distinct sentinel like undefined) instead
of silently returning empty values, then update callers to gate operations until
JWT is available: in host-service-client.ts (httpBatchLink headers callback)
bail out or delay requests when auth is not ready, in layout.tsx
(WorkspaceTrpcProvider init) defer provider creation until token readiness, and
in useWorkspaceEvent.ts prevent subscribing unless getHostServiceWsToken
indicates auth is ready. Ensure you reference getHostServiceHeaders and
getHostServiceWsToken when implementing the checks and use a clear, single
mechanism (error or sentinel) for all callers.
In `@apps/relay/Dockerfile`:
- Around line 15-23: The final Docker stage runs as root (see EXPOSE 8080 and
CMD ["bun", "run", "apps/relay/src/index.ts"]) which is unsafe for an
internet-facing relay; modify the Dockerfile to create a non-root runtime user
(e.g., add a user/group like relay or app), chown the application files and
node_modules to that user (packages copied from install stage), and switch to
that user with USER before the CMD so the container runs without root
privileges.
In `@apps/relay/src/access.ts`:
- Around line 7-8: The cache Map named "cache" can grow without bounds because
expired entries are only removed on access; update the code that reads and
writes this Map (the functions that populate/read "cache" for (userId, hostId)
entries) to (1) remove expired entries on get/set by checking expiresAt and
deleting stale keys, (2) enforce a configurable maximum size (evict oldest or
random entries when over limit), and (3) add a periodic cleanup task
(setInterval) that scans "cache" and deletes expired entries on a timer,
ensuring the interval is cleared on shutdown — this will prevent unbounded
memory growth for the cache.
In `@apps/relay/src/env.ts`:
- Line 9: REQUEST_TIMEOUT_MS currently allows zero and negative numbers which
will break setTimeout; update the zod schema for REQUEST_TIMEOUT_MS to coerce to
a number and constrain it to positive integers (e.g., use .int().positive() or
.min(1)) while preserving the default of 30000 so invalid config values fail
validation at startup; target the REQUEST_TIMEOUT_MS schema definition in env.ts
to apply this change.
- Line 13: The skipValidation property currently uses boolean coercion
(!!process.env.SKIP_ENV_VALIDATION) which treats any non-empty string (including
"false") as true; update the logic to parse the environment string explicitly
(e.g., compare process.env.SKIP_ENV_VALIDATION to "true" or "1" in a
case-insensitive way) so only explicit truthy strings enable skipping; change
the assignment for skipValidation in the env files (the key named skipValidation
using process.env.SKIP_ENV_VALIDATION) and apply the same fix consistently
across the other env files listed (apps/web/src/env.ts, apps/docs/src/env.ts,
apps/api/src/env.ts, apps/admin/src/env.ts, apps/marketing/src/env.ts,
packages/trpc/src/env.ts).
In `@apps/relay/src/tunnel.ts`:
- Around line 319-326: The session-token fallback currently accepts any token
without verification; update the else branch that logs "[relay:tunnel] accepting
session token for host" to enforce real auth by either calling the existing
checkHostAccess(hostId, userId) validation or invoking the Better Auth API to
validate the session token before registering the tunnel; if verification fails,
reject the connection (close socket/return error) instead of proceeding. Ensure
you reference the same hostId/session token handling code path and preserve
logging to include the verification result and context.
---
Outside diff comments:
In @.superset/lib/setup/steps.sh:
- Around line 443-473: The code writes RELAY_PORT/RELAY_URL but the later
ports.json generation block is missing a relay entry, causing downstream port
label drift; locate the ports.json creation section in the same function and add
a Relay entry using the same RELAY_PORT symbol (e.g., include "relay":
"$RELAY_PORT" in the JSON object or the same key pattern used for other apps) so
the generated .superset/ports.json stays in sync with write_env_var calls for
RELAY_PORT/RELAY_URL.
---
Nitpick comments:
In `@apps/desktop/src/main/host-service/index.ts`:
- Around line 138-162: connectRelay duplicates logic in registerAndConnect: both
call api.device.ensureV2Host.mutate and instantiate TunnelClient; extract this
into a shared utility (e.g., connectToRelay) in `@superset/host-service` that
accepts { api, machineId, name, relayUrl, localPort, getAuthToken } and performs
the ensureV2Host.mutate call, logs the host id, creates the TunnelClient and
calls connect(); then replace the body of connectRelay and registerAndConnect to
call this new connectToRelay and preserve existing try/catch/error logging
behavior (or let the caller handle errors) so callers still pass a getAuthToken
function to TunnelClient.
In `@apps/relay/src/auth.ts`:
- Around line 9-16: The module-level jwks cache only holds a single entry and
breaks when getJWKS(authUrl) is called with different URLs; change the cache to
a Map keyed by authUrl (e.g., const jwksCache = new Map<string,
ReturnType<typeof createRemoteJWKSet>>()) and update getJWKS to look up the Map
by authUrl, create and store a new createRemoteJWKSet(new URL("/api/auth/jwks",
authUrl)) if missing, then return the stored value; update any uses (e.g.,
verifyJWT) to call getJWKS(authUrl) as before.
- Around line 28-36: The catch block that currently swallows JWT verification
errors and returns null should log the failure at debug/trace level to aid
development debugging: in the function that extracts payload and returns { sub,
email, organizationIds } (the catch block around the code that reads sub, email,
organizationIds), call the existing logger (e.g., logger.debug or
processLogger.debug) to record the verification error and minimal context
(error.message and the attempted sub if available) but do not log sensitive
token contents; then continue returning null as before.
In `@apps/relay/src/index.ts`:
- Around line 14-15: The app currently mounts open CORS via app.use("*",
cors()), allowing all origins; change this to use a configurable allowlist in
production by reading an environment variable (e.g., CORS_ORIGINS or NODE_ENV)
and passing an { origin } option to cors() instead of the default. Update the
setup where app.use("*", cors()) is called so that in non-production (or when a
special flag is set) you keep the permissive behavior, but in production
construct an origin checker from a comma-separated allowlist and pass it into
cors({ origin: ... }) (or a function that validates against the list) and ensure
app.use("*", cors(...)) is used with that configured options.
In `@apps/relay/src/proxy.ts`:
- Around line 133-135: The catch blocks around the openWsChannel call silently
swallow errors; update both catch handlers (the one that calls ws.close(1011,
"Failed to open channel") and the similar block at the other location) to log
the caught error before closing the WebSocket: catch (err) {
processLogger.error(...) } or use the existing logger to record the error
message and stack including context (e.g., mention openWsChannel and the
peer/connection id) and then call ws.close(1011, "Failed to open channel") so
the failure is visible in logs for debugging.
In `@packages/host-service/src/tunnel/tunnel-client.ts`:
- Around line 109-113: The code casts a `{ type: "pong" }` object to
TunnelResponse in handleMessage which is incorrect because TunnelResponse
excludes ping/pong control messages; fix by adding a proper control path instead
of lying with a cast: either add a new TunnelPong type to the protocol union
(e.g., export type TunnelPong and include it where TunnelResponse is defined)
and use this new type when sending the pong, or add a dedicated
sendRaw/sendControl method on the TunnelClient and call that from handleMessage
to send `{ type: "pong" }` without forcing it into TunnelResponse; update
references to send/sendRaw in tunnel-client.ts and any protocol type definitions
accordingly.
- Around line 170-208: In handleWsOpen, avoid the race by registering the local
WebSocket in localChannels before attaching its event handlers: call
this.localChannels.set(req.id, localWs) immediately after creating localWs and
before assigning localWs.onmessage, onclose, and onerror; keep the existing
delete logic in the onclose/onerror handlers so cleanup still removes the entry
if those events fire later.
- Around line 137-168: In handleHttpRequest, avoid passing a body to fetch for
GET or HEAD requests: detect req.method (from TunnelHttpRequest) and only
include the body property in the fetch options when req.method is not "GET" or
"HEAD" and req.body is present; otherwise omit body (use undefined or simply
don't set the option) so fetch won't receive a body for GET/HEAD and you still
forward headers and method as before and send the response via this.send.
In `@packages/host-service/src/tunnel/types.ts`:
- Around line 40-46: Add the missing union type TunnelMessage that mirrors the
relay types by combining the existing TunnelRequest and TunnelResponse types so
host-side code can use a single discriminated type for parsing; specifically,
declare TunnelMessage = TunnelRequest | TunnelResponse alongside the existing
TunnelRequest and TunnelResponse declarations (referencing the TunnelRequest and
TunnelResponse type names to locate where to add it).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 487f38fb-5320-4ff2-95ca-d3111cc8f7f1
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
.superset/lib/setup/steps.shapps/desktop/electron.vite.config.tsapps/desktop/src/main/host-service/index.tsapps/desktop/src/renderer/env.renderer.tsapps/desktop/src/renderer/hooks/host-service/useWorkspaceHostUrl/useWorkspaceHostUrl.tsapps/desktop/src/renderer/index.htmlapps/desktop/src/renderer/lib/host-service-auth.tsapps/desktop/src/renderer/lib/v2-workspace-host.tsapps/relay/Dockerfileapps/relay/fly.tomlapps/relay/package.jsonapps/relay/src/access.tsapps/relay/src/auth.tsapps/relay/src/env.tsapps/relay/src/index.tsapps/relay/src/proxy.tsapps/relay/src/tunnel.tsapps/relay/src/types.tsapps/relay/tsconfig.jsonpackages/host-service/package.jsonpackages/host-service/src/app.tspackages/host-service/src/device-info.tspackages/host-service/src/index.tspackages/host-service/src/serve.tspackages/host-service/src/tunnel/index.tspackages/host-service/src/tunnel/tunnel-client.tspackages/host-service/src/tunnel/types.tspackages/workspace-client/src/lib/eventBus.ts
| const jwt = getJwt(); | ||
| return jwt ? { Authorization: `Bearer ${jwt}` } : {}; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 '\bgetHostServiceHeaders\s*\(|\bgetHostServiceWsToken\s*\(' --type=ts --type=tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
cat -n apps/desktop/src/renderer/lib/host-service-auth.tsRepository: superset-sh/superset
Length of output: 979
🏁 Script executed:
#!/bin/bash
# Search for getHostServiceHeaders and getHostServiceWsToken calls with proper rg syntax
rg -n 'getHostServiceHeaders|getHostServiceWsToken' --type tsRepository: superset-sh/superset
Length of output: 1358
🏁 Script executed:
cat -n apps/desktop/src/renderer/lib/host-service-client.tsRepository: superset-sh/superset
Length of output: 1228
🏁 Script executed:
sed -n '95,115p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsxRepository: superset-sh/superset
Length of output: 339
🏁 Script executed:
sed -n '25,45p' apps/desktop/src/renderer/hooks/host-service/useWorkspaceEvent/useWorkspaceEvent.tsRepository: superset-sh/superset
Length of output: 691
🏁 Script executed:
find apps/desktop/src -name "*WorkspaceTrpc*" -type fRepository: superset-sh/superset
Length of output: 193
🏁 Script executed:
rg -n "WorkspaceTrpcProvider" --type ts apps/desktop/src -A 20 | head -100Repository: superset-sh/superset
Length of output: 8415
🏁 Script executed:
cat -n apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/providers/WorkspaceTrpcProvider/WorkspaceTrpcProvider.tsxRepository: superset-sh/superset
Length of output: 237
🏁 Script executed:
rg -n "401|Unauthorized|getJwt" apps/desktop/src/renderer/lib/host-service-auth.ts -B 5 -A 5Repository: superset-sh/superset
Length of output: 769
🏁 Script executed:
rg -n "getJwt\(\)" apps/desktop/src/renderer --type ts -lRepository: superset-sh/superset
Length of output: 252
🏁 Script executed:
cat -n apps/desktop/src/renderer/lib/auth-client.tsRepository: superset-sh/superset
Length of output: 1963
🏁 Script executed:
rg -n "401|401s|unauthorized|retry|getHostServiceClient" apps/desktop/src/renderer --type ts -B 3 -A 3 | head -80Repository: superset-sh/superset
Length of output: 11407
🏁 Script executed:
rg -n "httpBatchLink|createTRPCClient" apps/desktop/src/renderer --type ts -A 10 | head -100Repository: superset-sh/superset
Length of output: 7321
Avoid silent unauthenticated fallback when JWT is not ready.
When no PSK exists and getJwt() returns null, getHostServiceHeaders() returns empty headers {} and getHostServiceWsToken() returns null. The tRPC client in host-service-client.ts (line 26) passes a headers callback that will execute on every request before JWT is available, sending unauthenticated requests that trigger unnecessary 401s.
Callers must gate outbound requests/subscriptions until authentication is ready. Currently, getHostServiceHeaders and getHostServiceWsToken are used in:
host-service-client.ts:26– httpBatchLink headers callback (no token check)layout.tsx:102-103– WorkspaceTrpcProvider initialization (no token check)useWorkspaceEvent.ts:35– Event bus subscription (gated onenabled && hostUrl, but not token availability)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/lib/host-service-auth.ts` around lines 17 - 18,
getHostServiceHeaders and getHostServiceWsToken currently return empty
headers/null when getJwt() is not ready, causing unauthenticated requests;
change them to signal "auth not ready" explicitly (e.g., throw a specific
AuthNotReadyError or return a distinct sentinel like undefined) instead of
silently returning empty values, then update callers to gate operations until
JWT is available: in host-service-client.ts (httpBatchLink headers callback)
bail out or delay requests when auth is not ready, in layout.tsx
(WorkspaceTrpcProvider init) defer provider creation until token readiness, and
in useWorkspaceEvent.ts prevent subscribing unless getHostServiceWsToken
indicates auth is ready. Ensure you reference getHostServiceHeaders and
getHostServiceWsToken when implementing the checks and use a clear, single
mechanism (error or sentinel) for all callers.
| RELAY_PORT: z.coerce.number().int().positive().default(8080), | ||
| NEXT_PUBLIC_API_URL: z.string().url(), | ||
| DATABASE_URL: z.string().min(1), | ||
| REQUEST_TIMEOUT_MS: z.coerce.number().default(30_000), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the env.ts file
find . -path "*/apps/relay/src/env.ts" -type fRepository: superset-sh/superset
Length of output: 87
🏁 Script executed:
# Check if the file exists and read its contents around line 9
if [ -f "apps/relay/src/env.ts" ]; then
wc -l "apps/relay/src/env.ts"
echo "---"
head -20 "apps/relay/src/env.ts" | cat -n
fiRepository: superset-sh/superset
Length of output: 618
🏁 Script executed:
# Search for REQUEST_TIMEOUT_MS usage in the codebase
rg "REQUEST_TIMEOUT_MS" -n --context 3Repository: superset-sh/superset
Length of output: 2730
🏁 Script executed:
# Find and examine TunnelManager to understand how it uses REQUEST_TIMEOUT_MS
rg "class TunnelManager" -A 20 --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Look for TunnelManager constructor specifically
rg "constructor.*REQUEST_TIMEOUT_MS|constructor.*timeout" -B 5 -A 10 --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Search for where REQUEST_TIMEOUT_MS is actually used in TunnelManager
rg "TunnelManager" -B 2 -A 2 --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Search for TunnelManager class definition without file type flags
rg "class TunnelManager" -A 20Repository: superset-sh/superset
Length of output: 1221
🏁 Script executed:
# Find where TunnelManager is imported/defined
rg "TunnelManager" apps/relay/src/ -lRepository: superset-sh/superset
Length of output: 136
🏁 Script executed:
# Look at apps/relay/src/index.ts more thoroughly to see TunnelManager usage
cat -n apps/relay/src/index.tsRepository: superset-sh/superset
Length of output: 1295
🏁 Script executed:
# Search for where requestTimeoutMs is used in the TunnelManager class
rg "requestTimeoutMs" apps/relay/src/tunnel.ts -B 2 -A 2Repository: superset-sh/superset
Length of output: 467
🏁 Script executed:
# Get full context of tunnel.ts to see all usages
cat apps/relay/src/tunnel.tsRepository: superset-sh/superset
Length of output: 8653
Constrain REQUEST_TIMEOUT_MS to a positive integer.
Currently, REQUEST_TIMEOUT_MS accepts zero and negative values, which would cause setTimeout() to fire immediately, making all HTTP requests fail instantly. This configuration error should be prevented at validation time.
Suggested fix
- REQUEST_TIMEOUT_MS: z.coerce.number().default(30_000),
+ REQUEST_TIMEOUT_MS: z.coerce.number().int().positive().default(30_000),📝 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.
| REQUEST_TIMEOUT_MS: z.coerce.number().default(30_000), | |
| REQUEST_TIMEOUT_MS: z.coerce.number().int().positive().default(30_000), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/relay/src/env.ts` at line 9, REQUEST_TIMEOUT_MS currently allows zero
and negative numbers which will break setTimeout; update the zod schema for
REQUEST_TIMEOUT_MS to coerce to a number and constrain it to positive integers
(e.g., use .int().positive() or .min(1)) while preserving the default of 30000
so invalid config values fail validation at startup; target the
REQUEST_TIMEOUT_MS schema definition in env.ts to apply this change.
| }, | ||
| runtimeEnv: process.env, | ||
| emptyStringAsUndefined: true, | ||
| skipValidation: !!process.env.SKIP_ENV_VALIDATION, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file at the specified lines
cat -n apps/relay/src/env.ts | head -20Repository: superset-sh/superset
Length of output: 589
🏁 Script executed:
# Check if there are any tests for this env variable
fd -t f -e test -e spec | xargs rg -l "SKIP_ENV_VALIDATION" 2>/dev/null | head -5Repository: superset-sh/superset
Length of output: 174
🏁 Script executed:
# Look for other env boolean parsing patterns in the codebase
rg "process\.env\." apps/relay/src/env.ts -A 2 -B 2Repository: superset-sh/superset
Length of output: 176
🏁 Script executed:
# Check if there are similar boolean env patterns elsewhere
rg "!!(process\.env\.|!!)" apps/ -t ts -t tsx | head -20Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Look for any tests related to env setup
rg "skipValidation" apps/ -t tsRepository: superset-sh/superset
Length of output: 557
🏁 Script executed:
# Check if there's any documentation about SKIP_ENV_VALIDATION
fd -type f -name "*.md" -o -name "README*" | xargs rg -l "SKIP_ENV_VALIDATION" 2>/dev/nullRepository: superset-sh/superset
Length of output: 1390
🏁 Script executed:
# Look for package.json to understand `@t3-oss/env-core` version
cat apps/relay/package.json | rg -A 50 '"dependencies"' | head -30Repository: superset-sh/superset
Length of output: 446
🏁 Script executed:
# Check if there are environment examples or .env files
fd -type f \( -name ".env*" -o -name "*.env.example" \) | head -10Repository: superset-sh/superset
Length of output: 234
🏁 Script executed:
# Let's check the `@t3-oss/env-core` documentation or comments in the codebase
rg "skipValidation" apps/relay/src/env.ts -B 5 -A 5Repository: superset-sh/superset
Length of output: 272
🏁 Script executed:
# Check if there's a pattern in the desktop app which seems different
cat -n apps/desktop/src/main/env.main.ts | grep -A 3 "skipValidation"Repository: superset-sh/superset
Length of output: 241
🏁 Script executed:
# Look for any actual usage or tests of SKIP_ENV_VALIDATION
rg "SKIP_ENV_VALIDATION" apps/ -B 2 -A 2 | head -40Repository: superset-sh/superset
Length of output: 1492
🏁 Script executed:
# Check web_search for `@t3-oss/env-core` skipValidation parameter behavior
echo "Need to understand `@t3-oss/env-core` skipValidation behavior"Repository: superset-sh/superset
Length of output: 123
🌐 Web query:
@t3-oss/env-core skipValidation parameter boolean coercion documentation
💡 Result:
skipValidation in @t3-oss/env-core is a plain boolean option—the library does not coerce strings like "true"/"false" for you. The docs describe it simply as “Whether to skip validation of environment variables” with type boolean. [1]
So if you want to control it via an env var (which is usually a string), you must coerce it yourself, e.g.:
skipValidation: process.env.SKIP_ENV_VALIDATION === "true"(or whatever coercion rules you prefer). The T3 Env docs also explicitly show “skipping validation” as a boolean config and warn it can put runtime values and types out of sync. [2]
For string→boolean coercion patterns, their recipes show doing it in your schema via transforms/refinements (examples for “coerced” boolean vs strictly "true" | "false"). [3]
Sources
[1] JSR docs: BaseOptions.skipValidation (boolean) https://jsr.io/%40t3-oss/env-core/doc/~/BaseOptions.skipValidation
[2] T3 Env “Customization → Skipping validation” https://env.t3.gg/docs/customization
[3] T3 Env “Recipes → (Boolean coercion examples)” https://env.t3.gg/docs/recipes
Fix SKIP_ENV_VALIDATION boolean coercion—any non-empty string (including "false") will skip validation.
The !! operator converts any non-empty environment variable string to true. This means setting SKIP_ENV_VALIDATION=false or SKIP_ENV_VALIDATION=0 will incorrectly skip validation instead of enforcing it. The library expects an explicit boolean, not a string.
Suggested fix
- skipValidation: !!process.env.SKIP_ENV_VALIDATION,
+ skipValidation:
+ (process.env.SKIP_ENV_VALIDATION ?? "").toLowerCase() === "true",This pattern is repeated across multiple files (apps/web/src/env.ts, apps/docs/src/env.ts, apps/api/src/env.ts, apps/admin/src/env.ts, apps/marketing/src/env.ts, packages/trpc/src/env.ts) and should be corrected consistently.
📝 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.
| skipValidation: !!process.env.SKIP_ENV_VALIDATION, | |
| skipValidation: | |
| (process.env.SKIP_ENV_VALIDATION ?? "").toLowerCase() === "true", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/relay/src/env.ts` at line 13, The skipValidation property currently uses
boolean coercion (!!process.env.SKIP_ENV_VALIDATION) which treats any non-empty
string (including "false") as true; update the logic to parse the environment
string explicitly (e.g., compare process.env.SKIP_ENV_VALIDATION to "true" or
"1" in a case-insensitive way) so only explicit truthy strings enable skipping;
change the assignment for skipValidation in the env files (the key named
skipValidation using process.env.SKIP_ENV_VALIDATION) and apply the same fix
consistently across the other env files listed (apps/web/src/env.ts,
apps/docs/src/env.ts, apps/api/src/env.ts, apps/admin/src/env.ts,
apps/marketing/src/env.ts, packages/trpc/src/env.ts).
- Remove handshake ceremony (hello:ok) — tunnel is ready when WS opens - Merge events + terminal WS proxy into single generic handler - Inline tunnel route registration into index.ts - Remove debug logging from auth middleware - Simplify TunnelManager: remove handshakeComplete, sendRaw, WsChannel - Rename handleTunnelMessage → handleMessage
One fewer file. All relay routes (tunnel, HTTP proxy, WS proxy) and auth middleware live in index.ts alongside the server setup. 715 → 487 total lines.
Both relay and host-service had duplicate tunnel message types with a TODO to share them. Moved to packages/shared/src/tunnel-protocol.ts. Added ping/pong to the type system, removing the `as unknown` cast in tunnel-client. Merged hostConnected/hostDisconnected into setHostOnline.
- Parse messages as TunnelRequest, use switch instead of if/else chain - Use URLSearchParams for query string parsing - Log socket errors instead of silently swallowing - Use map.clear() in cleanupChannels - Move auth token from query string to Authorization header
deviceClientId and deviceName were hardcoded to null in the tRPC context, breaking workspace.create. The host-service already has getHashedDeviceId/getDeviceName — read directly instead of threading through context. Remove unused serviceVersion/protocolVersion fields.
Both desktop and host-service had identical copies of device-info.ts. Moved to packages/shared, deleted both originals, updated all imports.
Instead of casting a partial object as Session in the tRPC context,
add a dedicated jwtProcedure middleware that verifies the JWT bearer
token and provides a typed ctx with { userId, email, activeOrganizationId }.
Relay endpoints (checkHostAccess, setHostOnline) use jwtProcedure.
Session-based endpoints use protectedProcedure unchanged. Context
builder reverted to plain getSession.
Only positive results are cached — denied requests always hit the API, so granting access is instant. Revocations propagate within 5 minutes.
- Add %RELAY_URL% to connect-src CSP in index.html - Add RELAY_URL replacement in htmlEnvTransformPlugin - Add RELAY_URL to main process env schema - Pass RELAY_URL through coordinator buildEnv to child process
- JwtApiAuthProvider now mints JWTs from session token via /api/auth/token with 55-min cache (JWT expires in 1h). Fresh JWT on each tunnel reconnect. - Tunnel client injects PSK (HOST_SERVICE_SECRET) on proxied HTTP requests and WebSocket upgrades to the local host-service. - getAuthToken is now async to support JWT fetching. - connectRelay accepts authProvider + hostServiceSecret instead of raw token.
The host-service authenticates with JWTs, not session tokens. ensureV2Host is only called by the host-service, so it should use jwtProcedure which verifies JWTs and provides ctx.userId/ctx.activeOrganizationId.
organization.getActive uses protectedProcedure (session auth) which doesn't work with JWT-authenticated host-service calls. Add a JWT-compatible variant and use it in host.info.
…nnel loop The JWT's activeOrganizationId reflects the session's active org, not the specific org each host-service was spawned for. Multiple host-service processes sharing the same JWT would register as the same host, causing a connect/disconnect loop. - ensureV2Host now takes organizationId as input, validated against the JWT's organizationIds array - organizationId added to createApp config and HostServiceContext - jwtProcedure ctx now includes organizationIds array - Clean up HostServiceContext: remove unused deviceClientId/deviceName/ serviceVersion/protocolVersion fields, make api non-nullable
Hono's upgradeWebSocket callback doesn't have access to route wildcard params. Parse the path from the full URL instead. Also make HTTP proxy path extraction consistent.
Vite's define replaces process.env references at build time. When RELAY_URL isn't set, defineEnv produces undefined which breaks the Zod validation. Add a production default like the other env vars.
- Relay HTTP proxy now preserves query string (fixes tRPC GET batch requests) - Tunnel client no longer sends duplicate ws:close (onerror+onclose) - Log errors in silent catches (JWT verify, HTTP proxy failures) - Dockerfile: run as non-root user, add missing workspace deps, use --ignore-scripts to skip postinstall
Uses the official Turborepo Docker pattern with `turbo prune --docker` which auto-resolves the dependency graph. Three stages: prune, install, runtime. Pinned bun version, runs as non-root.
…nore - oven/bun image is Debian-based, use groupadd/useradd instead of alpine's addgroup/adduser - Copy patches/ into builder so bun install can find patchedDependencies referenced in root package.json - Add .dockerignore to exclude node_modules from build context (was 745MB) Verified locally: builds clean, container starts, /health returns ok.
Summary
Adds
apps/relay— a Hono server (Fly.io) that proxies traffic between clients and remote host-services via WebSocket tunnels. This enables remote presence: accessing a teammate's workspaces from your own machine.Architecture
apps/relay/): Stateless transport gateway. Validates JWTs, routes framed traffic between clients and host tunnels.v2_users_hostswith 5min TTL cache.What works
ensureV2Host)Known issues (follow-up)
useWorkspaceHostUrllocal service check commented out for testingFiles
apps/relay/— new relay service (Hono + node-ws)packages/host-service/src/tunnel/— tunnel client + typespackages/host-service/src/device-info.ts— self-generating machineIdapps/desktop/src/main/host-service/index.ts— tunnel setup in desktop entryapps/desktop/src/renderer/— CSP, env, auth fallback updatesTest plan
cd apps/relay && bun devRELAY_URLin.envSummary by cubic
Adds a Relay service and end-to-end tunnel that routes client traffic to remote host-services over a single WebSocket per host with JWT auth. Desktop now uses a new coordinator and local provider, and resolves remote hosts via the relay using machine-scoped routing to avoid multi‑org tunnel loops.
New
apps/relay(Hono +@hono/node-ws): multiplexes HTTP/WS under/hosts/:hostId/*over one WS per host with ping/pong; verifies JWTs via JWKS; checks access through APIdevice.checkHostAccesswith a 5‑minute positive cache; setsv2_hosts.is_onlinethroughdevice.setHostOnline; Dockerfile uses turbo prune and runs as non‑root; Fly config added.Shared: tunnel protocol moved to
@superset/shared/tunnel-protocol; device identity to@superset/shared/device-info.Host-service: strict
createApp(explicit config, migrations folder, and providers); newhost.info(org metadata + version, cached 1h);JwtApiAuthProvidermints/caches JWTs via/api/auth/token;device.ensureV2Hostmoved tojwtProcedureand now takesorganizationId;connectRelay(...)registers the host and connects whenRELAY_URLis set, injecting PSK for local requests.Desktop: replaced manager with
HostServiceCoordinatorandLocalHostServiceProviderexposingmachineIdandactiveHostUrl; host URL resolution usesmachineId; tray querieshost.info; CSP allows127.0.0.1and relay;RELAY_URLis validated in env, passed via CI/build and HTML transform; removed “Cloud Workspace” picker option.DB: migration adds
v2_hosts.is_onlineand dropslast_seen_at.Bug Fixes
ws:close; improved JWT verification and error logging; rejects unauthenticated tunnel connects.organizationIdtoensureV2Host; relay access checks now go through the API.RELAY_URLin desktop env and CI; tightened CSP to127.0.0.1.groupadd/useradd, copyingpatches/, and adding.dockerignore.Written for commit 65c88e4. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements