Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 46 additions & 53 deletions src/auth/remove-provider.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, test } from "bun:test";
import { describe, expect, test } from "bun:test";

import {
listCodexProfiles,
Expand All @@ -13,21 +13,16 @@ import {
loadXaiProfile,
saveXaiProfile,
} from "../config/oauth-stores.js";
import {
clearSourceCredentials,
registerSourceCredentialRecord,
} from "../config/source-credentials.js";
import { oauthStoreForProvider } from "./remove-provider.js";

function registerOAuthProvider(
function oauthProvider(
providerName: string,
provider: "codex" | "xai",
profile: string,
): void {
registerSourceCredentialRecord(providerName, {
provenance: { kind: "oauth", provider, profile },
material: { secret: "token" },
});
): { name: string; codexProfile?: string; xaiProfile?: string } {
return provider === "codex"
? { name: providerName, codexProfile: profile }
: { name: providerName, xaiProfile: profile };
}

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

describe("oauthStoreForProvider", () => {
afterEach(() => {
clearSourceCredentials();
});

test("maps xai/ and codex/ provider names to their store and profile", () => {
clearSourceCredentials();
registerOAuthProvider("xai/work", "xai", "work");
registerOAuthProvider("codex/personal", "codex", "personal");
expect(oauthStoreForProvider("xai/work")?.profile).toBe("work");
expect(oauthStoreForProvider("codex/personal")?.profile).toBe("personal");
test("maps marked xai/ and codex/ catalog entries to their stores", () => {
expect(
oauthStoreForProvider(oauthProvider("xai/work", "xai", "work"))?.profile,
).toBe("work");
expect(
oauthStoreForProvider(
oauthProvider("codex/personal", "codex", "personal"),
)?.profile,
).toBe("personal");
});

test("rejects namespaced custom providers without OAuth ownership", () => {
clearSourceCredentials();
registerSourceCredentialRecord("codex/manual", {
provenance: { kind: "api-key" },
material: { secret: "manual-key" },
});
registerSourceCredentialRecord("xai/manual", {
provenance: { kind: "api-key" },
material: { secret: "manual-key" },
});

expect(oauthStoreForProvider("codex/manual")).toBeNull();
expect(oauthStoreForProvider("xai/manual")).toBeNull();
test("rejects unmarked and mismatched namespaced catalog entries", () => {
expect(oauthStoreForProvider({ name: "codex/manual" })).toBeNull();
expect(oauthStoreForProvider({ name: "xai/manual" })).toBeNull();
expect(
oauthStoreForProvider({ name: "xai/work", xaiProfile: "personal" }),
).toBeNull();
expect(
oauthStoreForProvider({ name: "codex/work", codexProfile: "personal" }),
).toBeNull();
});

test("returns null for API-key, keyless, and degenerate names", () => {
expect(oauthStoreForProvider("openai")).toBeNull();
expect(oauthStoreForProvider("ollama")).toBeNull();
expect(oauthStoreForProvider("my-custom")).toBeNull();
expect(oauthStoreForProvider("xai/")).toBeNull();
expect(oauthStoreForProvider("codex/")).toBeNull();
expect(oauthStoreForProvider({ name: "openai" })).toBeNull();
expect(oauthStoreForProvider({ name: "ollama" })).toBeNull();
expect(oauthStoreForProvider({ name: "my-custom" })).toBeNull();
expect(oauthStoreForProvider({ name: "xai/", xaiProfile: "" })).toBeNull();
expect(
oauthStoreForProvider({ name: "codex/", codexProfile: "" }),
).toBeNull();
});

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

const target = oauthStoreForProvider("xai/work");
const target = oauthStoreForProvider(
oauthProvider("xai/work", "xai", "work"),
);
expect(target?.profile).toBe("work");
expect(await target?.removeProfile(target.profile, home)).toEqual([
"work",
Expand All @@ -97,13 +89,13 @@ describe("oauthStoreForProvider", () => {

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

const target = oauthStoreForProvider("codex/night");
const target = oauthStoreForProvider(
oauthProvider("codex/night", "codex", "night"),
);
expect(await target?.removeProfile(target.profile, home)).toEqual([
"night",
]);
Expand All @@ -116,22 +108,23 @@ describe("oauthStoreForProvider", () => {

test("removing an already-absent profile is a no-op", async () => {
await withHome(async (home) => {
clearSourceCredentials();
registerOAuthProvider("xai/ghost", "xai", "ghost");
const target = oauthStoreForProvider("xai/ghost");
const target = oauthStoreForProvider(
oauthProvider("xai/ghost", "xai", "ghost"),
);
expect(await target?.removeProfile(target.profile, home)).toEqual([]);
});
});

test("missing auth files are tolerated", async () => {
await withHome(async (home) => {
clearSourceCredentials();
registerOAuthProvider("xai/work", "xai", "work");
registerOAuthProvider("codex/work", "codex", "work");
// No profiles ever saved: the store treats a missing file as empty.
const xai = oauthStoreForProvider("xai/work");
const xai = oauthStoreForProvider(
oauthProvider("xai/work", "xai", "work"),
);
expect(await xai?.removeProfile(xai.profile, home)).toEqual([]);
const codex = oauthStoreForProvider("codex/work");
const codex = oauthStoreForProvider(
oauthProvider("codex/work", "codex", "work"),
);
expect(await codex?.removeProfile(codex.profile, home)).toEqual([]);
});
});
Expand Down
25 changes: 14 additions & 11 deletions src/auth/remove-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
removeCodexProfile,
removeXaiProfile,
} from "../config/oauth-stores.js";
import { findSourceCredentialRecord } from "../config/source-credentials.js";
import type { ProviderCatalogEntry } from "../config/index.js";
import { xaiProfileFromProviderName } from "../config/xai-providers.js";

/**
Expand All @@ -21,26 +21,29 @@ export interface OAuthStoreTarget {
) => Promise<string[]>;
}

// Maps a catalog provider to its OAuth auth store only when the live source
// credential record proves ownership of the same provider family and profile.
// Names alone are user-controlled and cannot authorize credential deletion.
// Maps a catalog provider to its OAuth auth store only when the current catalog
// entry carries the matching auth-store profile marker. Names alone are
// user-controlled and cannot authorize credential deletion.
export function oauthStoreForProvider(
providerName: string,
provider: Pick<
ProviderCatalogEntry,
"name" | "codexProfile" | "xaiProfile"
> | null,
): OAuthStoreTarget | null {
const provenance = findSourceCredentialRecord(providerName)?.provenance;
if (provenance?.kind !== "oauth") return null;
if (provider === null) return null;
const providerName = provider.name;
const codexProfile = codexProfileFromProviderName(providerName);
if (
provenance.provider === "codex" &&
codexProfile === provenance.profile &&
codexProfile !== undefined &&
codexProfile === provider.codexProfile &&
codexProfile.length > 0
) {
return { profile: codexProfile, removeProfile: removeCodexProfile };
}
const xaiProfile = xaiProfileFromProviderName(providerName);
if (
provenance.provider === "xai" &&
xaiProfile === provenance.profile &&
xaiProfile !== undefined &&
xaiProfile === provider.xaiProfile &&
xaiProfile.length > 0
) {
return { profile: xaiProfile, removeProfile: removeXaiProfile };
Expand Down
81 changes: 55 additions & 26 deletions src/tui/provider-remove.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@ import {
loadXaiProfile,
saveXaiProfile,
} from "../config/oauth-stores.js";
import {
clearSourceCredentials,
registerSourceCredentialRecord,
} from "../config/source-credentials.js";
import { clearSourceCredentials } from "../config/source-credentials.js";
import { refreshLiveProviderCatalog } from "../config/index.js";
import { buildMainSessionSources } from "../config/inference-sources.js";
import { createGlobalSettingsWriter } from "../mcp/add-server.js";
import { withMockedHomedir } from "../../testkit/mock-module.js";
import { modelOptionId } from "./model-catalog.js";
Expand Down Expand Up @@ -149,6 +148,22 @@ async function executeRemove(
});
}

async function refreshCatalog(harness: RemoveHarness): Promise<void> {
await withMockedHomedir(harness.home, async () => {
const config = harness.state.config;
config.providers = await refreshLiveProviderCatalog(
config.settings ?? null,
{
apiKey: config.apiKey,
baseURL: config.baseURL,
model: config.model,
providerName: config.providerName,
...(config.keyless !== undefined ? { keyless: config.keyless } : {}),
},
);
});
}

const PROFILE_TOKENS = {
access: "a",
refresh: "r",
Expand Down Expand Up @@ -247,7 +262,7 @@ describe("provider removal execute path", () => {
}
});

test("removes an OAuth provider's auth profile and leaves siblings intact", async () => {
test("removes an inactive OAuth provider without source registration", async () => {
const seed: Settings = {
...SEED_WATERMARKS,
defaultProvider: "keeper",
Expand All @@ -271,10 +286,6 @@ describe("provider removal execute path", () => {
});
try {
clearSourceCredentials();
registerSourceCredentialRecord("xai/work", {
provenance: { kind: "oauth", provider: "xai", profile: "work" },
material: { secret: "token" },
});
await saveXaiProfile(
{ name: "work", createdAt: 0, tokens: PROFILE_TOKENS },
harness.home,
Expand All @@ -283,6 +294,7 @@ describe("provider removal execute path", () => {
{ name: "personal", createdAt: 0, tokens: PROFILE_TOKENS },
harness.home,
);
await refreshCatalog(harness);
await executeRemove(harness, modelOptionId("xai/work", "grok-4"));
const onDisk = await loadSettings(harness.settingsPath);
expect(onDisk?.providers["xai/work"]).toBeUndefined();
Expand Down Expand Up @@ -334,10 +346,6 @@ describe("provider removal execute path", () => {
});
try {
clearSourceCredentials();
registerSourceCredentialRecord("xai/work", {
provenance: { kind: "oauth", provider: "xai", profile: "work" },
material: { secret: "token" },
});
await saveXaiProfile(
{ name: "work", createdAt: 0, tokens: PROFILE_TOKENS },
harness.home,
Expand All @@ -346,6 +354,7 @@ describe("provider removal execute path", () => {
{ name: "personal", createdAt: 0, tokens: PROFILE_TOKENS },
harness.home,
);
await refreshCatalog(harness);
expect(
harness.wiring.describeRemoveProvider(
modelOptionId("xai/work", "grok-4"),
Expand All @@ -370,31 +379,26 @@ describe("provider removal execute path", () => {
}
});

test("custom namespaced providers leave unrelated OAuth profiles intact", async () => {
test("custom namespaced providers ignore stale OAuth source provenance", async () => {
for (const provider of ["xai/manual", "codex/manual"] as const) {
const seed: Settings = {
const oauthSeed: Settings = {
...SEED_WATERMARKS,
providers: {
keeper: keeperEntry(),
[provider]: {
baseURL: "https://manual.example/v1",
apiKey: "manual-key",
baseURL: "https://oauth-placeholder.invalid/v1",
models: ["manual-model"],
defaultModel: "manual-model",
},
},
};
const harness = await wireRemoveHarness({
seed,
seed: oauthSeed,
local: null,
liveProvider: "keeper",
});
try {
clearSourceCredentials();
registerSourceCredentialRecord(provider, {
provenance: { kind: "api-key" },
material: { secret: "manual-key" },
});
if (provider.startsWith("xai/")) {
await saveXaiProfile(
{ name: "manual", createdAt: 0, tokens: PROFILE_TOKENS },
Expand All @@ -406,6 +410,30 @@ describe("provider removal execute path", () => {
harness.home,
);
}
await refreshCatalog(harness);
buildMainSessionSources({
settings: harness.state.config.settings,
catalog: harness.state.config.providers,
activeProvider: provider,
activeModel: "manual-model",
sessionId: "stale-provenance",
});

const customSettings: Settings = {
...SEED_WATERMARKS,
providers: {
keeper: keeperEntry(),
[provider]: {
baseURL: "https://manual.example/v1",
apiKey: "manual-key",
models: ["manual-model"],
defaultModel: "manual-model",
},
},
};
await saveGlobalSettings(harness.settingsPath, customSettings);
harness.state.config.settings = customSettings;
await refreshCatalog(harness);
await executeRemove(harness, modelOptionId(provider, "manual-model"));
const profile = provider.startsWith("xai/")
? await loadXaiProfile("manual", harness.home)
Expand Down Expand Up @@ -546,10 +574,11 @@ describe("describeRemoveProvider", () => {
const harness = await wireLines(seed);
try {
clearSourceCredentials();
registerSourceCredentialRecord("xai/work", {
provenance: { kind: "oauth", provider: "xai", profile: "work" },
material: { secret: "token" },
});
await saveXaiProfile(
{ name: "work", createdAt: 0, tokens: PROFILE_TOKENS },
harness.home,
);
await refreshCatalog(harness);
const keyLine = harness.wiring.describeRemoveProvider(
modelOptionId("openai/work", "m1"),
);
Expand Down
Loading
Loading