From 5d163e5f7432e67dea45778f488a578d10d10ac1 Mon Sep 17 00:00:00 2001 From: luvs01 Date: Sun, 30 Aug 2026 14:34:17 +0900 Subject: [PATCH] fix(security): require dashboard consent for Codex restart --- .../ocx/references/01_management_surface.md | 22 ++------------- src/cli/capabilities.ts | 15 ---------- src/cli/system-command.ts | 10 ------- src/server/management/route-registry.ts | 2 +- src/server/management/system-routes.ts | 9 ++++++ tests/codex-restart-route.test.ts | 28 ++++++++++++++++++- 6 files changed, 39 insertions(+), 47 deletions(-) diff --git a/skills/ocx/references/01_management_surface.md b/skills/ocx/references/01_management_surface.md index 3a5a2ad989..9a6fdb17cd 100644 --- a/skills/ocx/references/01_management_surface.md +++ b/skills/ocx/references/01_management_surface.md @@ -472,24 +472,6 @@ JSON mode: `payload`. - `policy set` never enables implicitly: omitting `--enabled` keeps the stored value. - `policy run` forces a run regardless of schedule, so it needs `--yes`. -### `ocx system codex-restart` - -Restart the Codex app-server. - -| Method | Route | -|---|---| -| POST | `/api/system/codex-restart` | - -| Flag | Value | Meaning | -|---|---|---| -| `--yes` | boolean | Required: restarts the operator's running Codex app-server. | -| `--json` | boolean | Emit the restart result as JSON. | - -JSON mode: `payload`. - -- `sync --restart-codex` is not a substitute: it restarts only as a side effect after a catalog or cache write, so it cannot restart a healthy install on request. -- --yes is mandatory because this interrupts a running editor session, which must never happen because an agent guessed a subcommand. - ### `ocx integration native` Show or toggle the native Claude, Claude Desktop, Codex, and Grok integrations. @@ -530,6 +512,6 @@ JSON mode: `payload`. ## Counts -- declared capabilities: 29 -- of those, state-changing: 11 +- declared capabilities: 28 +- of those, state-changing: 10 - head-resolved invocations: 2 diff --git a/src/cli/capabilities.ts b/src/cli/capabilities.ts index 76b85e0690..ee451b7ca0 100644 --- a/src/cli/capabilities.ts +++ b/src/cli/capabilities.ts @@ -418,21 +418,6 @@ export const CAPABILITIES: readonly Capability[] = [ "The GUI reads this state directly; without a verb an agent could not tell whether the Codex app-server was reachable at all.", ], }, - { - command: ["system", "codex-restart"], - summary: "Restart the Codex app-server.", - routes: [{ method: "POST", path: "/api/system/codex-restart" }], - flags: [ - { name: "--yes", value: "boolean", summary: "Required: restarts the operator's running Codex app-server." }, - { name: "--json", value: "boolean", summary: "Emit the restart result as JSON." }, - ], - mutates: true, - json: "payload", - details: [ - "`sync --restart-codex` is not a substitute: it restarts only as a side effect after a catalog or cache write, so it cannot restart a healthy install on request.", - "--yes is mandatory because this interrupts a running editor session, which must never happen because an agent guessed a subcommand.", - ], - }, { command: ["claude", "desktop", "status"], summary: "Applied-vs-desired Claude Desktop state, including staleness, drift, and health.", diff --git a/src/cli/system-command.ts b/src/cli/system-command.ts index 8babcd3e03..e6296400de 100644 --- a/src/cli/system-command.ts +++ b/src/cli/system-command.ts @@ -18,7 +18,6 @@ const USAGE = `Usage: ocx system diagnostics [--json] ocx system sync [--json] ocx system codex-app-server [--json] - ocx system codex-restart --yes [--json] ocx system update check [--channel ] [--json] ocx system update run [--channel ] [--restart ] --yes [--json] ocx system update status [--json]`; @@ -111,15 +110,6 @@ export async function handleSystemCommand(argv: string[], deps: RuntimeApiDeps = // an agent could not see whether the Codex app-server was reachable at all. const args = [...rest]; const wantsJson = takeFlag(args, "--json"); rejectArgs(args, USAGE); printData(await runtimeRequest("/api/system/codex-app-server", {}, deps), wantsJson); - } else if (sub === "codex-restart") { - // --yes required: this restarts the user's running Codex app-server, so it is exactly the - // class of action that must not happen because an agent guessed a subcommand. - const args = [...rest]; - const wantsJson = takeFlag(args, "--json"); - const yes = takeFlag(args, "--yes"); - if (!yes) throw new CliUsageError("system codex-restart requires --yes", USAGE); - rejectArgs(args, USAGE); - printData(await runtimeRequest("/api/system/codex-restart", { method: "POST" }, deps), wantsJson, ["Codex app-server restart requested."]); } else if (sub === "update") await update(rest, deps); else throw new CliUsageError(`unknown system command ${sub}`, USAGE); }); diff --git a/src/server/management/route-registry.ts b/src/server/management/route-registry.ts index 3ebce889e2..f2a31cc5df 100644 --- a/src/server/management/route-registry.ts +++ b/src/server/management/route-registry.ts @@ -294,7 +294,7 @@ export const MANAGEMENT_ROUTES: readonly ManagementRoute[] = [ { method: "GET", path: "/api/storage", module: "server/management/storage-log-guard-routes", mutates: false, mechanism: "negated-guard" }, { method: "GET", path: "/api/routing-analytics", module: "server/management/routing-analytics-routes", mutates: false, mechanism: "negated-guard" }, { method: "GET", path: "/api/system/codex-app-server", module: "server/management/system-routes", mutates: false, mechanism: "path-constant" }, - { method: "POST", path: "/api/system/codex-restart", module: "server/management/system-routes", mutates: true, mechanism: "path-constant" }, + { method: "POST", path: "/api/system/codex-restart", module: "server/management/system-routes", mutates: true, mechanism: "path-constant", exempt: { reason: "session-only", why: "Interrupts active editor turns and therefore requires a gui-session principal; it must never gain an agent CLI capability." } }, { method: "POST", path: "/api/providers/reload", module: "server/management/provider-routes", mutates: true, mechanism: "path-constant", exempt: { reason: "capability-principal", why: "Gated on the local-provider-reload-capability principal (provider-routes.ts:467), not an operator action." } }, { method: "GET", path: "/api/client-integrations/{clientId}", module: "server/management/integration-routes", mutates: false, mechanism: "prefix-decode" }, { method: "PUT", path: "/api/client-integrations/{clientId}", module: "server/management/integration-routes", mutates: true, mechanism: "prefix-decode" }, diff --git a/src/server/management/system-routes.ts b/src/server/management/system-routes.ts index 867a3f94a0..db4454507b 100644 --- a/src/server/management/system-routes.ts +++ b/src/server/management/system-routes.ts @@ -164,6 +164,15 @@ export async function handleSystemRoutes(ctx: ManagementContext): Promise, + principal: ManagementContext["principal"] | null = "gui-session", ): ManagementContext { const url = new URL(`http://127.0.0.1:10100${path}`); return { @@ -46,6 +47,7 @@ function contextFor( url, config: loadConfig(), deps: { codexRestartService: service }, + ...(principal !== null ? { principal } : {}), } as ManagementContext; } @@ -93,6 +95,31 @@ describe("GET /api/system/codex-app-server", () => { }); describe("POST /api/system/codex-restart", () => { + test.each([ + ["a raw admin token", "admin-token" as const], + ["direct dispatch without a principal", null], + ])("rejects %s before performing the restart", async (_label, principal) => { + let restarted = false; + const response = await handleSystemRoutes(contextFor( + CODEX_RESTART_PATH, + "POST", + stubService({ + performRestart: async () => { + restarted = true; + return STOPPED; + }, + }), + principal, + )); + + expect(response?.status).toBe(403); + expect(await response?.json()).toEqual({ + success: false, + error: "Restart requires confirmation from the dashboard.", + }); + expect(restarted).toBe(false); + }); + test("returns the restart result in the contract shape", async () => { const response = await handleSystemRoutes( contextFor(CODEX_RESTART_PATH, "POST", stubService()), @@ -161,4 +188,3 @@ describe("POST /api/system/codex-restart", () => { expect(response).toBeNull(); }); }); -