From 9cdf8a1e2f9f327e4e294da40f55242a411adcc9 Mon Sep 17 00:00:00 2001 From: lukaIvanic <61480190+lukaIvanic@users.noreply.github.com> Date: Thu, 21 May 2026 03:27:34 +0200 Subject: [PATCH] Warn on plaintext secret config in doctor (#84718) Summary: - Adds a `doctor` security warning for plaintext secret-bearing `openclaw.json` fields by reusing the secrets target registry and shared model-provider header sensitivity policy. - Reproducibility: yes. for source-level behavior: current main has plaintext secret audit coverage but no doc ... llector for those config targets, and the PR body includes live patched CLI output showing the new warning. Automerge notes: - PR branch already contained follow-up commit before automerge: Warn on plaintext secret config in doctor Validation: - ClawSweeper review passed for head 31f83aae194fdb43a636d8056c3713216648fdd7. - Required merge gates passed before the squash merge. Prepared head SHA: 31f83aae194fdb43a636d8056c3713216648fdd7 Review: https://github.com/openclaw/openclaw/pull/84718#issuecomment-4503210496 Co-authored-by: qingsenlab Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com> --- CHANGELOG.md | 1 + src/commands/doctor-security.test.ts | 93 +++++++++++++++++++++ src/commands/doctor-security.ts | 51 ++++++++++- src/secrets/audit.test.ts | 36 ++++++++ src/secrets/audit.ts | 37 +------- src/secrets/model-provider-header-policy.ts | 37 ++++++++ 6 files changed, 218 insertions(+), 37 deletions(-) create mode 100644 src/secrets/model-provider-header-policy.ts 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), + ); +}