Skip to content

Commit 9f66d33

Browse files
Bartok MoltbotBartok Moltbot
Bartok Moltbot
authored and
Bartok Moltbot
committed
fix(security): redact resolved secrets from models.json
When the gateway resolves secrets at startup (env vars, SecretRef with Vault/exec/file providers), the resolved plaintext values were being written to agents/*/agent/models.json files on disk. This undermined the security benefits of centralized secret management. This change adds a redaction step before writing to models.json that: - Preserves env var names (UPPER_SNAKE_CASE patterns like OPENAI_API_KEY) - Preserves special markers (ollama-local, aws-sdk) - Strips resolved secret values that look like actual API keys The file permissions on models.json are already set to 0o600 (owner-only), but this change ensures secrets don't end up on disk at all. Fixes #37512
1 parent cbb96d9 commit 9f66d33

File tree

2 files changed

+247
-1
lines changed

2 files changed

+247
-1
lines changed

‎src/agents/models-config.redact-secrets.test.ts‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import fs from "node:fs/promises";
2+
import path from "node:path";
3+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
4+
import { ensureOpenClawModelsJson } from "./models-config.js";
5+
6+
vi.mock("../config/config.js", () => ({
7+
loadConfig: vi.fn(() => ({})),
8+
}));
9+
10+
vi.mock("./models-config.providers.js", async (importOriginal) => {
11+
const actual = await importOriginal();
12+
return {
13+
...actual,
14+
resolveImplicitProviders: vi.fn(async () => ({})),
15+
resolveImplicitBedrockProvider: vi.fn(async () => null),
16+
resolveImplicitCopilotProvider: vi.fn(async () => null),
17+
};
18+
});
19+
20+
describe("models-config secret redaction", () => {
21+
let agentDir: string;
22+
23+
beforeEach(async () => {
24+
agentDir = await fs.mkdtemp(path.join("/tmp", "openclaw-redact-test-"));
25+
});
26+
27+
afterEach(async () => {
28+
await fs.rm(agentDir, { recursive: true, force: true });
29+
});
30+
31+
it("preserves env var names in apiKey fields", async () => {
32+
const config = {
33+
models: {
34+
providers: {
35+
openai: {
36+
apiKey: "OPENAI_API_KEY",
37+
models: [{ id: "gpt-4" }],
38+
},
39+
anthropic: {
40+
apiKey: "ANTHROPIC_API_KEY",
41+
models: [{ id: "claude-3" }],
42+
},
43+
},
44+
},
45+
};
46+
47+
await ensureOpenClawModelsJson(config, agentDir);
48+
49+
const modelsJson = await fs.readFile(path.join(agentDir, "models.json"), "utf8");
50+
const parsed = JSON.parse(modelsJson);
51+
52+
expect(parsed.providers.openai?.apiKey).toBe("OPENAI_API_KEY");
53+
expect(parsed.providers.anthropic?.apiKey).toBe("ANTHROPIC_API_KEY");
54+
});
55+
56+
it("strips resolved secret values that look like API keys", async () => {
57+
const config = {
58+
models: {
59+
providers: {
60+
openai: {
61+
// Looks like a resolved OpenAI key
62+
apiKey: "sk-1234567890abcdefghijklmnop",
63+
models: [{ id: "gpt-4" }],
64+
},
65+
anthropic: {
66+
// Looks like a resolved Anthropic key
67+
apiKey: "sk-ant-1234567890abcdefghijklmnop",
68+
models: [{ id: "claude-3" }],
69+
},
70+
xai: {
71+
// Looks like a resolved xAI key
72+
apiKey: "xai-1234567890abcdefghijklmnop",
73+
models: [{ id: "grok-2" }],
74+
},
75+
},
76+
},
77+
};
78+
79+
await ensureOpenClawModelsJson(config, agentDir);
80+
81+
const modelsJson = await fs.readFile(path.join(agentDir, "models.json"), "utf8");
82+
const parsed = JSON.parse(modelsJson);
83+
84+
// Resolved secrets should be stripped
85+
expect(parsed.providers.openai?.apiKey).toBeUndefined();
86+
expect(parsed.providers.anthropic?.apiKey).toBeUndefined();
87+
expect(parsed.providers.xai?.apiKey).toBeUndefined();
88+
89+
// But the rest of the provider config should be preserved
90+
expect(parsed.providers.openai?.models).toEqual([{ id: "gpt-4" }]);
91+
expect(parsed.providers.anthropic?.models).toEqual([{ id: "claude-3" }]);
92+
expect(parsed.providers.xai?.models).toEqual([{ id: "grok-2" }]);
93+
});
94+
95+
it("preserves special marker values like ollama-local", async () => {
96+
const config = {
97+
models: {
98+
providers: {
99+
ollama: {
100+
apiKey: "ollama-local",
101+
baseUrl: "http://localhost:11434",
102+
models: [{ id: "llama3" }],
103+
},
104+
},
105+
},
106+
};
107+
108+
await ensureOpenClawModelsJson(config, agentDir);
109+
110+
const modelsJson = await fs.readFile(path.join(agentDir, "models.json"), "utf8");
111+
const parsed = JSON.parse(modelsJson);
112+
113+
expect(parsed.providers.ollama?.apiKey).toBe("ollama-local");
114+
});
115+
116+
it("strips mixed-case resolved secrets", async () => {
117+
const config = {
118+
models: {
119+
providers: {
120+
custom: {
121+
// Mixed case = resolved secret, not env var name
122+
apiKey: "Bearer_Token_12345",
123+
models: [{ id: "custom-model" }],
124+
},
125+
},
126+
},
127+
};
128+
129+
await ensureOpenClawModelsJson(config, agentDir);
130+
131+
const modelsJson = await fs.readFile(path.join(agentDir, "models.json"), "utf8");
132+
const parsed = JSON.parse(modelsJson);
133+
134+
expect(parsed.providers.custom?.apiKey).toBeUndefined();
135+
expect(parsed.providers.custom?.models).toEqual([{ id: "custom-model" }]);
136+
});
137+
138+
it("handles providers with aws-sdk auth (apiKey set to env var name)", async () => {
139+
const config = {
140+
models: {
141+
providers: {
142+
"amazon-bedrock": {
143+
auth: "aws-sdk",
144+
baseUrl: "https://bedrock-runtime.us-east-1.amazonaws.com",
145+
models: [{ id: "anthropic.claude-v2" }],
146+
},
147+
},
148+
},
149+
};
150+
151+
await ensureOpenClawModelsJson(config, agentDir);
152+
153+
const modelsJson = await fs.readFile(path.join(agentDir, "models.json"), "utf8");
154+
const parsed = JSON.parse(modelsJson);
155+
156+
expect(parsed.providers["amazon-bedrock"]?.auth).toBe("aws-sdk");
157+
// AWS SDK auth uses AWS_PROFILE env var name - this should be preserved
158+
expect(parsed.providers["amazon-bedrock"]?.apiKey).toBe("AWS_PROFILE");
159+
});
160+
161+
it("sets restrictive file permissions on models.json", async () => {
162+
const config = {
163+
models: {
164+
providers: {
165+
openai: {
166+
apiKey: "OPENAI_API_KEY",
167+
models: [{ id: "gpt-4" }],
168+
},
169+
},
170+
},
171+
};
172+
173+
await ensureOpenClawModelsJson(config, agentDir);
174+
175+
const stats = await fs.stat(path.join(agentDir, "models.json"));
176+
// 0o600 = owner read/write only
177+
expect(stats.mode & 0o777).toBe(0o600);
178+
});
179+
});

‎src/agents/models-config.ts‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,67 @@ import {
1414

1515
type ModelsConfig = NonNullable<OpenClawConfig["models"]>;
1616

17+
/**
18+
* Check if a string looks like an environment variable name rather than a
19+
* resolved secret value. Env var names are UPPER_SNAKE_CASE. Resolved secrets
20+
* are typically longer and contain mixed-case alphanumeric characters, dashes,
21+
* underscores in non-UPPER_SNAKE_CASE patterns.
22+
*
23+
* Special cases:
24+
* - "ollama-local" and similar synthetic markers are preserved
25+
* - "aws-sdk" auth mode markers are preserved
26+
*/
27+
function isEnvVarNameOrMarker(value: string): boolean {
28+
const trimmed = value.trim();
29+
if (!trimmed) {
30+
return false;
31+
}
32+
33+
// Synthetic local provider markers (e.g., "ollama-local")
34+
if (trimmed === "ollama-local") {
35+
return true;
36+
}
37+
38+
// AWS SDK auth marker
39+
if (trimmed === "aws-sdk") {
40+
return true;
41+
}
42+
43+
// Environment variable name pattern: UPPER_SNAKE_CASE
44+
// Must start with letter, contain only uppercase letters, digits, underscores
45+
// Examples: OPENAI_API_KEY, ANTHROPIC_API_KEY, AWS_ACCESS_KEY_ID
46+
return /^[A-Z][A-Z0-9_]*$/.test(trimmed);
47+
}
48+
49+
/**
50+
* Redact resolved secret values from provider configs before writing to disk.
51+
* Preserves env var names (UPPER_SNAKE_CASE) and special markers, but strips
52+
* actual resolved API keys to prevent plaintext secrets in models.json.
53+
*/
54+
function redactResolvedSecrets(
55+
providers: Record<string, ProviderConfig>,
56+
): Record<string, ProviderConfig> {
57+
const redacted: Record<string, ProviderConfig> = {};
58+
59+
for (const [key, provider] of Object.entries(providers)) {
60+
const { apiKey, ...rest } = provider;
61+
62+
// If apiKey is undefined or empty, or looks like an env var name/marker, preserve it
63+
if (
64+
apiKey === undefined ||
65+
apiKey === "" ||
66+
(typeof apiKey === "string" && isEnvVarNameOrMarker(apiKey))
67+
) {
68+
redacted[key] = provider;
69+
} else {
70+
// apiKey looks like a resolved secret - strip it
71+
redacted[key] = rest as ProviderConfig;
72+
}
73+
}
74+
75+
return redacted;
76+
}
77+
1778
const DEFAULT_MODE: NonNullable<ModelsConfig["mode"]> = "merge";
1879

1980
function resolvePreferredTokenLimit(explicitValue: number, implicitValue: number): number {
@@ -231,7 +292,13 @@ export async function ensureOpenClawModelsJson(
231292
providers: mergedProviders,
232293
agentDir,
233294
});
234-
const next = `${JSON.stringify({ providers: normalizedProviders }, null, 2)}\n`;
295+
296+
// Redact resolved secrets before writing to disk. This ensures that
297+
// regardless of how secrets were resolved (env vars, SecretRef providers,
298+
// etc.), only env var names are persisted, not plaintext API keys.
299+
// See: https://github.com/OpenClaw/openclaw/issues/37512
300+
const safeProviders = redactResolvedSecrets(normalizedProviders);
301+
const next = `${JSON.stringify({ providers: safeProviders }, null, 2)}\n`;
235302
const existingRaw = await readRawFile(targetPath);
236303

237304
if (existingRaw === next) {

0 commit comments

Comments
 (0)