fix(security): cap Slack media downloads and validate Slack file URLs (#6639)

davidiach · steipete · web-flow · commit 4e4ed2ea1793 · 2026-02-02T00:48:07.000-08:00
* Security: cap Slack media downloads and validate Slack file URLs * Security: relax web media fetch cap for compression * Fixes: sync pi-coding-agent options * Fixes: align system prompt override type * Slack: clarify fetchImpl assumptions * fix: respect raw media fetch cap (#6639) (thanks @davidiach) --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,6 +25,7 @@ Docs: https://docs.openclaw.ai
 
 - Auto-reply: avoid referencing workspace files in /new greeting prompt. (#5706) Thanks @bravostation.
 - Tools: treat `"*"` tool allowlist entries as valid to avoid spurious unknown-entry warnings.
+- Slack: harden media fetch limits and Slack file URL validation. (#6639) Thanks @davidiach.
 - Process: resolve Windows `spawn()` failures for npm-family CLIs by appending `.cmd` when needed. (#5815) Thanks @thejhinvirtuoso.
 - Discord: resolve PluralKit proxied senders for allowlists and labels. (#5838) Thanks @thewilloftheshadow.
 - Agents: ensure OpenRouter attribution headers apply in the embedded runner.
diff --git a/src/agents/pi-embedded-runner/system-prompt.ts b/src/agents/pi-embedded-runner/system-prompt.ts
@@ -74,11 +74,8 @@ export function buildEmbeddedSystemPrompt(params: {
   });
 }
 
-export function createSystemPromptOverride(
-  systemPrompt: string,
-): (defaultPrompt?: string) => string {
-  const trimmed = systemPrompt.trim();
-  return () => trimmed;
+export function createSystemPromptOverride(systemPrompt: string): string {
+  return systemPrompt.trim();
 }
 
 export function applySystemPromptOverrideToSession(
diff --git a/src/slack/monitor/media.test.ts b/src/slack/monitor/media.test.ts
@@ -40,6 +40,17 @@ describe("fetchWithSlackAuth", () => {
     });
   });
 
+  it("rejects non-Slack hosts to avoid leaking tokens", async () => {
+    const { fetchWithSlackAuth } = await import("./media.js");
+
+    await expect(
+      fetchWithSlackAuth("https://example.com/test.jpg", "xoxb-test-token"),
+    ).rejects.toThrow(/non-Slack host|non-Slack/i);
+
+    // Should fail fast without attempting a fetch.
+    expect(mockFetch).not.toHaveBeenCalled();
+  });
+
   it("follows redirects without Authorization header", async () => {
     const { fetchWithSlackAuth } = await import("./media.js");
 
diff --git a/src/slack/monitor/media.ts b/src/slack/monitor/media.ts
@@ -4,15 +4,57 @@ import type { SlackFile } from "../types.js";
 import { fetchRemoteMedia } from "../../media/fetch.js";
 import { saveMediaBuffer } from "../../media/store.js";
 
+function normalizeHostname(hostname: string): string {
+  const normalized = hostname.trim().toLowerCase().replace(/\.$/, "");
+  if (normalized.startsWith("[") && normalized.endsWith("]")) {
+    return normalized.slice(1, -1);
+  }
+  return normalized;
+}
+
+function isSlackHostname(hostname: string): boolean {
+  const normalized = normalizeHostname(hostname);
+  if (!normalized) {
+    return false;
+  }
+  // Slack-hosted files typically come from *.slack.com and redirect to Slack CDN domains.
+  // Include a small allowlist of known Slack domains to avoid leaking tokens if a file URL
+  // is ever spoofed or mishandled.
+  const allowedSuffixes = ["slack.com", "slack-edge.com", "slack-files.com"];
+  return allowedSuffixes.some(
+    (suffix) => normalized === suffix || normalized.endsWith(`.${suffix}`),
+  );
+}
+
+function assertSlackFileUrl(rawUrl: string): URL {
+  let parsed: URL;
+  try {
+    parsed = new URL(rawUrl);
+  } catch {
+    throw new Error(`Invalid Slack file URL: ${rawUrl}`);
+  }
+  if (parsed.protocol !== "https:") {
+    throw new Error(`Refusing Slack file URL with non-HTTPS protocol: ${parsed.protocol}`);
+  }
+  if (!isSlackHostname(parsed.hostname)) {
+    throw new Error(
+      `Refusing to send Slack token to non-Slack host "${parsed.hostname}" (url: ${rawUrl})`,
+    );
+  }
+  return parsed;
+}
+
 /**
  * Fetches a URL with Authorization header, handling cross-origin redirects.
  * Node.js fetch strips Authorization headers on cross-origin redirects for security.
- * Slack's files.slack.com URLs redirect to CDN domains with pre-signed URLs that
- * don't need the Authorization header, so we handle the initial auth request manually.
+ * Slack's file URLs redirect to CDN domains with pre-signed URLs that don't need the
+ * Authorization header, so we handle the initial auth request manually.
  */
 export async function fetchWithSlackAuth(url: string, token: string): Promise<Response> {
+  const parsed = assertSlackFileUrl(url);
+
   // Initial request with auth and manual redirect handling
-  const initialRes = await fetch(url, {
+  const initialRes = await fetch(parsed.href, {
     headers: { Authorization: `Bearer ${token}` },
     redirect: "manual",
   });
@@ -29,11 +71,16 @@ export async function fetchWithSlackAuth(url: string, token: string): Promise<Re
   }
 
   // Resolve relative URLs against the original
-  const resolvedUrl = new URL(redirectUrl, url).toString();
+  const resolvedUrl = new URL(redirectUrl, parsed.href);
+
+  // Only follow safe protocols (we do NOT include Authorization on redirects).
+  if (resolvedUrl.protocol !== "https:") {
+    return initialRes;
+  }
 
   // Follow the redirect without the Authorization header
   // (Slack's CDN URLs are pre-signed and don't need it)
-  return fetch(resolvedUrl, { redirect: "follow" });
+  return fetch(resolvedUrl.toString(), { redirect: "follow" });
 }
 
 export async function resolveSlackMedia(params: {
@@ -52,8 +99,9 @@ export async function resolveSlackMedia(params: {
       continue;
     }
     try {
-      // Note: We ignore init options because fetchWithSlackAuth handles
-      // redirect behavior specially. fetchRemoteMedia only passes the URL.
+      // Note: fetchRemoteMedia calls fetchImpl(url) with the URL string today and
+      // handles size limits internally. We ignore init options because
+      // fetchWithSlackAuth handles redirect/auth behavior specially.
       const fetchImpl: FetchLike = (input) => {
         const inputUrl =
           typeof input === "string" ? input : input instanceof URL ? input.href : input.url;
@@ -63,6 +111,7 @@ export async function resolveSlackMedia(params: {
         url,
         fetchImpl,
         filePathHint: file.name,
+        maxBytes: params.maxBytes,
       });
       if (fetched.buffer.byteLength > params.maxBytes) {
         continue;
diff --git a/src/web/media.test.ts b/src/web/media.test.ts
@@ -4,7 +4,7 @@ import path from "node:path";
 import sharp from "sharp";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { optimizeImageToPng } from "../media/image-ops.js";
-import { loadWebMedia, optimizeImageToJpeg } from "./media.js";
+import { loadWebMedia, loadWebMediaRaw, optimizeImageToJpeg } from "./media.js";
 
 const tmpFiles: string[] = [];
 
@@ -106,6 +106,22 @@ describe("web media loading", () => {
     fetchMock.mockRestore();
   });
 
+  it("respects maxBytes for raw URL fetches", async () => {
+    const fetchMock = vi.spyOn(globalThis, "fetch").mockResolvedValueOnce({
+      ok: true,
+      body: true,
+      arrayBuffer: async () => Buffer.alloc(2048).buffer,
+      headers: { get: () => "image/png" },
+      status: 200,
+    } as Response);
+
+    await expect(loadWebMediaRaw("https://example.com/too-big.png", 1024)).rejects.toThrow(
+      /exceeds maxBytes 1024/i,
+    );
+
+    fetchMock.mockRestore();
+  });
+
   it("uses content-disposition filename when available", async () => {
     const fetchMock = vi.spyOn(globalThis, "fetch").mockResolvedValueOnce({
       ok: true,
diff --git a/src/web/media.ts b/src/web/media.ts
@@ -200,7 +200,16 @@ async function loadWebMediaInternal(
   };
 
   if (/^https?:\/\//i.test(mediaUrl)) {
-    const fetched = await fetchRemoteMedia({ url: mediaUrl });
+    // Enforce a download cap during fetch to avoid unbounded memory usage.
+    // For optimized images, allow fetching larger payloads before compression.
+    const defaultFetchCap = maxBytesForKind("unknown");
+    const fetchCap =
+      maxBytes === undefined
+        ? defaultFetchCap
+        : optimizeImages
+          ? Math.max(maxBytes, defaultFetchCap)
+          : maxBytes;
+    const fetched = await fetchRemoteMedia({ url: mediaUrl, maxBytes: fetchCap });
     const { buffer, contentType, fileName } = fetched;
     const kind = mediaKindFromMime(contentType);
     return await clampAndFinalize({ buffer, contentType, kind, fileName });

Original file line number	Diff line number	Diff line change
`@@ -74,11 +74,8 @@ export function buildEmbeddedSystemPrompt(params: {`
`74`	`74`	`});`
`75`	`75`	`}`
`76`	`76`
`77`		`-export function createSystemPromptOverride(`
`78`		`- systemPrompt: string,`
`79`		`-): (defaultPrompt?: string) => string {`
`80`		`- const trimmed = systemPrompt.trim();`
`81`		`- return () => trimmed;`
	`77`	`+export function createSystemPromptOverride(systemPrompt: string): string {`
	`78`	`+ return systemPrompt.trim();`
`82`	`79`	`}`
`83`	`80`
`84`	`81`	`export function applySystemPromptOverrideToSession(`