fix(agents): redact Control UI tool payload secrets (#72319)

Fixes #72283.

- Redacts Control UI tool start args, partial/final result payloads, derived exec output, and patch summaries before event emission.
- Forces tool/UI payload redaction to include built-in patterns plus configured custom `logging.redactPatterns`.
- Covers object, details-only, primitive string, and top-level array tool-result shapes.

Tests:
- `pnpm test src/agents/pi-embedded-subscribe.tools.test.ts src/agents/pi-embedded-subscribe.handlers.tools.test.ts`
- `pnpm check:changed`

Co-authored-by: volcano303 <75143900+volcano303@users.noreply.github.com>
Co-authored-by: Val Alexander <bunsthedev@gmail.com>
This commit is contained in:
volcano303
2026-04-27 16:30:50 -06:00
committed by GitHub
parent 24c39de9c1
commit f3e8c50df3
6 changed files with 430 additions and 28 deletions

View File

@@ -16,6 +16,7 @@ Docs: https://docs.openclaw.ai
### Fixes
- Control UI/Agents: redact tool-call args, partial/final results, derived exec output, and configured custom secret patterns before streaming tool events to the Control UI, so tool output cannot expose provider or channel credentials. Fixes #72283. (#72319) Thanks @volcano303 and @BunsDev.
- CLI/model probes: fail local `infer model run` probes when the provider returns no text output, so unreachable local providers and empty completions no longer look like successful smoke tests. Refs #73023. Thanks @pavelyortho-cyber.
- CLI/Ollama: run local `infer model run` through the lean provider completion path and skip global model discovery for one-shot local probes, so Ollama smoke tests no longer pay full chat-agent/tool startup cost or hang before the native `/api/chat` request. Fixes #72851. Thanks @TotalRes2020.
- Doctor/gateway services: ignore launchd/systemd companion services that only reference the gateway as a dependency, suppress inactive Linux extra-service warnings, and avoid rewriting a running systemd gateway command/entrypoint during doctor repair. Carries forward #39118. Thanks @therk.

View File

@@ -1,5 +1,9 @@
import type { AgentEvent } from "@mariozechner/pi-agent-core";
import { describe, expect, it, vi } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import {
onAgentEvent as registerAgentEventListener,
resetAgentEventsForTest,
} from "../infra/agent-events.js";
import type { MessagingToolSend } from "./pi-embedded-messaging.types.js";
import {
handleToolExecutionEnd,
@@ -959,3 +963,147 @@ describe("messaging tool media URL tracking", () => {
expect(ctx.state.pendingMessagingMediaUrls.has("tool-m3")).toBe(false);
});
});
describe("control UI credential redaction (issue #72283)", () => {
afterEach(() => {
resetAgentEventsForTest();
});
it("redacts secrets in args before emitting the tool start event", async () => {
const events: Array<{ stream?: string; data?: Record<string, unknown> }> = [];
registerAgentEventListener((evt) => {
events.push(evt as never);
});
const { ctx } = createTestContext();
await handleToolExecutionStart(
ctx as never,
{
type: "tool_execution_start",
toolName: "gateway",
toolCallId: "tool-secret-args",
args: {
action: "config.apply",
raw: 'apiKey: "sk-1234567890abcdefXYZ"',
headers: { Authorization: "Bearer abcdef0123456789QWERTY=" },
},
} as never,
);
const startEvent = events.find(
(evt) => evt.stream === "tool" && (evt.data as { phase?: string })?.phase === "start",
);
expect(startEvent).toBeDefined();
const emittedArgs = (startEvent?.data as { args?: Record<string, unknown> })?.args ?? {};
const serialized = JSON.stringify(emittedArgs);
expect(serialized).not.toContain("sk-1234567890abcdefXYZ");
expect(serialized).not.toContain("abcdef0123456789QWERTY=");
expect(serialized).toContain("config.apply");
});
it("redacts secrets in exec aggregated stdout before emitting command_output", async () => {
const { ctx, onAgentEvent } = createTestContext();
await handleToolExecutionStart(
ctx as never,
{
type: "tool_execution_start",
toolName: "exec",
toolCallId: "tool-exec-secret",
args: { command: "cat ~/.openclaw/openclaw.json" },
} as never,
);
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "exec",
toolCallId: "tool-exec-secret",
isError: false,
result: {
details: {
status: "completed",
aggregated:
'OPENROUTER_API_KEY=sk-or-v1-abcdef0123456789\napiKey: "ghp_abcdefghij1234567890"',
exitCode: 0,
durationMs: 12,
cwd: "/tmp/work",
},
},
} as never,
);
const commandOutputCalls = onAgentEvent.mock.calls
.map((call) => call[0])
.filter((arg: unknown) => (arg as { stream?: string })?.stream === "command_output");
expect(commandOutputCalls.length).toBeGreaterThan(0);
const lastOutput = commandOutputCalls.at(-1) as { data?: { output?: string } } | undefined;
expect(lastOutput?.data?.output).toBeDefined();
expect(lastOutput?.data?.output).not.toContain("sk-or-v1-abcdef0123456789");
expect(lastOutput?.data?.output).not.toContain("ghp_abcdefghij1234567890");
expect(lastOutput?.data?.output).toContain("OPENROUTER_API_KEY=");
});
it("redacts details-only results before emitting the tool result event", async () => {
const events: Array<{ stream?: string; data?: Record<string, unknown> }> = [];
registerAgentEventListener((evt) => {
events.push(evt as never);
});
const { ctx } = createTestContext();
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "gateway",
toolCallId: "tool-details-secret",
isError: false,
result: {
details: {
config: { apiKey: "sk-1234567890abcdefXYZ", model: "gpt-4" },
},
},
} as never,
);
const resultEvent = events.find(
(evt) => evt.stream === "tool" && (evt.data as { phase?: string })?.phase === "result",
);
expect(resultEvent).toBeDefined();
const serialized = JSON.stringify(resultEvent?.data?.result);
expect(serialized).not.toContain("sk-1234567890abcdefXYZ");
expect(serialized).toContain("gpt-4");
});
it("redacts primitive string results before emitting the tool result event", async () => {
const events: Array<{ stream?: string; data?: Record<string, unknown> }> = [];
registerAgentEventListener((evt) => {
events.push(evt as never);
});
const { ctx } = createTestContext();
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "gateway",
toolCallId: "tool-string-secret",
isError: false,
result: "OPENROUTER_API_KEY=sk-or-v1-abcdef0123456789",
} as never,
);
const resultEvent = events.find(
(evt) => evt.stream === "tool" && (evt.data as { phase?: string })?.phase === "result",
);
expect(resultEvent).toBeDefined();
const emittedResult = resultEvent?.data?.result;
expect(typeof emittedResult).toBe("string");
if (typeof emittedResult !== "string") {
throw new Error("expected string result");
}
expect(emittedResult).not.toContain("sk-or-v1-abcdef0123456789");
expect(emittedResult).toContain("OPENROUTER_API_KEY=");
});
});

