Merge 062760fc28 into 6af205a13a

package.json

@@ -187,6 +187,7 @@
     "file-type": "^21.3.0",
     "grammy": "^1.39.3",
     "hono": "4.11.4",
+    "ipaddr.js": "^2.2.0",
     "jiti": "^2.6.1",
     "json5": "^2.2.3",
     "jszip": "^3.10.1",

pnpm-lock.yaml

@@ -112,6 +112,9 @@ importers:
       hono:
         specifier: 4.11.4
         version: 4.11.4
+      ipaddr.js:
+        specifier: ^2.2.0
+        version: 2.3.0
       jiti:
         specifier: ^2.6.1
         version: 2.6.1
@@ -3870,6 +3873,10 @@ packages:
     resolution: {integrity: sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==}
     engines: {node: '>= 0.10'}
 
+  ipaddr.js@2.3.0:
+    resolution: {integrity: sha512-Zv/pA+ciVFbCSBBjGfaKUya/CcGmUHzTydLMaTwrUUEM2DIEO3iZvueGxmacvmN50fGpGVKeTXpb2LcYQxeVdg==}
+    engines: {node: '>= 10'}
+
   ipull@3.9.3:
     resolution: {integrity: sha512-ZMkxaopfwKHwmEuGDYx7giNBdLxbHbRCWcQVA1D2eqE4crUguupfxej6s7UqbidYEwT69dkyumYkY8DPHIxF9g==}
     engines: {node: '>=18.0.0'}
@@ -9865,6 +9872,8 @@ snapshots:
 
   ipaddr.js@1.9.1: {}
 
+  ipaddr.js@2.3.0: {}
+
   ipull@3.9.3:
     dependencies:
       '@tinyhttp/content-disposition': 2.2.2

src/agents/cli-credentials.ts

@@ -1,4 +1,4 @@
-import { execSync } from "node:child_process";
+import { execSync, spawnSync } from "node:child_process";
 import { createHash } from "node:crypto";
 import fs from "node:fs";
 import path from "node:path";
