diff --git a/.opencode/lib/github-pr-search.lib.ts b/.opencode/lib/github-pr-search.lib.ts new file mode 100644 index 000000000000..ece642994276 --- /dev/null +++ b/.opencode/lib/github-pr-search.lib.ts @@ -0,0 +1,63 @@ +// Pure, dependency-free helpers for the github-pr-search tool. +// Kept in a separate module so they can be unit-tested without importing the +// plugin runtime (`@opencode-ai/plugin`). + +export interface PR { + number: number + title: string + html_url: string + state: string + user?: { login?: string } + labels?: Array<{ name?: string }> + body?: string | null + pull_request?: { merged_at?: string | null } +} + +/** GitHub Search returns state open/closed; a merged PR has pull_request.merged_at. */ +export function describeState(pr: PR): string { + if (pr.pull_request?.merged_at) return "merged" + return pr.state ?? "unknown" +} + +/** Collapse whitespace and truncate a PR body to a short snippet. */ +export function snippet(body: string | null | undefined, max = 200): string { + if (!body) return "" + const flat = body.replace(/\s+/g, " ").trim() + return flat.length > max ? flat.slice(0, max) + "…" : flat +} + +/** Clamp pagination inputs to the GitHub Search API's valid ranges. */ +export function clampPaging(limit: number, page: number): { perPage: number; page: number } { + return { + perPage: Math.min(Math.max(Math.trunc(limit), 1), 100), + page: Math.max(Math.trunc(page), 1), + } +} + +/** Build the GitHub Search `q` value. `state: "all"` omits the state qualifier. */ +export function buildSearchQuery( + query: string, + owner: string, + repo: string, + state: "open" | "closed" | "all", +): string { + const stateFilter = state === "all" ? "" : ` state:${state}` + return `${query} repo:${owner}/${repo} type:pr${stateFilter}` +} + +/** Render one PR into an LLM-friendly block. */ +export function formatPr(pr: PR): string { + const author = pr.user?.login ? `@${pr.user.login}` : "unknown" + const labels = (pr.labels ?? []) + .map((l) => l.name) + .filter(Boolean) + .join(", ") + const lines = [ + `#${pr.number} — ${pr.title}`, + `state: ${describeState(pr)} · author: ${author}${labels ? ` · labels: ${labels}` : ""}`, + pr.html_url, + ] + const body = snippet(pr.body) + if (body) lines.push(body) + return lines.join("\n") +} diff --git a/.opencode/lib/github-triage.lib.ts b/.opencode/lib/github-triage.lib.ts new file mode 100644 index 000000000000..46153d2402ce --- /dev/null +++ b/.opencode/lib/github-triage.lib.ts @@ -0,0 +1,27 @@ +// Pure, dependency-free helpers for the github-triage tool, split out so they +// can be unit-tested without importing the plugin runtime (`@opencode-ai/plugin`). + +export const TEAM = { + tui: ["kommander", "simonklee"], + desktop_web: ["Hona", "Brendonovich"], + core: ["jlongster", "rekram1-node", "nexxeln", "kitlangton", "starptech"], + inference: ["fwang", "MrMushrooooom", "starptech"], + windows: ["Hona"], +} as const + +export type Team = keyof typeof TEAM + +/** Pick a random element; throws a clear error instead of returning undefined for an empty list. */ +export function pick(items: readonly T[]): T { + if (items.length === 0) throw new Error("Cannot pick from an empty list") + return items[Math.floor(Math.random() * items.length)]! +} + +/** Parse and validate an issue number from an env-like string. Must be a positive integer. */ +export function parseIssueNumber(raw: string | undefined): number { + const issue = Number.parseInt((raw ?? "").trim(), 10) + if (!Number.isInteger(issue) || issue <= 0) { + throw new Error(`Invalid ISSUE_NUMBER: ${JSON.stringify(raw)}`) + } + return issue +} diff --git a/.opencode/test/github-pr-search.test.ts b/.opencode/test/github-pr-search.test.ts new file mode 100644 index 000000000000..857e009c837a --- /dev/null +++ b/.opencode/test/github-pr-search.test.ts @@ -0,0 +1,73 @@ +import { test, expect, describe } from "bun:test" +import { describeState, snippet, clampPaging, buildSearchQuery, formatPr, type PR } from "../lib/github-pr-search.lib" + +describe("describeState", () => { + test("merged when pull_request.merged_at is set", () => { + expect(describeState({ pull_request: { merged_at: "2024-01-01" }, state: "closed" } as PR)).toBe("merged") + }) + test("falls back to the GitHub state", () => { + expect(describeState({ state: "open" } as PR)).toBe("open") + }) + test("unknown when state missing", () => { + expect(describeState({} as PR)).toBe("unknown") + }) +}) + +describe("snippet", () => { + test("empty for null/empty body", () => { + expect(snippet(null)).toBe("") + expect(snippet("")).toBe("") + }) + test("collapses whitespace", () => { + expect(snippet("a\n\n b c")).toBe("a b c") + }) + test("truncates long bodies with an ellipsis", () => { + const out = snippet("x".repeat(250)) + expect(out.length).toBe(201) + expect(out.endsWith("…")).toBe(true) + }) +}) + +describe("clampPaging", () => { + test("passes valid values through", () => { + expect(clampPaging(10, 1)).toEqual({ perPage: 10, page: 1 }) + }) + test("clamps page below 1", () => { + expect(clampPaging(10, 0)).toEqual({ perPage: 10, page: 1 }) + }) + test("caps perPage at 100 and floors at 1", () => { + expect(clampPaging(500, 3)).toEqual({ perPage: 100, page: 3 }) + expect(clampPaging(0, 1)).toEqual({ perPage: 1, page: 1 }) + }) + test("truncates fractional input", () => { + expect(clampPaging(10.9, 2.7)).toEqual({ perPage: 10, page: 2 }) + }) +}) + +describe("buildSearchQuery", () => { + test("includes the state qualifier", () => { + expect(buildSearchQuery("foo", "o", "r", "open")).toBe("foo repo:o/r type:pr state:open") + }) + test("omits the state qualifier for 'all'", () => { + expect(buildSearchQuery("foo", "o", "r", "all")).toBe("foo repo:o/r type:pr") + }) +}) + +describe("formatPr", () => { + test("renders number, state, author, labels, url and snippet", () => { + const pr = { + number: 7, + title: "Fix", + html_url: "u", + state: "open", + user: { login: "a" }, + labels: [{ name: "bug" }], + body: "hi", + } as PR + expect(formatPr(pr)).toBe("#7 — Fix\nstate: open · author: @a · labels: bug\nu\nhi") + }) + test("handles missing author and labels", () => { + const pr = { number: 1, title: "T", html_url: "u", state: "closed" } as PR + expect(formatPr(pr)).toBe("#1 — T\nstate: closed · author: unknown\nu") + }) +}) diff --git a/.opencode/test/github-triage.test.ts b/.opencode/test/github-triage.test.ts new file mode 100644 index 000000000000..5e9044fa051c --- /dev/null +++ b/.opencode/test/github-triage.test.ts @@ -0,0 +1,34 @@ +import { test, expect, describe } from "bun:test" +import { pick, parseIssueNumber, TEAM } from "../lib/github-triage.lib" + +describe("parseIssueNumber", () => { + test("parses a valid positive integer", () => { + expect(parseIssueNumber("123")).toBe(123) + }) + test("trims surrounding whitespace", () => { + expect(parseIssueNumber(" 45 ")).toBe(45) + }) + test("parses a leading number", () => { + expect(parseIssueNumber("12abc")).toBe(12) + }) + test("throws on undefined, empty, zero, negative, and non-numeric", () => { + expect(() => parseIssueNumber(undefined)).toThrow() + expect(() => parseIssueNumber("")).toThrow() + expect(() => parseIssueNumber("0")).toThrow() + expect(() => parseIssueNumber("-5")).toThrow() + expect(() => parseIssueNumber("abc")).toThrow() + }) +}) + +describe("pick", () => { + test("throws on an empty list instead of returning undefined", () => { + expect(() => pick([])).toThrow() + }) + test("returns the only element of a single-item list", () => { + expect(pick(["only"])).toBe("only") + }) + test("returns a member of the provided list", () => { + const member = pick(TEAM.tui) + expect(TEAM.tui.includes(member)).toBe(true) + }) +}) diff --git a/.opencode/tool/github-pr-search.ts b/.opencode/tool/github-pr-search.ts index 8bc8c554aaee..e76148bbac5b 100644 --- a/.opencode/tool/github-pr-search.ts +++ b/.opencode/tool/github-pr-search.ts @@ -1,12 +1,16 @@ /// import { tool } from "@opencode-ai/plugin" +import { type PR, buildSearchQuery, clampPaging, formatPr } from "../lib/github-pr-search.lib" + async function githubFetch(endpoint: string, options: RequestInit = {}) { + const hasBody = options.body !== undefined && options.body !== null const response = await fetch(`https://api.github.com${endpoint}`, { ...options, headers: { Authorization: `Bearer ${process.env.GITHUB_TOKEN}`, Accept: "application/vnd.github+json", - "Content-Type": "application/json", + // Only send Content-Type when there is actually a request body. + ...(hasBody ? { "Content-Type": "application/json" } : {}), ...(options.headers instanceof Headers ? Object.fromEntries(options.headers.entries()) : options.headers), }, }) @@ -16,35 +20,32 @@ async function githubFetch(endpoint: string, options: RequestInit = {}) { return response.json() } -interface PR { - title: string - html_url: string -} - export default tool({ description: `Use this tool to search GitHub pull requests by title and description. -This tool searches PRs in the anomalyco/opencode repository and returns LLM-friendly results including: +This tool searches PRs in the anomalyco/opencode repository and returns LLM-friendly results. For each PR it returns: - PR number and title - Author - State (open/closed/merged) - Labels - Description snippet +- URL -Use the query parameter to search for keywords that might appear in PR titles or descriptions.`, +Pagination is page-based (1-indexed), matching the GitHub Search API exactly, so results never silently overlap or skip.`, args: { query: tool.schema.string().describe("Search query for PR titles and descriptions"), - limit: tool.schema.number().describe("Maximum number of results to return").default(10), - offset: tool.schema.number().describe("Number of results to skip for pagination").default(0), + limit: tool.schema.number().describe("Results per page (max 100)").default(10), + page: tool.schema.number().describe("1-indexed page number for pagination").default(1), + state: tool.schema.enum(["open", "closed", "all"]).describe("Filter by PR state").default("open"), }, async execute(args) { const owner = "anomalyco" const repo = "opencode" - const page = Math.floor(args.offset / args.limit) + 1 - const searchQuery = encodeURIComponent(`${args.query} repo:${owner}/${repo} type:pr state:open`) + const { perPage, page } = clampPaging(args.limit, args.page) + const searchQuery = encodeURIComponent(buildSearchQuery(args.query, owner, repo, args.state)) const result = await githubFetch( - `/search/issues?q=${searchQuery}&per_page=${args.limit}&page=${page}&sort=updated&order=desc`, + `/search/issues?q=${searchQuery}&per_page=${perPage}&page=${page}&sort=updated&order=desc`, ) if (result.total_count === 0) { @@ -54,11 +55,11 @@ Use the query parameter to search for keywords that might appear in PR titles or const prs = result.items as PR[] if (prs.length === 0) { - return `No other PRs found matching "${args.query}"` + return `No PRs on page ${page} matching "${args.query}" (${result.total_count} total)` } - const formatted = prs.map((pr) => `${pr.title}\n${pr.html_url}`).join("\n\n") + const formatted = prs.map(formatPr).join("\n\n") - return `Found ${result.total_count} PRs (showing ${prs.length}):\n\n${formatted}` + return `Found ${result.total_count} PRs (page ${page}, showing ${prs.length}):\n\n${formatted}` }, }) diff --git a/.opencode/tool/github-triage.ts b/.opencode/tool/github-triage.ts index f25ca48b384b..6911ef222354 100644 --- a/.opencode/tool/github-triage.ts +++ b/.opencode/tool/github-triage.ts @@ -1,31 +1,16 @@ /// import { tool } from "@opencode-ai/plugin" - -const TEAM = { - tui: ["kommander", "simonklee"], - desktop_web: ["Hona", "Brendonovich"], - core: ["jlongster", "rekram1-node", "nexxeln", "kitlangton", "starptech"], - inference: ["fwang", "MrMushrooooom", "starptech"], - windows: ["Hona"], -} as const - -function pick(items: readonly T[]) { - return items[Math.floor(Math.random() * items.length)]! -} - -function getIssueNumber(): number { - const issue = parseInt(process.env.ISSUE_NUMBER ?? "", 10) - if (!issue) throw new Error("ISSUE_NUMBER env var not set") - return issue -} +import { TEAM, type Team, pick, parseIssueNumber } from "../lib/github-triage.lib" async function githubFetch(endpoint: string, options: RequestInit = {}) { + const hasBody = options.body !== undefined && options.body !== null const response = await fetch(`https://api.github.com${endpoint}`, { ...options, headers: { Authorization: `Bearer ${process.env.GITHUB_TOKEN}`, Accept: "application/vnd.github+json", - "Content-Type": "application/json", + // Only send Content-Type when there is actually a request body. + ...(hasBody ? { "Content-Type": "application/json" } : {}), ...(options.headers instanceof Headers ? Object.fromEntries(options.headers.entries()) : options.headers), }, }) @@ -40,12 +25,10 @@ export default tool({ Provide the team that should own the issue. This tool picks a random assignee from that team and does not apply labels.`, args: { - team: tool.schema - .enum(Object.keys(TEAM) as [keyof typeof TEAM, ...(keyof typeof TEAM)[]]) - .describe("The owning team"), + team: tool.schema.enum(Object.keys(TEAM) as [Team, ...Team[]]).describe("The owning team"), }, async execute(args) { - const issue = getIssueNumber() + const issue = parseIssueNumber(process.env.ISSUE_NUMBER) const owner = "anomalyco" const repo = "opencode" const assignee = pick(TEAM[args.team]) diff --git a/packages/core/src/permission.ts b/packages/core/src/permission.ts index ec8038f7134d..e75f6863ad4d 100644 --- a/packages/core/src/permission.ts +++ b/packages/core/src/permission.ts @@ -18,6 +18,14 @@ export type Ruleset = typeof Ruleset.Type const EDIT_TOOLS = ["edit", "write", "apply_patch"] +/** + * Resolve the effective rule for a (permission, pattern) pair. + * + * Rules are flattened across rulesets and the LAST matching rule wins + * ("last match wins"), so more specific or later rules override earlier ones. + * A rule matches when both its `permission` glob matches `permission` AND its + * `pattern` glob matches `pattern`. When nothing matches, the default is `ask`. + */ export function evaluate(permission: string, pattern: string, ...rulesets: Ruleset[]): Rule { return ( rulesets @@ -34,6 +42,22 @@ export function merge(...rulesets: Ruleset[]): Ruleset { return rulesets.flat() } +/** + * Determine which tools should be fully hidden/disabled in the UI. + * + * INTENTIONAL semantics (see permission-task.test.ts): a tool is disabled only + * when the LAST rule whose `permission` glob matches the tool is a blanket deny + * (`pattern === "*" && action === "deny"`). Consequences worth knowing: + * - A narrower allow that comes AFTER a blanket deny re-enables the tool + * (the last matching rule is no longer the `*` deny). + * - Conversely, narrower allows that come BEFORE a trailing blanket `*` deny do + * NOT keep the tool enabled — the blanket deny still hides it, even though + * `evaluate()` may still permit specific patterns at call time. This asymmetry + * with `evaluate()` is deliberate: `disabled()` is a coarse UI gate, while + * `evaluate()` is the precise per-call check. + * Edit-family tools (edit/write/apply_patch) are collapsed onto the `edit` + * permission before matching. + */ export function disabled(tools: string[], ruleset: Ruleset): Set { return new Set( tools.filter((tool) => { diff --git a/packages/core/src/util/wildcard.ts b/packages/core/src/util/wildcard.ts index 0a67817bcb58..183120420fc2 100644 --- a/packages/core/src/util/wildcard.ts +++ b/packages/core/src/util/wildcard.ts @@ -1,5 +1,20 @@ export * as Wildcard from "./wildcard" +/** + * Glob-style matcher used for permission rules and path matching. + * + * Semantics (intentional): + * - `*` matches any run of characters, `?` matches a single character. + * - Backslashes are normalized to `/` on both sides so Windows and POSIX paths + * compare equally. + * - A trailing ` *` (space + star) is made optional, so a rule like `git push *` + * also matches the bare command `git push`. + * - Case sensitivity is platform-dependent: case-INsensitive on Windows (`si`) + * and case-SENSITIVE elsewhere (`s`). This mirrors real OS behavior — Windows + * paths and commands are case-insensitive, POSIX ones are not — so a deny rule + * such as `RM *` intentionally does NOT match `rm` on Linux/macOS. Author rules + * in the casing that matches the target platform. + */ export function match(input: string, pattern: string) { const normalized = input.replaceAll("\\", "/") let escaped = pattern diff --git a/packages/opencode/src/tool/webfetch.ts b/packages/opencode/src/tool/webfetch.ts index f8a4b6233ae9..f04468629bd3 100644 --- a/packages/opencode/src/tool/webfetch.ts +++ b/packages/opencode/src/tool/webfetch.ts @@ -1,6 +1,9 @@ -import { Effect, Schema } from "effect" -import { HttpClient, HttpClientRequest } from "effect/unstable/http" +import { Context, Effect, Schema } from "effect" +import * as Stream from "effect/Stream" +import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http" import { Parser } from "htmlparser2" +import dns from "node:dns/promises" +import net from "node:net" import * as Tool from "./tool" import TurndownService from "turndown" import DESCRIPTION from "./webfetch.txt" @@ -9,6 +12,124 @@ import { isImageAttachment } from "@/util/media" const MAX_RESPONSE_SIZE = 5 * 1024 * 1024 // 5MB const DEFAULT_TIMEOUT = 30 * 1000 // 30 seconds const MAX_TIMEOUT = 120 * 1000 // 2 minutes +const MAX_REDIRECTS = 5 + +/** + * Escape hatch for the SSRF guard below. Users who legitimately need to fetch + * private/internal addresses (e.g. a dev server on localhost) can set + * OPENCODE_WEBFETCH_ALLOW_PRIVATE=1; tests provide this reference directly. + */ +export const AllowPrivateFetch = Context.Reference("~opencode/webfetch/AllowPrivateFetch", { + defaultValue: () => { + const value = process.env["OPENCODE_WEBFETCH_ALLOW_PRIVATE"] + return value === "1" || value === "true" + }, +}) + +// SSRF protection: reject targets that resolve to loopback, link-local, or +// private address ranges. Without this, a webfetch (once permitted) could reach +// internal services or cloud metadata endpoints (e.g. 169.254.169.254). + +function isPrivateIPv4(p: readonly number[]): boolean { + const [a, b, c] = p as [number, number, number] + if (a === 0) return true // "this" network (0.0.0.0/8) + if (a === 10) return true // private (10.0.0.0/8) + if (a === 100 && b >= 64 && b <= 127) return true // CGNAT (100.64.0.0/10) + if (a === 127) return true // loopback (127.0.0.0/8) + if (a === 169 && b === 254) return true // link-local (169.254.0.0/16) + if (a === 172 && b >= 16 && b <= 31) return true // private (172.16.0.0/12) + if (a === 192 && b === 0 && (c === 0 || c === 2)) return true // IETF (192.0.0.0/24) + TEST-NET-1 (192.0.2.0/24) + if (a === 192 && b === 168) return true // private (192.168.0.0/16) + if (a === 198 && (b === 18 || b === 19)) return true // benchmarking (198.18.0.0/15) + if (a === 198 && b === 51 && c === 100) return true // TEST-NET-2 (198.51.100.0/24) + if (a === 203 && b === 0 && c === 113) return true // TEST-NET-3 (203.0.113.0/24) + if (a >= 224) return true // multicast (224.0.0.0/4), reserved (240.0.0.0/4), broadcast + return false +} + +/** + * Expand a valid IPv6 address into its 8 16-bit words. Handles `::` + * compression, trailing dotted-quad (`::ffff:127.0.0.1`), and zone ids + * (`fe80::1%eth0`). Returns null when the string is not valid IPv6. String + * prefix checks are NOT sufficient here: `0:0:0:0:0:0:0:1` is loopback but + * doesn't contain "::1", and fe80::/10 spans first words fe80–febf. + */ +function ipv6Words(ip: string): number[] | null { + let addr = ip.split("%")[0]! + if (!net.isIPv6(addr)) return null + // Convert a trailing dotted-quad into its two hextets. + const tail = addr.slice(addr.lastIndexOf(":") + 1) + if (tail.includes(".")) { + const v4 = tail.split(".").map(Number) + const hex = `${((v4[0]! << 8) | v4[1]!).toString(16)}:${((v4[2]! << 8) | v4[3]!).toString(16)}` + addr = addr.slice(0, addr.lastIndexOf(":") + 1) + hex + } + const [head = "", rest] = addr.split("::") + const headWords = head === "" ? [] : head.split(":").map((h) => parseInt(h, 16)) + if (!addr.includes("::")) return headWords.length === 8 ? headWords : null + const restWords = !rest ? [] : rest.split(":").map((h) => parseInt(h, 16)) + const fill = 8 - headWords.length - restWords.length + if (fill < 0) return null + return [...headWords, ...Array(fill).fill(0), ...restWords] +} + +function embeddedV4(hi: number, lo: number): number[] { + return [(hi >> 8) & 255, hi & 255, (lo >> 8) & 255, lo & 255] +} + +function isPrivateIPv6(w: number[]): boolean { + const zeroThrough = (n: number) => w.slice(0, n).every((x) => x === 0) + if (zeroThrough(7) && w[7]! <= 1) return true // unspecified (::) and loopback (::1) + const w0 = w[0]! + if ((w0 & 0xffc0) === 0xfe80) return true // link-local (fe80::/10) + if ((w0 & 0xffc0) === 0xfec0) return true // site-local, deprecated (fec0::/10) + if ((w0 & 0xfe00) === 0xfc00) return true // unique local (fc00::/7) + if ((w0 & 0xff00) === 0xff00) return true // multicast (ff00::/8) + // Addresses that embed an IPv4 target inherit its privacy: + if (zeroThrough(5) && w[5] === 0xffff) return isPrivateIPv4(embeddedV4(w[6]!, w[7]!)) // IPv4-mapped (::ffff:0:0/96) + if (w0 === 0x64 && w[1] === 0xff9b && w.slice(2, 6).every((x) => x === 0)) + return isPrivateIPv4(embeddedV4(w[6]!, w[7]!)) // NAT64 (64:ff9b::/96) + if (w0 === 0x2002) return isPrivateIPv4(embeddedV4(w[1]!, w[2]!)) // 6to4 (2002::/16) + return false +} + +export function isPrivateIp(ip: string): boolean { + if (net.isIPv4(ip)) return isPrivateIPv4(ip.split(".").map(Number)) + const words = ipv6Words(ip) + if (words) return isPrivateIPv6(words) + return false +} + +const ALLOW_PRIVATE_HINT = "Set OPENCODE_WEBFETCH_ALLOW_PRIVATE=1 to allow fetching private/internal addresses." + +export async function assertPublicUrl(rawUrl: string): Promise { + // URL.hostname keeps brackets for IPv6 literals (e.g. "[::1]"); strip them so net.isIP works. + const host = new URL(rawUrl).hostname.replace(/^\[|\]$/g, "") + // Strip a trailing dot ("localhost." is the same host as "localhost"). + const lower = host.toLowerCase().replace(/\.$/, "") + if ( + lower === "localhost" || + lower.endsWith(".localhost") || + lower.endsWith(".local") || + // Covers cloud-internal DNS such as metadata.google.internal and *.internal on GCP. + lower === "internal" || + lower.endsWith(".internal") + ) { + throw new Error(`Refusing to fetch internal host: ${host}. ${ALLOW_PRIVATE_HINT}`) + } + if (net.isIP(host)) { + if (isPrivateIp(host)) throw new Error(`Refusing to fetch private/loopback address: ${host}. ${ALLOW_PRIVATE_HINT}`) + return + } + const records = await dns.lookup(host, { all: true }) + for (const record of records) { + if (isPrivateIp(record.address)) { + throw new Error( + `Refusing to fetch host that resolves to a private address: ${host} -> ${record.address}. ${ALLOW_PRIVATE_HINT}`, + ) + } + } +} export const Parameters = Schema.Struct({ url: Schema.String.annotate({ description: "The URL to fetch content from" }), @@ -25,7 +146,6 @@ export const WebFetchTool = Tool.define( "webfetch", Effect.gen(function* () { const http = yield* HttpClient.HttpClient - const httpOk = HttpClient.filterStatusOk(http) return { description: DESCRIPTION, @@ -47,6 +167,19 @@ export const WebFetchTool = Tool.define( }, }) + // SSRF guard: block loopback / link-local / private targets. Redirect + // hops are re-validated in the fetch loop below. Remaining known gap: + // DNS rebinding (a hostname that resolves public here but private when + // the client re-resolves it) — closing it fully requires pinning the + // validated IP into the connection, which fetch does not expose. + const allowPrivate = yield* AllowPrivateFetch + if (!allowPrivate) { + yield* Effect.tryPromise({ + try: () => assertPublicUrl(params.url), + catch: (error) => (error instanceof Error ? error : new Error(String(error))), + }) + } + const timeout = Math.min((params.timeout ?? DEFAULT_TIMEOUT / 1000) * 1000, MAX_TIMEOUT) // Build Accept header based on requested format with q parameters for fallbacks @@ -73,35 +206,78 @@ export const WebFetchTool = Tool.define( "Accept-Language": "en-US,en;q=0.9", } - const request = HttpClientRequest.get(params.url).pipe(HttpClientRequest.setHeaders(headers)) - - // Retry with honest UA if blocked by Cloudflare bot detection (TLS fingerprint mismatch) - const response = yield* httpOk.execute(request).pipe( - Effect.catchIf( - (err) => - err.reason._tag === "StatusCodeError" && - err.reason.response.status === 403 && - err.reason.response.headers["cf-mitigated"] === "challenge", - () => - httpOk.execute( - HttpClientRequest.get(params.url).pipe( - HttpClientRequest.setHeaders({ ...headers, "User-Agent": "opencode" }), - ), - ), - ), + // Redirects are followed manually so that EVERY hop is re-validated by + // the SSRF guard — otherwise a public URL could 302 to an internal + // address and the client would follow it silently. `redirect: "manual"` + // makes the fetch-based client surface 3xx responses; a client layer + // that still auto-follows simply never yields a 3xx here and degrades + // to the previous behavior. + const fetchOnce = (url: string) => + http.execute(HttpClientRequest.get(url).pipe(HttpClientRequest.setHeaders(headers))).pipe( + // Retry with honest UA if blocked by Cloudflare bot detection (TLS fingerprint mismatch) + Effect.flatMap((response) => + response.status === 403 && response.headers["cf-mitigated"] === "challenge" + ? http.execute( + HttpClientRequest.get(url).pipe( + HttpClientRequest.setHeaders({ ...headers, "User-Agent": "opencode" }), + ), + ) + : Effect.succeed(response), + ), + Effect.provideService(FetchHttpClient.RequestInit, { redirect: "manual" }), + ) + + const response = yield* Effect.gen(function* () { + let url = params.url + for (let hop = 0; hop <= MAX_REDIRECTS; hop++) { + const response = yield* fetchOnce(url) + const location = response.headers["location"] + if (response.status >= 300 && response.status < 400 && location) { + const next = new URL(location, url) + if (next.protocol !== "http:" && next.protocol !== "https:") { + throw new Error(`Refusing to follow redirect to non-http(s) URL: ${next}`) + } + if (!allowPrivate) { + yield* Effect.tryPromise({ + try: () => assertPublicUrl(next.toString()), + catch: (error) => (error instanceof Error ? error : new Error(String(error))), + }) + } + url = next.toString() + continue + } + if (response.status < 200 || response.status >= 300) { + throw new Error(`Request failed with status code: ${response.status}`) + } + return response + } + throw new Error(`Too many redirects (limit: ${MAX_REDIRECTS})`) + }).pipe( Effect.timeoutOrElse({ duration: timeout, orElse: () => Effect.die(new Error("Request timed out")) }), ) - // Check content length + // Check content length up front when advertised. const contentLength = response.headers["content-length"] if (contentLength && parseInt(contentLength) > MAX_RESPONSE_SIZE) { throw new Error("Response too large (exceeds 5MB limit)") } - const arrayBuffer = yield* response.arrayBuffer - if (arrayBuffer.byteLength > MAX_RESPONSE_SIZE) { - throw new Error("Response too large (exceeds 5MB limit)") - } + // Stream the body and abort as soon as the size cap is exceeded, instead of + // buffering the entire (possibly unbounded) response into memory first. + const chunks: Uint8Array[] = [] + let received = 0 + yield* response.stream.pipe( + Stream.runForEach((chunk: Uint8Array) => + Effect.sync(() => { + received += chunk.length + if (received > MAX_RESPONSE_SIZE) { + throw new Error("Response too large (exceeds 5MB limit)") + } + chunks.push(chunk) + }), + ), + ) + const arrayBuffer = Buffer.concat(chunks) const contentType = response.headers["content-type"] || "" const mime = contentType.split(";")[0]?.trim().toLowerCase() || "" @@ -155,21 +331,27 @@ export const WebFetchTool = Tool.define( }), ) -function extractTextFromHTML(html: string) { +// Tags whose text content should never appear in extracted output. +const NON_TEXT_TAGS = new Set(["script", "style", "noscript", "iframe", "object", "embed"]) + +export function extractTextFromHTML(html: string) { let text = "" + // Count only opens/closes of non-text tags by name. The previous version + // incremented on *every* nested open tag once inside a skipped region, which + // could desync (and skip the rest of the document) when a void/self-closing + // element did not emit a matching close. Tracking skip tags by name is robust + // to unbalanced void elements. let skipDepth = 0 const parser = new Parser({ onopentag(name) { - if (skipDepth > 0 || ["script", "style", "noscript", "iframe", "object", "embed"].includes(name)) { - skipDepth++ - } + if (NON_TEXT_TAGS.has(name)) skipDepth++ }, ontext(input) { if (skipDepth === 0) text += input }, - onclosetag() { - if (skipDepth > 0) skipDepth-- + onclosetag(name) { + if (NON_TEXT_TAGS.has(name) && skipDepth > 0) skipDepth-- }, }) diff --git a/packages/opencode/test/permission/semantics.test.ts b/packages/opencode/test/permission/semantics.test.ts new file mode 100644 index 000000000000..a426aa37759e --- /dev/null +++ b/packages/opencode/test/permission/semantics.test.ts @@ -0,0 +1,86 @@ +import { test, expect, describe } from "bun:test" +import { Wildcard } from "@opencode-ai/core/util/wildcard" +import { PermissionV2 } from "@opencode-ai/core/permission" + +type Rule = PermissionV2.Rule + +describe("Wildcard.match", () => { + test("`*` matches any run of characters", () => { + expect(Wildcard.match("git push origin", "git *")).toBe(true) + }) + test("trailing ` *` is optional (bare command matches)", () => { + expect(Wildcard.match("git push", "git push *")).toBe(true) + }) + test("`?` matches exactly one character", () => { + expect(Wildcard.match("ab", "a?")).toBe(true) + expect(Wildcard.match("abc", "a?")).toBe(false) + }) + test("backslashes are normalized to `/`", () => { + expect(Wildcard.match("a\\b\\c", "a/b/*")).toBe(true) + }) + test("exact and non-matching", () => { + expect(Wildcard.match("bash", "bash")).toBe(true) + expect(Wildcard.match("bash", "edit")).toBe(false) + }) + test("case sensitivity follows the platform (intentional)", () => { + // case-insensitive on Windows, case-sensitive elsewhere (mirrors OS behavior) + const expected = process.platform === "win32" + expect(Wildcard.match("RM file", "rm *")).toBe(expected) + }) +}) + +describe("PermissionV2.evaluate (last match wins)", () => { + const rules: Rule[] = [ + { permission: "bash", pattern: "*", action: "deny" }, + { permission: "bash", pattern: "git *", action: "allow" }, + ] + test("later, more specific allow overrides an earlier blanket deny", () => { + expect(PermissionV2.evaluate("bash", "git push", rules).action).toBe("allow") + }) + test("falls back to the blanket deny for non-matching patterns", () => { + expect(PermissionV2.evaluate("bash", "rm -rf", rules).action).toBe("deny") + }) + test("defaults to `ask` when nothing matches", () => { + expect(PermissionV2.evaluate("read", "anything", []).action).toBe("ask") + }) +}) + +describe("PermissionV2.disabled (intentional coarse UI gate)", () => { + test("a blanket `*` deny disables the tool", () => { + const d = PermissionV2.disabled(["bash"], [{ permission: "*", pattern: "*", action: "deny" }]) + expect(d.has("bash")).toBe(true) + }) + test("a narrower allow BEFORE a trailing blanket deny does NOT keep it enabled", () => { + const d = PermissionV2.disabled( + ["bash"], + [ + { permission: "bash", pattern: "git *", action: "allow" }, + { permission: "bash", pattern: "*", action: "deny" }, + ], + ) + expect(d.has("bash")).toBe(true) + }) + test("a narrower allow AFTER a blanket deny re-enables it (last match wins)", () => { + const d = PermissionV2.disabled( + ["bash"], + [ + { permission: "bash", pattern: "*", action: "deny" }, + { permission: "bash", pattern: "git *", action: "allow" }, + ], + ) + expect(d.has("bash")).toBe(false) + }) + test("specific-only denies (no blanket `*`) do not disable", () => { + const d = PermissionV2.disabled(["task"], [{ permission: "task", pattern: "orch-*", action: "deny" }]) + expect(d.has("task")).toBe(false) + }) + test("edit-family tools collapse onto the `edit` permission", () => { + const d = PermissionV2.disabled( + ["edit", "write", "apply_patch"], + [{ permission: "edit", pattern: "*", action: "deny" }], + ) + expect(d.has("edit")).toBe(true) + expect(d.has("write")).toBe(true) + expect(d.has("apply_patch")).toBe(true) + }) +}) diff --git a/packages/opencode/test/tool/webfetch-extract.test.ts b/packages/opencode/test/tool/webfetch-extract.test.ts new file mode 100644 index 000000000000..91a6ce66f040 --- /dev/null +++ b/packages/opencode/test/tool/webfetch-extract.test.ts @@ -0,0 +1,26 @@ +import { test, expect, describe } from "bun:test" +import { extractTextFromHTML } from "../../src/tool/webfetch" + +describe("extractTextFromHTML", () => { + test("keeps visible text and drops tags", () => { + expect(extractTextFromHTML("

