Skip to content

Commit 3c5518f

Browse files
committed
fix(security): harden auth, remove leaked artifacts, and close audit alerts
1 parent 8c268d5 commit 3c5518f

14 files changed

Lines changed: 374 additions & 255 deletions

File tree

.env.example

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,18 @@
77
# =============================================================================
88

99
# API server settings
10-
VLLM_STUDIO_HOST=0.0.0.0
10+
VLLM_STUDIO_HOST=127.0.0.1
1111
VLLM_STUDIO_PORT=8080
1212

13-
# Optional API authentication (leave empty for no auth)
13+
# Required when binding to a non-loopback host such as 0.0.0.0
1414
VLLM_STUDIO_API_KEY=
1515

16+
# Only set this to true for trusted local environments.
17+
# VLLM_STUDIO_ALLOW_UNAUTHENTICATED=false
18+
19+
# Optional browser allowlist for direct controller access (comma-separated origins)
20+
# VLLM_STUDIO_CORS_ORIGINS=http://localhost:3000,http://127.0.0.1:3000
21+
1622
# Inference backend port (where vLLM/SGLang runs)
1723
VLLM_STUDIO_INFERENCE_PORT=8000
1824

@@ -45,7 +51,7 @@ VLLM_STUDIO_DATA_DIR=./data
4551
# ExLLaMA v3 command template. Must resolve a runnable launch command.
4652
# Supports any binary with flags; include placeholders exactly as you want them to run.
4753
# Example: /opt/exllamav3/bin/exllama-server --model /models/my-model --port 8000
48-
# VLLM_STUDIO_EXLLAMAV3_COMMAND=exllama-server --host 0.0.0.0 --port 8000
54+
# VLLM_STUDIO_EXLLAMAV3_COMMAND=exllama-server --host 127.0.0.1 --port 8000
4955

5056
# Strict OpenAI model matching for /v1/chat/completions.
5157
# If true, only configured recipes are routable through the controller.

