Skip to content

Commit 8db44c7

Browse files
TheGreatAxiosTheGreatAxios
authored
Merge pull request #1301 from corbitsdev/fix/provider-removal-provenance
fix(providers): use catalog provenance for OAuth removal
2 parents 6c70351 + 71bdd19 commit 8db44c7

4 files changed

Lines changed: 124 additions & 94 deletions

File tree

‎src/auth/remove-provider.test.ts‎

Lines changed: 46 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { mkdtemp, rm } from "node:fs/promises";
22
import { tmpdir } from "node:os";
33
import { join } from "node:path";
4-
import { afterEach, describe, expect, test } from "bun:test";
4+
import { describe, expect, test } from "bun:test";
55

66
import {
77
listCodexProfiles,
@@ -13,21 +13,16 @@ import {
1313
loadXaiProfile,
1414
saveXaiProfile,
1515
} from "../config/oauth-stores.js";
16-
import {
17-
clearSourceCredentials,
18-
registerSourceCredentialRecord,
19-
} from "../config/source-credentials.js";
2016
import { oauthStoreForProvider } from "./remove-provider.js";
2117

22-
function registerOAuthProvider(
18+
function oauthProvider(
2319
providerName: string,
2420
provider: "codex" | "xai",
2521
profile: string,
26-
): void {
27-
registerSourceCredentialRecord(providerName, {
28-
provenance: { kind: "oauth", provider, profile },
29-
material: { secret: "token" },
30-
});
22+
): { name: string; codexProfile?: string; xaiProfile?: string } {
23+
return provider === "codex"
24+
? { name: providerName, codexProfile: profile }
25+
: { name: providerName, xaiProfile: profile };
3126
}
3227

3328
async function withHome<T>(fn: (home: string) => Promise<T>): Promise<T> {
@@ -40,50 +35,47 @@ async function withHome<T>(fn: (home: string) => Promise<T>): Promise<T> {
4035
}
4136

4237
describe("oauthStoreForProvider", () => {
43-
afterEach(() => {
44-
clearSourceCredentials();
45-
});
46-
47-
test("maps xai/ and codex/ provider names to their store and profile", () => {
48-
clearSourceCredentials();
49-
registerOAuthProvider("xai/work", "xai", "work");
50-
registerOAuthProvider("codex/personal", "codex", "personal");
51-
expect(oauthStoreForProvider("xai/work")?.profile).toBe("work");
52-
expect(oauthStoreForProvider("codex/personal")?.profile).toBe("personal");
38+
test("maps marked xai/ and codex/ catalog entries to their stores", () => {
39+
expect(
40+
oauthStoreForProvider(oauthProvider("xai/work", "xai", "work"))?.profile,
41+
).toBe("work");
42+
expect(
43+
oauthStoreForProvider(
44+
oauthProvider("codex/personal", "codex", "personal"),
45+
)?.profile,
46+
).toBe("personal");
5347
});
5448

55-
test("rejects namespaced custom providers without OAuth ownership", () => {
56-
clearSourceCredentials();
57-
registerSourceCredentialRecord("codex/manual", {
58-
provenance: { kind: "api-key" },
59-
material: { secret: "manual-key" },
60-
});
61-
registerSourceCredentialRecord("xai/manual", {
62-
provenance: { kind: "api-key" },
63-
material: { secret: "manual-key" },
64-
});
65-
66-
expect(oauthStoreForProvider("codex/manual")).toBeNull();
67-
expect(oauthStoreForProvider("xai/manual")).toBeNull();
49+
test("rejects unmarked and mismatched namespaced catalog entries", () => {
50+
expect(oauthStoreForProvider({ name: "codex/manual" })).toBeNull();
51+
expect(oauthStoreForProvider({ name: "xai/manual" })).toBeNull();
52+
expect(
53+
oauthStoreForProvider({ name: "xai/work", xaiProfile: "personal" }),
54+
).toBeNull();
55+
expect(
56+
oauthStoreForProvider({ name: "codex/work", codexProfile: "personal" }),
57+
).toBeNull();
6858
});
6959

7060
test("returns null for API-key, keyless, and degenerate names", () => {
71-
expect(oauthStoreForProvider("openai")).toBeNull();
72-
expect(oauthStoreForProvider("ollama")).toBeNull();
73-
expect(oauthStoreForProvider("my-custom")).toBeNull();
74-
expect(oauthStoreForProvider("xai/")).toBeNull();
75-
expect(oauthStoreForProvider("codex/")).toBeNull();
61+
expect(oauthStoreForProvider({ name: "openai" })).toBeNull();
62+
expect(oauthStoreForProvider({ name: "ollama" })).toBeNull();
63+
expect(oauthStoreForProvider({ name: "my-custom" })).toBeNull();
64+
expect(oauthStoreForProvider({ name: "xai/", xaiProfile: "" })).toBeNull();
65+
expect(
66+
oauthStoreForProvider({ name: "codex/", codexProfile: "" }),
67+
).toBeNull();
7668
});
7769

7870
test("removing an xai profile leaves sibling profiles untouched", async () => {
7971
await withHome(async (home) => {
80-
clearSourceCredentials();
81-
registerOAuthProvider("xai/work", "xai", "work");
8272
const tokens = { access: "a", refresh: "r", expiresAt: 10_000_000 };
8373
await saveXaiProfile({ name: "work", createdAt: 0, tokens }, home);
8474
await saveXaiProfile({ name: "personal", createdAt: 0, tokens }, home);
8575

86-
const target = oauthStoreForProvider("xai/work");
76+
const target = oauthStoreForProvider(
77+
oauthProvider("xai/work", "xai", "work"),
78+
);
8779
expect(target?.profile).toBe("work");
8880
expect(await target?.removeProfile(target.profile, home)).toEqual([
8981
"work",
@@ -97,13 +89,13 @@ describe("oauthStoreForProvider", () => {
9789

9890
test("removing a codex profile leaves sibling profiles untouched", async () => {
9991
await withHome(async (home) => {
100-
clearSourceCredentials();
101-
registerOAuthProvider("codex/night", "codex", "night");
10292
const tokens = { access: "a", refresh: "r", expiresAt: 10_000_000 };
10393
await saveCodexProfile({ name: "day", createdAt: 0, tokens }, home);
10494
await saveCodexProfile({ name: "night", createdAt: 0, tokens }, home);
10595

106-
const target = oauthStoreForProvider("codex/night");
96+
const target = oauthStoreForProvider(
97+
oauthProvider("codex/night", "codex", "night"),
98+
);
10799
expect(await target?.removeProfile(target.profile, home)).toEqual([
108100
"night",
109101
]);
@@ -116,22 +108,23 @@ describe("oauthStoreForProvider", () => {
116108

117109
test("removing an already-absent profile is a no-op", async () => {
118110
await withHome(async (home) => {
119-
clearSourceCredentials();
120-
registerOAuthProvider("xai/ghost", "xai", "ghost");
121-
const target = oauthStoreForProvider("xai/ghost");
111+
const target = oauthStoreForProvider(
112+
oauthProvider("xai/ghost", "xai", "ghost"),
113+
);
122114
expect(await target?.removeProfile(target.profile, home)).toEqual([]);
123115
});
124116
});
125117

126118
test("missing auth files are tolerated", async () => {
127119
await withHome(async (home) => {
128-
clearSourceCredentials();
129-
registerOAuthProvider("xai/work", "xai", "work");
130-
registerOAuthProvider("codex/work", "codex", "work");
131120
// No profiles ever saved: the store treats a missing file as empty.
132-
const xai = oauthStoreForProvider("xai/work");
121+
const xai = oauthStoreForProvider(
122+
oauthProvider("xai/work", "xai", "work"),
123+
);
133124
expect(await xai?.removeProfile(xai.profile, home)).toEqual([]);
134-
const codex = oauthStoreForProvider("codex/work");
125+
const codex = oauthStoreForProvider(
126+
oauthProvider("codex/work", "codex", "work"),
127+
);
135128
expect(await codex?.removeProfile(codex.profile, home)).toEqual([]);
136129
});
137130
});

‎src/auth/remove-provider.ts‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
removeCodexProfile,
44
removeXaiProfile,
55
} from "../config/oauth-stores.js";
6-
import { findSourceCredentialRecord } from "../config/source-credentials.js";
6+
import type { ProviderCatalogEntry } from "../config/index.js";
77
import { xaiProfileFromProviderName } from "../config/xai-providers.js";
88

99
/**
@@ -21,26 +21,29 @@ export interface OAuthStoreTarget {
2121
) => Promise<string[]>;
2222
}
2323

24-
// Maps a catalog provider to its OAuth auth store only when the live source
25-
// credential record proves ownership of the same provider family and profile.
26-
// Names alone are user-controlled and cannot authorize credential deletion.
24+
// Maps a catalog provider to its OAuth auth store only when the current catalog
25+
// entry carries the matching auth-store profile marker. Names alone are
26+
// user-controlled and cannot authorize credential deletion.
2727
export function oauthStoreForProvider(
28-
providerName: string,
28+
provider: Pick<
29+
ProviderCatalogEntry,
30+
"name" | "codexProfile" | "xaiProfile"
31+
> | null,
2932
): OAuthStoreTarget | null {
30-
const provenance = findSourceCredentialRecord(providerName)?.provenance;
31-
if (provenance?.kind !== "oauth") return null;
33+
if (provider === null) return null;
34+
const providerName = provider.name;
3235
const codexProfile = codexProfileFromProviderName(providerName);
3336
if (
34-
provenance.provider === "codex" &&
35-
codexProfile === provenance.profile &&
37+
codexProfile !== undefined &&
38+
codexProfile === provider.codexProfile &&
3639
codexProfile.length > 0
3740
) {
3841
return { profile: codexProfile, removeProfile: removeCodexProfile };
3942
}
4043
const xaiProfile = xaiProfileFromProviderName(providerName);
4144
if (
42-
provenance.provider === "xai" &&
43-
xaiProfile === provenance.profile &&
45+
xaiProfile !== undefined &&
46+
xaiProfile === provider.xaiProfile &&
4447
xaiProfile.length > 0
4548
) {
4649
return { profile: xaiProfile, removeProfile: removeXaiProfile };

‎src/tui/provider-remove.test.ts‎

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,9 @@ import {
1717
loadXaiProfile,
1818
saveXaiProfile,
1919
} from "../config/oauth-stores.js";
20-
import {
21-
clearSourceCredentials,
22-
registerSourceCredentialRecord,
23-
} from "../config/source-credentials.js";
20+
import { clearSourceCredentials } from "../config/source-credentials.js";
21+
import { refreshLiveProviderCatalog } from "../config/index.js";
22+
import { buildMainSessionSources } from "../config/inference-sources.js";
2423
import { createGlobalSettingsWriter } from "../mcp/add-server.js";
2524
import { withMockedHomedir } from "../../testkit/mock-module.js";
2625
import { modelOptionId } from "./model-catalog.js";
@@ -149,6 +148,22 @@ async function executeRemove(
149148
});
150149
}
151150

151+
async function refreshCatalog(harness: RemoveHarness): Promise<void> {
152+
await withMockedHomedir(harness.home, async () => {
153+
const config = harness.state.config;
154+
config.providers = await refreshLiveProviderCatalog(
155+
config.settings ?? null,
156+
{
157+
apiKey: config.apiKey,
158+
baseURL: config.baseURL,
159+
model: config.model,
160+
providerName: config.providerName,
161+
...(config.keyless !== undefined ? { keyless: config.keyless } : {}),
162+
},
163+
);
164+
});
165+
}
166+
152167
const PROFILE_TOKENS = {
153168
access: "a",
154169
refresh: "r",
@@ -247,7 +262,7 @@ describe("provider removal execute path", () => {
247262
}
248263
});
249264

250-
test("removes an OAuth provider's auth profile and leaves siblings intact", async () => {
265+
test("removes an inactive OAuth provider without source registration", async () => {
251266
const seed: Settings = {
252267
...SEED_WATERMARKS,
253268
defaultProvider: "keeper",
@@ -271,10 +286,6 @@ describe("provider removal execute path", () => {
271286
});
272287
try {
273288
clearSourceCredentials();
274-
registerSourceCredentialRecord("xai/work", {
275-
provenance: { kind: "oauth", provider: "xai", profile: "work" },
276-
material: { secret: "token" },
277-
});
278289
await saveXaiProfile(
279290
{ name: "work", createdAt: 0, tokens: PROFILE_TOKENS },
280291
harness.home,
@@ -283,6 +294,7 @@ describe("provider removal execute path", () => {
283294
{ name: "personal", createdAt: 0, tokens: PROFILE_TOKENS },
284295
harness.home,
285296
);
297+
await refreshCatalog(harness);
286298
await executeRemove(harness, modelOptionId("xai/work", "grok-4"));
287299
const onDisk = await loadSettings(harness.settingsPath);
288300
expect(onDisk?.providers["xai/work"]).toBeUndefined();
@@ -334,10 +346,6 @@ describe("provider removal execute path", () => {
334346
});
335347
try {
336348
clearSourceCredentials();
337-
registerSourceCredentialRecord("xai/work", {
338-
provenance: { kind: "oauth", provider: "xai", profile: "work" },
339-
material: { secret: "token" },
340-
});
341349
await saveXaiProfile(
342350
{ name: "work", createdAt: 0, tokens: PROFILE_TOKENS },
343351
harness.home,
@@ -346,6 +354,7 @@ describe("provider removal execute path", () => {
346354
{ name: "personal", createdAt: 0, tokens: PROFILE_TOKENS },
347355
harness.home,
348356
);
357+
await refreshCatalog(harness);
349358
expect(
350359
harness.wiring.describeRemoveProvider(
351360
modelOptionId("xai/work", "grok-4"),
@@ -370,31 +379,26 @@ describe("provider removal execute path", () => {
370379
}
371380
});
372381

373-
test("custom namespaced providers leave unrelated OAuth profiles intact", async () => {
382+
test("custom namespaced providers ignore stale OAuth source provenance", async () => {
374383
for (const provider of ["xai/manual", "codex/manual"] as const) {
375-
const seed: Settings = {
384+
const oauthSeed: Settings = {
376385
...SEED_WATERMARKS,
377386
providers: {
378387
keeper: keeperEntry(),
379388
[provider]: {
380-
baseURL: "https://manual.example/v1",
381-
apiKey: "manual-key",
389+
baseURL: "https://oauth-placeholder.invalid/v1",
382390
models: ["manual-model"],
383391
defaultModel: "manual-model",
384392
},
385393
},
386394
};
387395
const harness = await wireRemoveHarness({
388-
seed,
396+
seed: oauthSeed,
389397
local: null,
390398
liveProvider: "keeper",
391399
});
392400
try {
393401
clearSourceCredentials();
394-
registerSourceCredentialRecord(provider, {
395-
provenance: { kind: "api-key" },
396-
material: { secret: "manual-key" },
397-
});
398402
if (provider.startsWith("xai/")) {
399403
await saveXaiProfile(
400404
{ name: "manual", createdAt: 0, tokens: PROFILE_TOKENS },
@@ -406,6 +410,30 @@ describe("provider removal execute path", () => {
406410
harness.home,
407411
);
408412
}
413+
await refreshCatalog(harness);
414+
buildMainSessionSources({
415+
settings: harness.state.config.settings,
416+
catalog: harness.state.config.providers,
417+
activeProvider: provider,
418+
activeModel: "manual-model",
419+
sessionId: "stale-provenance",
420+
});
421+
422+
const customSettings: Settings = {
423+
...SEED_WATERMARKS,
424+
providers: {
425+
keeper: keeperEntry(),
426+
[provider]: {
427+
baseURL: "https://manual.example/v1",
428+
apiKey: "manual-key",
429+
models: ["manual-model"],
430+
defaultModel: "manual-model",
431+
},
432+
},
433+
};
434+
await saveGlobalSettings(harness.settingsPath, customSettings);
435+
harness.state.config.settings = customSettings;
436+
await refreshCatalog(harness);
409437
await executeRemove(harness, modelOptionId(provider, "manual-model"));
410438
const profile = provider.startsWith("xai/")
411439
? await loadXaiProfile("manual", harness.home)
@@ -546,10 +574,11 @@ describe("describeRemoveProvider", () => {
546574
const harness = await wireLines(seed);
547575
try {
548576
clearSourceCredentials();
549-
registerSourceCredentialRecord("xai/work", {
550-
provenance: { kind: "oauth", provider: "xai", profile: "work" },
551-
material: { secret: "token" },
552-
});
577+
await saveXaiProfile(
578+
{ name: "work", createdAt: 0, tokens: PROFILE_TOKENS },
579+
harness.home,
580+
);
581+
await refreshCatalog(harness);
553582
const keyLine = harness.wiring.describeRemoveProvider(
554583
modelOptionId("openai/work", "m1"),
555584
);

0 commit comments

Comments
 (0)