mirror of https://github.com/openclaw/openclaw.git
fix: harden MCP SSE config redaction (#50396) (thanks @dhananjai1729)
This commit is contained in:
parent
2c6eb127d9
commit
bfb0907777
|
|
@ -8,6 +8,7 @@ Docs: https://docs.openclaw.ai
|
|||
|
||||
- LINE/outbound media: add LINE image, video, and audio outbound sends on the LINE-specific delivery path, including explicit preview/tracking handling for videos while keeping generic media sends on the existing image-only route. (#45826) Thanks @masatohoshino.
|
||||
- WhatsApp/reactions: agents can now react with emoji on incoming WhatsApp messages, enabling more natural conversational interactions like acknowledging a photo with ❤️ instead of typing a reply. Thanks @mcaxtr.
|
||||
- MCP: add remote HTTP/SSE server support for `mcp.servers` URL configs, including auth headers and safer config redaction for MCP credentials. (#50396) Thanks @dhananjai1729.
|
||||
|
||||
### Fixes
|
||||
|
||||
|
|
|
|||
|
|
@ -1,3 +1,8 @@
|
|||
import {
|
||||
redactSensitiveUrl,
|
||||
redactSensitiveUrlLikeString,
|
||||
} from "../shared/net/redact-sensitive-url.js";
|
||||
|
||||
type SseMcpServerLaunchConfig = {
|
||||
url: string;
|
||||
headers?: Record<string, string>;
|
||||
|
|
@ -51,14 +56,10 @@ export function resolveSseMcpServerLaunchConfig(
|
|||
try {
|
||||
parsed = new URL(url);
|
||||
} catch {
|
||||
// Redact potential credentials and sensitive query params from the invalid URL.
|
||||
const redactedUrl = url
|
||||
.replace(/\/\/([^@]+)@/, "//***:***@")
|
||||
.replace(
|
||||
/([?&])(token|key|api_key|apikey|secret|access_token|password|pass|auth|client_secret|refresh_token)=([^&]*)/gi,
|
||||
"$1$2=***",
|
||||
);
|
||||
return { ok: false, reason: `its url is not a valid URL: ${redactedUrl}` };
|
||||
return {
|
||||
ok: false,
|
||||
reason: `its url is not a valid URL: ${redactSensitiveUrlLikeString(url)}`,
|
||||
};
|
||||
}
|
||||
if (parsed.protocol !== "http:" && parsed.protocol !== "https:") {
|
||||
return {
|
||||
|
|
@ -85,35 +86,7 @@ export function resolveSseMcpServerLaunchConfig(
|
|||
}
|
||||
|
||||
export function describeSseMcpServerLaunchConfig(config: SseMcpServerLaunchConfig): string {
|
||||
try {
|
||||
const parsed = new URL(config.url);
|
||||
// Redact embedded credentials and query-token auth from log/description output.
|
||||
if (parsed.username || parsed.password) {
|
||||
parsed.username = parsed.username ? "***" : "";
|
||||
parsed.password = parsed.password ? "***" : "";
|
||||
}
|
||||
for (const key of parsed.searchParams.keys()) {
|
||||
const lower = key.toLowerCase();
|
||||
if (
|
||||
lower === "token" ||
|
||||
lower === "key" ||
|
||||
lower === "api_key" ||
|
||||
lower === "apikey" ||
|
||||
lower === "secret" ||
|
||||
lower === "access_token" ||
|
||||
lower === "password" ||
|
||||
lower === "pass" ||
|
||||
lower === "auth" ||
|
||||
lower === "client_secret" ||
|
||||
lower === "refresh_token"
|
||||
) {
|
||||
parsed.searchParams.set(key, "***");
|
||||
}
|
||||
}
|
||||
return parsed.toString();
|
||||
} catch {
|
||||
return config.url;
|
||||
}
|
||||
return redactSensitiveUrl(config.url);
|
||||
}
|
||||
|
||||
export type { SseMcpServerLaunchConfig, SseMcpServerLaunchResult };
|
||||
|
|
|
|||
|
|
@ -53,4 +53,35 @@ describe("config mcp config", () => {
|
|||
expect(loaded.path).toBe(configPath);
|
||||
});
|
||||
});
|
||||
|
||||
it("accepts SSE MCP configs with headers at the config layer", async () => {
|
||||
await withTempHomeConfig({}, async () => {
|
||||
const setResult = await setConfiguredMcpServer({
|
||||
name: "remote",
|
||||
server: {
|
||||
url: "https://example.com/mcp",
|
||||
headers: {
|
||||
Authorization: "Bearer token123",
|
||||
"X-Retry": 1,
|
||||
"X-Debug": true,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(setResult.ok).toBe(true);
|
||||
const loaded = await listConfiguredMcpServers();
|
||||
expect(loaded.ok).toBe(true);
|
||||
if (!loaded.ok) {
|
||||
throw new Error("expected MCP config to load");
|
||||
}
|
||||
expect(loaded.mcpServers.remote).toEqual({
|
||||
url: "https://example.com/mcp",
|
||||
headers: {
|
||||
Authorization: "Bearer token123",
|
||||
"X-Retry": 1,
|
||||
"X-Debug": true,
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ import {
|
|||
type TestSnapshot,
|
||||
} from "./redact-snapshot.test-helpers.js";
|
||||
import { redactSnapshotTestHints as mainSchemaHints } from "./redact-snapshot.test-hints.js";
|
||||
import type { ConfigUiHints } from "./schema.js";
|
||||
import { buildConfigSchema, type ConfigUiHints } from "./schema.js";
|
||||
import type { ConfigFileSnapshot } from "./types.openclaw.js";
|
||||
|
||||
function expectNestedLevelPairValue(
|
||||
|
|
@ -159,6 +159,71 @@ describe("redactConfigSnapshot", () => {
|
|||
expect(result.raw).not.toContain("alice:secret@");
|
||||
});
|
||||
|
||||
it("redacts and restores MCP SSE header values from schema hints", () => {
|
||||
const hints = buildConfigSchema().uiHints;
|
||||
const snapshot = makeSnapshot({
|
||||
mcp: {
|
||||
servers: {
|
||||
remote: {
|
||||
url: "https://example.com/mcp",
|
||||
headers: {
|
||||
Authorization: "Bearer secret-token",
|
||||
"X-Test": "ok",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = redactConfigSnapshot(snapshot, hints);
|
||||
const servers = (result.config.mcp as { servers: Record<string, Record<string, unknown>> })
|
||||
.servers;
|
||||
expect((servers.remote.headers as Record<string, string>).Authorization).toBe(
|
||||
REDACTED_SENTINEL,
|
||||
);
|
||||
expect((servers.remote.headers as Record<string, string>)["X-Test"]).toBe(REDACTED_SENTINEL);
|
||||
|
||||
const restored = restoreRedactedValues(result.config, snapshot.config, hints);
|
||||
expect(restored.mcp.servers.remote.headers.Authorization).toBe("Bearer secret-token");
|
||||
expect(restored.mcp.servers.remote.headers["X-Test"]).toBe("ok");
|
||||
});
|
||||
|
||||
it("redacts sensitive auth material from MCP SSE URLs", () => {
|
||||
const hints = buildConfigSchema().uiHints;
|
||||
const raw = `{
|
||||
mcp: {
|
||||
servers: {
|
||||
remote: {
|
||||
url: "https://user:pass@example.com/mcp?token=secret123&safe=value",
|
||||
},
|
||||
},
|
||||
},
|
||||
}`;
|
||||
const snapshot = makeSnapshot(
|
||||
{
|
||||
mcp: {
|
||||
servers: {
|
||||
remote: {
|
||||
url: "https://user:pass@example.com/mcp?token=secret123&safe=value",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
raw,
|
||||
);
|
||||
|
||||
const result = redactConfigSnapshot(snapshot, hints);
|
||||
expect(result.config.mcp.servers.remote.url).toBe(REDACTED_SENTINEL);
|
||||
expect(result.raw).toContain(REDACTED_SENTINEL);
|
||||
expect(result.raw).not.toContain("user:pass@");
|
||||
expect(result.raw).not.toContain("secret123");
|
||||
|
||||
const restored = restoreRedactedValues(result.config, snapshot.config, hints);
|
||||
expect(restored.mcp.servers.remote.url).toBe(
|
||||
"https://user:pass@example.com/mcp?token=secret123&safe=value",
|
||||
);
|
||||
});
|
||||
|
||||
it("does not redact maxTokens-style fields", () => {
|
||||
const snapshot = makeSnapshot({
|
||||
maxTokens: 16384,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import JSON5 from "json5";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { stripUrlUserInfo } from "../shared/net/url-userinfo.js";
|
||||
import { redactSensitiveUrlLikeString } from "../shared/net/redact-sensitive-url.js";
|
||||
import {
|
||||
replaceSensitiveValuesInRaw,
|
||||
shouldFallbackToStructuredRawRedaction,
|
||||
|
|
@ -29,8 +29,11 @@ function isWholeObjectSensitivePath(path: string): boolean {
|
|||
return lowered.endsWith("serviceaccount") || lowered.endsWith("serviceaccountref");
|
||||
}
|
||||
|
||||
function isUserInfoUrlPath(path: string): boolean {
|
||||
return path.endsWith(".baseUrl") || path.endsWith(".httpUrl");
|
||||
function isSensitiveUrlPath(path: string): boolean {
|
||||
if (path.endsWith(".baseUrl") || path.endsWith(".httpUrl")) {
|
||||
return true;
|
||||
}
|
||||
return /^mcp\.servers\.[^.]+\.url$/.test(path);
|
||||
}
|
||||
|
||||
function collectSensitiveStrings(value: unknown, values: string[]): void {
|
||||
|
|
@ -217,8 +220,8 @@ function redactObjectWithLookup(
|
|||
) {
|
||||
// Keep primitives at explicitly-sensitive paths fully redacted.
|
||||
result[key] = REDACTED_SENTINEL;
|
||||
} else if (typeof value === "string" && isUserInfoUrlPath(path)) {
|
||||
const scrubbed = stripUrlUserInfo(value);
|
||||
} else if (typeof value === "string" && isSensitiveUrlPath(path)) {
|
||||
const scrubbed = redactSensitiveUrlLikeString(value);
|
||||
if (scrubbed !== value) {
|
||||
values.push(value);
|
||||
result[key] = REDACTED_SENTINEL;
|
||||
|
|
@ -242,8 +245,8 @@ function redactObjectWithLookup(
|
|||
) {
|
||||
result[key] = REDACTED_SENTINEL;
|
||||
values.push(value);
|
||||
} else if (typeof value === "string" && isUserInfoUrlPath(path)) {
|
||||
const scrubbed = stripUrlUserInfo(value);
|
||||
} else if (typeof value === "string" && isSensitiveUrlPath(path)) {
|
||||
const scrubbed = redactSensitiveUrlLikeString(value);
|
||||
if (scrubbed !== value) {
|
||||
values.push(value);
|
||||
result[key] = REDACTED_SENTINEL;
|
||||
|
|
@ -314,8 +317,8 @@ function redactObjectGuessing(
|
|||
) {
|
||||
collectSensitiveStrings(value, values);
|
||||
result[key] = REDACTED_SENTINEL;
|
||||
} else if (typeof value === "string" && isUserInfoUrlPath(dotPath)) {
|
||||
const scrubbed = stripUrlUserInfo(value);
|
||||
} else if (typeof value === "string" && isSensitiveUrlPath(dotPath)) {
|
||||
const scrubbed = redactSensitiveUrlLikeString(value);
|
||||
if (scrubbed !== value) {
|
||||
values.push(value);
|
||||
result[key] = REDACTED_SENTINEL;
|
||||
|
|
@ -655,7 +658,7 @@ function restoreRedactedValuesWithLookup(
|
|||
matched = true;
|
||||
if (
|
||||
value === REDACTED_SENTINEL &&
|
||||
(hints[candidate]?.sensitive === true || isUserInfoUrlPath(path))
|
||||
(hints[candidate]?.sensitive === true || isSensitiveUrlPath(path))
|
||||
) {
|
||||
result[key] = restoreOriginalValueOrThrow({ key, path: candidate, original: orig });
|
||||
} else if (typeof value === "object" && value !== null) {
|
||||
|
|
@ -669,7 +672,7 @@ function restoreRedactedValuesWithLookup(
|
|||
if (
|
||||
!markedNonSensitive &&
|
||||
value === REDACTED_SENTINEL &&
|
||||
(isSensitivePath(path) || isUserInfoUrlPath(path))
|
||||
(isSensitivePath(path) || isSensitiveUrlPath(path))
|
||||
) {
|
||||
result[key] = restoreOriginalValueOrThrow({ key, path, original: orig });
|
||||
} else if (typeof value === "object" && value !== null) {
|
||||
|
|
@ -711,7 +714,7 @@ function restoreRedactedValuesGuessing(
|
|||
if (
|
||||
!isExplicitlyNonSensitivePath(hints, [path, wildcardPath]) &&
|
||||
value === REDACTED_SENTINEL &&
|
||||
(isSensitivePath(path) || isUserInfoUrlPath(path))
|
||||
(isSensitivePath(path) || isSensitiveUrlPath(path))
|
||||
) {
|
||||
result[key] = restoreOriginalValueOrThrow({ key, path, original: orig });
|
||||
} else if (typeof value === "object" && value !== null) {
|
||||
|
|
|
|||
|
|
@ -11107,6 +11107,25 @@ export const GENERATED_BASE_CONFIG_SCHEMA = {
|
|||
type: "string",
|
||||
format: "uri",
|
||||
},
|
||||
headers: {
|
||||
type: "object",
|
||||
propertyNames: {
|
||||
type: "string",
|
||||
},
|
||||
additionalProperties: {
|
||||
anyOf: [
|
||||
{
|
||||
type: "string",
|
||||
},
|
||||
{
|
||||
type: "number",
|
||||
},
|
||||
{
|
||||
type: "boolean",
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
additionalProperties: {},
|
||||
},
|
||||
|
|
@ -15505,6 +15524,10 @@ export const GENERATED_BASE_CONFIG_SCHEMA = {
|
|||
sensitive: true,
|
||||
tags: ["security", "auth", "tools"],
|
||||
},
|
||||
"mcp.servers.*.headers.*": {
|
||||
sensitive: true,
|
||||
tags: ["security"],
|
||||
},
|
||||
"skills.entries.*.apiKey": {
|
||||
sensitive: true,
|
||||
tags: ["security", "auth"],
|
||||
|
|
|
|||
|
|
@ -100,10 +100,31 @@ describe("config schema", () => {
|
|||
expect(res.uiHints.gateway?.label).toBe("Gateway");
|
||||
expect(res.uiHints["gateway.auth.token"]?.sensitive).toBe(true);
|
||||
expect(res.uiHints["channels.defaults.groupPolicy"]?.label).toBeTruthy();
|
||||
expect(res.uiHints["mcp.servers.*.headers.*"]?.sensitive).toBe(true);
|
||||
expect(res.uiHints["channels.discord.threadBindings.spawnAcpSessions"]?.label).toBeTruthy();
|
||||
expect(res.version).toBeTruthy();
|
||||
expect(res.generatedAt).toBeTruthy();
|
||||
});
|
||||
|
||||
it("includes MCP SSE header schema under mcp.servers entries", () => {
|
||||
const schema = baseSchema.schema as {
|
||||
properties?: Record<string, unknown>;
|
||||
};
|
||||
const mcpNode = schema.properties?.mcp as
|
||||
| {
|
||||
properties?: Record<string, unknown>;
|
||||
}
|
||||
| undefined;
|
||||
const serversNode = mcpNode?.properties?.servers as
|
||||
| {
|
||||
additionalProperties?: {
|
||||
properties?: Record<string, unknown>;
|
||||
};
|
||||
}
|
||||
| undefined;
|
||||
expect(serversNode?.additionalProperties?.properties?.headers).toBeTruthy();
|
||||
});
|
||||
|
||||
it("merges plugin ui hints", () => {
|
||||
const res = buildConfigSchema(pluginUiHintInput);
|
||||
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ export type McpServerConfig = {
|
|||
/** SSE transport: URL of the remote MCP server (http or https). */
|
||||
url?: string;
|
||||
/** SSE transport: extra HTTP headers sent with every request. */
|
||||
headers?: Record<string, string>;
|
||||
headers?: Record<string, string | number | boolean>;
|
||||
[key: string]: unknown;
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -218,6 +218,12 @@ const McpServerSchema = z
|
|||
cwd: z.string().optional(),
|
||||
workingDirectory: z.string().optional(),
|
||||
url: HttpUrlSchema.optional(),
|
||||
headers: z
|
||||
.record(
|
||||
z.string(),
|
||||
z.union([z.string().register(sensitive), z.number(), z.boolean()]).register(sensitive),
|
||||
)
|
||||
.optional(),
|
||||
})
|
||||
.catchall(z.unknown());
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,42 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
isSensitiveUrlQueryParamName,
|
||||
redactSensitiveUrl,
|
||||
redactSensitiveUrlLikeString,
|
||||
} from "./redact-sensitive-url.js";
|
||||
|
||||
describe("redactSensitiveUrl", () => {
|
||||
it("redacts userinfo and sensitive query params from valid URLs", () => {
|
||||
expect(redactSensitiveUrl("https://user:pass@example.com/mcp?token=secret&safe=value")).toBe(
|
||||
"https://***:***@example.com/mcp?token=***&safe=value",
|
||||
);
|
||||
});
|
||||
|
||||
it("treats query param names case-insensitively", () => {
|
||||
expect(redactSensitiveUrl("https://example.com/mcp?Access_Token=secret")).toBe(
|
||||
"https://example.com/mcp?Access_Token=***",
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps non-sensitive URLs unchanged", () => {
|
||||
expect(redactSensitiveUrl("https://example.com/mcp?safe=value")).toBe(
|
||||
"https://example.com/mcp?safe=value",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("redactSensitiveUrlLikeString", () => {
|
||||
it("redacts invalid URL-like strings", () => {
|
||||
expect(redactSensitiveUrlLikeString("//user:pass@example.com/mcp?client_secret=secret")).toBe(
|
||||
"//***:***@example.com/mcp?client_secret=***",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isSensitiveUrlQueryParamName", () => {
|
||||
it("matches the auth-oriented query params used by MCP SSE config redaction", () => {
|
||||
expect(isSensitiveUrlQueryParamName("token")).toBe(true);
|
||||
expect(isSensitiveUrlQueryParamName("refresh_token")).toBe(true);
|
||||
expect(isSensitiveUrlQueryParamName("safe")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,50 @@
|
|||
const SENSITIVE_URL_QUERY_PARAM_NAMES = new Set([
|
||||
"token",
|
||||
"key",
|
||||
"api_key",
|
||||
"apikey",
|
||||
"secret",
|
||||
"access_token",
|
||||
"password",
|
||||
"pass",
|
||||
"auth",
|
||||
"client_secret",
|
||||
"refresh_token",
|
||||
]);
|
||||
|
||||
export function isSensitiveUrlQueryParamName(name: string): boolean {
|
||||
return SENSITIVE_URL_QUERY_PARAM_NAMES.has(name.toLowerCase());
|
||||
}
|
||||
|
||||
export function redactSensitiveUrl(value: string): string {
|
||||
try {
|
||||
const parsed = new URL(value);
|
||||
let mutated = false;
|
||||
if (parsed.username || parsed.password) {
|
||||
parsed.username = parsed.username ? "***" : "";
|
||||
parsed.password = parsed.password ? "***" : "";
|
||||
mutated = true;
|
||||
}
|
||||
for (const key of Array.from(parsed.searchParams.keys())) {
|
||||
if (isSensitiveUrlQueryParamName(key)) {
|
||||
parsed.searchParams.set(key, "***");
|
||||
mutated = true;
|
||||
}
|
||||
}
|
||||
return mutated ? parsed.toString() : value;
|
||||
} catch {
|
||||
return value;
|
||||
}
|
||||
}
|
||||
|
||||
export function redactSensitiveUrlLikeString(value: string): string {
|
||||
const redactedUrl = redactSensitiveUrl(value);
|
||||
if (redactedUrl !== value) {
|
||||
return redactedUrl;
|
||||
}
|
||||
return value
|
||||
.replace(/\/\/([^@/?#]+)@/, "//***:***@")
|
||||
.replace(/([?&])([^=&]+)=([^&]*)/g, (match, prefix: string, key: string) =>
|
||||
isSensitiveUrlQueryParamName(key) ? `${prefix}${key}=***` : match,
|
||||
);
|
||||
}
|
||||
Loading…
Reference in New Issue