mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(tlon): release failed upload response bodies (#110442)
* fix(tlon): release failed upload response bodies * fix(tlon): settle guarded upload responses Co-authored-by: ZengWen-DT <ceng.wen@xydigit.com> --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
Wynne668
co-authored by
Peter Steinberger
Peter Steinberger
parent
a7e47653fc
commit
37b0b12eb7
@@ -55,14 +55,30 @@ function createMemexResponse(
|
||||
);
|
||||
}
|
||||
|
||||
function createGuardedResult(response: Response, finalUrl: string) {
|
||||
function createGuardedResult(
|
||||
response: Response,
|
||||
finalUrl: string,
|
||||
release: () => Promise<void> = mockRelease,
|
||||
) {
|
||||
return {
|
||||
response,
|
||||
finalUrl,
|
||||
release: mockRelease,
|
||||
release,
|
||||
};
|
||||
}
|
||||
|
||||
function responseWithCancelableBody(
|
||||
status: number,
|
||||
cancelBody: () => void | PromiseLike<void>,
|
||||
): Response {
|
||||
return new Response(
|
||||
new ReadableStream<Uint8Array>({
|
||||
cancel: cancelBody,
|
||||
}),
|
||||
{ status },
|
||||
);
|
||||
}
|
||||
|
||||
function guardedFetchCall(index: number): Parameters<typeof fetchWithSsrFGuard>[0] {
|
||||
const call = mockGuardedFetch.mock.calls[index]?.at(0);
|
||||
if (call === undefined) {
|
||||
@@ -109,16 +125,16 @@ describe("uploadFile memex upload hardening", () => {
|
||||
});
|
||||
|
||||
it("routes the memex upload URL through the SSRF guard", async () => {
|
||||
const lookupResponse = createMemexResponse("https://uploads.tlon.network/put");
|
||||
const lookupCancel = vi.spyOn(lookupResponse.body!, "cancel");
|
||||
const uploadCancel = vi.fn();
|
||||
mockGuardedFetch
|
||||
.mockResolvedValueOnce(
|
||||
createGuardedResult(
|
||||
createMemexResponse("https://uploads.tlon.network/put"),
|
||||
"https://memex.tlon.network/v1/zod/upload",
|
||||
),
|
||||
createGuardedResult(lookupResponse, "https://memex.tlon.network/v1/zod/upload"),
|
||||
)
|
||||
.mockResolvedValueOnce(
|
||||
createGuardedResult(
|
||||
new Response(null, { status: 200 }),
|
||||
responseWithCancelableBody(200, uploadCancel),
|
||||
"https://uploads.tlon.network/put",
|
||||
),
|
||||
);
|
||||
@@ -159,9 +175,40 @@ describe("uploadFile memex upload hardening", () => {
|
||||
expect(secondCall?.maxRedirects).toBe(0);
|
||||
expect(secondCall?.timeoutMs).toBe(300_000);
|
||||
expect(secondCall?.init?.body).toBeInstanceOf(Blob);
|
||||
expect(lookupCancel).not.toHaveBeenCalled();
|
||||
expect(uploadCancel).toHaveBeenCalledTimes(1);
|
||||
expect(mockRelease).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("cancels failed Memex upload URL responses before releasing their guard", async () => {
|
||||
const events: string[] = [];
|
||||
const cancelBody = vi.fn(() => {
|
||||
events.push("cancel");
|
||||
});
|
||||
const release = vi.fn(async () => {
|
||||
events.push("release");
|
||||
});
|
||||
mockGuardedFetch.mockResolvedValueOnce(
|
||||
createGuardedResult(
|
||||
responseWithCancelableBody(500, cancelBody),
|
||||
"https://memex.tlon.network/v1/zod/upload",
|
||||
release,
|
||||
),
|
||||
);
|
||||
|
||||
await expect(
|
||||
uploadFile({
|
||||
blob: new Blob(["image-bytes"], { type: "image/png" }),
|
||||
fileName: "avatar.png",
|
||||
contentType: "image/png",
|
||||
}),
|
||||
).rejects.toThrow("Memex upload request failed: 500");
|
||||
|
||||
expect(cancelBody).toHaveBeenCalledTimes(1);
|
||||
expect(release).toHaveBeenCalledTimes(1);
|
||||
expect(events).toEqual(["cancel", "release"]);
|
||||
});
|
||||
|
||||
it("surfaces guarded upload failures for hosted Memex targets", async () => {
|
||||
mockGuardedFetch
|
||||
.mockResolvedValueOnce(
|
||||
@@ -190,6 +237,62 @@ describe("uploadFile memex upload hardening", () => {
|
||||
expect(mockRelease).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("cancels failed Memex upload responses before releasing their guard", async () => {
|
||||
const cancelBody = vi.fn();
|
||||
mockGuardedFetch
|
||||
.mockResolvedValueOnce(
|
||||
createGuardedResult(
|
||||
createMemexResponse("https://uploads.tlon.network/put"),
|
||||
"https://memex.tlon.network/v1/zod/upload",
|
||||
),
|
||||
)
|
||||
.mockResolvedValueOnce(
|
||||
createGuardedResult(
|
||||
responseWithCancelableBody(500, cancelBody),
|
||||
"https://uploads.tlon.network/put",
|
||||
),
|
||||
);
|
||||
|
||||
await expect(
|
||||
uploadFile({
|
||||
blob: new Blob(["image-bytes"], { type: "image/png" }),
|
||||
fileName: "avatar.png",
|
||||
contentType: "image/png",
|
||||
}),
|
||||
).rejects.toThrow("Upload failed: 500");
|
||||
|
||||
expect(cancelBody).toHaveBeenCalledTimes(1);
|
||||
expect(mockRelease).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("cancels hosted upload responses when their final URL is untrusted", async () => {
|
||||
const cancelBody = vi.fn();
|
||||
mockGuardedFetch
|
||||
.mockResolvedValueOnce(
|
||||
createGuardedResult(
|
||||
createMemexResponse("https://uploads.tlon.network/put"),
|
||||
"https://memex.tlon.network/v1/zod/upload",
|
||||
),
|
||||
)
|
||||
.mockResolvedValueOnce(
|
||||
createGuardedResult(
|
||||
responseWithCancelableBody(200, cancelBody),
|
||||
"https://evil.example/put",
|
||||
),
|
||||
);
|
||||
|
||||
await expect(
|
||||
uploadFile({
|
||||
blob: new Blob(["image-bytes"], { type: "image/png" }),
|
||||
fileName: "avatar.png",
|
||||
contentType: "image/png",
|
||||
}),
|
||||
).rejects.toThrow("Memex final upload URL must target a trusted hosted Tlon domain");
|
||||
|
||||
expect(cancelBody).toHaveBeenCalledTimes(1);
|
||||
expect(mockRelease).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("rejects Memex upload targets outside the hosted Tlon domain allowlist", async () => {
|
||||
mockGuardedFetch.mockResolvedValueOnce(
|
||||
createGuardedResult(
|
||||
@@ -483,10 +586,13 @@ describe("uploadFile custom S3 upload hardening", () => {
|
||||
});
|
||||
|
||||
it("routes the custom S3 signed URL through the SSRF guard", async () => {
|
||||
const cancelBody = vi.fn(async () => {
|
||||
throw new Error("stream cancellation failed");
|
||||
});
|
||||
mockGetSignedUrl.mockResolvedValueOnce("https://s3.example.com/uploads/file?sig=abc");
|
||||
mockGuardedFetch.mockResolvedValueOnce(
|
||||
createGuardedResult(
|
||||
new Response(null, { status: 200 }),
|
||||
responseWithCancelableBody(200, cancelBody),
|
||||
"https://s3.example.com/uploads/file?sig=abc",
|
||||
),
|
||||
);
|
||||
@@ -513,6 +619,7 @@ describe("uploadFile custom S3 upload hardening", () => {
|
||||
expect(uploadCall?.maxRedirects).toBe(0);
|
||||
expect(uploadCall?.timeoutMs).toBe(300_000);
|
||||
expect(uploadCall?.policy).toBeUndefined();
|
||||
expect(cancelBody).toHaveBeenCalledTimes(1);
|
||||
expect(mockRelease).toHaveBeenCalledTimes(1);
|
||||
expect(vi.mocked(globalThis.fetch)).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -534,6 +641,28 @@ describe("uploadFile custom S3 upload hardening", () => {
|
||||
expect(vi.mocked(globalThis.fetch)).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("cancels failed custom S3 upload responses before releasing their guard", async () => {
|
||||
const cancelBody = vi.fn();
|
||||
mockGetSignedUrl.mockResolvedValueOnce("https://s3.example.com/uploads/file?sig=abc");
|
||||
mockGuardedFetch.mockResolvedValueOnce(
|
||||
createGuardedResult(
|
||||
responseWithCancelableBody(500, cancelBody),
|
||||
"https://s3.example.com/uploads/file?sig=abc",
|
||||
),
|
||||
);
|
||||
|
||||
await expect(
|
||||
uploadFile({
|
||||
blob: new Blob(["image-bytes"], { type: "image/png" }),
|
||||
fileName: "avatar.png",
|
||||
contentType: "image/png",
|
||||
}),
|
||||
).rejects.toThrow("Upload failed: 500");
|
||||
|
||||
expect(cancelBody).toHaveBeenCalledTimes(1);
|
||||
expect(mockRelease).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("passes the private-network opt-in to guarded custom S3 uploads", async () => {
|
||||
configureClient({
|
||||
shipUrl: "https://ship.example.com",
|
||||
|
||||
@@ -58,6 +58,25 @@ const TLON_UPLOAD_TIMEOUT_MS = 300_000;
|
||||
|
||||
let currentClientConfig: ClientConfig | null = null;
|
||||
|
||||
async function releaseUploadResponse(
|
||||
guarded: Awaited<ReturnType<typeof fetchWithSsrFGuard>> | undefined,
|
||||
): Promise<void> {
|
||||
if (!guarded) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
// Guard release closes the dispatcher, not an unread response stream. Settle terminal
|
||||
// upload bodies first so a streaming response cannot delay dispatcher cleanup.
|
||||
if (!guarded.response.bodyUsed) {
|
||||
await guarded.response.body?.cancel();
|
||||
}
|
||||
} catch {
|
||||
// Response cancellation is best-effort; dispatcher release must still run.
|
||||
} finally {
|
||||
await guarded.release();
|
||||
}
|
||||
}
|
||||
|
||||
export function configureClient(params: ClientConfig): void {
|
||||
currentClientConfig = {
|
||||
...params,
|
||||
@@ -240,9 +259,9 @@ async function getMemexUploadUrl(params: {
|
||||
}
|
||||
|
||||
const endpoint = `${MEMEX_BASE_URL}/v1/${params.config.shipName}/upload`;
|
||||
let release: (() => Promise<void>) | undefined;
|
||||
let guarded: Awaited<ReturnType<typeof fetchWithSsrFGuard>> | undefined;
|
||||
try {
|
||||
const guarded = await fetchWithSsrFGuard({
|
||||
guarded = await fetchWithSsrFGuard({
|
||||
url: endpoint,
|
||||
init: {
|
||||
method: "PUT",
|
||||
@@ -259,7 +278,6 @@ async function getMemexUploadUrl(params: {
|
||||
maxRedirects: 0,
|
||||
timeoutMs: TLON_MEMEX_UPLOAD_URL_TIMEOUT_MS,
|
||||
});
|
||||
release = guarded.release;
|
||||
if (!guarded.response.ok) {
|
||||
throw new Error(`Memex upload request failed: ${guarded.response.status}`);
|
||||
}
|
||||
@@ -275,7 +293,7 @@ async function getMemexUploadUrl(params: {
|
||||
|
||||
return { hostedUrl: data.filePath, uploadUrl: data.url };
|
||||
} finally {
|
||||
await release?.();
|
||||
await releaseUploadResponse(guarded);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -310,9 +328,9 @@ export async function uploadFile(params: UploadFileParams): Promise<UploadResult
|
||||
});
|
||||
const trustedUploadUrl = assertTrustedMemexUploadUrl(uploadUrl, "Memex upload URL");
|
||||
|
||||
let release: (() => Promise<void>) | undefined;
|
||||
let guarded: Awaited<ReturnType<typeof fetchWithSsrFGuard>> | undefined;
|
||||
try {
|
||||
const guarded = await fetchWithSsrFGuard({
|
||||
guarded = await fetchWithSsrFGuard({
|
||||
url: trustedUploadUrl,
|
||||
init: {
|
||||
method: "PUT",
|
||||
@@ -327,13 +345,12 @@ export async function uploadFile(params: UploadFileParams): Promise<UploadResult
|
||||
maxRedirects: 0,
|
||||
timeoutMs: TLON_UPLOAD_TIMEOUT_MS,
|
||||
});
|
||||
release = guarded.release;
|
||||
assertTrustedMemexUploadUrl(guarded.finalUrl, "Memex final upload URL");
|
||||
if (!guarded.response.ok) {
|
||||
throw new Error(`Upload failed: ${guarded.response.status}`);
|
||||
}
|
||||
} finally {
|
||||
await release?.();
|
||||
await releaseUploadResponse(guarded);
|
||||
}
|
||||
|
||||
return { url: assertTrustedMemexUploadUrl(hostedUrl, "Memex hosted URL") };
|
||||
@@ -371,9 +388,9 @@ export async function uploadFile(params: UploadFileParams): Promise<UploadResult
|
||||
expiresIn: 3600,
|
||||
});
|
||||
|
||||
let release: (() => Promise<void>) | undefined;
|
||||
let guarded: Awaited<ReturnType<typeof fetchWithSsrFGuard>> | undefined;
|
||||
try {
|
||||
const guarded = await fetchWithSsrFGuard({
|
||||
guarded = await fetchWithSsrFGuard({
|
||||
url: signedUrl,
|
||||
init: {
|
||||
method: "PUT",
|
||||
@@ -386,12 +403,11 @@ export async function uploadFile(params: UploadFileParams): Promise<UploadResult
|
||||
policy: privateNetworkPolicy,
|
||||
timeoutMs: TLON_UPLOAD_TIMEOUT_MS,
|
||||
});
|
||||
release = guarded.release;
|
||||
if (!guarded.response.ok) {
|
||||
throw new Error(`Upload failed: ${guarded.response.status}`);
|
||||
}
|
||||
} finally {
|
||||
await release?.();
|
||||
await releaseUploadResponse(guarded);
|
||||
}
|
||||
|
||||
const publicUrl = storageConfig.publicUrlBase
|
||||
|
||||
Reference in New Issue
Block a user