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,10 @@ const DEFAULT_IO: AlibabaBackupIO = {
   exists: existsSync,
   read: path => readFileSync(path),
   copy: (source, destination) => copyFileSync(source, destination),
+  harden: path => {
+    try { chmodSync(path, 0o600); } catch { /* platform may ignore chmod */ }
+    if (process.platform === "win32") hardenSecretPath(path, { required: true });
+  },
   publishNoReplace: linkSync,
   remove: path => rmSync(path, { force: true }),
 };
@@ -57,6 +63,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

@@ -103,7 +103,8 @@ 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";
 export function logLiveSidebandFrame(dir: "c2u" | "u2c", data: unknown): void {
@@ -134,7 +135,7 @@ export function logLiveSidebandFrame(dir: "c2u" | "u2c", data: unknown): void {
       bytes,
       fffd,
     };
-    appendFileSync(logPath, `${JSON.stringify(record)}\n`);
+    appendFileSync(logPath, `${JSON.stringify(record)}\n`, { mode: 0o600 });
   } catch {
     // Frame forensics must never break the relay.
   }
@@ -168,7 +169,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");
+    appendFileSync(logPath, JSON.stringify(record) + "\n", { mode: 0o600 });
   } 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 { existsSync, mkdirSync, readFileSync, statSync } 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 {