fix(security): refuse plaintext remote hub origins and harden credential-file modes

src/client/hub-client.ts

@@ -6,14 +6,8 @@ import { clearableDeadline } from "../lib/abort";
 import type { Desktop3pModelEntry } from "../claude/desktop-3p";
 import { assertDesktop3pModelsValid } from "../claude/desktop-3p-guard";
 
-/**
- * A pairing grant may cross loopback or authenticated HTTPS, and nothing else.
- *
- * Mirrors the hub-side rule in src/server/gui-session.ts. Checking here too is not
- * redundant: it keeps the client from spending a single-use code on a request the hub is
- * certain to refuse.
- */
-function isPairingTransportPermitted(origin: string): boolean {
+/** Hub traffic may cross loopback or authenticated HTTPS, and nothing else. */
+function isHubTransportPermitted(origin: string): boolean {
   let url: URL;
   try {
     url = new URL(origin);
@@ -193,6 +187,12 @@ export function normalizeHubOrigin(input: string): string {
       "Hub URL must be an HTTP(S) origin without credentials, query, fragment, or non-/v1 path",
     );
   }
+  if (!isHubTransportPermitted(parsed.origin)) {
+    throw new HubClientError(
+      "insecure_http_refused",
+      "Hub URLs require loopback or HTTPS; plaintext remote HTTP is not permitted",
+    );
+  }
   return parsed.origin;
 }
 
@@ -252,7 +252,7 @@ export async function exchangeConnectPairingGrant(
   // Deliberateness is not the control that matters: the grant is readable by anything on the
   // path and the session it mints is reusable. The hub refuses this exchange outright now, so
   // sending it would only burn a single-use code against a certain rejection.
-  if (!isPairingTransportPermitted(origin)) {
+  if (!isHubTransportPermitted(origin)) {
     throw new HubClientError("insecure_http_refused", "Pairing requires loopback or HTTPS; plaintext HTTP cannot carry a grant");
   }
   const response = await fetchBounded(options.fetchImpl ?? fetch, `${origin}/opencodex-session`, {
@@ -481,7 +481,7 @@ export async function fetchHubUsage(
   options: { timeoutMs?: number; fetchImpl?: typeof fetch } = {},
 ): Promise<HubUsageReport> {
   const origin = normalizeHubOrigin(serverUrl);
-  if (!isPairingTransportPermitted(origin)) {
+  if (!isHubTransportPermitted(origin)) {
     throw new HubClientError("insecure_http_refused", "Client usage requires HTTPS or loopback HTTP");
   }
   const response = await fetchBounded(options.fetchImpl ?? fetch, `${origin}/v1/usage?${query}`, {
@@ -593,7 +593,7 @@ export async function downloadDesktop3pModels(
   options: { timeoutMs?: number; fetchImpl?: typeof fetch } = {},
 ): Promise<{ version: 1; models: Desktop3pModelEntry[] }> {
   const origin = normalizeHubOrigin(serverUrl);
-  if (!isPairingTransportPermitted(origin)) {
+  if (!isHubTransportPermitted(origin)) {
     throw new HubClientError("insecure_http_refused", "Desktop model snapshots require HTTPS or loopback HTTP");
   }
   try {

src/providers/alibaba-region-backup.ts

@@ -1,10 +1,12 @@
-import { copyFileSync, existsSync, linkSync, readFileSync, rmSync } from "node:fs";
+import { chmodSync, copyFileSync, existsSync, linkSync, readFileSync, rmSync } from "node:fs";
 import { getConfigPath } from "../config";
+import { hardenSecretPath } from "../lib/windows-secret-acl";
 
 export interface AlibabaBackupIO {
   exists: (path: string) => boolean;
   read: (path: string) => Buffer;
   copy: (source: string, destination: string) => void;
+  harden: (path: string) => void;
   /** Publish with no-replace semantics: fails with EEXIST if the destination exists. */
   publishNoReplace: (temp: string, destination: string) => void;
   remove: (path: string) => void;
@@ -14,6 +16,16 @@ const DEFAULT_IO: AlibabaBackupIO = {
   exists: existsSync,
   read: path => readFileSync(path),
   copy: (source, destination) => copyFileSync(source, destination),
+  harden: path => {
+    // POSIX: a failed chmod must not publish a credential-bearing backup with weak permissions.
+    // Windows keeps its own control via hardenSecretPath below, which is the required check.
+    if (process.platform === "win32") {
+      try { chmodSync(path, 0o600); } catch { /* Windows may not support POSIX chmod */ }
+    } else {
+      chmodSync(path, 0o600);
+    }
+    if (process.platform === "win32") hardenSecretPath(path, { required: true });
+  },
   publishNoReplace: linkSync,
   remove: path => rmSync(path, { force: true }),
 };
@@ -57,6 +69,9 @@ export function backupConfigBeforeAlibabaRegionMigration(
   const temp = `${backup}.${process.pid}.tmp`;
   try {
     io.copy(configPath, temp);
+    // The snapshot contains credentials. Harden it before publication so the
+    // stable backup path is never exposed with inherited permissions or ACLs.
+    io.harden(temp);
     // Verify before publishing: a short copy must never become the snapshot.
     if (!io.read(temp).equals(source)) {
       throw new AlibabaBackupIntegrityError(`failed to write a complete backup to ${temp}`);

src/server/live.ts

@@ -27,7 +27,7 @@ import { codexCompatibleUrl } from "../codex/context-compat";
  * - `GET /v1/realtime?model=` — RealtimeV2 standalone (no intent)
  * - `GET /v1/live?model=` — Frameless standalone
  */
-import { appendFileSync } from "node:fs";
+import { closeSync, fchmodSync, openSync, writeSync } from "node:fs";
 import { formatErrorResponse } from "../bridge";
 import {
   CodexAccountCooldownError,
@@ -103,9 +103,25 @@ export const LIVE_CLIENT_PROTOCOL_HEADERS = [
  * JSONL record: direction, frame kind, byte length, and whether the payload contains U+FFFD.
  * Privacy: no frame content is written, including excerpts around replacement characters.
  * For binary frames, U+FFFD may also be introduced by UTF-8 decoding; the flag alone does not
- * identify the source of corruption. Disabled entirely when the env var is unset.
+ * identify the source of corruption. The log is created with owner-only permissions and is
+ * disabled entirely when the env var is unset.
  */
 export const LIVE_FRAME_LOG_ENV = "OCX_LIVE_FRAME_LOG";
+/**
+ * Append one JSONL record with owner-only permissions. `appendFileSync`'s `mode` only applies
+ * when it creates the file, so an existing permissive log would stay readable by other local
+ * users. Open for append, harden the opened descriptor, then write.
+ */
+function appendOwnerOnly(path: string, line: string): void {
+  const fd = openSync(path, "a", 0o600);
+  try {
+    try { fchmodSync(fd, 0o600); } catch { /* platforms without fchmod keep the create mode */ }
+    writeSync(fd, line);
+  } finally {
+    closeSync(fd);
+  }
+}
+
 export function logLiveSidebandFrame(dir: "c2u" | "u2c", data: unknown): void {
   const logPath = process.env[LIVE_FRAME_LOG_ENV];
   if (!logPath) return;
@@ -134,7 +150,7 @@ export function logLiveSidebandFrame(dir: "c2u" | "u2c", data: unknown): void {
       bytes,
       fffd,
     };
-    appendFileSync(logPath, `${JSON.stringify(record)}\n`);
+    appendOwnerOnly(logPath, `${JSON.stringify(record)}\n`);
   } catch {
     // Frame forensics must never break the relay.
   }
@@ -168,7 +184,7 @@ export function logLiveSidebandStage(
     const record: Record<string, unknown> = { ts: new Date().toISOString(), stage };
     if (detail?.status !== undefined) record.status = detail.status;
     if (detail?.code !== undefined) record.code = detail.code;
-    appendFileSync(logPath, JSON.stringify(record) + "\n");
+    appendOwnerOnly(logPath, JSON.stringify(record) + "\n");
   } catch {
     // Diagnostics must never break the relay.
   }

tests/clients/client-connect.test.ts

@@ -237,8 +237,12 @@ describe("remote hub client boundary", () => {
   test("canonicalizes origin and terminal /v1 only", () => {
     expect(normalizeHubOrigin("https://hub.example.test/v1")).toBe("https://hub.example.test");
     expect(normalizeHubOrigin("https://hub.example.test/v1/")).toBe("https://hub.example.test");
+    expect(normalizeHubOrigin("http://localhost:10100/v1")).toBe("http://localhost:10100");
+    expect(normalizeHubOrigin("http://127.0.0.1:10100")).toBe("http://127.0.0.1:10100");
+    expect(normalizeHubOrigin("http://[::1]:10100")).toBe("http://[::1]:10100");
     for (const value of [
       "ftp://hub.example.test",
+      "http://hub.example.test",
       "https://user@hub.example.test",
       "https://hub.example.test/private",
       "https://hub.example.test/?secret=1",
@@ -263,9 +267,17 @@ describe("remote hub client boundary", () => {
     }
   });
 
+  test("rejects plaintext remote discovery before sending a request", async () => {
+    let calls = 0;
+    await expect(fetchHubReady("http://hub.example.test", {
+      fetchImpl: async () => { calls += 1; return Response.json(readyBody()); },
+    })).rejects.toThrow("plaintext remote HTTP is not permitted");
+    expect(calls).toBe(0);
+  });
+
   test("admin key issuance is HTTPS-only and pairing exchanges into a full GUI session", async () => {
     let calls = 0;
-    await expect(issueClientKey("http://hub.example.test", {
+    await expect(issueClientKey("http://localhost:10100", {
       kind: "admin",
       value: new TextEncoder().encode("ocx_admin_secret"),
     }, "client", {

tests/providers/alibaba-region-backup.test.ts

@@ -1,5 +1,5 @@
 import { expect, test } from "bun:test";
-import { existsSync, linkSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { existsSync, linkSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import {
@@ -23,6 +23,7 @@ test("creates a snapshot, then never replaces it", () => {
     writeFileSync(configPath, '{"before":true}', "utf8");
     expect(backupConfigBeforeAlibabaRegionMigration(configPath)).toBe("created");
     expect(readFileSync(backupPath, "utf8")).toBe('{"before":true}');
+    if (process.platform !== "win32") expect(statSync(backupPath).mode & 0o777).toBe(0o600);
     expect(backupConfigBeforeAlibabaRegionMigration(configPath)).toBe("reused");
     expect(readFileSync(backupPath, "utf8")).toBe('{"before":true}');
   } finally { removeTreeWithRetry(dir); }
@@ -53,6 +54,7 @@ test("a short copy is never published", () => {
       exists: existsSync,
       read: path => readFileSync(path),
       copy: (_source, destination) => { writeFileSync(destination, '{"bef', "utf8"); },
+      harden: () => {},
       publishNoReplace: linkSync,
       remove: path => rmSync(path, { force: true }),
     })).toThrow(AlibabaBackupIntegrityError);
@@ -70,10 +72,30 @@ test("a failed copy leaves no snapshot and no temp file", () => {
       exists: existsSync,
       read: path => readFileSync(path),
       copy: () => { throw new Error("disk full"); },
+      harden: () => {},
       publishNoReplace: linkSync,
       remove: path => { removed.push(path); rmSync(path, { force: true }); },
     })).toThrow("disk full");
     expect(existsSync(`${configPath}.pre-alibaba-region-v1.bak`)).toBe(false);
     expect(removed).toHaveLength(1);
   } finally { removeTreeWithRetry(dir); }
 });
+
+test("a failed harden never publishes the secret-bearing snapshot", () => {
+  const dir = mkdtempSync(join(tmpdir(), "ocx-bak-"));
+  const configPath = join(dir, "config.json");
+  const backupPath = `${configPath}.pre-alibaba-region-v1.bak`;
+  try {
+    writeFileSync(configPath, '{"before":true}', "utf8");
+    expect(() => backupConfigBeforeAlibabaRegionMigration(configPath, {
+      exists: existsSync,
+      read: path => readFileSync(path),
+      copy: (source, destination) => writeFileSync(destination, readFileSync(source)),
+      harden: () => { throw new Error("ACL hardening failed"); },
+      publishNoReplace: linkSync,
+      remove: path => rmSync(path, { force: true }),
+    })).toThrow("ACL hardening failed");
+    expect(existsSync(backupPath)).toBe(false);
+    expect(existsSync(`${backupPath}.${process.pid}.tmp`)).toBe(false);
+  } finally { removeTreeWithRetry(dir); }
+});

tests/server/server-live.test.ts

@@ -3,7 +3,7 @@
  * so the proxy must relay it to an OpenAI upstream instead of the /v1/* JSON-404 guard.
  */
 import { afterEach, beforeEach, describe, expect, test } from "bun:test";
-import { existsSync, mkdirSync, readFileSync } from "node:fs";
+import { chmodSync, existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 import { saveCodexAccountCredential } from "../../src/codex/account-store";
 import { clearAccountNeedsReauth, clearAccountQuota } from "../../src/codex/auth-api";
@@ -1334,6 +1334,7 @@ test("sideband frame log preserves delivery without recording damaged or clean t
       expect(JSON.stringify(line)).not.toContain("clean-frame");
       expect(JSON.stringify(line)).not.toContain(FFFD_TEXT);
     }
+    if (process.platform !== "win32") expect(statSync(frameLogPath).mode & 0o777).toBe(0o600);
 
     client.close();
   } finally {
@@ -1345,6 +1346,28 @@ test("sideband frame log preserves delivery without recording damaged or clean t
   }
 });
 
+// appendFileSync's mode option only applies when it creates the file, so an
+// existing permissive log would have stayed readable by other local users.
+// appendOwnerOnly hardens the opened descriptor instead.
+test("frame log hardens a pre-existing permissive file", async () => {
+  if (process.platform === "win32") return;
+  const { logLiveSidebandStage } = await import("../../src/server/live");
+  const frameLogPath = join(TEST_DIR, "frames-permissive.jsonl");
+  const previousFrameLog = process.env.OCX_LIVE_FRAME_LOG;
+  try {
+    writeFileSync(frameLogPath, "", { mode: 0o644 });
+    chmodSync(frameLogPath, 0o644);
+    process.env.OCX_LIVE_FRAME_LOG = frameLogPath;
+    logLiveSidebandStage("relay-attached");
+    expect(statSync(frameLogPath).mode & 0o777).toBe(0o600);
+    const line = readFileSync(frameLogPath, "utf8").trim();
+    expect(JSON.parse(line)).toMatchObject({ stage: "relay-attached" });
+  } finally {
+    if (previousFrameLog === undefined) delete process.env.OCX_LIVE_FRAME_LOG;
+    else process.env.OCX_LIVE_FRAME_LOG = previousFrameLog;
+  }
+});
+
 test("frame diagnostics retain only metadata for text, binary, and bounded views", async () => {
   const { logLiveSidebandFrame } = await import("../../src/server/live");
   const previousFrameLog = process.env.OCX_LIVE_FRAME_LOG;