mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:30:42 +00:00
fix(gateway): redact secrets in skills.update response (#69998)
Merged via squash.
Prepared head SHA: 61fc06f33f
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
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
@@ -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<string, unknown> }).config;
|
||||
expect(config.apiKey).toBe(REDACTED_SENTINEL);
|
||||
const env = config.env as Record<string, string>;
|
||||
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",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user