fix(core): apply webfetch URL rules to redirect targets (#59748)

This commit is contained in:
Aiden Cline
2026-10-27 16:57:53 -05:00
committed by GitHub
Unverified
parent 2ed57439da
commit 17837a20d6
6 changed files with 134 additions and 10 deletions
+14 -1
View File
@@ -91,6 +91,8 @@ export interface Interface {
readonly close: Effect.Effect<void>
readonly ask: (input: AssertInput) => Effect.Effect<AskResult, SessionErrors.NotFoundError>
readonly assert: (input: AssertInput) => Effect.Effect<void, Error | SessionErrors.NotFoundError>
/** Fails only when the rules deny the request; never asks the user. */
readonly assertNotDenied: (input: AssertInput) => Effect.Effect<void, BlockedError | SessionErrors.NotFoundError>
readonly reply: (input: ReplyInput) => Effect.Effect<void, NotFoundError>
readonly get: (id: ID) => Effect.Effect<Request | undefined>
readonly forSession: (sessionID: SessionSchema.ID) => Effect.Effect<ReadonlyArray<Request>>
@@ -253,6 +255,17 @@ const layer = Layer.effect(
}),
)
const assertNotDenied = Effect.fn("Permission.assertNotDenied")(function* (input: AssertInput) {
const result = yield* evaluateInput(input)
if (result.effect !== "deny") return
return yield* new BlockedError({
rules: relevant(input, result.rules),
permission: input.action,
resources: input.resources,
reason: result.message,
})
})
const reply = Effect.fn("Permission.reply")((input: ReplyInput) =>
Effect.uninterruptible(
Effect.gen(function* () {
@@ -327,7 +340,7 @@ const layer = Layer.effect(
return Array.from(pending.values(), (item) => item.request).filter((request) => request.sessionID === sessionID)
})
return Service.of({ ask, assert, reply, get, forSession, list, close })
return Service.of({ ask, assert, assertNotDenied, reply, get, forSession, list, close })
}),
)
+48 -7
View File
@@ -3,7 +3,7 @@ export * as WebFetchTool from "./webfetch.js"
import type { Context } from "@opencode/plugin/effect/plugin"
import { ToolFailure } from "@opencode/ai/effect"
import { Duration, Effect, Schema } from "effect"
import { HttpClient, type HttpClientError, HttpClientRequest, HttpClientResponse } from "effect/http"
import { FetchHttpClient, HttpClient, HttpClientError, HttpClientRequest, HttpClientResponse } from "effect/http"
import { Parser } from "htmlparser2"
import { Mime } from "../../mime.js"
import { Permission } from "../../permission.js"
@@ -59,8 +59,8 @@ const headers = (format: Format, userAgent: string) => ({
const openCodeUserAgent =
"Mozilla/5.0 AppleWebKit/537.36 (KHTML, like Gecko); compatible; OpenCode-User/1.0; +https://opencode.ai"
const isCloudflareChallenge = (error: HttpClientError.HttpClientError) => {
if (error.reason._tag !== "StatusCodeError") return false
const isCloudflareChallenge = (error: unknown) => {
if (!HttpClientError.isHttpClientError(error) || error.reason._tag !== "StatusCodeError") return false
const response = error.reason.response
return response.status === 403 && response.headers["cf-mitigated"] === "challenge"
}
@@ -72,8 +72,38 @@ const assertHttpUrl = (url: URL) => {
if (url.protocol !== "http:" && url.protocol !== "https:") throw new Error("URL must use http:// or https://")
}
const execute = (http: HttpClient.HttpClient, url: string, format: Format, userAgent = openCodeUserAgent) =>
http.execute(request(url, format, userAgent)).pipe(Effect.flatMap(HttpClientResponse.filterStatusOk))
export const MAX_REDIRECTS = 10
const redirectStatuses = new Set([301, 302, 303, 307, 308])
// Follows redirects here instead of in fetch so each new URL is checked against the permission rules first.
const execute = (
http: HttpClient.HttpClient,
check: (url: string) => Effect.Effect<void, Error>,
url: string,
format: Format,
userAgent = openCodeUserAgent,
redirects = 0,
): Effect.Effect<HttpClientResponse.HttpClientResponse, Error> =>
http.execute(request(url, format, userAgent)).pipe(
Effect.provideService(FetchHttpClient.RequestInit, { redirect: "manual" }),
Effect.flatMap((response) => {
const location = response.headers.location
if (!redirectStatuses.has(response.status) || !location) return HttpClientResponse.filterStatusOk(response)
if (redirects >= MAX_REDIRECTS) return Effect.fail(new Error(`Too many redirects (more than ${MAX_REDIRECTS})`))
return Effect.gen(function* () {
const next = yield* Effect.try({
try: () => {
const target = new URL(location, url)
assertHttpUrl(target)
return target.href
},
catch: (error) => (error instanceof Error ? error : new Error(String(error))),
})
yield* check(next)
return yield* execute(http, check, next, format, userAgent, redirects + 1)
})
}),
)
const collectBody = (response: HttpClientResponse.HttpClientResponse) =>
collectBoundedResponseBody(
@@ -124,8 +154,19 @@ export const Plugin = {
})
const { body, contentType } = yield* Effect.gen(function* () {
const response = yield* execute(http, input.url, input.format).pipe(
Effect.catchIf(isCloudflareChallenge, () => execute(http, input.url, input.format, "opencode")),
const check = (url: string) =>
permission.assertNotDenied({
action: name,
resources: [url],
metadata: { ...input, url },
sessionID: context.sessionID,
agent: context.agent,
source: { type: "tool", messageID: context.messageID, id: context.id },
})
const response = yield* execute(http, check, input.url, input.format).pipe(
Effect.catchIf(isCloudflareChallenge, () =>
execute(http, check, input.url, input.format, "opencode"),
),
)
const contentType = response.headers["content-type"] || ""
const mime = mimeFrom(contentType)
+11
View File
@@ -148,6 +148,17 @@ describe("Permission", () => {
}),
)
it.effect("checks for a deny without asking", () =>
Effect.gen(function* () {
yield* setup([])
const service = yield* Permission.Service
yield* service.assertNotDenied(assertion())
expect(yield* service.list()).toEqual([])
yield* setRules([{ action: "read", resource: "*", effect: "deny" }])
expect(yield* service.assertNotDenied(assertion()).pipe(Effect.flip)).toBeInstanceOf(Permission.BlockedError)
}),
)
test("matches Windows rule resources against slash-normalized file access resources", () => {
const rules: Permission.Ruleset = [
{ action: "external_directory", resource: "C:\\Users\\x\\proj\\*", effect: "allow" },
+1
View File
@@ -70,6 +70,7 @@ const permissionLayer = Layer.succeed(
close: Effect.void,
ask: (input) => Effect.succeed({ id: input.id ?? Permission.ID.create(), effect: "ask" }),
assert: () => Effect.void,
assertNotDenied: () => Effect.void,
reply: () => Effect.void,
get: () => Effect.succeed(undefined),
forSession: () => Effect.succeed([]),
@@ -26,6 +26,7 @@ const permission = Layer.succeed(
if (input.action === "declined") return Effect.die(new Permission.DeclinedError())
return Effect.void
},
assertNotDenied: () => Effect.void,
reply: () => Effect.void,
get: () => Effect.succeed(undefined),
forSession: () => Effect.succeed([]),
+59 -2
View File
@@ -27,6 +27,8 @@ const webFetchUserAgent =
"Mozilla/5.0 AppleWebKit/537.36 (KHTML, like Gecko); compatible; OpenCode-User/1.0; +https://opencode.ai"
const requests: Array<{ readonly url: string; readonly headers: Record<string, string> }> = []
const assertions: Permission.AssertInput[] = []
const checks: Permission.AssertInput[] = []
const denied = new Set<string>()
let respond = (_request: HttpClientRequest.HttpClientRequest) =>
Effect.succeed(new Response("hello", { headers: { "content-type": "text/plain" } }))
@@ -39,7 +41,19 @@ const http = Layer.succeed(
),
),
)
const permission = permissionLayer({ assert: (input) => Effect.sync(() => assertions.push(input)) })
const permission = permissionLayer({
assert: (input) => Effect.sync(() => assertions.push(input)),
assertNotDenied: (input) =>
Effect.sync(() => checks.push(input)).pipe(
Effect.andThen(
input.resources.some((resource) => denied.has(resource))
? Effect.fail(
new Permission.BlockedError({ rules: [], permission: input.action, resources: input.resources }),
)
: Effect.void,
),
),
})
const toolLayer = (replacements: LayerNode.Replacements = []) =>
AppNodeBuilder.build(LayerNode.group([Tool.node, webFetchToolNode]), [
Permission.node.replace(permission),
@@ -52,6 +66,8 @@ const live = testEffect(toolLayer())
const reset = () => {
requests.length = 0
assertions.length = 0
checks.length = 0
denied.clear()
respond = () => Effect.succeed(new Response("hello", { headers: { "content-type": "text/plain" } }))
}
@@ -456,7 +472,7 @@ describe("WebFetchTool registration", () => {
}),
)
live.effect("follows redirects while approving only the requested URL", () => {
live.effect("follows redirects, checking each target against the rules without asking", () => {
const received: Array<Record<string, string | null>> = []
return Effect.acquireUseRelease(
Effect.sync(() =>
@@ -488,6 +504,8 @@ describe("WebFetchTool registration", () => {
expect(assertions).toMatchObject([
{ sessionID, action: "webfetch", resources: [url], save: ["*"], metadata: { url, format: "text" } },
])
const target = new URL("/target", server.url).toString()
expect(checks).toMatchObject([{ sessionID, action: "webfetch", resources: [target] }])
expect(received).toEqual(
Array.from({ length: 2 }, () => ({
accept: "text/plain;q=1.0, text/markdown;q=0.9, text/html;q=0.8, */*;q=0.1",
@@ -501,6 +519,45 @@ describe("WebFetchTool registration", () => {
)
})
it.effect("stops at a redirect target the rules deny", () =>
Effect.gen(function* () {
reset()
const registry = yield* Tool.Service
const url = "https://example.com/start"
const target = "http://169.254.169.254/latest/meta-data"
denied.add(target)
respond = () => Effect.succeed(new Response("", { status: 302, headers: { location: target } }))
expect(yield* executeTool(registry, call({ url, format: "text" }))).toMatchObject({
status: "error",
error: { message: "Permission denied: webfetch" },
})
expect(checks.map((check) => check.resources)).toEqual([[target]])
expect(requests.map((request) => request.url)).toEqual([url])
}),
)
it.effect("rejects redirects to non-HTTP schemes and endless redirect chains", () =>
Effect.gen(function* () {
reset()
const registry = yield* Tool.Service
respond = () => Effect.succeed(new Response("", { status: 301, headers: { location: "file:///etc/passwd" } }))
expect(yield* executeTool(registry, call({ url: "https://example.com/a", format: "text" }))).toMatchObject({
status: "error",
error: { message: "URL must use http:// or https://" },
})
expect(checks).toEqual([])
reset()
respond = (request) => Effect.succeed(new Response("", { status: 307, headers: { location: `${request.url}x` } }))
expect(yield* executeTool(registry, call({ url: "https://example.com/a", format: "text" }))).toMatchObject({
status: "error",
error: { message: `Too many redirects (more than ${WebFetchTool.MAX_REDIRECTS})` },
})
expect(requests).toHaveLength(WebFetchTool.MAX_REDIRECTS + 1)
}),
)
it.effect("rejects non-HTTP schemes before permission or transport", () =>
Effect.gen(function* () {
reset()