Skip to content

Commit fc1b37c

Browse files
author
lsc
committed
fix(security): validate presigned URL scheme before PUT / download
yujiawei's P1: `uploadMcpIconReal` and `uploadFile` used the backend- returned `presigned_url` directly with no scheme/host check; likewise `downloadSkill` assigned `result.url` to `anchor.href` without a scheme check. A misconfigured or compromised marketplace could point the browser at `javascript:`, `data:`, `file:`, or an internal-only host. Both packages now gate every backend-supplied external URL through a tiny checker that only accepts: - `https://…` (production case), or - `http://localhost` / `http://127.0.0.1` (dev proxy) Everything else throws before the network call happens. Credential leakage was already covered (raw axios / no interceptors on the PUT); this closes the destination-side gap. Refs: PR Mininglamp-OSS#851 review by @yujiawei
1 parent e7c8c39 commit fc1b37c

2 files changed

Lines changed: 46 additions & 0 deletions

File tree

packages/dmworkmcp/src/api/mcpService.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,25 @@ function delay<T>(value: T, ms = MOCK_DELAY_MS): Promise<T> {
6464
return new Promise((resolve) => setTimeout(() => resolve(value), ms));
6565
}
6666

67+
/**
68+
* Reject presigned upload / download URLs whose scheme is not http(s), or
69+
* whose http-scheme host is not a loopback (dev proxy). Backend-supplied
70+
* URLs are still trusted for the exact host they name, but we refuse to
71+
* PUT / GET against `javascript:`, `data:`, `file:` etc. — defense in
72+
* depth against a misconfigured or compromised marketplace.
73+
*/
74+
function assertSafeUploadURL(raw: string): void {
75+
let u: URL;
76+
try {
77+
u = new URL(raw);
78+
} catch {
79+
throw new Error(t("mcp.create.iconUploadFailed"));
80+
}
81+
if (u.protocol === "https:") return;
82+
if (u.protocol === "http:" && (u.hostname === "localhost" || u.hostname === "127.0.0.1")) return;
83+
throw new Error(t("mcp.create.iconUploadFailed"));
84+
}
85+
6786
// ─── Mock implementations ──────────────────────────────────────────────────
6887

6988
function buildCategories(): McpCategory[] {
@@ -553,6 +572,12 @@ async function uploadMcpIconReal(_id: string, file: File): Promise<string> {
553572
throw new Error(t("mcp.create.iconUploadFailed"));
554573
}
555574
const { presigned_url, download_url, headers } = init.data;
575+
// Defense-in-depth: the presigned URLs come back from our own marketplace
576+
// backend, but any downstream misconfiguration/compromise could point them
577+
// at an internal address or a non-HTTPS host. Only allow https:// (or
578+
// http:// on localhost for dev proxies).
579+
assertSafeUploadURL(presigned_url);
580+
assertSafeUploadURL(download_url);
556581

557582
// PUT via raw axios (no interceptors) — the presigned URL points at
558583
// storage / local proxy, not marketplace, and any Accept-Language /

packages/dmworkskillmarket/src/api/skillApiReal.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,25 @@ function wait(ms: number): Promise<void> {
126126
return new Promise((resolve) => globalThis.setTimeout(resolve, ms));
127127
}
128128

129+
/**
130+
* Reject presigned upload / download URLs whose scheme is not http(s), or
131+
* whose http-scheme host is not loopback. Guards against a compromised or
132+
* misconfigured marketplace returning `javascript:` / `data:` / `file:` /
133+
* arbitrary internal targets that a browser would otherwise honor
134+
* (anchor.href / xhr.open both accept unusual schemes).
135+
*/
136+
function assertSafeExternalURL(raw: string): void {
137+
let u: URL;
138+
try {
139+
u = new URL(raw);
140+
} catch {
141+
throw normalizeError({ code: "invalid_response", message: "URL 无效" });
142+
}
143+
if (u.protocol === "https:") return;
144+
if (u.protocol === "http:" && (u.hostname === "localhost" || u.hostname === "127.0.0.1")) return;
145+
throw normalizeError({ code: "invalid_response", message: "URL scheme 不允许" });
146+
}
147+
129148
// ─── Mappers ───────────────────────────────────────────────────────────────
130149

131150
function mapCategory(raw: RawCategory, index: number): Category {
@@ -273,6 +292,7 @@ export async function downloadSkill(id: string): Promise<void> {
273292
if (!result.url) {
274293
throw normalizeError({ code: "invalid_response", message: "下载地址无效" });
275294
}
295+
assertSafeExternalURL(result.url);
276296
const anchor = document.createElement("a");
277297
anchor.href = result.url;
278298
anchor.target = "_blank";
@@ -298,6 +318,7 @@ export function initUpload(fileName: string, fileSize: number): Promise<UploadIn
298318

299319
/** Step 2: Upload the file to the pre-signed URL (PUT). */
300320
export async function uploadFile(presignedUrl: string, file: File, headers?: Record<string, string>, onProgress?: (percent: number) => void): Promise<void> {
321+
assertSafeExternalURL(presignedUrl);
301322
// Use XMLHttpRequest for progress support
302323
return new Promise((resolve, reject) => {
303324
const xhr = new XMLHttpRequest();

0 commit comments

Comments
 (0)