controller/src/config/env.ts

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export interface Config {
1414
host: string;
1515
port: number;
1616
api_key?: string;
17+
cors_origins?: string[];
1718
daytona_api_url?: string;
1819
daytona_api_key?: string;
1920
daytona_proxy_url?: string;
@@ -69,10 +70,45 @@ export const createConfig = (): Config => {
6970
: localDataDirectory;
7071
const defaultDatabasePath = resolve(defaultDataDirectory, "controller.db");
7172

73+
const isLoopbackHost = (value: string): boolean => {
74+
const normalized = value.trim().toLowerCase();
75+
return normalized === "127.0.0.1" || normalized === "localhost" || normalized === "::1";
76+
};
77+
78+
const parseBooleanFlag = (value: string | undefined): boolean => {
79+
if (!value) return false;
80+
return ["1", "true", "yes", "on"].includes(value.trim().toLowerCase());
81+
};
82+
83+
const normalizeOrigin = (value: string): string | null => {
84+
try {
85+
const origin = new URL(value.trim()).origin;
86+
return origin === "null" ? null : origin;
87+
} catch {
88+
return null;
89+
}
90+
};
91+
92+
const parseCorsOrigins = (value: string | undefined): string[] => {
93+
const defaults = [
94+
"http://localhost:3000",
95+
"http://127.0.0.1:3000",
96+
"http://localhost:3001",
97+
"http://127.0.0.1:3001",
98+
"http://host.docker.internal:3000",
99+
"http://host.docker.internal:3001",
100+
];
101+
const candidates =
102+
value && value.trim().length > 0 ? value.split(",").map((entry) => entry.trim()) : defaults;
103+
return [...new Set(candidates.map((entry) => normalizeOrigin(entry)).filter((entry): entry is string => Boolean(entry)))];
104+
};
105+
72106
const schema = z.object({
73-
VLLM_STUDIO_HOST: z.string().default("0.0.0.0"),
107+
VLLM_STUDIO_HOST: z.string().default("127.0.0.1"),
74108
VLLM_STUDIO_PORT: z.coerce.number().int().positive().default(8080),
75109
VLLM_STUDIO_API_KEY: z.string().optional(),
110+
VLLM_STUDIO_ALLOW_UNAUTHENTICATED: z.string().optional(),
111+
VLLM_STUDIO_CORS_ORIGINS: z.string().optional(),
76112
VLLM_STUDIO_DAYTONA_API_KEY: z.string().optional(),
77113
VLLM_STUDIO_DAYTONA_API_URL: z.string().optional(),
78114
VLLM_STUDIO_DAYTONA_PROXY_URL: z.string().optional(),
@@ -96,6 +132,7 @@ export const createConfig = (): Config => {
96132
});
97133

98134
const parsed = schema.parse(process.env);
135+
const host = parsed.VLLM_STUDIO_HOST.trim() || "127.0.0.1";
99136

100137
const strictOpenAIModels = parsed.VLLM_STUDIO_STRICT_OPENAI_MODELS;
101138
const strictOpenAIModelsEnabled = strictOpenAIModels
@@ -116,7 +153,7 @@ export const createConfig = (): Config => {
116153
activationPolicyRaw === "switch_on_request" ? "switch_on_request" : "load_if_idle";
117154

118155
const config: Config = {
119-
host: parsed.VLLM_STUDIO_HOST,
156+
host,
120157
port: parsed.VLLM_STUDIO_PORT,
121158
inference_port: parsed.VLLM_STUDIO_INFERENCE_PORT,
122159

@@ -127,6 +164,7 @@ export const createConfig = (): Config => {
127164
daytona_agent_mode: daytonaAgentMode,
128165
agent_fs_local_fallback: localAgentFsFallback,
129166
openai_model_activation_policy: openaiModelActivationPolicy,
167+
cors_origins: parseCorsOrigins(parsed.VLLM_STUDIO_CORS_ORIGINS),
130168
providers: [],
131169
};
132170

@@ -139,6 +177,14 @@ export const createConfig = (): Config => {
139177
if (parsed.VLLM_STUDIO_API_KEY) {
140178
config.api_key = parsed.VLLM_STUDIO_API_KEY;
141179
}
180+
181+
const allowUnauthenticated = parseBooleanFlag(parsed.VLLM_STUDIO_ALLOW_UNAUTHENTICATED);
182+
if (!config.api_key && !allowUnauthenticated && !isLoopbackHost(host)) {
183+
throw new Error(
184+
"VLLM_STUDIO_API_KEY is required when binding the controller to a non-loopback host. Set VLLM_STUDIO_ALLOW_UNAUTHENTICATED=true only for trusted local environments."
185+
);
186+
}
187+
142188
if (parsed.VLLM_STUDIO_DAYTONA_API_KEY) {
143189
config.daytona_api_key = parsed.VLLM_STUDIO_DAYTONA_API_KEY;
144190
}

controller/src/config/persisted-config.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// CRITICAL
2-
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
2+
import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
33
import { resolve } from "node:path";
44

55
export interface ProviderConfig {
@@ -64,7 +64,13 @@ export const savePersistedConfig = (
6464
writable[key] = value;
6565
}
6666
});
67-
mkdirSync(dataDirectory, { recursive: true });
67+
mkdirSync(dataDirectory, { recursive: true, mode: 0o700 });
6868
writeFileSync(path, JSON.stringify(next, null, 2));
69+
try {
70+
chmodSync(dataDirectory, 0o700);
71+
chmodSync(path, 0o600);
72+
} catch {
73+
// Ignore permission hardening failures on unsupported filesystems.
74+
}
6975
return next;
7076
};

controller/src/http/app.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,16 @@ import { createMutatingAuthMiddleware, createMutatingRateLimitMiddleware } from
2424
*/
2525
export const createApp = (context: AppContext): Hono => {
2626
const app = new Hono();
27+
const allowedCorsOrigins = context.config.cors_origins ?? [];
2728

2829
app.use(
2930
"*",
3031
cors({
31-
origin: "*",
32-
allowMethods: ["GET", "POST", "PUT", "DELETE", "OPTIONS"],
33-
allowHeaders: ["*"],
32+
origin: (origin) => (allowedCorsOrigins.includes(origin) ? origin : null),
33+
allowMethods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"],
34+
allowHeaders: ["Authorization", "Content-Type", "X-API-Key"],
35+
exposeHeaders: ["X-RateLimit-Limit", "X-RateLimit-Remaining", "X-RateLimit-Reset", "Retry-After"],
36+
maxAge: 600,
3437
})
3538
);
3639

controller/src/http/security-middleware.test.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ function createContext(apiKey?: string): AppContext {
2525
strict_openai_models: false,
2626
daytona_agent_mode: false,
2727
agent_fs_local_fallback: false,
28+
cors_origins: ["http://localhost:3000"],
2829
...(apiKey ? { api_key: apiKey } : {}),
2930
},
3031
logger: {
@@ -37,35 +38,35 @@ function createContext(apiKey?: string): AppContext {
3738
}
3839

3940
describe("security middleware", () => {
40-
it("blocks mutating requests without API key when configured", async () => {
41+
it("blocks read requests without API key when configured", async () => {
4142
resetMutatingRateLimitStoreForTests();
4243
const app = new Hono();
4344
const context = createContext("secret-token");
4445

4546
app.use("*", createMutatingAuthMiddleware(context));
46-
app.post("/secure", (ctx) => ctx.json({ ok: true }));
47+
app.get("/secure", (ctx) => ctx.json({ ok: true }));
4748

48-
const response = await app.request("/secure", { method: "POST" });
49+
const response = await app.request("/secure", { method: "GET" });
4950
expect(response.status).toBe(401);
5051
expect(await response.json()).toEqual({ detail: "Unauthorized" });
5152
});
5253

53-
it("accepts mutating requests with bearer token or x-api-key", async () => {
54+
it("accepts protected requests with bearer token or x-api-key", async () => {
5455
resetMutatingRateLimitStoreForTests();
5556
const app = new Hono();
5657
const context = createContext("secret-token");
5758

5859
app.use("*", createMutatingAuthMiddleware(context));
59-
app.post("/secure", (ctx) => ctx.json({ ok: true }));
60+
app.get("/secure", (ctx) => ctx.json({ ok: true }));
6061

6162
const bearer = await app.request("/secure", {
62-
method: "POST",
63+
method: "GET",
6364
headers: { Authorization: "Bearer secret-token" },
6465
});
6566
expect(bearer.status).toBe(200);
6667

6768
const apiKeyHeader = await app.request("/secure", {
68-
method: "POST",
69+
method: "GET",
6970
headers: { "X-API-Key": "secret-token" },
7071
});
7172
expect(apiKeyHeader.status).toBe(200);
@@ -83,6 +84,18 @@ describe("security middleware", () => {
8384
expect(response.status).toBe(200);
8485
});
8586

87+
it("allows public health checks without auth", async () => {
88+
resetMutatingRateLimitStoreForTests();
89+
const app = new Hono();
90+
const context = createContext("secret-token");
91+
92+
app.use("*", createMutatingAuthMiddleware(context));
93+
app.get("/health", (ctx) => ctx.json({ ok: true }));
94+
95+
const response = await app.request("/health", { method: "GET" });
96+
expect(response.status).toBe(200);
97+
});
98+
8699
it("rate limits mutating requests only", async () => {
87100
resetMutatingRateLimitStoreForTests();
88101
const app = new Hono();

controller/src/http/security-middleware.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { MiddlewareHandler } from "hono";
44
import type { AppContext } from "../types/context";
55

66
const MUTATING_METHODS = new Set(["POST", "PUT", "PATCH", "DELETE"]);
7+
const PUBLIC_PATHS = new Set(["/health"]);
78
const DEFAULT_RATE_LIMIT_WINDOW_MS = 60_000;
89
const DEFAULT_RATE_LIMIT_MAX_REQUESTS = 120;
910

@@ -22,6 +23,10 @@ function isMutatingRequest(method: string): boolean {
2223
return MUTATING_METHODS.has(method.toUpperCase());
2324
}
2425

26+
function isPublicRequest(method: string, path: string): boolean {
27+
return method.toUpperCase() === "OPTIONS" || PUBLIC_PATHS.has(path);
28+
}
29+
2530
function getClientIpFromRequestHeaders(header: (name: string) => string | undefined): string {
2631
const forwarded = header("x-forwarded-for")
2732
?.split(",")
@@ -63,7 +68,7 @@ function buildMutatingRateLimitKey(path: string, method: string, clientIp: strin
6368

6469
export function createMutatingAuthMiddleware(context: AppContext): MiddlewareHandler {
6570
return async (ctx, next) => {
66-
if (!isMutatingRequest(ctx.req.method)) {
71+
if (isPublicRequest(ctx.req.method, ctx.req.path)) {
6772
return next();
6873
}
6974

controller/src/services/daytona/toolbox-client.test.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ const createFileListResponse = (): Response =>
3232
headers: { "content-type": "application/json" },
3333
});
3434

35+
const toOriginAndPath = (value: string): string => {
36+
const parsed = new URL(value);
37+
return `${parsed.origin}${parsed.pathname}`;
38+
};
39+
3540
describe("daytona toolbox config", () => {
3641
it("derives proxy URL from API base URL", () => {
3742
expect(resolveDaytonaProxyBaseUrl("https://app.daytona.io/api")).toBe(
@@ -356,9 +361,15 @@ describe("daytona toolbox retry behavior", () => {
356361
const files = await client.listFiles("session-explicit", "");
357362
expect(files).toEqual([]);
358363
expect(
359-
calls.some((entry) => entry.url.includes("https://proxy.custom.daytona/toolbox/sandbox-explicit/files/folder"))
364+
calls.some(
365+
(entry) =>
366+
toOriginAndPath(entry.url) ===
367+
"https://proxy.custom.daytona/toolbox/sandbox-explicit/files/folder"
368+
)
360369
).toBe(true);
361-
expect(calls.some((entry) => entry.url.includes("/toolbox/toolbox/"))).toBe(false);
370+
expect(calls.some((entry) => new URL(entry.url).pathname.includes("/toolbox/toolbox/"))).toBe(
371+
false
372+
);
362373
} finally {
363374
globalThis.fetch = originalFetch;
364375
}

frontend/.env.example

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@ NEXT_PUBLIC_LITELLM_URL=http://localhost:4100
99
LITELLM_URL=http://localhost:4100
1010
LITELLM_MASTER_KEY=sk-master
1111

12-
# Optional API authentication
12+
# Optional controller API authentication
13+
# Prefer saving this server-side via /api/settings instead of persisting it in the browser.
1314
API_KEY=

0 commit comments

Comments
 (0)