fix(tool): port upstream webfetch redirect rule checks (anomalyco/opencode#59748)

packages/opencode/src/tool/webfetch.ts

@@ -3,6 +3,11 @@ import { Tool } from "./tool"
 import TurndownService from "turndown"
 import DESCRIPTION from "./webfetch.txt"
 import { abortAfterAny } from "../util/abort"
+// altimate_change start — upstream_fix: check webfetch redirect targets against permission rules (anomalyco/opencode#59748, anomalyco/opencode#61701)
+import { PermissionV1 } from "@opencode-ai/core/v1/permission"
+import { PermissionEvaluate } from "@/permission/evaluate"
+import { Wildcard } from "@opencode-ai/core/util/wildcard"
+// altimate_change end
 
 const MAX_RESPONSE_SIZE = 5 * 1024 * 1024 // 5MB
 const DEFAULT_TIMEOUT = 30 * 1000 // 30 seconds
@@ -15,6 +20,45 @@ const BROWSER_UA =
 // Status codes that warrant a retry with a different User-Agent
 const RETRYABLE_STATUSES = new Set([403, 406])
 
+// altimate_change start — upstream_fix: check webfetch redirect targets against permission rules (anomalyco/opencode#59748, anomalyco/opencode#61701)
+export const MAX_REDIRECTS = 10
+const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308])
+
+function assertFetchableUrl(url: URL) {
+  if (url.protocol !== "http:" && url.protocol !== "https:") throw new Error("URL must use http:// or https://")
+  // Rules match the URL, so `https://allowed.com@evil.com` would slip past a rule for `https://allowed.com*`.
+  if (url.username || url.password) throw new Error("URL must not include credentials")
+}
+
+// Follows redirects here instead of in fetch so each new URL is checked against the rules first.
+// A target only fails when the rules deny it; the user is not asked again.
+async function fetchCheckingRedirects(
+  url: string,
+  init: RequestInit,
+  ruleset: PermissionV1.Ruleset,
+): Promise<Response> {
+  let current = url
+  for (let redirects = 0; ; redirects++) {
+    const response = await fetch(current, { ...init, redirect: "manual" })
+    const location = response.headers.get("location")
+    if (!REDIRECT_STATUSES.has(response.status) || !location) return response
+    await response.body?.cancel().catch(() => {})
+    if (redirects >= MAX_REDIRECTS) throw new Error(`Too many redirects (more than ${MAX_REDIRECTS})`)
+    const target = new URL(location, current)
+    assertFetchableUrl(target)
+    // The fragment is never sent, and would keep a rule for the exact URL from matching.
+    target.hash = ""
+    const rule = PermissionEvaluate.evaluate("webfetch", target.href, ruleset)
+    if (rule.action === "deny") {
+      throw new PermissionV1.DeniedError({
+        ruleset: ruleset.filter((item) => Wildcard.match("webfetch", item.permission)),
+      })
+    }
+    current = target.href
+  }
+}
+// altimate_change end
+
 // altimate_change start — process-level URL failure cache (#471)
 // Prevents repeated fetches to URLs that already returned 404/410 in this
 // process. Cache is module-scope so in daemon mode (`altimate serve`) and
