From 383ea34efe11d94ede84871cbc38ab21d0991241 Mon Sep 17 00:00:00 2001 From: Mariano Date: Fri, 10 Apr 2026 14:56:30 +0200 Subject: [PATCH] fix(reply): keep resolved secret config stable (#64249) Merged via squash. Prepared head SHA: 973f863d8cbba32cc1f45f9e24447c8f933e48ff Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky --- CHANGELOG.md | 1 + .../memory-host-sdk/src/host/secret-input.ts | 9 +++ .../pi-embedded-runner/skills-runtime.test.ts | 40 ++++++++++ src/agents/skills.test.ts | 79 +++++++++++++++++++ src/agents/skills/env-overrides.ts | 14 ++-- src/agents/skills/runtime-config.ts | 31 +++++++- src/memory-host-sdk/host/secret-input.test.ts | 36 +++++++++ src/memory-host-sdk/host/secret-input.ts | 9 +++ 8 files changed, 211 insertions(+), 8 deletions(-) create mode 100644 src/memory-host-sdk/host/secret-input.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a943ff346d..605efb692fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,7 @@ Docs: https://docs.openclaw.ai - Claude CLI/skills: pass eligible OpenClaw skills into CLI runs, including native Claude Code skill resolution via a temporary plugin plus per-run skill env/API key injection. (#62686, #62723) Thanks @zomars. - Heartbeat: ignore doc-only Markdown fence markers in the default `HEARTBEAT.md` template so comment-only heartbeat scaffolds skip API calls again. (#63434) Thanks @ravyg. - Control UI/BTW: render `/btw` side results as dismissible ephemeral cards in the browser, send `/btw` immediately during active runs, and clear stale BTW cards on reset flows so webchat matches the intended detached side-question behavior. (#64290) Thanks @ngutman. +- Reply/skills: keep resolved skill and memory secret config stable through embedded reply runs so raw SecretRefs in secondary skill settings no longer crash replies when the gateway already has the live env. (#64249) Thanks @mbelinky. ## 2026.4.9 diff --git a/packages/memory-host-sdk/src/host/secret-input.ts b/packages/memory-host-sdk/src/host/secret-input.ts index b6a0b317f8c..4a995d4eba6 100644 --- a/packages/memory-host-sdk/src/host/secret-input.ts +++ b/packages/memory-host-sdk/src/host/secret-input.ts @@ -1,6 +1,8 @@ import { hasConfiguredSecretInput, normalizeResolvedSecretInputString, + normalizeSecretInputString, + resolveSecretInputRef, } from "../../../../src/config/types.secrets.js"; export function hasConfiguredMemorySecretInput(value: unknown): boolean { @@ -11,6 +13,13 @@ export function resolveMemorySecretInputString(params: { value: unknown; path: string; }): string | undefined { + const { ref } = resolveSecretInputRef({ value: params.value }); + if (ref?.source === "env") { + const envValue = normalizeSecretInputString(process.env[ref.id]); + if (envValue) { + return envValue; + } + } return normalizeResolvedSecretInputString({ value: params.value, path: params.path, diff --git a/src/agents/pi-embedded-runner/skills-runtime.test.ts b/src/agents/pi-embedded-runner/skills-runtime.test.ts index 54f6eddc8a2..91a32392c90 100644 --- a/src/agents/pi-embedded-runner/skills-runtime.test.ts +++ b/src/agents/pi-embedded-runner/skills-runtime.test.ts @@ -97,6 +97,46 @@ describe("resolveEmbeddedRunSkillEntries", () => { }); }); + it("prefers caller config when the active runtime snapshot still contains raw skill SecretRefs", () => { + const sourceConfig: OpenClawConfig = { + skills: { + entries: { + diffs: { + apiKey: { + source: "file", + provider: "default", + id: "/skills/entries/diffs/apiKey", + }, + }, + }, + }, + }; + const runtimeConfig: OpenClawConfig = structuredClone(sourceConfig); + const callerConfig: OpenClawConfig = { + skills: { + entries: { + diffs: { + apiKey: "resolved-key", + }, + }, + }, + }; + setRuntimeConfigSnapshot(runtimeConfig, sourceConfig); + + resolveEmbeddedRunSkillEntries({ + workspaceDir: "/tmp/workspace", + config: callerConfig, + skillsSnapshot: { + prompt: "skills prompt", + skills: [], + }, + }); + + expect(loadWorkspaceSkillEntriesSpy).toHaveBeenCalledWith("/tmp/workspace", { + config: callerConfig, + }); + }); + it("skips skill entry loading when resolved snapshot skills are present", () => { const snapshot: SkillSnapshot = { prompt: "skills prompt", diff --git a/src/agents/skills.test.ts b/src/agents/skills.test.ts index a4272b41596..3975fee893c 100644 --- a/src/agents/skills.test.ts +++ b/src/agents/skills.test.ts @@ -470,6 +470,85 @@ describe("applySkillEnvOverrides", () => { }); }); + it("prefers resolved caller skill config when the active runtime snapshot is still raw", async () => { + const workspaceDir = await makeWorkspace(); + await writeEnvSkill(workspaceDir); + + const entries = loadWorkspaceSkillEntries(workspaceDir, resolveTestSkillDirs(workspaceDir)); + const sourceConfig: OpenClawConfig = { + skills: { + entries: { + "env-skill": { + apiKey: { + source: "file", + provider: "default", + id: "/skills/entries/env-skill/apiKey", + }, + }, + }, + }, + }; + const callerConfig: OpenClawConfig = { + skills: { + entries: { + "env-skill": { + apiKey: "resolved-key", + }, + }, + }, + }; + setRuntimeConfigSnapshot(sourceConfig, sourceConfig); + + withClearedEnv(["ENV_KEY"], () => { + const restore = applySkillEnvOverrides({ + skills: entries, + config: callerConfig, + }); + + try { + expect(process.env.ENV_KEY).toBe("resolved-key"); + } finally { + restore(); + expect(process.env.ENV_KEY).toBeUndefined(); + } + }); + }); + + it("does not resolve raw skill apiKey refs when the host already provides primaryEnv", async () => { + const workspaceDir = await makeWorkspace(); + await writeEnvSkill(workspaceDir); + + const entries = loadWorkspaceSkillEntries(workspaceDir, resolveTestSkillDirs(workspaceDir)); + + withClearedEnv(["ENV_KEY"], () => { + process.env.ENV_KEY = "host-key"; + const restore = applySkillEnvOverrides({ + skills: entries, + config: { + skills: { + entries: { + "env-skill": { + apiKey: { + source: "env", + provider: "default", + id: "OPENAI_API_KEY", + }, + }, + }, + }, + }, + }); + + try { + expect(process.env.ENV_KEY).toBe("host-key"); + } finally { + restore(); + expect(process.env.ENV_KEY).toBe("host-key"); + delete process.env.ENV_KEY; + } + }); + }); + it("blocks unsafe env overrides but allows declared secrets", async () => { const workspaceDir = await makeWorkspace(); const skillDir = path.join(workspaceDir, "skills", "unsafe-env-skill"); diff --git a/src/agents/skills/env-overrides.ts b/src/agents/skills/env-overrides.ts index 1631526d13a..277877bb812 100644 --- a/src/agents/skills/env-overrides.ts +++ b/src/agents/skills/env-overrides.ts @@ -172,17 +172,17 @@ function applySkillConfigEnvOverrides(params: { } } - const resolvedApiKey = - normalizeResolvedSecretInputString({ - value: skillConfig.apiKey, - path: `skills.entries.${skillKey}.apiKey`, - }) ?? ""; const canInjectPrimaryEnv = normalizedPrimaryEnv && (process.env[normalizedPrimaryEnv] === undefined || activeSkillEnvEntries.has(normalizedPrimaryEnv)); - if (canInjectPrimaryEnv && resolvedApiKey) { - if (!pendingOverrides[normalizedPrimaryEnv]) { + if (canInjectPrimaryEnv && !pendingOverrides[normalizedPrimaryEnv]) { + const resolvedApiKey = + normalizeResolvedSecretInputString({ + value: skillConfig.apiKey, + path: `skills.entries.${skillKey}.apiKey`, + }) ?? ""; + if (resolvedApiKey) { pendingOverrides[normalizedPrimaryEnv] = resolvedApiKey; } } diff --git a/src/agents/skills/runtime-config.ts b/src/agents/skills/runtime-config.ts index 400454d8b6c..afee2a6ac7b 100644 --- a/src/agents/skills/runtime-config.ts +++ b/src/agents/skills/runtime-config.ts @@ -1,5 +1,34 @@ import { getRuntimeConfigSnapshot, type OpenClawConfig } from "../../config/config.js"; +import { coerceSecretRef } from "../../config/types.secrets.js"; + +function hasConfiguredSkillApiKeyRef(config?: OpenClawConfig): boolean { + const entries = config?.skills?.entries; + if (!entries || typeof entries !== "object") { + return false; + } + for (const skillConfig of Object.values(entries)) { + if (!skillConfig || typeof skillConfig !== "object") { + continue; + } + if (coerceSecretRef(skillConfig.apiKey) !== null) { + return true; + } + } + return false; +} export function resolveSkillRuntimeConfig(config?: OpenClawConfig): OpenClawConfig | undefined { - return getRuntimeConfigSnapshot() ?? config; + const runtimeConfig = getRuntimeConfigSnapshot(); + if (!runtimeConfig) { + return config; + } + if (!config) { + return runtimeConfig; + } + const runtimeHasRawSkillSecretRefs = hasConfiguredSkillApiKeyRef(runtimeConfig); + const configHasRawSkillSecretRefs = hasConfiguredSkillApiKeyRef(config); + if (runtimeHasRawSkillSecretRefs && !configHasRawSkillSecretRefs) { + return config; + } + return runtimeConfig; } diff --git a/src/memory-host-sdk/host/secret-input.test.ts b/src/memory-host-sdk/host/secret-input.test.ts new file mode 100644 index 00000000000..78de30cd519 --- /dev/null +++ b/src/memory-host-sdk/host/secret-input.test.ts @@ -0,0 +1,36 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { resolveMemorySecretInputString } from "./secret-input.js"; + +describe("resolveMemorySecretInputString", () => { + afterEach(() => { + delete process.env.GOOGLE_API_KEY; + }); + + it("uses the daemon env for env-backed SecretRefs", () => { + process.env.GOOGLE_API_KEY = "resolved-key"; + + expect( + resolveMemorySecretInputString({ + value: { + source: "env", + provider: "default", + id: "GOOGLE_API_KEY", + }, + path: "agents.main.memorySearch.remote.apiKey", + }), + ).toBe("resolved-key"); + }); + + it("still throws when an env-backed SecretRef is missing from the daemon env", () => { + expect(() => + resolveMemorySecretInputString({ + value: { + source: "env", + provider: "default", + id: "GOOGLE_API_KEY", + }, + path: "agents.main.memorySearch.remote.apiKey", + }), + ).toThrow(/unresolved SecretRef/); + }); +}); diff --git a/src/memory-host-sdk/host/secret-input.ts b/src/memory-host-sdk/host/secret-input.ts index 98dd0c87084..5a4bbfed13f 100644 --- a/src/memory-host-sdk/host/secret-input.ts +++ b/src/memory-host-sdk/host/secret-input.ts @@ -1,6 +1,8 @@ import { hasConfiguredSecretInput, normalizeResolvedSecretInputString, + normalizeSecretInputString, + resolveSecretInputRef, } from "../../config/types.secrets.js"; export function hasConfiguredMemorySecretInput(value: unknown): boolean { @@ -11,6 +13,13 @@ export function resolveMemorySecretInputString(params: { value: unknown; path: string; }): string | undefined { + const { ref } = resolveSecretInputRef({ value: params.value }); + if (ref?.source === "env") { + const envValue = normalizeSecretInputString(process.env[ref.id]); + if (envValue) { + return envValue; + } + } return normalizeResolvedSecretInputString({ value: params.value, path: params.path,