mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:30:42 +00:00
fix(qqbot): add SSRF guard to direct-upload URL paths in uploadC2CMedia and uploadGroupMedia [AI-assisted] (#69595)
* fix: address issue * fix: address review feedback * fix: address PR review feedback * fix: address PR review feedback * docs: add changelog entry for PR merge
This commit is contained in:
committed by
GitHub
parent
3e43306346
commit
49db424c80
@@ -18,6 +18,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- fix(qqbot): add SSRF guard to direct-upload URL paths in uploadC2CMedia and uploadGroupMedia [AI-assisted]. (#69595) Thanks @pgondhi987.
|
||||
- fix(gateway): enforce allowRequestSessionKey gate on template-rendered mapping sessionKeys. (#69381) Thanks @pgondhi987.
|
||||
- Webchat/images: treat inline image attachments as media for empty-turn gating while still ignoring metadata-only blank turns. (#69474) Thanks @Jaswir.
|
||||
- OpenAI/Responses: resolve `/think` levels against each GPT model's supported reasoning efforts so `/think off` no longer becomes high reasoning or sends unsupported `reasoning.effort: "none"` payloads.
|
||||
|
||||
145
extensions/qqbot/src/api.security.test.ts
Normal file
145
extensions/qqbot/src/api.security.test.ts
Normal file
@@ -0,0 +1,145 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const ssrfMocks = vi.hoisted(() => ({
|
||||
fetchWithSsrFGuard: vi.fn(),
|
||||
resolvePinnedHostnameWithPolicy: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("openclaw/plugin-sdk/ssrf-runtime", () => ({
|
||||
fetchWithSsrFGuard: ssrfMocks.fetchWithSsrFGuard,
|
||||
resolvePinnedHostnameWithPolicy: ssrfMocks.resolvePinnedHostnameWithPolicy,
|
||||
}));
|
||||
|
||||
vi.mock("./utils/debug-log.js", () => ({
|
||||
debugError: vi.fn(),
|
||||
debugLog: vi.fn(),
|
||||
}));
|
||||
|
||||
import { MediaFileType, uploadC2CMedia, uploadGroupMedia } from "./api.js";
|
||||
import { clearUploadCache, computeFileHash, setCachedFileInfo } from "./utils/upload-cache.js";
|
||||
|
||||
describe("qqbot direct upload SSRF guard", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
clearUploadCache();
|
||||
ssrfMocks.resolvePinnedHostnameWithPolicy.mockResolvedValue({
|
||||
hostname: "example.com",
|
||||
addresses: ["203.0.113.10"],
|
||||
lookup: vi.fn(),
|
||||
});
|
||||
ssrfMocks.fetchWithSsrFGuard.mockResolvedValue({
|
||||
response: new Response(JSON.stringify({ file_uuid: "uuid", file_info: "info", ttl: 3600 }), {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
release: async () => {},
|
||||
});
|
||||
});
|
||||
|
||||
it("blocks direct-upload URLs that target private or internal hosts", async () => {
|
||||
ssrfMocks.resolvePinnedHostnameWithPolicy.mockRejectedValueOnce(
|
||||
new Error("Blocked hostname or private/internal/special-use IP address"),
|
||||
);
|
||||
|
||||
await expect(
|
||||
uploadC2CMedia(
|
||||
"access-token",
|
||||
"user-1",
|
||||
MediaFileType.IMAGE,
|
||||
"https://169.254.169.254/latest/meta-data/iam/security-credentials/",
|
||||
),
|
||||
).rejects.toThrow("Blocked hostname or private/internal/special-use IP address");
|
||||
|
||||
expect(ssrfMocks.fetchWithSsrFGuard).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("blocks non-HTTPS direct-upload URLs before the QQ upload request", async () => {
|
||||
await expect(
|
||||
uploadGroupMedia(
|
||||
"access-token",
|
||||
"group-1",
|
||||
MediaFileType.FILE,
|
||||
"http://cdn.qpic.cn/payload.txt",
|
||||
),
|
||||
).rejects.toThrow("Direct-upload media URL must use HTTPS");
|
||||
|
||||
expect(ssrfMocks.resolvePinnedHostnameWithPolicy).not.toHaveBeenCalled();
|
||||
expect(ssrfMocks.fetchWithSsrFGuard).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows public HTTPS direct-upload URLs", async () => {
|
||||
const result = await uploadC2CMedia(
|
||||
"access-token",
|
||||
"user-1",
|
||||
MediaFileType.IMAGE,
|
||||
"https://example.com/payload.png",
|
||||
);
|
||||
|
||||
expect(result).toEqual({ file_uuid: "uuid", file_info: "info", ttl: 3600 });
|
||||
expect(ssrfMocks.resolvePinnedHostnameWithPolicy).toHaveBeenCalledWith("example.com");
|
||||
expect(ssrfMocks.fetchWithSsrFGuard).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("allows public HTTPS direct-upload URLs for group uploads", async () => {
|
||||
const result = await uploadGroupMedia(
|
||||
"access-token",
|
||||
"group-1",
|
||||
MediaFileType.FILE,
|
||||
"https://example.com/payload.txt",
|
||||
);
|
||||
|
||||
expect(result).toEqual({ file_uuid: "uuid", file_info: "info", ttl: 3600 });
|
||||
expect(ssrfMocks.resolvePinnedHostnameWithPolicy).toHaveBeenCalledWith("example.com");
|
||||
expect(ssrfMocks.fetchWithSsrFGuard).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("skips URL validation on c2c cache hits when fileData is reused", async () => {
|
||||
const fileData = "cached-file-data";
|
||||
setCachedFileInfo(
|
||||
computeFileHash(fileData),
|
||||
"c2c",
|
||||
"user-1",
|
||||
MediaFileType.IMAGE,
|
||||
"cached-info",
|
||||
"cached-uuid",
|
||||
3600,
|
||||
);
|
||||
|
||||
const result = await uploadC2CMedia(
|
||||
"access-token",
|
||||
"user-1",
|
||||
MediaFileType.IMAGE,
|
||||
"https://example.com/stale.png",
|
||||
fileData,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ file_uuid: "", file_info: "cached-info", ttl: 0 });
|
||||
expect(ssrfMocks.resolvePinnedHostnameWithPolicy).not.toHaveBeenCalled();
|
||||
expect(ssrfMocks.fetchWithSsrFGuard).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("skips URL validation on group cache hits when fileData is reused", async () => {
|
||||
const fileData = "cached-group-file-data";
|
||||
setCachedFileInfo(
|
||||
computeFileHash(fileData),
|
||||
"group",
|
||||
"group-1",
|
||||
MediaFileType.FILE,
|
||||
"cached-group-info",
|
||||
"cached-group-uuid",
|
||||
3600,
|
||||
);
|
||||
|
||||
const result = await uploadGroupMedia(
|
||||
"access-token",
|
||||
"group-1",
|
||||
MediaFileType.FILE,
|
||||
"https://example.com/stale.txt",
|
||||
fileData,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ file_uuid: "", file_info: "cached-group-info", ttl: 0 });
|
||||
expect(ssrfMocks.resolvePinnedHostnameWithPolicy).not.toHaveBeenCalled();
|
||||
expect(ssrfMocks.fetchWithSsrFGuard).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -2,7 +2,10 @@ import { createRequire } from "node:module";
|
||||
import os from "node:os";
|
||||
import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime";
|
||||
import { readPluginPackageVersion } from "openclaw/plugin-sdk/extension-shared";
|
||||
import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime";
|
||||
import {
|
||||
fetchWithSsrFGuard,
|
||||
resolvePinnedHostnameWithPolicy,
|
||||
} from "openclaw/plugin-sdk/ssrf-runtime";
|
||||
import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime";
|
||||
import { debugLog, debugError } from "./utils/debug-log.js";
|
||||
import { sanitizeFileName } from "./utils/platform.js";
|
||||
@@ -534,6 +537,22 @@ export interface UploadMediaResponse {
|
||||
id?: string;
|
||||
}
|
||||
|
||||
async function assertDirectUploadUrlAllowed(url: string): Promise<string> {
|
||||
let parsed: URL;
|
||||
try {
|
||||
parsed = new URL(url);
|
||||
} catch (err) {
|
||||
throw new Error(`Invalid media URL: ${formatErrorMessage(err)}`, { cause: err });
|
||||
}
|
||||
|
||||
if (parsed.protocol !== "https:") {
|
||||
throw new Error("Direct-upload media URL must use HTTPS");
|
||||
}
|
||||
|
||||
await resolvePinnedHostnameWithPolicy(parsed.hostname);
|
||||
return parsed.toString();
|
||||
}
|
||||
|
||||
export async function uploadC2CMedia(
|
||||
accessToken: string,
|
||||
openid: string,
|
||||
@@ -557,7 +576,7 @@ export async function uploadC2CMedia(
|
||||
|
||||
const body: Record<string, unknown> = { file_type: fileType, srv_send_msg: srvSendMsg };
|
||||
if (url) {
|
||||
body.url = url;
|
||||
body.url = await assertDirectUploadUrlAllowed(url);
|
||||
} else if (fileData) {
|
||||
body.file_data = fileData;
|
||||
}
|
||||
@@ -610,7 +629,7 @@ export async function uploadGroupMedia(
|
||||
|
||||
const body: Record<string, unknown> = { file_type: fileType, srv_send_msg: srvSendMsg };
|
||||
if (url) {
|
||||
body.url = url;
|
||||
body.url = await assertDirectUploadUrlAllowed(url);
|
||||
} else if (fileData) {
|
||||
body.file_data = fileData;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user