fix: enforce local media root checks for attachment hydration

This commit is contained in:
Peter Steinberger
2026-02-24 23:16:59 +00:00
parent b67e600bff
commit 270ab03e37
4 changed files with 94 additions and 28 deletions

View File

@@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai
- Security/Sandbox media: restrict sandbox media tmp-path allowances to OpenClaw-managed tmp roots instead of broad host `os.tmpdir()` trust, and add outbound/channel guardrails (tmp-path lint + media-root smoke tests) to prevent regressions in local media attachment reads.
- Config/Plugins: treat stale removed `google-antigravity-auth` plugin references as compatibility warnings (not hard validation errors) across `plugins.entries`, `plugins.allow`, `plugins.deny`, and `plugins.slots.memory`, so startup no longer fails after antigravity removal. (#25538, #25862) Thanks @chilu18.
- Security/Message actions: enforce local media root checks for `sendAttachment` and `setGroupIcon` when `sandboxRoot` is unset, preventing attachment hydration from reading arbitrary host files via local absolute paths. This ships in the next npm release. Thanks @GCXWLP for reporting.
- Security/Workspace FS: normalize `@`-prefixed paths before workspace-boundary checks (including workspace-only read/write/edit and sandbox mount path guards), preventing absolute-path escape attempts from bypassing guard validation. This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Native images: enforce `tools.fs.workspaceOnly` for native prompt image auto-load (including history refs), preventing out-of-workspace sandbox mounts from being implicitly ingested as vision input. This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Exec approvals: bind `system.run` command display/approval text to full argv when shell-wrapper inline payloads carry positional argv values, and reject payload-only `rawCommand` mismatches for those wrapper-carrier forms, preventing hidden command execution under misleading approval text. This ships in the next npm release. Thanks @tdjackey for reporting.

View File

@@ -178,6 +178,8 @@ async function hydrateAttachmentPayload(params: {
contentTypeParam?: string | null;
mediaHint?: string | null;
fileHint?: string | null;
sandboxRoot?: string;
mediaLocalRoots?: readonly string[];
}) {
const contentTypeParam = params.contentTypeParam ?? undefined;
const rawBuffer = readStringParam(params.args, "buffer", { trim: false });
@@ -201,12 +203,17 @@ async function hydrateAttachmentPayload(params: {
channel: params.channel,
accountId: params.accountId,
});
// mediaSource already validated by normalizeSandboxMediaList; allow bypass but force explicit readFile.
const media = await loadWebMedia(mediaSource, {
maxBytes,
sandboxValidated: true,
readFile: (filePath: string) => fs.readFile(filePath),
});
const sandboxRoot = params.sandboxRoot?.trim();
const media = sandboxRoot
? await loadWebMedia(mediaSource, {
maxBytes,
sandboxValidated: true,
readFile: (filePath: string) => fs.readFile(filePath),
})
: await loadWebMedia(mediaSource, {
maxBytes,
localRoots: params.mediaLocalRoots,
});
params.args.buffer = media.buffer.toString("base64");
if (!contentTypeParam && media.contentType) {
params.args.contentType = media.contentType;
@@ -280,6 +287,8 @@ async function hydrateAttachmentActionPayload(params: {
dryRun?: boolean;
/** If caption is missing, copy message -> caption. */
allowMessageCaptionFallback?: boolean;
sandboxRoot?: string;
mediaLocalRoots?: readonly string[];
}): Promise<void> {
const mediaHint = readStringParam(params.args, "media", { trim: false });
const fileHint =
@@ -305,6 +314,8 @@ async function hydrateAttachmentActionPayload(params: {
contentTypeParam,
mediaHint,
fileHint,
sandboxRoot: params.sandboxRoot,
mediaLocalRoots: params.mediaLocalRoots,
});
}
@@ -315,6 +326,8 @@ export async function hydrateSetGroupIconParams(params: {
args: Record<string, unknown>;
action: ChannelMessageActionName;
dryRun?: boolean;
sandboxRoot?: string;
mediaLocalRoots?: readonly string[];
}): Promise<void> {
if (params.action !== "setGroupIcon") {
return;
@@ -329,6 +342,8 @@ export async function hydrateSendAttachmentParams(params: {
args: Record<string, unknown>;
action: ChannelMessageActionName;
dryRun?: boolean;
sandboxRoot?: string;
mediaLocalRoots?: readonly string[];
}): Promise<void> {
if (params.action !== "sendAttachment") {
return;

View File

@@ -424,6 +424,15 @@ describe("runMessageAction context isolation", () => {
});
describe("runMessageAction sendAttachment hydration", () => {
const cfg = {
channels: {
bluebubbles: {
enabled: true,
serverUrl: "http://localhost:1234",
password: "test-password",
},
},
} as OpenClawConfig;
const attachmentPlugin: ChannelPlugin = {
id: "bluebubbles",
meta: {
@@ -433,15 +442,15 @@ describe("runMessageAction sendAttachment hydration", () => {
docsPath: "/channels/bluebubbles",
blurb: "BlueBubbles test plugin.",
},
capabilities: { chatTypes: ["direct"], media: true },
capabilities: { chatTypes: ["direct", "group"], media: true },
config: {
listAccountIds: () => ["default"],
resolveAccount: () => ({ enabled: true }),
isConfigured: () => true,
},
actions: {
listActions: () => ["sendAttachment"],
supportsAction: ({ action }) => action === "sendAttachment",
listActions: () => ["sendAttachment", "setGroupIcon"],
supportsAction: ({ action }) => action === "sendAttachment" || action === "setGroupIcon",
handleAction: async ({ params }) =>
jsonResult({
ok: true,
@@ -476,17 +485,12 @@ describe("runMessageAction sendAttachment hydration", () => {
vi.clearAllMocks();
});
it("hydrates buffer and filename from media for sendAttachment", async () => {
const cfg = {
channels: {
bluebubbles: {
enabled: true,
serverUrl: "http://localhost:1234",
password: "test-password",
},
},
} as OpenClawConfig;
async function restoreRealMediaLoader() {
const actual = await vi.importActual<typeof import("../../web/media.js")>("../../web/media.js");
vi.mocked(loadWebMedia).mockImplementation(actual.loadWebMedia);
}
it("hydrates buffer and filename from media for sendAttachment", async () => {
const result = await runMessageAction({
cfg,
action: "sendAttachment",
@@ -511,15 +515,6 @@ describe("runMessageAction sendAttachment hydration", () => {
});
it("rewrites sandboxed media paths for sendAttachment", async () => {
const cfg = {
channels: {
bluebubbles: {
enabled: true,
serverUrl: "http://localhost:1234",
password: "test-password",
},
},
} as OpenClawConfig;
await withSandbox(async (sandboxDir) => {
await runMessageAction({
cfg,
@@ -537,6 +532,55 @@ describe("runMessageAction sendAttachment hydration", () => {
expect(call?.[0]).toBe(path.join(sandboxDir, "data", "pic.png"));
});
});
it("rejects local absolute path for sendAttachment when sandboxRoot is missing", async () => {
await restoreRealMediaLoader();
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-attachment-"));
try {
const outsidePath = path.join(tempDir, "secret.txt");
await fs.writeFile(outsidePath, "secret", "utf8");
await expect(
runMessageAction({
cfg,
action: "sendAttachment",
params: {
channel: "bluebubbles",
target: "+15551234567",
media: outsidePath,
message: "caption",
},
}),
).rejects.toThrow(/allowed directory|path-not-allowed/i);
} finally {
await fs.rm(tempDir, { recursive: true, force: true });
}
});
it("rejects local absolute path for setGroupIcon when sandboxRoot is missing", async () => {
await restoreRealMediaLoader();
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-group-icon-"));
try {
const outsidePath = path.join(tempDir, "secret.txt");
await fs.writeFile(outsidePath, "secret", "utf8");
await expect(
runMessageAction({
cfg,
action: "setGroupIcon",
params: {
channel: "bluebubbles",
target: "group:123",
media: outsidePath,
},
}),
).rejects.toThrow(/allowed directory|path-not-allowed/i);
} finally {
await fs.rm(tempDir, { recursive: true, force: true });
}
});
});
describe("runMessageAction sandboxed media validation", () => {

View File

@@ -13,6 +13,7 @@ import type {
ChannelThreadingToolContext,
} from "../../channels/plugins/types.js";
import type { OpenClawConfig } from "../../config/config.js";
import { getAgentScopedMediaLocalRoots } from "../../media/local-roots.js";
import {
isDeliverableMessageChannel,
normalizeMessageChannel,
@@ -757,6 +758,7 @@ export async function runMessageAction(
params.accountId = accountId;
}
const dryRun = Boolean(input.dryRun ?? readBooleanParam(params, "dryRun"));
const mediaLocalRoots = getAgentScopedMediaLocalRoots(cfg, resolvedAgentId);
await normalizeSandboxMediaParams({
args: params,
@@ -770,6 +772,8 @@ export async function runMessageAction(
args: params,
action,
dryRun,
sandboxRoot: input.sandboxRoot,
mediaLocalRoots,
});
await hydrateSetGroupIconParams({
@@ -779,6 +783,8 @@ export async function runMessageAction(
args: params,
action,
dryRun,
sandboxRoot: input.sandboxRoot,
mediaLocalRoots,
});
const resolvedTarget = await resolveActionTarget({