mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix: normalize provider keys during model config merge (#95722)
* fix: normalize provider keys during model config merge * fix: normalize source-managed provider keys when reapplying secret markers Make enforceSourceManagedProviderSecrets canonicalize provider keys with normalizeProviderId so mixed-case (e.g. "OpenAI") source config still matches the canonical "openai" generated provider. Without this, the trim-only source lookup misses, the source SecretRef marker is never reapplied, and resolved runtime secret values leak into generated models.json. Adds plan-level regressions in models-config.runtime-source-snapshot.test.ts covering mixed-case apiKey and header source configs. * fix: use Array#toSorted in mixed-case provider regression tests oxlint(no-array-sort) flagged the new test assertions for using .sort() on Object.keys(...). Switch to .toSorted() to keep the check-lint shard green. * style(agents): oxfmt models-config provider merge files * fix(agents): define provider key collision precedence --------- Co-authored-by: Peter Steinberger <steipete@gmail.com> Co-authored-by: Peter Steinberger <peter@steipete.me>
This commit is contained in:
co-authored by
Peter Steinberger
Peter Steinberger
parent
8dd204e4fc
commit
60040d86b2
@@ -159,6 +159,101 @@ describe("models-config merge helpers", () => {
|
||||
expect(merged.custom?.api).toBe("openai-responses");
|
||||
});
|
||||
|
||||
it("merges explicit providers onto case-normalized implicit provider ids", () => {
|
||||
const merged = mergeProviders({
|
||||
implicit: {
|
||||
openai: {
|
||||
api: "openai-responses",
|
||||
models: [
|
||||
createModel({
|
||||
id: "gpt-5.4",
|
||||
name: "GPT-5.4",
|
||||
reasoning: true,
|
||||
}),
|
||||
],
|
||||
} as ProviderConfig,
|
||||
},
|
||||
explicit: {
|
||||
" OpenAI ": {
|
||||
apiKey: configApiKey,
|
||||
models: [
|
||||
createModel({
|
||||
id: "gpt-5.4",
|
||||
name: "GPT-5.4",
|
||||
reasoning: false,
|
||||
}),
|
||||
],
|
||||
} as ProviderConfig,
|
||||
},
|
||||
});
|
||||
|
||||
expect(Object.keys(merged)).toEqual(["openai"]);
|
||||
expect(merged.openai?.apiKey).toBe(configApiKey);
|
||||
expect(merged.openai?.api).toBe("openai-responses");
|
||||
expect(merged.OpenAI).toBeUndefined();
|
||||
});
|
||||
|
||||
it("normalizes implicit provider ids before merging explicit providers", () => {
|
||||
const merged = mergeProviders({
|
||||
implicit: {
|
||||
" OpenAI ": {
|
||||
api: "openai-responses",
|
||||
models: [
|
||||
createModel({
|
||||
id: "gpt-5.4",
|
||||
name: "GPT-5.4",
|
||||
reasoning: true,
|
||||
}),
|
||||
],
|
||||
} as ProviderConfig,
|
||||
},
|
||||
explicit: {
|
||||
openai: {
|
||||
apiKey: configApiKey,
|
||||
models: [
|
||||
createModel({
|
||||
id: "gpt-5.4",
|
||||
name: "GPT-5.4",
|
||||
reasoning: false,
|
||||
}),
|
||||
],
|
||||
} as ProviderConfig,
|
||||
},
|
||||
});
|
||||
|
||||
expect(Object.keys(merged)).toEqual(["openai"]);
|
||||
expect(merged.openai?.apiKey).toBe(configApiKey);
|
||||
expect(merged.OpenAI).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["before", true],
|
||||
["after", false],
|
||||
])("prefers canonical provider keys when they appear %s case variants", (_position, first) => {
|
||||
const canonical = createConfigProvider({ baseUrl: "https://canonical.example/v1" });
|
||||
const caseVariant = createConfigProvider({ baseUrl: "https://variant.example/v1" });
|
||||
const explicit: Record<string, ProviderConfig> = first
|
||||
? { openai: canonical, OpenAI: caseVariant }
|
||||
: { OpenAI: caseVariant, openai: canonical };
|
||||
|
||||
const merged = mergeProviders({ explicit });
|
||||
|
||||
expect(Object.keys(merged)).toEqual(["openai"]);
|
||||
expect(merged.openai?.baseUrl).toBe("https://canonical.example/v1");
|
||||
});
|
||||
|
||||
it("keeps the later provider when no collision key uses canonical spelling", () => {
|
||||
const merged = mergeProviders({
|
||||
explicit: {
|
||||
OpenAI: createConfigProvider({ baseUrl: "https://first.example/v1" }),
|
||||
" OPENAI ": createConfigProvider({ baseUrl: "https://second.example/v1" }),
|
||||
},
|
||||
});
|
||||
|
||||
expect(Object.keys(merged)).toEqual(["openai"]);
|
||||
expect(merged.openai?.baseUrl).toBe("https://second.example/v1");
|
||||
});
|
||||
|
||||
it("keeps existing providers alongside newly configured providers in merge mode", () => {
|
||||
const merged = mergeWithExistingProviderSecrets({
|
||||
nextProviders: {
|
||||
@@ -249,6 +344,48 @@ describe("models-config merge helpers", () => {
|
||||
expect(merged.custom?.baseUrl).toBe("https://agent.example/v1");
|
||||
});
|
||||
|
||||
it("preserves existing secrets after provider key normalization", () => {
|
||||
const normalized = mergeProviders({
|
||||
explicit: {
|
||||
openai: createConfigProvider(),
|
||||
},
|
||||
});
|
||||
const merged = mergeWithExistingProviderSecrets({
|
||||
nextProviders: normalized,
|
||||
existingProviders: {
|
||||
" OpenAI ": createExistingProvider(),
|
||||
},
|
||||
secretRefManagedProviders: new Set<string>(),
|
||||
});
|
||||
|
||||
expect(Object.keys(merged)).toEqual(["openai"]);
|
||||
expect(merged.openai?.apiKey).toBe(preservedApiKey);
|
||||
expect(merged.openai?.baseUrl).toBe("https://agent.example/v1");
|
||||
expect(merged.OpenAI).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["before", true],
|
||||
["after", false],
|
||||
])(
|
||||
"prefers canonical existing providers when they appear %s case variants",
|
||||
(_position, first) => {
|
||||
const canonical = createExistingProvider({ baseUrl: "https://canonical.example/v1" });
|
||||
const caseVariant = createExistingProvider({ baseUrl: "https://variant.example/v1" });
|
||||
const existingProviders: Record<string, ExistingProviderConfig> = first
|
||||
? { openai: canonical, OpenAI: caseVariant }
|
||||
: { OpenAI: caseVariant, openai: canonical };
|
||||
const merged = mergeWithExistingProviderSecrets({
|
||||
nextProviders: { openai: createConfigProvider() },
|
||||
existingProviders,
|
||||
secretRefManagedProviders: new Set<string>(),
|
||||
});
|
||||
|
||||
expect(Object.keys(merged)).toEqual(["openai"]);
|
||||
expect(merged.openai?.baseUrl).toBe("https://canonical.example/v1");
|
||||
},
|
||||
);
|
||||
|
||||
it("preserves implicit provider headers when explicit config adds extra headers", () => {
|
||||
const merged = mergeProviderModels(
|
||||
{
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
*/
|
||||
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
|
||||
import { isNonSecretApiKeyMarker } from "./model-auth-markers.js";
|
||||
import { normalizeProviderMapKeys } from "./models-config.providers.keys.js";
|
||||
import type { ProviderConfig } from "./models-config.providers.secrets.js";
|
||||
|
||||
/** Existing provider config shape that may carry persisted secret/base URL fields. */
|
||||
@@ -146,12 +147,8 @@ export function mergeProviders(params: {
|
||||
implicit?: Record<string, ProviderConfig> | null;
|
||||
explicit?: Record<string, ProviderConfig> | null;
|
||||
}): Record<string, ProviderConfig> {
|
||||
const out: Record<string, ProviderConfig> = params.implicit ? { ...params.implicit } : {};
|
||||
for (const [key, explicit] of Object.entries(params.explicit ?? {})) {
|
||||
const providerKey = normalizeOptionalString(key) ?? "";
|
||||
if (!providerKey) {
|
||||
continue;
|
||||
}
|
||||
const out = normalizeProviderMapKeys(params.implicit);
|
||||
for (const [providerKey, explicit] of Object.entries(normalizeProviderMapKeys(params.explicit))) {
|
||||
const implicit = out[providerKey];
|
||||
out[providerKey] = implicit ? mergeProviderModels(implicit, explicit) : explicit;
|
||||
}
|
||||
@@ -234,15 +231,18 @@ export function mergeWithExistingProviderSecrets(params: {
|
||||
secretRefManagedProviders: ReadonlySet<string>;
|
||||
}): Record<string, ProviderConfig> {
|
||||
const { nextProviders, existingProviders, secretRefManagedProviders } = params;
|
||||
const normalizedExistingProviders = normalizeProviderMapKeys(existingProviders);
|
||||
const normalizedNextProviders = normalizeProviderMapKeys(nextProviders);
|
||||
|
||||
const mergedProviders: Record<string, ProviderConfig> = {};
|
||||
for (const [key, entry] of Object.entries(existingProviders)) {
|
||||
for (const [key, entry] of Object.entries(normalizedExistingProviders)) {
|
||||
if (!isExistingProviderSelfContained(entry)) {
|
||||
continue;
|
||||
}
|
||||
mergedProviders[key] = entry;
|
||||
}
|
||||
for (const [key, newEntry] of Object.entries(nextProviders)) {
|
||||
const existing = existingProviders[key];
|
||||
for (const [key, newEntry] of Object.entries(normalizedNextProviders)) {
|
||||
const existing = normalizedExistingProviders[key];
|
||||
if (!existing) {
|
||||
mergedProviders[key] = newEntry;
|
||||
continue;
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
/** Canonical provider-key handling shared by models.json merge boundaries. */
|
||||
import { normalizeProviderId } from "@openclaw/model-catalog-core/provider-id";
|
||||
|
||||
export function normalizeProviderMapKeys<T>(
|
||||
providers: Record<string, T> | null | undefined,
|
||||
): Record<string, T> {
|
||||
const entries = Object.entries(providers ?? {});
|
||||
const canonicalKeys = new Set<string>();
|
||||
for (const [key] of entries) {
|
||||
const providerKey = normalizeProviderId(key);
|
||||
if (providerKey && key === providerKey) {
|
||||
canonicalKeys.add(providerKey);
|
||||
}
|
||||
}
|
||||
|
||||
const normalized: Record<string, T> = {};
|
||||
for (const [key, value] of entries) {
|
||||
const providerKey = normalizeProviderId(key);
|
||||
if (!providerKey || (canonicalKeys.has(providerKey) && key !== providerKey)) {
|
||||
continue;
|
||||
}
|
||||
// Exact canonical spelling wins over aliases regardless of object order.
|
||||
// Without one, the later variant wins, matching existing trim-collision behavior.
|
||||
normalized[providerKey] = value;
|
||||
}
|
||||
return normalized;
|
||||
}
|
||||
@@ -1,3 +1,4 @@
|
||||
import { normalizeProviderId } from "@openclaw/model-catalog-core/provider-id";
|
||||
/**
|
||||
* Enforces source-managed provider secret ownership rules.
|
||||
*/
|
||||
@@ -9,6 +10,7 @@ import {
|
||||
resolveNonEnvSecretRefHeaderValueMarker,
|
||||
resolveEnvSecretRefHeaderValueMarker,
|
||||
} from "./model-auth-markers.js";
|
||||
import { normalizeProviderMapKeys } from "./models-config.providers.keys.js";
|
||||
import type { ProviderConfig, SecretDefaults } from "./models-config.providers.secrets.js";
|
||||
|
||||
/**
|
||||
@@ -25,15 +27,12 @@ function normalizeSourceProviderLookup(
|
||||
if (!providers) {
|
||||
return {};
|
||||
}
|
||||
const out: Record<string, ProviderConfig> = {};
|
||||
for (const [key, provider] of Object.entries(providers)) {
|
||||
const normalizedKey = key.trim();
|
||||
if (!normalizedKey || !isRecord(provider)) {
|
||||
continue;
|
||||
}
|
||||
out[normalizedKey] = provider;
|
||||
}
|
||||
return out;
|
||||
const validProviders = Object.fromEntries(
|
||||
Object.entries(providers).filter(([, provider]) => isRecord(provider)),
|
||||
) as Record<string, ProviderConfig>;
|
||||
// Use the merge boundary's collision rule so a case alias cannot displace the
|
||||
// canonical SecretRef owner and expose its resolved runtime value to models.json.
|
||||
return normalizeProviderMapKeys(validProviders);
|
||||
}
|
||||
|
||||
function resolveSourceManagedApiKeyMarker(params: {
|
||||
@@ -100,7 +99,8 @@ export function enforceSourceManagedProviderSecrets(params: {
|
||||
if (!isRecord(provider)) {
|
||||
continue;
|
||||
}
|
||||
const sourceProvider = sourceProvidersByKey[providerKey.trim()];
|
||||
const canonicalProviderKey = normalizeProviderId(providerKey);
|
||||
const sourceProvider = sourceProvidersByKey[canonicalProviderKey];
|
||||
if (!sourceProvider) {
|
||||
continue;
|
||||
}
|
||||
@@ -112,7 +112,7 @@ export function enforceSourceManagedProviderSecrets(params: {
|
||||
sourceSecretDefaults: params.sourceSecretDefaults,
|
||||
});
|
||||
if (sourceApiKeyMarker) {
|
||||
params.secretRefManagedProviders?.add(providerKey.trim());
|
||||
params.secretRefManagedProviders?.add(canonicalProviderKey);
|
||||
if (nextProvider.apiKey !== sourceApiKeyMarker) {
|
||||
providerMutated = true;
|
||||
nextProvider = {
|
||||
|
||||
@@ -425,4 +425,107 @@ describe("models-config runtime source snapshot", () => {
|
||||
expect(providers.openai?.apiKey).toBe("OPENAI_API_KEY"); // pragma: allowlist secret
|
||||
expectOpenAiHeaderMarkers(providers);
|
||||
});
|
||||
|
||||
it("reapplies source markers when sourceConfigForSecrets uses mixed-case provider keys", async () => {
|
||||
// Regression: provider keys in sourceConfigForSecrets may arrive as "OpenAI" while the
|
||||
// merge boundary canonicalizes to "openai". The source-managed marker lookup must use the
|
||||
// same provider-id normalizer, otherwise the resolved runtime apiKey leaks into models.json.
|
||||
const mixedCaseSourceConfig: OpenClawConfig = {
|
||||
models: {
|
||||
providers: {
|
||||
OpenAI: {
|
||||
baseUrl: "https://api.openai.com/v1",
|
||||
apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, // pragma: allowlist secret
|
||||
api: "openai-completions" as const,
|
||||
models: [],
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
const providers = await planGeneratedProviders({
|
||||
config: createOpenAiApiKeyRuntimeConfig(),
|
||||
sourceConfigForSecrets: mixedCaseSourceConfig,
|
||||
});
|
||||
expect(Object.keys(providers).toSorted()).toEqual(["openai"]);
|
||||
expect(providers.OpenAI).toBeUndefined();
|
||||
expect(providers.openai?.apiKey).toBe("OPENAI_API_KEY"); // pragma: allowlist secret
|
||||
});
|
||||
|
||||
it("reapplies source header markers when sourceConfigForSecrets uses mixed-case provider keys", async () => {
|
||||
const sourceConfig: OpenClawConfig = {
|
||||
models: {
|
||||
providers: {
|
||||
" OpenAI ": {
|
||||
baseUrl: "https://api.openai.com/v1",
|
||||
api: "openai-completions" as const,
|
||||
apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, // pragma: allowlist secret
|
||||
headers: {
|
||||
Authorization: {
|
||||
source: "env",
|
||||
provider: "default",
|
||||
id: "OPENAI_HEADER_TOKEN", // pragma: allowlist secret
|
||||
},
|
||||
"X-Tenant-Token": {
|
||||
source: "file",
|
||||
provider: "vault",
|
||||
id: "/providers/openai/tenantToken",
|
||||
},
|
||||
},
|
||||
models: [],
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
const providers = await planGeneratedProviders({
|
||||
config: createOpenAiRuntimeConfigWithHeadersAndApiKey(),
|
||||
sourceConfigForSecrets: sourceConfig,
|
||||
});
|
||||
expect(Object.keys(providers).toSorted()).toEqual(["openai"]);
|
||||
expect(providers.OpenAI).toBeUndefined();
|
||||
expect(providers.openai?.apiKey).toBe("OPENAI_API_KEY"); // pragma: allowlist secret
|
||||
expectOpenAiHeaderMarkers(providers);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["before", true],
|
||||
["after", false],
|
||||
])(
|
||||
"prefers canonical source secret ownership when it appears %s a case variant",
|
||||
async (_position, first) => {
|
||||
const canonical = getOpenAiProvider(createOpenAiApiKeySourceConfig());
|
||||
const caseVariant = {
|
||||
...canonical,
|
||||
apiKey: {
|
||||
source: "env" as const,
|
||||
provider: "default",
|
||||
id: "OPENAI_CASE_VARIANT",
|
||||
},
|
||||
};
|
||||
const sourceProviders = first
|
||||
? { openai: canonical, OpenAI: caseVariant }
|
||||
: { OpenAI: caseVariant, openai: canonical };
|
||||
const providers = await planGeneratedProviders({
|
||||
config: createOpenAiApiKeyRuntimeConfig(),
|
||||
sourceConfigForSecrets: { models: { providers: sourceProviders } },
|
||||
});
|
||||
|
||||
expect(Object.keys(providers)).toEqual(["openai"]);
|
||||
expect(providers.openai?.apiKey).toBe("OPENAI_API_KEY"); // pragma: allowlist secret
|
||||
},
|
||||
);
|
||||
|
||||
it("uses a valid case alias when the canonical source entry is not a provider record", () => {
|
||||
const runtimeConfig = createOpenAiApiKeyRuntimeConfig();
|
||||
const sourceProviders = {
|
||||
openai: null,
|
||||
OpenAI: getOpenAiProvider(createOpenAiApiKeySourceConfig()),
|
||||
} as unknown as NonNullable<NonNullable<OpenClawConfig["models"]>["providers"]>;
|
||||
|
||||
const providers = enforceSourceManagedProviderSecrets({
|
||||
providers: runtimeConfig.models!.providers!,
|
||||
sourceProviders,
|
||||
});
|
||||
|
||||
expect(providers?.openai?.apiKey).toBe("OPENAI_API_KEY"); // pragma: allowlist secret
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user