[PR #151] [MERGED] fix: prevent http.DefaultClient mutation in search tools #194

New Issue

2026-06-06T22:09:38-04:00

yindo commented

2026-06-06 22:09:38 -04:00

📋 Pull Request Information

Original PR: https://github.com/vxcontrol/pentagi/pull/151
Author: @mason5052
Created: 2/26/2026
Status: ✅ Merged
Merged: 3/2/2026
Merged by: @asdek

Base: feature/next_release ← Head: fix/http-client-global-mutation

📝 Commits (2)

e36f47f fix: prevent http.DefaultClient mutation in search tools
c716ec6 test: remove unused test server and dead imports in search tool tests

📊 Changes

4 files changed (+298 additions, -6 deletions)

View changed files

📝 backend/pkg/tools/tavily.go (+5 -3)
➕ backend/pkg/tools/tavily_test.go (+148 -0)
📝 backend/pkg/tools/traversaal.go (+5 -3)
➕ backend/pkg/tools/traversaal_test.go (+140 -0)

📄 Description

Description

Problem

Both tavily.go and traversaal.go assign http.DefaultClient to a local variable and then mutate its Transport field when configuring a proxy URL:

client := http.DefaultClient
if t.proxyURL != "" {
    client.Transport = &http.Transport{...}  // mutates global singleton
}

Since http.DefaultClient is a process-wide singleton, this creates a data race: concurrent requests from different tool instances overwrite each other's proxy configuration, and all other HTTP callers in the process inherit the mutated transport.

Solution

Create a new http.Client instance when proxy is configured instead of mutating the global. When no proxy is set, DefaultClient is used read-only (no mutation). This matches the correct pattern already used in browser.go:callScraper().

client := http.DefaultClient
if t.proxyURL != "" {
    client = &http.Client{           // new instance, no global mutation
        Transport: &http.Transport{
            Proxy: func(req *http.Request) (*url.URL, error) {
                return url.Parse(t.proxyURL)
            },
        },
    }
}

Type of Change

Bug fix (non-breaking change which fixes an issue)

Areas Affected

Core Services (Backend API / Workers)
External Integrations (Tavily, Traversaal search APIs)

Testing

Configuration

Local code review + CI validation (Go not installed locally)

Steps to Reproduce the Bug

Configure two tavily tool instances with different proxy URLs
Run concurrent search requests
Observe that http.DefaultClient.Transport is overwritten by whichever goroutine runs last
All subsequent HTTP requests in the process use the wrong proxy

Test Results

Added tavily_test.go: Tests that http.DefaultClient.Transport is NOT mutated after search() with proxy configured. Also covers IsAvailable() and parseHTTPResponse() error paths.
Added traversaal_test.go: Same mutation guard test, plus IsAvailable() and parseHTTPResponse() success/error paths.

Security Considerations

This fix eliminates a race condition that could cause proxy misconfiguration
No new dependencies or attack surface introduced
The fix is a minimal, targeted change to client instantiation

Checklist

My code follows the project's coding standards
I have performed a self-review of my code
I have added tests that prove my fix is effective
New and existing tests pass with my changes
My changes generate no new warnings
I have checked my code for potential security vulnerabilities
My changes are backward compatible

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/vxcontrol/pentagi/pull/151 **Author:** [@mason5052](https://github.com/mason5052) **Created:** 2/26/2026 **Status:** ✅ Merged **Merged:** 3/2/2026 **Merged by:** [@asdek](https://github.com/asdek) **Base:** `feature/next_release` ← **Head:** `fix/http-client-global-mutation` --- ### 📝 Commits (2) - [`e36f47f`](https://github.com/vxcontrol/pentagi/commit/e36f47ffd379ea93c944c7a5303e990884834cc0) fix: prevent http.DefaultClient mutation in search tools - [`c716ec6`](https://github.com/vxcontrol/pentagi/commit/c716ec6a7430fb47df5de3b2bc114afb9676c418) test: remove unused test server and dead imports in search tool tests ### 📊 Changes **4 files changed** (+298 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `backend/pkg/tools/tavily.go` (+5 -3) ➕ `backend/pkg/tools/tavily_test.go` (+148 -0) 📝 `backend/pkg/tools/traversaal.go` (+5 -3) ➕ `backend/pkg/tools/traversaal_test.go` (+140 -0) </details> ### 📄 Description ## Description ### Problem Both `tavily.go` and `traversaal.go` assign `http.DefaultClient` to a local variable and then **mutate its `Transport` field** when configuring a proxy URL: ```go client := http.DefaultClient if t.proxyURL != "" { client.Transport = &http.Transport{...} // mutates global singleton } ``` Since `http.DefaultClient` is a process-wide singleton, this creates a **data race**: concurrent requests from different tool instances overwrite each other's proxy configuration, and all other HTTP callers in the process inherit the mutated transport. ### Solution Create a **new `http.Client` instance** when proxy is configured instead of mutating the global. When no proxy is set, `DefaultClient` is used read-only (no mutation). This matches the correct pattern already used in `browser.go:callScraper()`. ```go client := http.DefaultClient if t.proxyURL != "" { client = &http.Client{ // new instance, no global mutation Transport: &http.Transport{ Proxy: func(req *http.Request) (*url.URL, error) { return url.Parse(t.proxyURL) }, }, } } ``` ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) ## Areas Affected - [x] Core Services (Backend API / Workers) - [x] External Integrations (Tavily, Traversaal search APIs) ## Testing ### Configuration - Local code review + CI validation (Go not installed locally) ### Steps to Reproduce the Bug 1. Configure two tavily tool instances with different proxy URLs 2. Run concurrent search requests 3. Observe that `http.DefaultClient.Transport` is overwritten by whichever goroutine runs last 4. All subsequent HTTP requests in the process use the wrong proxy ### Test Results - Added `tavily_test.go`: Tests that `http.DefaultClient.Transport` is NOT mutated after `search()` with proxy configured. Also covers `IsAvailable()` and `parseHTTPResponse()` error paths. - Added `traversaal_test.go`: Same mutation guard test, plus `IsAvailable()` and `parseHTTPResponse()` success/error paths. ## Security Considerations - This fix eliminates a race condition that could cause proxy misconfiguration - No new dependencies or attack surface introduced - The fix is a minimal, targeted change to client instantiation ## Checklist - [x] My code follows the project's coding standards - [x] I have performed a self-review of my code - [x] I have added tests that prove my fix is effective - [x] New and existing tests pass with my changes - [x] My changes generate no new warnings - [x] I have checked my code for potential security vulnerabilities - [x] My changes are backward compatible --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

yindo added the pull-request label 2026-06-06 22:09:38 -04:00

yindo closed this issue

2026-06-06 22:09:39 -04:00

yindo referenced this issue

2026-06-06 22:09:46 -04:00

[PR #194] [CLOSED] fix(providers): add iteration cap to performAgentChain to prevent infinite loops #223

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vxcontrol/pentagi#194