From 2e9c58c0c2bb3be26ded519b5107d2a395bdaddb Mon Sep 17 00:00:00 2001 From: Ayush Date: Mon, 26 Oct 2026 15:04:47 +0530 Subject: [PATCH 1/2] fix(server): accept signed links only for the methods their pages use --- apps/server/src/app.ts | 10 +++++++--- tests/api.test.ts | 37 +++++++++++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index 4d935ea51..b13a12f57 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -131,10 +131,14 @@ export async function createApp( ); }); app.use("/api/*", async (c, next) => { + // A signed URL grants only the requests its page makes: reads, and the console's input. + const method = c.req.method === "HEAD" ? "GET" : c.req.method; const signedRoute = - /^\/api\/files\/[^/]+\/content$|^\/api\/browsers\/[^/]+\/(?:preview|console)$/.test( - c.req.path, - ); + (method === "GET" && + /^\/api\/files\/[^/]+\/content$|^\/api\/browsers\/[^/]+\/(?:preview|console)$/.test( + c.req.path, + )) || + (method === "POST" && /^\/api\/browsers\/[^/]+\/console$/.test(c.req.path)); const owner = signedRoute && c.req.query("signature") ? auth.verify(new URL(c.req.url)) diff --git a/tests/api.test.ts b/tests/api.test.ts index 4c98ab6bd..c987d59f7 100644 --- a/tests/api.test.ts +++ b/tests/api.test.ts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { createHash } from "node:crypto"; +import { createHash, randomUUID } from "node:crypto"; import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -18,6 +18,7 @@ import { createSamplePdf } from "../packages/integrations/src/pdf.ts"; let db: Store, app: Awaited>["app"], + auth: Auth, agent: AgentService, config: Config, token: string, @@ -37,7 +38,7 @@ before(async () => { googleRedirectUri: "http://localhost:8787/api/google/callback", allowedOrigins: ["http://localhost:8081"], }; - ({ app, agent } = await createApp(db, config)); + ({ app, agent, auth } = await createApp(db, config)); const response = await app.request("/api/session", { method: "POST", headers: { "Content-Type": "application/json" }, @@ -388,3 +389,35 @@ test("guided document delegation streams a rich tool result bound to its saved t assert.equal(task?.kind, "document"); assert.equal(task?.input.messageId, "mail-fieldtrip"); }); + +test("signed links authorize only the requests their page makes", async () => { + const link = (path: string) => { + const url = new URL(auth.sign("local-user", path)); + return `${url.pathname}${url.search}`; + }; + const file = link(`/api/files/${randomUUID()}/content`); + const consoleLink = link(`/api/browsers/${randomUUID()}/console`); + // Accepted links reach the handler, which then finds no such resource. + assert.equal((await app.request(file)).status, 404); + assert.equal((await app.request(file, { method: "HEAD" })).status, 404); + assert.equal((await app.request(consoleLink)).status, 404); + assert.equal( + ( + await app.request(consoleLink, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ type: "key", key: "Enter" }), + }) + ).status, + 404, + ); + // Any other method needs a session, so a route added later cannot inherit link access. + for (const [path, method] of [ + [file, "PUT"], + [file, "DELETE"], + [file, "POST"], + [consoleLink, "DELETE"], + [consoleLink, "PATCH"], + ]) + assert.equal((await app.request(path, { method })).status, 401, `${method} ${path}`); +}); From 120f2cb71da2c1bf05afef1f4d475a0f865f81ec Mon Sep 17 00:00:00 2001 From: Ayush Date: Mon, 26 Oct 2026 15:04:47 +0530 Subject: [PATCH 2/2] fix(computer): give new installs their own Docker deployment name --- .env.example | 5 ++-- apps/server/src/app.ts | 3 ++- apps/server/src/auth.ts | 37 ++++++++++++++----------- apps/server/src/computer.ts | 24 +++++++++++++++-- apps/server/src/config.ts | 3 ++- tests/auth-startup.test.ts | 54 ++++++++++++++++++++++++++++++++++++- tests/config.test.ts | 3 +++ tests/helpers/computer.ts | 2 ++ tests/model-worker.test.ts | 1 + 9 files changed, 109 insertions(+), 23 deletions(-) diff --git a/.env.example b/.env.example index 4ffae83bb..36e6580c9 100644 --- a/.env.example +++ b/.env.example @@ -76,8 +76,9 @@ CPK_INTELLIGENCE_API_KEY= # Docker must be reachable from the API and any separate task worker. COMPUTER_ENABLED=false COMPUTER_IMAGE=openmuse-computer:local -# Stable deployment name shared by API and worker; defaults to a hash of PUBLIC_API_URL. -# COMPUTER_DEPLOYMENT_ID=my-openmuse +# Optional deployment name shared by API and worker. When unset, a new install keeps a random +# name in DATA_DIR; an install created before that keeps a hash of PUBLIC_API_URL. +# COMPUTER_DEPLOYMENT_ID= # e.g. from `openssl rand -hex 12` # Optional scoped Docker engine, e.g. DOCKER_CONTEXT=colima-openmuse # Or run each owner's computer in an E2B Desktop sandbox instead of Docker # (graphical desktop with internet access; see docs/COMPUTER.md): diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index b13a12f57..bff91dd2f 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -15,7 +15,7 @@ import { ActionService } from "./actions.ts"; import { agentConfigured, makeRuntime } from "./agent.ts"; import { createAuth } from "./auth.ts"; import { BrowserService } from "./browser.ts"; -import { ComputerService, type DockerRunner } from "./computer.ts"; +import { ComputerService, type DockerRunner, resolveComputerDeployment } from "./computer.ts"; import { computerRoutes } from "./computer-routes.ts"; import { assertApiDeploymentConfig, type Config } from "./config.ts"; import type { Store } from "./db.ts"; @@ -33,6 +33,7 @@ export async function createApp( options: { docker?: DockerRunner } = {}, ) { assertApiDeploymentConfig(config); + await resolveComputerDeployment(config); const auth = await createAuth(db, config), files = new Files(db, config, auth), google = new GoogleAuth(db, config), diff --git a/apps/server/src/auth.ts b/apps/server/src/auth.ts index 5cf51cf21..0c63c921c 100644 --- a/apps/server/src/auth.ts +++ b/apps/server/src/auth.ts @@ -76,24 +76,29 @@ export class Auth { } export async function createAuth(db: Store, config: Config) { await mkdir(config.dataDir, { recursive: true, mode: 0o700 }); - const path = join(config.dataDir, "session-signing-key"); - let key: string; + const key = await readOrCreate(join(config.dataDir, "session-signing-key"), () => + randomBytes(32).toString("base64"), + ); + return new Auth(db, config, key); +} +/** Reads a private file, or creates it without replacing another process's value. */ +export async function readOrCreate(path: string, create: () => string) { try { - key = await readFile(path, "utf8"); + return await readFile(path, "utf8"); } catch (error) { if (!(error instanceof Error && "code" in error && error.code === "ENOENT")) throw error; - key = randomBytes(32).toString("base64"); - const temporary = `${path}.${randomBytes(16).toString("hex")}.tmp`; - await writeFile(temporary, key, { mode: 0o600, flag: "wx" }); - try { - // Publish only complete bytes, without replacing another process's key. - await link(temporary, path); - } catch (error) { - if (!(error instanceof Error && "code" in error && error.code === "EEXIST")) throw error; - key = await readFile(path, "utf8"); - } finally { - await unlink(temporary); - } } - return new Auth(db, config, key); + let value = create(); + const temporary = `${path}.${randomBytes(16).toString("hex")}.tmp`; + await writeFile(temporary, value, { mode: 0o600, flag: "wx" }); + try { + // Publish only complete bytes, without replacing another process's value. + await link(temporary, path); + } catch (error) { + if (!(error instanceof Error && "code" in error && error.code === "EEXIST")) throw error; + value = await readFile(path, "utf8"); + } finally { + await unlink(temporary); + } + return value; } diff --git a/apps/server/src/computer.ts b/apps/server/src/computer.ts index d4eab19e2..5ea5cce96 100644 --- a/apps/server/src/computer.ts +++ b/apps/server/src/computer.ts @@ -1,6 +1,6 @@ import { spawn } from "node:child_process"; -import { createHash, randomUUID } from "node:crypto"; -import { mkdir, readFile, rename, unlink, writeFile } from "node:fs/promises"; +import { createHash, randomBytes, randomUUID } from "node:crypto"; +import { access, mkdir, readFile, rename, unlink, writeFile } from "node:fs/promises"; import { join, posix } from "node:path"; import { z } from "zod"; import type { @@ -8,6 +8,7 @@ import type { ComputerDirectory, ComputerSnapshot, } from "../../../packages/domain/src/computer.ts"; +import { readOrCreate } from "./auth.ts"; import { type ComputerBackend, DesktopComputerBackend, @@ -149,6 +150,25 @@ export function computerIdentity(config: Config, owner: string) { } as Record, }; } +/** + * Without COMPUTER_DEPLOYMENT_ID, a new install gets a random deployment name, so two installs + * on one Docker engine never derive the same computer. An install that already has a signing + * key started before this name existed: it keeps the PUBLIC_API_URL name its workspace uses. + * Runs before createAuth, so a new install writes its name before its signing key. The key is + * checked first, so a process starting alongside sees the name whenever it sees the key. + */ +export async function resolveComputerDeployment(config: Config) { + if (config.computerDeploymentId !== undefined) return; + await mkdir(config.dataDir, { recursive: true, mode: 0o700 }); + const path = join(config.dataDir, "computer-deployment-id"); + const exists = (file: string) => + access(file).then( + () => true, + () => false, + ); + if ((await exists(join(config.dataDir, "session-signing-key"))) && !(await exists(path))) return; + config.computerDeploymentId = await readOrCreate(path, () => randomBytes(16).toString("hex")); +} export function workspacePath(path: string): string { if ( path.includes("\0") || diff --git a/apps/server/src/config.ts b/apps/server/src/config.ts index d34a90929..25797a979 100644 --- a/apps/server/src/config.ts +++ b/apps/server/src/config.ts @@ -159,7 +159,8 @@ export function readConfig(): Config { webSearchEnabled: process.env.WEB_SEARCH_ENABLED !== "false", computerEnabled: process.env.COMPUTER_ENABLED === "true", computerImage: process.env.COMPUTER_IMAGE ?? "openmuse-computer:local", - computerDeploymentId: process.env.COMPUTER_DEPLOYMENT_ID, + // A blank line from .env is unset; otherwise every such install would share one name. + computerDeploymentId: process.env.COMPUTER_DEPLOYMENT_ID?.trim() || undefined, computerProvider, computerE2bTemplate: process.env.COMPUTER_E2B_TEMPLATE?.trim() || "desktop", e2bApiKey, diff --git a/tests/auth-startup.test.ts b/tests/auth-startup.test.ts index bcf39823b..27201e41b 100644 --- a/tests/auth-startup.test.ts +++ b/tests/auth-startup.test.ts @@ -3,9 +3,11 @@ import { mkdtemp, readdir, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import test from "node:test"; +import { createApp } from "../apps/server/src/app.ts"; import { createAuth } from "../apps/server/src/auth.ts"; +import { computerIdentity, resolveComputerDeployment } from "../apps/server/src/computer.ts"; import type { Config } from "../apps/server/src/config.ts"; -import type { Store } from "../apps/server/src/db.ts"; +import { createStore, type Store } from "../apps/server/src/db.ts"; function config(dataDir: string): Config { return { @@ -47,3 +49,53 @@ test("startup preserves an existing signing key and its links", async (t) => { assert.equal(await readFile(join(directory, "session-signing-key"), "utf8"), key); assert.deepEqual(await readdir(directory), ["session-signing-key"]); }); + +test("new installs get their own computer name, shared by concurrent first starts", async (t) => { + const first = await mkdtemp(join(tmpdir(), "openmuse-deployment-a-")); + const second = await mkdtemp(join(tmpdir(), "openmuse-deployment-b-")); + t.after(() => rm(first, { recursive: true, force: true })); + t.after(() => rm(second, { recursive: true, force: true })); + const starts = Array.from({ length: 8 }, () => config(first)); + await Promise.all(starts.map(resolveComputerDeployment)); + const id = await readFile(join(first, "computer-deployment-id"), "utf8"); + assert.match(id, /^[0-9a-f]{32}$/); + for (const start of starts) assert.equal(start.computerDeploymentId, id); + // Same PUBLIC_API_URL on one Docker engine no longer means the same container and volume. + const other = config(second); + await resolveComputerDeployment(other); + assert.notEqual( + computerIdentity(other, "local-user").volume, + computerIdentity(starts[0], "local-user").volume, + ); + assert.deepEqual(await readdir(first), ["computer-deployment-id"]); +}); + +test("a new install keeps its computer name after restart", async (t) => { + const directory = await mkdtemp(join(tmpdir(), "openmuse-deployment-restart-")); + t.after(() => rm(directory, { recursive: true, force: true })); + const names: (string | undefined)[] = []; + for (let start = 0; start < 2; start++) { + const db = await createStore(); + t.after(() => db.close()); + const started = { ...config(directory), intelligenceApiKey: "test-project-key-never-sent" }; + await createApp(db, started); + names.push(started.computerDeploymentId); + } + assert.match(names[0] ?? "", /^[0-9a-f]{32}$/); + assert.equal(names[1], names[0]); +}); + +test("existing installs and explicit names keep their computer", async (t) => { + const directory = await mkdtemp(join(tmpdir(), "openmuse-deployment-legacy-")); + t.after(() => rm(directory, { recursive: true, force: true })); + await writeFile(join(directory, "session-signing-key"), Buffer.alloc(32, 7).toString("base64")); + const legacy = config(directory); + const before = computerIdentity(legacy, "local-user"); + await resolveComputerDeployment(legacy); + assert.equal(legacy.computerDeploymentId, undefined); + assert.deepEqual(computerIdentity(legacy, "local-user"), before); + const named = { ...config(directory), computerDeploymentId: "fixture-deployment" }; + await resolveComputerDeployment(named); + assert.equal(named.computerDeploymentId, "fixture-deployment"); + assert.deepEqual(await readdir(directory), ["session-signing-key"]); +}); diff --git a/tests/config.test.ts b/tests/config.test.ts index b323dd3b9..a2845fb7e 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -130,6 +130,9 @@ test("computer provider defaults to Docker and e2b-desktop requires a server-sid process.env.CPK_INTELLIGENCE_API_KEY = "test-project-key-never-sent"; for (const key of keys.slice(0, 5)) delete process.env[key]; assert.equal(readConfig().computerProvider, "docker"); + process.env.COMPUTER_DEPLOYMENT_ID = " "; + assert.equal(readConfig().computerDeploymentId, undefined); + delete process.env.COMPUTER_DEPLOYMENT_ID; process.env.COMPUTER_PROVIDER = "k8s"; assert.throws(() => readConfig(), /COMPUTER_PROVIDER/); process.env.COMPUTER_PROVIDER = "e2b-desktop"; diff --git a/tests/helpers/computer.ts b/tests/helpers/computer.ts index 63344d6f4..51b3bdedf 100644 --- a/tests/helpers/computer.ts +++ b/tests/helpers/computer.ts @@ -15,6 +15,8 @@ export const config: Config = { googleRedirectUri: "http://localhost/callback", allowedOrigins: [], computerEnabled: true, + // Fixtures copy this config into fresh data directories, so name the deployment explicitly. + computerDeploymentId: "fixture-deployment", }; export const ok = (stdout = ""): DockerResult => ({ stdout, diff --git a/tests/model-worker.test.ts b/tests/model-worker.test.ts index 97df7e178..1092b96ae 100644 --- a/tests/model-worker.test.ts +++ b/tests/model-worker.test.ts @@ -135,6 +135,7 @@ test("CopilotKit model worker executes server tools and persists the confirmed o googleRedirectUri: "http://localhost:8787/api/google/callback", allowedOrigins: [], computerEnabled: true, + computerDeploymentId: "fixture-deployment", }, { docker: computerFixture().runner }, );