Skip to content

Commit 07bade7

Browse files
fix(auth): scope settings sidecar denylist to the settings dir (#1002)
Workspace files such as .vscode/settings.json.lock and <base>.<pid>.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.
1 parent 43e7eef commit 07bade7

4 files changed

Lines changed: 121 additions & 11 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { join } from "node:path";
3+
import { buildCredentialPatterns } from "./credential-surface.js";
4+
5+
// CL-7929: the .lock/.tmp sidecar legs matched any directory, so a
6+
// workspace settings file (e.g. .vscode/settings.json.lock) denied as a
7+
// credential. They are scoped to the settings dir like the .bak legs.
8+
describe("credential sidecar scoping (CL-7929)", () => {
9+
test("workspace-path decoys do not deny", () => {
10+
const patterns = buildCredentialPatterns();
11+
const decoys = [
12+
join("project", ".vscode", "settings.json.lock"),
13+
join("project", "settings.json.12345.tmp"),
14+
join("project", "settings.json.12345.1.tmp"),
15+
join("project", ".vscode", "permissions.json.lock"),
16+
join("project", "permissions.json.999.1.tmp"),
17+
];
18+
for (const decoy of decoys) {
19+
expect(
20+
patterns.some((pattern) => pattern.test(decoy)),
21+
`${decoy} must not match the credential denylist`,
22+
).toBe(false);
23+
}
24+
});
25+
26+
test("settings-dir sidecars still deny", () => {
27+
const patterns = buildCredentialPatterns();
28+
const home = join("/tmp", "cl7929-never-created");
29+
const deny = [
30+
join(home, ".corbits", "settings.json.lock"),
31+
join(home, ".corbits", "settings.json.12345.1.tmp"),
32+
join(home, ".corbits", "permissions.json.lock"),
33+
join(home, ".corbits", "permissions.json.12345.1.tmp"),
34+
join(home, ".corbits", "codex-auth.json.lock"),
35+
join(home, ".corbits", "xai-auth.json.12345.1.tmp"),
36+
];
37+
for (const path of deny) {
38+
expect(
39+
patterns.some((pattern) => pattern.test(path)),
40+
`${path} must match the credential denylist`,
41+
).toBe(true);
42+
}
43+
});
44+
});

‎src/auth/credential-surface.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,11 @@ export function buildCredentialPatterns(): RegExp[] {
6363
patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}~$`));
6464
patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}\\.swp$`));
6565
patterns.push(new RegExp(`(^|\\/)${dir}\\/\\.${base}\\.swp$`));
66-
patterns.push(new RegExp(`(^|\\/)${base}\\.lock$`));
66+
patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}\\.lock$`));
6767
// Writers emit a pid.counter middle segment (auth/store.ts,
6868
// mcp/auth-store.ts), so the middle segment is required; a bare
6969
// `<base>.tmp` has no known writer and stays unmatched.
70-
patterns.push(new RegExp(`(^|\\/)${base}\\.[^/]*\\.tmp$`));
70+
patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}\\.[^/]*\\.tmp$`));
7171
}
7272
return patterns;
7373
}