View File

@@ -34,6 +34,7 @@ import {
filterToolResultMediaUrls,
isToolResultError,
isToolResultTimedOut,
sanitizeToolArgs,
sanitizeToolResult,
} from "./pi-embedded-subscribe.tools.js";
import { inferToolMetaFromArgs } from "./pi-embedded-utils.js";
@@ -634,7 +635,7 @@ export function handleToolExecutionStart(
phase: "start",
name: toolName,
toolCallId,
args: args as Record<string, unknown>,
args: sanitizeToolArgs(args) as Record<string, unknown>,
},
});
const itemData: AgentItemEventData = {
@@ -945,7 +946,8 @@ export async function handleToolExecutionEnd(
});
if (isExecToolName(toolName)) {
const execDetails = readExecToolDetails(result);
// Use sanitizedResult so `aggregated` is redacted before reaching command_output.
const execDetails = readExecToolDetails(sanitizedResult);
const commandItemId = buildCommandItemId(toolCallId);
if (
execDetails?.status === "approval-pending" ||
@@ -1083,7 +1085,7 @@ export async function handleToolExecutionEnd(
}
if (isPatchToolName(toolName)) {
const patchSummary = readApplyPatchSummary(result);
const patchSummary = readApplyPatchSummary(sanitizedResult);
const patchItemId = buildPatchItemId(toolCallId);
const summaryText = patchSummary ? buildPatchSummaryText(patchSummary) : undefined;
emitTrackedItemEvent(ctx, {

View File

@@ -1,5 +1,14 @@
import { describe, expect, it } from "vitest";
import { extractToolErrorMessage } from "./pi-embedded-subscribe.tools.js";
import { afterEach, describe, expect, it, vi } from "vitest";
import * as loggingConfigModule from "../logging/config.js";
import {
extractToolErrorMessage,
sanitizeToolArgs,
sanitizeToolResult,
} from "./pi-embedded-subscribe.tools.js";
afterEach(() => {
vi.restoreAllMocks();
});
describe("extractToolErrorMessage", () => {
it("ignores non-error status values", () => {
@@ -34,3 +43,178 @@ describe("extractToolErrorMessage", () => {
).toBe("SYSTEM_RUN_DENIED: approval required");
});
});
function getTextContent(result: unknown, index = 0): string {
const record = result as { content: Array<{ text: string }> };
return record.content[index].text;
}
describe("sanitizeToolResult", () => {
it("redacts JSON-style apiKey fields in text content blocks", () => {
const result = {
content: [
{
type: "text",
text: '{"apiKey":"sk-1234567890abcdef","model":"gpt-4"}',
},
],
};
const text = getTextContent(sanitizeToolResult(result));
expect(text).not.toContain("sk-1234567890abcdef");
expect(text).toContain("model");
});
it("redacts ENV-style credential assignments", () => {
const result = {
content: [
{
type: "text",
text: "OPENROUTER_API_KEY=sk-or-v1-abcdef0123456789\nMODEL=gpt-4",
},
],
};
const text = getTextContent(sanitizeToolResult(result));
expect(text).not.toContain("sk-or-v1-abcdef0123456789");
expect(text).toContain("MODEL=gpt-4");
});
it("redacts Bearer authorization tokens", () => {
const result = {
content: [{ type: "text", text: "Authorization: Bearer abcdef0123456789QWERTY=" }],
};
const text = getTextContent(sanitizeToolResult(result));
expect(text).not.toContain("abcdef0123456789QWERTY=");
});
it("preserves image content stripping behavior", () => {
const result = {
content: [{ type: "image", data: "base64imagedata", mimeType: "image/png" }],
};
const sanitized = sanitizeToolResult(result) as {
content: Array<{ data?: string; bytes?: number; omitted?: boolean }>;
};
expect(sanitized.content[0].data).toBeUndefined();
expect(sanitized.content[0].omitted).toBe(true);
expect(sanitized.content[0].bytes).toBe("base64imagedata".length);
});
it("redacts secrets inside result.details (e.g. exec aggregated stdout)", () => {
const result = {
content: [{ type: "text", text: "ok" }],
details: {
status: "completed",
aggregated:
'OPENROUTER_API_KEY=sk-or-v1-abcdef0123456789\napiKey: "ghp_abcdefghij1234567890"',
exitCode: 0,
cwd: "/tmp/work",
},
};
const sanitized = sanitizeToolResult(result) as {
details: { status: string; aggregated: string; exitCode: number; cwd: string };
};
expect(sanitized.details.aggregated).not.toContain("sk-or-v1-abcdef0123456789");
expect(sanitized.details.aggregated).not.toContain("ghp_abcdefghij1234567890");
expect(sanitized.details.status).toBe("completed");
expect(sanitized.details.exitCode).toBe(0);
expect(sanitized.details.cwd).toBe("/tmp/work");
});
it("redacts secrets at the top level outside content/details", () => {
const result = {
output: "OPENROUTER_API_KEY=sk-or-v1-abcdef0123456789",
metadata: {
token: "ghp_abcdefghij1234567890ABCDEF",
nested: { auth: "Bearer abcdef0123456789QWERTY=" },
},
summary: "ok",
};
const sanitized = sanitizeToolResult(result) as {
output: string;
metadata: { token: string; nested: { auth: string } };
summary: string;
};
expect(sanitized.output).not.toContain("sk-or-v1-abcdef0123456789");
expect(sanitized.metadata.token).not.toContain("ghp_abcdefghij1234567890ABCDEF");
expect(sanitized.metadata.nested.auth).not.toContain("abcdef0123456789QWERTY=");
expect(sanitized.summary).toBe("ok");
});
it("redacts a details-only result with no content array", () => {
const result = {
details: {
config: { apiKey: "sk-1234567890abcdefXYZ", model: "gpt-4" },
},
};
const sanitized = sanitizeToolResult(result) as {
details: { config: { apiKey: string; model: string } };
};
expect(sanitized.details.config.apiKey).not.toContain("sk-1234567890abcdefXYZ");
expect(sanitized.details.config.model).toBe("gpt-4");
});
it("redacts primitive string results", () => {
const sanitized = sanitizeToolResult("OPENROUTER_API_KEY=sk-or-v1-abcdef0123456789") as string;
expect(sanitized).not.toContain("sk-or-v1-abcdef0123456789");
expect(sanitized).toContain("OPENROUTER_API_KEY=");
});
it("preserves top-level arrays while redacting nested strings", () => {
const sanitized = sanitizeToolResult([
{ output: "Authorization: Bearer abcdef0123456789QWERTY=" },
"apiKey=sk-1234567890abcdefXYZ",
]) as Array<{ output: string } | string>;
expect(Array.isArray(sanitized)).toBe(true);
expect(JSON.stringify(sanitized)).not.toContain("abcdef0123456789QWERTY=");
expect(JSON.stringify(sanitized)).not.toContain("sk-1234567890abcdefXYZ");
expect((sanitized[0] as { output: string }).output).toContain("Authorization: Bearer");
});
it("applies configured redact patterns to Control UI tool payloads", () => {
vi.spyOn(loggingConfigModule, "readLoggingConfig").mockReturnValue({
redactSensitive: "off",
redactPatterns: [String.raw`\bcustom-secret-[A-Za-z0-9]+\b`],
});
const result = {
content: [{ type: "text", text: "value custom-secret-abc123" }],
};
const text = getTextContent(sanitizeToolResult(result));
expect(text).not.toContain("custom-secret-abc123");
expect(text).toContain("custom…c123");
});
});
describe("sanitizeToolArgs", () => {
it("redacts string-valued credentials nested anywhere in args", () => {
const args = {
apiKey: "sk-1234567890abcdefXYZ",
headers: { Authorization: "Bearer abcdef0123456789QWERTY=" },
command: "OPENROUTER_API_KEY=sk-or-v1-abcdef0123456789 ./run.sh",
flags: ["--api-key", "sk-1234567890abcdefXYZ"],
};
const sanitized = sanitizeToolArgs(args) as {
apiKey: string;
headers: { Authorization: string };
command: string;
flags: string[];
};
expect(sanitized.apiKey).not.toContain("sk-1234567890abcdefXYZ");
expect(sanitized.headers.Authorization).not.toContain("abcdef0123456789QWERTY=");
expect(sanitized.command).not.toContain("sk-or-v1-abcdef0123456789");
expect(sanitized.flags.join(" ")).not.toContain("sk-1234567890abcdefXYZ");
expect(sanitized.flags[0]).toBe("--api-key");
});
it("passes through null/undefined and non-string primitives unchanged", () => {
expect(sanitizeToolArgs(undefined)).toBeUndefined();
expect(sanitizeToolArgs(null)).toBeNull();
expect(sanitizeToolArgs(42)).toBe(42);
expect(sanitizeToolArgs({ count: 3, file_path: "/tmp/x.txt" })).toEqual({
count: 3,
file_path: "/tmp/x.txt",
});
});
});

View File

@@ -1,5 +1,6 @@
import { getChannelPlugin, normalizeChannelId } from "../channels/plugins/index.js";
import { normalizeTargetForProvider } from "../infra/outbound/target-normalization.js";
import { redactToolPayloadText } from "../logging/redact.js";
import { splitMediaFromOutput } from "../media/parse.js";
import { pluginRegistrationContractRegistry } from "../plugins/contracts/registry.js";
import {
@@ -114,34 +115,84 @@ function isHostDenialToolText(text: string): boolean {
return normalized.toLowerCase().includes("approval cannot safely bind");
}
function redactStringsDeep(value: unknown, seen = new WeakSet<object>()): unknown {
if (typeof value === "string") {
return redactToolPayloadText(value);
}
if (Array.isArray(value)) {
if (seen.has(value)) {
return "[Circular]";
}
seen.add(value);
return value.map((item) => redactStringsDeep(item, seen));
}
if (value && typeof value === "object") {
if (seen.has(value)) {
return "[Circular]";
}
seen.add(value);
const out: Record<string, unknown> = {};
for (const [key, child] of Object.entries(value as Record<string, unknown>)) {
out[key] = redactStringsDeep(child, seen);
}
return out;
}
return value;
}
export function sanitizeToolArgs(args: unknown): unknown {
return redactStringsDeep(args);
}
export function sanitizeToolResult(result: unknown): unknown {
if (typeof result === "string") {
return redactToolPayloadText(result);
}
if (Array.isArray(result)) {
return redactStringsDeep(result);
}
if (!result || typeof result !== "object") {
return result;
}
const record = result as Record<string, unknown>;
const content = Array.isArray(record.content) ? record.content : null;
if (!content) {
return record;
// Strip image data first so the deep redaction pass doesn't waste work
// scanning base64 payloads (and so we capture the original byte counts).
const preCleaned: Record<string, unknown> = { ...record };
const originalContent = Array.isArray(record.content) ? record.content : null;
if (originalContent) {
preCleaned.content = originalContent.map((item) => {
if (!item || typeof item !== "object") {
return item;
}
const entry = item as Record<string, unknown>;
if (readStringValue(entry.type) === "image") {
const data = readStringValue(entry.data);
const bytes = data ? data.length : undefined;
const cleaned = { ...entry };
delete cleaned.data;
return Object.assign({}, cleaned, { bytes, omitted: true });
}
return entry;
});
}
const sanitized = content.map((item) => {
if (!item || typeof item !== "object") {
return item;
}
const entry = item as Record<string, unknown>;
const type = readStringValue(entry.type);
if (type === "text" && typeof entry.text === "string") {
return Object.assign({}, entry, { text: truncateToolText(entry.text) });
}
if (type === "image") {
const data = readStringValue(entry.data);
const bytes = data ? data.length : undefined;
const cleaned = { ...entry };
delete cleaned.data;
return Object.assign({}, cleaned, { bytes, omitted: true });
}
return entry;
});
return { ...record, content: sanitized };
// Deep-redact the entire result so any top-level or nested string is
// protected, not just `details` and text content blocks.
const baseline = redactStringsDeep(preCleaned) as Record<string, unknown>;
const out: Record<string, unknown> = { ...baseline };
const content = Array.isArray(baseline.content) ? baseline.content : null;
if (content) {
out.content = content.map((item) => {
if (!item || typeof item !== "object") {
return item;
}
const entry = item as Record<string, unknown>;
if (readStringValue(entry.type) === "text" && typeof entry.text === "string") {
return Object.assign({}, entry, { text: truncateToolText(entry.text) });
}
return entry;
});
}
return out;
}
export function extractToolResultText(result: unknown): string | undefined {

View File

@@ -165,6 +165,22 @@ export function redactToolDetail(detail: string): string {
return redactSensitiveText(detail, resolved);
}
// Forces tools-mode regardless of `logging.redactSensitive` (which governs log
// output, not UI surfaces), and merges user `logging.redactPatterns` with the
// built-in defaults so both apply.
export function redactToolPayloadText(text: string): string {
if (!text) {
return text;
}
const cfg = readLoggingConfig();
const userPatterns = cfg?.redactPatterns;
const patterns =
userPatterns && userPatterns.length > 0
? [...userPatterns, ...DEFAULT_REDACT_PATTERNS]
: undefined;
return redactSensitiveText(text, { mode: "tools", patterns });
}
export function getDefaultRedactPatterns(): string[] {
return [...DEFAULT_REDACT_PATTERNS];
}