From 5093809ea08ebcec3675d3ae208df3173238b607 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Wed, 28 Oct 2026 19:26:36 +0200 Subject: [PATCH] fix: authorize CLI probes and preserve executable identity --- app/api/claude-code/route.ts | 6 +-- components/settings/claude-code-status.tsx | 2 +- src/agent/claude-code/binary.ts | 7 ++- src/server/claude-code.ts | 19 +++++++ src/server/index.ts | 2 +- tests/claude-code-binary.test.ts | 6 +++ tests/claude-code-status-http.test.ts | 39 ++++++++++++++ tests/claude-code-status-ui.test.tsx | 1 + tests/claude-code-version-runner.test.ts | 63 ++++++++++++++++++++++ 9 files changed, 138 insertions(+), 7 deletions(-) create mode 100644 tests/claude-code-status-http.test.ts create mode 100644 tests/claude-code-version-runner.test.ts diff --git a/app/api/claude-code/route.ts b/app/api/claude-code/route.ts index e587386..1708fe9 100644 --- a/app/api/claude-code/route.ts +++ b/app/api/claude-code/route.ts @@ -1,6 +1,6 @@ -import { claudeCodeInstallationStatus } from "@/src/server"; +import { claudeCodeInstallationResponse } from "@/src/server"; /** Installation/version only; never reads auth status or makes a model request. */ -export async function GET() { - return Response.json(await claudeCodeInstallationStatus(), { headers: { "Cache-Control": "no-store" } }); +export async function GET(request: Request) { + return claudeCodeInstallationResponse(request); } diff --git a/components/settings/claude-code-status.tsx b/components/settings/claude-code-status.tsx index 274d5b4..cc5fd33 100644 --- a/components/settings/claude-code-status.tsx +++ b/components/settings/claude-code-status.tsx @@ -22,7 +22,7 @@ export function ClaudeCodeInstallationStatus() { request.current = controller; setBusy(true); try { - const response = await fetch("/api/claude-code", { cache: "no-store", signal: controller.signal }); + const response = await fetch("/api/claude-code", { cache: "no-store", signal: controller.signal, headers: { "x-learnharness-claude-code": "1" } }); if (!response.ok) throw new Error("status unavailable"); const value = await response.json() as ClaudeCodeStatus; if (!["not-found", "meets-minimum", "update-required", "version-unchecked"].includes(value.state) diff --git a/src/agent/claude-code/binary.ts b/src/agent/claude-code/binary.ts index 13e86e6..1ee1871 100644 --- a/src/agent/claude-code/binary.ts +++ b/src/agent/claude-code/binary.ts @@ -6,6 +6,7 @@ type BinaryLookup = { env: Record; platform: NodeJS.Platform; homeDir: string; + cwd?: string; isExecutable: (path: string) => boolean; }; @@ -51,7 +52,7 @@ function getPosixSearchSpec(env: BinaryLookup["env"], homeDir: string): SearchSp }; } -export function resolveClaudeBinary({ env, platform, homeDir, isExecutable }: BinaryLookup): string | null { +export function resolveClaudeBinary({ env, platform, homeDir, cwd = process.cwd(), isExecutable }: BinaryLookup): string | null { const { paths, names, knownDirs } = platform === "win32" ? getWindowsSearchSpec(env, homeDir) : getPosixSearchSpec(env, homeDir); @@ -60,7 +61,9 @@ export function resolveClaudeBinary({ env, platform, homeDir, isExecutable }: Bi for (const name of names) { for (const dir of [...pathDirs, ...knownDirs]) { - const candidate = paths.join(dir, name); + // Discovery is relative to the server cwd; probes/model work use + // different directories and must receive the same absolute identity. + const candidate = paths.resolve(cwd, dir, name); if (isExecutable(candidate)) return candidate; } } diff --git a/src/server/claude-code.ts b/src/server/claude-code.ts index be52c06..bd40ebe 100644 --- a/src/server/claude-code.ts +++ b/src/server/claude-code.ts @@ -1,8 +1,27 @@ import { inspectClaudeInstallation } from "../agent/claude-code/installation.ts"; import { initAuthEnv } from "./keys.ts"; +import { subscriptionRequestOrigin } from "./web-subscription-http.ts"; /** The Settings status seam: initialize provenance before any child env is copied. */ export async function claudeCodeInstallationStatus() { initAuthEnv(); return inspectClaudeInstallation(); } + +/** Same-origin browser intent, not user authentication. No CORS grants. + * A public/shared server still requires its own access-control boundary. */ +export async function claudeCodeInstallationResponse( + request: Request, + inspect = claudeCodeInstallationStatus, +): Promise { + const origin = subscriptionRequestOrigin(request); + const supplied = request.headers.get("origin"); + const headers = { "Cache-Control": "no-store", "X-Content-Type-Options": "nosniff" }; + if (request.method !== "GET" || !origin + || request.headers.get("x-learnharness-claude-code") !== "1" + || request.headers.get("sec-fetch-site") === "cross-site" + || (supplied !== null && supplied !== origin)) { + return Response.json({ error: "Claude Code checks require a same-origin LearnHarness request." }, { status: 403, headers }); + } + return Response.json(await inspect(), { headers }); +} diff --git a/src/server/index.ts b/src/server/index.ts index cd65868..0c3b002 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -45,4 +45,4 @@ export * from "./observability.ts"; export * from "./model-catalog.ts"; export { subscriptionControlsResponse, subscriptionBrowserResponse } from "./web-subscription-http.ts"; export { sessionCost } from "./pricing.ts"; -export { claudeCodeInstallationStatus } from "./claude-code.ts"; +export { claudeCodeInstallationResponse } from "./claude-code.ts"; diff --git a/tests/claude-code-binary.test.ts b/tests/claude-code-binary.test.ts index 599f5d9..ae9c855 100644 --- a/tests/claude-code-binary.test.ts +++ b/tests/claude-code-binary.test.ts @@ -59,6 +59,12 @@ test("windows prefers claude.exe anywhere over claude.cmd, which Node cannot spa assert.equal(resolveWith(executables, env, "win32", winHome), `${winHome}\\.local\\bin\\claude.exe`); }); +test("relative Windows PATH entries are fixed to an absolute discovery identity", () => { + assert.equal(resolveClaudeBinary({ env: { PATH: ".\\bin" }, platform: "win32", homeDir: "C:\\Users\\u", cwd: "C:\\server", + isExecutable: (candidate) => candidate === "C:\\server\\bin\\claude.exe", + }), "C:\\server\\bin\\claude.exe"); +}); + test("returns null when every location misses", () => { assert.equal(resolveWith([], { PATH: "/usr/bin:/bin" }), null); }); diff --git a/tests/claude-code-status-http.test.ts b/tests/claude-code-status-http.test.ts new file mode 100644 index 0000000..1c8f746 --- /dev/null +++ b/tests/claude-code-status-http.test.ts @@ -0,0 +1,39 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { claudeCodeInstallationResponse } from "../src/server/claude-code.ts"; + +function request(headers: Record = {}) { + return new Request("http://localhost:3000/api/claude-code", { headers: { "x-learnharness-claude-code": "1", ...headers } }); +} +const status = { state: "not-found" as const, version: null, minimumVersion: "2.1.259" }; + +for (const headers of [ + { origin: "https://attacker.invalid" }, + { "sec-fetch-site": "cross-site" }, + { origin: "http://localhost:3000", "sec-fetch-site": "cross-site" }, + { origin: "null" }, + { "x-learnharness-claude-code": "" }, +] as Record[]) test(`unsafe status request ${JSON.stringify(headers)} never invokes the probe`, async () => { + let probes = 0; + const response = await claudeCodeInstallationResponse(request(headers), async () => { probes++; return status; }); + assert.equal(response.status, 403); assert.equal(probes, 0); + assert.equal(response.headers.get("access-control-allow-origin"), null); +}); + +test("ordinary image/subresource GET without intent header cannot start a probe", async () => { + const response = await claudeCodeInstallationResponse(new Request("http://localhost:3000/api/claude-code"), async () => assert.fail("must not probe")); + assert.equal(response.status, 403); +}); + +for (const headers of [{}, { origin: "http://localhost:3000", "sec-fetch-site": "same-origin" }] as Record[]) test(`legitimate Providers GET ${JSON.stringify(headers)} is supported`, async () => { + let probes = 0; + const response = await claudeCodeInstallationResponse(request(headers), async () => { probes++; return status; }); + assert.equal(response.status, 200); assert.equal(probes, 1); + assert.equal(response.headers.get("cache-control"), "no-store"); + assert.deepEqual(await response.json(), status); +}); + +test("a forged forwarded origin cannot authorize a foreign Origin header", async () => { + const response = await claudeCodeInstallationResponse(request({ origin: "https://attacker.invalid", "x-forwarded-host": "attacker.invalid", "x-forwarded-proto": "https" }), async () => assert.fail("must not probe")); + assert.equal(response.status, 403); +}); diff --git a/tests/claude-code-status-ui.test.tsx b/tests/claude-code-status-ui.test.tsx index 5c09ddb..015fe66 100644 --- a/tests/claude-code-status-ui.test.tsx +++ b/tests/claude-code-status-ui.test.tsx @@ -47,6 +47,7 @@ test("mount checks once, Check again refreshes, and neither request asks for aut globalThis.fetch = (async (url, init) => { calls.push(String(url)); assert.equal(init?.cache, "no-store"); + assert.equal(new Headers(init?.headers).get("x-learnharness-claude-code"), "1"); return Response.json(calls.length === 1 ? status("not-found") : status("meets-minimum", "2.1.310")); }) as typeof fetch; await mount(); diff --git a/tests/claude-code-version-runner.test.ts b/tests/claude-code-version-runner.test.ts new file mode 100644 index 0000000..6faafee --- /dev/null +++ b/tests/claude-code-version-runner.test.ts @@ -0,0 +1,63 @@ +/** Real execFile boundary, authored fixture executables only. Never the installed CLI. */ +import assert from "node:assert/strict"; +import { type spawn } from "node:child_process"; +import { accessSync, constants, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test, type TestContext } from "node:test"; +import { resolveClaudeBinary } from "../src/agent/claude-code/binary.ts"; +import { readClaudeVersion } from "../src/agent/claude-code/installation.ts"; +import { createClaudeLauncher } from "../src/agent/claude-code/launch.ts"; + +const unsupportedShebang = process.platform === "win32" || /\s/.test(process.execPath); +function fixture(t: TestContext, mode: "normal" | "oversize" | "hang", name = "claude") { + const dir = mkdtempSync(join(tmpdir(), "lh-version-runner-")); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const bin = join(dir, "bin"); mkdirSync(bin); + const binary = join(bin, name); + const body = mode === "hang" ? ` +import { writeFileSync } from "node:fs"; +writeFileSync(process.argv[1] + ".pid", String(process.pid)); +process.on("SIGTERM", () => {}); +setInterval(() => {}, 1000); +` : mode === "oversize" ? 'process.stdout.write("x".repeat(100000));' : ` +if (JSON.stringify(process.argv.slice(2)) !== '["--version"]') process.exit(2); +process.stdout.write("2.1.310 (Claude Code)\\n"); +`; + writeFileSync(binary, `#!${process.execPath}\n${body}`, { mode: 0o755 }); + return { dir, binary }; +} + +test("production version runner directly executes a spaced/metacharacter filename with only --version", { skip: unsupportedShebang }, async (t) => { + const { binary } = fixture(t, "normal", "claude ; fixture"); + assert.equal(await readClaudeVersion(binary), "2.1.310 (Claude Code)\n"); +}); + +test("production version runner bounds stdout", { skip: unsupportedShebang }, async (t) => { + const { binary } = fixture(t, "oversize"); + await assert.rejects(readClaudeVersion(binary), (error: NodeJS.ErrnoException) => error.code === "ERR_CHILD_PROCESS_STDIO_MAXBUFFER"); +}); + +test("production version runner times out and SIGKILLs an uncooperative fixture", { skip: unsupportedShebang, timeout: 10000 }, async (t) => { + const { binary } = fixture(t, "hang"); + const start = Date.now(); + await assert.rejects(readClaudeVersion(binary), (error: Error & { signal?: string }) => error.signal === "SIGKILL"); + assert(Date.now() - start < 8500, "bounded five-second timeout"); + const pid = Number(readFileSync(`${binary}.pid`, "utf8")); + assert.throws(() => process.kill(pid, 0), /ESRCH/, "fixture no longer exists"); +}); + +test("relative PATH is resolved at server cwd and stays the same for probe and model launch", { skip: unsupportedShebang }, async (t) => { + const { dir, binary } = fixture(t, "normal"); + const found = resolveClaudeBinary({ env: { PATH: "./bin" }, homeDir: dir, platform: process.platform, cwd: dir, + isExecutable: (path) => { try { accessSync(path, constants.X_OK); return statSync(path).isFile(); } catch { return false; } }, + }); + assert.equal(found, binary); + assert.equal(await readClaudeVersion(found!), "2.1.310 (Claude Code)\n", "probe changes cwd to tmpdir but executes the discovered file"); + let spawned = false; + const launch = createClaudeLauncher({ findBinary: () => found, + spawnProcess: ((path: string) => { assert.equal(path, binary); spawned = true; return null; }) as unknown as typeof spawn, + }); + await launch(["-p"], { cwd: tmpdir(), env: { NODE_ENV: "test" } }); + assert(spawned, "same verified absolute identity passed to the model launcher"); +});