From 190f8895d25c47d09ae969a6000d5ab9e51e3af1 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 13 Sep 2026 13:46:54 -0700 Subject: [PATCH] fix(auth): scope settings sidecar denylist to the settings dir Workspace files such as .vscode/settings.json.lock and ..tmp were denied by the generic settings.json/permissions.json sidecar legs. Restricting those legs to the settings directory keeps decoys usable while real settings-sidecars still deny. fix(config): keep proxy bare rows alongside OAuth entries The CL-5606 legacy bare codex/xai drop now compares the row baseURL against the OAuth endpoint first, so a proxy or mirror row is preserved. --- src/auth/credential-surface.test.ts | 44 +++++++++++++++++++++++ src/auth/credential-surface.ts | 4 +-- src/config/index.ts | 29 +++++++++++++-- src/config/oauth-catalog.test.ts | 55 +++++++++++++++++++++++++---- 4 files changed, 121 insertions(+), 11 deletions(-) create mode 100644 src/auth/credential-surface.test.ts diff --git a/src/auth/credential-surface.test.ts b/src/auth/credential-surface.test.ts new file mode 100644 index 000000000..e59b30e14 --- /dev/null +++ b/src/auth/credential-surface.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, test } from "bun:test"; +import { join } from "node:path"; +import { buildCredentialPatterns } from "./credential-surface.js"; + +// CL-7929: the .lock/.tmp sidecar legs matched any directory, so a +// workspace settings file (e.g. .vscode/settings.json.lock) denied as a +// credential. They are scoped to the settings dir like the .bak legs. +describe("credential sidecar scoping (CL-7929)", () => { + test("workspace-path decoys do not deny", () => { + const patterns = buildCredentialPatterns(); + const decoys = [ + join("project", ".vscode", "settings.json.lock"), + join("project", "settings.json.12345.tmp"), + join("project", "settings.json.12345.1.tmp"), + join("project", ".vscode", "permissions.json.lock"), + join("project", "permissions.json.999.1.tmp"), + ]; + for (const decoy of decoys) { + expect( + patterns.some((pattern) => pattern.test(decoy)), + `${decoy} must not match the credential denylist`, + ).toBe(false); + } + }); + + test("settings-dir sidecars still deny", () => { + const patterns = buildCredentialPatterns(); + const home = join("/tmp", "cl7929-never-created"); + const deny = [ + join(home, ".corbits", "settings.json.lock"), + join(home, ".corbits", "settings.json.12345.1.tmp"), + join(home, ".corbits", "permissions.json.lock"), + join(home, ".corbits", "permissions.json.12345.1.tmp"), + join(home, ".corbits", "codex-auth.json.lock"), + join(home, ".corbits", "xai-auth.json.12345.1.tmp"), + ]; + for (const path of deny) { + expect( + patterns.some((pattern) => pattern.test(path)), + `${path} must match the credential denylist`, + ).toBe(true); + } + }); +}); diff --git a/src/auth/credential-surface.ts b/src/auth/credential-surface.ts index b79fe2e25..b76c75b31 100644 --- a/src/auth/credential-surface.ts +++ b/src/auth/credential-surface.ts @@ -63,11 +63,11 @@ export function buildCredentialPatterns(): RegExp[] { patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}~$`)); patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}\\.swp$`)); patterns.push(new RegExp(`(^|\\/)${dir}\\/\\.${base}\\.swp$`)); - patterns.push(new RegExp(`(^|\\/)${base}\\.lock$`)); + patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}\\.lock$`)); // Writers emit a pid.counter middle segment (auth/store.ts, // mcp/auth-store.ts), so the middle segment is required; a bare // `.tmp` has no known writer and stays unmatched. - patterns.push(new RegExp(`(^|\\/)${base}\\.[^/]*\\.tmp$`)); + patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}\\.[^/]*\\.tmp$`)); } return patterns; } diff --git a/src/config/index.ts b/src/config/index.ts index 4ae9a2e66..00765f766 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -1154,6 +1154,21 @@ export async function loadConfig( // (there is no logout/disconnect surface or auth-store watcher; refresh // runs on connect, prefetch, and startup), so removal takes effect on the // next rebuild, not live. +// Compare a settings-row baseURL against the OAuth endpoint so a +// proxy/mirror row is never mistaken for the legacy bare-row duplicate. +// Normalization failures fall back to a trailing-slash-insensitive compare +// rather than dropping a row whose URL cannot be parsed. +function sameEndpoint(raw: string | undefined, oauthBaseURL: string): boolean { + if (raw === undefined) return false; + const normalized = (value: string): string => { + try { + return normalizeOpenAICompatibleBaseURL(value); + } catch { + return value.trim().replace(/\/+$/, ""); + } + }; + return normalized(raw) === normalized(oauthBaseURL); +} export function mergeOAuthCatalog( settings: Settings | null, resolved: ResolvedProvider, @@ -1166,10 +1181,18 @@ export function mergeOAuthCatalog( // connect key) reads as a second, separately-added provider next to the // credential-backed `/` entries. Drop it once that family // has a live profile; when nothing is connected the bare row is the only - // ChatGPT/Grok access and stays. + // ChatGPT/Grok access and stays. A bare row pointed at a different + // endpoint (proxy/mirror) is a distinct provider, not the legacy + // duplicate, so it stays alongside the credential-backed entries. const dropBare = new Set([ - ...(codexEntries.length > 0 ? ["codex"] : []), - ...(xaiEntries.length > 0 ? ["xai"] : []), + ...(codexEntries.length > 0 && + sameEndpoint(settings?.providers["codex"]?.baseURL, CODEX_BASE_URL) + ? ["codex"] + : []), + ...(xaiEntries.length > 0 && + sameEndpoint(settings?.providers["xai"]?.baseURL, XAI_BASE_URL) + ? ["xai"] + : []), ]); return [ ...buildProviderCatalog(settings, resolved).filter( diff --git a/src/config/oauth-catalog.test.ts b/src/config/oauth-catalog.test.ts index 7efd74bc5..fb0356931 100644 --- a/src/config/oauth-catalog.test.ts +++ b/src/config/oauth-catalog.test.ts @@ -1,6 +1,8 @@ import { describe, expect, test } from "bun:test"; import type { CodexProfile } from "../auth/codex/store.js"; +import { CODEX_BASE_URL } from "../auth/codex/constants.js"; import type { XaiProfile } from "../auth/xai/store.js"; +import { XAI_BASE_URL } from "../auth/xai/constants.js"; import { mergeOAuthCatalog } from "./index.js"; import type { ProviderSettings, @@ -11,6 +13,10 @@ import type { // Regression tests for CL-5606: after a successful ChatGPT browser login the // merged catalog must not list a separately-added legacy bare `codex` row // alongside the credential-backed `codex/` entry. +// +// CL-7929: the drop only applies when the bare row points at the OAuth +// endpoint. A bare row pointed at a proxy/mirror is a distinct provider and +// stays alongside the credential-backed entries. const resolved: ResolvedProvider = { providerName: "openai", @@ -23,11 +29,16 @@ function settingsWith(providers: Record): Settings { return { providers } as Settings; } -const entry = (): ProviderSettings => ({ - baseURL: "https://chatgpt.com/backend-api/codex/responses", +const codexEntry = (): ProviderSettings => ({ + baseURL: CODEX_BASE_URL, models: ["gpt-5.1-codex-max"], }); +const xaiEntry = (): ProviderSettings => ({ + baseURL: XAI_BASE_URL, + models: ["grok-4-1"], +}); + const codexDefault: CodexProfile = { name: "default", tokens: { access: "codex-access", refresh: "r", expiresAt: 1 }, @@ -42,7 +53,7 @@ const xaiWork: XaiProfile = { describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => { test("a legacy bare codex row is dropped once codex/default is connected", () => { const merged = mergeOAuthCatalog( - settingsWith({ codex: entry(), "codex/default": entry() }), + settingsWith({ codex: codexEntry(), "codex/default": codexEntry() }), resolved, [codexDefault], [], @@ -52,7 +63,7 @@ describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => { test("a legacy bare xai row is dropped once xai/work is connected", () => { const merged = mergeOAuthCatalog( - settingsWith({ xai: entry() }), + settingsWith({ xai: xaiEntry() }), resolved, [], [xaiWork], @@ -64,7 +75,7 @@ describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => { // Not connected: no auth-store profile and no qualified entry. The legacy // single-instance row is the only ChatGPT access — keep it. const merged = mergeOAuthCatalog( - settingsWith({ codex: entry() }), + settingsWith({ codex: codexEntry() }), resolved, [], [], @@ -80,7 +91,7 @@ describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => { apiKey: "sk-test", models: ["gpt-5"], }, - codex: entry(), + codex: codexEntry(), }), resolved, [codexDefault], @@ -88,4 +99,36 @@ describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => { ); expect(merged.map((p) => p.name)).toEqual(["openai", "codex/default"]); }); + + test("a bare codex row pointed at a proxy survives alongside codex/default (CL-7929)", () => { + const merged = mergeOAuthCatalog( + settingsWith({ + codex: { + baseURL: "https://proxy.example.com/v1", + apiKey: "sk-proxy", + models: ["gpt-5.1-codex-max"], + }, + }), + resolved, + [codexDefault], + [], + ); + expect(merged.map((p) => p.name)).toEqual(["codex", "codex/default"]); + }); + + test("a bare xai row pointed at a mirror survives alongside xai/work (CL-7929)", () => { + const merged = mergeOAuthCatalog( + settingsWith({ + xai: { + baseURL: "https://mirror.example.com/v1", + apiKey: "sk-mirror", + models: ["grok-4-1"], + }, + }), + resolved, + [], + [xaiWork], + ); + expect(merged.map((p) => p.name)).toEqual(["xai", "xai/work"]); + }); });