mirror of https://github.com/openclaw/openclaw.git
Security: harden tool media paths
This commit is contained in:
parent
67edc7790f
commit
c378439246
|
|
@ -25,6 +25,7 @@ Docs: https://docs.openclaw.ai
|
|||
- Gateway/Control UI: return 404 for missing static-asset paths instead of serving SPA fallback HTML, while preserving client-route fallback behavior for extensionless and non-asset dotted paths. (#12060) thanks @mcaxtr.
|
||||
- Gateway/Pairing: prevent device-token rotate scope escalation by enforcing an approved-scope baseline, preserving approved scopes across metadata updates, and rejecting rotate requests that exceed approved role scope implications. (#20703) thanks @coygeek.
|
||||
- Gateway/Security: require secure context and paired-device checks for Control UI auth even when `gateway.controlUi.allowInsecureAuth` is set, and align audit messaging with the hardened behavior. (#20684) thanks @coygeek.
|
||||
- Security/Agents: restrict local MEDIA tool attachments to core tools and the OpenClaw temp root to prevent untrusted MCP tool file exfiltration. Thanks @NucleiAv and @thewilloftheshadow.
|
||||
- macOS/Build: default release packaging to `BUNDLE_ID=ai.openclaw.mac` in `scripts/package-mac-dist.sh`, so Sparkle feed URL is retained and auto-update no longer fails with an empty appcast feed. (#19750) thanks @loganprit.
|
||||
- Gateway/Pairing: clear persisted paired-device state when the gateway client closes with `device token mismatch` (`1008`) so reconnect flows can cleanly re-enter pairing. (#22071) Thanks @mbelinky.
|
||||
|
||||
|
|
|
|||
|
|
@ -103,6 +103,42 @@ describe("handleToolExecutionEnd media emission", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("does NOT emit local media for untrusted tools", async () => {
|
||||
const onToolResult = vi.fn();
|
||||
const ctx = createMockContext({ shouldEmitToolOutput: false, onToolResult });
|
||||
|
||||
await handleToolExecutionEnd(ctx, {
|
||||
type: "tool_execution_end",
|
||||
toolName: "plugin_tool",
|
||||
toolCallId: "tc-1",
|
||||
isError: false,
|
||||
result: {
|
||||
content: [{ type: "text", text: "MEDIA:/tmp/secret.png" }],
|
||||
},
|
||||
});
|
||||
|
||||
expect(onToolResult).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("emits remote media for untrusted tools", async () => {
|
||||
const onToolResult = vi.fn();
|
||||
const ctx = createMockContext({ shouldEmitToolOutput: false, onToolResult });
|
||||
|
||||
await handleToolExecutionEnd(ctx, {
|
||||
type: "tool_execution_end",
|
||||
toolName: "plugin_tool",
|
||||
toolCallId: "tc-1",
|
||||
isError: false,
|
||||
result: {
|
||||
content: [{ type: "text", text: "MEDIA:https://example.com/file.png" }],
|
||||
},
|
||||
});
|
||||
|
||||
expect(onToolResult).toHaveBeenCalledWith({
|
||||
mediaUrls: ["https://example.com/file.png"],
|
||||
});
|
||||
});
|
||||
|
||||
it("does NOT emit media when verbose is full (emitToolOutput handles it)", async () => {
|
||||
const onToolResult = vi.fn();
|
||||
const ctx = createMockContext({ shouldEmitToolOutput: true, onToolResult });
|
||||
|
|
|
|||
|
|
@ -9,10 +9,11 @@ import type {
|
|||
ToolHandlerContext,
|
||||
} from "./pi-embedded-subscribe.handlers.types.js";
|
||||
import {
|
||||
extractMessagingToolSend,
|
||||
extractToolErrorMessage,
|
||||
extractToolResultMediaPaths,
|
||||
extractToolResultText,
|
||||
extractMessagingToolSend,
|
||||
filterToolResultMediaUrls,
|
||||
isToolResultError,
|
||||
sanitizeToolResult,
|
||||
} from "./pi-embedded-subscribe.tools.js";
|
||||
|
|
@ -381,7 +382,7 @@ export async function handleToolExecutionEnd(
|
|||
// When shouldEmitToolOutput() is true, emitToolOutput already delivers media
|
||||
// via parseReplyDirectives (MEDIA: text extraction), so skip to avoid duplicates.
|
||||
if (ctx.params.onToolResult && !isToolError && !ctx.shouldEmitToolOutput()) {
|
||||
const mediaPaths = extractToolResultMediaPaths(result);
|
||||
const mediaPaths = filterToolResultMediaUrls(toolName, extractToolResultMediaPaths(result));
|
||||
if (mediaPaths.length > 0) {
|
||||
try {
|
||||
void ctx.params.onToolResult({ mediaUrls: mediaPaths });
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { MEDIA_TOKEN_RE } from "../media/parse.js";
|
|||
import { truncateUtf16Safe } from "../utils.js";
|
||||
import { collectTextContentBlocks } from "./content-blocks.js";
|
||||
import { type MessagingToolSend } from "./pi-embedded-messaging.js";
|
||||
import { normalizeToolName } from "./tool-policy.js";
|
||||
|
||||
const TOOL_RESULT_MAX_CHARS = 8000;
|
||||
const TOOL_ERROR_MAX_CHARS = 400;
|
||||
|
|
@ -129,6 +130,58 @@ export function extractToolResultText(result: unknown): string | undefined {
|
|||
return texts.join("\n");
|
||||
}
|
||||
|
||||
// Core tool names that are allowed to emit local MEDIA: paths.
|
||||
// Plugin/MCP tools are intentionally excluded to prevent untrusted file reads.
|
||||
const TRUSTED_TOOL_RESULT_MEDIA = new Set([
|
||||
"agents_list",
|
||||
"apply_patch",
|
||||
"browser",
|
||||
"canvas",
|
||||
"cron",
|
||||
"edit",
|
||||
"exec",
|
||||
"gateway",
|
||||
"image",
|
||||
"memory_get",
|
||||
"memory_search",
|
||||
"message",
|
||||
"nodes",
|
||||
"process",
|
||||
"read",
|
||||
"session_status",
|
||||
"sessions_history",
|
||||
"sessions_list",
|
||||
"sessions_send",
|
||||
"sessions_spawn",
|
||||
"subagents",
|
||||
"tts",
|
||||
"web_fetch",
|
||||
"web_search",
|
||||
"write",
|
||||
]);
|
||||
const HTTP_URL_RE = /^https?:\/\//i;
|
||||
|
||||
export function isToolResultMediaTrusted(toolName?: string): boolean {
|
||||
if (!toolName) {
|
||||
return false;
|
||||
}
|
||||
const normalized = normalizeToolName(toolName);
|
||||
return TRUSTED_TOOL_RESULT_MEDIA.has(normalized);
|
||||
}
|
||||
|
||||
export function filterToolResultMediaUrls(
|
||||
toolName: string | undefined,
|
||||
mediaUrls: string[],
|
||||
): string[] {
|
||||
if (mediaUrls.length === 0) {
|
||||
return mediaUrls;
|
||||
}
|
||||
if (isToolResultMediaTrusted(toolName)) {
|
||||
return mediaUrls;
|
||||
}
|
||||
return mediaUrls.filter((url) => HTTP_URL_RE.test(url.trim()));
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract media file paths from a tool result.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import type {
|
|||
EmbeddedPiSubscribeContext,
|
||||
EmbeddedPiSubscribeState,
|
||||
} from "./pi-embedded-subscribe.handlers.types.js";
|
||||
import { filterToolResultMediaUrls } from "./pi-embedded-subscribe.tools.js";
|
||||
import type { SubscribeEmbeddedPiSessionParams } from "./pi-embedded-subscribe.types.js";
|
||||
import { formatReasoningMessage, stripDowngradedToolCallText } from "./pi-embedded-utils.js";
|
||||
import { hasNonzeroUsage, normalizeUsage, type UsageLike } from "./usage.js";
|
||||
|
|
@ -324,13 +325,14 @@ export function subscribeEmbeddedPiSession(params: SubscribeEmbeddedPiSessionPar
|
|||
markdown: useMarkdown,
|
||||
});
|
||||
const { text: cleanedText, mediaUrls } = parseReplyDirectives(agg);
|
||||
if (!cleanedText && (!mediaUrls || mediaUrls.length === 0)) {
|
||||
const filteredMediaUrls = filterToolResultMediaUrls(toolName, mediaUrls ?? []);
|
||||
if (!cleanedText && filteredMediaUrls.length === 0) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
void params.onToolResult({
|
||||
text: cleanedText,
|
||||
mediaUrls: mediaUrls?.length ? mediaUrls : undefined,
|
||||
mediaUrls: filteredMediaUrls.length ? filteredMediaUrls : undefined,
|
||||
});
|
||||
} catch {
|
||||
// ignore tool result delivery failures
|
||||
|
|
@ -345,13 +347,14 @@ export function subscribeEmbeddedPiSession(params: SubscribeEmbeddedPiSessionPar
|
|||
});
|
||||
const message = `${agg}\n${formatToolOutputBlock(output)}`;
|
||||
const { text: cleanedText, mediaUrls } = parseReplyDirectives(message);
|
||||
if (!cleanedText && (!mediaUrls || mediaUrls.length === 0)) {
|
||||
const filteredMediaUrls = filterToolResultMediaUrls(toolName, mediaUrls ?? []);
|
||||
if (!cleanedText && filteredMediaUrls.length === 0) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
void params.onToolResult({
|
||||
text: cleanedText,
|
||||
mediaUrls: mediaUrls?.length ? mediaUrls : undefined,
|
||||
mediaUrls: filteredMediaUrls.length ? filteredMediaUrls : undefined,
|
||||
});
|
||||
} catch {
|
||||
// ignore tool result delivery failures
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { randomUUID } from "node:crypto";
|
||||
import * as os from "node:os";
|
||||
import fs from "node:fs";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
|
||||
|
||||
export function asRecord(value: unknown): Record<string, unknown> {
|
||||
return typeof value === "object" && value !== null ? (value as Record<string, unknown>) : {};
|
||||
|
|
@ -22,8 +23,12 @@ export function resolveTempPathParts(opts: { ext: string; tmpDir?: string; id?:
|
|||
tmpDir: string;
|
||||
id: string;
|
||||
} {
|
||||
const tmpDir = opts.tmpDir ?? resolvePreferredOpenClawTmpDir();
|
||||
if (!opts.tmpDir) {
|
||||
fs.mkdirSync(tmpDir, { recursive: true, mode: 0o700 });
|
||||
}
|
||||
return {
|
||||
tmpDir: opts.tmpDir ?? os.tmpdir(),
|
||||
tmpDir,
|
||||
id: opts.id ?? randomUUID(),
|
||||
ext: opts.ext.startsWith(".") ? opts.ext : `.${opts.ext}`,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1,13 +1,14 @@
|
|||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { resolveAgentWorkspaceDir } from "../agents/agent-scope.js";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import { resolveStateDir } from "../config/paths.js";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
|
||||
|
||||
function buildMediaLocalRoots(stateDir: string): string[] {
|
||||
const resolvedStateDir = path.resolve(stateDir);
|
||||
const preferredTmpDir = resolvePreferredOpenClawTmpDir();
|
||||
return [
|
||||
os.tmpdir(),
|
||||
preferredTmpDir,
|
||||
path.join(resolvedStateDir, "media"),
|
||||
path.join(resolvedStateDir, "agents"),
|
||||
path.join(resolvedStateDir, "workspace"),
|
||||
|
|
|
|||
|
|
@ -9,7 +9,6 @@ import {
|
|||
renameSync,
|
||||
unlinkSync,
|
||||
} from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import type { ReplyPayload } from "../auto-reply/types.js";
|
||||
import { normalizeChannelId } from "../channels/plugins/index.js";
|
||||
|
|
@ -23,6 +22,7 @@ import type {
|
|||
TtsModelOverrideConfig,
|
||||
} from "../config/types.tts.js";
|
||||
import { logVerbose } from "../globals.js";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
|
||||
import { stripMarkdown } from "../line/markdown-to-line.js";
|
||||
import { isVoiceCompatibleAudio } from "../media/audio.js";
|
||||
import { CONFIG_DIR, resolveUserPath } from "../utils.js";
|
||||
|
|
@ -563,7 +563,9 @@ export async function textToSpeech(params: {
|
|||
continue;
|
||||
}
|
||||
|
||||
const tempDir = mkdtempSync(path.join(tmpdir(), "tts-"));
|
||||
const tempRoot = resolvePreferredOpenClawTmpDir();
|
||||
mkdirSync(tempRoot, { recursive: true, mode: 0o700 });
|
||||
const tempDir = mkdtempSync(path.join(tempRoot, "tts-"));
|
||||
let edgeOutputFormat = resolveEdgeOutputFormat(config);
|
||||
const fallbackEdgeOutputFormat =
|
||||
edgeOutputFormat !== DEFAULT_EDGE_OUTPUT_FORMAT ? DEFAULT_EDGE_OUTPUT_FORMAT : undefined;
|
||||
|
|
@ -670,7 +672,9 @@ export async function textToSpeech(params: {
|
|||
|
||||
const latencyMs = Date.now() - providerStart;
|
||||
|
||||
const tempDir = mkdtempSync(path.join(tmpdir(), "tts-"));
|
||||
const tempRoot = resolvePreferredOpenClawTmpDir();
|
||||
mkdirSync(tempRoot, { recursive: true, mode: 0o700 });
|
||||
const tempDir = mkdtempSync(path.join(tempRoot, "tts-"));
|
||||
const audioPath = path.join(tempDir, `voice-${Date.now()}${output.extension}`);
|
||||
writeFileSync(audioPath, audioBuffer);
|
||||
scheduleCleanup(tempDir);
|
||||
|
|
|
|||
|
|
@ -108,7 +108,7 @@ afterEach(() => {
|
|||
describe("web media loading", () => {
|
||||
beforeAll(() => {
|
||||
// Ensure state dir is stable and not influenced by other tests that stub OPENCLAW_STATE_DIR.
|
||||
// Also keep it outside os.tmpdir() so tmpdir localRoots doesn't accidentally make all state readable.
|
||||
// Also keep it outside the OpenClaw temp root so default localRoots doesn't accidentally make all state readable.
|
||||
stateDirSnapshot = captureEnv(["OPENCLAW_STATE_DIR"]);
|
||||
process.env.OPENCLAW_STATE_DIR = path.join(
|
||||
path.parse(os.tmpdir()).root,
|
||||
|
|
|
|||
|
|
@ -73,9 +73,9 @@ async function assertLocalMediaAllowed(
|
|||
resolved = path.resolve(mediaPath);
|
||||
}
|
||||
|
||||
// Hardening: the default allowlist includes `os.tmpdir()`, and tests/CI may
|
||||
// Hardening: the default allowlist includes the OpenClaw temp dir, and tests/CI may
|
||||
// override the state dir into tmp. Avoid accidentally allowing per-agent
|
||||
// `workspace-*` state roots via the tmpdir prefix match; require explicit
|
||||
// `workspace-*` state roots via the temp-root prefix match; require explicit
|
||||
// localRoots for those.
|
||||
if (localRoots === undefined) {
|
||||
const workspaceRoot = roots.find((root) => path.basename(root) === "workspace");
|
||||
|
|
|
|||
Loading…
Reference in New Issue