fix(models): prevent plaintext apiKey writes to models state (#38889)

Land #38889 by @gambletan.

Co-authored-by: gambletan <ethanchang32@gmail.com>
This commit is contained in:
Peter Steinberger
2026-03-07 19:29:46 +00:00
parent de2ccffec1
commit 17ab46aedd
4 changed files with 86 additions and 0 deletions

View File

@@ -258,6 +258,7 @@ Docs: https://docs.openclaw.ai
- Agents/compaction thresholding: apply `agents.defaults.contextTokens` cap to the model passed into embedded run and `/compact` session creation so auto-compaction thresholds use the effective context window, not native model max context. (#39099) Thanks @MumuTW. - Agents/compaction thresholding: apply `agents.defaults.contextTokens` cap to the model passed into embedded run and `/compact` session creation so auto-compaction thresholds use the effective context window, not native model max context. (#39099) Thanks @MumuTW.
- Models/merge mode provider precedence: when `models.mode: "merge"` is active and config explicitly sets a provider `baseUrl`, keep config as source of truth instead of preserving stale runtime `models.json` `baseUrl` values; includes normalized provider-key coverage. (#39103) Thanks @BigUncle. - Models/merge mode provider precedence: when `models.mode: "merge"` is active and config explicitly sets a provider `baseUrl`, keep config as source of truth instead of preserving stale runtime `models.json` `baseUrl` values; includes normalized provider-key coverage. (#39103) Thanks @BigUncle.
- UI/Control chat tool streaming: render tool events live in webchat without requiring refresh by enabling `tool-events` capability, fixing stream/event correlation, and resetting/reloading stream state around tool results and terminal events. (#39104) Thanks @jakepresent. - UI/Control chat tool streaming: render tool events live in webchat without requiring refresh by enabling `tool-events` capability, fixing stream/event correlation, and resetting/reloading stream state around tool results and terminal events. (#39104) Thanks @jakepresent.
- Models/provider apiKey persistence hardening: when a provider `apiKey` value equals a known provider env var value, persist the canonical env var name into `models.json` instead of resolved plaintext secrets. (#38889) Thanks @gambletan.
## 2026.3.2 ## 2026.3.2

View File

@@ -406,6 +406,39 @@ describe("models-config", () => {
}); });
}); });
it("does not persist resolved env var value as plaintext in models.json", async () => {
await withEnvVar("OPENAI_API_KEY", "sk-plaintext-should-not-appear", async () => {
await withTempHome(async () => {
const cfg: OpenClawConfig = {
models: {
providers: {
openai: {
apiKey: "sk-plaintext-should-not-appear", // already resolved by loadConfig
api: "openai-completions",
models: [
{
id: "gpt-4.1",
name: "GPT-4.1",
input: ["text"],
reasoning: false,
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
contextWindow: 128000,
maxTokens: 16384,
},
],
},
},
},
};
await ensureOpenClawModelsJson(cfg);
const result = await readGeneratedModelsJson<{
providers: Record<string, { apiKey?: string }>;
}>();
expect(result.providers.openai?.apiKey).toBe("OPENAI_API_KEY");
});
});
});
it("preserves explicit larger token limits when they exceed implicit catalog defaults", async () => { it("preserves explicit larger token limits when they exceed implicit catalog defaults", async () => {
await withTempHome(async () => { await withTempHome(async () => {
await withEnvVar("MOONSHOT_API_KEY", "sk-moonshot-test", async () => { await withEnvVar("MOONSHOT_API_KEY", "sk-moonshot-test", async () => {

View File

@@ -74,6 +74,39 @@ describe("normalizeProviders", () => {
await fs.rm(agentDir, { recursive: true, force: true }); await fs.rm(agentDir, { recursive: true, force: true });
} }
}); });
it("replaces resolved env var value with env var name to prevent plaintext persistence", async () => {
const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-agent-"));
const original = process.env.OPENAI_API_KEY;
process.env.OPENAI_API_KEY = "sk-test-secret-value-12345";
try {
const providers: NonNullable<NonNullable<OpenClawConfig["models"]>["providers"]> = {
openai: {
apiKey: "sk-test-secret-value-12345", // simulates resolved ${OPENAI_API_KEY}
api: "openai-completions",
models: [
{
id: "gpt-4.1",
name: "GPT-4.1",
input: ["text"],
reasoning: false,
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
contextWindow: 128000,
maxTokens: 16384,
},
],
},
};
const normalized = normalizeProviders({ providers, agentDir });
expect(normalized?.openai?.apiKey).toBe("OPENAI_API_KEY");
} finally {
if (original === undefined) {
delete process.env.OPENAI_API_KEY;
} else {
process.env.OPENAI_API_KEY = original;
}
await fs.rm(agentDir, { recursive: true, force: true });
}
});
it("normalizes SecretRef-backed provider headers to non-secret marker values", async () => { it("normalizes SecretRef-backed provider headers to non-secret marker values", async () => {
const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-agent-")); const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-agent-"));

View File

@@ -392,6 +392,8 @@ async function discoverVllmModels(
} }
} }
const ENV_VAR_NAME_RE = /^[A-Z_][A-Z0-9_]*$/;
function normalizeApiKeyConfig(value: string): string { function normalizeApiKeyConfig(value: string): string {
const trimmed = value.trim(); const trimmed = value.trim();
const match = /^\$\{([A-Z0-9_]+)\}$/.exec(trimmed); const match = /^\$\{([A-Z0-9_]+)\}$/.exec(trimmed);
@@ -655,6 +657,23 @@ export function normalizeProviders(params: {
} }
} }
// Reverse-lookup: if apiKey looks like a resolved secret value (not an env
// var name), check whether it matches the canonical env var for this provider.
// This prevents resolveConfigEnvVars()-resolved secrets from being persisted
// to models.json as plaintext. (Fixes #38757)
const currentApiKey = normalizedProvider.apiKey;
if (
typeof currentApiKey === "string" &&
currentApiKey.trim() &&
!ENV_VAR_NAME_RE.test(currentApiKey.trim())
) {
const envVarName = resolveEnvApiKeyVarName(normalizedKey);
if (envVarName && process.env[envVarName] === currentApiKey) {
mutated = true;
normalizedProvider = { ...normalizedProvider, apiKey: envVarName };
}
}
// If a provider defines models, pi's ModelRegistry requires apiKey to be set. // If a provider defines models, pi's ModelRegistry requires apiKey to be set.
// Fill it from the environment or auth profiles when possible. // Fill it from the environment or auth profiles when possible.
const hasModels = const hasModels =