Hello world

")).toBe("Hello world") + }) + test("removes

b

")).toBe("ab") + }) + test("removes

c

")).toBe("c") + }) + test("removes

vis2

")).toBe("visvis2") + }) + test("a void element (
) does not desync skip tracking", () => { + // Regression: previously any nested open tag inside a skipped region could + // leave skipDepth stuck, swallowing the rest of the document. The
here + // must not prevent "after" (outside the script) from being extracted. + expect(extractTextFromHTML("

one
two

after")).toBe("onetwoafter") + }) + test("nested skip tags are fully removed", () => { + expect(extractTextFromHTML("
keeptail
")).toBe("keeptail") + }) +}) diff --git a/packages/opencode/test/tool/webfetch-ssrf.test.ts b/packages/opencode/test/tool/webfetch-ssrf.test.ts new file mode 100644 index 000000000000..1bab35423715 --- /dev/null +++ b/packages/opencode/test/tool/webfetch-ssrf.test.ts @@ -0,0 +1,96 @@ +import { test, expect, describe } from "bun:test" +import { isPrivateIp, assertPublicUrl } from "../../src/tool/webfetch" + +describe("isPrivateIp", () => { + const priv = [ + // IPv4 + "127.0.0.1", + "10.0.0.5", + "172.16.0.1", + "172.31.255.255", + "192.168.1.1", + "169.254.169.254", + "100.64.0.1", + "0.0.0.0", + "192.0.0.8", // IETF protocol assignments + "192.0.2.1", // TEST-NET-1 + "198.18.0.1", // benchmarking + "198.19.255.255", // benchmarking + "198.51.100.7", // TEST-NET-2 + "203.0.113.9", // TEST-NET-3 + "224.0.0.1", // multicast + "240.0.0.1", // reserved + "255.255.255.255", // broadcast + // IPv6 + "::1", + "::", + "0:0:0:0:0:0:0:1", // loopback, uncompressed — string checks like a === "::1" miss this + "fe80::1", + "FE80::1", + "febf::1", // still inside fe80::/10 + "fec0::1", // deprecated site-local + "fd00::1", + "fc00::1", + "ff02::1", // multicast + "::ffff:127.0.0.1", + "::ffff:7f00:1", + "::ffff:169.254.169.254", + "64:ff9b::7f00:1", // NAT64 embedding 127.0.0.1 + "2002:7f00:1::", // 6to4 embedding 127.0.0.1 + "fe80::1%eth0", // zone id + ] + const pub = [ + "8.8.8.8", + "1.1.1.1", + "93.184.216.34", + "172.32.0.1", + "172.15.0.1", + "100.63.0.1", + "100.128.0.1", + "198.17.0.1", + "198.20.0.1", + "223.255.255.255", + "2606:4700:4700::1111", + "::ffff:808:808", // IPv4-mapped 8.8.8.8 — mapped form of a PUBLIC address stays public + "64:ff9b::808:808", // NAT64 embedding 8.8.8.8 + "fe00::1", // outside fe80::/10 + ] + + test.each(priv)("treats %s as private", (ip) => { + expect(isPrivateIp(ip)).toBe(true) + }) + test.each(pub)("treats %s as public", (ip) => { + expect(isPrivateIp(ip)).toBe(false) + }) +}) + +describe("assertPublicUrl", () => { + const blocked = [ + "http://localhost/", + "http://localhost./", // trailing dot must not bypass the name check + "http://foo.local/", + "http://metadata.google.internal/", + "http://anything.internal/", + "http://127.0.0.1/", + "http://169.254.169.254/latest/meta-data/", + "http://10.0.0.1/", + "http://[::1]/", + "http://[0:0:0:0:0:0:0:1]/", + "http://[fe80::1]:8080/", + "http://[::ffff:169.254.169.254]/", + // WHATWG URL normalizes non-dotted numeric hosts to IPv4 dotted form + "http://2130706433/", // 127.0.0.1 as a decimal integer + "http://0x7f000001/", // 127.0.0.1 in hex + ] + test.each(blocked)("rejects %s", async (url) => { + await expect(assertPublicUrl(url)).rejects.toThrow() + }) + + test("allows a public IP literal without DNS", async () => { + await expect(assertPublicUrl("http://1.1.1.1/")).resolves.toBeUndefined() + }) + + test("allows a public IPv6 literal without DNS", async () => { + await expect(assertPublicUrl("http://[2606:4700:4700::1111]/")).resolves.toBeUndefined() + }) +}) diff --git a/packages/opencode/test/tool/webfetch.test.ts b/packages/opencode/test/tool/webfetch.test.ts index fdf5210b9c13..a5b728970027 100644 --- a/packages/opencode/test/tool/webfetch.test.ts +++ b/packages/opencode/test/tool/webfetch.test.ts @@ -3,7 +3,7 @@ import { Effect, 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 { AllowPrivateFetch, WebFetchTool } from "../../src/tool/webfetch" import { SessionID, MessageID } from "../../src/session/schema" import { Tool } from "@/tool/tool" import { testEffect } from "../lib/effect" @@ -31,10 +31,15 @@ const withFetch = ( (server) => Effect.sync(() => server.stop(true)), ) -const exec = Effect.fn("WebFetchToolTest.exec")(function* (args: Tool.InferParameters) { +const exec = Effect.fn("WebFetchToolTest.exec")(function* ( + args: Tool.InferParameters, + options?: { allowPrivate?: boolean }, +) { const info = yield* WebFetchTool const tool = yield* info.init() - return yield* tool.execute(args, ctx) + // These tests fetch from a local Bun.serve instance, which the SSRF guard + // would otherwise reject, so they opt in to private fetches by default. + return yield* tool.execute(args, ctx).pipe(Effect.provideService(AllowPrivateFetch, options?.allowPrivate ?? true)) }) describe("tool.webfetch", () => { @@ -110,4 +115,43 @@ describe("tool.webfetch", () => { }), ), ) + + it.instance("follows redirects to the final response", () => + withFetch( + (req) => { + const path = new URL(req.url).pathname + if (path === "/start") return new Response(null, { status: 302, headers: { location: "/end" } }) + return new Response("landed", { status: 200, headers: { "content-type": "text/plain" } }) + }, + (url) => + Effect.gen(function* () { + const result = yield* exec({ url: new URL("/start", url).toString(), format: "text" }) + expect(result.output).toBe("landed") + }), + ), + ) + + it.instance("fails after too many redirects", () => + withFetch( + () => new Response(null, { status: 302, headers: { location: "/loop" } }), + (url) => + Effect.gen(function* () { + const exit = yield* Effect.exit(exec({ url: new URL("/loop", url).toString(), format: "text" })) + expect(exit._tag).toBe("Failure") + }), + ), + ) + + it.instance("rejects loopback targets when the SSRF guard is enabled", () => + withFetch( + () => new Response("secret", { status: 200 }), + (url) => + Effect.gen(function* () { + const exit = yield* Effect.exit( + exec({ url: new URL("/", url).toString(), format: "text" }, { allowPrivate: false }), + ) + expect(exit._tag).toBe("Failure") + }), + ), + ) })