fix(mcp): keep server config private in groups (#103502)

* fix(mcp): keep server config private in groups

* fix(config): redact underscore credential flags

* fix(config): redact plural credential flags
This commit is contained in:
Peter Steinberger
2026-07-10 08:09:48 +01:00
committed by GitHub
parent a97633a9ad
commit ce47e6ac53
9 changed files with 691 additions and 210 deletions
+5
View File
@@ -402,6 +402,11 @@ updates persist across restarts.
```
`/mcp` stores config in OpenClaw config, not embedded-agent project settings.
`/mcp show` redacts credential-bearing fields, recognized credential flag
values, and known secret-shaped arguments. When run from a group, the
configuration is sent to the owner privately; if no private owner route is
available, the command fails closed and asks the owner to retry from a direct
chat.
## `/debug`: runtime-only overrides
+139 -7
View File
@@ -4,7 +4,7 @@ import type { OpenClawConfig } from "../../config/config.js";
import { withTempHome } from "../../config/home-env.test-harness.js";
import { REDACTED_SENTINEL } from "../../config/redact-snapshot.js";
import { createCommandWorkspaceHarness } from "./commands-filesystem.test-support.js";
import { handleMcpCommand } from "./commands-mcp.js";
import { createMcpCommandHandler, handleMcpCommand } from "./commands-mcp.js";
import { buildCommandTestParams } from "./commands.test-harness.js";
const mcpServers = vi.hoisted(() => new Map<string, Record<string, unknown>>());
@@ -164,14 +164,44 @@ describe("handleCommands /mcp", () => {
});
});
it("redacts credential-bearing headers and env from /mcp show in groups", async () => {
it("routes group /mcp show privately and redacts the delivered config", async () => {
await withTempHome("openclaw-command-mcp-home-", async () => {
const workspaceDir = await workspaceHarness.createWorkspace();
const privateReplies: string[] = [];
const groupHandler = createMcpCommandHandler({
resolvePrivateMcpTargets: async () => [{ channel: "telegram", to: "owner-1" }],
deliverPrivateMcpReply: async ({ reply }) => {
privateReplies.push(reply.text ?? "");
return true;
},
});
const headerSecret = "Bearer sk-test-secret-value";
const envSecret = "stdio-process-token-value";
const separateArgSecret = "plain-separate-arg-secret";
const inlineArgSecret = "plain-inline-arg-secret";
const positionalArgSecret = "ghp_realgithubtoken1234567890ABCD";
const secretKeyArg = "opaque-secret-key-value";
const awsSecretAccessKeyArg = "opaque-aws-secret-access-key-value";
const underscoreApiKeyArg = "opaque-underscore-api-key-value";
const pluralCredentialsArg = "opaque-plural-credentials-value";
mcpServers.set("billing-server", {
command: "uvx",
args: ["billing-mcp"],
args: [
"billing-mcp",
"--api-key",
separateArgSecret,
`--token=${inlineArgSecret}`,
positionalArgSecret,
"--secret-key",
secretKeyArg,
`--aws-secret-access-key=${awsSecretAccessKeyArg}`,
"--openai_api_key",
underscoreApiKeyArg,
"--credentials",
pluralCredentialsArg,
"--region",
"us-east-1",
],
transport: "streamable-http",
url: "https://billing.example.com/mcp",
headers: {
@@ -199,13 +229,34 @@ describe("handleCommands /mcp", () => {
);
namedParams.command.senderIsOwner = true;
namedParams.isGroup = true;
const namedResult = expectMcpResult(await handleMcpCommand(namedParams, true));
const namedText = namedResult.reply?.text ?? "";
const namedResult = expectMcpResult(await groupHandler(namedParams, true));
const namedGroupText = namedResult.reply?.text ?? "";
expect(namedGroupText).toContain("sent the details to the owner privately");
expect(namedGroupText).not.toContain("billing-server");
expect(namedGroupText).not.toContain("/tmp/openclaw.json");
expect(namedGroupText).not.toContain(headerSecret);
expect(privateReplies).toHaveLength(1);
const namedText = privateReplies[0] ?? "";
expect(namedText).toContain('MCP server "billing-server"');
expect(namedText).toContain('"command": "uvx"');
expect(namedText).toContain('"billing-mcp"');
expect(namedText).toContain('"--api-key"');
expect(namedText).toContain(`"--token=${REDACTED_SENTINEL}"`);
expect(namedText).toContain('"--secret-key"');
expect(namedText).toContain(`"--aws-secret-access-key=${REDACTED_SENTINEL}"`);
expect(namedText).toContain('"--openai_api_key"');
expect(namedText).toContain('"--region"');
expect(namedText).toContain('"us-east-1"');
expect(namedText).toContain(REDACTED_SENTINEL);
expect(namedText).not.toContain(headerSecret);
expect(namedText).not.toContain(envSecret);
expect(namedText).not.toContain(separateArgSecret);
expect(namedText).not.toContain(inlineArgSecret);
expect(namedText).not.toContain(positionalArgSecret);
expect(namedText).not.toContain(secretKeyArg);
expect(namedText).not.toContain(awsSecretAccessKeyArg);
expect(namedText).not.toContain(underscoreApiKeyArg);
expect(namedText).not.toContain(pluralCredentialsArg);
expect(namedText).not.toContain("sk-test-secret-value");
const allParams = buildCommandTestParams("/mcp show", buildCfg(), undefined, {
@@ -213,14 +264,95 @@ describe("handleCommands /mcp", () => {
});
allParams.command.senderIsOwner = true;
allParams.isGroup = true;
const allResult = expectMcpResult(await handleMcpCommand(allParams, true));
const allText = allResult.reply?.text ?? "";
const allResult = expectMcpResult(await groupHandler(allParams, true));
const allGroupText = allResult.reply?.text ?? "";
expect(allGroupText).toContain("sent the details to the owner privately");
expect(allGroupText).not.toContain("billing-server");
expect(allGroupText).not.toContain("/tmp/openclaw.json");
expect(privateReplies).toHaveLength(2);
const allText = privateReplies[1] ?? "";
expect(allText).toContain('"billing-server"');
expect(allText).toContain('"local-tools"');
expect(allText).toContain(REDACTED_SENTINEL);
expect(allText).not.toContain(headerSecret);
expect(allText).not.toContain(envSecret);
expect(allText).not.toContain(separateArgSecret);
expect(allText).not.toContain(inlineArgSecret);
expect(allText).not.toContain(positionalArgSecret);
expect(allText).not.toContain(secretKeyArg);
expect(allText).not.toContain(awsSecretAccessKeyArg);
expect(allText).not.toContain(underscoreApiKeyArg);
expect(allText).not.toContain(pluralCredentialsArg);
expect(allText).not.toContain("local-env-secret-value");
});
});
it.each([
{
name: "no private owner target",
resolvePrivateMcpTargets: async () => [],
deliverPrivateMcpReply: async () => true,
},
{
name: "private delivery failure",
resolvePrivateMcpTargets: async () => [{ channel: "telegram", to: "owner-1" }],
deliverPrivateMcpReply: async () => false,
},
])("fails closed for group /mcp show with $name", async (route) => {
await withTempHome("openclaw-command-mcp-home-", async () => {
const workspaceDir = await workspaceHarness.createWorkspace();
const secret = "group-route-secret-value";
mcpServers.set("billing-server", {
command: "uvx",
args: ["billing-mcp", "--api-key", secret],
});
const handler = createMcpCommandHandler(route);
const params = buildCommandTestParams("/mcp show billing-server", buildCfg(), undefined, {
workspaceDir,
});
params.command.senderIsOwner = true;
params.isGroup = true;
const result = expectMcpResult(await handler(params, true));
const groupText = result.reply?.text ?? "";
expect(groupText).toContain("Run /mcp show from an owner DM");
expect(groupText).not.toContain("billing-server");
expect(groupText).not.toContain("/tmp/openclaw.json");
expect(groupText).not.toContain(secret);
});
});
it("tries later private owner routes without exposing config to the group", async () => {
await withTempHome("openclaw-command-mcp-home-", async () => {
const workspaceDir = await workspaceHarness.createWorkspace();
const attemptedTargets: string[] = [];
mcpServers.set("billing-server", {
command: "uvx",
args: ["billing-mcp", "--api-key", "private-route-secret"],
});
const handler = createMcpCommandHandler({
resolvePrivateMcpTargets: async () => [
{ channel: "telegram", to: "stale-owner-route" },
{ channel: "signal", to: "working-owner-route" },
],
deliverPrivateMcpReply: async ({ targets }) => {
const target = targets[0]?.to ?? "";
attemptedTargets.push(target);
return target === "working-owner-route";
},
});
const params = buildCommandTestParams("/mcp show billing-server", buildCfg(), undefined, {
workspaceDir,
});
params.command.senderIsOwner = true;
params.isGroup = true;
const result = expectMcpResult(await handler(params, true));
const groupText = result.reply?.text ?? "";
expect(attemptedTargets).toEqual(["stale-owner-route", "working-owner-route"]);
expect(groupText).toContain("sent the details to the owner privately");
expect(groupText).not.toContain("billing-server");
expect(groupText).not.toContain("private-route-secret");
});
});
});
+215 -97
View File
@@ -1,152 +1,270 @@
/** Handles /mcp commands for showing and mutating configured MCP servers. */
import { resolveSessionAgentId } from "../../agents/agent-scope.js";
import {
listConfiguredMcpServers,
setConfiguredMcpServer,
unsetConfiguredMcpServer,
} from "../../config/mcp-config.js";
import { redactConfigObject } from "../../config/redact-snapshot.js";
import { redactSensitiveArgv } from "../../config/redact-argv.js";
import { REDACTED_SENTINEL, redactConfigObject } from "../../config/redact-snapshot.js";
import { buildConfigSchema } from "../../config/schema.js";
import type { ExecApprovalRequest } from "../../infra/exec-approvals.js";
import type { ReplyPayload } from "../types.js";
import {
rejectNonOwnerCommand,
rejectUnauthorizedCommand,
requireCommandFlagEnabled,
requireGatewayClientScope,
} from "./command-gates.js";
import type { CommandHandler } from "./commands-types.js";
import {
deliverPrivateCommandReply,
readCommandDeliveryTarget,
readCommandMessageThreadId,
resolvePrivateCommandApprovalRouteExpiresAtMs,
resolvePrivateCommandRouteTargets,
type PrivateCommandRouteTarget,
} from "./commands-private-route.js";
import type { CommandHandler, HandleCommandsParams } from "./commands-types.js";
import { parseMcpCommand } from "./mcp-commands.js";
const MCP_SHOW_PRIVATE_ROUTE_UNAVAILABLE =
"I couldn't find a private owner route for MCP configuration. Run /mcp show from an owner DM so sensitive server details are not posted in this chat.";
const MCP_SHOW_PRIVATE_ROUTE_ACK =
"MCP server configuration is sensitive. I sent the details to the owner privately.";
type McpCommandDeps = {
resolvePrivateMcpTargets: (params: HandleCommandsParams) => Promise<PrivateCommandRouteTarget[]>;
deliverPrivateMcpReply: (params: {
commandParams: HandleCommandsParams;
targets: PrivateCommandRouteTarget[];
reply: ReplyPayload;
}) => Promise<boolean>;
};
const defaultMcpCommandDeps: McpCommandDeps = {
resolvePrivateMcpTargets: resolvePrivateMcpTargetsForCommand,
deliverPrivateMcpReply: deliverPrivateCommandReply,
};
function renderJsonBlock(label: string, value: unknown): string {
return `${label}\n\`\`\`json\n${JSON.stringify(value, null, 2)}\n\`\`\``;
}
/** Redact MCP server secrets (headers/env/url material) before chat display. */
function redactMcpServerArgsForDisplay(server: unknown): unknown {
if (!server || typeof server !== "object" || Array.isArray(server)) {
return server;
}
const record = server as Record<string, unknown>;
if (!Array.isArray(record.args) || !record.args.every((arg) => typeof arg === "string")) {
return server;
}
return {
...record,
args: redactSensitiveArgv(record.args, REDACTED_SENTINEL),
};
}
/** Redact MCP server secrets before chat display. */
function redactMcpServersForDisplay(servers: Record<string, unknown>): Record<string, unknown> {
const redactedRoot = redactConfigObject({ mcp: { servers } }, buildConfigSchema().uiHints) as {
const argvRedacted = Object.fromEntries(
Object.entries(servers).map(([name, server]) => [name, redactMcpServerArgsForDisplay(server)]),
);
const redactedRoot = redactConfigObject(
{ mcp: { servers: argvRedacted } },
buildConfigSchema().uiHints,
) as {
mcp?: { servers?: Record<string, unknown> };
};
return redactedRoot.mcp?.servers ?? {};
}
/** Command handler for /mcp show/set/unset operations. */
export const handleMcpCommand: CommandHandler = async (params, allowTextCommands) => {
if (!allowTextCommands) {
return null;
async function buildMcpShowReply(name?: string): Promise<ReplyPayload> {
const loaded = await listConfiguredMcpServers();
if (!loaded.ok) {
return { text: `⚠️ ${loaded.error}` };
}
const mcpCommand = parseMcpCommand(params.command.commandBodyNormalized);
if (!mcpCommand) {
return null;
}
const unauthorized = rejectUnauthorizedCommand(params, "/mcp");
if (unauthorized) {
return unauthorized;
}
const nonOwner = rejectNonOwnerCommand(params, "/mcp");
if (nonOwner) {
return nonOwner;
}
const disabled = requireCommandFlagEnabled(params.cfg, {
label: "/mcp",
configKey: "mcp",
});
if (disabled) {
return disabled;
}
if (mcpCommand.action === "error") {
if (name) {
const server = loaded.mcpServers[name];
if (!server) {
return { text: `🔌 No MCP server named "${name}" in ${loaded.path}.` };
}
const redactedServer = redactMcpServersForDisplay({
[name]: server,
})[name];
return {
shouldContinue: false,
reply: { text: `⚠️ ${mcpCommand.message}` },
text: renderJsonBlock(`🔌 MCP server "${name}" (${loaded.path})`, redactedServer),
};
}
if (Object.keys(loaded.mcpServers).length === 0) {
return { text: `🔌 No MCP servers configured in ${loaded.path}.` };
}
return {
text: renderJsonBlock(
`🔌 MCP servers (${loaded.path})`,
redactMcpServersForDisplay(loaded.mcpServers),
),
};
}
if (mcpCommand.action === "show") {
const loaded = await listConfiguredMcpServers();
if (!loaded.ok) {
async function resolvePrivateMcpTargetsForCommand(
params: HandleCommandsParams,
): Promise<PrivateCommandRouteTarget[]> {
return await resolvePrivateCommandRouteTargets({
commandParams: params,
request: buildMcpShowPrivateRouteRequest(params),
});
}
function buildMcpShowPrivateRouteRequest(params: HandleCommandsParams): ExecApprovalRequest {
const now = Date.now();
const agentId =
params.agentId ??
resolveSessionAgentId({
sessionKey: params.sessionKey,
config: params.cfg,
});
return {
id: "mcp-show-private-route",
request: {
command: params.command.commandBodyNormalized,
agentId,
...(params.sessionKey ? { sessionKey: params.sessionKey } : {}),
turnSourceChannel: params.command.channel,
turnSourceTo: readCommandDeliveryTarget(params) ?? null,
turnSourceAccountId: params.ctx.AccountId ?? null,
turnSourceThreadId: readCommandMessageThreadId(params) ?? null,
},
createdAtMs: now,
expiresAtMs: resolvePrivateCommandApprovalRouteExpiresAtMs(now),
};
}
async function deliverGroupMcpShowReplyPrivately(
deps: McpCommandDeps,
params: HandleCommandsParams,
name?: string,
) {
const targets = await deps.resolvePrivateMcpTargets(params);
if (targets.length === 0) {
return {
shouldContinue: false,
reply: { text: MCP_SHOW_PRIVATE_ROUTE_UNAVAILABLE },
};
}
const privateReply = await buildMcpShowReply(name);
for (const target of targets) {
if (
await deps.deliverPrivateMcpReply({
commandParams: params,
targets: [target],
reply: privateReply,
})
) {
return {
shouldContinue: false,
reply: { text: `⚠️ ${loaded.error}` },
reply: { text: MCP_SHOW_PRIVATE_ROUTE_ACK },
};
}
if (mcpCommand.name) {
const server = loaded.mcpServers[mcpCommand.name];
if (!server) {
}
return {
shouldContinue: false,
reply: { text: MCP_SHOW_PRIVATE_ROUTE_UNAVAILABLE },
};
}
/** Creates an MCP command handler with injectable private-route dependencies. */
export function createMcpCommandHandler(deps: Partial<McpCommandDeps> = {}): CommandHandler {
const resolvedDeps: McpCommandDeps = {
...defaultMcpCommandDeps,
...deps,
};
return async (params, allowTextCommands) => {
if (!allowTextCommands) {
return null;
}
const mcpCommand = parseMcpCommand(params.command.commandBodyNormalized);
if (!mcpCommand) {
return null;
}
const unauthorized = rejectUnauthorizedCommand(params, "/mcp");
if (unauthorized) {
return unauthorized;
}
const nonOwner = rejectNonOwnerCommand(params, "/mcp");
if (nonOwner) {
return nonOwner;
}
const disabled = requireCommandFlagEnabled(params.cfg, {
label: "/mcp",
configKey: "mcp",
});
if (disabled) {
return disabled;
}
if (mcpCommand.action === "error") {
return {
shouldContinue: false,
reply: { text: `⚠️ ${mcpCommand.message}` },
};
}
if (mcpCommand.action === "show") {
if (params.isGroup) {
return await deliverGroupMcpShowReplyPrivately(resolvedDeps, params, mcpCommand.name);
}
return {
shouldContinue: false,
reply: await buildMcpShowReply(mcpCommand.name),
};
}
const missingAdminScope = requireGatewayClientScope(params, {
label: "/mcp write",
allowedScopes: ["operator.admin"],
missingText: "❌ /mcp set|unset requires operator.admin for gateway clients.",
});
if (missingAdminScope) {
return missingAdminScope;
}
if (mcpCommand.action === "set") {
const result = await setConfiguredMcpServer({
name: mcpCommand.name,
server: mcpCommand.value,
});
if (!result.ok) {
return {
shouldContinue: false,
reply: { text: `🔌 No MCP server named "${mcpCommand.name}" in ${loaded.path}.` },
reply: { text: `⚠️ ${result.error}` },
};
}
const redactedServer = redactMcpServersForDisplay({
[mcpCommand.name]: server,
})[mcpCommand.name];
return {
shouldContinue: false,
reply: {
text: renderJsonBlock(
`🔌 MCP server "${mcpCommand.name}" (${loaded.path})`,
redactedServer,
),
text: `🔌 MCP server "${mcpCommand.name}" saved to ${result.path}.`,
},
};
}
if (Object.keys(loaded.mcpServers).length === 0) {
return {
shouldContinue: false,
reply: { text: `🔌 No MCP servers configured in ${loaded.path}.` },
};
}
return {
shouldContinue: false,
reply: {
text: renderJsonBlock(
`🔌 MCP servers (${loaded.path})`,
redactMcpServersForDisplay(loaded.mcpServers),
),
},
};
}
const missingAdminScope = requireGatewayClientScope(params, {
label: "/mcp write",
allowedScopes: ["operator.admin"],
missingText: "❌ /mcp set|unset requires operator.admin for gateway clients.",
});
if (missingAdminScope) {
return missingAdminScope;
}
if (mcpCommand.action === "set") {
const result = await setConfiguredMcpServer({
name: mcpCommand.name,
server: mcpCommand.value,
});
const result = await unsetConfiguredMcpServer({ name: mcpCommand.name });
if (!result.ok) {
return {
shouldContinue: false,
reply: { text: `⚠️ ${result.error}` },
};
}
if (!result.removed) {
return {
shouldContinue: false,
reply: { text: `🔌 No MCP server named "${mcpCommand.name}" in ${result.path}.` },
};
}
return {
shouldContinue: false,
reply: {
text: `🔌 MCP server "${mcpCommand.name}" saved to ${result.path}.`,
},
reply: { text: `🔌 MCP server "${mcpCommand.name}" removed from ${result.path}.` },
};
}
const result = await unsetConfiguredMcpServer({ name: mcpCommand.name });
if (!result.ok) {
return {
shouldContinue: false,
reply: { text: `⚠️ ${result.error}` },
};
}
if (!result.removed) {
return {
shouldContinue: false,
reply: { text: `🔌 No MCP server named "${mcpCommand.name}" in ${result.path}.` },
};
}
return {
shouldContinue: false,
reply: { text: `🔌 MCP server "${mcpCommand.name}" removed from ${result.path}.` },
};
};
}
/** Command handler for /mcp show/set/unset operations. */
export const handleMcpCommand: CommandHandler = createMcpCommandHandler();
+18
View File
@@ -348,6 +348,15 @@ describe("config io audit helpers", () => {
"--rotated-signing-key=PEM-LIKE-MATERIAL",
"--ops-master-key",
"ABCDEF1234567890",
"--secret-key",
"opaque-secret-key",
"--aws-secret-access-key=opaque-aws-secret",
"--openai_api_key",
"opaque-underscore-key",
"--aws_secret_access_key=opaque-underscore-aws-secret",
"--credentials",
"opaque-plural-credential",
"--service-credentials=opaque-inline-plural-credential",
];
const result = redactConfigAuditArgv(argv);
expect(result).toEqual([
@@ -358,6 +367,15 @@ describe("config io audit helpers", () => {
"--rotated-signing-key=***",
"--ops-master-key",
"***",
"--secret-key",
"***",
"--aws-secret-access-key=***",
"--openai_api_key",
"***",
"--aws_secret_access_key=***",
"--credentials",
"***",
"--service-credentials=***",
]);
});
+3 -99
View File
@@ -1,109 +1,13 @@
// Audits config paths and values for diagnostics and safety checks.
import path from "node:path";
import { redactSecrets, redactToolPayloadText } from "../logging/redact.js";
import { redactSecrets } from "../logging/redact.js";
import { resolveStateDir } from "./paths.js";
import { redactSensitiveArgv } from "./redact-argv.js";
const CONFIG_AUDIT_ARGV_CAP = 8;
// Conservative list of credential-bearing flags. The heuristic suffix
// classifier below catches the long tail (`--custom-api-key`,
// `--alibaba-model-studio-api-key`, plugin-defined `cliFlag` values, etc.)
// without needing every name enumerated here.
const SECRET_FLAG_NAMES = new Set([
"--token",
"--api-key",
"--apikey",
"--secret",
"--password",
"--passwd",
"--auth-token",
"--access-token",
"--refresh-token",
"--client-secret",
"--hook-token",
"--gateway-token",
"--bot-token",
"--app-token",
"--remote-token",
"--push-token",
"--webhook-secret",
"--webhook-token",
"--service-account-token",
"--op-service-account-token",
"--bearer",
"--bearer-token",
"--pat",
"--personal-access-token",
"--oauth-token",
"--id-token",
"--identity-token",
"--session-token",
"--service-token",
"--private-key",
"--recovery-key",
"--gateway-key",
"--session-key",
"--active-key",
]);
// Suffix-based heuristic. Any `--…-(token|secret|password|passwd|api-key|
// apikey|api-secret|webhook|credential|bearer|pat|private-key|recovery-key|
// signing-key|encryption-key|master-key|session-key|gateway-key|service-key|
// hook-key)` is treated as a secret flag in addition to the explicit list.
// The leading `--` is required so we don't mismatch arbitrary positional args.
const SECRET_FLAG_SUFFIX_PATTERN =
/^--(?:[a-z0-9]+(?:-[a-z0-9]+)*-)?(?:token|secret|password|passwd|api[-_]?key|api[-_]?secret|webhook|credential|bearer|pat|private[-_]?key|recovery[-_]?key|signing[-_]?key|encryption[-_]?key|master[-_]?key|session[-_]?key|gateway[-_]?key|service[-_]?key|hook[-_]?key)$/;
function isSecretFlagName(flagName: string): boolean {
if (SECRET_FLAG_NAMES.has(flagName)) {
return true;
}
return SECRET_FLAG_SUFFIX_PATTERN.test(flagName);
}
function parseFlagName(arg: string): string | null {
if (!arg.startsWith("--")) {
return null;
}
const eq = arg.indexOf("=");
return (eq === -1 ? arg : arg.slice(0, eq)).toLowerCase();
}
// Redacts CLI argv before it lands in the persistent config-audit log.
// Layers, applied per element:
// 1. `--flag=value` form for any name matching the explicit list or the
// suffix heuristic — mask the value half.
// 2. value following a bare `--flag` form — emit `***` instead of the
// next arg, even if it starts with `-`. Command parsers accept
// dash-leading values for required options, and this persistent audit
// log should fail closed.
// 3. fall back to redactToolPayloadText for everything else, which catches
// `KEY=VALUE` env-style assignments, raw token shapes (sk-, ghp_, xox*,
// gsk_, AIza*, npm_, Telegram bot tokens, PEM blocks, Bearer headers,
// URL query secrets) using the shared redaction patterns.
export function redactConfigAuditArgv(argv: readonly string[]): string[] {
const result: string[] = [];
let redactNext = false;
for (const current of argv) {
if (redactNext) {
redactNext = false;
result.push("***");
continue;
}
const currentFlag = parseFlagName(current);
if (currentFlag !== null && isSecretFlagName(currentFlag)) {
if (current.includes("=")) {
const eq = current.indexOf("=");
result.push(`${current.slice(0, eq + 1)}***`);
continue;
}
result.push(current);
redactNext = true;
continue;
}
result.push(redactToolPayloadText(current));
}
return result;
return redactSensitiveArgv(argv);
}
function capArgv(argv: readonly string[] | undefined): string[] {
+83 -4
View File
@@ -164,7 +164,15 @@ describe("config mcp config", () => {
servers: {
billing: {
command: "uvx",
args: ["billing-mcp"],
args: [
"billing-mcp",
"--api-key",
"real-argv-key",
"--token=real-inline-token",
"ghp_realgithubtoken1234567890ABCD",
"--region",
"us-east-1",
],
headers: {
Authorization: "Bearer real-token",
},
@@ -180,7 +188,15 @@ describe("config mcp config", () => {
name: "billing",
server: {
command: "uvx",
args: ["billing-mcp", "--verbose"],
args: [
"billing-mcp",
"--api-key",
REDACTED_SENTINEL,
`--token=${REDACTED_SENTINEL}`,
REDACTED_SENTINEL,
"--region",
"us-east-1",
],
headers: {
Authorization: REDACTED_SENTINEL,
},
@@ -198,7 +214,15 @@ describe("config mcp config", () => {
}
expect(loaded.mcpServers.billing).toEqual({
command: "uvx",
args: ["billing-mcp", "--verbose"],
args: [
"billing-mcp",
"--api-key",
"real-argv-key",
"--token=real-inline-token",
"ghp_realgithubtoken1234567890ABCD",
"--region",
"us-east-1",
],
headers: {
Authorization: "Bearer real-token",
},
@@ -210,13 +234,68 @@ describe("config mcp config", () => {
);
});
it("rejects redacted MCP argv when its flag binding or shape changed", async () => {
await withMcpConfigHome(
{
mcp: {
servers: {
billing: {
command: "uvx",
args: ["billing-mcp", "--api-key", "real-argv-key"],
},
},
},
},
async () => {
const changedFlag = await setConfiguredMcpServer({
name: "billing",
server: {
command: "uvx",
args: ["billing-mcp", "--output", REDACTED_SENTINEL],
},
});
expect(changedFlag.ok).toBe(false);
if (changedFlag.ok) {
throw new Error("expected changed argv binding to fail");
}
expect(changedFlag.error).toContain(REDACTED_SENTINEL);
const changedNonSecretArg = await setConfiguredMcpServer({
name: "billing",
server: {
command: "uvx",
args: ["other-mcp", "--api-key", REDACTED_SENTINEL],
},
});
expect(changedNonSecretArg.ok).toBe(false);
if (changedNonSecretArg.ok) {
throw new Error("expected argv edit with a redacted value to fail");
}
expect(changedNonSecretArg.error).toContain("Replace every redacted value explicitly");
const changedShape = await setConfiguredMcpServer({
name: "billing",
server: {
command: "uvx",
args: ["--api-key", REDACTED_SENTINEL],
},
});
expect(changedShape.ok).toBe(false);
if (changedShape.ok) {
throw new Error("expected changed argv shape to fail");
}
expect(changedShape.error).toContain(REDACTED_SENTINEL);
},
);
});
it("rejects unrestorable redacted MCP secrets on set for a new server", async () => {
await withMcpConfigHome({}, async () => {
const setResult = await setConfiguredMcpServer({
name: "new-server",
server: {
command: "uvx",
args: ["new-mcp"],
args: ["new-mcp", "--api-key", REDACTED_SENTINEL],
headers: {
Authorization: REDACTED_SENTINEL,
},
+68 -2
View File
@@ -6,13 +6,18 @@ import {
normalizeConfiguredMcpServers,
} from "./mcp-config-normalize.js";
import { replaceConfigFile } from "./mutate.js";
import { restoreRedactedValues } from "./redact-snapshot.js";
import { redactSensitiveArgv } from "./redact-argv.js";
import { REDACTED_SENTINEL, restoreRedactedValues } from "./redact-snapshot.js";
import { buildConfigSchema } from "./schema.js";
import type { OpenClawConfig } from "./types.openclaw.js";
import { validateConfigObjectWithPlugins } from "./validation.js";
type ConfigMcpServers = ReturnType<typeof normalizeConfiguredMcpServers>;
type McpArgvRestoreResult =
| { ok: true; server: Record<string, unknown> }
| { ok: false; error: string };
type ConfigMcpReadResult =
| {
ok: true;
@@ -50,6 +55,55 @@ function normalizeToolSelectionList(value: readonly string[] | undefined): strin
return normalized.length > 0 ? normalized : undefined;
}
function restoreMcpServerArgvSentinels(params: {
incoming: Record<string, unknown>;
original: Record<string, unknown> | undefined;
}): McpArgvRestoreResult {
const incomingArgs = params.incoming.args;
if (!Array.isArray(incomingArgs)) {
return { ok: true, server: params.incoming };
}
const hasSentinel = incomingArgs.some(
(arg) => typeof arg === "string" && arg.includes(REDACTED_SENTINEL),
);
if (!hasSentinel) {
return { ok: true, server: params.incoming };
}
const originalArgs = params.original?.args;
if (
!Array.isArray(originalArgs) ||
!originalArgs.every((arg) => typeof arg === "string") ||
incomingArgs.length !== originalArgs.length
) {
return {
ok: false,
error:
'Cannot restore MCP args containing "' +
REDACTED_SENTINEL +
'" without the same original argv shape.',
};
}
const displayedArgs = redactSensitiveArgv(originalArgs, REDACTED_SENTINEL);
if (incomingArgs.some((arg, index) => arg !== displayedArgs[index])) {
return {
ok: false,
error:
'Cannot restore MCP args containing "' +
REDACTED_SENTINEL +
'" after argv changed. Replace every redacted value explicitly before editing args.',
};
}
return {
ok: true,
server: {
...params.incoming,
args: originalArgs,
},
};
}
export async function listConfiguredMcpServers(): Promise<ConfigMcpReadResult> {
const snapshot = await readSourceConfigSnapshot();
if (!snapshot.valid) {
@@ -178,10 +232,22 @@ export async function setConfiguredMcpServer(params: {
return loaded;
}
const argvRestored = restoreMcpServerArgvSentinels({
incoming: params.server,
original: loaded.mcpServers[name],
});
if (!argvRestored.ok) {
return {
ok: false,
path: loaded.path,
error: argvRestored.error,
};
}
// Restore redaction sentinels from the existing server entry so a show→set
// round-trip cannot replace real credentials with the display placeholder.
const restored = restoreRedactedValues(
{ mcp: { servers: { [name]: params.server } } },
{ mcp: { servers: { [name]: argvRestored.server } } },
{ mcp: { servers: loaded.mcpServers } },
buildConfigSchema().uiHints,
);
+88
View File
@@ -0,0 +1,88 @@
// Redacts credential-bearing command arguments while preserving argv shape.
import { redactToolPayloadText } from "../logging/redact.js";
// Conservative list of credential-bearing flags. The heuristic suffix
// classifier below catches plugin-defined flags without enumerating each one.
const SECRET_FLAG_NAMES = new Set([
"--token",
"--api-key",
"--apikey",
"--secret",
"--password",
"--passwd",
"--auth-token",
"--access-token",
"--refresh-token",
"--client-secret",
"--hook-token",
"--gateway-token",
"--bot-token",
"--app-token",
"--remote-token",
"--push-token",
"--webhook-secret",
"--webhook-token",
"--service-account-token",
"--op-service-account-token",
"--bearer",
"--bearer-token",
"--pat",
"--personal-access-token",
"--oauth-token",
"--id-token",
"--identity-token",
"--session-token",
"--service-token",
"--private-key",
"--recovery-key",
"--gateway-key",
"--session-key",
"--active-key",
]);
const SECRET_FLAG_SUFFIX_PATTERN =
/^--(?:[a-z0-9]+(?:[-_][a-z0-9]+)*[-_])?(?:token|secret|password|passwd|passphrase|pin|api[-_]?key|api[-_]?secret|secret[-_]?key|secret[-_]?access[-_]?key|access[-_]?key(?:[-_]?id)?|account[-_]?key|client[-_]?key|consumer[-_]?key|license[-_]?key|subscription[-_]?key|webhook|credentials?|creds?|auth(?:orization)?|bearer|pat|cookie|private[-_]?key|recovery[-_]?key|signing[-_]?key|encryption[-_]?key|master[-_]?key|session[-_]?key|gateway[-_]?key|service[-_]?key|hook[-_]?key)$/;
function parseFlagName(arg: string): string | null {
if (!arg.startsWith("--")) {
return null;
}
const equalsIndex = arg.indexOf("=");
return (equalsIndex === -1 ? arg : arg.slice(0, equalsIndex)).toLowerCase();
}
function isSecretFlagName(flagName: string): boolean {
return SECRET_FLAG_NAMES.has(flagName) || SECRET_FLAG_SUFFIX_PATTERN.test(flagName);
}
/**
* Redacts recognized argv secrets without changing array length or non-secret flags.
* Known secret flags bind the next value even when it begins with a dash; other
* elements use the shared deterministic secret-text patterns.
*/
export function redactSensitiveArgv(argv: readonly string[], redactedValue?: string): string[] {
const replacement = redactedValue ?? "***";
const result: string[] = [];
let redactNext = false;
for (const current of argv) {
if (redactNext) {
redactNext = false;
result.push(replacement);
continue;
}
const currentFlag = parseFlagName(current);
if (currentFlag !== null && isSecretFlagName(currentFlag)) {
const equalsIndex = current.indexOf("=");
if (equalsIndex !== -1) {
result.push(`${current.slice(0, equalsIndex + 1)}${replacement}`);
continue;
}
result.push(current);
redactNext = true;
continue;
}
const redacted = redactToolPayloadText(current);
result.push(redacted === current ? current : (redactedValue ?? redacted));
}
return result;
}
+72 -1
View File
@@ -16,6 +16,9 @@ const CHAT_FINAL_TIMEOUT_MS = 45_000;
const HEADER_SECRET = "Bearer e2e-live-mcp-header-secret-value";
const ENV_SECRET = "e2e-live-mcp-env-secret-value";
const ARG_SECRET = "e2e-live-mcp-argv-secret-value";
const ARG_INLINE_SECRET = "e2e-live-mcp-inline-argv-secret-value";
const ARG_POSITIONAL_SECRET = "ghp_e2elivemcpargvtoken1234567890ABCD";
const SERVER_NAME = "billing-server";
type ChatEventPayload = {
@@ -93,7 +96,15 @@ describe("mcp show redaction e2e", () => {
servers: {
[SERVER_NAME]: {
command: "uvx",
args: ["billing-mcp"],
args: [
"billing-mcp",
"--api-key",
ARG_SECRET,
`--token=${ARG_INLINE_SECRET}`,
ARG_POSITIONAL_SECRET,
"--region",
"us-east-1",
],
transport: "streamable-http",
url: "https://billing.example.com/mcp",
headers: {
@@ -151,6 +162,14 @@ describe("mcp show redaction e2e", () => {
expect(namedText).toContain(REDACTED_SENTINEL);
expect(namedText).not.toContain(HEADER_SECRET);
expect(namedText).not.toContain(ENV_SECRET);
expect(namedText).not.toContain(ARG_SECRET);
expect(namedText).not.toContain(ARG_INLINE_SECRET);
expect(namedText).not.toContain(ARG_POSITIONAL_SECRET);
expect(namedText).toContain('"billing-mcp"');
expect(namedText).toContain('"--api-key"');
expect(namedText).toContain(`"--token=${REDACTED_SENTINEL}"`);
expect(namedText).toContain('"--region"');
expect(namedText).toContain('"us-east-1"');
expect(namedText).not.toContain("e2e-live-mcp-header-secret-value");
expect(namedText).not.toContain("e2e-live-mcp-env-secret-value");
@@ -169,8 +188,60 @@ describe("mcp show redaction e2e", () => {
expect(listText).toContain(REDACTED_SENTINEL);
expect(listText).not.toContain(HEADER_SECRET);
expect(listText).not.toContain(ENV_SECRET);
expect(listText).not.toContain(ARG_SECRET);
expect(listText).not.toContain(ARG_INLINE_SECRET);
expect(listText).not.toContain(ARG_POSITIONAL_SECRET);
expect(listText).not.toContain("second-env-secret-value");
// CLI show → set can safely round-trip redacted argv because it preserves JSON arrays.
// The chat set below intentionally omits args to avoid chat body bracket normalization.
const cliArgSet = await instance.cli([
"mcp",
"set",
SERVER_NAME,
JSON.stringify({
command: "uvx",
args: [
"billing-mcp",
"--api-key",
REDACTED_SENTINEL,
`--token=${REDACTED_SENTINEL}`,
REDACTED_SENTINEL,
"--region",
"us-east-1",
],
headers: { Authorization: REDACTED_SENTINEL },
env: { BILLING_TOKEN: REDACTED_SENTINEL },
}),
]);
expect(cliArgSet.code).toBe(0);
expect(cliArgSet.stdout + cliArgSet.stderr).toContain(`Saved MCP server "${SERVER_NAME}"`);
const afterCliArgSet = JSON.parse(await fs.readFile(instance.configPath, "utf8")) as {
mcp?: {
servers?: Record<
string,
{
args?: string[];
headers?: Record<string, string>;
env?: Record<string, string>;
}
>;
};
};
const argvRestored = afterCliArgSet.mcp?.servers?.[SERVER_NAME];
expect(argvRestored?.args).toEqual([
"billing-mcp",
"--api-key",
ARG_SECRET,
`--token=${ARG_INLINE_SECRET}`,
ARG_POSITIONAL_SECRET,
"--region",
"us-east-1",
]);
expect(argvRestored?.headers?.Authorization).toBe(HEADER_SECRET);
expect(argvRestored?.env?.BILLING_TOKEN).toBe(ENV_SECRET);
expect(JSON.stringify(argvRestored)).not.toContain(REDACTED_SENTINEL);
// show → set with redacted secrets must restore live values, not write the sentinel.
// Keep the JSON free of array brackets so chat body normalization cannot mangle it.
const setPayload = {