‎src/config/index.ts‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,6 +1154,21 @@ export async function loadConfig(
11541154
// (there is no logout/disconnect surface or auth-store watcher; refresh
11551155
// runs on connect, prefetch, and startup), so removal takes effect on the
11561156
// next rebuild, not live.
1157+
// Compare a settings-row baseURL against the OAuth endpoint so a
1158+
// proxy/mirror row is never mistaken for the legacy bare-row duplicate.
1159+
// Normalization failures fall back to a trailing-slash-insensitive compare
1160+
// rather than dropping a row whose URL cannot be parsed.
1161+
function sameEndpoint(raw: string | undefined, oauthBaseURL: string): boolean {
1162+
if (raw === undefined) return false;
1163+
const normalized = (value: string): string => {
1164+
try {
1165+
return normalizeOpenAICompatibleBaseURL(value);
1166+
} catch {
1167+
return value.trim().replace(/\/+$/, "");
1168+
}
1169+
};
1170+
return normalized(raw) === normalized(oauthBaseURL);
1171+
}
11571172
export function mergeOAuthCatalog(
11581173
settings: Settings | null,
11591174
resolved: ResolvedProvider,
@@ -1166,10 +1181,18 @@ export function mergeOAuthCatalog(
11661181
// connect key) reads as a second, separately-added provider next to the
11671182
// credential-backed `<kind>/<profile>` entries. Drop it once that family
11681183
// has a live profile; when nothing is connected the bare row is the only
1169-
// ChatGPT/Grok access and stays.
1184+
// ChatGPT/Grok access and stays. A bare row pointed at a different
1185+
// endpoint (proxy/mirror) is a distinct provider, not the legacy
1186+
// duplicate, so it stays alongside the credential-backed entries.
11701187
const dropBare = new Set([
1171-
...(codexEntries.length > 0 ? ["codex"] : []),
1172-
...(xaiEntries.length > 0 ? ["xai"] : []),
1188+
...(codexEntries.length > 0 &&
1189+
sameEndpoint(settings?.providers["codex"]?.baseURL, CODEX_BASE_URL)
1190+
? ["codex"]
1191+
: []),
1192+
...(xaiEntries.length > 0 &&
1193+
sameEndpoint(settings?.providers["xai"]?.baseURL, XAI_BASE_URL)
1194+
? ["xai"]
1195+
: []),
11731196
]);
11741197
return [
11751198
...buildProviderCatalog(settings, resolved).filter(

‎src/config/oauth-catalog.test.ts‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { describe, expect, test } from "bun:test";
22
import type { CodexProfile } from "../auth/codex/store.js";
3+
import { CODEX_BASE_URL } from "../auth/codex/constants.js";
34
import type { XaiProfile } from "../auth/xai/store.js";
5+
import { XAI_BASE_URL } from "../auth/xai/constants.js";
46
import { mergeOAuthCatalog } from "./index.js";
57
import type {
68
ProviderSettings,
@@ -11,6 +13,10 @@ import type {
1113
// Regression tests for CL-5606: after a successful ChatGPT browser login the
1214
// merged catalog must not list a separately-added legacy bare `codex` row
1315
// alongside the credential-backed `codex/<profile>` entry.
16+
//
17+
// CL-7929: the drop only applies when the bare row points at the OAuth
18+
// endpoint. A bare row pointed at a proxy/mirror is a distinct provider and
19+
// stays alongside the credential-backed entries.
1420

1521
const resolved: ResolvedProvider = {
1622
providerName: "openai",
@@ -23,11 +29,16 @@ function settingsWith(providers: Record<string, ProviderSettings>): Settings {
2329
return { providers } as Settings;
2430
}
2531

26-
const entry = (): ProviderSettings => ({
27-
baseURL: "https://chatgpt.com/backend-api/codex/responses",
32+
const codexEntry = (): ProviderSettings => ({
33+
baseURL: CODEX_BASE_URL,
2834
models: ["gpt-5.1-codex-max"],
2935
});
3036

37+
const xaiEntry = (): ProviderSettings => ({
38+
baseURL: XAI_BASE_URL,
39+
models: ["grok-4-1"],
40+
});
41+
3142
const codexDefault: CodexProfile = {
3243
name: "default",
3344
tokens: { access: "codex-access", refresh: "r", expiresAt: 1 },
@@ -42,7 +53,7 @@ const xaiWork: XaiProfile = {
4253
describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => {
4354
test("a legacy bare codex row is dropped once codex/default is connected", () => {
4455
const merged = mergeOAuthCatalog(
45-
settingsWith({ codex: entry(), "codex/default": entry() }),
56+
settingsWith({ codex: codexEntry(), "codex/default": codexEntry() }),
4657
resolved,
4758
[codexDefault],
4859
[],
@@ -52,7 +63,7 @@ describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => {
5263

5364
test("a legacy bare xai row is dropped once xai/work is connected", () => {
5465
const merged = mergeOAuthCatalog(
55-
settingsWith({ xai: entry() }),
66+
settingsWith({ xai: xaiEntry() }),
5667
resolved,
5768
[],
5869
[xaiWork],
@@ -64,7 +75,7 @@ describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => {
6475
// Not connected: no auth-store profile and no qualified entry. The legacy
6576
// single-instance row is the only ChatGPT access — keep it.
6677
const merged = mergeOAuthCatalog(
67-
settingsWith({ codex: entry() }),
78+
settingsWith({ codex: codexEntry() }),
6879
resolved,
6980
[],
7081
[],
@@ -80,12 +91,44 @@ describe("mergeOAuthCatalog legacy bare-row dedupe (CL-5606)", () => {
8091
apiKey: "sk-test",
8192
models: ["gpt-5"],
8293
},
83-
codex: entry(),
94+
codex: codexEntry(),
8495
}),
8596
resolved,
8697
[codexDefault],
8798
[],
8899
);
89100
expect(merged.map((p) => p.name)).toEqual(["openai", "codex/default"]);
90101
});
102+
103+
test("a bare codex row pointed at a proxy survives alongside codex/default (CL-7929)", () => {
104+
const merged = mergeOAuthCatalog(
105+
settingsWith({
106+
codex: {
107+
baseURL: "https://proxy.example.com/v1",
108+
apiKey: "sk-proxy",
109+
models: ["gpt-5.1-codex-max"],
110+
},
111+
}),
112+
resolved,
113+
[codexDefault],
114+
[],
115+
);
116+
expect(merged.map((p) => p.name)).toEqual(["codex", "codex/default"]);
117+
});
118+
119+
test("a bare xai row pointed at a mirror survives alongside xai/work (CL-7929)", () => {
120+
const merged = mergeOAuthCatalog(
121+
settingsWith({
122+
xai: {
123+
baseURL: "https://mirror.example.com/v1",
124+
apiKey: "sk-mirror",
125+
models: ["grok-4-1"],
126+
},
127+
}),
128+
resolved,
129+
[],
130+
[xaiWork],
131+
);
132+
expect(merged.map((p) => p.name)).toEqual(["xai", "xai/work"]);
133+
});
91134
});

0 commit comments

Comments
 (0)