From cde614a152f3faf35af1e403e156fc8a7fc4e69b Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sun, 12 Jul 2026 00:25:14 +0200 Subject: [PATCH 1/7] fix: harden request and provider security --- src/config.ts | 25 ++- src/lib/destination-policy.ts | 117 ++++++++++++++ src/lib/windows-secret-acl.ts | 173 +++++++++++++++++++++ src/oauth/store.ts | 1 + src/providers/registry.ts | 8 +- src/router.ts | 7 +- src/server/auth-cors.ts | 4 + src/server/management-api.ts | 1 + src/server/request-decompress.ts | 20 ++- src/service.ts | 4 + src/types.ts | 5 + tests/cli-models.test.ts | 2 + tests/config.test.ts | 120 +++++++++++++++ tests/request-decompress.test.ts | 39 +++++ tests/router.test.ts | 63 ++++++++ tests/server-auth.test.ts | 102 ++++++++++++- tests/server-images.test.ts | 5 +- tests/server-key-failover-e2e.test.ts | 2 + tests/server-search.test.ts | 3 +- tests/vision-sidecar-e2e.test.ts | 6 +- tests/windows-secret-acl.test.ts | 210 ++++++++++++++++++++++++++ 21 files changed, 896 insertions(+), 21 deletions(-) create mode 100644 src/lib/destination-policy.ts create mode 100644 src/lib/windows-secret-acl.ts create mode 100644 tests/windows-secret-acl.test.ts diff --git a/src/config.ts b/src/config.ts index 69095be81..9bf3ba7c7 100644 --- a/src/config.ts +++ b/src/config.ts @@ -3,6 +3,8 @@ import { copyFileSync, existsSync, mkdirSync, readFileSync, renameSync, unlinkSy import { homedir } from "node:os"; import { join, resolve } from "node:path"; import * as z from "zod/v4"; +import { hardenSecretDir, hardenSecretPath } from "./lib/windows-secret-acl"; +import { providerDestinationConfigError } from "./lib/destination-policy"; import type { OcxConfig } from "./types"; let _atomicSeq = 0; @@ -54,6 +56,7 @@ const warnedConfigFallbacks = new Set(); const providerConfigSchema = z.object({ adapter: z.string().min(1), baseUrl: z.string().min(1), + allowPrivateNetwork: z.boolean().optional(), }).passthrough(); const RESERVED_PROVIDER_NAMES = new Set(["__proto__", "prototype", "constructor"]); @@ -128,6 +131,15 @@ const configSchema = z.object({ path: ["providers", name, "baseUrl"], message: baseUrlError, }); + } else { + const destinationError = providerDestinationConfigError(name, provider); + if (destinationError) { + ctx.addIssue({ + code: "custom", + path: ["providers", name, "baseUrl"], + message: destinationError, + }); + } } const headersError = providerHeadersConfigError((provider as { headers?: unknown }).headers); if (headersError) { @@ -178,15 +190,20 @@ export function hardenConfigDir(): void { const dir = getConfigDir(); if (existsSync(dir)) { try { chmodSync(dir, 0o700); } catch { /* best-effort */ } + if (process.platform === "win32") { + hardenSecretDir(dir, { required: false }); + } } } export function hardenExistingSecret(path: string): void { if (existsSync(path)) { try { chmodSync(path, 0o600); } catch { /* best-effort */ } + if (process.platform === "win32") { + hardenSecretPath(path, { required: false }); + } } } - export function loadConfig(): OcxConfig { const dir = getConfigDir(); const configPath = getConfigPath(); @@ -305,7 +322,11 @@ export function saveConfig(config: OcxConfig): void { } else { try { chmodSync(dir, 0o700); } catch { /* best-effort on existing dir */ } } - atomicWriteFile(getConfigPath(), JSON.stringify(config, null, 2) + "\n"); + if (process.platform === "win32") { + hardenSecretDir(dir, { required: true }); + } + const configPath = getConfigPath(); + atomicWriteFile(configPath, JSON.stringify(config, null, 2) + "\n"); } export function websocketsEnabled(config: Pick): boolean { diff --git a/src/lib/destination-policy.ts b/src/lib/destination-policy.ts new file mode 100644 index 000000000..aa859e49b --- /dev/null +++ b/src/lib/destination-policy.ts @@ -0,0 +1,117 @@ +import { isIP } from "node:net"; +import { getProviderRegistryEntry } from "../providers/registry"; +import type { OcxProviderConfig } from "../types"; + +const BLOCKED_METADATA_HOSTS = new Set([ + "instance-data.ec2.internal", + "metadata.azure.internal", + "metadata.google.internal", +]); + +const BLOCKED_METADATA_IPV4 = new Set([ + "100.100.100.200", + "169.254.169.254", + "169.254.170.2", +]); + +const BLOCKED_METADATA_IPV6 = new Set([ + "fd00:ec2::254", +]); + +type DestinationKind = + | "public" + | "hostname" + | "localhost" + | "loopback" + | "private" + | "link-local" + | "unspecified" + | "metadata"; + +interface DestinationAssessment { + kind: DestinationKind; + detail: string; +} + +function normalizeHostname(hostname: string): string { + const trimmed = hostname.trim().toLowerCase().replace(/\.+$/, ""); + return trimmed.startsWith("[") && trimmed.endsWith("]") ? trimmed.slice(1, -1) : trimmed; +} + +function parseIpv4(hostname: string): number[] | null { + const parts = hostname.split("."); + if (parts.length !== 4) return null; + const octets = parts.map(part => Number(part)); + return octets.every(octet => Number.isInteger(octet) && octet >= 0 && octet <= 255) ? octets : null; +} + +function classifyIpv4(hostname: string): DestinationAssessment { + if (BLOCKED_METADATA_IPV4.has(hostname)) return { kind: "metadata", detail: "blocked metadata endpoint" }; + const octets = parseIpv4(hostname); + if (!octets) return { kind: "public", detail: "public IP" }; + const [a, b] = octets; + if (a === 127) return { kind: "loopback", detail: "loopback address" }; + if (a === 10 || (a === 172 && b >= 16 && b <= 31) || (a === 192 && b === 168) || (a === 100 && b >= 64 && b <= 127)) { + return { kind: "private", detail: "private-network address" }; + } + if (a === 169 && b === 254) return { kind: "link-local", detail: "link-local address" }; + if (a === 0) return { kind: "unspecified", detail: "unspecified address" }; + return { kind: "public", detail: "public IP" }; +} + +function firstIpv6Hextet(hostname: string): number | null { + const head = hostname.split(":")[0]; + if (!head) return 0; + const parsed = Number.parseInt(head, 16); + return Number.isNaN(parsed) ? null : parsed; +} + +function classifyIpv6(hostname: string): DestinationAssessment { + if (BLOCKED_METADATA_IPV6.has(hostname)) return { kind: "metadata", detail: "blocked metadata endpoint" }; + const mappedIpv4 = hostname.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/i)?.[1]; + if (mappedIpv4) return classifyIpv4(mappedIpv4); + if (hostname === "::1") return { kind: "loopback", detail: "loopback address" }; + if (hostname === "::") return { kind: "unspecified", detail: "unspecified address" }; + const hextet = firstIpv6Hextet(hostname); + if (hextet === null) return { kind: "public", detail: "public IP" }; + if (hextet >= 0xfc00 && hextet <= 0xfdff) return { kind: "private", detail: "private-network address" }; + if (hextet >= 0xfe80 && hextet <= 0xfebf) return { kind: "link-local", detail: "link-local address" }; + return { kind: "public", detail: "public IP" }; +} + +function assessDestination(baseUrl: string): DestinationAssessment | null { + try { + const parsed = new URL(baseUrl.trim()); + const hostname = normalizeHostname(parsed.hostname); + if (!hostname) return null; + if (BLOCKED_METADATA_HOSTS.has(hostname)) return { kind: "metadata", detail: "blocked metadata endpoint" }; + if (hostname === "localhost" || hostname.endsWith(".localhost")) { + return { kind: "localhost", detail: "localhost destination" }; + } + const ipKind = isIP(hostname); + if (ipKind === 4) return classifyIpv4(hostname); + if (ipKind === 6) return classifyIpv6(hostname); + return { kind: "hostname", detail: "hostname destination" }; + } catch { + return null; + } +} + +function registryAllowsPrivateNetwork(name: string): boolean { + return getProviderRegistryEntry(name)?.allowPrivateNetworkByDefault === true; +} + +export function providerDestinationConfigError(name: string, provider: Pick): string | null { + const assessment = assessDestination(provider.baseUrl); + if (!assessment) return null; + if (assessment.kind === "public" || assessment.kind === "hostname") return null; + if (assessment.kind === "metadata") return "baseUrl targets a blocked metadata endpoint"; + if (registryAllowsPrivateNetwork(name)) return null; + if (provider.allowPrivateNetwork === true) return null; + return `baseUrl points to a ${assessment.detail}; set allowPrivateNetwork:true only for intentionally local/self-hosted providers`; +} + +export function assertProviderDestinationAllowed(name: string, provider: Pick): void { + const error = providerDestinationConfigError(name, provider); + if (error) throw new Error(`provider ${name} ${error}`); +} diff --git a/src/lib/windows-secret-acl.ts b/src/lib/windows-secret-acl.ts new file mode 100644 index 000000000..8a481d706 --- /dev/null +++ b/src/lib/windows-secret-acl.ts @@ -0,0 +1,173 @@ +/** + * Windows per-user NTFS ACL hardening for secret files and directories. + * + * On Windows, `chmod` only controls POSIX-style bits in the ACE list and does NOT remove + * inherited permissions from other users. Real per-user isolation requires icacls to: + * 1. Disable inheritance (icacls path /inheritance:r) + * 2. Strip broad explicit grants by SID (Everyone, Users, Authenticated Users) + * 3. Grant the current user full control (icacls path /grant:r "CURRENTUSER:(F)") + * + * On non-Windows platforms the helpers fall through to the caller's existing chmod-based + * behaviour: they return ok:true without invoking any external process. + * + * Design: + * hardenSecretPath(path, { required: false }) — non-fatal read-path mode. + * Never throws. Returns { ok, diagnostics? }. + * hardenSecretPath(path, { required: true }) — write-path mode. + * Throws a sanitized error (no raw path) on Windows ACL failure. + * hardenSecretDir — same contract for directories. + */ + +import { execFileSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { env, platform } from "node:process"; + +const hardenedDirectories = new Set(); +const hardenedPaths = new Set(); + +export interface HardenResult { + ok: boolean; + diagnostics?: string; +} + +export interface HardenOptions { + required: boolean; +} + +/** + * Return the current Windows username from the environment. + * Falls back to USERDOMAIN\USERNAME if USERNAME alone is ambiguous. + * The value is used directly in icacls arguments, so it must be present. + */ +function currentWindowsUser(): string | undefined { + const username = env["USERNAME"]; + const domain = env["USERDOMAIN"]; + if (!username) return undefined; + // USERDOMAIN is the machine/domain name; USERNAME is the account name. + // icacls accepts "DOMAIN\User" or just "User" for local accounts. + return domain ? `${domain}\\${username}` : username; +} + +/** + * Run icacls to harden a single file system entry. + * - Disables inheritance (keeps nothing: /inheritance:r) + * - Grants the current user Full Control + * + * We do NOT use a shell string; all arguments are passed as an array so no + * shell injection is possible even for paths with unusual characters. + * + * Throws the raw child_process error on failure (caller sanitizes). + */ +function runIcacls(targetPath: string, directory: boolean): void { + const user = currentWindowsUser(); + if (!user) { + throw new Error("Cannot determine current Windows user for ACL hardening"); + } + + // Step 1: disable inheritance and remove inherited ACEs + execFileSync("icacls.exe", [targetPath, "/inheritance:r"], { + stdio: ["ignore", "pipe", "ignore"], + timeout: 5000, + shell: false, + }); + + // Step 2: remove broad explicit grants using stable SIDs (not localized names). + execFileSync("icacls.exe", [ + targetPath, + "/remove:g", + "*S-1-1-0", + "*S-1-5-11", + "*S-1-5-32-545", + ], { + stdio: ["ignore", "pipe", "ignore"], + timeout: 5000, + shell: false, + }); + + // Step 3: grant current user full control. + const grant = directory ? `${user}:(OI)(CI)(F)` : `${user}:(F)`; + execFileSync("icacls.exe", [targetPath, "/grant:r", grant], { + stdio: ["ignore", "pipe", "ignore"], + timeout: 5000, + shell: false, + }); +} + +/** + * Sanitize an error from a failed ACL operation into a safe diagnostic string. + * The raw path must not appear in the returned string (it may contain + * sensitive username components or PII from the home directory path). + */ +function sanitizeDiagnostics(error: unknown): string { + // We do not expose the raw error message or any path-like fragments. + // Just describe what failed generically. + const code = error instanceof Error && "code" in error ? String((error as NodeJS.ErrnoException).code) : ""; + const codePart = code ? ` (${code})` : ""; + return `ACL hardening failed${codePart} — filesystem may not support per-user NTFS ACLs`; +} + +/** + * Harden a single file path with per-user NTFS ACLs on Windows. + * On non-Windows platforms, returns ok:true immediately (caller owns chmod). + * + * @param targetPath Absolute path to the file to harden. + * @param opts { required: boolean } — required:true throws on failure. + */ +export function hardenSecretPath(targetPath: string, opts: HardenOptions): HardenResult { + // Skip for missing files — we cannot harden what does not exist yet. + if (!existsSync(targetPath)) { + return { ok: true }; + } + + // Non-Windows: no NTFS ACLs; caller handles chmod. + if (platform !== "win32") { + return { ok: true }; + } + + if (hardenedPaths.has(targetPath)) return { ok: true }; + + try { + runIcacls(targetPath, false); + hardenedPaths.add(targetPath); + return { ok: true }; + } catch (err) { + const diagnostics = sanitizeDiagnostics(err); + if (opts.required) { + throw new Error(diagnostics); + } + return { ok: false, diagnostics }; + } +} + +/** + * Harden a directory path with per-user NTFS ACLs on Windows. + * On non-Windows platforms, returns ok:true immediately (caller owns chmod). + * + * @param targetPath Absolute path to the directory to harden. + * @param opts { required: boolean } — required:true throws on failure. + */ +export function hardenSecretDir(targetPath: string, opts: HardenOptions): HardenResult { + // Skip for missing directories — we cannot harden what does not exist yet. + if (!existsSync(targetPath)) { + return { ok: true }; + } + + // Non-Windows: no NTFS ACLs; caller handles chmod. + if (platform !== "win32") { + return { ok: true }; + } + + if (hardenedDirectories.has(targetPath)) return { ok: true }; + + try { + runIcacls(targetPath, true); + hardenedDirectories.add(targetPath); + return { ok: true }; + } catch (err) { + const diagnostics = sanitizeDiagnostics(err); + if (opts.required) { + throw new Error(diagnostics); + } + return { ok: false, diagnostics }; + } +} diff --git a/src/oauth/store.ts b/src/oauth/store.ts index df45d6565..6edb82b6b 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -54,6 +54,7 @@ function persist(store: AuthStore): void { } else { try { chmodSync(dir, 0o700); } catch { /* best-effort on existing dir */ } } + hardenConfigDir(); atomicWriteFile(authPath(), JSON.stringify(store, null, 2) + "\n"); } diff --git a/src/providers/registry.ts b/src/providers/registry.ts index d9c624eab..4dd002af7 100644 --- a/src/providers/registry.ts +++ b/src/providers/registry.ts @@ -18,6 +18,7 @@ export interface ProviderRegistryEntry { adapter: string; baseUrl: string; authKind: ProviderAuthKind; + allowPrivateNetworkByDefault?: boolean; keyOptional?: boolean; allowBaseUrlOverride?: boolean; modelSuffixBracketStrip?: boolean; @@ -436,9 +437,9 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ { id: "google-vertex", label: "Google Vertex AI", adapter: "google", baseUrl: "https://aiplatform.googleapis.com", authKind: "key", dashboardUrl: "https://console.cloud.google.com/vertex-ai", defaultModel: "gemini-3-pro", googleMode: "vertex", jawcodeBundle: "google", extraMetadataAliases: ["gemini-vertex"] }, { id: "google-antigravity", label: "Google Antigravity", adapter: "google", baseUrl: "https://daily-cloudcode-pa.googleapis.com", authKind: "oauth", dashboardUrl: "https://antigravity.google", models: ANTIGRAVITY_MODELS, defaultModel: "gemini-3.5-flash-low", modelContextWindows: ANTIGRAVITY_MODEL_CONTEXT_WINDOWS, googleMode: "cloud-code-assist", jawcodeBundle: "google", extraMetadataAliases: ["antigravity", "gemini-antigravity"] }, { id: "azure-openai", label: "Azure OpenAI", adapter: "azure-openai", baseUrl: "https://{resource}.openai.azure.com/openai", authKind: "key", featured: true, dashboardUrl: "https://portal.azure.com" }, - { id: "ollama", label: "Ollama (local)", adapter: "openai-chat", baseUrl: "http://localhost:11434/v1", authKind: "local", allowBaseUrlOverride: true, featured: true, note: "Local — key usually blank" }, - { id: "vllm", label: "vLLM (local)", adapter: "openai-chat", baseUrl: "http://localhost:8000/v1", authKind: "local", allowBaseUrlOverride: true, featured: true, note: "Local — key usually blank" }, - { id: "lm-studio", label: "LM Studio (local)", adapter: "openai-chat", baseUrl: "http://localhost:1234/v1", authKind: "local", allowBaseUrlOverride: true, featured: true, note: "Local — no key needed" }, + { id: "ollama", label: "Ollama (local)", adapter: "openai-chat", baseUrl: "http://localhost:11434/v1", authKind: "local", allowPrivateNetworkByDefault: true, allowBaseUrlOverride: true, featured: true, note: "Local — key usually blank" }, + { id: "vllm", label: "vLLM (local)", adapter: "openai-chat", baseUrl: "http://localhost:8000/v1", authKind: "local", allowPrivateNetworkByDefault: true, allowBaseUrlOverride: true, featured: true, note: "Local — key usually blank" }, + { id: "lm-studio", label: "LM Studio (local)", adapter: "openai-chat", baseUrl: "http://localhost:1234/v1", authKind: "local", allowPrivateNetworkByDefault: true, allowBaseUrlOverride: true, featured: true, note: "Local — no key needed" }, { id: "deepseek", label: "DeepSeek", @@ -521,6 +522,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ { id: "litellm", label: "LiteLLM (self-hosted)", baseUrl: "http://localhost:4000/v1", adapter: "openai-chat", authKind: "key", dashboardUrl: "https://docs.litellm.ai/docs/proxy/quick_start", + allowPrivateNetworkByDefault: true, allowBaseUrlOverride: true, // A self-hosted proxy may legitimately run without a master key. keyOptional: true, diff --git a/src/router.ts b/src/router.ts index f6d4f6318..eb5315561 100644 --- a/src/router.ts +++ b/src/router.ts @@ -1,5 +1,6 @@ import type { OcxConfig, OcxProviderConfig } from "./types"; import { hasOwnProvider, resolveEnvValue } from "./config"; +import { assertProviderDestinationAllowed } from "./lib/destination-policy"; import { PROVIDER_REGISTRY } from "./providers/registry"; interface RouteResult { @@ -79,7 +80,10 @@ function mergeStringArrayRecord( function routedProviderConfig(providerName: string, provider: OcxProviderConfig): OcxProviderConfig { const registryEntry = PROVIDER_REGISTRY.find(entry => entry.id === providerName); - if (!registryEntry) return { ...provider, apiKey: resolveEnvValue(provider.apiKey) }; + if (!registryEntry) { + assertProviderDestinationAllowed(providerName, provider); + return { ...provider, apiKey: resolveEnvValue(provider.apiKey) }; + } const canonicalAuthMode = registryEntry.authKind === "forward" || registryEntry.authKind === "oauth" ? registryEntry.authKind : provider.authMode === "forward" ? undefined : provider.authMode; @@ -107,6 +111,7 @@ function routedProviderConfig(providerName: string, provider: OcxProviderConfig) const baseUrl = (registryBaseUrlIsTemplate || registryEntry.allowBaseUrlOverride) && userBaseUrlIsResolved ? userBaseUrl : registryEntry.baseUrl; + assertProviderDestinationAllowed(providerName, { baseUrl, allowPrivateNetwork: provider.allowPrivateNetwork }); return { ...provider, diff --git a/src/server/auth-cors.ts b/src/server/auth-cors.ts index 2ed5f35c6..774bbe33c 100644 --- a/src/server/auth-cors.ts +++ b/src/server/auth-cors.ts @@ -5,6 +5,7 @@ import { providerBaseUrlConfigError, providerHeadersConfigError, } from "../config"; +import { providerDestinationConfigError } from "../lib/destination-policy"; import type { OcxConfig, OcxProviderConfig } from "../types"; let _corsOrigin = "http://localhost:10100"; @@ -154,6 +155,8 @@ export function requireApiAuth(req: Request, config: OcxConfig, kind: "managemen export function providerManagementConfigError(name: string, provider: OcxProviderConfig): string | null { const baseUrlError = providerBaseUrlConfigError(provider.baseUrl); if (baseUrlError) return `provider ${name} ${baseUrlError}`; + const destinationError = providerDestinationConfigError(name, provider); + if (destinationError) return `provider ${name} ${destinationError}`; const headersError = providerHeadersConfigError(provider.headers); if (headersError) return `provider ${name} ${headersError}`; if (provider.authMode === "forward") { @@ -203,6 +206,7 @@ export function safeConfigDTO(config: OcxConfig): unknown { for (const key of [ "defaultModel", "disabled", + "allowPrivateNetwork", "authMode", "liveModels", "models", diff --git a/src/server/management-api.ts b/src/server/management-api.ts index 797e80501..2e7485c3f 100644 --- a/src/server/management-api.ts +++ b/src/server/management-api.ts @@ -267,6 +267,7 @@ export async function handleManagementAPI(req: Request, url: URL, config: OcxCon return jsonResponse(Object.entries(config.providers).map(([name, p]) => ({ name, adapter: p.adapter, baseUrl: publicProviderBaseUrl(p.baseUrl), defaultModel: p.defaultModel, hasApiKey: !!p.apiKey, + allowPrivateNetwork: p.allowPrivateNetwork === true, disabled: p.disabled === true, }))); } diff --git a/src/server/request-decompress.ts b/src/server/request-decompress.ts index a16b8a99c..5c87c4ea6 100644 --- a/src/server/request-decompress.ts +++ b/src/server/request-decompress.ts @@ -30,23 +30,27 @@ export class DecompressedBodyTooLargeError extends Error { } } -export function decodeRequestBody(raw: Uint8Array, contentEncoding: string | null): Uint8Array { +function assertBodySizeWithinLimit(body: Uint8Array): Uint8Array { + if (body.byteLength > MAX_DECOMPRESSED_BODY_BYTES) throw new DecompressedBodyTooLargeError(body.byteLength); + return body; +} + +export function decodeRequestBody(raw: Uint8Array, contentEncoding: string | null): Uint8Array { const encoding = (contentEncoding ?? "").trim().toLowerCase(); - if (encoding === "" || encoding === "identity") return raw; + if (encoding === "" || encoding === "identity") return assertBodySizeWithinLimit(raw); + const compressed = raw as Uint8Array; let decoded: Uint8Array; - if (encoding === "zstd") decoded = Bun.zstdDecompressSync(raw); - else if (encoding === "gzip" || encoding === "x-gzip") decoded = Bun.gunzipSync(raw); - else if (encoding === "deflate") decoded = Bun.inflateSync(raw); + if (encoding === "zstd") decoded = Bun.zstdDecompressSync(compressed); + else if (encoding === "gzip" || encoding === "x-gzip") decoded = Bun.gunzipSync(compressed); + else if (encoding === "deflate") decoded = Bun.inflateSync(compressed); // Multi-codings ("zstd, gzip") and unknown tokens are rejected rather than guessed. else throw new UnsupportedContentEncodingError(encoding); - if (decoded.byteLength > MAX_DECOMPRESSED_BODY_BYTES) throw new DecompressedBodyTooLargeError(decoded.byteLength); - return decoded; + return assertBodySizeWithinLimit(decoded); } /** Parse a JSON request body, transparently decoding compressed payloads. */ export async function readJsonRequestBody(req: Request): Promise { const encoding = req.headers.get("content-encoding"); - if (!encoding || encoding.trim().toLowerCase() === "identity") return await req.json(); const decoded = decodeRequestBody(new Uint8Array(await req.arrayBuffer()), encoding); return JSON.parse(new TextDecoder().decode(decoded)); } diff --git a/src/service.ts b/src/service.ts index f41d92806..09988e54d 100644 --- a/src/service.ts +++ b/src/service.ts @@ -16,6 +16,7 @@ import { isWslRuntime } from "./codex/home"; import { durableBunPath, durableBunRuntime } from "./lib/bun-runtime"; import { isProcessAlive, stopProxy } from "./lib/process-control"; import { serviceApiTokenFilePath } from "./lib/service-secrets"; +import { hardenSecretDir, hardenSecretPath } from "./lib/windows-secret-acl"; import { windowsEnvIndirectBatchPathList, windowsEnvIndirectBatchValue } from "./lib/win-paths"; const LABEL = "com.opencodex.proxy"; @@ -103,6 +104,7 @@ function writeServiceInstallState(): void { if (!existsSync(dir)) mkdirSync(dir, { recursive: true, mode: 0o700 }); writeFileSync(path, JSON.stringify(state, null, 2) + "\n", { encoding: "utf8", mode: 0o600 }); try { chmodSync(path, 0o600); } catch { /* best-effort */ } + if (process.platform === "win32") hardenSecretPath(path, { required: true }); } } @@ -169,8 +171,10 @@ function writeServiceApiTokenFile(): string | null { const path = serviceApiTokenFilePath(); const dir = getConfigDir(); if (!existsSync(dir)) mkdirSync(dir, { recursive: true, mode: 0o700 }); + if (process.platform === "win32") hardenSecretDir(dir, { required: true }); writeFileSync(path, `${token}\n`, { encoding: "utf8", mode: 0o600 }); try { chmodSync(path, 0o600); } catch { /* best-effort */ } + if (process.platform === "win32") hardenSecretPath(path, { required: true }); return path; } diff --git a/src/types.ts b/src/types.ts index 10328145c..675e7487b 100644 --- a/src/types.ts +++ b/src/types.ts @@ -419,6 +419,11 @@ export interface OcxWebSearchSidecarConfig { export interface OcxProviderConfig { adapter: string; baseUrl: string; + /** + * Explicit opt-in for non-registry private-network destinations such as localhost, RFC1918, + * link-local, or unique-local upstreams. Metadata endpoints remain blocked. + */ + allowPrivateNetwork?: boolean; /** Keep provider settings on disk but exclude it from routing and model/catalog listings. */ disabled?: boolean; apiKey?: string; diff --git a/tests/cli-models.test.ts b/tests/cli-models.test.ts index e8af82729..8a91f08ce 100644 --- a/tests/cli-models.test.ts +++ b/tests/cli-models.test.ts @@ -29,6 +29,7 @@ function freshConfig(extra?: Record) { test: { adapter: "openai-chat", baseUrl: "http://localhost:8080/v1", + allowPrivateNetwork: true, defaultModel: "test-model-1", models: ["test-model-1", "test-model-2", "test-model-3"], }, @@ -128,6 +129,7 @@ describe("ocx models richer metadata", () => { test: { adapter: "openai-chat", baseUrl: "http://localhost:8080/v1", + allowPrivateNetwork: true, defaultModel: "model-a", models: ["model-a", "model-b"], modelContextWindows: { "model-a": 128000, "model-b": 32000 }, diff --git a/tests/config.test.ts b/tests/config.test.ts index 8d66d7cae..40bd555ae 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -20,6 +20,8 @@ import { writePid, } from "../src/config"; +import * as windowsAcl from "../src/lib/windows-secret-acl"; +import { hardenConfigDir, hardenExistingSecret, saveConfig } from "../src/config"; let testDir = ""; beforeEach(() => { @@ -436,3 +438,121 @@ describe("opencodex config defaults", () => { expect(readRuntimePort()).toBeNull(); }); }); + +describe("config.ts – Windows ACL hardening integration", () => { + test("hardenConfigDir delegates to hardenSecretDir with required:false on win32", () => { + const origPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + try { + const spy = spyOn(windowsAcl, "hardenSecretDir").mockReturnValue({ ok: true }); + mkdirSync(testDir, { recursive: true }); + hardenConfigDir(); + expect(spy).toHaveBeenCalledWith(testDir, { required: false }); + spy.mockRestore(); + } finally { + Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); + } + }); + + test("hardenConfigDir does not call hardenSecretDir on non-Windows", () => { + const origPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "linux", configurable: true }); + try { + const spy = spyOn(windowsAcl, "hardenSecretDir"); + mkdirSync(testDir, { recursive: true }); + hardenConfigDir(); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + } finally { + Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); + } + }); + + test("hardenExistingSecret delegates to hardenSecretPath with required:false on win32", () => { + const origPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + try { + const spy = spyOn(windowsAcl, "hardenSecretPath").mockReturnValue({ ok: true }); + const filePath = join(testDir, "auth.json"); + writeFileSync(filePath, "{}", "utf-8"); + hardenExistingSecret(filePath); + expect(spy).toHaveBeenCalledWith(filePath, { required: false }); + spy.mockRestore(); + } finally { + Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); + } + }); + + test("hardenExistingSecret does not throw when ACL helper returns ok:false on win32", () => { + const origPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + try { + const spy = spyOn(windowsAcl, "hardenSecretPath").mockReturnValue({ + ok: false, + diagnostics: "ACL hardening not supported on this filesystem", + }); + const filePath = join(testDir, "auth.json"); + writeFileSync(filePath, "{}", "utf-8"); + expect(() => hardenExistingSecret(filePath)).not.toThrow(); + spy.mockRestore(); + } finally { + Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); + } + }); + + test("saveConfig applies hardenSecretDir with required:true on win32", () => { + const origPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + try { + const spy = spyOn(windowsAcl, "hardenSecretDir").mockReturnValue({ ok: true }); + saveConfig(getDefaultConfig()); + expect(spy).toHaveBeenCalledWith(testDir, { required: true }); + expect(existsSync(getConfigPath())).toBe(true); + spy.mockRestore(); + } finally { + Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); + } + }); + + test("saveConfig throws when hardenSecretDir fails in required mode on win32", () => { + const origPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + try { + const spy = spyOn(windowsAcl, "hardenSecretDir").mockImplementation((_path, opts) => { + if (opts?.required) throw new Error("ACL hardening failed: access denied"); + return { ok: true }; + }); + expect(() => saveConfig(getDefaultConfig())).toThrow(/ACL/i); + spy.mockRestore(); + } finally { + Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); + } + }); + + test("loadConfig does not throw when ACL helper fails non-fatally on win32", () => { + const origPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + const spy1 = spyOn(windowsAcl, "hardenSecretDir").mockReturnValue({ + ok: false, + diagnostics: "ACL hardening failed — filesystem may not support per-user ACLs", + }); + const spy2 = spyOn(windowsAcl, "hardenSecretPath").mockReturnValue({ ok: false, diagnostics: "ACL unavailable" }); + try { + const config = loadConfig(); + expect(config).toEqual(getDefaultConfig()); + } finally { + spy1.mockRestore(); + spy2.mockRestore(); + Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); + } + }); + + test("saveConfig does not call hardenSecretDir on non-Windows", () => { + if (process.platform === "win32") return; + const spy = spyOn(windowsAcl, "hardenSecretDir"); + saveConfig(getDefaultConfig()); + expect(spy).not.toHaveBeenCalled(); + expect(existsSync(getConfigPath())).toBe(true); + spy.mockRestore(); + }); +}); diff --git a/tests/request-decompress.test.ts b/tests/request-decompress.test.ts index ecee6a4ba..e5c7a92b6 100644 --- a/tests/request-decompress.test.ts +++ b/tests/request-decompress.test.ts @@ -17,6 +17,18 @@ describe("decodeRequestBody", () => { expect(decodeRequestBody(PAYLOAD_BYTES, "identity")).toBe(PAYLOAD_BYTES); }); + test("allows identity bodies exactly at the shared byte cap", () => { + const exact = new Uint8Array(MAX_DECOMPRESSED_BODY_BYTES); + expect(decodeRequestBody(exact, "identity")).toBe(exact); + expect(decodeRequestBody(exact, null)).toBe(exact); + }); + + test("rejects identity bodies over the shared byte cap", () => { + const over = new Uint8Array(MAX_DECOMPRESSED_BODY_BYTES + 1); + expect(() => decodeRequestBody(over, "identity")).toThrow(DecompressedBodyTooLargeError); + expect(() => decodeRequestBody(over, null)).toThrow(DecompressedBodyTooLargeError); + }); + test("round-trips zstd (the codex enable_request_compression encoding)", () => { const compressed = Bun.zstdCompressSync(PAYLOAD_BYTES); expect(new TextDecoder().decode(decodeRequestBody(compressed, "zstd"))).toBe(JSON.stringify(PAYLOAD)); @@ -112,4 +124,31 @@ describe("readJsonRequestBody", () => { }); await expect(readJsonRequestBody(req)).rejects.toBeInstanceOf(DecompressedBodyTooLargeError); }); + + test("surfaces DecompressedBodyTooLargeError for oversized identity bodies too", async () => { + const req = new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json" }, + body: new Uint8Array(MAX_DECOMPRESSED_BODY_BYTES + 1), + }); + await expect(readJsonRequestBody(req)).rejects.toBeInstanceOf(DecompressedBodyTooLargeError); + }); + + test("preserves SyntaxError for malformed identity JSON", async () => { + const req = new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{\"model\":", + }); + await expect(readJsonRequestBody(req)).rejects.toBeInstanceOf(SyntaxError); + }); + + test("preserves SyntaxError for malformed compressed JSON", async () => { + const req = new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json", "content-encoding": "zstd" }, + body: Bun.zstdCompressSync(new TextEncoder().encode("{\"model\":")), + }); + await expect(readJsonRequestBody(req)).rejects.toBeInstanceOf(SyntaxError); + }); }); diff --git a/tests/router.test.ts b/tests/router.test.ts index 2e83c1596..05848a51b 100644 --- a/tests/router.test.ts +++ b/tests/router.test.ts @@ -92,6 +92,69 @@ describe("routeModel registry effort defaults", () => { expect(route.provider.modelReasoningEffortMap).toBeUndefined(); }); + test("blocks custom private-network providers without explicit opt-in before routing", () => { + const config: OcxConfig = { + port: 10100, + defaultProvider: "custom-local", + providers: { + "custom-local": { + adapter: "openai-chat", + baseUrl: "http://127.0.0.1:11434/v1", + models: ["glm-5.2"], + apiKey: "sk-test", + }, + }, + }; + + expect(() => routeModel(config, "custom-local/glm-5.2")).toThrow("allowPrivateNetwork"); + }); + + test("allows trusted self-hosted presets and explicit private-network opt-in", () => { + const trustedPreset: OcxConfig = { + port: 10100, + defaultProvider: "litellm", + providers: { + litellm: { + adapter: "openai-chat", + baseUrl: "http://192.168.1.9:4000/v1", + models: ["gpt-4.1-mini"], + }, + }, + }; + expect(routeModel(trustedPreset, "litellm/gpt-4.1-mini").provider.baseUrl).toBe("http://192.168.1.9:4000/v1"); + + const optedIn: OcxConfig = { + port: 10100, + defaultProvider: "custom-private", + providers: { + "custom-private": { + adapter: "openai-chat", + baseUrl: "http://192.168.1.9:8080/v1", + allowPrivateNetwork: true, + models: ["glm-5.2"], + }, + }, + }; + expect(routeModel(optedIn, "custom-private/glm-5.2").provider.baseUrl).toBe("http://192.168.1.9:8080/v1"); + }); + + test("blocks metadata endpoints even when private-network access is opted in", () => { + const config: OcxConfig = { + port: 10100, + defaultProvider: "custom-metadata", + providers: { + "custom-metadata": { + adapter: "openai-chat", + baseUrl: "http://169.254.169.254/latest/meta-data", + allowPrivateNetwork: true, + models: ["glm-5.2"], + }, + }, + }; + + expect(() => routeModel(config, "custom-metadata/glm-5.2")).toThrow("metadata"); + }); + test("does not hydrate legacy nested xhigh to max maps for stale persisted configs", () => { const config: OcxConfig = { port: 10100, diff --git a/tests/server-auth.test.ts b/tests/server-auth.test.ts index 9789f3ebc..aba51f74c 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -373,6 +373,96 @@ describe("server local API auth", () => { } }); + test("provider management rejects private-network destinations without explicit opt-in", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig(config("127.0.0.1")); + + const server = startServer(0); + try { + const response = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "custom-local", + provider: { + adapter: "openai-chat", + baseUrl: "http://127.0.0.1:11434/v1", + }, + }), + }); + + expect(response.status).toBe(400); + expect(await response.json()).toMatchObject({ + error: expect.stringContaining("allowPrivateNetwork"), + }); + } finally { + await server.stop(true); + } + }); + + test("provider management allows private-network destinations only with explicit opt-in", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig(config("127.0.0.1")); + + const server = startServer(0); + try { + const response = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "custom-local", + provider: { + adapter: "openai-chat", + baseUrl: "http://127.0.0.1:11434/v1", + allowPrivateNetwork: true, + }, + }), + }); + + expect(response.status).toBe(200); + const saved = await fetch(new URL("/api/config", server.url)).then(r => r.json()) as { + providers: Record; + }; + expect(saved.providers["custom-local"].allowPrivateNetwork).toBe(true); + } finally { + await server.stop(true); + } + }); + + test("provider management always rejects metadata endpoints", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig(config("127.0.0.1")); + + const server = startServer(0); + try { + const response = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "metadata-hop", + provider: { + adapter: "openai-chat", + baseUrl: "http://169.254.169.254/latest/meta-data", + allowPrivateNetwork: true, + }, + }), + }); + + expect(response.status).toBe(400); + expect(await response.json()).toMatchObject({ + error: expect.stringContaining("metadata"), + }); + } finally { + await server.stop(true); + } + }); + test("provider management rejects sensitive or injectable provider headers", async () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); mkdirSync(TEST_DIR, { recursive: true }); @@ -951,7 +1041,7 @@ describe("server local API auth", () => { saveConfig({ port: 0, websockets: false, defaultProvider: "routed-fb", providers: { - "routed-fb": { adapter: "openai-chat", baseUrl: `http://127.0.0.1:${upstream.port}/v1`, apiKey: "key-fb-000111222333" }, + "routed-fb": { adapter: "openai-chat", baseUrl: `http://127.0.0.1:${upstream.port}/v1`, allowPrivateNetwork: true, apiKey: "key-fb-000111222333" }, }, } as never); @@ -994,7 +1084,7 @@ describe("server local API auth", () => { saveConfig({ port: 0, defaultProvider: "routed-cmp", providers: { - "routed-cmp": { adapter: "openai-chat", baseUrl: `http://127.0.0.1:${upstream.port}/v1`, apiKey: "key-cmp-000111222333" }, + "routed-cmp": { adapter: "openai-chat", baseUrl: `http://127.0.0.1:${upstream.port}/v1`, allowPrivateNetwork: true, apiKey: "key-cmp-000111222333" }, }, } as never); @@ -1062,6 +1152,7 @@ describe("server local API auth", () => { "anthropic-test": { adapter: "anthropic", baseUrl: upstream.url.toString().replace(/\/$/, ""), + allowPrivateNetwork: true, apiKey: "provider-key", defaultModel: "claude-fable-5", }, @@ -1125,6 +1216,7 @@ describe("server local API auth", () => { chatgpt: { adapter: "openai-responses", baseUrl: `${upstream.url}backend-api/codex`, + allowPrivateNetwork: true, authMode: "forward", }, }, @@ -1217,6 +1309,7 @@ describe("server local API auth", () => { chatgpt: { adapter: "openai-responses", baseUrl: `${upstream.url}backend-api/codex`, + allowPrivateNetwork: true, authMode: "forward", }, }, @@ -1317,6 +1410,7 @@ describe("server local API auth", () => { "anthropic-test": { adapter: "anthropic", baseUrl: upstream.url.toString().replace(/\/$/, ""), + allowPrivateNetwork: true, apiKey: "provider-key", defaultModel: "claude-fable-5", }, @@ -1393,6 +1487,7 @@ describe("server local API auth", () => { chatgpt: { adapter: "openai-responses", baseUrl: "http://127.0.0.1:9/backend-api/codex", + allowPrivateNetwork: true, authMode: "forward", }, }, @@ -1459,6 +1554,7 @@ describe("server local API auth", () => { chatgpt: { adapter: "openai-responses", baseUrl: `${upstream.url}backend-api/codex`, + allowPrivateNetwork: true, authMode: "forward", }, }, @@ -1535,6 +1631,7 @@ describe("server local API auth", () => { "test-openai": { adapter: "openai-responses", baseUrl: upstream.url.toString(), + allowPrivateNetwork: true, apiKey: "provider-key", defaultModel: "gpt-5.5", }, @@ -1689,6 +1786,7 @@ describe("server local API auth", () => { "test-openai": { adapter: "openai-chat", baseUrl: `${upstream.url}v1`, + allowPrivateNetwork: true, apiKey: "provider-key", defaultModel: "gpt-test", }, diff --git a/tests/server-images.test.ts b/tests/server-images.test.ts index 7aac36fa3..f6cf59b30 100644 --- a/tests/server-images.test.ts +++ b/tests/server-images.test.ts @@ -75,7 +75,7 @@ function forwardConfig(baseUrl: string): OcxConfig { port: 0, defaultProvider: "chatgpt", providers: { - chatgpt: { adapter: "openai-responses", baseUrl, authMode: "forward" }, + chatgpt: { adapter: "openai-responses", baseUrl, authMode: "forward", allowPrivateNetwork: true }, }, } as OcxConfig; } @@ -93,10 +93,11 @@ const unreachableChatgptProvider = { adapter: "openai-responses", baseUrl: "http://127.0.0.1:1/backend-api/codex", authMode: "forward", + allowPrivateNetwork: true, } as const; function keyedProvider(baseUrl: string) { - return { adapter: "openai-responses", baseUrl, apiKey: "sk-platform-key" }; + return { adapter: "openai-responses", baseUrl, allowPrivateNetwork: true, apiKey: "sk-platform-key" }; } test("POST /v1/images/generations relays to the ChatGPT forward provider with forwarded auth", async () => { diff --git a/tests/server-key-failover-e2e.test.ts b/tests/server-key-failover-e2e.test.ts index 344ee78c8..b56e760c6 100644 --- a/tests/server-key-failover-e2e.test.ts +++ b/tests/server-key-failover-e2e.test.ts @@ -57,6 +57,7 @@ describe("server 429 key failover (end-to-end)", () => { pooled: { adapter: "openai-chat", baseUrl: `http://127.0.0.1:${upstream.port}/v1`, + allowPrivateNetwork: true, apiKey: "key-alpha-000111222333", apiKeyPool: [ { id: "k1", key: "key-alpha-000111222333", addedAt: 1 }, @@ -155,6 +156,7 @@ describe("server 429 key failover (end-to-end)", () => { textonly: { adapter: "openai-chat", baseUrl: `http://127.0.0.1:${upstream.port}/v1`, + allowPrivateNetwork: true, apiKey: "key-alpha-000111222333", noVisionModels: ["blind-model"], }, diff --git a/tests/server-search.test.ts b/tests/server-search.test.ts index 1ef9a9ef5..95e5cecd5 100644 --- a/tests/server-search.test.ts +++ b/tests/server-search.test.ts @@ -73,7 +73,7 @@ function forwardConfig(baseUrl: string): OcxConfig { port: 0, defaultProvider: "chatgpt", providers: { - chatgpt: { adapter: "openai-responses", baseUrl, authMode: "forward" }, + chatgpt: { adapter: "openai-responses", baseUrl, authMode: "forward", allowPrivateNetwork: true }, }, } as OcxConfig; } @@ -89,6 +89,7 @@ const unreachableChatgptProvider = { adapter: "openai-responses", baseUrl: "http://127.0.0.1:1/backend-api/codex", authMode: "forward", + allowPrivateNetwork: true, } as const; test("POST /v1/alpha/search relays to the ChatGPT forward provider with forwarded auth", async () => { diff --git a/tests/vision-sidecar-e2e.test.ts b/tests/vision-sidecar-e2e.test.ts index 05169149b..313bf3066 100644 --- a/tests/vision-sidecar-e2e.test.ts +++ b/tests/vision-sidecar-e2e.test.ts @@ -98,10 +98,11 @@ describe("vision sidecar fallback (issue #88, end-to-end)", () => { textonly: { adapter: "openai-chat", baseUrl: `http://127.0.0.1:${upstream.port}/v1`, + allowPrivateNetwork: true, apiKey: "key-alpha-000111222333", noVisionModels: ["blind-model"], }, - chatgpt: { adapter: "openai-responses", authMode: "forward", baseUrl: `http://127.0.0.1:${sidecar.port}` }, + chatgpt: { adapter: "openai-responses", authMode: "forward", baseUrl: `http://127.0.0.1:${sidecar.port}`, allowPrivateNetwork: true }, }, } as OcxConfig; saveConfig(config); @@ -141,10 +142,11 @@ describe("vision sidecar fallback (issue #88, end-to-end)", () => { seeing: { adapter: "openai-chat", baseUrl: `http://127.0.0.1:${upstream.port}/v1`, + allowPrivateNetwork: true, apiKey: "key-alpha-000111222333", noVisionModels: ["blind-model"], }, - chatgpt: { adapter: "openai-responses", authMode: "forward", baseUrl: `http://127.0.0.1:${sidecar.port}` }, + chatgpt: { adapter: "openai-responses", authMode: "forward", baseUrl: `http://127.0.0.1:${sidecar.port}`, allowPrivateNetwork: true }, }, } as OcxConfig; saveConfig(config); diff --git a/tests/windows-secret-acl.test.ts b/tests/windows-secret-acl.test.ts new file mode 100644 index 000000000..b614d91f0 --- /dev/null +++ b/tests/windows-secret-acl.test.ts @@ -0,0 +1,210 @@ +/** + * Tests for src/lib/windows-secret-acl.ts + * + * Contract: + * - hardenSecretPath(path, { required: false }) => non-fatal: never throws, returns + * HardenResult { ok, diagnostics? } + * - hardenSecretPath(path, { required: true }) => write-path: throws on failure. + * - On non-Windows platforms: deterministic, no external command invocation. + * - Windows failure diagnostics are sanitized: no raw path in the error message. + * - hardenSecretDir mirrors the same contract for directories. + */ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + hardenSecretDir, + hardenSecretPath, + type HardenResult, +} from "../src/lib/windows-secret-acl"; + +let testDir = ""; + +beforeEach(() => { + testDir = mkdtempSync(join(tmpdir(), "ocx-acl-test-")); +}); + +afterEach(() => { + if (testDir && existsSync(testDir)) rmSync(testDir, { recursive: true, force: true }); + testDir = ""; +}); + +// --------------------------------------------------------------------------- +// Cross-platform: non-fatal (read-path) mode — must never throw +// --------------------------------------------------------------------------- + +describe("hardenSecretPath – non-fatal mode (required: false)", () => { + test("returns ok:true for an existing file", () => { + const filePath = join(testDir, "secret.json"); + writeFileSync(filePath, "data", "utf-8"); + + const result: HardenResult = hardenSecretPath(filePath, { required: false }); + + expect(result.ok).toBe(true); + }); + + test("returns ok:true for a missing file without throwing and without creating it", () => { + const filePath = join(testDir, "nonexistent.json"); + + const result: HardenResult = hardenSecretPath(filePath, { required: false }); + + expect(result.ok).toBe(true); + expect(existsSync(filePath)).toBe(false); + }); + + test("never throws even when the path contains non-ASCII characters", () => { + const filePath = join(testDir, "한글-secret.json"); + writeFileSync(filePath, "data", "utf-8"); + + expect(() => hardenSecretPath(filePath, { required: false })).not.toThrow(); + }); + + test("result has ok boolean and optional diagnostics string fields", () => { + const filePath = join(testDir, "secret.json"); + writeFileSync(filePath, "data", "utf-8"); + + const result = hardenSecretPath(filePath, { required: false }); + + expect(typeof result.ok).toBe("boolean"); + if (result.diagnostics !== undefined) { + expect(typeof result.diagnostics).toBe("string"); + } + }); +}); + +// --------------------------------------------------------------------------- +// Cross-platform: required (write-path) mode on the current platform +// --------------------------------------------------------------------------- + +describe("hardenSecretPath – required mode (required: true)", () => { + test("returns ok:true for an existing file on the current platform", () => { + const filePath = join(testDir, "secret.json"); + writeFileSync(filePath, "data", "utf-8"); + + const result: HardenResult = hardenSecretPath(filePath, { required: true }); + + expect(result.ok).toBe(true); + }); + + test("does not create file when it does not exist even in required mode", () => { + const filePath = join(testDir, "nonexistent-required.json"); + + // required mode on a missing path: should not create the file, return ok:true + const result: HardenResult = hardenSecretPath(filePath, { required: true }); + + expect(result.ok).toBe(true); + expect(existsSync(filePath)).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// hardenSecretDir +// --------------------------------------------------------------------------- + +describe("hardenSecretDir", () => { + test("returns ok:true for an existing directory in non-fatal mode", () => { + const result: HardenResult = hardenSecretDir(testDir, { required: false }); + expect(result.ok).toBe(true); + }); + + test("returns ok:true for an existing directory in required mode", () => { + const result: HardenResult = hardenSecretDir(testDir, { required: true }); + expect(result.ok).toBe(true); + }); + + test("returns ok:true for a missing directory without creating it", () => { + const missingDir = join(testDir, "does-not-exist"); + const result: HardenResult = hardenSecretDir(missingDir, { required: false }); + expect(result.ok).toBe(true); + expect(existsSync(missingDir)).toBe(false); + }); + + test("result shape matches HardenResult interface", () => { + const result = hardenSecretDir(testDir, { required: false }); + expect(typeof result.ok).toBe("boolean"); + if (result.diagnostics !== undefined) { + expect(typeof result.diagnostics).toBe("string"); + } + }); +}); + +// --------------------------------------------------------------------------- +// Windows-specific contract: sanitized diagnostics +// We can only test the real Windows ACL path when running on win32. +// --------------------------------------------------------------------------- + +describe("Windows ACL diagnostics (win32 only)", () => { + const isWin32 = process.platform === "win32"; + + test("on win32: hardenSecretPath returns ok:true for existing file (real ACL)", () => { + if (!isWin32) return; // skip on non-Windows + const filePath = join(testDir, "win-secret.json"); + writeFileSync(filePath, "sensitive data", "utf-8"); + + const result = hardenSecretPath(filePath, { required: false }); + + // On a normal NTFS Windows filesystem, this should succeed + expect(result.ok).toBe(true); + }); + + test("on win32: hardenSecretDir returns ok:true for existing dir (real ACL)", () => { + if (!isWin32) return; // skip on non-Windows + const result = hardenSecretDir(testDir, { required: false }); + expect(result.ok).toBe(true); + }); + + test("on win32: hardenSecretPath with required:true for existing file completes", () => { + if (!isWin32) return; + const filePath = join(testDir, "win-required-secret.json"); + writeFileSync(filePath, "data", "utf-8"); + + // Must not throw on a normal NTFS volume + expect(() => hardenSecretPath(filePath, { required: true })).not.toThrow(); + }); +}); + +// --------------------------------------------------------------------------- +// Non-Windows determinism: helper must not invoke external processes +// We verify this by checking the module uses platform-branched logic. +// On non-Windows we can verify the contract is met without mocking internals. +// --------------------------------------------------------------------------- + +describe("non-Windows determinism", () => { + test("on non-win32: hardenSecretPath completes without error for existing file", () => { + if (process.platform === "win32") return; // This suite is for non-Windows + const filePath = join(testDir, "posix-secret.json"); + writeFileSync(filePath, "data", "utf-8"); + + const result = hardenSecretPath(filePath, { required: false }); + + expect(result.ok).toBe(true); + }); + + test("on non-win32: hardenSecretDir completes without error for existing dir", () => { + if (process.platform === "win32") return; + const result = hardenSecretDir(testDir, { required: false }); + expect(result.ok).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// Diagnostics sanitization: failure messages must not expose raw paths +// This tests the contract via the exported sanitizeDiagnostics helper if present, +// otherwise verifies that hardenSecretPath failure messages meet the contract. +// --------------------------------------------------------------------------- + +describe("diagnostics sanitization contract", () => { + test("HardenResult diagnostics field is a plain string when present", () => { + const filePath = join(testDir, "diag-test.json"); + writeFileSync(filePath, "data", "utf-8"); + + const result = hardenSecretPath(filePath, { required: false }); + + if (result.diagnostics !== undefined) { + expect(typeof result.diagnostics).toBe("string"); + // Must contain "ACL" as a hint (per contract) + expect(result.diagnostics.toLowerCase()).toMatch(/acl|permission|access/i); + } + }); +}); From c6af7f90cdd8067fa83a5cd15335c78adffcfb9a Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sun, 12 Jul 2026 00:26:14 +0200 Subject: [PATCH 2/7] chore: strengthen CI and release safeguards --- .github/ISSUE_TEMPLATE/bug_report.yml | 85 +++++++ .github/ISSUE_TEMPLATE/config.yml | 8 + .github/ISSUE_TEMPLATE/feature_request.yml | 34 +++ .github/PULL_REQUEST_TEMPLATE.md | 13 ++ .github/dependabot.yml | 33 +++ .github/workflows/ci.yml | 3 - CONTRIBUTING.md | 11 + README.md | 4 +- SECURITY.md | 41 ++++ scripts/privacy-scan.ts | 3 +- scripts/release.ts | 8 +- structure/06_docs-and-release.md | 27 ++- tests/release-helper.test.ts | 251 +++++++++++++++++++++ 13 files changed, 506 insertions(+), 15 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.yml create mode 100644 .github/ISSUE_TEMPLATE/config.yml create mode 100644 .github/ISSUE_TEMPLATE/feature_request.yml create mode 100644 .github/PULL_REQUEST_TEMPLATE.md create mode 100644 .github/dependabot.yml create mode 100644 CONTRIBUTING.md create mode 100644 SECURITY.md create mode 100644 tests/release-helper.test.ts diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 000000000..1b72954ee --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,85 @@ +name: Bug report +description: Report a reproducible defect in the proxy, CLI, dashboard, docs, or packaging. +title: "[Bug]: " +labels: + - bug +body: + - type: markdown + attributes: + value: | + Thanks for the report. Please include a minimal reproduction and the exact environment details. + + - type: textarea + id: summary + attributes: + label: Summary + description: What happened, and what did you expect instead? + placeholder: A clear and concise description of the bug. + validations: + required: true + + - type: textarea + id: reproduction + attributes: + label: Reproduction + description: List the exact steps, commands, and config shape needed to reproduce the problem. + placeholder: | + 1. Run `ocx init` + 2. Start the proxy with ... + 3. Open ... + 4. Observe ... + validations: + required: true + + - type: textarea + id: logs + attributes: + label: Logs and screenshots + description: Paste relevant logs, stack traces, request IDs, or screenshots. + render: shell + + - type: dropdown + id: area + attributes: + label: Area + options: + - CLI + - Proxy runtime + - GUI dashboard + - Docs + - Packaging or install + - Provider adapter + - Other + validations: + required: true + + - type: input + id: version + attributes: + label: Version + description: The installed `@bitkyc08/opencodex` version or commit SHA. + placeholder: 2.7.7 + + - type: input + id: os + attributes: + label: OS + description: Operating system and version. + placeholder: Windows 11, macOS 15, Ubuntu 24.04 + + - type: textarea + id: config + attributes: + label: Config shape + description: Share a redacted config snippet if the bug depends on provider or routing setup. + render: json + + - type: checkboxes + id: checks + attributes: + label: Checks + options: + - label: I searched existing issues and docs first. + required: true + - label: I removed secrets, tokens, and personal data from logs and screenshots. + required: true diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 000000000..bf280d67c --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,8 @@ +blank_issues_enabled: true +contact_links: + - name: Security policy + url: https://github.com/lidge-jun/opencodex/blob/main/SECURITY.md + about: Read the supported-version and reporting guidance before sharing security-sensitive details. + - name: Contributing guide + url: https://lidge-jun.github.io/opencodex/contributing/ + about: Review setup, build, and verification guidance for contributors. diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml new file mode 100644 index 000000000..6a87d7dd9 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -0,0 +1,34 @@ +name: Feature request +description: Propose a new capability or workflow improvement. +title: "[Feature]: " +labels: + - enhancement +body: + - type: textarea + id: problem + attributes: + label: Problem to solve + description: What workflow or limitation are you trying to improve? + placeholder: I want opencodex to ... + validations: + required: true + + - type: textarea + id: proposal + attributes: + label: Proposed solution + description: Describe the behavior you want, including any CLI, GUI, or config surface. + validations: + required: true + + - type: textarea + id: alternatives + attributes: + label: Alternatives considered + description: What workarounds or competing approaches did you try? + + - type: textarea + id: context + attributes: + label: Additional context + description: Link docs, screenshots, example configs, or upstream APIs if helpful. diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 000000000..e740a4d99 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,13 @@ +## Summary + +- Explain the user-visible or maintainer-facing change. + +## Verification + +- List the commands or checks you ran. + +## Checklist + +- [ ] Scope stays focused and avoids unrelated cleanup. +- [ ] Docs or release notes were updated when needed. +- [ ] Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 000000000..055692da6 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,33 @@ +version: 2 +updates: + - package-ecosystem: "npm" + directory: "/" + schedule: + interval: "weekly" + open-pull-requests-limit: 10 + labels: + - "dependencies" + + - package-ecosystem: "npm" + directory: "/gui" + schedule: + interval: "weekly" + open-pull-requests-limit: 10 + labels: + - "dependencies" + + - package-ecosystem: "npm" + directory: "/docs-site" + schedule: + interval: "weekly" + open-pull-requests-limit: 10 + labels: + - "dependencies" + + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + open-pull-requests-limit: 10 + labels: + - "dependencies" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2d7ca2f06..437fa19a6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,9 +73,6 @@ jobs: - name: Check release helper syntax run: bun build scripts/release.ts --target=bun --outdir=.tmp/ci-release-script-check - - name: GUI build - run: cd gui && bun install --frozen-lockfile && bun run build - - name: CLI help smoke run: bun run src/cli/index.ts help diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..8498aa0ae --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,11 @@ +# Contributing + +Thanks for helping with opencodex. + +- Start with the canonical guide: [Contributing](https://lidge-jun.github.io/opencodex/contributing/) +- Public user docs live in [`docs-site/`](./docs-site) +- Current maintainer invariants live in [`structure/`](./structure) +- Historical investigations live in [`docs/`](./docs) + +For local development commands, architecture notes, and release workflow details, use the hosted +contributing guide above instead of duplicating instructions here. diff --git a/README.md b/README.md index 58e2c8717..37fe55951 100644 --- a/README.md +++ b/README.md @@ -381,6 +381,8 @@ See the **[Configuration reference](https://lidge-jun.github.io/opencodex/refere The public docs — install, providers, routing, sidecars, Codex integration, Codex App model picker, and CLI/config reference — are built from [`docs-site/`](./docs-site) and published to **[lidge-jun.github.io/opencodex](https://lidge-jun.github.io/opencodex/)**. Maintainer source-of-truth notes live under [`structure/`](./structure). Historical investigations remain under [`docs/`](./docs). +Contributor setup lives in [`CONTRIBUTING.md`](./CONTRIBUTING.md), and security reporting guidance +lives in [`SECURITY.md`](./SECURITY.md). ## Development @@ -402,7 +404,7 @@ the proxy API exposes `/healthz`, `/v1/responses`, `POST /v1/images/generations` bun run dev:gui ``` -See **[Contributing](https://lidge-jun.github.io/opencodex/contributing/)**. +See **[Contributing](./CONTRIBUTING.md)**. ## Disclaimer diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 000000000..a9802c920 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,41 @@ +# Security Policy + +## Supported Versions + +opencodex accepts security fixes on a best-effort basis for these lines: + +| Version | Supported | +| --- | --- | +| `main` | ✅ | +| Latest published npm release | ✅ | +| Older releases | ❌ | + +If you report an issue against an older release, maintainers may ask you to reproduce it on `main` +or the latest published package before triage continues. + +## Reporting a Vulnerability + +Please avoid posting undisclosed vulnerabilities as public GitHub issues. + +- Prefer this repository's GitHub private vulnerability reporting or GitHub Security Advisory flow + when that option is available in the repository UI. +- If no private reporting option is available, do not include exploit details, secrets, or live + targets in a public issue. Open a minimal issue that asks maintainers for a safe coordination path. +- Include affected versions, reproduction steps, impact, and any required configuration details. + +The project does not publish a dedicated private security email in this repository. + +## Response Expectations + +Maintainers will review reports on a best-effort basis. Triage usually starts with: + +- confirming the affected version or commit, +- reproducing the issue locally, +- evaluating impact and safe remediation scope, +- coordinating disclosure timing if a fix is needed. + +## Operational Notes + +- Remove secrets, tokens, cookies, and personal data from screenshots and logs before sharing them. +- For non-sensitive hardening ideas, public issues and pull requests are welcome after disclosure is + no longer sensitive. diff --git a/scripts/privacy-scan.ts b/scripts/privacy-scan.ts index 51bb7e1f7..0eef81faa 100644 --- a/scripts/privacy-scan.ts +++ b/scripts/privacy-scan.ts @@ -1,4 +1,4 @@ -import { readFileSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; type Finding = { file: string; @@ -127,6 +127,7 @@ function scanFile(file: string): Finding[] { } const findings = gitLsFiles() + .filter(existsSync) .filter(shouldScan) .flatMap(scanFile); diff --git a/scripts/release.ts b/scripts/release.ts index ebbd26c97..b35e2c181 100644 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -4,7 +4,7 @@ * * Usage: * bun scripts/release.ts [--tag latest|preview] [--publish] - * Preflight (clean tree on main + typecheck) → bump package.json → commit → push → + * Preflight (clean tree + typecheck + tests + privacy scan) → bump package.json → commit → push → * wait for Cross-platform CI → dispatch the Release workflow → watch it. * The version bump commit/push is real; the Release workflow publish step is dry-run by default. * Pass --publish to publish. @@ -212,7 +212,7 @@ if (!version || !/^\d+\.\d+\.\d+(-[\w.]+)?$/.test(version)) { } const dryRun = !args.includes("--publish"); -// 1. Preflight — must be on main or preview, and typecheck must pass. +// 1. Preflight — must be on main or preview, and local verification must pass. const branch = (await $`git rev-parse --abbrev-ref HEAD`.text()).trim(); const allowedBranches = ["main", "preview"]; const expectedTag = branch === "preview" ? "preview" : "latest"; @@ -236,6 +236,10 @@ console.log(`→ release metadata preflight (${packageName}@${version})`); await assertUnusedReleaseVersion(packageName, version); console.log("→ typecheck"); await $`bun x tsc --noEmit`; +console.log("→ test suite"); +await $`bun test tests`; +console.log("→ privacy scan"); +await $`bun run privacy:scan`; // 2. Bump package.json only; the workflow creates the version tag after npm publish. console.log(`→ bump package.json → ${version}`); diff --git a/structure/06_docs-and-release.md b/structure/06_docs-and-release.md index 7cce6ebc7..86d344003 100644 --- a/structure/06_docs-and-release.md +++ b/structure/06_docs-and-release.md @@ -31,7 +31,7 @@ bun run build | Workflow | Trigger | Purpose | | --- | --- | --- | -| `.github/workflows/ci.yml` | `pull_request`, `push` to `main`/`dev`/`preview`, or manual dispatch when runtime/package paths change | Short Linux + Windows quality gate. `test` job (Bun) runs typecheck/tests/GUI build; `npm-global-smoke` job (Node only, **no setup-bun**) packs and `npm install -g`s the tarball, then runs `ocx help` to prove the bundled-Bun launcher works without a separate Bun install. | +| `.github/workflows/ci.yml` | `pull_request`, `push` to `main`/`dev`/`preview`, or manual dispatch when runtime/package paths change | Cross-platform runtime/package quality gate on Linux, Windows, and macOS. The `test` job (Bun) runs typecheck, `bun test tests`, the privacy scan, release-helper syntax check, GUI lint/build, and `ocx help`; `npm-global-smoke` (Node only, **no setup-bun**) builds package assets, packs the tarball, installs it globally, and runs `ocx help` to prove the bundled-Bun launcher works without a separate Bun install. | | `.github/workflows/release.yml` | Manual dispatch only | npm publish/dry-run workflow. It requires the exact `GITHUB_SHA` to have a successful Cross-platform CI run before publish or dry-run. | | `.github/workflows/deploy-docs.yml` | `push` to `main` touching `docs-site/**` or the workflow, or manual dispatch | Build and publish the Astro/Starlight docs site to GitHub Pages. | | `.github/workflows/service-lifecycle.yml` | `push` touching `src/service.ts`, `src/cli/index.ts`, or the workflow, or manual dispatch | Linux systemd smoke test: install, verify, `ocx stop` stops the service, uninstall. | @@ -76,9 +76,9 @@ Invariants: ## Release workflow Package release is npm-focused. `package.json` exposes `opencodex` and `ocx`, `prepublishOnly` runs -typecheck and GUI build, and `scripts/release.ts` handles version bump, commit/push, waiting for -Cross-platform CI, and dispatching the GitHub Release workflow. Docs publishing is separate from npm -release publishing. +typecheck and GUI build, and `scripts/release.ts` now runs local typecheck, `bun test tests`, and +`bun run privacy:scan` before the version bump, commit/push, Cross-platform CI wait, and GitHub +Release workflow dispatch. Docs publishing is separate from npm release publishing. ## Release metadata invariants @@ -114,19 +114,30 @@ version through `scripts/release.ts`. ## Cross-platform CI `.github/workflows/ci.yml` is the ordinary quality gate for runtime/package changes. It runs on -Linux and Windows only, using the intentionally short command set: +Linux, Windows, and macOS with two job families: ```bash bun install --frozen-lockfile bun x tsc --noEmit bun test tests +bun run privacy:scan bun build scripts/release.ts --target=bun --outdir=.tmp/ci-release-script-check +cd gui && bun install --frozen-lockfile && bun run lint && bun run build bun run src/cli/index.ts help ``` -The CI intentionally does not build docs, build the GUI, run coverage, run macOS, or perform remote -Ubuntu/RDP smoke tests. Those stay outside the default gate until a concrete regression justifies the -extra runtime. +and the Node-only global-install smoke path: + +```bash +npm install +npm run build:gui +npm pack --json > pack.json +npm install -g ./bitkyc08-opencodex-*.tgz +ocx help +``` + +The CI intentionally does not build docs, run coverage, or perform remote Ubuntu/RDP smoke tests. +Those stay outside the default gate until a concrete regression justifies the extra runtime. The Release workflow remains manual and publish-focused. Before any dry-run or publish step, it checks that the exact release commit (`GITHUB_SHA`) already has a successful Cross-platform CI run. diff --git a/tests/release-helper.test.ts b/tests/release-helper.test.ts new file mode 100644 index 000000000..773bada5b --- /dev/null +++ b/tests/release-helper.test.ts @@ -0,0 +1,251 @@ +import { describe, expect, setDefaultTimeout, test } from "bun:test"; +import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +setDefaultTimeout(30_000); + +const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); +const releaseScriptPath = join(repoRoot, "scripts", "release.ts"); + +interface LoggedCall { + args: string[]; + name: string; +} + +interface ReleaseScenario { + branch?: string; + headSha?: string; + privacyExitCode?: number; + testExitCode?: number; + typecheckExitCode?: number; +} + +function writeExecutable(path: string, contents: string): void { + writeFileSync(path, contents, "utf8"); + chmodSync(path, 0o755); +} + +function shimProgramSource(name: "bun" | "gh" | "git" | "npm"): string { + if (name === "bun") { + return `import { appendFileSync } from "node:fs"; + +const args = process.argv.slice(2); +appendFileSync(process.env.FAKE_RELEASE_LOG, JSON.stringify({ name: "bun", args }) + "\\n"); + +const exitCode = + args[0] === "x" && args[1] === "tsc" ? Number(process.env.FAKE_BUN_TSC_EXIT_CODE ?? "0") + : args[0] === "test" && args[1] === "tests" ? Number(process.env.FAKE_BUN_TEST_EXIT_CODE ?? "0") + : args[0] === "run" && args[1] === "privacy:scan" ? Number(process.env.FAKE_BUN_PRIVACY_EXIT_CODE ?? "0") + : 0; + +if (exitCode !== 0) { + console.error(\`fake bun failure: \${args.join(" ")}\`); +} + +process.exit(exitCode); +`; + } + + if (name === "git") { + return `import { appendFileSync } from "node:fs"; + +const args = process.argv.slice(2); +appendFileSync(process.env.FAKE_RELEASE_LOG, JSON.stringify({ name: "git", args }) + "\\n"); + +const headSha = process.env.FAKE_GIT_HEAD_SHA ?? "abc123def456"; +const branch = process.env.FAKE_GIT_BRANCH ?? "main"; +const stdout = (text) => process.stdout.write(text); +const stderr = (text) => process.stderr.write(text); + +if (args[0] === "rev-parse" && args[1] === "--abbrev-ref" && args[2] === "HEAD") { + stdout(branch + "\\n"); + process.exit(0); +} + +if (args[0] === "status" && args[1] === "--porcelain") { + stdout((process.env.FAKE_GIT_STATUS ?? "") + "\\n"); + process.exit(0); +} + +if (args[0] === "ls-remote") { + process.exit(0); +} + +if (args[0] === "add" || args[0] === "commit" || args[0] === "push") { + process.exit(0); +} + +if (args[0] === "rev-parse" && args[1] === "HEAD") { + stdout(headSha + "\\n"); + process.exit(0); +} + +if (args[0] === "rev-parse" && args[1]?.startsWith("origin/")) { + stdout(headSha + "\\n"); + process.exit(0); +} + +stderr(\`unexpected git args: \${args.join(" ")}\\n\`); +process.exit(1); +`; + } + + if (name === "npm") { + return `import { appendFileSync } from "node:fs"; + +const args = process.argv.slice(2); +appendFileSync(process.env.FAKE_RELEASE_LOG, JSON.stringify({ name: "npm", args }) + "\\n"); + +if (args[0] === "view") { + console.error("npm ERR! code E404"); + process.exit(1); +} + +if (args[0] === "version") { + process.exit(0); +} + +console.error(\`unexpected npm args: \${args.join(" ")}\`); +process.exit(1); +`; + } + + return `import { appendFileSync } from "node:fs"; + +const args = process.argv.slice(2); +appendFileSync(process.env.FAKE_RELEASE_LOG, JSON.stringify({ name: "gh", args }) + "\\n"); + +const headSha = process.env.FAKE_GIT_HEAD_SHA ?? "abc123def456"; +const stdout = (text) => process.stdout.write(text); +const stderr = (text) => process.stderr.write(text); + +if (args[0] === "release" && args[1] === "view") { + stderr("release not found\\n"); + process.exit(1); +} + +if (args[0] === "run" && args[1] === "list") { + if (args.includes("ci.yml")) { + stdout(JSON.stringify([{ conclusion: "success", databaseId: 7, headSha, status: "completed", url: "https://example.test/ci" }])); + process.exit(0); + } + + if (args.includes("release.yml")) { + stdout(JSON.stringify([{ createdAt: new Date().toISOString(), databaseId: 9, headSha, status: "queued", url: "https://example.test/release" }])); + process.exit(0); + } +} + +if (args[0] === "workflow" && args[1] === "run") { + process.exit(0); +} + +if (args[0] === "run" && args[1] === "watch") { + process.exit(0); +} + +stderr(\`unexpected gh args: \${args.join(" ")}\\n\`); +process.exit(1); +`; +} + +function installCommandShim(binDir: string, name: "bun" | "gh" | "git" | "npm"): void { + const jsPath = join(binDir, `${name}.js`); + const launcherPath = join(binDir, name); + const cmdPath = join(binDir, `${name}.cmd`); + + writeFileSync(jsPath, shimProgramSource(name), "utf8"); + writeExecutable(launcherPath, `#!${process.execPath}\nimport "./${name}.js";\n`); + writeFileSync(cmdPath, `@echo off\r\n"${process.execPath}" "%~dp0\\${name}.js" %*\r\n`, "utf8"); +} + +function readLoggedCalls(logPath: string): LoggedCall[] { + const raw = readFileSync(logPath, "utf8").trim(); + if (!raw) return []; + return raw.split(/\r?\n/).filter(Boolean).map(line => JSON.parse(line) as LoggedCall); +} + +function findCallIndex(calls: LoggedCall[], name: string, matcher: (call: LoggedCall) => boolean): number { + return calls.findIndex(call => call.name === name && matcher(call)); +} + +function runRelease(version: string, scenario: ReleaseScenario = {}) { + const shimDir = mkdtempSync(join(tmpdir(), "ocx-release-helper-")); + const logPath = join(shimDir, "release-log.jsonl"); + writeFileSync(logPath, "", "utf8"); + + for (const name of ["bun", "gh", "git", "npm"] as const) { + installCommandShim(shimDir, name); + } + + const result = spawnSync(process.execPath, [releaseScriptPath, version], { + cwd: repoRoot, + env: { + ...process.env, + PATH: `${shimDir}${process.platform === "win32" ? ";" : ":"}${process.env.PATH ?? ""}`, + FAKE_RELEASE_LOG: logPath, + FAKE_GIT_BRANCH: scenario.branch ?? "main", + FAKE_GIT_HEAD_SHA: scenario.headSha ?? "abc123def456", + FAKE_BUN_TSC_EXIT_CODE: String(scenario.typecheckExitCode ?? 0), + FAKE_BUN_TEST_EXIT_CODE: String(scenario.testExitCode ?? 0), + FAKE_BUN_PRIVACY_EXIT_CODE: String(scenario.privacyExitCode ?? 0), + }, + encoding: "utf8", + }); + + const calls = readLoggedCalls(logPath); + rmSync(shimDir, { recursive: true, force: true }); + return { calls, result }; +} + +describe("release helper", () => { + test("preflight runs typecheck, test suite, and privacy scan before version bump on main dry-runs", () => { + const { calls, result } = runRelease("9.9.9"); + + expect(result.status).toBe(0); + + const typecheckIndex = findCallIndex(calls, "bun", call => call.args.join(" ") === "x tsc --noEmit"); + const testIndex = findCallIndex(calls, "bun", call => call.args.join(" ") === "test tests"); + const privacyIndex = findCallIndex(calls, "bun", call => call.args.join(" ") === "run privacy:scan"); + const versionIndex = findCallIndex(calls, "npm", call => call.args.join(" ") === "version 9.9.9 --no-git-tag-version"); + const dispatchIndex = findCallIndex(calls, "gh", call => + call.args[0] === "workflow" + && call.args[1] === "run" + && call.args.includes("release.yml") + && call.args.includes("tag=latest") + && call.args.includes("dry-run=true"), + ); + + expect(typecheckIndex).toBeGreaterThanOrEqual(0); + expect(testIndex).toBeGreaterThan(typecheckIndex); + expect(privacyIndex).toBeGreaterThan(testIndex); + expect(versionIndex).toBeGreaterThan(privacyIndex); + expect(dispatchIndex).toBeGreaterThan(versionIndex); + }); + + test("failed privacy scan aborts before version bump, commit, and push", () => { + const { calls, result } = runRelease("9.9.9", { privacyExitCode: 1 }); + + expect(result.status).not.toBe(0); + expect(findCallIndex(calls, "bun", call => call.args.join(" ") === "run privacy:scan")).toBeGreaterThanOrEqual(0); + expect(findCallIndex(calls, "npm", call => call.args[0] === "version")).toBe(-1); + expect(findCallIndex(calls, "git", call => call.args[0] === "commit")).toBe(-1); + expect(findCallIndex(calls, "git", call => call.args[0] === "push")).toBe(-1); + }); + + test("preview branch still defaults to preview tag and dry-run dispatch", () => { + const { calls, result } = runRelease("9.9.9-preview.1", { branch: "preview" }); + + expect(result.status).toBe(0); + expect(findCallIndex(calls, "gh", call => + call.args[0] === "workflow" + && call.args[1] === "run" + && call.args.includes("release.yml") + && call.args.includes("tag=preview") + && call.args.includes("dry-run=true"), + )).toBeGreaterThanOrEqual(0); + }); +}); From 6bb3aecfa8e14a4f91763e7e3a4aa605de77d4d4 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sun, 12 Jul 2026 00:26:42 +0200 Subject: [PATCH 3/7] refactor: share upstream retry and error handling --- src/adapters/cursor/transport-retry.ts | 8 ++- src/adapters/google-errors.ts | 28 +++----- src/adapters/google-http.ts | 95 ++++++++------------------ src/adapters/kiro-errors.ts | 33 +++------ src/adapters/kiro-retry.ts | 84 +++++++---------------- src/adapters/upstream-http-error.ts | 48 +++++++++++++ src/lib/upstream-retry.ts | 56 ++++++++++++++- tests/upstream-http-error.test.ts | 43 ++++++++++++ tests/upstream-retry.test.ts | 45 +++++++++++- 9 files changed, 267 insertions(+), 173 deletions(-) create mode 100644 src/adapters/upstream-http-error.ts create mode 100644 tests/upstream-http-error.test.ts diff --git a/src/adapters/cursor/transport-retry.ts b/src/adapters/cursor/transport-retry.ts index c6f29206e..210afbfea 100644 --- a/src/adapters/cursor/transport-retry.ts +++ b/src/adapters/cursor/transport-retry.ts @@ -1,6 +1,6 @@ import type { CursorRunRequest, CursorServerMessage } from "./types"; import type { CursorTransport, CursorTransportFactory, CursorTransportFactoryInput } from "./transport"; -import { abortError, sleepWithAbort } from "../../lib/upstream-retry"; +import { abortError, retryBackoffDelayMs, sleepWithAbort } from "../../lib/upstream-retry"; import { debugProviderDiagnostic } from "../../lib/debug"; import { safeCursorErrorMessage } from "./cursor-errors"; @@ -40,8 +40,10 @@ export function isRetryableCursorError(err: unknown): boolean { } export function cursorRetryDelayMs(attempt: number): number { - const exp = Math.min(CURSOR_RETRY_BASE_MS * 2 ** attempt, CURSOR_RETRY_MAX_MS); - return Math.floor(exp * (0.8 + Math.random() * 0.4)); + return retryBackoffDelayMs(attempt, { + baseDelayMs: CURSOR_RETRY_BASE_MS, + maxDelayMs: CURSOR_RETRY_MAX_MS, + }); } /** diff --git a/src/adapters/google-errors.ts b/src/adapters/google-errors.ts index 251234752..69e6d0cef 100644 --- a/src/adapters/google-errors.ts +++ b/src/adapters/google-errors.ts @@ -1,14 +1,4 @@ -import { redactSecretString } from "../lib/redact"; - -const ABSOLUTE_PATH_PATTERN = /(?:\/Users\/[^ "';,]+|\/home\/[^ "';,]+|\/root\/[^ "';,]*|[A-Za-z]:\\Users\\[^ "';,]+)/g; - -function sanitizeGoogleErrorText(value: string): string { - return redactSecretString(value).replace(ABSOLUTE_PATH_PATTERN, "[REDACTED_PATH]"); -} - -function safeString(value: unknown): string | undefined { - return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined; -} +import { parseUpstreamJsonPayload, safeUpstreamErrorString, sanitizeUpstreamErrorText } from "./upstream-http-error"; /** Pull the human detail out of the Google API error envelope `{error:{message,status,code}}`. */ function googleErrorDetail(payloadText: string): { message?: string; status?: string } { @@ -16,13 +6,13 @@ function googleErrorDetail(payloadText: string): { message?: string; status?: st if (!trimmed || (!trimmed.startsWith("{") && !trimmed.startsWith("["))) { return { message: trimmed || undefined }; } - try { - const parsed = JSON.parse(trimmed) as { error?: { message?: unknown; status?: unknown } }; - const err = parsed.error; - return { message: safeString(err?.message), status: safeString(err?.status) }; - } catch { - return {}; - } + const parsed = parseUpstreamJsonPayload(trimmed); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return {}; + const err = (parsed as { error?: { message?: unknown; status?: unknown } }).error; + return { + message: safeUpstreamErrorString(err?.message), + status: safeUpstreamErrorString(err?.status), + }; } function classifyGoogle(label: string, status: number | undefined, enumStatus: string | undefined, text: string): string { @@ -59,7 +49,7 @@ function classifyGoogle(label: string, status: number | undefined, enumStatus: s export function safeGoogleHttpErrorMessage(label: string, status: number, payloadText: string): string { const { message, status: enumStatus } = googleErrorDetail(payloadText); const prefix = classifyGoogle(label, status, enumStatus, [message, enumStatus].filter(Boolean).join(" ")); - const detail = message ? sanitizeGoogleErrorText(message).slice(0, 500) : `HTTP ${status}`; + const detail = message ? sanitizeUpstreamErrorText(message).slice(0, 500) : `HTTP ${status}`; return `${prefix}: ${detail}`; } diff --git a/src/adapters/google-http.ts b/src/adapters/google-http.ts index a1d0ee85e..924a1c93f 100644 --- a/src/adapters/google-http.ts +++ b/src/adapters/google-http.ts @@ -1,57 +1,22 @@ import type { AdapterFetchContext, AdapterRequest } from "./base"; import { isQuotaExhaustedBody, retryableGoogleStatus, safeGoogleHttpErrorMessage } from "./google-errors"; -import { clearableDeadline } from "../lib/abort"; -import { readBoundedResponseBody } from "../lib/bounded-body"; -import { abortError, sleepWithAbort } from "../lib/upstream-retry"; +import { normalizeUpstreamHttpErrorResponse, readDisplaySafeErrorPayloadText } from "./upstream-http-error"; +import { + abortError, + cancelResponseBodyBestEffort, + fetchWithAttemptDeadline, + retryBackoffDelayMs, + sleepWithAbort, +} from "../lib/upstream-retry"; const GOOGLE_RETRY_ATTEMPTS = 3; const GOOGLE_RETRY_BASE_MS = 250; const GOOGLE_RETRY_MAX_MS = 2_000; -function retryAfterMs(headers: Headers): number | undefined { - const raw = headers.get("retry-after")?.trim(); - if (!raw) return undefined; - const seconds = Number(raw); - if (Number.isFinite(seconds)) return Math.max(0, seconds * 1000); - const dateMs = Date.parse(raw); - if (!Number.isFinite(dateMs)) return undefined; - return Math.max(0, dateMs - Date.now()); -} - -function retryDelayMs(attempt: number, headers?: Headers): number { - const retryAfter = headers ? retryAfterMs(headers) : undefined; - if (retryAfter !== undefined) return Math.min(retryAfter, GOOGLE_RETRY_MAX_MS); - const exp = Math.min(GOOGLE_RETRY_BASE_MS * (2 ** attempt), GOOGLE_RETRY_MAX_MS); - return Math.floor(exp * (0.8 + Math.random() * 0.4)); -} - -function cancelResponseBodyBestEffort(res: Response): void { - try { - const cancellation = res.body?.cancel(); - if (cancellation) void cancellation.catch(() => {}); - } catch { - // Cancellation is cleanup only; retries must not wait for or fail because of it. - } -} - -async function boundedBodyText(res: Response, signal?: AbortSignal): Promise { - try { - const body = await readBoundedResponseBody(res, { signal }); - return body.displaySafe ? body.text : ""; - } catch (error) { - if (signal?.aborted) throw error; - return ""; - } -} - async function normalizeFinalGoogleError(label: string, res: Response, signal?: AbortSignal): Promise { - if (res.ok) return res; - const payloadText = await boundedBodyText(res, signal); - const headers = new Headers(res.headers); - headers.delete("content-encoding"); - headers.delete("content-length"); - return new Response(safeGoogleHttpErrorMessage(label, res.status, payloadText), { - status: res.status, statusText: res.statusText, headers, + return normalizeUpstreamHttpErrorResponse(res, { + signal, + formatMessage: payloadText => safeGoogleHttpErrorMessage(label, res.status, payloadText), }); } @@ -67,41 +32,39 @@ export async function fetchGoogleWithRetry(label: string, request: AdapterReques for (let attempt = 0; attempt < GOOGLE_RETRY_ATTEMPTS; attempt++) { if (ctx.abortSignal?.aborted) throw abortError(ctx.abortSignal); try { - const attemptTimeout = clearableDeadline(timeoutMs, ctx.abortSignal); - let res: Response; - try { - res = await fetch(request.url, { - method: request.method, headers: request.headers, body: request.body, - signal: attemptTimeout.signal, - }); - } finally { - // Only the header timer is cleared. The composed signal still contains the parent, so a - // caller abort after headers continue to cancel consumption of the returned response body. - attemptTimeout.clear(); - } + const res = await fetchWithAttemptDeadline(request.url, { + method: request.method, + headers: request.headers, + body: request.body, + }, timeoutMs, ctx.abortSignal); if (!retryableGoogleStatus(res.status) || attempt === GOOGLE_RETRY_ATTEMPTS - 1) { return ctx.returnRawErrors ? res : normalizeFinalGoogleError(label, res, ctx.abortSignal); } // A 429 may be a transient rate limit (retry) or hard quota exhaustion (do NOT retry — // it won't recover for hours and burns retries). Peek the body to tell them apart. if (res.status === 429 && !ctx.returnRawErrors) { - const peek = await boundedBodyText(res, ctx.abortSignal); + const peek = await readDisplaySafeErrorPayloadText(res, ctx.abortSignal); if (isQuotaExhaustedBody(peek)) { - const headers = new Headers(res.headers); - headers.delete("content-encoding"); - headers.delete("content-length"); - return new Response(safeGoogleHttpErrorMessage(label, res.status, peek), { - status: res.status, statusText: res.statusText, headers, + return normalizeUpstreamHttpErrorResponse(res, { + signal: ctx.abortSignal, + formatMessage: payloadText => safeGoogleHttpErrorMessage(label, res.status, payloadText || peek), }); } } cancelResponseBodyBestEffort(res); - await sleepWithAbort(retryDelayMs(attempt, res.headers), ctx.abortSignal); + await sleepWithAbort(retryBackoffDelayMs(attempt, { + baseDelayMs: GOOGLE_RETRY_BASE_MS, + maxDelayMs: GOOGLE_RETRY_MAX_MS, + headers: res.headers, + }), ctx.abortSignal); } catch (err) { if (ctx.abortSignal?.aborted) throw err; lastError = err; if (attempt === GOOGLE_RETRY_ATTEMPTS - 1) throw err; - await sleepWithAbort(retryDelayMs(attempt), ctx.abortSignal); + await sleepWithAbort(retryBackoffDelayMs(attempt, { + baseDelayMs: GOOGLE_RETRY_BASE_MS, + maxDelayMs: GOOGLE_RETRY_MAX_MS, + }), ctx.abortSignal); } } throw lastError ?? new Error(`${label} fetch failed`); diff --git a/src/adapters/kiro-errors.ts b/src/adapters/kiro-errors.ts index 753f172f0..ed6da15e4 100644 --- a/src/adapters/kiro-errors.ts +++ b/src/adapters/kiro-errors.ts @@ -1,35 +1,22 @@ -import { redactSecretString } from "../lib/redact"; - -const ABSOLUTE_PATH_PATTERN = /(?:\/Users\/[^ "';,]+|\/home\/[^ "';,]+|[A-Za-z]:\\Users\\[^ "';,]+)/g; +import { parseUpstreamJsonPayload, safeUpstreamErrorString, sanitizeUpstreamErrorText } from "./upstream-http-error"; const DETAIL_KEYS = ["__type", "code", "error", "name", "message", "Message", "errorMessage"]; -function sanitizeKiroErrorText(value: string): string { - return redactSecretString(value).replace(ABSOLUTE_PATH_PATTERN, "[REDACTED_PATH]"); -} - -function safeString(value: unknown): string | undefined { - return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined; -} - function headerValue(headers: Headers | Record, name: string): string | undefined { - if (headers instanceof Headers) return name.startsWith(":") ? undefined : safeString(headers.get(name)); - return safeString(headers[name]) || safeString(headers[name.toLowerCase()]); + if (headers instanceof Headers) return name.startsWith(":") ? undefined : safeUpstreamErrorString(headers.get(name)); + return safeUpstreamErrorString(headers[name]) || safeUpstreamErrorString(headers[name.toLowerCase()]); } function payloadDetails(payloadText: string): string[] { const trimmed = payloadText.trim(); if (!trimmed) return []; if (!trimmed.startsWith("{") && !trimmed.startsWith("[")) return [trimmed]; - try { - const parsed = JSON.parse(trimmed) as unknown; - if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { - const obj = parsed as Record; - return DETAIL_KEYS.map(key => safeString(obj[key])).filter((v): v is string => !!v); - } - if (typeof parsed === "string" && parsed.trim()) return [parsed.trim()]; - } catch { - return []; + const parsed = parseUpstreamJsonPayload(trimmed); + if (parsed === undefined) return []; + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + const obj = parsed as Record; + return DETAIL_KEYS.map(key => safeUpstreamErrorString(obj[key])).filter((v): v is string => !!v); } + if (typeof parsed === "string" && parsed.trim()) return [parsed.trim()]; return []; } @@ -87,7 +74,7 @@ function classifyKiroText(status: number | undefined, text: string): string { function normalizedKiroErrorMessage(headers: Headers | Record, payloadText: string, status?: number): string { const headerType = headerValue(headers, ":exception-type") || headerValue(headers, ":error-type"); const parts = [headerType, ...payloadDetails(payloadText)].filter((part): part is string => !!part); - const detail = parts.length > 0 ? sanitizeKiroErrorText(parts.join(": ")).slice(0, 500) : status ? `HTTP ${status}` : ""; + const detail = parts.length > 0 ? sanitizeUpstreamErrorText(parts.join(": ")).slice(0, 500) : status ? `HTTP ${status}` : ""; const prefix = classifyKiroText(status, [detail, headerType].filter(Boolean).join(" ")); return detail ? `${prefix}: ${detail}` : prefix; } diff --git a/src/adapters/kiro-retry.ts b/src/adapters/kiro-retry.ts index 10c85e8ce..c81934115 100644 --- a/src/adapters/kiro-retry.ts +++ b/src/adapters/kiro-retry.ts @@ -1,8 +1,14 @@ import type { AdapterFetchContext, AdapterRequest } from "./base"; import { safeKiroHttpErrorMessage } from "./kiro-errors"; -import { clearableDeadline } from "../lib/abort"; -import { readBoundedResponseBody } from "../lib/bounded-body"; -import { abortError, isConnectionResetError, sleepWithAbort } from "../lib/upstream-retry"; +import { normalizeUpstreamHttpErrorResponse } from "./upstream-http-error"; +import { + abortError, + cancelResponseBodyBestEffort, + fetchWithAttemptDeadline, + isConnectionResetError, + retryBackoffDelayMs, + sleepWithAbort, +} from "../lib/upstream-retry"; const KIRO_RETRY_ATTEMPTS = 3; const KIRO_RETRY_BASE_MS = 250; @@ -12,52 +18,14 @@ function retryableKiroStatus(status: number): boolean { return status === 429 || status === 500 || status === 502 || status === 503 || status === 504; } -function retryAfterMs(headers: Headers): number | undefined { - const raw = headers.get("retry-after")?.trim(); - if (!raw) return undefined; - const seconds = Number(raw); - if (Number.isFinite(seconds)) return Math.max(0, seconds * 1000); - const dateMs = Date.parse(raw); - if (!Number.isFinite(dateMs)) return undefined; - return Math.max(0, dateMs - Date.now()); -} - -function retryDelayMs(attempt: number, headers?: Headers): number { - const retryAfter = headers ? retryAfterMs(headers) : undefined; - if (retryAfter !== undefined) return Math.min(retryAfter, KIRO_RETRY_MAX_MS); - const exp = Math.min(KIRO_RETRY_BASE_MS * (2 ** attempt), KIRO_RETRY_MAX_MS); - return Math.floor(exp * (0.8 + Math.random() * 0.4)); -} - -function cancelResponseBodyBestEffort(res: Response): void { - try { - const cancellation = res.body?.cancel(); - if (cancellation) void cancellation.catch(() => {}); - } catch { - // Cancellation is cleanup only; retries must not wait for or fail because of it. - } -} - function retryableKiroFetchError(err: unknown): boolean { return isConnectionResetError(err) || (err instanceof Error && err.name === "TimeoutError"); } async function normalizeFinalKiroHttpError(res: Response, signal?: AbortSignal): Promise { - if (res.ok) return res; - let payloadText = ""; - try { - const body = await readBoundedResponseBody(res, { signal }); - if (body.displaySafe) payloadText = body.text; - } catch (error) { - if (signal?.aborted) throw error; - } - const headers = new Headers(res.headers); - headers.delete("content-encoding"); - headers.delete("content-length"); - return new Response(safeKiroHttpErrorMessage(res.status, res.headers, payloadText), { - status: res.status, - statusText: res.statusText, - headers, + return normalizeUpstreamHttpErrorResponse(res, { + signal, + formatMessage: payloadText => safeKiroHttpErrorMessage(res.status, res.headers, payloadText), }); } @@ -67,28 +35,28 @@ export async function fetchKiroWithRetry(request: AdapterRequest, ctx: AdapterFe for (let attempt = 0; attempt < KIRO_RETRY_ATTEMPTS; attempt++) { if (ctx.abortSignal?.aborted) throw abortError(ctx.abortSignal); try { - const attemptTimeout = clearableDeadline(timeoutMs, ctx.abortSignal); - let res: Response; - try { - res = await fetch(request.url, { - method: request.method, - headers: request.headers, - body: request.body, - signal: attemptTimeout.signal, - }); - } finally { - attemptTimeout.clear(); - } + const res = await fetchWithAttemptDeadline(request.url, { + method: request.method, + headers: request.headers, + body: request.body, + }, timeoutMs, ctx.abortSignal); if (!retryableKiroStatus(res.status) || attempt === KIRO_RETRY_ATTEMPTS - 1) { return ctx.returnRawErrors ? res : normalizeFinalKiroHttpError(res, ctx.abortSignal); } cancelResponseBodyBestEffort(res); - await sleepWithAbort(retryDelayMs(attempt, res.headers), ctx.abortSignal); + await sleepWithAbort(retryBackoffDelayMs(attempt, { + baseDelayMs: KIRO_RETRY_BASE_MS, + maxDelayMs: KIRO_RETRY_MAX_MS, + headers: res.headers, + }), ctx.abortSignal); } catch (err) { if (ctx.abortSignal?.aborted) throw err; if (!retryableKiroFetchError(err) || attempt === KIRO_RETRY_ATTEMPTS - 1) throw err; lastError = err; - await sleepWithAbort(retryDelayMs(attempt), ctx.abortSignal); + await sleepWithAbort(retryBackoffDelayMs(attempt, { + baseDelayMs: KIRO_RETRY_BASE_MS, + maxDelayMs: KIRO_RETRY_MAX_MS, + }), ctx.abortSignal); } } throw lastError ?? new Error("Kiro fetch failed"); diff --git a/src/adapters/upstream-http-error.ts b/src/adapters/upstream-http-error.ts new file mode 100644 index 000000000..f0627ceff --- /dev/null +++ b/src/adapters/upstream-http-error.ts @@ -0,0 +1,48 @@ +import { readBoundedResponseBody } from "../lib/bounded-body"; +import { redactSecretString } from "../lib/redact"; + +const ABSOLUTE_PATH_PATTERN = /(?:\/Users\/[^ "';,]+|\/home\/[^ "';,]+|\/root\/[^ "';,]*|[A-Za-z]:\\Users\\[^ "';,]+)/g; + +export function sanitizeUpstreamErrorText(value: string): string { + return redactSecretString(value).replace(ABSOLUTE_PATH_PATTERN, "[REDACTED_PATH]"); +} + +export function safeUpstreamErrorString(value: unknown): string | undefined { + return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined; +} + +export function parseUpstreamJsonPayload(payloadText: string): unknown | undefined { + const trimmed = payloadText.trim(); + if (!trimmed || (!trimmed.startsWith("{") && !trimmed.startsWith("["))) return undefined; + try { + return JSON.parse(trimmed) as unknown; + } catch { + return undefined; + } +} + +export async function readDisplaySafeErrorPayloadText(res: Response, signal?: AbortSignal): Promise { + try { + const body = await readBoundedResponseBody(res, { signal }); + return body.displaySafe ? body.text : ""; + } catch (error) { + if (signal?.aborted) throw error; + return ""; + } +} + +export async function normalizeUpstreamHttpErrorResponse( + res: Response, + opts: { signal?: AbortSignal; formatMessage: (payloadText: string) => string | Promise }, +): Promise { + if (res.ok) return res; + const payloadText = await readDisplaySafeErrorPayloadText(res, opts.signal); + const headers = new Headers(res.headers); + headers.delete("content-encoding"); + headers.delete("content-length"); + return new Response(await opts.formatMessage(payloadText), { + status: res.status, + statusText: res.statusText, + headers, + }); +} diff --git a/src/lib/upstream-retry.ts b/src/lib/upstream-retry.ts index ae92d8ffd..5aa26bb51 100644 --- a/src/lib/upstream-retry.ts +++ b/src/lib/upstream-retry.ts @@ -14,12 +14,19 @@ * MUST stay a leaf module: imports nothing from server.ts or adapters (kiro-retry imports * the shared abort helpers from here). */ +import { clearableDeadline } from "./abort"; // 1 initial + 2 retries: the pool may hold more than one stale socket. const RESET_RETRY_MAX_ATTEMPTS = 3; const RESET_RETRY_BASE_DELAY_MS = 150; const RESET_RETRY_MAX_DELAY_MS = 1_000; +export interface RetryBackoffOptions { + baseDelayMs: number; + maxDelayMs: number; + headers?: Headers; +} + export function abortError(signal?: AbortSignal): unknown { return signal?.reason ?? new DOMException("The operation was aborted", "AbortError"); } @@ -56,11 +63,51 @@ export function isConnectionResetError(err: unknown): boolean { || msg.includes("connection reset by peer"); } -function retryDelayMs(attempt: number): number { - const exp = Math.min(RESET_RETRY_BASE_DELAY_MS * (2 ** attempt), RESET_RETRY_MAX_DELAY_MS); +function retryAfterDelayMs(headers: Headers): number | undefined { + const raw = headers.get("retry-after")?.trim(); + if (!raw) return undefined; + const seconds = Number(raw); + if (Number.isFinite(seconds)) return Math.max(0, seconds * 1000); + const dateMs = Date.parse(raw); + if (!Number.isFinite(dateMs)) return undefined; + return Math.max(0, dateMs - Date.now()); +} + +export function retryBackoffDelayMs(attempt: number, opts: RetryBackoffOptions): number { + const retryAfter = opts.headers ? retryAfterDelayMs(opts.headers) : undefined; + if (retryAfter !== undefined) return Math.min(retryAfter, opts.maxDelayMs); + const exp = Math.min(opts.baseDelayMs * (2 ** attempt), opts.maxDelayMs); return Math.floor(exp * (0.8 + Math.random() * 0.4)); } +export function cancelResponseBodyBestEffort(res: Response): void { + try { + const cancellation = res.body?.cancel(); + if (cancellation) void cancellation.catch(() => {}); + } catch { + // Cancellation is cleanup only; retries must not wait for or fail because of it. + } +} + +export async function fetchWithAttemptDeadline( + url: string, + init: RequestInit, + timeoutMs: number, + abortSignal?: AbortSignal, +): Promise { + const attemptTimeout = clearableDeadline(timeoutMs, abortSignal); + try { + return await fetch(url, { + ...init, + signal: attemptTimeout.signal, + }); + } finally { + // Only the header timer is cleared. The composed signal still contains the parent, so a + // caller abort after headers continue to cancel consumption of the returned response body. + attemptTimeout.clear(); + } +} + export interface ResetRetryOptions { abortSignal?: AbortSignal; /** Short host/path label for the retry warn log (no secrets/query strings). */ @@ -89,7 +136,10 @@ export async function fetchWithResetRetry( console.warn( `[upstream-retry] connection reset${opts.label ? ` (${opts.label})` : ""} — retrying (${attempt + 2}/${attempts})`, ); - await sleepWithAbort(retryDelayMs(attempt), opts.abortSignal); + await sleepWithAbort(retryBackoffDelayMs(attempt, { + baseDelayMs: RESET_RETRY_BASE_DELAY_MS, + maxDelayMs: RESET_RETRY_MAX_DELAY_MS, + }), opts.abortSignal); } } throw lastError ?? new Error("upstream fetch failed"); diff --git a/tests/upstream-http-error.test.ts b/tests/upstream-http-error.test.ts new file mode 100644 index 000000000..4a5f80244 --- /dev/null +++ b/tests/upstream-http-error.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, test } from "bun:test"; +import { + normalizeUpstreamHttpErrorResponse, + sanitizeUpstreamErrorText, +} from "../src/adapters/upstream-http-error"; + +describe("sanitizeUpstreamErrorText", () => { + test("redacts secrets and absolute paths", () => { + const text = sanitizeUpstreamErrorText( + "Authorization: Bearer secret-token at /Users/example/private.json and C:\\Users\\JK\\secret.txt", + ); + expect(text).not.toContain("secret-token"); + expect(text).not.toContain("/Users/example/private.json"); + expect(text).not.toContain("C:\\Users\\JK\\secret.txt"); + }); +}); + +describe("normalizeUpstreamHttpErrorResponse", () => { + test("strips encoded-length headers and keeps safe headers", async () => { + const res = new Response("provider-private-detail", { + status: 503, + statusText: "Service Unavailable", + headers: { + "content-encoding": "gzip", + "content-length": "999", + "retry-after": "0", + "x-provider-error": "kept", + }, + }); + + const normalized = await normalizeUpstreamHttpErrorResponse(res, { + formatMessage: payloadText => `formatted: ${payloadText}`, + }); + + expect(normalized.status).toBe(503); + expect(normalized.statusText).toBe("Service Unavailable"); + expect(normalized.headers.get("content-encoding")).toBeNull(); + expect(normalized.headers.get("content-length")).toBeNull(); + expect(normalized.headers.get("retry-after")).toBe("0"); + expect(normalized.headers.get("x-provider-error")).toBe("kept"); + expect(await normalized.text()).toBe("formatted: provider-private-detail"); + }); +}); diff --git a/tests/upstream-retry.test.ts b/tests/upstream-retry.test.ts index 8004ba9b2..b1f4d4b65 100644 --- a/tests/upstream-retry.test.ts +++ b/tests/upstream-retry.test.ts @@ -1,5 +1,9 @@ import { afterEach, describe, expect, spyOn, test } from "bun:test"; -import { fetchWithResetRetry, isConnectionResetError } from "../src/lib/upstream-retry"; +import { + fetchWithResetRetry, + isConnectionResetError, + retryBackoffDelayMs, +} from "../src/lib/upstream-retry"; function bunResetError(): Error { // Shape of Bun's fetch rejection on a stale pooled socket. @@ -136,3 +140,42 @@ describe("fetchWithResetRetry", () => { await expect(fetchWithResetRetry(doFetch, { abortSignal: ac.signal })).rejects.toThrow("socket connection was closed unexpectedly"); }); }); + +describe("retryBackoffDelayMs", () => { + test("honors Retry-After seconds before exponential jitter", () => { + const headers = new Headers({ "Retry-After": "3" }); + expect(retryBackoffDelayMs(0, { + baseDelayMs: 250, + maxDelayMs: 5_000, + headers, + })).toBe(3_000); + }); + + test("parses Retry-After HTTP dates and caps them", () => { + const nowSpy = spyOn(Date, "now").mockReturnValue(1_700_000_000_000); + try { + const headers = new Headers({ + "Retry-After": new Date(1_700_000_004_000).toUTCString(), + }); + expect(retryBackoffDelayMs(0, { + baseDelayMs: 250, + maxDelayMs: 2_000, + headers, + })).toBe(2_000); + } finally { + nowSpy.mockRestore(); + } + }); + + test("falls back to capped exponential jitter when Retry-After is absent", () => { + const randomSpy = spyOn(Math, "random").mockReturnValue(0); + try { + expect(retryBackoffDelayMs(2, { + baseDelayMs: 250, + maxDelayMs: 2_000, + })).toBe(800); + } finally { + randomSpy.mockRestore(); + } + }); +}); From e8794d22f0331ab42a9d15d0d04b1ee5422b87b4 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sun, 12 Jul 2026 00:27:11 +0200 Subject: [PATCH 4/7] refactor: satisfy GUI lint rules --- .github/workflows/ci.yml | 11 ++++++ gui/src/App.tsx | 10 ++++- gui/src/i18n/index.ts | 2 + gui/src/i18n/index.tsx | 78 ------------------------------------- gui/src/i18n/provider.tsx | 24 ++++++++++++ gui/src/i18n/shared.ts | 55 ++++++++++++++++++++++++++ gui/src/pages/ApiKeys.tsx | 7 +++- gui/src/pages/CodexAuth.tsx | 21 +++++++--- gui/src/pages/Dashboard.tsx | 25 ++++++++---- gui/src/pages/Debug.tsx | 20 ++++++++-- gui/src/pages/Models.tsx | 45 ++++++++++++--------- gui/src/pages/Providers.tsx | 69 +++++++++++++++++++------------- gui/src/pages/Subagents.tsx | 13 +++++-- gui/src/pages/Usage.tsx | 37 +++++++++--------- 14 files changed, 252 insertions(+), 165 deletions(-) create mode 100644 gui/src/i18n/index.ts delete mode 100644 gui/src/i18n/index.tsx create mode 100644 gui/src/i18n/provider.tsx create mode 100644 gui/src/i18n/shared.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 437fa19a6..46a9b768c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,6 +73,17 @@ jobs: - name: Check release helper syntax run: bun build scripts/release.ts --target=bun --outdir=.tmp/ci-release-script-check + - name: GUI lint + run: | + cd gui + bun install --frozen-lockfile + bun run lint + + - name: GUI build + run: | + cd gui + bun run build + - name: CLI help smoke run: bun run src/cli/index.ts help diff --git a/gui/src/App.tsx b/gui/src/App.tsx index ce9ae49be..dee7f6875 100644 --- a/gui/src/App.tsx +++ b/gui/src/App.tsx @@ -56,7 +56,6 @@ function readStoredTheme(): Theme { export default function App() { const [page, setPageState] = useState(readPageFromHash); - const setPage = (p: Page) => { location.hash = p; setPageState(p); }; const [theme, setTheme] = useState(readStoredTheme); const [runtimeVersion, setRuntimeVersion] = useState(null); const { locale, setLocale } = useI18n(); @@ -68,6 +67,13 @@ export default function App() { return () => window.removeEventListener("hashchange", onHash); }, []); + useEffect(() => { + const nextHash = `#${page}`; + if (window.location.hash !== nextHash) { + window.location.hash = page; + } + }, [page]); + useEffect(() => { const el = document.documentElement; if (theme === "system") { el.removeAttribute("data-theme"); localStorage.removeItem(THEME_KEY); } @@ -112,7 +118,7 @@ export default function App() {