@@ -308,11 +352,17 @@ export const WebFetchTool = Tool.define("webfetch", {
     if (!params.url.startsWith("http://") && !params.url.startsWith("https://")) {
       throw new Error("URL must start with http:// or https://")
     }
+    // altimate_change start — upstream_fix: reject URL credentials and match rules on the requested URL (anomalyco/opencode#61701, anomalyco/opencode#60505)
+    let parsed: URL
     try {
-      new URL(params.url)
+      parsed = new URL(params.url)
     } catch {
       throw new Error(`Invalid URL: "${params.url.slice(0, 200)}" is not a valid URL. Check the format and try again.`)
     }
+    assertFetchableUrl(parsed)
+    parsed.hash = ""
+    const url = parsed.href
+    // altimate_change end
 
     // Check failure cache — avoid re-fetching URLs that already returned 404/410/451.
     // Prefix with "(cached failure, Nm ago)" so the agent can tell a cache-hit
@@ -328,7 +378,9 @@ export const WebFetchTool = Tool.define("webfetch", {
 
     await ctx.ask({
       permission: "webfetch",
-      patterns: [params.url],
+      // altimate_change start — upstream_fix: match rules on the requested URL (anomalyco/opencode#60505)
+      patterns: [url],
+      // altimate_change end
       always: ["*"],
       metadata: {
         url: params.url,
@@ -370,13 +422,16 @@ export const WebFetchTool = Tool.define("webfetch", {
     let arrayBuffer: ArrayBuffer
     let response: Response
     try {
-      response = await fetch(params.url, { signal, headers: honestHeaders })
+      // altimate_change start — upstream_fix: check webfetch redirect targets against permission rules (anomalyco/opencode#59748)
+      const ruleset = ctx.permissions ?? []
+      response = await fetchCheckingRedirects(url, { signal, headers: honestHeaders }, ruleset)
 
       // Retry with browser UA if the honest UA was rejected
       if (!response.ok && RETRYABLE_STATUSES.has(response.status)) {
         await response.body?.cancel().catch(() => {})
-        response = await fetch(params.url, { signal, headers: browserHeaders })
+        response = await fetchCheckingRedirects(url, { signal, headers: browserHeaders }, ruleset)
       }
+      // altimate_change end
 
       // altimate_change start — actionable error messages and failure caching (#471)
       if (!response.ok) {

packages/opencode/test/tool/webfetch.test.ts

@@ -1,11 +1,12 @@
 import { describe, expect } from "bun:test"
-import { Effect, Layer } from "effect"
+import { Cause, Effect, Exit, Layer } from "effect"
 import { FetchHttpClient } from "effect/unstable/http"
 import { Agent } from "../../src/agent/agent"
 import { Truncate } from "@/tool/truncate"
 import { WebFetchTool } from "../../src/tool/webfetch"
 import { SessionID, MessageID } from "../../src/session/schema"
 import { Tool } from "@/tool/tool"
+import { PermissionV1 } from "@opencode-ai/core/v1/permission"
 import { testEffect } from "../lib/effect"
 
 const it = testEffect(Layer.mergeAll(FetchHttpClient.layer, Truncate.defaultLayer, Agent.defaultLayer))
@@ -31,12 +32,27 @@ const withFetch = <A, E, R>(
     (server) => Effect.sync(() => server.stop(true)),
   )
 
-const exec = Effect.fn("WebFetchToolTest.exec")(function* (args: Tool.InferParameters<typeof WebFetchTool>) {
+const exec = Effect.fn("WebFetchToolTest.exec")(function* (
+  args: Tool.InferParameters<typeof WebFetchTool>,
+  // altimate_change start — upstream_fix: redirect rule tests pass the effective ruleset (anomalyco/opencode#59748)
+  permissions?: PermissionV1.Ruleset,
+  // altimate_change end
+) {
   const info = yield* WebFetchTool
   const tool = yield* info.init()
-  return yield* tool.execute(args, ctx)
+  return yield* tool.execute(args, permissions ? { ...ctx, permissions } : ctx)
 })
 
+// altimate_change start — upstream_fix: redirect rule tests (anomalyco/opencode#59748, anomalyco/opencode#61701)
+const failure = <A, E, R>(effect: Effect.Effect<A, E, R>) =>
+  Effect.gen(function* () {
+    const exit = yield* Effect.exit(effect)
+    expect(Exit.isFailure(exit)).toBe(true)
+    if (Exit.isSuccess(exit)) return ""
+    return String(Cause.squash(exit.cause))
+  })
+// altimate_change end
+
 describe("tool.webfetch", () => {
   it.instance("returns image responses as file attachments", () =>
     Effect.gen(function* () {
@@ -110,4 +126,93 @@ describe("tool.webfetch", () => {
         }),
     ),
   )
+
+  // altimate_change start — upstream_fix: redirect rule tests (anomalyco/opencode#59748, anomalyco/opencode#61701)
+  it.instance("follows redirects to targets the rules allow", () => {
+    const requested: string[] = []
+    return withFetch(
+      (req) => {
+        const path = new URL(req.url).pathname
+        requested.push(path)
+        if (path === "/start") return new Response(null, { status: 302, headers: { location: "/target#frag" } })
+        return new Response("arrived", { status: 200, headers: { "content-type": "text/plain" } })
+      },
+      (url) =>
+        Effect.gen(function* () {
+          const result = yield* exec({ url: new URL("/start", url).toString(), format: "text" }, [
+            { permission: "webfetch", pattern: "*", action: "allow" },
+          ])
+          expect(result.output).toBe("arrived")
+          expect(requested).toEqual(["/start", "/target"])
+        }),
+    )
+  })
+
+  it.instance("stops at a redirect target the rules deny", () => {
+    const requested: string[] = []
+    return withFetch(
+      (req) => {
+        const path = new URL(req.url).pathname
+        requested.push(path)
+        if (path === "/start") return new Response(null, { status: 302, headers: { location: "/secret" } })
+        return new Response("secret", { status: 200, headers: { "content-type": "text/plain" } })
+      },
+      (url) =>
+        Effect.gen(function* () {
+          const message = yield* failure(
+            exec({ url: new URL("/start", url).toString(), format: "text" }, [
+              { permission: "webfetch", pattern: "*", action: "allow" },
+              { permission: "webfetch", pattern: new URL("/secret", url).toString(), action: "deny" },
+            ]),
+          )
+          expect(message).toContain("prevents you from using this specific tool call")
+          expect(requested).toEqual(["/start"])
+        }),
+    )
+  })
+
+  it.instance("rejects redirects to non-HTTP schemes and endless redirect chains", () =>
+    withFetch(
+      (req) => {
+        const path = new URL(req.url).pathname
+        if (path === "/file") return new Response(null, { status: 301, headers: { location: "file:///etc/passwd" } })
+        return new Response(null, { status: 307, headers: { location: `${path}x` } })
+      },
+      (url) =>
+        Effect.gen(function* () {
+          expect(yield* failure(exec({ url: new URL("/file", url).toString(), format: "text" }))).toContain(
+            "URL must use http:// or https://",
+          )
+          expect(yield* failure(exec({ url: new URL("/loop", url).toString(), format: "text" }))).toContain(
+            "Too many redirects",
+          )
+        }),
+    ),
+  )
+
+  it.instance("rejects credentials in the requested URL and in redirect targets", () => {
+    let requests = 0
+    return withFetch(
+      (req) => {
+        requests++
+        const target = new URL("/next", req.url)
+        target.username = "user"
+        return new Response(null, { status: 302, headers: { location: target.toString() } })
+      },
+      (url) =>
+        Effect.gen(function* () {
+          const withCredentials = new URL("/start", url)
+          withCredentials.username = "user"
+          expect(yield* failure(exec({ url: withCredentials.toString(), format: "text" }))).toContain(
+            "URL must not include credentials",
+          )
+          expect(requests).toBe(0)
+          expect(yield* failure(exec({ url: new URL("/start", url).toString(), format: "text" }))).toContain(
+            "URL must not include credentials",
+          )
+          expect(requests).toBe(1)
+        }),
+    )
+  })
+  // altimate_change end
 })