forked from anomalyco/opencode
-
Notifications
You must be signed in to change notification settings - Fork 0
fix(opencode): close webfetch SSRF redirect bypass and harden IP checks #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+394
−33
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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("<p>Hello <b>world</b></p>")).toBe("Hello world") | ||
| }) | ||
| test("removes <script> content", () => { | ||
| expect(extractTextFromHTML("<p>a</p><script>var x=1;</script><p>b</p>")).toBe("ab") | ||
| }) | ||
| test("removes <style> content", () => { | ||
| expect(extractTextFromHTML("<style>.x{color:red}</style><p>c</p>")).toBe("c") | ||
| }) | ||
| test("removes <iframe> content", () => { | ||
| expect(extractTextFromHTML("<p>vis</p><iframe>frame</iframe><p>vis2</p>")).toBe("visvis2") | ||
| }) | ||
| test("a void element (<br>) 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 <br> here | ||
| // must not prevent "after" (outside the script) from being extracted. | ||
| expect(extractTextFromHTML("<p>one<br>two</p><script>HIDDEN</script>after")).toBe("onetwoafter") | ||
| }) | ||
| test("nested skip tags are fully removed", () => { | ||
| expect(extractTextFromHTML("<div>keep<script>no<style>x</style>more</script>tail</div>")).toBe("keeptail") | ||
| }) | ||
| }) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Does effect's FetchHttpClient or undici support pinning a resolved IP address (custom DNS lookup) per request to prevent DNS rebinding?💡 Result:
Yes, it is possible to pin a resolved IP address in undici to prevent DNS rebinding [1][2][3]. While Effect's FetchHttpClient layer is designed for environments like Bun, browsers, or the global Node.js fetch [4], the underlying undici library (used by Node.js or when using NodeHttpClient) provides the necessary control via its Agent configuration [5][6]. To pin an IP address and close the DNS-rebinding time-of-check to time-of-use (TOCTOU) gap, you must provide a custom
lookupfunction within theconnectoptions of an undici Agent [1][7][3]. This hook allows you to perform your own DNS resolution (or usedns.lookup), validate the IP address (e.g., against a blocklist of private/internal ranges), and then return that validated IP so the socket connects directly to it, bypassing any further resolution [1][8]. Example implementation: import { Agent } from 'undici'; import dns from 'node:dns'; // Create a custom Agent with a pinned DNS lookup const safeAgent = new Agent({ connect: { lookup: (hostname, options, callback) => { // 1. Perform custom resolution dns.lookup(hostname, options, (err, address, family) => { if (err) return callback(err, address, family); // 2. Validate address (e.g., check for private IP ranges) if (isUnsafe(address)) { return callback(new Error('SSRF blocked'), address, family); } // 3. Return the validated, pinned address callback(null, address, family); }); }, }, }); // Use the agent for your request // const client = new Client(origin, { dispatcher: safeAgent }); // or // await fetch(url, { dispatcher: safeAgent }); Note that if you are using Effect, you should ensure you are utilizing the undici-compatible HTTP client (e.g., NodeHttpClient.layerUndici) rather than the generic FetchHttpClient layer, as the latter might rely on the global fetch environment where dispatcher injection may be restricted or have different compatibility characteristics [9][4][3]. Using undici directly allows you to control the socket-level connection details required for robust SSRF defense [1][7].Citations:
🏁 Script executed:
Repository: CreatorGhost/TheCode
Length of output: 24147
🏁 Script executed:
Repository: CreatorGhost/TheCode
Length of output: 1171
🏁 Script executed:
Repository: CreatorGhost/TheCode
Length of output: 1874
🏁 Script executed:
Repository: CreatorGhost/TheCode
Length of output: 50376
🏁 Script executed:
Repository: CreatorGhost/TheCode
Length of output: 11129
🌐 Web query:
Effect unstable/http FetchHttpClient.RequestInit dispatcher custom lookup undici pin IP💡 Result:
To pin an IP address or use a custom DNS lookup in Node.js using Undici, you must provide a custom lookup function within the connect options of a dispatcher (such as Client, Pool, or Agent) [1][2]. The standard approach involves creating an instance of a dispatcher with a connect configuration that overrides the default DNS lookup behavior [2]. Example using Undici Client: import { Client } from 'undici'; import dns from 'node:dns'; const client = new Client('https://example.com', { connect: { lookup: (hostname, options, callback) => { // Pin to a specific IP address callback(null, [{ address: '1.2.3.4', family: 4 }]); } } }); In the context of the Effect framework (e.g.,
@effect/platform/FetchHttpClient), you can provide a custom dispatcher by configuring the underlying HTTP client [3]. Because FetchHttpClient relies on Undici for its implementation, you can instantiate an Agent or Client with the custom connect options shown above and pass it to your fetch requests or provide it via the Effect layer [1][3]. Note that the dispatcher option in fetch or RequestInit is used to override the default global dispatcher [4]. Ensure that when you use a custom dispatcher, it is configured correctly to handle the requirements of your specific application [5]. The lookup function signature follows the standard Node.js dns.lookup callback pattern [6][7].Citations:
🌐 Web query:
Bun fetch dispatcher lookup undici custom Agent DNS rebinding💡 Result:
In Bun, using a custom Undici Agent to control DNS resolution—a common technique to prevent DNS rebinding—has historically faced compatibility issues because Bun's native fetch did not always fully respect custom dispatchers or their connect functions [1][2]. However, as of August 2025, Bun has implemented support for custom connect functions within the dispatcher option for fetch [3]. This improvement ensures that when you provide an Undici Agent (configured with a custom connect lookup) to the fetch dispatcher option, Bun correctly utilizes that logic to resolve hostnames [3]. To implement a DNS-rebinding-safe client in Bun, you can follow this pattern: 1. Perform your own DNS lookup for the target hostname [4]. 2. Validate the resolved IP address against your security policy (e.g., ensuring it is not a private or loopback address) [4]. 3. Create an Undici Agent with a custom connect.lookup function that returns your pre-validated IP address [4]. 4. Pass this Agent as the dispatcher option to your fetch request [4]. Example implementation: import { Agent } from "undici"; import dns from "dns/promises"; // 1. Resolve and validate IP manually const { address } = await dns.lookup("example.com"); // 2. Define custom dispatcher const dispatcher = new Agent({ connect: { lookup: (_hostname, _options, cb) => { // Force the agent to use the validated IP cb(null, address, 4); }, }, }); // 3. Use in fetch const response = await fetch("https://example.com", { dispatcher }); While Bun continues to improve its Undici compatibility to support libraries like fetch-socks or custom agents [5][2], always verify your specific Bun version if you encounter unexpected behavior, as some features were implemented recently as part of maintaining ecosystem compatibility [3][5].
Citations:
undici.Agent'sconnectoption goes unused, makingfetch-socksnot work oven-sh/bun#21492🌐 Web query:
"effect/unstable/http" "FetchHttpClient.RequestInit" RequestInit service dispatcher💡 Result:
In Effect, FetchHttpClient is an implementation of the HttpClient service provided by the
@effect/platformpackage [1][2]. The RequestInit configuration for FetchHttpClient allows you to customize the underlying Fetch API request settings [3][1]. To customize the FetchHttpClient, you provide a configuration object using the FetchHttpClient.RequestInit tag [1][2]. This is typically done by creating a Layer that provides this configuration to the runtime [1][2]. Example usage: import { FetchHttpClient, HttpClient } from "@effect/platform" import { Effect, Layer } from "effect" const CustomFetchLive = FetchHttpClient.layer.pipe( Layer.provide( Layer.succeed(FetchHttpClient.RequestInit, { credentials: "include", cache: "force-cache" // Example of customizing cache behavior [4] }))) const program = Effect.gen(function* { const client = yield* HttpClient.HttpClient const response = yield* client.get("https://jsonplaceholder.typicode.com/posts/1") const json = yield* response.json console.log(json) }).pipe( Effect.scoped, Effect.provide(CustomFetchLive)) Regarding a "service dispatcher," the HttpClient service in Effect functions as an abstraction over HTTP operations [5]. It does not typically use a "dispatcher" by that specific name in its public API; rather, it uses the Effect environment (Context) to provide the implementation (e.g., FetchHttpClient) and uses standard Effect piping and layering for dependency injection [1][2][5]. The HttpClient interface provides methods like get, post, put, and del, which are executed using the provided client implementation [5][6]. If you require custom dispatching or middleware logic, you can use HttpClient.mapRequest, HttpClient.mapRequestEffect, or create custom Layers that wrap or augment the base HttpClient implementation [5][6][7].Citations:
🌐 Web query:
site:effect.website FetchHttpClient RequestInit dispatcher undici💡 Result:
In the context of the Effect ecosystem, particularly within
@effect/platform-node, theundicilibrary is utilized as a backend for HTTP requests [1]. When usingFetchHttpClient, which serves as an implementation of theHttpClientinterface, the underlying communication is handled byundici[1][2]. Regarding the use ofdispatcherwithFetchHttpClientandRequestInit: 1. Undici Dispatcher: Theundicilibrary uses aDispatcherto manage HTTP requests. WhenFetchHttpClientis employed, it is typically configured via a layer (e.g.,FetchHttpClient.layerorlayerUndici) [3][4]. If you need to customize theundiciclient (such as providing a specificdispatcher), this is generally handled at the layer creation level when configuring theHttpClientservice, rather than through individualRequestInitobjects [3][2]. 2. RequestInit: The standardRequestInitinterface (used by the nativefetchAPI) does not natively support anundici-specificdispatcherproperty.RequestInitis intended for standard fetch options likemethod,headers, andbody[5]. 3. Implementation Detail: BecauseFetchHttpClientis an Effect-based abstraction, configuration that requires underlying engine details—like a customundici.Dispatcher—is managed through Effect's dependency injection system (Layers) [3][2]. You should configure theHttpClientlayer with your requiredundicisettings (like a customAgentorDispatcher) when providing the layer to your application, ensuring that all requests made through the injectedHttpClientutilize the desired configuration [3][2]. If you are encountering issues where a globalundicidispatcher is not being picked up or needs to be explicitly defined, verify that your layer configuration in the Effect program correctly injects theHttpClientwith theundiciclient configured to your specifications [6][4].Citations:
Close the DNS-rebinding gap at the transport layer.
assertPublicUrl()only validates one DNS answer before the request; the fetch path can still re-resolve for the actual connection and each redirect hop.FetchHttpClient.RequestInitdoesn’t expose per-request IP pinning, so this needs a customHttpClient/dispatcher layer that reuses the validated address for the socket.🤖 Prompt for AI Agents