From 749eb4efeadc9cb381dcfe99c54c47c39a709422 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 8 Mar 2026 16:51:43 +0000 Subject: [PATCH] refactor: thread config runtime env through models config --- ...els-config.applies-config-env-vars.test.ts | 48 ++++++++++++++----- src/agents/models-config.providers.ts | 15 +++--- src/agents/models-config.ts | 15 +++--- src/config/config.env-vars.test.ts | 16 ++++++- src/config/env-vars.ts | 9 ++++ 5 files changed, 76 insertions(+), 27 deletions(-) diff --git a/src/agents/models-config.applies-config-env-vars.test.ts b/src/agents/models-config.applies-config-env-vars.test.ts index 617e153f4b9..fa4adb86168 100644 --- a/src/agents/models-config.applies-config-env-vars.test.ts +++ b/src/agents/models-config.applies-config-env-vars.test.ts @@ -1,7 +1,7 @@ +import fs from "node:fs/promises"; import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; import { - CUSTOM_PROXY_MODELS_CONFIG, installModelsConfigTestHooks, unsetEnv, withModelsTempHome as withTempHome, @@ -14,33 +14,55 @@ installModelsConfigTestHooks(); const TEST_ENV_VAR = "OPENCLAW_MODELS_CONFIG_TEST_ENV"; describe("models-config", () => { - it("applies config env.vars entries while ensuring models.json", async () => { + it("uses config env.vars entries for implicit provider discovery without mutating process.env", async () => { await withTempHome(async () => { - await withTempEnv([TEST_ENV_VAR], async () => { - unsetEnv([TEST_ENV_VAR]); + await withTempEnv(["OPENROUTER_API_KEY", TEST_ENV_VAR], async () => { + unsetEnv(["OPENROUTER_API_KEY", TEST_ENV_VAR]); const cfg: OpenClawConfig = { - ...CUSTOM_PROXY_MODELS_CONFIG, - env: { vars: { [TEST_ENV_VAR]: "from-config" } }, + models: { providers: {} }, + env: { + vars: { + OPENROUTER_API_KEY: "from-config", + [TEST_ENV_VAR]: "from-config", + }, + }, }; - await ensureOpenClawModelsJson(cfg); + const { agentDir } = await ensureOpenClawModelsJson(cfg); - expect(process.env[TEST_ENV_VAR]).toBe("from-config"); + expect(process.env.OPENROUTER_API_KEY).toBeUndefined(); + expect(process.env[TEST_ENV_VAR]).toBeUndefined(); + + const modelsJson = JSON.parse(await fs.readFile(`${agentDir}/models.json`, "utf8")) as { + providers?: { openrouter?: { apiKey?: string } }; + }; + expect(modelsJson.providers?.openrouter?.apiKey).toBe("OPENROUTER_API_KEY"); }); }); }); - it("does not overwrite already-set host env vars", async () => { + it("does not overwrite already-set host env vars while ensuring models.json", async () => { await withTempHome(async () => { - await withTempEnv([TEST_ENV_VAR], async () => { + await withTempEnv(["OPENROUTER_API_KEY", TEST_ENV_VAR], async () => { + process.env.OPENROUTER_API_KEY = "from-host"; process.env[TEST_ENV_VAR] = "from-host"; const cfg: OpenClawConfig = { - ...CUSTOM_PROXY_MODELS_CONFIG, - env: { vars: { [TEST_ENV_VAR]: "from-config" } }, + models: { providers: {} }, + env: { + vars: { + OPENROUTER_API_KEY: "from-config", + [TEST_ENV_VAR]: "from-config", + }, + }, }; - await ensureOpenClawModelsJson(cfg); + const { agentDir } = await ensureOpenClawModelsJson(cfg); + const modelsJson = JSON.parse(await fs.readFile(`${agentDir}/models.json`, "utf8")) as { + providers?: { openrouter?: { apiKey?: string } }; + }; + expect(modelsJson.providers?.openrouter?.apiKey).toBe("OPENROUTER_API_KEY"); + expect(process.env.OPENROUTER_API_KEY).toBe("from-host"); expect(process.env[TEST_ENV_VAR]).toBe("from-host"); }); }); diff --git a/src/agents/models-config.providers.ts b/src/agents/models-config.providers.ts index 56dd6b5d948..8aca2c26d0e 100644 --- a/src/agents/models-config.providers.ts +++ b/src/agents/models-config.providers.ts @@ -415,8 +415,8 @@ function resolveEnvApiKeyVarName( return match ? match[1] : undefined; } -function resolveAwsSdkApiKeyVarName(): string { - return resolveAwsSdkEnvVarName() ?? "AWS_PROFILE"; +function resolveAwsSdkApiKeyVarName(env: NodeJS.ProcessEnv = process.env): string { + return resolveAwsSdkEnvVarName(env) ?? "AWS_PROFILE"; } function normalizeHeaderValues(params: { @@ -603,6 +603,7 @@ function normalizeAntigravityProvider(provider: ProviderConfig): ProviderConfig export function normalizeProviders(params: { providers: ModelsConfig["providers"]; agentDir: string; + env?: NodeJS.ProcessEnv; secretDefaults?: { env?: string; file?: string; @@ -614,6 +615,7 @@ export function normalizeProviders(params: { if (!providers) { return providers; } + const env = params.env ?? process.env; const authStore = ensureAuthProfileStore(params.agentDir, { allowKeychainPrompt: false, }); @@ -646,6 +648,7 @@ export function normalizeProviders(params: { const profileApiKey = resolveApiKeyFromProfiles({ provider: normalizedKey, store: authStore, + env, }); if (configuredApiKeyRef && configuredApiKeyRef.id.trim()) { @@ -687,8 +690,8 @@ export function normalizeProviders(params: { currentApiKey.trim() && !ENV_VAR_NAME_RE.test(currentApiKey.trim()) ) { - const envVarName = resolveEnvApiKeyVarName(normalizedKey); - if (envVarName && process.env[envVarName] === currentApiKey) { + const envVarName = resolveEnvApiKeyVarName(normalizedKey, env); + if (envVarName && env[envVarName] === currentApiKey) { mutated = true; normalizedProvider = { ...normalizedProvider, apiKey: envVarName }; } @@ -704,11 +707,11 @@ export function normalizeProviders(params: { const authMode = normalizedProvider.auth ?? (normalizedKey === "amazon-bedrock" ? "aws-sdk" : undefined); if (authMode === "aws-sdk") { - const apiKey = resolveAwsSdkApiKeyVarName(); + const apiKey = resolveAwsSdkApiKeyVarName(env); mutated = true; normalizedProvider = { ...normalizedProvider, apiKey }; } else { - const fromEnv = resolveEnvApiKeyVarName(normalizedKey); + const fromEnv = resolveEnvApiKeyVarName(normalizedKey, env); const apiKey = fromEnv ?? profileApiKey?.apiKey; if (apiKey?.trim()) { if (profileApiKey && profileApiKey.source !== "plaintext") { diff --git a/src/agents/models-config.ts b/src/agents/models-config.ts index 9a470ee5cdc..8fa237fcaf3 100644 --- a/src/agents/models-config.ts +++ b/src/agents/models-config.ts @@ -6,7 +6,7 @@ import { type OpenClawConfig, loadConfig, } from "../config/config.js"; -import { applyConfigEnvVars } from "../config/env-vars.js"; +import { createConfigRuntimeEnv } from "../config/env-vars.js"; import { isRecord } from "../utils.js"; import { resolveOpenClawAgentDir } from "./agent-paths.js"; import { @@ -46,12 +46,14 @@ async function readExistingModelsFile(pathname: string): Promise<{ async function resolveProvidersForModelsJson(params: { cfg: OpenClawConfig; agentDir: string; + env: NodeJS.ProcessEnv; }): Promise> { - const { cfg, agentDir } = params; + const { cfg, agentDir, env } = params; const explicitProviders = cfg.models?.providers ?? {}; const implicitProviders = await resolveImplicitProviders({ agentDir, config: cfg, + env, explicitProviders, }); const providers: Record = mergeProviders({ @@ -143,12 +145,10 @@ export async function ensureOpenClawModelsJson( return await withModelsJsonWriteLock(targetPath, async () => { // Ensure config env vars (e.g. AWS_PROFILE, AWS_ACCESS_KEY_ID) are - // available in process.env before implicit provider discovery. Some - // callers (agent runner, tools) pass config objects that haven't gone - // through the full loadConfig() pipeline which applies these. - applyConfigEnvVars(cfg); + // are available to provider discovery without mutating process.env. + const env = createConfigRuntimeEnv(cfg); - const providers = await resolveProvidersForModelsJson({ cfg, agentDir }); + const providers = await resolveProvidersForModelsJson({ cfg, agentDir, env }); if (Object.keys(providers).length === 0) { return { agentDir, wrote: false }; @@ -170,6 +170,7 @@ export async function ensureOpenClawModelsJson( normalizeProviders({ providers, agentDir, + env, secretDefaults: cfg.secrets?.defaults, secretRefManagedProviders, }) ?? providers; diff --git a/src/config/config.env-vars.test.ts b/src/config/config.env-vars.test.ts index d2927387948..389edc6d11d 100644 --- a/src/config/config.env-vars.test.ts +++ b/src/config/config.env-vars.test.ts @@ -3,7 +3,11 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; import { loadDotEnv } from "../infra/dotenv.js"; import { resolveConfigEnvVars } from "./env-substitution.js"; -import { applyConfigEnvVars, collectConfigRuntimeEnvVars } from "./env-vars.js"; +import { + applyConfigEnvVars, + collectConfigRuntimeEnvVars, + createConfigRuntimeEnv, +} from "./env-vars.js"; import { withEnvOverride, withTempHome } from "./test-helpers.js"; import type { OpenClawConfig } from "./types.js"; @@ -29,6 +33,16 @@ describe("config env vars", () => { }); }); + it("can build a merged runtime env without mutating process.env", async () => { + await withEnvOverride({ OPENROUTER_API_KEY: undefined }, async () => { + const merged = createConfigRuntimeEnv({ + env: { vars: { OPENROUTER_API_KEY: "config-key" } }, + } as OpenClawConfig); + expect(merged.OPENROUTER_API_KEY).toBe("config-key"); + expect(process.env.OPENROUTER_API_KEY).toBeUndefined(); + }); + }); + it("blocks dangerous startup env vars from config env", async () => { await withEnvOverride( { diff --git a/src/config/env-vars.ts b/src/config/env-vars.ts index edfa44db302..8692e163e22 100644 --- a/src/config/env-vars.ts +++ b/src/config/env-vars.ts @@ -67,6 +67,15 @@ export function collectConfigEnvVars(cfg?: OpenClawConfig): Record