diff --git a/CHANGELOG.md b/CHANGELOG.md index c99a9564015..3600d3ce7e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ Docs: https://docs.openclaw.ai - Browser/sandbox: pass the resolved `browser.ssrfPolicy` into sandbox browser bridges and refresh cached bridges when the effective policy changes, so sandboxed browser navigation honors private-network opt-ins. Fixes #45153 and #57055. Thanks @jzakirov, @zuoanCo, and @kybrcore. - Dashboard/Windows: open Control UI and OAuth URLs through the system URL handler without `cmd.exe` parsing or PATH-based `rundll32` lookup, and reject non-HTTP browser-open inputs. Fixes #71098. Thanks @Sanjays2402. +- Config/doctor: reject legacy `secretref-env:` marker strings on SecretRef credential paths and migrate valid markers to structured env SecretRefs with `openclaw doctor --fix`. Fixes #51794. Thanks @halointellicore. - Providers/OpenAI: separate API-key and Codex sign-in onboarding groups, and avoid replaying stale OpenAI Responses reasoning blocks after a model route switch. - Providers/ElevenLabs: omit the MP3-only `Accept` header for PCM telephony synthesis, so Voice Call requests for `pcm_22050` no longer receive MP3 audio. Fixes #67340. Thanks @marcchabot. - Plugins/Voice Call: honor configured TTS timeouts for Twilio media-stream playback and fail empty telephony audio instead of completing as silence. Fixes #42071; supersedes #60957. Thanks @Ryce and @sliekens. diff --git a/docs/reference/secretref-credential-surface.md b/docs/reference/secretref-credential-surface.md index e822f4b1f3c..77904decad4 100644 --- a/docs/reference/secretref-credential-surface.md +++ b/docs/reference/secretref-credential-surface.md @@ -114,6 +114,7 @@ Notes: - Auth-profile plan targets require `agentId`. - Plan entries target `profiles.*.key` / `profiles.*.token` and write sibling refs (`keyRef` / `tokenRef`). - Auth-profile refs are included in runtime resolution and audit coverage. +- In `openclaw.json`, SecretRefs must use structured objects such as `{"source":"env","provider":"default","id":"DISCORD_BOT_TOKEN"}`. Legacy `secretref-env:` marker strings are rejected on SecretRef credential paths; run `openclaw doctor --fix` to migrate valid markers. - OAuth policy guard: `auth.profiles..mode = "oauth"` cannot be combined with SecretRef inputs for that profile. Startup/reload and auth-profile resolution fail fast when this policy is violated. - For SecretRef-managed model providers, generated `agents/*/agent/models.json` entries persist non-secret markers (not resolved secret values) for `apiKey`/header surfaces. - Marker persistence is source-authoritative: OpenClaw writes markers from the active source config snapshot (pre-resolution), not from resolved runtime secret values. diff --git a/src/commands/doctor-legacy-config.migrations.test.ts b/src/commands/doctor-legacy-config.migrations.test.ts index 108aec2b1fa..dc5a92d48a1 100644 --- a/src/commands/doctor-legacy-config.migrations.test.ts +++ b/src/commands/doctor-legacy-config.migrations.test.ts @@ -47,6 +47,64 @@ vi.mock("./doctor/shared/channel-legacy-config-migrate.js", () => ({ }), })); +vi.mock("../secrets/target-registry.js", () => { + const entry = { + id: "channels.discord.token", + targetType: "channels.discord.token", + configFile: "openclaw.json", + pathPattern: "channels.discord.token", + secretShape: "secret_input", + expectedResolvedValue: "string", + includeInPlan: true, + includeInConfigure: true, + includeInAudit: true, + }; + + const readRecord = (value: unknown): Record | null => + value && typeof value === "object" && !Array.isArray(value) + ? (value as Record) + : null; + + return { + discoverConfigSecretTargets: (cfg: OpenClawConfig) => { + const targets: Array<{ + entry: typeof entry; + path: string; + pathSegments: string[]; + value: unknown; + accountId?: string; + }> = []; + const channels = readRecord(cfg.channels); + const discord = readRecord(channels?.discord); + if (!discord) { + return targets; + } + targets.push({ + entry, + path: "channels.discord.token", + pathSegments: ["channels", "discord", "token"], + value: discord.token, + }); + + const accounts = readRecord(discord.accounts); + for (const [accountId, accountConfig] of Object.entries(accounts ?? {})) { + const account = readRecord(accountConfig); + if (!account) { + continue; + } + targets.push({ + entry, + path: `channels.discord.accounts.${accountId}.token`, + pathSegments: ["channels", "discord", "accounts", accountId, "token"], + value: account.token, + accountId, + }); + } + return targets; + }, + }; +}); + describe("normalizeCompatibilityConfigValues", () => { let previousOauthDir: string | undefined; let tempOauthDir = ""; @@ -115,6 +173,57 @@ describe("normalizeCompatibilityConfigValues", () => { }); }); + it("migrates legacy secretref-env markers on SecretRef credential paths", () => { + const res = normalizeCompatibilityConfigValues({ + secrets: { + defaults: { + env: "gateway-env", + }, + }, + channels: { + discord: { + token: "secretref-env:DISCORD_BOT_TOKEN", + accounts: { + work: { + token: "secretref-env:DISCORD_WORK_TOKEN", + }, + }, + }, + }, + } as unknown as OpenClawConfig); + + expect(res.config.channels?.discord?.token).toBeUndefined(); + expect(res.config.channels?.discord?.accounts?.default?.token).toEqual({ + source: "env", + provider: "gateway-env", + id: "DISCORD_BOT_TOKEN", + }); + expect(res.config.channels?.discord?.accounts?.work?.token).toEqual({ + source: "env", + provider: "gateway-env", + id: "DISCORD_WORK_TOKEN", + }); + expect(res.changes).toContain( + "Moved channels.discord.accounts.default.token secretref-env:DISCORD_BOT_TOKEN marker → structured env SecretRef.", + ); + expect(res.changes).toContain( + "Moved channels.discord.accounts.work.token secretref-env:DISCORD_WORK_TOKEN marker → structured env SecretRef.", + ); + }); + + it("leaves invalid legacy secretref-env markers for validation to reject", () => { + const res = normalizeCompatibilityConfigValues({ + channels: { + discord: { + token: "secretref-env:not-valid", + }, + }, + } as unknown as OpenClawConfig); + + expect(res.config.channels?.discord?.token).toBe("secretref-env:not-valid"); + expect(res.changes).toEqual([]); + }); + it("moves WhatsApp access defaults into accounts.default for named accounts", () => { const res = normalizeCompatibilityConfigValues({ channels: { diff --git a/src/commands/doctor/shared/legacy-config-core-migrate.ts b/src/commands/doctor/shared/legacy-config-core-migrate.ts index f8310cc2076..a381a665413 100644 --- a/src/commands/doctor/shared/legacy-config-core-migrate.ts +++ b/src/commands/doctor/shared/legacy-config-core-migrate.ts @@ -1,5 +1,6 @@ import type { OpenClawConfig } from "../../../config/types.openclaw.js"; import { runPluginSetupConfigMigrations } from "../../../plugins/setup-registry.js"; +import { migrateLegacySecretRefEnvMarkers } from "../../../secrets/legacy-secretref-env-marker.js"; import { applyChannelDoctorCompatibilityMigrations } from "./channel-legacy-config-migrate.js"; import { normalizeBaseCompatibilityConfigValues } from "./legacy-config-compatibility-base.js"; import { @@ -27,6 +28,11 @@ export function normalizeCompatibilityConfigValues(cfg: OpenClawConfig): { next = channelMigrations.next; changes.push(...channelMigrations.changes); } + const secretRefMarkers = migrateLegacySecretRefEnvMarkers(next); + if (secretRefMarkers.changes.length > 0) { + next = secretRefMarkers.config; + changes.push(...secretRefMarkers.changes); + } next = normalizeLegacyCommandsConfig(next, changes); next = normalizeLegacyOpenAICodexModelsAddMetadata(next, changes); diff --git a/src/config/types.secrets.resolution.test.ts b/src/config/types.secrets.resolution.test.ts index 3023f2d4e27..cb7bbc8477d 100644 --- a/src/config/types.secrets.resolution.test.ts +++ b/src/config/types.secrets.resolution.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; -import { normalizeResolvedSecretInputString, resolveSecretInputString } from "./types.secrets.js"; +import { + normalizeResolvedSecretInputString, + parseLegacySecretRefEnvMarker, + resolveSecretInputString, +} from "./types.secrets.js"; describe("resolveSecretInputString", () => { it("returns available for non-empty string values", () => { @@ -78,3 +82,25 @@ describe("normalizeResolvedSecretInputString", () => { ).toThrow(/unresolved SecretRef/); }); }); + +describe("parseLegacySecretRefEnvMarker", () => { + it("parses legacy env marker strings without making them valid SecretInput strings", () => { + expect(parseLegacySecretRefEnvMarker("secretref-env:OPENAI_API_KEY")).toEqual({ + source: "env", + provider: "default", + id: "OPENAI_API_KEY", + }); + expect(parseLegacySecretRefEnvMarker("secretref-env:not-valid")).toBeNull(); + expect( + resolveSecretInputString({ + value: "secretref-env:OPENAI_API_KEY", + path: "models.providers.openai.apiKey", + mode: "inspect", + }), + ).toEqual({ + status: "available", + value: "secretref-env:OPENAI_API_KEY", + ref: null, + }); + }); +}); diff --git a/src/config/types.secrets.ts b/src/config/types.secrets.ts index 00d9dec700a..5cb58778718 100644 --- a/src/config/types.secrets.ts +++ b/src/config/types.secrets.ts @@ -18,6 +18,7 @@ export type SecretRef = { export type SecretInput = string | SecretRef; export const DEFAULT_SECRET_PROVIDER_ALIAS = "default"; // pragma: allowlist secret export const ENV_SECRET_REF_ID_RE = /^[A-Z][A-Z0-9_]{0,127}$/; +export const LEGACY_SECRETREF_ENV_MARKER_PREFIX = "secretref-env:"; // pragma: allowlist secret const ENV_SECRET_TEMPLATE_RE = /^\$\{([A-Z][A-Z0-9_]{0,127})\}$/; export type SecretInputStringResolutionMode = "strict" | "inspect"; export type SecretInputStringResolution = @@ -82,6 +83,28 @@ export function parseEnvTemplateSecretRef( }; } +export function parseLegacySecretRefEnvMarker( + value: unknown, + provider = DEFAULT_SECRET_PROVIDER_ALIAS, +): SecretRef | null { + if (typeof value !== "string") { + return null; + } + const trimmed = value.trim(); + if (!trimmed.startsWith(LEGACY_SECRETREF_ENV_MARKER_PREFIX)) { + return null; + } + const id = trimmed.slice(LEGACY_SECRETREF_ENV_MARKER_PREFIX.length); + if (!ENV_SECRET_REF_ID_RE.test(id)) { + return null; + } + return { + source: "env", + provider: provider.trim() || DEFAULT_SECRET_PROVIDER_ALIAS, + id, + }; +} + export function coerceSecretRef(value: unknown, defaults?: SecretDefaults): SecretRef | null { if (isSecretRef(value)) { return value; diff --git a/src/config/validation.policy.test.ts b/src/config/validation.policy.test.ts index ada44cdd2e3..2c90db19291 100644 --- a/src/config/validation.policy.test.ts +++ b/src/config/validation.policy.test.ts @@ -98,6 +98,51 @@ describe("config validation SecretRef policy guards", () => { expect(result.ok).toBe(true); }); + it("rejects legacy secretref-env markers on supported SecretRef credential paths", () => { + const result = validateConfigObjectRaw({ + secrets: { + defaults: { + env: "gateway-env", + }, + }, + channels: { + discord: { + token: "secretref-env:DISCORD_BOT_TOKEN", + }, + }, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + const issue = result.issues.find((entry) => entry.path === "channels.discord.token"); + expect(issue).toBeDefined(); + expect(issue?.message).toContain( + '"secretref-env:DISCORD_BOT_TOKEN" is a legacy SecretRef marker', + ); + expect(issue?.message).toContain( + '{"source":"env","provider":"gateway-env","id":"DISCORD_BOT_TOKEN"}', + ); + expect(issue?.message).toContain('Run "openclaw doctor --fix"'); + } + }); + + it("rejects invalid legacy secretref-env markers that doctor cannot migrate", () => { + const result = validateConfigObjectRaw({ + channels: { + discord: { + token: "secretref-env:not-valid", + }, + }, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + const issue = result.issues.find((entry) => entry.path === "channels.discord.token"); + expect(issue?.message).toContain('"secretref-env:not-valid" is a legacy SecretRef marker'); + expect(issue?.message).toContain('{"source":"env","provider":"default","id":"ENV_VAR"}'); + } + }); + it("replaces derived unrecognized-key errors with policy guidance for discord thread binding webhookToken", () => { const result = validateConfigObjectRaw({ channels: { diff --git a/src/config/validation.ts b/src/config/validation.ts index b457416d613..1278cb8f641 100644 --- a/src/config/validation.ts +++ b/src/config/validation.ts @@ -19,6 +19,7 @@ import { } from "../plugins/manifest-registry.js"; import { validateJsonSchemaValue } from "../plugins/schema-validator.js"; import { hasKind } from "../plugins/slots.js"; +import { collectLegacySecretRefEnvMarkerCandidates } from "../secrets/legacy-secretref-env-marker.js"; import { collectUnsupportedSecretRefConfigCandidates } from "../secrets/unsupported-surface-policy.js"; import { hasAvatarUriScheme, @@ -454,7 +455,35 @@ function mergeUnsupportedMutableSecretRefIssues( } export function collectUnsupportedSecretRefPolicyIssues(raw: unknown): ConfigValidationIssue[] { - return collectUnsupportedMutableSecretRefIssues(raw); + return [ + ...collectUnsupportedMutableSecretRefIssues(raw), + ...collectLegacySecretRefEnvMarkerIssues(raw), + ]; +} + +function formatLegacySecretRefEnvMarkerMessage(candidate: { + value: string; + ref: { id: string; provider: string } | null; +}): string { + const replacement = candidate.ref + ? JSON.stringify({ source: "env", provider: candidate.ref.provider, id: candidate.ref.id }) + : '{"source":"env","provider":"default","id":"ENV_VAR"}'; + return [ + `${JSON.stringify(candidate.value)} is a legacy SecretRef marker and is not valid openclaw.json config.`, + `Use a structured SecretRef object instead, for example ${replacement}.`, + 'Run "openclaw doctor --fix" to migrate valid secretref-env: markers.', + `See ${SECRETREF_POLICY_DOC_URL}.`, + ].join(" "); +} + +function collectLegacySecretRefEnvMarkerIssues(raw: unknown): ConfigValidationIssue[] { + if (!isRecord(raw)) { + return []; + } + return collectLegacySecretRefEnvMarkerCandidates(raw as OpenClawConfig).map((candidate) => ({ + path: candidate.path, + message: formatLegacySecretRefEnvMarkerMessage(candidate), + })); } function mapZodIssueToConfigIssue(issue: unknown): ConfigValidationIssue { diff --git a/src/secrets/legacy-secretref-env-marker.ts b/src/secrets/legacy-secretref-env-marker.ts new file mode 100644 index 00000000000..a851eeb24a1 --- /dev/null +++ b/src/secrets/legacy-secretref-env-marker.ts @@ -0,0 +1,73 @@ +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { + LEGACY_SECRETREF_ENV_MARKER_PREFIX, + parseLegacySecretRefEnvMarker, + type SecretRef, +} from "../config/types.secrets.js"; +import { setPathExistingStrict } from "./path-utils.js"; +import { + discoverConfigSecretTargets, + type DiscoveredConfigSecretTarget, +} from "./target-registry.js"; + +export type LegacySecretRefEnvMarkerCandidate = { + path: string; + pathSegments: string[]; + value: string; + ref: SecretRef | null; +}; + +function isLegacySecretRefEnvMarker(value: unknown): value is string { + return typeof value === "string" && value.trim().startsWith(LEGACY_SECRETREF_ENV_MARKER_PREFIX); +} + +function toCandidate( + target: DiscoveredConfigSecretTarget, + defaults: NonNullable["defaults"] | undefined, +): LegacySecretRefEnvMarkerCandidate | null { + if (!isLegacySecretRefEnvMarker(target.value)) { + return null; + } + return { + path: target.path, + pathSegments: target.pathSegments, + value: target.value.trim(), + ref: parseLegacySecretRefEnvMarker(target.value, defaults?.env), + }; +} + +export function collectLegacySecretRefEnvMarkerCandidates( + config: OpenClawConfig, +): LegacySecretRefEnvMarkerCandidate[] { + const defaults = config.secrets?.defaults; + return discoverConfigSecretTargets(config) + .map((target) => toCandidate(target, defaults)) + .filter((candidate): candidate is LegacySecretRefEnvMarkerCandidate => candidate !== null); +} + +export function migrateLegacySecretRefEnvMarkers(config: OpenClawConfig): { + config: OpenClawConfig; + changes: string[]; +} { + const candidates = collectLegacySecretRefEnvMarkerCandidates(config).filter( + (candidate) => candidate.ref !== null, + ); + if (candidates.length === 0) { + return { config, changes: [] }; + } + + const next = structuredClone(config) as OpenClawConfig & Record; + const changes: string[] = []; + for (const candidate of candidates) { + const ref = candidate.ref; + if (!ref) { + continue; + } + if (setPathExistingStrict(next, candidate.pathSegments, ref)) { + changes.push( + `Moved ${candidate.path} ${LEGACY_SECRETREF_ENV_MARKER_PREFIX}${ref.id} marker → structured env SecretRef.`, + ); + } + } + return { config: next, changes }; +}