diff --git a/src/server/management/native-integration-routes.ts b/src/server/management/native-integration-routes.ts index 894c00960..b6754bed2 100644 --- a/src/server/management/native-integration-routes.ts +++ b/src/server/management/native-integration-routes.ts @@ -32,6 +32,13 @@ import type { ManagementContext } from "./context"; export type NativeIntegrationClientId = "claude" | "grok" | "codex" | "claude-desktop"; +function isNativeToggleBody(body: unknown): body is { enabled: boolean } { + return typeof body === "object" + && body !== null + && !Array.isArray(body) + && typeof (body as { enabled?: unknown }).enabled === "boolean"; +} + /** Every reason this module can decline, in one place (audit r3 #6). */ export type NativeRefusalReason = | "not_installed" @@ -289,14 +296,14 @@ async function handleCodexToggle(ctx: ManagementContext): Promise { "Another Codex change is already in flight. Nothing was written — try again in a moment."); } codexToggleFlight = (async (): Promise => { - let body: { enabled?: unknown }; + let body: unknown; try { body = await readManagementJsonBody(req); } catch (error) { rethrowManagementBodyTooLarge(error); return jsonResponse({ error: "invalid JSON body" }, 400); } - if (typeof body.enabled !== "boolean") { + if (!isNativeToggleBody(body)) { return jsonResponse({ error: "enabled must be a boolean" }, 400); } const enabled = body.enabled; @@ -393,14 +400,14 @@ async function handleGrokToggle(ctx: ManagementContext): Promise { "Another Grok change is already in flight. Nothing was written — try again in a moment."); } grokToggleFlight = (async (): Promise => { - let body: { enabled?: unknown }; + let body: unknown; try { body = await readManagementJsonBody(req); } catch (error) { rethrowManagementBodyTooLarge(error); return jsonResponse({ error: "invalid JSON body" }, 400); } - if (typeof body.enabled !== "boolean") { + if (!isNativeToggleBody(body)) { return jsonResponse({ error: "enabled must be a boolean" }, 400); } const enabled = body.enabled; @@ -606,14 +613,14 @@ async function handleClaudeDesktopToggle(ctx: ManagementContext): Promise => { - let body: { enabled?: unknown }; + let body: unknown; try { body = await readManagementJsonBody(ctx.req); } catch (error) { rethrowManagementBodyTooLarge(error); return jsonResponse({ error: "invalid JSON body" }, 400); } - if (typeof body.enabled !== "boolean") return jsonResponse({ error: "enabled must be a boolean" }, 400); + if (!isNativeToggleBody(body)) return jsonResponse({ error: "enabled must be a boolean" }, 400); const { setIntegrationEnabled } = await import("../../codex/desired-state"); const persisted = setIntegrationEnabled("claude-desktop", body.enabled); @@ -689,14 +696,14 @@ export async function handleNativeIntegrationRoutes(ctx: ManagementContext): Pro } if (url.pathname === "/api/native-integrations/claude" && req.method === "PUT") { - let body: { enabled?: unknown }; + let body: unknown; try { body = await readManagementJsonBody(req); } catch (error) { rethrowManagementBodyTooLarge(error); return jsonResponse({ error: "invalid JSON body" }, 400); } - if (typeof body.enabled !== "boolean") { + if (!isNativeToggleBody(body)) { return jsonResponse({ error: "enabled must be a boolean" }, 400); } @@ -722,6 +729,8 @@ export async function handleNativeIntegrationRoutes(ctx: ManagementContext): Pro * subscription — a failure that surfaces nowhere near this route. */ if (!next.authModeMigratedAt) next.authModeMigratedAt = new Date().toISOString(); + const hadClaudeCode = Object.prototype.hasOwnProperty.call(config, "claudeCode"); + const previousClaudeCode = config.claudeCode; config.claudeCode = next; /* @@ -732,6 +741,11 @@ export async function handleNativeIntegrationRoutes(ctx: ManagementContext): Pro try { persist(config); } catch (error) { + // Persistence is the commit point. A failed lock acquisition must not + // leave the live server snapshot ahead of disk or make a retry look + // idempotent and skip the write entirely. + if (hadClaudeCode) config.claudeCode = previousClaudeCode; + else delete config.claudeCode; if (isConfigLockError(error)) { return isLockContention(error) ? refusal(409, "claude", "config_busy", diff --git a/tests/native-claude-code-toggle.test.ts b/tests/native-claude-code-toggle.test.ts index d6f0777ce..3da0b4ced 100644 --- a/tests/native-claude-code-toggle.test.ts +++ b/tests/native-claude-code-toggle.test.ts @@ -117,6 +117,23 @@ test("a non-boolean enabled is rejected", async () => { expect(res!.status).toBe(400); }); +test("non-object bodies are rejected without mutating or saving", async () => { + for (const rawBody of ["null", "[]", "true", "1", '"yes"']) { + const config = baseConfig({ claudeCode: { enabled: true } }); + const before = structuredClone(config); + const { response, saved } = dispatch(config, "/api/native-integrations/claude", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: rawBody, + }); + const res = await response; + expect(res!.status).toBe(400); + expect(await res!.json()).toEqual({ error: "enabled must be a boolean" }); + expect(config).toEqual(before); + expect(saved).toHaveLength(0); + } +}); + test("genuine lock contention refuses 409 config_busy, a broken lock is a 500", async () => { /* * `ConfigMutationLockError` wraps EVERY acquisition failure behind one @@ -201,25 +218,24 @@ test("a held REAL config transaction refuses 409 config_busy, and release lets a {}, ); }; - const refused = await putReal(baseConfig({ claudeCode: { enabled: true } })); + const config = baseConfig({ claudeCode: { enabled: true } }); + const refused = await putReal(config); expect(refused!.status).toBe(409); const refusedBody = await refused!.json() as { code: string; reason: string }; expect(refusedBody.code).toBe("native_integration_refused"); expect(refusedBody.reason).toBe("config_busy"); + expect(config.claudeCode?.enabled).toBe(true); holder.exec("ROLLBACK"); holder.close(); - /* - * A FRESH config object, not the refused one (wp3 A-gate): the route - * mutates the in-memory config before persistence, so the refused object - * already reads disabled and would short-circuit at the idempotent guard - * without ever re-acquiring the lock. - */ - const retry = await putReal(baseConfig({ claudeCode: { enabled: true } })); + // Retry the exact live object that saw the refusal. A failed save must + // restore it so the retry re-acquires the lock and persists the change. + const retry = await putReal(config); expect(retry!.status).toBe(200); const retryBody = await retry!.json() as { changed: boolean; state: string }; expect(retryBody.changed).toBe(true); expect(retryBody.state).toBe("absent"); + expect(config.claudeCode?.enabled).toBe(false); } finally { try { holder.exec("ROLLBACK"); } catch { /* already closed */ } try { holder.close(); } catch { /* already closed */ } diff --git a/tests/native-claude-desktop-toggle.test.ts b/tests/native-claude-desktop-toggle.test.ts index 9b1844f01..2ac636b23 100644 --- a/tests/native-claude-desktop-toggle.test.ts +++ b/tests/native-claude-desktop-toggle.test.ts @@ -60,6 +60,18 @@ afterEach(() => { rmSync(root, { recursive: true, force: true }); }); +test("a null toggle body is rejected before desired state or Desktop files change", async () => { + const response = await dispatch("/api/native-integrations/claude-desktop", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: "null", + }); + expect(response!.status).toBe(400); + expect(await response!.json()).toEqual({ error: "enabled must be a boolean" }); + expect(persistedIntent()).toBeUndefined(); + expect(existsSync(library)).toBe(false); +}); + test("the native route advertises Claude Desktop and OFF persists intent before removal", async () => { let sawPersistedOff = false; const result = await toggle(false, { diff --git a/tests/native-codex-toggle.test.ts b/tests/native-codex-toggle.test.ts index 9aa62df73..c16f93a00 100644 --- a/tests/native-codex-toggle.test.ts +++ b/tests/native-codex-toggle.test.ts @@ -90,6 +90,13 @@ afterEach(() => { }); describe("request validation", () => { + test("a null body is rejected before anything is written", async () => { + const result = await put(baseConfig(), null); + expect(result.status).toBe(400); + expect(result.body).toEqual({ error: "enabled must be a boolean" }); + expect(persistedCodexIntent()).toBeUndefined(); + }); + test("a non-boolean enabled is rejected before anything is written", async () => { const result = await put(baseConfig(), { enabled: "false" }); expect(result.status).toBe(400); diff --git a/tests/native-grok-toggle.test.ts b/tests/native-grok-toggle.test.ts index 2a405e9e3..e3fd89417 100644 --- a/tests/native-grok-toggle.test.ts +++ b/tests/native-grok-toggle.test.ts @@ -146,6 +146,19 @@ test("GET reports not-installed when GROK_HOME is missing", async () => { expect(row.state).toBe("absent"); }); +test("a null toggle body is rejected before the Grok config is touched", async () => { + writeConfig("# user only\n"); + const before = readConfig(); + const response = await dispatch(baseConfig(), "/api/native-integrations/grok", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: "null", + }); + expect(response!.status).toBe(400); + expect(await response!.json()).toEqual({ error: "enabled must be a boolean" }); + expect(readConfig()).toBe(before); +}); + test("GET reports unsafe and blocks the switch on an orphaned marker", async () => { writeConfig(`# user\n${BEGIN}\n[model.ocx-a]\n`); const row = await get(baseConfig());