From 27ee5c00980101fc7f25a653d70aa830d830afa3 Mon Sep 17 00:00:00 2001 From: ziyitan Date: Mon, 27 Apr 2026 17:45:16 +0800 Subject: [PATCH] fix(gateway): redact secrets in skills.update response (#69998) Merged via squash. Prepared head SHA: 61fc06f33f29804dcc1c3ec90cbd66f1946df714 Co-authored-by: Ziy1-Tan <49604965+Ziy1-Tan@users.noreply.github.com> Co-authored-by: hxy91819 <8814856+hxy91819@users.noreply.github.com> Reviewed-by: @hxy91819 --- CHANGELOG.md | 1 + src/gateway/server-methods/skills.ts | 14 ++- .../skills.update.normalizes-api-key.test.ts | 115 +++++++++++++++++- 3 files changed, 123 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b21ef343bb5..0ed936692b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -190,6 +190,7 @@ Docs: https://docs.openclaw.ai - Plugins/memory-core: respect configured memory-search embedding concurrency during non-batch indexing so local Ollama embedding backends can serialize indexing instead of flooding the server. Fixes #66822. (#66931) Thanks @oliviareid-svg and @LyraInTheFlesh. - Docker/update smoke: keep the package-derived update-channel fixture on package-shipped files and make its UI build stub create the asset the updater verifies. Thanks @vincentkoc. - Gateway/models: repair legacy `models.providers.*.api = "openai"` config values to `openai-completions`, and skip providers with future stale API enum values during startup instead of bricking the gateway. Fixes #72477. (#72542) Thanks @JooyoungChoi14 and @obviyus. +- Gateway/skills: redact `apiKey` and secret-named `env` values from the `skills.update` RPC response to prevent leaking credentials into WebSocket traffic, client logs, or session transcripts. Config is still written to disk in full; only the response payload is redacted. (#69998) Thanks @Ziy1-Tan. ## 2026.4.26 diff --git a/src/gateway/server-methods/skills.ts b/src/gateway/server-methods/skills.ts index 4fd8994098d..0f5af972960 100644 --- a/src/gateway/server-methods/skills.ts +++ b/src/gateway/server-methods/skills.ts @@ -14,6 +14,7 @@ import { buildWorkspaceSkillStatus } from "../../agents/skills-status.js"; import { loadWorkspaceSkillEntries, type SkillEntry } from "../../agents/skills.js"; import { listAgentWorkspaceDirs } from "../../agents/workspace-dirs.js"; import { loadConfig, writeConfigFile } from "../../config/config.js"; +import { redactConfigObject, REDACTED_SENTINEL } from "../../config/redact-snapshot.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { fetchClawHubSkillDetail } from "../../infra/clawhub.js"; import { formatErrorMessage } from "../../infra/errors.js"; @@ -312,7 +313,9 @@ export const skillsHandlers: GatewayRequestHandlers = { } if (typeof p.apiKey === "string") { const trimmed = normalizeSecretInput(p.apiKey); - if (trimmed) { + if (trimmed === REDACTED_SENTINEL) { + // Keep the stored secret when a client round-trips a redacted response value. + } else if (trimmed) { current.apiKey = trimmed; } else { delete current.apiKey; @@ -326,6 +329,9 @@ export const skillsHandlers: GatewayRequestHandlers = { continue; } const trimmedVal = value.trim(); + if (trimmedVal === REDACTED_SENTINEL) { + continue; + } if (!trimmedVal) { delete nextEnv[trimmedKey]; } else { @@ -341,6 +347,10 @@ export const skillsHandlers: GatewayRequestHandlers = { skills, }; await writeConfigFile(nextConfig); - respond(true, { ok: true, skillKey: p.skillKey, config: current }, undefined); + respond( + true, + { ok: true, skillKey: p.skillKey, config: redactConfigObject(current) }, + undefined, + ); }, }; diff --git a/src/gateway/server-methods/skills.update.normalizes-api-key.test.ts b/src/gateway/server-methods/skills.update.normalizes-api-key.test.ts index 6476ad671d0..1dc80a801da 100644 --- a/src/gateway/server-methods/skills.update.normalizes-api-key.test.ts +++ b/src/gateway/server-methods/skills.update.normalizes-api-key.test.ts @@ -1,14 +1,16 @@ import { describe, expect, it, vi } from "vitest"; +import { REDACTED_SENTINEL } from "../../config/redact-snapshot.js"; let writtenConfig: unknown = null; +let loadedConfig: unknown = { + skills: { + entries: {}, + }, +}; vi.mock("../../config/config.js", () => { return { - loadConfig: () => ({ - skills: { - entries: {}, - }, - }), + loadConfig: () => loadedConfig, writeConfigFile: async (cfg: unknown) => { writtenConfig = cfg; }, @@ -20,6 +22,11 @@ const { skillsHandlers } = await import("./skills.js"); describe("skills.update", () => { it("strips embedded CR/LF from apiKey", async () => { writtenConfig = null; + loadedConfig = { + skills: { + entries: {}, + }, + }; let ok: boolean | null = null; let error: unknown = null; @@ -50,4 +57,102 @@ describe("skills.update", () => { }, }); }); + + it("redacts apiKey and secret env values from the response but writes full values to config", async () => { + writtenConfig = null; + loadedConfig = { + skills: { + entries: {}, + }, + }; + + let responseResult: unknown = null; + await skillsHandlers["skills.update"]({ + params: { + skillKey: "demo-skill", + apiKey: "secret-api-key-123", + env: { + GEMINI_API_KEY: "secret-env-key-456", + BRAVE_REGION: "us", + }, + }, + req: {} as never, + client: null as never, + isWebchatConnect: () => false, + context: {} as never, + respond: (_success, result, _err) => { + responseResult = result; + }, + }); + + // Full values must be persisted to config + expect(writtenConfig).toMatchObject({ + skills: { + entries: { + "demo-skill": { + apiKey: "secret-api-key-123", + env: { + GEMINI_API_KEY: "secret-env-key-456", + BRAVE_REGION: "us", + }, + }, + }, + }, + }); + + // Response must not expose plaintext secrets + const config = (responseResult as { config: Record }).config; + expect(config.apiKey).toBe(REDACTED_SENTINEL); + const env = config.env as Record; + expect(env.GEMINI_API_KEY).toBe(REDACTED_SENTINEL); + // Non-secret env values should still be present + expect(env.BRAVE_REGION).toBe("us"); + }); + + it("keeps existing secrets when clients submit redacted sentinel values", async () => { + writtenConfig = null; + loadedConfig = { + skills: { + entries: { + "demo-skill": { + apiKey: "secret-api-key-123", + env: { + GEMINI_API_KEY: "secret-env-key-456", + BRAVE_REGION: "us", + }, + }, + }, + }, + }; + + await skillsHandlers["skills.update"]({ + params: { + skillKey: "demo-skill", + apiKey: REDACTED_SENTINEL, + env: { + GEMINI_API_KEY: REDACTED_SENTINEL, + BRAVE_REGION: "eu", + }, + }, + req: {} as never, + client: null as never, + isWebchatConnect: () => false, + context: {} as never, + respond: () => {}, + }); + + expect(writtenConfig).toMatchObject({ + skills: { + entries: { + "demo-skill": { + apiKey: "secret-api-key-123", + env: { + GEMINI_API_KEY: "secret-env-key-456", + BRAVE_REGION: "eu", + }, + }, + }, + }, + }); + }); });