fix: address PR review issues for tlon plugin

- upload.ts: Use fetchWithSsrFGuard directly instead of urbitFetch to preserve full URL path when fetching external images; add release() call - media.ts: Same fix - use fetchWithSsrFGuard for external media downloads; add release() call to clean up resources - channel.ts: Use urbitFetch for poke API to maintain consistent SSRF protection (DNS pinning + redirect handling) - upload.test.ts: Update mocks to use fetchWithSsrFGuard instead of urbitFetch Addresses blocking issues from jalehman's review: 1. Fixed incorrect URL being fetched (validateUrbitBaseUrl was stripping path) 2. Fixed missing release() calls that could leak resources 3. Restored guarded fetch semantics for poke operations
2026-04-25 08:02:04 +00:00 · 2026-02-27 13:12:19 -06:00
parent f587591b80
commit a91d8e19b9
4 changed files with 113 additions and 90 deletions
--- a/extensions/tlon/src/channel.ts
+++ b/extensions/tlon/src/channel.ts
@@ -39,7 +39,7 @@ async function createHttpPokeApi(params: {
  const ssrfPolicy = ssrfPolicyFromAllowPrivateNetwork(params.allowPrivateNetwork);
  const cookie = await authenticate(params.url, params.code, { ssrfPolicy });
  const channelId = `${Math.floor(Date.now() / 1000)}-${crypto.randomUUID()}`;
-  const channelUrl = `${params.url}/~/channel/${channelId}`;
+  const channelPath = `/~/channel/${channelId}`;
  const shipName = params.ship.replace(/^~/, "");

  return {
@@ -54,21 +54,32 @@ async function createHttpPokeApi(params: {
        json: pokeParams.json,
      };

-      const response = await fetch(channelUrl, {
-        method: "PUT",
-        headers: {
-          "Content-Type": "application/json",
-          Cookie: cookie.split(";")[0],
+      // Use urbitFetch for consistent SSRF protection (DNS pinning + redirect handling)
+      const { response, release } = await urbitFetch({
+        baseUrl: params.url,
+        path: channelPath,
+        init: {
+          method: "PUT",
+          headers: {
+            "Content-Type": "application/json",
+            Cookie: cookie.split(";")[0],
+          },
+          body: JSON.stringify([pokeData]),
        },
-        body: JSON.stringify([pokeData]),
+        ssrfPolicy,
+        auditContext: "tlon-poke",
      });

-      if (!response.ok && response.status !== 204) {
-        const errorText = await response.text();
-        throw new Error(`Poke failed: ${response.status} - ${errorText}`);
-      }
+      try {
+        if (!response.ok && response.status !== 204) {
+          const errorText = await response.text();
+          throw new Error(`Poke failed: ${response.status} - ${errorText}`);
+        }

-      return pokeId;
+        return pokeId;
+      } finally {
+        await release();
+      }
    },
    delete: async () => {
      // No-op for HTTP-only client
--- a/extensions/tlon/src/monitor/media.ts
+++ b/extensions/tlon/src/monitor/media.ts
@@ -5,8 +5,8 @@ import { homedir } from "node:os";
 import * as path from "node:path";
 import { Readable } from "node:stream";
 import { pipeline } from "node:stream/promises";
+import { fetchWithSsrFGuard } from "openclaw/plugin-sdk";
 import { getDefaultSsrFPolicy } from "../urbit/context.js";
-import { urbitFetch } from "../urbit/fetch.js";

 // Default to OpenClaw workspace media directory
 const DEFAULT_MEDIA_DIR = path.join(homedir(), ".openclaw", "workspace", "media", "inbound");
@@ -65,42 +65,46 @@ export async function downloadMedia(
    await mkdir(mediaDir, { recursive: true });

    // Fetch with SSRF protection
-    const { response } = await urbitFetch({
-      baseUrl: url,
-      path: "",
+    // Use fetchWithSsrFGuard directly (not urbitFetch) to preserve the full URL path
+    const { response, release } = await fetchWithSsrFGuard({
+      url,
      init: { method: "GET" },
-      ssrfPolicy: getDefaultSsrFPolicy(),
+      policy: getDefaultSsrFPolicy(),
      auditContext: "tlon-media-download",
    });

-    if (!response.ok) {
-      console.error(`[tlon-media] Failed to fetch ${url}: ${response.status}`);
-      return null;
+    try {
+      if (!response.ok) {
+        console.error(`[tlon-media] Failed to fetch ${url}: ${response.status}`);
+        return null;
+      }
+
+      // Determine content type and extension
+      const contentType = response.headers.get("content-type") || "application/octet-stream";
+      const ext = getExtensionFromContentType(contentType) || getExtensionFromUrl(url) || "bin";
+
+      // Generate unique filename
+      const filename = `${randomUUID()}.${ext}`;
+      const localPath = path.join(mediaDir, filename);
+
+      // Stream to file
+      const body = response.body;
+      if (!body) {
+        console.error(`[tlon-media] No response body for ${url}`);
+        return null;
+      }
+
+      const writeStream = createWriteStream(localPath);
+      await pipeline(Readable.fromWeb(body as any), writeStream);
+
+      return {
+        localPath,
+        contentType,
+        originalUrl: url,
+      };
+    } finally {
+      await release();
    }
-
-    // Determine content type and extension
-    const contentType = response.headers.get("content-type") || "application/octet-stream";
-    const ext = getExtensionFromContentType(contentType) || getExtensionFromUrl(url) || "bin";
-
-    // Generate unique filename
-    const filename = `${randomUUID()}.${ext}`;
-    const localPath = path.join(mediaDir, filename);
-
-    // Stream to file
-    const body = response.body;
-    if (!body) {
-      console.error(`[tlon-media] No response body for ${url}`);
-      return null;
-    }
-
-    const writeStream = createWriteStream(localPath);
-    await pipeline(Readable.fromWeb(body as any), writeStream);
-
-    return {
-      localPath,
-      contentType,
-      originalUrl: url,
-    };
  } catch (error: any) {
    console.error(`[tlon-media] Error downloading ${url}: ${error?.message ?? String(error)}`);
    return null;
--- a/extensions/tlon/src/urbit/upload.test.ts
+++ b/extensions/tlon/src/urbit/upload.test.ts
@@ -1,9 +1,13 @@
 import { describe, expect, it, vi, afterEach, beforeEach } from "vitest";

-// Mock urbitFetch
-vi.mock("./fetch.js", () => ({
-  urbitFetch: vi.fn(),
-}));
+// Mock fetchWithSsrFGuard from plugin-sdk
+vi.mock("openclaw/plugin-sdk", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("openclaw/plugin-sdk")>();
+  return {
+    ...actual,
+    fetchWithSsrFGuard: vi.fn(),
+  };
+});

 // Mock @tloncorp/api
 vi.mock("@tloncorp/api", () => ({
@@ -20,15 +24,15 @@ describe("uploadImageFromUrl", () => {
  });

  it("fetches image and calls uploadFile, returns uploaded URL", async () => {
-    const { urbitFetch } = await import("./fetch.js");
-    const mockUrbitFetch = vi.mocked(urbitFetch);
+    const { fetchWithSsrFGuard } = await import("openclaw/plugin-sdk");
+    const mockFetch = vi.mocked(fetchWithSsrFGuard);

    const { uploadFile } = await import("@tloncorp/api");
    const mockUploadFile = vi.mocked(uploadFile);

-    // Mock urbitFetch to return a successful response with a blob
+    // Mock fetchWithSsrFGuard to return a successful response with a blob
    const mockBlob = new Blob(["fake-image"], { type: "image/png" });
-    mockUrbitFetch.mockResolvedValue({
+    mockFetch.mockResolvedValue({
      response: {
        ok: true,
        headers: new Headers({ "content-type": "image/png" }),
@@ -55,11 +59,11 @@ describe("uploadImageFromUrl", () => {
  });

  it("returns original URL if fetch fails", async () => {
-    const { urbitFetch } = await import("./fetch.js");
-    const mockUrbitFetch = vi.mocked(urbitFetch);
+    const { fetchWithSsrFGuard } = await import("openclaw/plugin-sdk");
+    const mockFetch = vi.mocked(fetchWithSsrFGuard);

-    // Mock urbitFetch to return a failed response
-    mockUrbitFetch.mockResolvedValue({
+    // Mock fetchWithSsrFGuard to return a failed response
+    mockFetch.mockResolvedValue({
      response: {
        ok: false,
        status: 404,
@@ -75,15 +79,15 @@ describe("uploadImageFromUrl", () => {
  });

  it("returns original URL if upload fails", async () => {
-    const { urbitFetch } = await import("./fetch.js");
-    const mockUrbitFetch = vi.mocked(urbitFetch);
+    const { fetchWithSsrFGuard } = await import("openclaw/plugin-sdk");
+    const mockFetch = vi.mocked(fetchWithSsrFGuard);

    const { uploadFile } = await import("@tloncorp/api");
    const mockUploadFile = vi.mocked(uploadFile);

-    // Mock urbitFetch to return a successful response
+    // Mock fetchWithSsrFGuard to return a successful response
    const mockBlob = new Blob(["fake-image"], { type: "image/png" });
-    mockUrbitFetch.mockResolvedValue({
+    mockFetch.mockResolvedValue({
      response: {
        ok: true,
        headers: new Headers({ "content-type": "image/png" }),
@@ -123,14 +127,14 @@ describe("uploadImageFromUrl", () => {
  });

  it("extracts filename from URL path", async () => {
-    const { urbitFetch } = await import("./fetch.js");
-    const mockUrbitFetch = vi.mocked(urbitFetch);
+    const { fetchWithSsrFGuard } = await import("openclaw/plugin-sdk");
+    const mockFetch = vi.mocked(fetchWithSsrFGuard);

    const { uploadFile } = await import("@tloncorp/api");
    const mockUploadFile = vi.mocked(uploadFile);

    const mockBlob = new Blob(["fake-image"], { type: "image/jpeg" });
-    mockUrbitFetch.mockResolvedValue({
+    mockFetch.mockResolvedValue({
      response: {
        ok: true,
        headers: new Headers({ "content-type": "image/jpeg" }),
@@ -153,14 +157,14 @@ describe("uploadImageFromUrl", () => {
  });

  it("uses default filename when URL has no path", async () => {
-    const { urbitFetch } = await import("./fetch.js");
-    const mockUrbitFetch = vi.mocked(urbitFetch);
+    const { fetchWithSsrFGuard } = await import("openclaw/plugin-sdk");
+    const mockFetch = vi.mocked(fetchWithSsrFGuard);

    const { uploadFile } = await import("@tloncorp/api");
    const mockUploadFile = vi.mocked(uploadFile);

    const mockBlob = new Blob(["fake-image"], { type: "image/png" });
-    mockUrbitFetch.mockResolvedValue({
+    mockFetch.mockResolvedValue({
      response: {
        ok: true,
        headers: new Headers({ "content-type": "image/png" }),
--- a/extensions/tlon/src/urbit/upload.ts
+++ b/extensions/tlon/src/urbit/upload.ts
@@ -2,8 +2,8 @@
 * Upload an image from a URL to Tlon storage.
 */
 import { uploadFile } from "@tloncorp/api";
+import { fetchWithSsrFGuard } from "openclaw/plugin-sdk";
 import { getDefaultSsrFPolicy } from "./context.js";
-import { urbitFetch } from "./fetch.js";

 /**
 * Fetch an image from a URL and upload it to Tlon storage.
@@ -21,34 +21,38 @@ export async function uploadImageFromUrl(imageUrl: string): Promise<string> {
    }

    // Fetch the image with SSRF protection
-    const { response } = await urbitFetch({
-      baseUrl: imageUrl,
-      path: "",
+    // Use fetchWithSsrFGuard directly (not urbitFetch) to preserve the full URL path
+    const { response, release } = await fetchWithSsrFGuard({
+      url: imageUrl,
      init: { method: "GET" },
-      ssrfPolicy: getDefaultSsrFPolicy(),
+      policy: getDefaultSsrFPolicy(),
      auditContext: "tlon-upload-image",
    });

-    if (!response.ok) {
-      console.warn(`[tlon] Failed to fetch image from ${imageUrl}: ${response.status}`);
-      return imageUrl;
+    try {
+      if (!response.ok) {
+        console.warn(`[tlon] Failed to fetch image from ${imageUrl}: ${response.status}`);
+        return imageUrl;
+      }
+
+      const contentType = response.headers.get("content-type") || "image/png";
+      const blob = await response.blob();
+
+      // Extract filename from URL or use a default
+      const urlPath = new URL(imageUrl).pathname;
+      const fileName = urlPath.split("/").pop() || `upload-${Date.now()}.png`;
+
+      // Upload to Tlon storage
+      const result = await uploadFile({
+        blob,
+        fileName,
+        contentType,
+      });
+
+      return result.url;
+    } finally {
+      await release();
    }
-
-    const contentType = response.headers.get("content-type") || "image/png";
-    const blob = await response.blob();
-
-    // Extract filename from URL or use a default
-    const urlPath = new URL(imageUrl).pathname;
-    const fileName = urlPath.split("/").pop() || `upload-${Date.now()}.png`;
-
-    // Upload to Tlon storage
-    const result = await uploadFile({
-      blob,
-      fileName,
-      contentType,
-    });
-
-    return result.url;
  } catch (err) {
    console.warn(`[tlon] Failed to upload image, using original URL: ${err}`);
    return imageUrl;