diff --git a/CHANGELOG.md b/CHANGELOG.md index f77e90227b9..e6be9ae1fbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Docs: https://docs.openclaw.ai - Infra/secrets: restore the fail-closed contract for `tryReadSecretFileSync` so credential loaders that pass `rejectSymlink: true` (Telegram, LINE, Zalo, IRC, Nextcloud Talk tokens) refuse symlinked credential files instead of silently accepting them, and the infra-state CI shard's secret-file symlink test passes again. Thanks @romneyda. - Browser: honor the configured image sanitization limit for screenshots and labeled snapshots so browser-captured images follow the same resize policy as other image results. (#84595) - Doctor: remove unrecognized `models.providers.*.models[*].compat.thinkingFormat` values during `doctor --fix` so stale provider model config can validate after upgrade. Fixes #77803. +- Doctor: warn when `openclaw.json` stores plaintext secret-bearing config fields, including model provider API keys and sensitive provider headers. (#84718) Thanks @lukaIvanic. - Status: show the configured default, session-selected model, reason, clear hint, and docs link when a session remains pinned to a model that differs from `agents.defaults.model.primary`. - Mac app: keep local packaging signed with a stable app identity for permission testing and fix Control UI production builds under current Vite/Highlight.js exports. - macOS app: update the embedded Peekaboo bridge to 3.2.1 so OpenClaw-hosted UI automation works with current Peekaboo CLI capture flows. diff --git a/src/commands/doctor-security.test.ts b/src/commands/doctor-security.test.ts index f1704968d47..72e45a40b83 100644 --- a/src/commands/doctor-security.test.ts +++ b/src/commands/doctor-security.test.ts @@ -332,6 +332,99 @@ describe("noteSecurityWarnings gateway exposure", () => { ); }); + it("warns when model provider API keys are stored as plaintext in config", async () => { + await noteSecurityWarnings({ + models: { + providers: { + openai: { + apiKey: "sk-openai-plaintext", + }, + }, + }, + } as unknown as OpenClawConfig); + + const message = lastMessage(); + expect(message).toContain("plaintext secret-bearing config fields"); + expect(message).toContain("models.providers.openai.apiKey"); + expect(message).toContain("openclaw secrets audit --check"); + }); + + it("warns when sensitive model provider headers are stored as plaintext in config", async () => { + await noteSecurityWarnings({ + models: { + providers: { + openai: { + headers: { + Authorization: "Bearer sk-header-plaintext", + }, + }, + }, + }, + } as unknown as OpenClawConfig); + + const message = lastMessage(); + expect(message).toContain("plaintext secret-bearing config fields"); + expect(message).toContain("models.providers.openai.headers.Authorization"); + }); + + it("does not warn when non-sensitive model provider headers are stored as plaintext in config", async () => { + await noteSecurityWarnings({ + models: { + providers: { + openai: { + headers: { + "X-Proxy-Region": "us-west", + }, + }, + }, + }, + } as unknown as OpenClawConfig); + + const message = lastMessage(); + expect(message).not.toContain("plaintext secret-bearing config fields"); + expect(message).not.toContain("models.providers.openai.headers.X-Proxy-Region"); + }); + + it("keeps request headers aligned with secrets audit plaintext checks", async () => { + await noteSecurityWarnings({ + models: { + providers: { + openai: { + request: { + headers: { + "X-Proxy-Region": "us-west", + }, + }, + }, + }, + }, + } as unknown as OpenClawConfig); + + const message = lastMessage(); + expect(message).toContain("plaintext secret-bearing config fields"); + expect(message).toContain("models.providers.openai.request.headers.X-Proxy-Region"); + }); + + it("does not warn when model provider API keys are stored as SecretRefs", async () => { + await noteSecurityWarnings({ + secrets: { + providers: { + default: { source: "env" }, + }, + }, + models: { + providers: { + openai: { + apiKey: "${OPENAI_API_KEY}", + }, + }, + }, + } as unknown as OpenClawConfig); + + const message = lastMessage(); + expect(message).not.toContain("plaintext secret-bearing config fields"); + }); + it("warns when tools.exec is broader than host exec defaults", async () => { await withExecApprovalsFile( { diff --git a/src/commands/doctor-security.ts b/src/commands/doctor-security.ts index 333af06232c..80f5217b440 100644 --- a/src/commands/doctor-security.ts +++ b/src/commands/doctor-security.ts @@ -4,12 +4,15 @@ import type { ChannelId } from "../channels/plugins/types.public.js"; import { formatCliCommand } from "../cli/command-format.js"; import type { OpenClawConfig, GatewayBindMode } from "../config/config.js"; import type { AgentConfig } from "../config/types.agents.js"; -import { hasConfiguredSecretInput } from "../config/types.secrets.js"; +import { hasConfiguredSecretInput, resolveSecretInputRef } from "../config/types.secrets.js"; import { resolveGatewayAuthTokenSourceConflict } from "../gateway/auth-token-source-conflict.js"; import { resolveGatewayAuth } from "../gateway/auth.js"; import { isLoopbackHost, resolveGatewayBindHost } from "../gateway/net.js"; import { resolveExecPolicyScopeSnapshot } from "../infra/exec-approvals-effective.js"; import { loadExecApprovals, type ExecAsk, type ExecSecurity } from "../infra/exec-approvals.js"; +import { isLikelySensitiveModelProviderHeaderName } from "../secrets/model-provider-header-policy.js"; +import { hasConfiguredPlaintextSecretValue } from "../secrets/secret-value.js"; +import { discoverConfigSecretTargets } from "../secrets/target-registry.js"; import { collectExecFilesystemPolicyDriftHits } from "../security/exec-filesystem-policy.js"; import { normalizeOptionalString } from "../shared/string-coerce.js"; import { note } from "../terminal/note.js"; @@ -180,6 +183,51 @@ function collectExecFilesystemPolicyWarnings(cfg: OpenClawConfig): string[] { ); } +function collectPlaintextConfigSecretWarnings(cfg: OpenClawConfig): string[] { + const plaintextPaths: string[] = []; + const defaults = cfg.secrets?.defaults; + + for (const target of discoverConfigSecretTargets(cfg)) { + if (!target.entry.includeInAudit) { + continue; + } + if ( + target.entry.id === "models.providers.*.headers.*" && + !isLikelySensitiveModelProviderHeaderName(target.pathSegments.at(-1) ?? "") + ) { + continue; + } + const { ref } = resolveSecretInputRef({ + value: target.value, + refValue: target.refValue, + defaults, + }); + if (ref) { + continue; + } + if (!hasConfiguredPlaintextSecretValue(target.value, target.entry.expectedResolvedValue)) { + continue; + } + plaintextPaths.push(target.path); + } + + if (plaintextPaths.length === 0) { + return []; + } + + const samplePaths = plaintextPaths.slice(0, 5); + const extraCount = plaintextPaths.length - samplePaths.length; + const pathLine = + extraCount > 0 ? `${samplePaths.join(", ")} (+${extraCount} more)` : samplePaths.join(", "); + + return [ + "- WARNING: openclaw.json contains plaintext secret-bearing config fields.", + ` Paths: ${pathLine}`, + " Agents or workspace tools that can read config files may see these API keys/tokens.", + ` Migrate them to SecretRefs with ${formatCliCommand("openclaw secrets configure")} or ${formatCliCommand("openclaw secrets apply")}, then verify with ${formatCliCommand("openclaw secrets audit --check")}.`, + ]; +} + export async function collectSecurityWarnings( cfg: OpenClawConfig, env: NodeJS.ProcessEnv = process.env, @@ -197,6 +245,7 @@ export async function collectSecurityWarnings( warnings.push(...collectImplicitHeartbeatDirectPolicyWarnings(cfg)); warnings.push(...collectExecPolicyConflictWarnings(cfg)); warnings.push(...collectExecFilesystemPolicyWarnings(cfg)); + warnings.push(...collectPlaintextConfigSecretWarnings(cfg)); warnings.push(...collectDurableExecApprovalWarnings(cfg)); // =========================================== diff --git a/src/secrets/audit.test.ts b/src/secrets/audit.test.ts index a2a2a92e087..6f9bc26e67e 100644 --- a/src/secrets/audit.test.ts +++ b/src/secrets/audit.test.ts @@ -710,4 +710,40 @@ describe("secrets audit", () => { ), ).toBe(false); }); + + it("keeps request headers in openclaw config covered by plaintext audit", async () => { + await writeJsonFile(fixture.configPath, { + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + api: "openai-completions", + apiKey: { source: "env", provider: "default", id: OPENAI_API_KEY_MARKER }, + request: { + headers: { + "X-Proxy-Region": "us-west", + }, + }, + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }, + }); + await writeJsonFile(fixture.authStorePath, { + version: 1, + profiles: {}, + }); + await fs.writeFile(fixture.envPath, "", "utf8"); + + const report = await runSecretsAudit({ env: fixture.env }); + expect( + hasFinding( + report, + (entry) => + entry.code === "PLAINTEXT_FOUND" && + entry.file === fixture.configPath && + entry.jsonPath === "models.providers.openai.request.headers.X-Proxy-Region", + ), + ).toBe(true); + }); }); diff --git a/src/secrets/audit.ts b/src/secrets/audit.ts index 09170c961f6..8daf3970821 100644 --- a/src/secrets/audit.ts +++ b/src/secrets/audit.ts @@ -10,12 +10,12 @@ import { resolveStateDir, type OpenClawConfig } from "../config/config.js"; import { coerceSecretRef } from "../config/types.secrets.js"; import { resolveSecretInputRef, type SecretRef } from "../config/types.secrets.js"; import { formatErrorMessage } from "../infra/errors.js"; -import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js"; import { resolveConfigDir, resolveUserPath } from "../utils.js"; import { runTasksWithConcurrency } from "../utils/run-with-concurrency.js"; import { iterateAuthProfileCredentials } from "./auth-profiles-scan.js"; import { createSecretsConfigIO } from "./config-io.js"; import { getSkippedExecRefStaticError, selectRefsForExecPolicy } from "./exec-resolution-policy.js"; +import { isLikelySensitiveModelProviderHeaderName } from "./model-provider-header-policy.js"; import { listKnownSecretEnvVarNames } from "./provider-env-vars.js"; import { secretRefKey } from "./ref-contract.js"; import { @@ -105,41 +105,6 @@ type AuditCollector = { const REF_RESOLVE_FALLBACK_CONCURRENCY = 8; const MAX_AUDIT_MODELS_JSON_BYTES = 5 * 1024 * 1024; -const ALWAYS_SENSITIVE_MODEL_PROVIDER_HEADER_NAMES = new Set([ - "authorization", - "proxy-authorization", - "x-api-key", - "api-key", - "apikey", - "x-auth-token", - "auth-token", - "x-access-token", - "access-token", - "x-secret-key", - "secret-key", -]); -const SENSITIVE_MODEL_PROVIDER_HEADER_NAME_FRAGMENTS = [ - "api-key", - "apikey", - "token", - "secret", - "password", - "credential", -]; - -function isLikelySensitiveModelProviderHeaderName(value: string): boolean { - const normalized = normalizeLowercaseStringOrEmpty(value); - if (!normalized) { - return false; - } - if (ALWAYS_SENSITIVE_MODEL_PROVIDER_HEADER_NAMES.has(normalized)) { - return true; - } - return SENSITIVE_MODEL_PROVIDER_HEADER_NAME_FRAGMENTS.some((fragment) => - normalized.includes(fragment), - ); -} - function addFinding(collector: AuditCollector, finding: SecretsAuditFinding): void { collector.findings.push(finding); } diff --git a/src/secrets/model-provider-header-policy.ts b/src/secrets/model-provider-header-policy.ts new file mode 100644 index 00000000000..0f78e4a80d6 --- /dev/null +++ b/src/secrets/model-provider-header-policy.ts @@ -0,0 +1,37 @@ +import { normalizeLowercaseStringOrEmpty } from "../shared/string-coerce.js"; + +const ALWAYS_SENSITIVE_MODEL_PROVIDER_HEADER_NAMES = new Set([ + "authorization", + "proxy-authorization", + "x-api-key", + "api-key", + "apikey", + "x-auth-token", + "auth-token", + "x-access-token", + "access-token", + "x-secret-key", + "secret-key", +]); + +const SENSITIVE_MODEL_PROVIDER_HEADER_NAME_FRAGMENTS = [ + "api-key", + "apikey", + "token", + "secret", + "password", + "credential", +]; + +export function isLikelySensitiveModelProviderHeaderName(value: string): boolean { + const normalized = normalizeLowercaseStringOrEmpty(value); + if (!normalized) { + return false; + } + if (ALWAYS_SENSITIVE_MODEL_PROVIDER_HEADER_NAMES.has(normalized)) { + return true; + } + return SENSITIVE_MODEL_PROVIDER_HEADER_NAME_FRAGMENTS.some((fragment) => + normalized.includes(fragment), + ); +}