mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 08:50:43 +00:00
fix(line): guard outbound media targets
This commit is contained in:
@@ -41,6 +41,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Control UI/CSP: tighten `img-src` to `'self' data:` only, and make Control UI avatar helpers drop remote `http(s)` and protocol-relative URLs so the UI falls back to the built-in logo/badge instead of issuing arbitrary remote image fetches. Same-origin avatar routes (relative paths) and `data:image/...` avatars still render. (#69773)
|
||||
- CLI/channels: keep `status`, `health`, `channels list`, and `channels status` on read-only channel metadata when Telegram, Slack, Discord, or third-party channel plugins are configured, avoiding full bundled plugin runtime imports on those cold paths. Fixes #69042. (#69479) Thanks @gumadeiras.
|
||||
- Synology Chat: validate outbound webhook `file_url` values against the shared SSRF policy before forwarding to the NAS, rejecting malformed URLs, non-`http(s)` schemes, and private/blocked network targets so the NAS cannot be used as a confused deputy to fetch internal addresses. (#69784) Thanks @eleqtrizit.
|
||||
- LINE: validate outbound media URLs against the shared public-network guard before handing them to LINE, preserving arbitrary public HTTPS media while rejecting loopback, link-local, and private-network targets.
|
||||
- Gateway/Control UI: require gateway auth on the Control UI avatar route (`GET /avatar/<agentId>` and `?meta=1` metadata) when auth is configured, matching the sibling assistant-media route, and propagate the existing gateway token through the UI avatar fetch (bearer header + authenticated blob URL) so authenticated dashboards still load local avatars. (#69775)
|
||||
- Exec/allowlist: reject POSIX parameter expansion forms such as `$VAR`, `$?`, `$$`, `$1`, and `$@` inside unquoted heredocs during shell approval analysis, so these heredocs no longer pass allowlist review as plain text. (#69795) Thanks @drobison00.
|
||||
- Gateway/MCP loopback: derive owner-only tool visibility from distinct authenticated owner vs non-owner loopback bearers instead of the caller-controlled owner header, so non-owner MCP child processes cannot recover owner access by spoofing request metadata. (#69796)
|
||||
|
||||
@@ -205,6 +205,8 @@ The LINE plugin supports sending images, videos, and audio files through the age
|
||||
- **Videos**: sent with explicit preview and content-type handling.
|
||||
- **Audio**: sent as LINE audio messages.
|
||||
|
||||
Outbound media URLs must be public HTTPS URLs. OpenClaw validates the target hostname before handing the URL to LINE and rejects loopback, link-local, and private-network targets.
|
||||
|
||||
Generic media sends fall back to the existing image-only route when a LINE-specific path is not available.
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
@@ -1,4 +1,13 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const ssrfMocks = vi.hoisted(() => ({
|
||||
resolvePinnedHostnameWithPolicy: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("openclaw/plugin-sdk/ssrf-runtime", () => ({
|
||||
resolvePinnedHostnameWithPolicy: ssrfMocks.resolvePinnedHostnameWithPolicy,
|
||||
}));
|
||||
|
||||
import {
|
||||
detectLineMediaKind,
|
||||
resolveLineOutboundMedia,
|
||||
@@ -6,22 +15,53 @@ import {
|
||||
} from "./outbound-media.js";
|
||||
|
||||
describe("validateLineMediaUrl", () => {
|
||||
it("accepts HTTPS URL", () => {
|
||||
expect(() => validateLineMediaUrl("https://example.com/image.jpg")).not.toThrow();
|
||||
beforeEach(() => {
|
||||
ssrfMocks.resolvePinnedHostnameWithPolicy.mockReset();
|
||||
ssrfMocks.resolvePinnedHostnameWithPolicy.mockResolvedValue({
|
||||
hostname: "example.com",
|
||||
addresses: ["93.184.216.34"],
|
||||
});
|
||||
});
|
||||
|
||||
it("accepts uppercase HTTPS scheme", () => {
|
||||
expect(() => validateLineMediaUrl("HTTPS://EXAMPLE.COM/img.jpg")).not.toThrow();
|
||||
it("accepts HTTPS URL", async () => {
|
||||
await expect(validateLineMediaUrl("https://example.com/image.jpg")).resolves.toBeUndefined();
|
||||
expect(ssrfMocks.resolvePinnedHostnameWithPolicy).toHaveBeenCalledWith("example.com", {
|
||||
policy: { allowPrivateNetwork: false },
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects HTTP URL", () => {
|
||||
expect(() => validateLineMediaUrl("http://example.com/image.jpg")).toThrow(/must use HTTPS/i);
|
||||
it("accepts uppercase HTTPS scheme", async () => {
|
||||
await expect(validateLineMediaUrl("HTTPS://EXAMPLE.COM/img.jpg")).resolves.toBeUndefined();
|
||||
expect(ssrfMocks.resolvePinnedHostnameWithPolicy).toHaveBeenCalledWith("example.com", {
|
||||
policy: { allowPrivateNetwork: false },
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects URL longer than 2000 chars", () => {
|
||||
it("rejects HTTP URL", async () => {
|
||||
await expect(validateLineMediaUrl("http://example.com/image.jpg")).rejects.toThrow(
|
||||
/must use HTTPS/i,
|
||||
);
|
||||
expect(ssrfMocks.resolvePinnedHostnameWithPolicy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects URL longer than 2000 chars", async () => {
|
||||
const longUrl = `https://example.com/${"a".repeat(1981)}`;
|
||||
expect(longUrl.length).toBeGreaterThan(2000);
|
||||
expect(() => validateLineMediaUrl(longUrl)).toThrow(/2000 chars or less/i);
|
||||
await expect(validateLineMediaUrl(longUrl)).rejects.toThrow(/2000 chars or less/i);
|
||||
expect(ssrfMocks.resolvePinnedHostnameWithPolicy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects private-network targets through the shared SSRF policy", async () => {
|
||||
ssrfMocks.resolvePinnedHostnameWithPolicy.mockRejectedValueOnce(
|
||||
new Error("SSRF blocked private network target"),
|
||||
);
|
||||
|
||||
await expect(validateLineMediaUrl("https://127.0.0.1/image.jpg")).rejects.toThrow(
|
||||
/private network/i,
|
||||
);
|
||||
expect(ssrfMocks.resolvePinnedHostnameWithPolicy).toHaveBeenCalledWith("127.0.0.1", {
|
||||
policy: { allowPrivateNetwork: false },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -48,6 +88,14 @@ describe("detectLineMediaKind", () => {
|
||||
});
|
||||
|
||||
describe("resolveLineOutboundMedia", () => {
|
||||
beforeEach(() => {
|
||||
ssrfMocks.resolvePinnedHostnameWithPolicy.mockReset();
|
||||
ssrfMocks.resolvePinnedHostnameWithPolicy.mockResolvedValue({
|
||||
hostname: "example.com",
|
||||
addresses: ["93.184.216.34"],
|
||||
});
|
||||
});
|
||||
|
||||
it("respects explicit media kind without remote MIME probing", async () => {
|
||||
await expect(
|
||||
resolveLineOutboundMedia("https://example.com/download?id=123", { mediaKind: "video" }),
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { resolvePinnedHostnameWithPolicy, type SsrFPolicy } from "openclaw/plugin-sdk/ssrf-runtime";
|
||||
import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/text-runtime";
|
||||
|
||||
export type LineOutboundMediaKind = "image" | "video" | "audio";
|
||||
@@ -17,7 +18,11 @@ type ResolveLineOutboundMediaOpts = {
|
||||
trackingId?: string;
|
||||
};
|
||||
|
||||
export function validateLineMediaUrl(url: string): void {
|
||||
const LINE_OUTBOUND_MEDIA_SSRF_POLICY: SsrFPolicy = {
|
||||
allowPrivateNetwork: false,
|
||||
};
|
||||
|
||||
export async function validateLineMediaUrl(url: string): Promise<void> {
|
||||
let parsed: URL;
|
||||
try {
|
||||
parsed = new URL(url);
|
||||
@@ -30,6 +35,9 @@ export function validateLineMediaUrl(url: string): void {
|
||||
if (url.length > 2000) {
|
||||
throw new Error(`LINE outbound media URL must be 2000 chars or less (got ${url.length})`);
|
||||
}
|
||||
await resolvePinnedHostnameWithPolicy(parsed.hostname, {
|
||||
policy: LINE_OUTBOUND_MEDIA_SSRF_POLICY,
|
||||
});
|
||||
}
|
||||
|
||||
export function detectLineMediaKind(mimeType: string): LineOutboundMediaKind {
|
||||
@@ -78,10 +86,10 @@ export async function resolveLineOutboundMedia(
|
||||
): Promise<LineOutboundMediaResolved> {
|
||||
const trimmedUrl = mediaUrl.trim();
|
||||
if (isHttpsUrl(trimmedUrl)) {
|
||||
validateLineMediaUrl(trimmedUrl);
|
||||
await validateLineMediaUrl(trimmedUrl);
|
||||
const previewImageUrl = opts.previewImageUrl?.trim();
|
||||
if (previewImageUrl) {
|
||||
validateLineMediaUrl(previewImageUrl);
|
||||
await validateLineMediaUrl(previewImageUrl);
|
||||
}
|
||||
const mediaKind =
|
||||
opts.mediaKind ??
|
||||
|
||||
@@ -11,6 +11,7 @@ const {
|
||||
resolveLineChannelAccessTokenMock,
|
||||
recordChannelActivityMock,
|
||||
logVerboseMock,
|
||||
resolvePinnedHostnameWithPolicyMock,
|
||||
} = vi.hoisted(() => {
|
||||
const pushMessageMock = vi.fn();
|
||||
const replyMessageMock = vi.fn();
|
||||
@@ -29,6 +30,7 @@ const {
|
||||
const resolveLineChannelAccessTokenMock = vi.fn(() => "line-token");
|
||||
const recordChannelActivityMock = vi.fn();
|
||||
const logVerboseMock = vi.fn();
|
||||
const resolvePinnedHostnameWithPolicyMock = vi.fn();
|
||||
return {
|
||||
pushMessageMock,
|
||||
replyMessageMock,
|
||||
@@ -40,6 +42,7 @@ const {
|
||||
resolveLineChannelAccessTokenMock,
|
||||
recordChannelActivityMock,
|
||||
logVerboseMock,
|
||||
resolvePinnedHostnameWithPolicyMock,
|
||||
};
|
||||
});
|
||||
|
||||
@@ -73,6 +76,10 @@ vi.mock("openclaw/plugin-sdk/runtime-env", async () => {
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("openclaw/plugin-sdk/ssrf-runtime", () => ({
|
||||
resolvePinnedHostnameWithPolicy: resolvePinnedHostnameWithPolicyMock,
|
||||
}));
|
||||
|
||||
let sendModule: typeof import("./send.js");
|
||||
|
||||
describe("LINE send helpers", () => {
|
||||
@@ -88,6 +95,7 @@ describe("LINE send helpers", () => {
|
||||
resolveLineChannelAccessTokenMock.mockReset();
|
||||
recordChannelActivityMock.mockReset();
|
||||
logVerboseMock.mockReset();
|
||||
resolvePinnedHostnameWithPolicyMock.mockReset();
|
||||
|
||||
MessagingApiClientMock.mockImplementation(function () {
|
||||
return {
|
||||
@@ -100,6 +108,10 @@ describe("LINE send helpers", () => {
|
||||
loadConfigMock.mockReturnValue({});
|
||||
resolveLineAccountMock.mockReturnValue({ accountId: "default" });
|
||||
resolveLineChannelAccessTokenMock.mockReturnValue("line-token");
|
||||
resolvePinnedHostnameWithPolicyMock.mockResolvedValue({
|
||||
hostname: "example.com",
|
||||
addresses: ["93.184.216.34"],
|
||||
});
|
||||
pushMessageMock.mockResolvedValue({});
|
||||
replyMessageMock.mockResolvedValue({});
|
||||
showLoadingAnimationMock.mockResolvedValue({});
|
||||
@@ -205,6 +217,20 @@ describe("LINE send helpers", () => {
|
||||
).rejects.toThrow(/require previewimageurl/i);
|
||||
});
|
||||
|
||||
it("blocks private-network media URLs before calling LINE", async () => {
|
||||
resolvePinnedHostnameWithPolicyMock.mockRejectedValueOnce(
|
||||
new Error("SSRF blocked private network target"),
|
||||
);
|
||||
|
||||
await expect(
|
||||
sendModule.sendMessageLine("line:user:U200", "Image", {
|
||||
mediaUrl: "https://127.0.0.1/image.jpg",
|
||||
}),
|
||||
).rejects.toThrow(/private network/i);
|
||||
|
||||
expect(pushMessageMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("omits trackingId for non-user destinations", async () => {
|
||||
await sendModule.sendMessageLine("line:group:C100", "Video", {
|
||||
mediaUrl: "https://example.com/video.mp4",
|
||||
|
||||
@@ -4,6 +4,7 @@ import { recordChannelActivity } from "openclaw/plugin-sdk/infra-runtime";
|
||||
import { logVerbose } from "openclaw/plugin-sdk/runtime-env";
|
||||
import { resolveLineAccount } from "./accounts.js";
|
||||
import { resolveLineChannelAccessToken } from "./channel-access-token.js";
|
||||
import { validateLineMediaUrl } from "./outbound-media.js";
|
||||
import type { LineSendResult } from "./types.js";
|
||||
|
||||
type Message = messagingApi.Message;
|
||||
@@ -248,12 +249,14 @@ export async function sendMessageLine(
|
||||
|
||||
const mediaUrl = opts.mediaUrl?.trim();
|
||||
if (mediaUrl) {
|
||||
await validateLineMediaUrl(mediaUrl);
|
||||
switch (opts.mediaKind) {
|
||||
case "video": {
|
||||
const previewImageUrl = opts.previewImageUrl?.trim();
|
||||
if (!previewImageUrl) {
|
||||
throw new Error("LINE video messages require previewImageUrl to reference an image URL");
|
||||
}
|
||||
await validateLineMediaUrl(previewImageUrl);
|
||||
const trackingId = isLineUserChatId(chatId) ? opts.trackingId : undefined;
|
||||
messages.push(createVideoMessage(mediaUrl, previewImageUrl, trackingId));
|
||||
break;
|
||||
@@ -264,7 +267,11 @@ export async function sendMessageLine(
|
||||
case "image":
|
||||
default:
|
||||
// Backward compatibility: keep image as default when media kind is unspecified.
|
||||
messages.push(createImageMessage(mediaUrl, opts.previewImageUrl?.trim() || mediaUrl));
|
||||
{
|
||||
const previewImageUrl = opts.previewImageUrl?.trim() || mediaUrl;
|
||||
await validateLineMediaUrl(previewImageUrl);
|
||||
messages.push(createImageMessage(mediaUrl, previewImageUrl));
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
@@ -336,6 +343,10 @@ export async function pushImageMessage(
|
||||
previewImageUrl?: string,
|
||||
opts: LinePushOpts = {},
|
||||
): Promise<LineSendResult> {
|
||||
await validateLineMediaUrl(originalContentUrl);
|
||||
if (previewImageUrl) {
|
||||
await validateLineMediaUrl(previewImageUrl);
|
||||
}
|
||||
return pushLineMessages(to, [createImageMessage(originalContentUrl, previewImageUrl)], opts, {
|
||||
verboseMessage: (chatId) => `line: pushed image to ${chatId}`,
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user