diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index 626148fd0c..c70cf2edcc 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -105,6 +105,63 @@ type ProviderPatchApplication = headersTouched: boolean; }; +const PROVIDER_ALIAS_OVERLAY_FIELDS = ["alias", "modelAliases", "defaultAliases"] as const; +type ProviderAliasOverlayField = typeof PROVIDER_ALIAS_OVERLAY_FIELDS[number]; + +/** + * Alias overlays are owned by the dedicated alias management routes. A full provider POST + * may round-trip an already-persisted value, but it must not create, clear, or change one. + * PATCH is field-masked and rejects these keys outright below. + */ +function providerAliasOverlayOwnershipError( + submitted: Record, + existing: OcxProviderConfig | undefined, +): string | null { + for (const field of PROVIDER_ALIAS_OVERLAY_FIELDS) { + if (!Object.hasOwn(submitted, field)) continue; + if (!existing || !Object.hasOwn(existing, field)) { + return `${field} is managed by the dedicated alias API`; + } + const incoming = submitted[field]; + const persisted = existing[field]; + if (field === "modelAliases") { + if (!isPlainRecord(incoming) || !isPlainRecord(persisted)) { + return "modelAliases is managed by the dedicated alias API"; + } + const incomingEntries = Object.entries(incoming); + const persistedEntries = Object.entries(persisted); + if ( + incomingEntries.length !== persistedEntries.length + || incomingEntries.some(([model, alias]) => typeof alias !== "string" || persisted[model] !== alias) + ) { + return "modelAliases is managed by the dedicated alias API"; + } + continue; + } + if (incoming !== persisted) return `${field} is managed by the dedicated alias API`; + } + return null; +} + +/** Remove only alias overlays whose ownership has already been established by the caller. */ +function providerTransportValidationCandidate(provider: Record): Record { + const candidate = { ...provider }; + for (const field of PROVIDER_ALIAS_OVERLAY_FIELDS) delete candidate[field]; + return candidate; +} + +/** Preserve the authoritative alias values from the stored provider during a full edit. */ +function restorePersistedAliasOverlays(target: OcxProviderConfig, existing: OcxProviderConfig | undefined): void { + for (const field of PROVIDER_ALIAS_OVERLAY_FIELDS) { + delete (target as Record)[field]; + if (!existing || !Object.hasOwn(existing, field)) continue; + const value = existing[field]; + (target as Record)[field] = field === "modelAliases" + ? structuredClone(value) + : value; + } +} + /** * Apply the recognized PATCH field mask onto a provider copy. The caller runs this once * for validation and again inside the config mutation lock against the newest provider, @@ -501,7 +558,10 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise), + ) ?? providerEmptyToolOutputConfigError(name, provider); if (providerError) return jsonResponse({ error: "provider reload target invalid" }, 409); const namespaceCollision = codexAccountNamespaceProviderCollisionError( @@ -561,12 +621,17 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise Object.hasOwn(rawBody, field)); + if (aliasField) return jsonResponse({ error: `${aliasField} is managed by the dedicated alias API` }, 400); const hasMode = Object.hasOwn(rawBody, "codexAccountMode"); const hasSetDefault = Object.hasOwn(rawBody, "setDefault"); const canonicalBudgetOnly = name === "openai" @@ -746,7 +816,10 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise), + ) ?? providerEmptyToolOutputConfigError(name, next); if (providerError) return jsonResponse({ error: providerError }, 400); if (!canonicalBudgetOnly) { @@ -786,7 +859,10 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise), + ) ?? providerEmptyToolOutputConfigError(name, replay.next); if (syncError) { replayError = syncError; diff --git a/tests/management-provider-validation.test.ts b/tests/management-provider-validation.test.ts index ac976fc785..6149eeefb6 100644 --- a/tests/management-provider-validation.test.ts +++ b/tests/management-provider-validation.test.ts @@ -914,6 +914,271 @@ describe("provider management validation", () => { } }); + test("full provider edit preserves aliases owned by the dedicated APIs", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + const overlays = { + alias: "codex-native", + modelAliases: { "gpt-5.6-luna": "luna" }, + defaultAliases: false, + } as const; + saveConfig({ + port: 0, + openaiProviderTierVersion: 2, + defaultProvider: "openai", + providers: { openai: { ...canonicalDirect, ...overlays } }, + } as OcxConfig); + const resolvedError = spyOn(destinationPolicy, "providerDestinationResolvedError").mockResolvedValue(null); + const server = startServer(0); + try { + // The dashboard's full editor omits alias-owned fields. The stored values must survive. + const omitted = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "openai", provider: canonicalDirect }), + }); + expect(omitted.status).toBe(200); + expect(loadConfig().providers.openai).toMatchObject(overlays); + + // A full-object client may round-trip the exact stored values, but still does not own them. + const roundTrip = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "openai", provider: { ...canonicalDirect, ...overlays } }), + }); + expect(roundTrip.status).toBe(200); + expect(loadConfig().providers.openai).toMatchObject(overlays); + } finally { + resolvedError.mockRestore(); + await server.stop(true); + } + }); + + test("general provider writes cannot introduce a provider alias collision", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig({ + port: 0, + openaiProviderTierVersion: 2, + defaultProvider: "openai", + providers: { + openai: { ...canonicalDirect }, + deepseek: { adapter: "openai-chat", baseUrl: "https://api.deepseek.com/v1" }, + }, + } as OcxConfig); + const server = startServer(0); + try { + const before = readFileSync(join(TEST_DIR, "config.json")); + const post = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "openai", provider: { ...canonicalDirect, alias: "deepseek" } }), + }); + expect(post.status).toBe(400); + + const patch = await fetch(new URL("/api/providers?name=openai", server.url), { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ alias: "deepseek" }), + }); + expect(patch.status).toBe(400); + expect(readFileSync(join(TEST_DIR, "config.json"))).toEqual(before); + expect(loadConfig().providers.openai?.alias).toBeUndefined(); + } finally { + await server.stop(true); + } + }); + + test("general provider writes reject reserved duplicate and invalid model aliases", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig({ + port: 0, + openaiProviderTierVersion: 2, + defaultProvider: "openai", + providers: { openai: { ...canonicalDirect } }, + } as OcxConfig); + const server = startServer(0); + try { + const before = readFileSync(join(TEST_DIR, "config.json")); + for (const modelAliases of [ + { "gpt-5.6-luna": "gpt-5.6-sol" }, + { "gpt-5.6-sol": "same", "gpt-5.6-luna": "SAME" }, + { "gpt-5.6-luna": "not an alias" }, + ]) { + const response = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "openai", provider: { ...canonicalDirect, modelAliases } }), + }); + expect(response.status).toBe(400); + } + const patch = await fetch(new URL("/api/providers?name=openai", server.url), { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ modelAliases: { "gpt-5.6-luna": "luna" } }), + }); + expect(patch.status).toBe(400); + expect(readFileSync(join(TEST_DIR, "config.json"))).toEqual(before); + } finally { + await server.stop(true); + } + }); + + test("malformed alias overlays return bounded 4xx without config persistence", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig({ + port: 0, + openaiProviderTierVersion: 2, + defaultProvider: "openai", + providers: { openai: { ...canonicalDirect } }, + } as OcxConfig); + const server = startServer(0); + try { + const before = readFileSync(join(TEST_DIR, "config.json")); + for (const overlay of [ + { defaultAliases: "yes" }, + { modelAliases: null }, + { modelAliases: [] }, + { modelAliases: { "gpt-5.6-luna": 42 } }, + ]) { + const post = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "openai", provider: { ...canonicalDirect, ...overlay } }), + }); + expect(post.status).toBeGreaterThanOrEqual(400); + expect(post.status).toBeLessThan(500); + + const patch = await fetch(new URL("/api/providers?name=openai", server.url), { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify(overlay), + }); + expect(patch.status).toBeGreaterThanOrEqual(400); + expect(patch.status).toBeLessThan(500); + } + expect(readFileSync(join(TEST_DIR, "config.json"))).toEqual(before); + } finally { + await server.stop(true); + } + }); + + test("canonical transport tampering stays rejected with persisted alias overlays", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + const overlays = { + alias: "codex-native", + modelAliases: { "gpt-5.6-luna": "luna" }, + defaultAliases: true, + } as const; + saveConfig({ + port: 0, + openaiProviderTierVersion: 2, + defaultProvider: "openai", + providers: { openai: { ...canonicalDirect, ...overlays } }, + } as OcxConfig); + const server = startServer(0); + try { + for (const tampering of [ + { baseUrl: "https://attacker.example/backend-api/codex" }, + { adapter: "openai-chat" }, + { authMode: "key" }, + ]) { + const response = await fetch(new URL("/api/providers?name=openai", server.url), { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify(tampering), + }); + expect(response.status).toBe(400); + } + expect(loadConfig().providers.openai).toMatchObject({ ...canonicalDirect, ...overlays }); + } finally { + await server.stop(true); + } + }); + + test("unrelated non-openai provider edits preserve persisted alias overlays", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig({ + port: 0, + openaiProviderTierVersion: 2, + defaultProvider: "deepseek", + providers: { + deepseek: { + adapter: "openai-chat", + baseUrl: "https://api.deepseek.com/v1", + alias: "ds", + modelAliases: { "deepseek-v4": "ds4-custom" }, + defaultAliases: false, + }, + }, + } as OcxConfig); + const resolvedError = spyOn(destinationPolicy, "providerDestinationResolvedError").mockResolvedValue(null); + const server = startServer(0); + try { + const response = await fetch(new URL("/api/providers?name=deepseek", server.url), { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ contextWindow: 128000 }), + }); + expect(response.status).toBe(200); + expect(loadConfig().providers.deepseek).toMatchObject({ + alias: "ds", + modelAliases: { "deepseek-v4": "ds4-custom" }, + defaultAliases: false, + contextWindow: 128000, + }); + } finally { + resolvedError.mockRestore(); + await server.stop(true); + } + }); + + test("canonical OpenAI with defaultAliases can still PATCH modelContextWindows", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig({ + port: 0, + // Match a post-migration config (openaiProviderTierVersion set) so the startup + // openai tier migration does not rewrite the row: this test targets the seed + // comparison, not the one-time legacy migration. + openaiProviderTierVersion: 2, + defaultProvider: "openai", + providers: { + openai: { ...canonicalDirect, defaultAliases: true }, + }, + } as OcxConfig); + // This test targets the seed comparison, not the DNS policy; stub the destination + // probe so the assertion stays independent of how chatgpt.com resolves locally. + const resolvedError = spyOn(destinationPolicy, "providerDestinationResolvedError").mockResolvedValue(null); + + const server = startServer(0); + try { + const patch = await fetch(new URL("/api/providers?name=openai", server.url), { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ modelContextWindows: { "gpt-5.6-luna": 900000 } }), + }); + expect(patch.status).toBe(200); + expect(loadConfig().providers.openai?.modelContextWindows).toEqual({ "gpt-5.6-luna": 900000 }); + // The alias overlay itself must survive the patch untouched. + expect(loadConfig().providers.openai?.defaultAliases).toBe(true); + } finally { + resolvedError.mockRestore(); + await server.stop(true); + } + }); + // #1409: the add/edit form's payload type has no member for contextWindow or test("provider POST overwrite preserves an explicit annotateEmptyToolOutputs: false", async () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true });