diff --git a/packages/core/src/permission.ts b/packages/core/src/permission.ts index 37d58111af0..2192c0e07a2 100644 --- a/packages/core/src/permission.ts +++ b/packages/core/src/permission.ts @@ -91,6 +91,8 @@ export interface Interface { readonly close: Effect.Effect readonly ask: (input: AssertInput) => Effect.Effect readonly assert: (input: AssertInput) => Effect.Effect + /** Fails only when the rules deny the request; never asks the user. */ + readonly assertNotDenied: (input: AssertInput) => Effect.Effect readonly reply: (input: ReplyInput) => Effect.Effect readonly get: (id: ID) => Effect.Effect readonly forSession: (sessionID: SessionSchema.ID) => Effect.Effect> @@ -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 }) }), ) diff --git a/packages/core/src/tool/plugin/webfetch.ts b/packages/core/src/tool/plugin/webfetch.ts index 64b63f682df..3c9cf10926c 100644 --- a/packages/core/src/tool/plugin/webfetch.ts +++ b/packages/core/src/tool/plugin/webfetch.ts @@ -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, + url: string, + format: Format, + userAgent = openCodeUserAgent, + redirects = 0, +): Effect.Effect => + 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) diff --git a/packages/core/test/permission.test.ts b/packages/core/test/permission.test.ts index d85e03247d8..e6a245a5ba0 100644 --- a/packages/core/test/permission.test.ts +++ b/packages/core/test/permission.test.ts @@ -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" }, diff --git a/packages/core/test/plugin/fixture.ts b/packages/core/test/plugin/fixture.ts index 1357876e486..fca86018b82 100644 --- a/packages/core/test/plugin/fixture.ts +++ b/packages/core/test/plugin/fixture.ts @@ -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([]), diff --git a/packages/core/test/plugin/permission-assert.test.ts b/packages/core/test/plugin/permission-assert.test.ts index ef4490e63f9..b7d468e2b0a 100644 --- a/packages/core/test/plugin/permission-assert.test.ts +++ b/packages/core/test/plugin/permission-assert.test.ts @@ -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([]), diff --git a/packages/core/test/tool-webfetch.test.ts b/packages/core/test/tool-webfetch.test.ts index 55718272374..1b6cbe85f31 100644 --- a/packages/core/test/tool-webfetch.test.ts +++ b/packages/core/test/tool-webfetch.test.ts @@ -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 }> = [] const assertions: Permission.AssertInput[] = [] +const checks: Permission.AssertInput[] = [] +const denied = new Set() 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> = [] 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()