@@ -78,6 +78,37 @@ type ClaudeCliWriteOptions = ClaudeCliFileOptions & {
 
 type ExecSyncFn = typeof execSync;
 
+// Secure keychain operations using spawnSync to prevent shell injection
+function secureKeychainFind(service: string, account?: string): string | null {
+  const args = ["find-generic-password", "-s", service];
+  if (account) {
+    args.push("-a", account);
+  }
+  args.push("-w");
+  const result = spawnSync("security", args, {
+    encoding: "utf8",
+    timeout: 5000,
+    stdio: ["pipe", "pipe", "pipe"],
+  });
+  if (result.status !== 0 || result.error) {
+    return null;
+  }
+  return result.stdout?.trim() ?? null;
+}
+
+function secureKeychainWrite(service: string, account: string, value: string): boolean {
+  const result = spawnSync(
+    "security",
+    ["add-generic-password", "-U", "-s", service, "-a", account, "-w", value],
+    {
+      encoding: "utf8",
+      timeout: 5000,
+      stdio: ["pipe", "pipe", "pipe"],
+    },
+  );
+  return result.status === 0 && !result.error;
+}
+
 function resolveClaudeCliCredentialsPath(homeDir?: string) {
   const baseDir = homeDir ?? resolveUserPath("~");
   return path.join(baseDir, CLAUDE_CLI_CREDENTIALS_RELATIVE_PATH);
@@ -113,20 +144,22 @@ function readCodexKeychainCredentials(options?: {
 }): CodexCliCredential | null {
   const platform = options?.platform ?? process.platform;
   if (platform !== "darwin") return null;
-  const execSyncImpl = options?.execSync ?? execSync;
+  // Note: execSync option preserved for test mocking but secure helper used by default
+  const _execSyncImpl = options?.execSync;
 
   const codexHome = resolveCodexHomePath();
   const account = computeCodexKeychainAccount(codexHome);
 
   try {
-    const secret = execSyncImpl(
+    // Use secure helper to prevent shell injection
-      `security find-generic-password -s "Codex Auth" -a "${account}" -w`,
+    const secret = _execSyncImpl
-      {
+      ? _execSyncImpl(`security find-generic-password -s "Codex Auth" -a "${account}" -w`, {
-        encoding: "utf8",
+          encoding: "utf8",
-        timeout: 5000,
+          timeout: 5000,
-        stdio: ["pipe", "pipe", "pipe"],
+          stdio: ["pipe", "pipe", "pipe"],
-      },
+        }).trim()
-    ).trim();
+      : secureKeychainFind("Codex Auth", account);
+    if (!secret) return null;
 
     const parsed = JSON.parse(secret) as Record<string, unknown>;
     const tokens = parsed.tokens as Record<string, unknown> | undefined;
@@ -186,16 +219,19 @@ function readQwenCliCredentials(options?: { homeDir?: string }): QwenCliCredenti
   };
 }
 
-function readClaudeCliKeychainCredentials(
+function readClaudeCliKeychainCredentials(execSyncImpl?: ExecSyncFn): ClaudeCliCredential | null {
-  execSyncImpl: ExecSyncFn = execSync,
-): ClaudeCliCredential | null {
   try {
-    const result = execSyncImpl(
+    // Use secure helper to prevent shell injection (unless test mock provided)
-      `security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w`,
+    const result = execSyncImpl
-      { encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
+      ? execSyncImpl(`security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w`, {
-    );
+          encoding: "utf8",
+          timeout: 5000,
+          stdio: ["pipe", "pipe", "pipe"],
+        })
+      : secureKeychainFind(CLAUDE_CLI_KEYCHAIN_SERVICE);
+    if (!result) return null;
 
-    const data = JSON.parse(result.trim());
+    const data = JSON.parse(typeof result === "string" ? result.trim() : result);
     const claudeOauth = data?.claudeAiOauth;
     if (!claudeOauth || typeof claudeOauth !== "object") return null;
 
@@ -311,14 +347,20 @@ export function writeClaudeCliKeychainCredentials(
   newCredentials: OAuthCredentials,
   options?: { execSync?: ExecSyncFn },
 ): boolean {
-  const execSyncImpl = options?.execSync ?? execSync;
+  const execSyncImpl = options?.execSync;
   try {
-    const existingResult = execSyncImpl(
+    // Use secure helper to prevent shell injection (unless test mock provided)
-      `security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w 2>/dev/null`,
+    const existingResult = execSyncImpl
-      { encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
+      ? execSyncImpl(
-    );
+          `security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w 2>/dev/null`,
+          { encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
+        )
+      : secureKeychainFind(CLAUDE_CLI_KEYCHAIN_SERVICE);
+    if (!existingResult) return false;
 
-    const existingData = JSON.parse(existingResult.trim());
+    const existingData = JSON.parse(
+      typeof existingResult === "string" ? existingResult.trim() : existingResult,
+    );
     const existingOauth = existingData?.claudeAiOauth;
     if (!existingOauth || typeof existingOauth !== "object") {
       return false;
@@ -333,10 +375,23 @@ export function writeClaudeCliKeychainCredentials(
 
     const newValue = JSON.stringify(existingData);
 
-    execSyncImpl(
+    // Use secure helper for write (unless test mock provided)
-      `security add-generic-password -U -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -a "${CLAUDE_CLI_KEYCHAIN_ACCOUNT}" -w '${newValue.replace(/'/g, "'\"'\"'")}'`,
+    if (execSyncImpl) {
-      { encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
+      execSyncImpl(
-    );
+        `security add-generic-password -U -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -a "${CLAUDE_CLI_KEYCHAIN_ACCOUNT}" -w '${newValue.replace(/'/g, "'\"'\"'")}'`,
+        { encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
+      );
+    } else {
+      const writeOk = secureKeychainWrite(
+        CLAUDE_CLI_KEYCHAIN_SERVICE,
+        CLAUDE_CLI_KEYCHAIN_ACCOUNT,
+        newValue,
+      );
+      if (!writeOk) {
+        log.warn("failed to write credentials to claude cli keychain via secure helper");
+        return false;
+      }
+    }
 
     log.info("wrote refreshed credentials to claude cli keychain", {
       expires: new Date(newCredentials.expires).toISOString(),

src/gateway/auth.test.ts

@@ -1,6 +1,11 @@
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, beforeEach } from "vitest";
 
-import { authorizeGatewayConnect } from "./auth.js";
+import {
+  authorizeGatewayConnect,
+  checkRateLimit,
+  recordAuthFailure,
+  resetRateLimiter,
+} from "./auth.js";
 
 describe("gateway auth", () => {
   it("does not throw when req is missing socket", async () => {
@@ -100,3 +105,85 @@ describe("gateway auth", () => {
     expect(res.user).toBe("peter");
   });
 });
+
+describe("rate limiting", () => {
+  const mockReq = (ip: string) => ({ socket: { remoteAddress: ip } }) as never;
+
+  beforeEach(() => {
+    resetRateLimiter();
+  });
+
+  it("allows requests initially", () => {
+    const result = checkRateLimit(mockReq("192.0.2.1"));
+    expect(result.allowed).toBe(true);
+    expect(result.retryAfterMs).toBeUndefined();
+  });
+
+  it("allows requests after fewer than 5 failures", () => {
+    const req = mockReq("192.0.2.2");
+    for (let i = 0; i < 4; i++) {
+      recordAuthFailure(req);
+    }
+    const result = checkRateLimit(req);
+    expect(result.allowed).toBe(true);
+  });
+
+  it("blocks requests after 5 failures", () => {
+    const req = mockReq("192.0.2.3");
+    for (let i = 0; i < 5; i++) {
+      recordAuthFailure(req);
+    }
+    const result = checkRateLimit(req);
+    expect(result.allowed).toBe(false);
+    expect(result.retryAfterMs).toBeGreaterThan(0);
+    expect(result.retryAfterMs).toBeLessThanOrEqual(60000);
+  });
+
+  it("tracks different IPs independently", () => {
+    const req1 = mockReq("192.0.2.10");
+    const req2 = mockReq("192.0.2.20");
+
+    // Exhaust rate limit for req1
+    for (let i = 0; i < 5; i++) {
+      recordAuthFailure(req1);
+    }
+
+    // req1 should be blocked
+    expect(checkRateLimit(req1).allowed).toBe(false);
+
+    // req2 should still be allowed
+    expect(checkRateLimit(req2).allowed).toBe(true);
+  });
+
+  it("uses 'unknown' key when socket is missing", () => {
+    const reqWithoutSocket = {} as never;
+    for (let i = 0; i < 5; i++) {
+      recordAuthFailure(reqWithoutSocket);
+    }
+    expect(checkRateLimit(reqWithoutSocket).allowed).toBe(false);
+  });
+
+  it("integrates with authorizeGatewayConnect", async () => {
+    const req = mockReq("192.0.2.100");
+
+    // Fail auth 5 times
+    for (let i = 0; i < 5; i++) {
+      const res = await authorizeGatewayConnect({
+        auth: { mode: "token", token: "secret", allowTailscale: false },
+        connectAuth: { token: "wrong" },
+        req,
+      });
+      expect(res.ok).toBe(false);
+      expect(res.reason).toBe("token_mismatch");
+    }
+
+    // 6th attempt should be rate limited
+    const res = await authorizeGatewayConnect({
+      auth: { mode: "token", token: "secret", allowTailscale: false },
+      connectAuth: { token: "correct" }, // Even correct token should be blocked
+      req,
+    });
+    expect(res.ok).toBe(false);
+    expect(res.reason).toBe("rate_limited");
+  });
+});

src/gateway/auth.ts

@@ -3,6 +3,76 @@ import type { IncomingMessage } from "node:http";
 import type { GatewayAuthConfig, GatewayTailscaleMode } from "../config/config.js";
 import { readTailscaleWhoisIdentity, type TailscaleWhoisIdentity } from "../infra/tailscale.js";
 import { isTrustedProxyAddress, parseForwardedForClientIp, resolveGatewayClientIp } from "./net.js";
+
+// Rate limiting for brute-force protection
+type RateLimitEntry = {
+  failures: number;
+  lastAttempt: number;
+  lockedUntil: number;
+};
+
+const rateLimitMap = new Map<string, RateLimitEntry>();
+const MAX_FAILURES = 5;
+const LOCKOUT_MS = 60_000; // 1 minute
+const WINDOW_MS = 300_000; // 5 minutes
+const CLEANUP_INTERVAL_MS = 60_000;
+
+// Cleanup stale entries to prevent memory leak
+let cleanupInterval: ReturnType<typeof setInterval> | null = null;
+function ensureCleanupInterval(): void {
+  if (cleanupInterval) return;
+  cleanupInterval = setInterval(() => {
+    const now = Date.now();
+    for (const [key, entry] of rateLimitMap) {
+      if (now - entry.lastAttempt > WINDOW_MS) {
+        rateLimitMap.delete(key);
+      }
+    }
+  }, CLEANUP_INTERVAL_MS);
+  // Don't keep process alive just for cleanup
+  cleanupInterval.unref?.();
+}
+
+function getRateLimitKey(req?: IncomingMessage): string {
+  // Use socket address, not forwarded headers (spoofable)
+  return req?.socket?.remoteAddress ?? "unknown";
+}
+
+export function checkRateLimit(req?: IncomingMessage): { allowed: boolean; retryAfterMs?: number } {
+  ensureCleanupInterval();
+  const key = getRateLimitKey(req);
+  const now = Date.now();
+  const entry = rateLimitMap.get(key);
+
+  if (entry && now < entry.lockedUntil) {
+    return { allowed: false, retryAfterMs: entry.lockedUntil - now };
+  }
+
+  return { allowed: true };
+}
+
+export function recordAuthFailure(req?: IncomingMessage): void {
+  ensureCleanupInterval();
+  const key = getRateLimitKey(req);
+  const now = Date.now();
+  const entry = rateLimitMap.get(key) ?? { failures: 0, lastAttempt: 0, lockedUntil: 0 };
+
+  entry.failures += 1;
+  entry.lastAttempt = now;
+
+  if (entry.failures >= MAX_FAILURES) {
+    entry.lockedUntil = now + LOCKOUT_MS;
+    entry.failures = 0; // Reset after lockout
+  }
+
+  rateLimitMap.set(key, entry);
+}
+
+// Test helper
+export function resetRateLimiter(): void {
+  rateLimitMap.clear();
+}
+
 export type ResolvedGatewayAuthMode = "token" | "password";
 
 export type ResolvedGatewayAuth = {
@@ -209,6 +279,13 @@ export async function authorizeGatewayConnect(params: {
   tailscaleWhois?: TailscaleWhoisLookup;
 }): Promise<GatewayAuthResult> {
   const { auth, connectAuth, req, trustedProxies } = params;
+
+  // Rate limit check - prevent brute-force attacks
+  const rateLimit = checkRateLimit(req);
+  if (!rateLimit.allowed) {
+    return { ok: false, reason: "rate_limited" };
+  }
+
   const tailscaleWhois = params.tailscaleWhois ?? readTailscaleWhoisIdentity;
   const localDirect = isLocalDirectRequest(req, trustedProxies);
 
@@ -224,6 +301,14 @@ export async function authorizeGatewayConnect(params: {
         user: tailscaleCheck.user.login,
       };
     }
+    // Record auth failure for Tailscale attempts that had user headers but failed verification
+    // (don't count missing headers as a "failure" since that's just falling through to token/password)
+    if (
+      tailscaleCheck.reason === "tailscale_user_mismatch" ||
+      tailscaleCheck.reason === "tailscale_whois_failed"
+    ) {
+      recordAuthFailure(req);
+    }
   }
 
   if (auth.mode === "token") {
@@ -234,6 +319,7 @@ export async function authorizeGatewayConnect(params: {
       return { ok: false, reason: "token_missing" };
     }
     if (!safeEqual(connectAuth.token, auth.token)) {
+      recordAuthFailure(req);
       return { ok: false, reason: "token_mismatch" };
     }
     return { ok: true, method: "token" };
@@ -248,6 +334,7 @@ export async function authorizeGatewayConnect(params: {
       return { ok: false, reason: "password_missing" };
     }
     if (!safeEqual(password, auth.password)) {
+      recordAuthFailure(req);
       return { ok: false, reason: "password_mismatch" };
     }
     return { ok: true, method: "password" };

src/logging/logger.ts

@@ -82,7 +82,29 @@ export function isFileLogLevelEnabled(level: LogLevel): boolean {
 }
 
 function buildLogger(settings: ResolvedSettings): TsLogger<LogObj> {
-  fs.mkdirSync(path.dirname(settings.file), { recursive: true });
+  const logDir = path.dirname(settings.file);
+  // Create log directory with secure permissions (owner-only access)
+  try {
+    if (!fs.existsSync(logDir)) {
+      fs.mkdirSync(logDir, { recursive: true, mode: 0o700 });
+    } else {
+      // Ensure existing dir has secure permissions
+      fs.chmodSync(logDir, 0o700);
+    }
+  } catch (err) {
+    // Fall back to default umask if chmod fails (e.g., on Windows)
+    // Log a warning since permissions may not be secure
+    if (process.platform !== "win32") {
+      console.warn(
+        `[clawdbot] Could not set secure permissions on log directory ${logDir}: ${err}`,
+      );
+    }
+    try {
+      fs.mkdirSync(logDir, { recursive: true });
+    } catch {
+      // Directory may already exist, ignore
+    }
+  }
   // Clean up stale rolling logs when using a dated log filename.
   if (isRollingPath(settings.file)) {
     pruneOldRollingLogs(path.dirname(settings.file));

src/media/fetch.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from "vitest";
 
-import { fetchRemoteMedia } from "./fetch.js";
+import { fetchRemoteMedia, _isUrlAllowed } from "./fetch.js";
 
 function makeStream(chunks: Uint8Array[]) {
   return new ReadableStream<Uint8Array>({
@@ -44,4 +44,128 @@ describe("fetchRemoteMedia", () => {
       }),
     ).rejects.toThrow("exceeds maxBytes");
   });
+
+  it("blocks localhost URLs", async () => {
+    await expect(fetchRemoteMedia({ url: "http://localhost:8080/file" })).rejects.toThrow(
+      "URL not allowed",
+    );
+
+    await expect(fetchRemoteMedia({ url: "http://localhost/file" })).rejects.toThrow(
+      "URL not allowed",
+    );
+  });
+
+  it("blocks private IP ranges", async () => {
+    // 10.x.x.x
+    await expect(fetchRemoteMedia({ url: "http://10.0.0.1/file" })).rejects.toThrow(
+      "URL not allowed",
+    );
+    // 172.16.x.x - 172.31.x.x
+    await expect(fetchRemoteMedia({ url: "http://172.16.0.1/file" })).rejects.toThrow(
+      "URL not allowed",
+    );
+    // 192.168.x.x
+    await expect(fetchRemoteMedia({ url: "http://192.168.1.1/file" })).rejects.toThrow(
+      "URL not allowed",
+    );
+  });
+
+  it("blocks loopback addresses", async () => {
+    await expect(fetchRemoteMedia({ url: "http://127.0.0.1/file" })).rejects.toThrow(
+      "URL not allowed",
+    );
+    await expect(fetchRemoteMedia({ url: "http://127.0.0.5/file" })).rejects.toThrow(
+      "URL not allowed",
+    );
+  });
+
+  it("blocks IPv6 loopback", async () => {
+    await expect(fetchRemoteMedia({ url: "http://[::1]/file" })).rejects.toThrow("URL not allowed");
+  });
+
+  it("blocks link-local addresses (AWS metadata)", async () => {
+    await expect(
+      fetchRemoteMedia({ url: "http://169.254.169.254/latest/meta-data/" }),
+    ).rejects.toThrow("URL not allowed");
+  });
+
+  it("blocks redirects to private IPs", async () => {
+    const fetchImpl = async () =>
+      new Response(null, {
+        status: 302,
+        headers: { location: "http://127.0.0.1/admin" },
+      });
+
+    await expect(
+      fetchRemoteMedia({ url: "https://example.com/redirect", fetchImpl }),
+    ).rejects.toThrow("Redirect blocked");
+  });
+
+  it("limits redirect count", async () => {
+    let redirectCount = 0;
+    const fetchImpl = async () => {
+      redirectCount++;
+      return new Response(null, {
+        status: 302,
+        headers: { location: `https://example.com/redirect${redirectCount}` },
+      });
+    };
+
+    await expect(fetchRemoteMedia({ url: "https://example.com/start", fetchImpl })).rejects.toThrow(
+      "Too many redirects",
+    );
+  });
+});
+
+describe("isUrlAllowed (SSRF protection)", () => {
+  it("allows valid external URLs", () => {
+    expect(_isUrlAllowed("https://example.com/file.jpg")).toBe(true);
+    expect(_isUrlAllowed("http://cdn.example.org/image.png")).toBe(true);
+    expect(_isUrlAllowed("https://8.8.8.8/file")).toBe(true);
+  });
+
+  it("blocks non-http protocols", () => {
+    expect(_isUrlAllowed("file:///etc/passwd")).toBe(false);
+    expect(_isUrlAllowed("ftp://example.com/file")).toBe(false);
+    expect(_isUrlAllowed("data:text/plain,hello")).toBe(false);
+  });
+
+  it("blocks localhost variations", () => {
+    expect(_isUrlAllowed("http://localhost/file")).toBe(false);
+    expect(_isUrlAllowed("http://localhost:3000/api")).toBe(false);
+    expect(_isUrlAllowed("http://sub.localhost/file")).toBe(false);
+  });
+
+  it("blocks loopback IPs", () => {
+    expect(_isUrlAllowed("http://127.0.0.1/file")).toBe(false);
+    expect(_isUrlAllowed("http://127.0.0.255/file")).toBe(false);
+    expect(_isUrlAllowed("http://[::1]/file")).toBe(false);
+  });
+
+  it("blocks private IP ranges", () => {
+    // Class A private
+    expect(_isUrlAllowed("http://10.0.0.1/file")).toBe(false);
+    expect(_isUrlAllowed("http://10.255.255.255/file")).toBe(false);
+    // Class B private
+    expect(_isUrlAllowed("http://172.16.0.1/file")).toBe(false);
+    expect(_isUrlAllowed("http://172.31.255.255/file")).toBe(false);
+    // Class C private
+    expect(_isUrlAllowed("http://192.168.0.1/file")).toBe(false);
+    expect(_isUrlAllowed("http://192.168.255.255/file")).toBe(false);
+  });
+
+  it("blocks link-local addresses", () => {
+    expect(_isUrlAllowed("http://169.254.169.254/latest/meta-data/")).toBe(false);
+    expect(_isUrlAllowed("http://169.254.0.1/file")).toBe(false);
+  });
+
+  it("blocks unspecified addresses", () => {
+    expect(_isUrlAllowed("http://0.0.0.0/file")).toBe(false);
+    expect(_isUrlAllowed("http://[::]/file")).toBe(false);
+  });
+
+  it("rejects invalid URLs", () => {
+    expect(_isUrlAllowed("not-a-url")).toBe(false);
+    expect(_isUrlAllowed("")).toBe(false);
+  });
 });

src/media/fetch.ts

@@ -1,7 +1,59 @@
 import path from "node:path";
 
+import * as ipaddr from "ipaddr.js";
+
 import { detectMime, extensionForMime } from "./mime.js";
 
+/**
+ * SSRF protection: validates that a URL is safe to fetch (not localhost, private IPs, etc.)
+ */
+function isUrlAllowed(url: string): boolean {
+  let parsed: URL;
+  try {
+    parsed = new URL(url);
+  } catch {
+    return false;
+  }
+
+  // Only allow http/https
+  if (!["http:", "https:"].includes(parsed.protocol)) {
+    return false;
+  }
+
+  const hostname = parsed.hostname;
+
+  // Block localhost variations
+  if (hostname === "localhost" || hostname.endsWith(".localhost")) {
+    return false;
+  }
+
+  // Check if hostname is an IP address
+  try {
+    // Handle IPv6 brackets [::1] -> ::1
+    const cleanHostname = hostname.replace(/^\[|\]$/g, "");
+    const addr = ipaddr.parse(cleanHostname);
+    const range = addr.range();
+
+    // Block all private/special ranges
+    const blockedRanges = [
+      "loopback", // 127.0.0.0/8, ::1
+      "private", // 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, fc00::/7
+      "linkLocal", // 169.254.0.0/16, fe80::/10
+      "uniqueLocal", // fc00::/7
+      "unspecified", // 0.0.0.0, ::
+    ];
+
+    if (blockedRanges.includes(range)) {
+      return false;
+    }
+  } catch {
+    // Not a valid IP - it's a hostname, allow DNS resolution
+    // Note: DNS rebinding is still possible but harder to exploit
+  }
+
+  return true;
+}
+
 type FetchMediaResult = {
   buffer: Buffer;
   contentType?: string;
@@ -63,23 +115,78 @@ async function readErrorBodySnippet(res: Response, maxChars = 200): Promise<stri
   }
 }
 
+const MAX_REDIRECTS = 5;
+
 export async function fetchRemoteMedia(options: FetchMediaOptions): Promise<FetchMediaResult> {
   const { url, fetchImpl, filePathHint, maxBytes } = options;
+
+  // SSRF protection: block private/local addresses
+  if (!isUrlAllowed(url)) {
+    throw new MediaFetchError("fetch_failed", `URL not allowed: blocked private/local address`);
+  }
+
   const fetcher: FetchLike | undefined = fetchImpl ?? globalThis.fetch;
   if (!fetcher) {
     throw new Error("fetch is not available");
   }
 
+  // Follow redirects manually to validate each redirect URL against SSRF rules
+  let currentUrl = url;
   let res: Response;
-  try {
+  let redirectCount = 0;
-    res = await fetcher(url);
+
-  } catch (err) {
+  while (true) {
-    throw new MediaFetchError("fetch_failed", `Failed to fetch media from ${url}: ${String(err)}`);
+    try {
+      res = await fetcher(currentUrl, { redirect: "manual" });
+    } catch (err) {
+      throw new MediaFetchError(
+        "fetch_failed",
+        `Failed to fetch media from ${currentUrl}: ${String(err)}`,
+      );
+    }
+
+    // Handle redirects (3xx status codes)
+    if (res.status >= 300 && res.status < 400) {
+      const location = res.headers.get("location");
+      if (!location) {
+        throw new MediaFetchError(
+          "http_error",
+          `Redirect from ${currentUrl} missing Location header`,
+        );
+      }
+
+      // Resolve relative redirects
+      let redirectUrl: string;
+      try {
+        redirectUrl = new URL(location, currentUrl).href;
+      } catch {
+        throw new MediaFetchError("http_error", `Invalid redirect URL: ${location}`);
+      }
+
+      // SSRF protection: validate redirect target
+      if (!isUrlAllowed(redirectUrl)) {
+        throw new MediaFetchError(
+          "fetch_failed",
+          `Redirect blocked: target URL not allowed (private/local address)`,
+        );
+      }
+
+      redirectCount++;
+      if (redirectCount > MAX_REDIRECTS) {
+        throw new MediaFetchError("fetch_failed", `Too many redirects (max ${MAX_REDIRECTS})`);
+      }
+
+      currentUrl = redirectUrl;
+      continue;
+    }
+
+    // Not a redirect, exit loop
+    break;
   }
 
   if (!res.ok) {
     const statusText = res.statusText ? ` ${res.statusText}` : "";
-    const redirected = res.url && res.url !== url ? ` (redirected to ${res.url})` : "";
+    const redirected = currentUrl !== url ? ` (redirected to ${currentUrl})` : "";
     let detail = `HTTP ${res.status}${statusText}`;
     if (!res.body) {
       detail = `HTTP ${res.status}${statusText}; empty response body`;
@@ -184,3 +291,6 @@ async function readResponseWithLimit(res: Response, maxBytes: number): Promise<B
     total,
   );
 }
+
+// Export for testing
+export { isUrlAllowed as _isUrlAllowed };

src/web/media.test.ts

@@ -6,7 +6,7 @@ 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, optimizeImageToJpeg, _isPathAllowed } from "./media.js";
 
 const tmpFiles: string[] = [];
 
@@ -262,3 +262,68 @@ describe("web media loading", () => {
     expect(result.buffer.length).toBeLessThanOrEqual(cap);
   });
 });
+
+describe("path traversal protection", () => {
+  it("allows paths within ~/.clawdbot", () => {
+    const clawdbotPath = path.join(os.homedir(), ".clawdbot", "test.txt");
+    expect(_isPathAllowed(clawdbotPath)).toBe(true);
+  });
+
+  it("allows paths within tmpdir", async () => {
+    // Create actual temp file to test real path resolution
+    const tmpFile = path.join(
+      os.tmpdir(),
+      `clawdbot-test-${Date.now()}-${Math.random().toString(16).slice(2)}.txt`,
+    );
+    await fs.writeFile(tmpFile, "test");
+    tmpFiles.push(tmpFile);
+
+    expect(_isPathAllowed(tmpFile)).toBe(true);
+  });
+
+  it("allows non-existent paths within allowed directories", () => {
+    const nonExistent = path.join(os.tmpdir(), "does-not-exist-12345.txt");
+    // Non-existent paths within allowed directories return true (let the read fail with proper error)
+    expect(_isPathAllowed(nonExistent)).toBe(true);
+  });
+
+  it("rejects non-existent paths outside allowed directories", () => {
+    const nonExistent = "/etc/does-not-exist-12345.txt";
+    // Non-existent paths outside allowed directories should be rejected
+    expect(_isPathAllowed(nonExistent)).toBe(false);
+  });
+
+  it("rejects file:// URLs to restricted paths via loadWebMedia", async () => {
+    // Use /bin/ls which exists on macOS and is definitely outside allowed roots
+    await expect(loadWebMedia("file:///bin/ls", 1024 * 1024)).rejects.toThrow(
+      /Access denied.*outside allowed directories/,
+    );
+  });
+
+  it("rejects local paths to restricted areas via loadWebMedia", async () => {
+    // Use /bin/ls which exists on macOS and is definitely outside allowed roots
+    await expect(loadWebMedia("/bin/ls", 1024 * 1024)).rejects.toThrow(
+      /Access denied.*outside allowed directories/,
+    );
+  });
+
+  it("blocks symlink escapes", async () => {
+    // Create a symlink in tmpdir that points to home directory (which is outside allowed roots)
+    const symlinkPath = path.join(
+      os.tmpdir(),
+      `clawdbot-symlink-test-${Date.now()}-${Math.random().toString(16).slice(2)}`,
+    );
+
+    try {
+      await fs.symlink(os.homedir(), symlinkPath);
+      tmpFiles.push(symlinkPath);
+
+      // The symlink is in tmpdir, but its target (home dir) is not in allowed roots
+      // isPathAllowed should block this because realpathSync resolves to home dir
+      expect(_isPathAllowed(symlinkPath)).toBe(false);
+    } catch {
+      // If we can't create symlinks (permissions, Windows), skip this test
+      console.log("Skipping symlink test (cannot create symlink)");
+    }
+  });
+});

src/web/media.ts

@@ -1,4 +1,6 @@
+import fsSync from "node:fs";
 import fs from "node:fs/promises";
+import os from "node:os";
 import path from "node:path";
 import { fileURLToPath } from "node:url";
 
@@ -30,6 +32,53 @@ const HEIC_MIME_RE = /^image\/hei[cf]$/i;
 const HEIC_EXT_RE = /\.(heic|heif)$/i;
 const MB = 1024 * 1024;
 
+/**
+ * Path traversal protection: validates that a file path is within allowed directories.
+ * Follows symlinks to ensure the real path is within bounds.
+ */
+function isPathAllowed(filePath: string): boolean {
+  // Resolve the input path (handles ..)
+  const resolved = path.resolve(filePath);
+
+  // Allowlist of safe roots (resolve them too since macOS /tmp is a symlink)
+  const allowedRoots = [path.join(os.homedir(), ".clawdbot"), os.tmpdir()].map((root) => {
+    try {
+      return fsSync.realpathSync(root);
+    } catch {
+      return root;
+    }
+  });
+
+  // Helper to check if a path is within allowed roots
+  const isWithinAllowedRoots = (pathToCheck: string): boolean => {
+    return allowedRoots.some(
+      (root) => pathToCheck === root || pathToCheck.startsWith(root + path.sep),
+    );
+  };
+
+  // Follow symlinks to get real path
+  let realPath: string;
+  try {
+    realPath = fsSync.realpathSync(resolved);
+  } catch {
+    // Path doesn't exist - try to resolve the parent directory to handle symlinks
+    // (e.g., /var -> /private/var on macOS)
+    const parentDir = path.dirname(resolved);
+    try {
+      const realParent = fsSync.realpathSync(parentDir);
+      const fileName = path.basename(resolved);
+      realPath = path.join(realParent, fileName);
+    } catch {
+      // Parent also doesn't exist - check raw resolved path against allowed roots
+      // This is more restrictive but safe
+      return isWithinAllowedRoots(resolved);
+    }
+  }
+
+  // Check REAL path (after symlink resolution) is within allowed roots
+  return isWithinAllowedRoots(realPath);
+}
+
 function formatMb(bytes: number, digits = 2): string {
   return (bytes / MB).toFixed(digits);
 }
@@ -119,6 +168,10 @@ async function loadWebMediaInternal(
     } catch {
       throw new Error(`Invalid file:// URL: ${mediaUrl}`);
     }
+    // Path traversal protection: ensure file is within allowed directories
+    if (!isPathAllowed(mediaUrl)) {
+      throw new Error("Access denied: file path outside allowed directories");
+    }
   }
 
   const optimizeAndClampImage = async (
@@ -200,6 +253,11 @@ async function loadWebMediaInternal(
     mediaUrl = resolveUserPath(mediaUrl);
   }
 
+  // Path traversal protection for local paths
+  if (!isPathAllowed(mediaUrl)) {
+    throw new Error("Access denied: file path outside allowed directories");
+  }
+
   // Local path
   const data = await fs.readFile(mediaUrl);
   const mime = await detectMime({ buffer: data, filePath: mediaUrl });
@@ -302,3 +360,6 @@ export async function optimizeImageToJpeg(
 }
 
 export { optimizeImageToPng };
+
+// Export for testing
+export { isPathAllowed as _isPathAllowed };