fix(host-service): add CORS lockdown + PSK auth for local host-service#2927
Merged
Conversation
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis pull request implements host service authentication infrastructure by introducing per-organization secret generation, refactoring API authentication providers with Changes
Sequence DiagramsequenceDiagram
participant Client as Desktop Client
participant Manager as Host Service Manager
participant Process as Host Service Process
participant Auth as PSK Auth
participant Router as TRPC Router
Client->>Manager: getLocalPort(orgId)
Manager->>Process: spawn(secret)
Process->>Process: generate 32-byte hex secret
Process->>Process: set HOST_SERVICE_SECRET env var
Manager->>Manager: getSecret(orgId) → secret
Manager-->>Client: {port, secret}
Client->>Client: setHostServiceSecret(url, secret)
Client->>Process: POST /trpc with Authorization header
Process->>Auth: validate(request)
Auth->>Auth: extract Bearer token from header
Auth->>Auth: timingSafeEqual(token, secret)
Auth-->>Process: valid: true
Process->>Router: protectedProcedure checks ctx.isAuthenticated
alt isAuthenticated
Router->>Router: execute procedure logic
Router-->>Client: {result}
else not authenticated
Router-->>Client: TRPCError UNAUTHORIZED
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 40 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/serve.ts">
<violation number="1" location="packages/host-service/src/serve.ts:15">
P1: Do not log `HOST_SERVICE_SECRET`; this exposes the PSK and weakens the new inbound auth control.</violation>
</file>
<file name="apps/desktop/src/main/host-service/index.ts">
<violation number="1" location="apps/desktop/src/main/host-service/index.ts:27">
P1: Fail closed when HOST_SERVICE_SECRET is missing. The current fallback to `undefined` disables host auth entirely, leaving the WebSocket routes unauthenticated if the env var isn’t set.</violation>
</file>
<file name="packages/host-service/src/app.ts">
<violation number="1" location="packages/host-service/src/app.ts:95">
P1: WebSocket auth is fail-open: if `hostAuth` is omitted, terminal and filesystem WebSocket routes are exposed without authentication.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+27
to
+29
| const hostAuth = hostServiceSecret | ||
| ? new PskHostAuthProvider(hostServiceSecret) | ||
| : undefined; |
Contributor
There was a problem hiding this comment.
P1: Fail closed when HOST_SERVICE_SECRET is missing. The current fallback to undefined disables host auth entirely, leaving the WebSocket routes unauthenticated if the env var isn’t set.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/main/host-service/index.ts, line 27:
<comment>Fail closed when HOST_SERVICE_SECRET is missing. The current fallback to `undefined` disables host auth entirely, leaving the WebSocket routes unauthenticated if the env var isn’t set.</comment>
<file context>
@@ -10,26 +10,33 @@
const auth =
- authToken && cloudApiUrl ? new JwtAuthProvider(authToken) : undefined;
+ authToken && cloudApiUrl ? new JwtApiAuthProvider(authToken) : undefined;
+const hostAuth = hostServiceSecret
+ ? new PskHostAuthProvider(hostServiceSecret)
+ : undefined;
</file context>
Suggested change
| const hostAuth = hostServiceSecret | |
| ? new PskHostAuthProvider(hostServiceSecret) | |
| : undefined; | |
| if (!hostServiceSecret) { | |
| throw new Error("HOST_SERVICE_SECRET is required to start host-service"); | |
| } | |
| const hostAuth = new PskHostAuthProvider(hostServiceSecret); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsx`:
- Line 16: The provider currently calls setHostServiceSecret for registered org
services but never calls removeHostServiceSecret, causing the secrets Map to
grow; update HostServiceProvider to unregister secrets when services are removed
or when the component unmounts by tracking registered service URLs (from the
services Map/OrgService entries) and calling removeHostServiceSecret(url) for
each removed entry, e.g., add cleanup logic inside the useMemo that builds
services or add a useEffect cleanup that iterates services.values() on unmount
and calls removeHostServiceSecret for each service.url; ensure you reference
setHostServiceSecret and removeHostServiceSecret and perform removals whenever
an org is deregistered or on provider unmount.
- Around line 77-86: The renderer is using a cached secret (set via
setHostServiceSecret in addOrg) so when spawn() restarts host-service and issues
a new secret the client (created by getHostServiceClient) keeps using the stale
secret from the getLocalPort query; removeHostServiceSecret is unused. Fix by
implementing retry-on-401 in the client code: detect 401 responses from the
client returned by getHostServiceClient, call a function that re-queries the
latest port/secret (re-run the getLocalPort query or call a new tRPC helper),
update the secret via setHostServiceSecret, recreate the client for that orgId
(the map entry created in addOrg) and retry the failing request once;
alternatively implement a tRPC subscription that emits {port, secret} on spawn
and wire it to update the map and setHostServiceSecret so clients always get the
latest secret. Ensure removeHostServiceSecret is either removed or used
consistently.
🪄 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: 260666a6-9d3c-4605-bd89-ae5c4593d3c5
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
apps/desktop/src/lib/trpc/routers/host-service-manager/index.tsapps/desktop/src/main/host-service/index.tsapps/desktop/src/main/lib/host-service-manager.tsapps/desktop/src/renderer/lib/host-service-auth.tsapps/desktop/src/renderer/lib/host-service-client.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceTerminal/WorkspaceTerminal.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/providers/WorkspaceTrpcProvider/WorkspaceTrpcProvider.tsxapps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsxpackages/host-service/src/api/createApiClient/createApiClient.tspackages/host-service/src/app.tspackages/host-service/src/env.tspackages/host-service/src/index.tspackages/host-service/src/providers/auth/DeviceKeyAuthProvider/DeviceKeyAuthProvider.tspackages/host-service/src/providers/auth/DeviceKeyAuthProvider/index.tspackages/host-service/src/providers/auth/JwtAuthProvider/JwtAuthProvider.tspackages/host-service/src/providers/auth/JwtAuthProvider/index.tspackages/host-service/src/providers/auth/index.tspackages/host-service/src/providers/auth/types.tspackages/host-service/src/providers/host-auth/PskHostAuthProvider/PskHostAuthProvider.tspackages/host-service/src/providers/host-auth/PskHostAuthProvider/index.tspackages/host-service/src/providers/host-auth/index.tspackages/host-service/src/providers/host-auth/types.tspackages/host-service/src/serve.tspackages/host-service/src/trpc/index.tspackages/host-service/src/trpc/router/chat/chat.tspackages/host-service/src/trpc/router/cloud/cloud.tspackages/host-service/src/trpc/router/filesystem/filesystem.tspackages/host-service/src/trpc/router/git/git.tspackages/host-service/src/trpc/router/github/github.tspackages/host-service/src/trpc/router/project/project.tspackages/host-service/src/trpc/router/pull-requests/pull-requests.tspackages/host-service/src/trpc/router/workspace/workspace.tspackages/host-service/src/types.tspackages/scripts/package.jsonpackages/scripts/tsconfig.jsonpackages/workspace-client/src/index.tspackages/workspace-client/src/providers/WorkspaceClientProvider/WorkspaceClientProvider.tsxpackages/workspace-client/src/providers/WorkspaceClientProvider/index.ts
💤 Files with no reviewable changes (2)
- packages/scripts/package.json
- packages/scripts/tsconfig.json
| getHostServiceClient, | ||
| type HostServiceClient, | ||
| } from "renderer/lib/host-service-client"; | ||
| import { setHostServiceSecret } from "renderer/lib/host-service-auth"; |
Contributor
There was a problem hiding this comment.
Memory accumulation: removeHostServiceSecret is defined but never called.
Secrets are registered via setHostServiceSecret but never removed when organizations are deregistered or the provider unmounts. Over time, this causes unbounded growth of the secrets Map.
Consider cleaning up secrets when entries are removed from the services map, or in an unmount effect.
♻️ Suggested cleanup approach
// Option 1: Track and clean up in useMemo
const services = useMemo(() => {
const map = new Map<string, OrgService>();
const registeredUrls = new Set<string>();
const addOrg = (orgId: string, port: number, secret: string | null) => {
const url = `http://127.0.0.1:${port}`;
registeredUrls.add(url);
if (secret) {
setHostServiceSecret(url, secret);
}
// ... rest
};
// Return cleanup function or track for later cleanup
return map;
}, [/* deps */]);
// Option 2: Add cleanup effect when provider unmounts
useEffect(() => {
return () => {
for (const service of services.values()) {
removeHostServiceSecret(service.url);
}
};
}, [services]);Also applies to: 77-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsx`
at line 16, The provider currently calls setHostServiceSecret for registered org
services but never calls removeHostServiceSecret, causing the secrets Map to
grow; update HostServiceProvider to unregister secrets when services are removed
or when the component unmounts by tracking registered service URLs (from the
services Map/OrgService entries) and calling removeHostServiceSecret(url) for
each removed entry, e.g., add cleanup logic inside the useMemo that builds
services or add a useEffect cleanup that iterates services.values() on unmount
and calls removeHostServiceSecret for each service.url; ensure you reference
setHostServiceSecret and removeHostServiceSecret and perform removals whenever
an org is deregistered or on provider unmount.
Comment on lines
+77
to
86
| const addOrg = (orgId: string, port: number, secret: string | null) => { | ||
| const url = `http://127.0.0.1:${port}`; | ||
| if (secret) { | ||
| setHostServiceSecret(url, secret); | ||
| } | ||
| map.set(orgId, { | ||
| port, | ||
| url: `http://127.0.0.1:${port}`, | ||
| url, | ||
| client: getHostServiceClient(port), | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's any subscription or event mechanism for host-service restarts
rg -n "subscription|onRestart|secretChanged" --type=ts apps/desktop/src/lib/trpc/routers/host-service-manager/
rg -n "subscription|onRestart|secretChanged" --type=ts apps/desktop/src/main/lib/host-service-manager.tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# First, check the full HostServiceProvider implementation
cat -n apps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsxRepository: superset-sh/superset
Length of output: 4364
🏁 Script executed:
# Check the host-service-manager implementation to see how secrets are generated and managed
cat -n apps/desktop/src/main/lib/host-service-manager.tsRepository: superset-sh/superset
Length of output: 11494
🏁 Script executed:
# Check if removeHostServiceSecret is ever called/used
rg -n "removeHostServiceSecret" --type=ts apps/desktop/Repository: superset-sh/superset
Length of output: 182
🏁 Script executed:
# Check how getHostServiceClient is implemented and if it has 401 retry logic
fd -e ts -e tsx ".*" apps/desktop/src/lib/ | xargs rg -l "getHostServiceClient"Repository: superset-sh/superset
Length of output: 48
🏁 Script executed:
# Find the getLocalPort router implementation
fd -type f "host-service-manager" apps/desktop/src/lib/trpc/routers/ | head -5Repository: superset-sh/superset
Length of output: 234
🏁 Script executed:
# Search for getLocalPort query definition
rg -n "getLocalPort" --type=ts apps/desktop/src/lib/trpc/routers/Repository: superset-sh/superset
Length of output: 161
🏁 Script executed:
# Check removeHostServiceSecret usage throughout the codebase
rg -n "removeHostServiceSecret" --type=ts apps/desktop/Repository: superset-sh/superset
Length of output: 182
🏁 Script executed:
# Read the full host-service-manager router implementation
cat -n apps/desktop/src/lib/trpc/routers/host-service-manager/index.tsRepository: superset-sh/superset
Length of output: 1313
🏁 Script executed:
# Double-check removeHostServiceSecret usage with more context
rg -B2 -A2 "removeHostServiceSecret" --type ts apps/desktop/Repository: superset-sh/superset
Length of output: 414
🏁 Script executed:
# Verify if there's any 401 retry logic in the host-service client
cat -n apps/desktop/src/renderer/lib/host-service-client.tsRepository: superset-sh/superset
Length of output: 1228
🏁 Script executed:
# Check if there's any mechanism to invalidate or refetch on app focus/resume
rg -n "refetch|invalidate|onFocus" --type ts apps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/Repository: superset-sh/superset
Length of output: 46
Stale secret after host-service restart causes authentication failures.
When the host-service crashes and respawns, spawn() generates a new secret, but getLocalPort is a query (not a subscription). The renderer's useQuery() only refreshes when activeOrganizationId changes. Since a service restart doesn't change the organization ID, clients continue using the cached stale secret and receive 401 errors until getLocalPort is manually re-queried.
The removeHostServiceSecret() function is also dead code—never called anywhere in the codebase.
Consider either:
- Add a tRPC subscription that emits when a restart occurs with the new
{ port, secret } - Implement 401 retry logic that re-fetches the secret on authentication failures
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsx`
around lines 77 - 86, The renderer is using a cached secret (set via
setHostServiceSecret in addOrg) so when spawn() restarts host-service and issues
a new secret the client (created by getHostServiceClient) keeps using the stale
secret from the getLocalPort query; removeHostServiceSecret is unused. Fix by
implementing retry-on-401 in the client code: detect 401 responses from the
client returned by getHostServiceClient, call a function that re-queries the
latest port/secret (re-run the getLocalPort query or call a new tRPC helper),
update the secret via setHostServiceSecret, recreate the client for that orgId
(the map entry created in addOrg) and retry the failing request once;
alternatively implement a tRPC subscription that emits {port, secret} on spawn
and wire it to update the map and setHostServiceSecret so clients always get the
latest secret. Ensure removeHostServiceSecret is either removed or used
consistently.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsx (1)
94-95:⚠️ Potential issue | 🟡 MinorAdd secret cleanup when services disappear or have no secret.
Current flow only sets secrets; it never clears them for dropped org services (or null-secret cases), so stale auth entries can persist.
🧹 Minimal cleanup patch
-import { setHostServiceSecret } from "renderer/lib/host-service-auth"; +import { + removeHostServiceSecret, + setHostServiceSecret, +} from "renderer/lib/host-service-auth"; const addOrg = (orgId: string, port: number, secret: string | null) => { const url = `http://127.0.0.1:${port}`; if (secret) { setHostServiceSecret(url, secret); + } else { + removeHostServiceSecret(url); } ... }; +useEffect(() => { + return () => { + for (const service of services.values()) { + removeHostServiceSecret(service.url); + } + }; +}, [services]);Also applies to: 104-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsx` around lines 94 - 95, The code only ever sets secrets via addOrg(orgId, cached.port, cached.secret ?? null) and never clears stale auth entries when a service is removed or when cached.secret is null; update the flow so that when a cached service is absent or cached.secret is falsy you explicitly clear the secret (e.g., call the project’s clear/remove API such as removeOrg(orgId) or updateOrgSecret(orgId, null)) instead of leaving stale state, and apply the same fix in the nearby branch around lines handling addOrg at 104-108; locate the addOrg usage and add a branch that calls the appropriate clear function (removeOrg or updateOrgSecret) when cached.secret is null or the service no longer exists.
🤖 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/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsx`:
- Around line 77-81: addOrg (inside the useMemo) is performing a side-effect by
calling setHostServiceSecret during render; remove that mutation from the
memoized addOrg to keep it pure, and instead synchronize the secret in a
committed effect: have addOrg only compute/return the url (and update any local
state like host/port/secret if needed), then add a useEffect that depends on the
secret/url state and calls setHostServiceSecret(url, secret) when both are
present; reference addOrg, setHostServiceSecret, the existing useMemo, and
replace the in-render call with a useEffect-based call so secret sync runs only
after commit.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsx`:
- Around line 94-95: The code only ever sets secrets via addOrg(orgId,
cached.port, cached.secret ?? null) and never clears stale auth entries when a
service is removed or when cached.secret is null; update the flow so that when a
cached service is absent or cached.secret is falsy you explicitly clear the
secret (e.g., call the project’s clear/remove API such as removeOrg(orgId) or
updateOrgSecret(orgId, null)) instead of leaving stale state, and apply the same
fix in the nearby branch around lines handling addOrg at 104-108; locate the
addOrg usage and add a branch that calls the appropriate clear function
(removeOrg or updateOrgSecret) when cached.secret is null or the service no
longer exists.
🪄 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: 8ec902bc-06cb-44fd-8212-d2bebfa59f6e
📒 Files selected for processing (6)
apps/desktop/src/renderer/lib/host-service-auth.tsapps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsxpackages/host-service/src/app.tspackages/host-service/src/index.tspackages/host-service/src/trpc/index.tspackages/host-service/src/trpc/router/chat/chat.ts
✅ Files skipped from review due to trivial changes (1)
- packages/host-service/src/app.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/host-service/src/trpc/index.ts
- packages/host-service/src/index.ts
- packages/host-service/src/trpc/router/chat/chat.ts
Comment on lines
+77
to
+81
| const addOrg = (orgId: string, port: number, secret: string | null) => { | ||
| const url = `http://127.0.0.1:${port}`; | ||
| if (secret) { | ||
| setHostServiceSecret(url, secret); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify render-phase mutation inside useMemo in HostServiceProvider.
rg -n -C3 'useMemo\(|setHostServiceSecret\(' apps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsxRepository: superset-sh/superset
Length of output: 809
🏁 Script executed:
# Locate and examine setHostServiceSecret implementation
fd -e ts -e tsx "host-service-auth" apps/desktop/src/renderer/Repository: superset-sh/superset
Length of output: 114
🏁 Script executed:
# Also check the full HostServiceProvider file to understand context
wc -l apps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsxRepository: superset-sh/superset
Length of output: 169
🏁 Script executed:
# Read the setHostServiceSecret implementation
cat -n apps/desktop/src/renderer/lib/host-service-auth.tsRepository: superset-sh/superset
Length of output: 752
🏁 Script executed:
# Read the full HostServiceProvider to understand context and dependencies
cat -n apps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsxRepository: superset-sh/superset
Length of output: 4364
Move secret synchronization out of useMemo and into useEffect.
At line 80, setHostServiceSecret mutates module-level auth state during render. In React's strict mode and concurrent rendering, render can be replayed or aborted, causing stale mutations to persist. Keep the memoized function pure and sync secrets in a committed effect.
♻️ Suggested refactor
-import { setHostServiceSecret } from "renderer/lib/host-service-auth";
+import {
+ removeHostServiceSecret,
+ setHostServiceSecret,
+} from "renderer/lib/host-service-auth";
export interface OrgService {
port: number;
url: string;
client: HostServiceClient;
+ secret: string | null;
}
const services = useMemo(() => {
const map = new Map<string, OrgService>();
const addOrg = (orgId: string, port: number, secret: string | null) => {
const url = `http://127.0.0.1:${port}`;
- if (secret) {
- setHostServiceSecret(url, secret);
- }
map.set(orgId, {
port,
url,
client: getHostServiceClient(port),
+ secret,
});
};
...
}, [orgIds, utils, activeOrganizationId, activePortData]);
+useEffect(() => {
+ const currentUrls = new Set<string>();
+ for (const service of services.values()) {
+ currentUrls.add(service.url);
+ if (service.secret) {
+ setHostServiceSecret(service.url, service.secret);
+ } else {
+ removeHostServiceSecret(service.url);
+ }
+ }
+ return () => {
+ for (const url of currentUrls) {
+ removeHostServiceSecret(url);
+ }
+ };
+}, [services]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/HostServiceProvider/HostServiceProvider.tsx`
around lines 77 - 81, addOrg (inside the useMemo) is performing a side-effect by
calling setHostServiceSecret during render; remove that mutation from the
memoized addOrg to keep it pure, and instead synchronize the secret in a
committed effect: have addOrg only compute/return the url (and update any local
state like host/port/secret if needed), then add a useEffect that depends on the
secret/url state and calls setHostServiceSecret(url, secret) when both are
present; reference addOrg, setHostServiceSecret, the existing useMemo, and
replace the in-render call with a useEffect-based call so secret sync runs only
after commit.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/host-service/src/app.ts`:
- Around line 94-106: The WebSocket routes (/terminal/* and
/workspace-filesystem/*) are left open when options?.hostAuth is falsy, creating
a mismatch with tRPC's fail-closed behavior; update the registration so that
instead of skipping middleware when options?.hostAuth is missing, you always
call app.use("/terminal/*", ...) and app.use("/workspace-filesystem/*", ...)
with a wsAuth-like MiddlewareHandler that either delegates to
hostAuth.validate/validateToken when hostAuth exists or immediately responds
with 401 Unauthorized when hostAuth is absent; modify the wsAuth construction to
reference options?.hostAuth (or hostAuth variable) and add the explicit deny
branch so these routes default to fail-closed like isAuthenticated-protected
tRPC procedures.
🪄 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: 7ad14791-ac90-4066-afff-5697ab199b4a
📒 Files selected for processing (4)
apps/desktop/src/renderer/lib/host-service-auth.tspackages/host-service/src/app.tspackages/host-service/src/providers/host-auth/types.tspackages/workspace-client/src/providers/WorkspaceClientProvider/WorkspaceClientProvider.tsx
✅ Files skipped from review due to trivial changes (2)
- packages/host-service/src/providers/host-auth/types.ts
- apps/desktop/src/renderer/lib/host-service-auth.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/workspace-client/src/providers/WorkspaceClientProvider/WorkspaceClientProvider.tsx
Comment on lines
+94
to
+106
| if (options?.hostAuth) { | ||
| const { hostAuth } = options; | ||
| const wsAuth: MiddlewareHandler = async (c, next) => { | ||
| const token = c.req.query("token"); | ||
| const authorized = | ||
| (await hostAuth.validate(c.req.raw)) || | ||
| (token && (await hostAuth.validateToken(token))); | ||
| if (!authorized) return c.json({ error: "Unauthorized" }, 401); | ||
| return next(); | ||
| }; | ||
| app.use("/terminal/*", wsAuth); | ||
| app.use("/workspace-filesystem/*", wsAuth); | ||
| } |
Contributor
There was a problem hiding this comment.
Inconsistent fail-open behavior for WebSocket routes when hostAuth is not configured.
When options?.hostAuth is falsy, no auth middleware is applied to /terminal/* and /workspace-filesystem/*, leaving these routes publicly accessible. Meanwhile, tRPC protected procedures fail-closed (isAuthenticated = false blocks all access).
This inconsistency could lead to security gaps if hostAuth configuration is accidentally omitted. Consider matching the tRPC fail-closed behavior:
🛡️ Proposed fix to fail-closed for WebSocket routes
- if (options?.hostAuth) {
- const { hostAuth } = options;
- const wsAuth: MiddlewareHandler = async (c, next) => {
- const token = c.req.query("token");
- const authorized =
- (await hostAuth.validate(c.req.raw)) ||
- (token && (await hostAuth.validateToken(token)));
- if (!authorized) return c.json({ error: "Unauthorized" }, 401);
- return next();
- };
- app.use("/terminal/*", wsAuth);
- app.use("/workspace-filesystem/*", wsAuth);
- }
+ const wsAuth: MiddlewareHandler = async (c, next) => {
+ const { hostAuth } = options ?? {};
+ if (!hostAuth) {
+ return c.json({ error: "Unauthorized" }, 401);
+ }
+ const token = c.req.query("token");
+ const authorized =
+ (await hostAuth.validate(c.req.raw)) ||
+ (token && (await hostAuth.validateToken(token)));
+ if (!authorized) return c.json({ error: "Unauthorized" }, 401);
+ return next();
+ };
+ app.use("/terminal/*", wsAuth);
+ app.use("/workspace-filesystem/*", wsAuth);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/host-service/src/app.ts` around lines 94 - 106, The WebSocket routes
(/terminal/* and /workspace-filesystem/*) are left open when options?.hostAuth
is falsy, creating a mismatch with tRPC's fail-closed behavior; update the
registration so that instead of skipping middleware when options?.hostAuth is
missing, you always call app.use("/terminal/*", ...) and
app.use("/workspace-filesystem/*", ...) with a wsAuth-like MiddlewareHandler
that either delegates to hostAuth.validate/validateToken when hostAuth exists or
immediately responds with 401 Unauthorized when hostAuth is absent; modify the
wsAuth construction to reference options?.hostAuth (or hostAuth variable) and
add the explicit deny branch so these routes default to fail-closed like
isAuthenticated-protected tRPC procedures.
siarhei-belavus
pushed a commit
to siarhei-belavus/localset
that referenced
this pull request
Mar 30, 2026
superset-sh#2927) * Clean enougH * fix: biome lint/format fixes * chore: remove unnecessary comments, clean up ws auth middleware * fix: biome format fix * fix: fix flaky external worktree test, remove broken workspaceRun test * Clean enougH (cherry picked from commit 23b26c2)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes pentest finding SS-004 (SUPER-390): the host-service had wide-open CORS (
Access-Control-Allow-Origin: *) and no inbound authentication — anyone who could reach the port had full access to terminal, filesystem, git, and all other endpoints.allowedOrigins(defaults to empty = reject all cross-origin). Desktop passes["http://127.0.0.1"].HostAuthProviderinterface withPskHostAuthProviderimplementation. Desktop generates arandomBytes(32)secret per host-service process, passed via env var. All tRPC routers (excepthealth) useprotectedProcedure. WebSocket routes (/terminal/*,/workspace-filesystem/*) validate via bearer header or?token=query param.host-service-auth.ts) —HostServiceProviderwrites secrets, all clients read lazily viaheaders()callbacks.WorkspaceClientProvideraccepts injectedheaders/wsTokencallbacks for environment-agnostic auth.AuthProvider→ApiAuthProviderto distinguish from the newHostAuthProvider.env.tswith Zod schema forserve.tsconfig (no more silent fallbacks).Test plan
bun run typecheckpasses (22/22)curl http://127.0.0.1:<port>/trpc/health.checksucceeds (public)curl http://127.0.0.1:<port>/trpc/chat.listMessageswithout auth → 401fetch()from browser devtools → blocked by CORSbun run devinpackages/host-service→ secret printed, endpoints require authSummary by cubic
Lock down local
@superset/host-servicewith an explicit CORS allowlist and pre-shared key (PSK) host authentication. Addresses pentest finding SS-004 (SUPER-390) by blocking unauthenticated cross-origin and local access.New Features
allowedOrigins(server option) orCORS_ORIGINS(env). Default denies all; Desktop allowshttp://127.0.0.1.HostAuthProviderandPskHostAuthProvider. ValidatesAuthorization: Bearer <secret>and?token=<secret>for WebSockets (/terminal/*,/workspace-filesystem/*).protectedProcedure; most routers switched to protected;healthremains public.HostServiceManager(router now returns{ port, secret });HostServiceProviderstores it; renderer injects headers/tokens viahost-service-auth.ts.WorkspaceClientProvideracceptsheaders/wsTokenand exposesuseWorkspaceWsUrl; terminal now uses it.serve: newenv.tsvalidates config (secret, port, CORS origins); applies CORS and host auth.Migration
@superset/host-service:AuthProvider→ApiAuthProvider;JwtAuthProvider→JwtApiAuthProvider;DeviceKeyAuthProvider→DeviceKeyApiAuthProvider.serve: setHOST_SERVICE_SECRETand optionalCORS_ORIGINS. SendAuthorization: Bearer <secret>for tRPC; append?token=<secret>for WS upgrades.Written for commit 913c50a. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Security
Refactoring