Skip to content

Commit b1eaf53

Browse files
ooiuuiisteipete
ooiuuii
and
steipete
authored
fix(feishu): avoid local media fallback leaks (#98251)
* Fix Feishu local media fallback leaks * refactor(feishu): clarify media fallback link style * fix(feishu): reject credentialed media fallbacks * fix(feishu): reject ambiguous media fallback URLs --------- Co-authored-by: Peter Steinberger <[email protected]>
1 parent 0598020 commit b1eaf53

5 files changed

Lines changed: 186 additions & 18 deletions

File tree

‎extensions/feishu/src/media-fallback.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import {
2+
isBlockedHostnameOrIp,
3+
resolvePinnedHostnameWithPolicy,
4+
} from "openclaw/plugin-sdk/ssrf-runtime";
5+
6+
const FEISHU_MEDIA_UPLOAD_FAILURE_FALLBACK_TEXT = "Media upload failed. Please try again.";
7+
8+
function hasAsciiControlCharacter(value: string): boolean {
9+
return Array.from(value).some((character) => {
10+
const code = character.charCodeAt(0);
11+
return code <= 0x1f || code === 0x7f;
12+
});
13+
}
14+
15+
async function resolvePublicFeishuMediaReference(
16+
value: string | undefined,
17+
): Promise<string | undefined> {
18+
const raw = value?.trim();
19+
if (!raw || hasAsciiControlCharacter(raw)) {
20+
return undefined;
21+
}
22+
try {
23+
const parsed = new URL(raw);
24+
if (parsed.protocol !== "https:" && parsed.protocol !== "http:") {
25+
return undefined;
26+
}
27+
if (parsed.username || parsed.password || isBlockedHostnameOrIp(parsed.hostname)) {
28+
return undefined;
29+
}
30+
// A proxy route cannot prove its destination is public. Fail closed unless
31+
// local pinned DNS validates the URL without private or special-use answers.
32+
await resolvePinnedHostnameWithPolicy(parsed.hostname);
33+
return parsed.href;
34+
} catch {
35+
return undefined;
36+
}
37+
}
38+
39+
export async function buildFeishuMediaFallbackText(params: {
40+
text?: string;
41+
mediaUrl?: string;
42+
mediaLinkStyle?: "attachment" | "plain";
43+
}): Promise<string> {
44+
const mediaUrl = await resolvePublicFeishuMediaReference(params.mediaUrl);
45+
const attachmentText = mediaUrl
46+
? `${params.mediaLinkStyle === "plain" ? "" : "📎 "}${mediaUrl}`
47+
: FEISHU_MEDIA_UPLOAD_FAILURE_FALLBACK_TEXT;
48+
return [params.text?.trim(), attachmentText].filter(Boolean).join("\n\n");
49+
}

‎extensions/feishu/src/outbound.test.ts‎

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,26 @@ const shouldSuppressFeishuTextForVoiceMediaMock = vi.hoisted(
3030
? params.ttsSupplement.visibleTextAlreadyDelivered === true
3131
: params.audioAsVoice === true || /\.(?:ogg|opus)(?:[?#]|$)/i.test(params.mediaUrl ?? ""),
3232
);
33+
const resolvePinnedHostnameWithPolicyMock = vi.hoisted(() =>
34+
vi.fn(async (hostname: string) => {
35+
if (hostname === "files.example.test") {
36+
throw new Error("Blocked: resolves to private/internal/special-use IP address");
37+
}
38+
return {
39+
hostname,
40+
addresses: ["93.184.216.34"],
41+
lookup: vi.fn(),
42+
};
43+
}),
44+
);
45+
46+
vi.mock("openclaw/plugin-sdk/ssrf-runtime", async (importOriginal) => {
47+
const actual = await importOriginal<Record<string, unknown>>();
48+
return {
49+
...actual,
50+
resolvePinnedHostnameWithPolicy: resolvePinnedHostnameWithPolicyMock,
51+
};
52+
});
3353

3454
vi.mock("./media.js", () => ({
3555
sendMediaFeishu: sendMediaFeishuMock,
@@ -193,6 +213,7 @@ afterAll(() => {
193213
vi.doUnmock("./client.js");
194214
vi.doUnmock("./drive.js");
195215
vi.doUnmock("./comment-reaction.js");
216+
vi.doUnmock("openclaw/plugin-sdk/ssrf-runtime");
196217
vi.resetModules();
197218
});
198219

@@ -496,7 +517,7 @@ describe("feishuOutbound.sendText local-image auto-convert", () => {
496517
expectFeishuResult(result, "native_card_msg");
497518
});
498519

499-
it("falls back to plain text if local-image media send fails", async () => {
520+
it("does not leak local-image paths if auto-send fails", async () => {
500521
const { dir, file } = await createTmpImage();
501522
sendMediaFeishuMock.mockRejectedValueOnce(new Error("upload failed"));
502523
try {
@@ -509,7 +530,8 @@ describe("feishuOutbound.sendText local-image auto-convert", () => {
509530

510531
expect(sendMediaFeishuMock).toHaveBeenCalledTimes(1);
511532
expect(sendMessageCall()?.to).toBe("chat_1");
512-
expect(sendMessageCall()?.text).toBe(file);
533+
expect(sendMessageCall()?.text).toBe("Media upload failed. Please try again.");
534+
expect(sendMessageCall()?.text).not.toContain(file);
513535
expect(sendMessageCall()?.accountId).toBe("main");
514536
} finally {
515537
await fs.rm(dir, { recursive: true, force: true });
@@ -2074,6 +2096,26 @@ describe("feishuOutbound comment-thread routing", () => {
20742096
expectFeishuResult(result, "reply_msg");
20752097
});
20762098

2099+
it.each([
2100+
["local path", path.join(os.tmpdir(), "openclaw-feishu-comment-local-voice.mp3")],
2101+
["loopback URL", "http://127.0.0.1:3000/tmp/openclaw-voice.mp3"],
2102+
])("does not leak a %s in comment-thread media fallbacks", async (_label, mediaUrl) => {
2103+
const result = await feishuOutbound.sendMedia?.({
2104+
cfg: emptyConfig,
2105+
to: "comment:docx:doxcn123:7623358762119646411",
2106+
text: "see attachment",
2107+
mediaUrl,
2108+
accountId: "main",
2109+
});
2110+
2111+
expect(commentThreadParams()?.content).toBe(
2112+
"see attachment\n\nMedia upload failed. Please try again.",
2113+
);
2114+
expect(commentThreadParams()?.content).not.toContain(mediaUrl);
2115+
expect(sendMediaFeishuMock).not.toHaveBeenCalled();
2116+
expectFeishuResult(result, "reply_msg");
2117+
});
2118+
20772119
it("preserves comment-thread routing when deliverCommentThreadText falls back to add_comment", async () => {
20782120
deliverCommentThreadTextMock.mockResolvedValueOnce({
20792121
delivery_mode: "add_comment",
@@ -2511,6 +2553,32 @@ describe("feishuOutbound.sendMedia replyToId forwarding", () => {
25112553
expect(sendMessageCall()?.text).toBe("spoken reply\n\n📎 https://example.com/reply.mp3");
25122554
});
25132555

2556+
it.each([
2557+
["local path", path.join(os.tmpdir(), "openclaw-feishu-local-voice.mp3")],
2558+
["file URL", "file:///tmp/openclaw-feishu-local-voice.mp3"],
2559+
["relative path", "./outbound/openclaw-feishu-local-voice.mp3"],
2560+
["loopback URL", "http://127.0.0.1:3000/tmp/openclaw-voice.mp3"],
2561+
["localhost URL", "https://localhost/tmp/openclaw-voice.mp3"],
2562+
["private-DNS URL", "https://files.example.test/openclaw-voice.mp3"],
2563+
["credentialed URL", "https://[email protected]/openclaw-voice.mp3"],
2564+
["control-character URL", "https://example.com/\nhttp://127.0.0.1/private"],
2565+
])("does not leak a %s in the upload failure fallback", async (_label, mediaUrl) => {
2566+
sendMediaFeishuMock.mockRejectedValueOnce(new Error("upload failed"));
2567+
2568+
await feishuOutbound.sendMedia?.({
2569+
cfg: emptyConfig,
2570+
to: "chat_1",
2571+
text: "spoken reply",
2572+
mediaUrl,
2573+
audioAsVoice: true,
2574+
accountId: "main",
2575+
});
2576+
2577+
expect(sendMessageFeishuMock).toHaveBeenCalledTimes(1);
2578+
expect(sendMessageCall()?.text).toBe("spoken reply\n\nMedia upload failed. Please try again.");
2579+
expect(sendMessageCall()?.text).not.toContain(mediaUrl);
2580+
});
2581+
25142582
it("forwards replyToId to text caption send", async () => {
25152583
await feishuOutbound.sendMedia?.({
25162584
cfg: emptyConfig,

‎extensions/feishu/src/outbound.ts‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
chunkFeishuPostMarkdown,
4040
materializeFeishuPostMarkdownSoftBreaks,
4141
} from "./markdown.js";
42+
import { buildFeishuMediaFallbackText } from "./media-fallback.js";
4243
import {
4344
sendMediaFeishu,
4445
shouldSuppressFeishuTextForVoiceMedia,
@@ -788,7 +789,14 @@ export const feishuOutbound: ChannelOutboundAdapter = {
788789
});
789790
} catch (err) {
790791
console.error(`[feishu] local image path auto-send failed:`, err);
791-
// fall through to plain text as last resort
792+
return await sendOutboundText({
793+
cfg,
794+
to,
795+
text: await buildFeishuMediaFallbackText({}),
796+
accountId: accountId ?? undefined,
797+
replyToMessageId,
798+
replyInThread,
799+
});
792800
}
793801
}
794802

@@ -863,11 +871,17 @@ export const feishuOutbound: ChannelOutboundAdapter = {
863871
});
864872
const commentTarget = parseFeishuCommentTarget(to);
865873
if (commentTarget) {
866-
const commentText = [text?.trim(), mediaUrl?.trim()].filter(Boolean).join("\n\n");
874+
const commentText = mediaUrl?.trim()
875+
? await buildFeishuMediaFallbackText({
876+
text,
877+
mediaUrl,
878+
mediaLinkStyle: "plain",
879+
})
880+
: (text?.trim() ?? "");
867881
return await sendOutboundText({
868882
cfg,
869883
to,
870-
text: commentText || mediaUrl || text || "",
884+
text: commentText,
871885
accountId: accountId ?? undefined,
872886
replyToMessageId,
873887
replyInThread,
@@ -916,10 +930,10 @@ export const feishuOutbound: ChannelOutboundAdapter = {
916930
} catch (err) {
917931
// Log the error for debugging
918932
console.error(`[feishu] sendMediaFeishu failed:`, err);
919-
// Fallback to URL link if upload fails
920-
const fallbackText = [textSent ? undefined : text?.trim(), `📎 ${mediaUrl}`]
921-
.filter(Boolean)
922-
.join("\n\n");
933+
const fallbackText = await buildFeishuMediaFallbackText({
934+
text: textSent ? undefined : text,
935+
mediaUrl,
936+
});
923937
const fallbackResult = await sendOutboundText({
924938
cfg,
925939
to,

‎extensions/feishu/src/reply-dispatcher.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
// Feishu tests cover reply dispatcher plugin behavior.
2+
import os from "node:os";
3+
import path from "node:path";
24
import { afterAll, beforeEach, describe, expect, it, vi } from "vitest";
35

46
type StreamingSessionStub = {
@@ -34,6 +36,18 @@ const shouldSuppressFeishuTextForVoiceMediaMock = vi.hoisted(
3436
? params.ttsSupplement.visibleTextAlreadyDelivered === true
3537
: params.audioAsVoice === true || /\.(?:ogg|opus)(?:[?#]|$)/i.test(params.mediaUrl ?? ""),
3638
);
39+
const resolvePinnedHostnameWithPolicyMock = vi.hoisted(() =>
40+
vi.fn(async (hostname: string) => {
41+
if (hostname === "files.example.test") {
42+
throw new Error("Blocked: resolves to private/internal/special-use IP address");
43+
}
44+
return {
45+
hostname,
46+
addresses: ["93.184.216.34"],
47+
lookup: vi.fn(),
48+
};
49+
}),
50+
);
3751

3852
function mergeStreamingText(
3953
previousText: string | undefined,
@@ -76,6 +90,13 @@ vi.mock("./media.js", () => ({
7690
sendMediaFeishu: sendMediaFeishuMock,
7791
shouldSuppressFeishuTextForVoiceMedia: shouldSuppressFeishuTextForVoiceMediaMock,
7892
}));
93+
vi.mock("openclaw/plugin-sdk/ssrf-runtime", async (importOriginal) => {
94+
const actual = await importOriginal<Record<string, unknown>>();
95+
return {
96+
...actual,
97+
resolvePinnedHostnameWithPolicy: resolvePinnedHostnameWithPolicyMock,
98+
};
99+
});
79100
vi.mock("./client.js", () => ({ createFeishuClient: createFeishuClientMock }));
80101
vi.mock("./targets.js", () => ({ resolveReceiveIdType: resolveReceiveIdTypeMock }));
81102
vi.mock("./typing.js", () => ({
@@ -122,6 +143,7 @@ afterAll(() => {
122143
vi.doUnmock("./targets.js");
123144
vi.doUnmock("./typing.js");
124145
vi.doUnmock("./streaming-card.js");
146+
vi.doUnmock("openclaw/plugin-sdk/ssrf-runtime");
125147
vi.resetModules();
126148
});
127149

@@ -1521,6 +1543,26 @@ describe("createFeishuReplyDispatcher streaming behavior", () => {
15211543
});
15221544
});
15231545

1546+
it("does not leak local media paths in the upload failure fallback", async () => {
1547+
const mediaPath = path.join(os.tmpdir(), "openclaw-feishu-reply-local-voice.mp3");
1548+
sendMediaFeishuMock.mockRejectedValueOnce(new Error("media failed"));
1549+
1550+
const { options } = createDispatcherHarness();
1551+
await options.deliver(
1552+
{
1553+
text: "spoken reply",
1554+
mediaUrl: mediaPath,
1555+
audioAsVoice: true,
1556+
},
1557+
{ kind: "final" },
1558+
);
1559+
1560+
expect(sendMessageFeishuMock).toHaveBeenCalledTimes(1);
1561+
const fallbackText = String(firstMockArg(sendMessageFeishuMock, "message send params").text);
1562+
expect(fallbackText).toBe("spoken reply\n\nMedia upload failed. Please try again.");
1563+
expect(fallbackText).not.toContain(mediaPath);
1564+
});
1565+
15241566
it("falls back to legacy mediaUrl when mediaUrls is an empty array", async () => {
15251567
useNonStreamingAutoAccount();
15261568
const { options } = createDispatcherHarness();

‎extensions/feishu/src/reply-dispatcher.ts‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { resolveConfiguredHttpTimeoutMs } from "./client-timeout.js";
2020
import { createFeishuClient } from "./client.js";
2121
import { resolveFeishuIdentityEmoji } from "./identity-header.js";
2222
import { chunkFeishuPostMarkdown, materializeFeishuPostMarkdownSoftBreaks } from "./markdown.js";
23+
import { buildFeishuMediaFallbackText } from "./media-fallback.js";
2324
import { sendMediaFeishu, shouldSuppressFeishuTextForVoiceMedia } from "./media.js";
2425
import type { MentionTarget } from "./mention-target.types.js";
2526
import {
@@ -84,12 +85,6 @@ function rememberStreamingStartFailure(accountId: string, now = Date.now()): num
8485
return backoffUntil;
8586
}
8687

87-
function formatMediaFallbackText(text: string | undefined, mediaUrl: string): string {
88-
const trimmedText = text?.trim() ?? "";
89-
const attachmentText = `📎 ${mediaUrl}`;
90-
return trimmedText ? `${trimmedText}\n\n${attachmentText}` : attachmentText;
91-
}
92-
9388
function normalizeEpochMs(timestamp: number | undefined): number | undefined {
9489
if (!Number.isFinite(timestamp) || timestamp === undefined || timestamp <= 0) {
9590
return undefined;
@@ -581,10 +576,10 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP
581576
options?.fallbackText === undefined
582577
? undefined
583578
: async ({ mediaUrl }) => {
584-
const fallbackText = formatMediaFallbackText(
585-
sentFallbackText ? undefined : options.fallbackText,
579+
const fallbackText = await buildFeishuMediaFallbackText({
580+
text: sentFallbackText ? undefined : options.fallbackText,
586581
mediaUrl,
587-
);
582+
});
588583
sentFallbackText = true;
589584
await sendChunkedTextReply({
590585
text: fallbackText,

0 commit comments

Comments
 (0)