Skip to content

fix: resolve proxy transport false-positives and windows linter issues #3900

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.

Sign up for GitHub

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

Open
Piyush0049 wants to merge 1 commit into
base: main
Choose a base branch
from
+36 −23
Open

fix: resolve proxy transport false-positives and windows linter issues #3900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions pkg/desktop/transport/transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,24 +140,24 @@ func (f *fallbackTransport) RoundTrip(req *http.Request) (*http.Response, error)
}

// isProxySocketError checks if the error indicates the proxy socket is unavailable.
// This includes:
// - "no such file or directory" - socket file was deleted
// - "connection refused" - socket exists but nothing is listening
// - "dial unix" errors - general Unix socket connection failures
// Direct target TCP dial errors (e.g. dial tcp) return false to avoid disabling the proxy.
func isProxySocketError(err error) bool {
if err == nil {
return false
}

errStr := strings.ToLower(err.Error())

// Check for common proxy socket failure patterns
// Target TCP connection errors are target host errors, not proxy socket failures.
if strings.Contains(errStr, "dial tcp") && !strings.Contains(errStr, "proxyconnect tcp") {

@docker-agent docker-agent Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] Removal of "connect: connection refused" pattern may leave bare-TCP proxy failures undetected

The PR removes "connect: connection refused" from proxyErrorPatterns (previously the safety net for the case where a socket exists but nothing is listening). The replacement relies on two assumptions: (1) every genuine proxy connection failure will contain "proxyconnect tcp", and (2) every "dial tcp … connect: connection refused" without "proxyconnect tcp" is a target-host error.

Assumption (1) holds for standard Go net/http HTTP-CONNECT proxy paths — those always produce a proxyconnect tcp: … prefix. However if the proxy transport is configured as a direct TCP dial (not an HTTP CONNECT tunnel), Go wraps the failure without that prefix, yielding a bare "dial tcp 127.0.0.1:<port>: connect: connection refused" error. That string hits the new early-return guard at line 152 and returns false, so disableProxy() is never called and the agent keeps retrying against the dead proxy.

In practice this codebase appears to go through HTTP CONNECT, so the risk is low — but the gap is real and there is no test covering the bare-TCP proxy case. Consider re-adding the pattern with a tighter guard (e.g. only match when the error also contains "proxyconnect tcp" or "dial unix"), or add a comment documenting why bare-TCP proxy failures cannot occur here.

Confidence Score
🟡 moderate 60/100

return false
}

proxyErrorPatterns := []string{
"no such file or directory", // Socket file deleted
"connect: connection refused", // Socket exists but no listener
"proxyconnect tcp", // Proxy connection failure
"dial unix", // Unix socket dial failure
"unix socket", // Generic Unix socket error
"no such file or directory", // Socket file deleted
"proxyconnect tcp", // Proxy connection failure
"dial unix", // Unix socket dial failure
"unix socket", // Generic Unix socket error
}

for _, pattern := range proxyErrorPatterns {
Expand Down
15 changes: 15 additions & 0 deletions pkg/desktop/transport/transport_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ func TestIsProxySocketError(t *testing.T) {
errStr: "Post https://api.anthropic.com/v1/messages: proxyconnect tcp: some error",
expected: true,
},
{
name: "proxyconnect tcp with dial tcp error",
errStr: "proxyconnect tcp: dial tcp 10.0.0.1:443: connect: connection refused",
expected: true,
},
{
name: "dial unix error",
errStr: "dial unix /var/run/docker.sock: operation timed out",
Expand All @@ -93,6 +98,16 @@ func TestIsProxySocketError(t *testing.T) {
errStr: "dial tcp 192.168.1.1:443: i/o timeout",
expected: false,
},
{
name: "target TCP connection refused",
errStr: "dial tcp 127.0.0.1:8080: connect: connection refused",
expected: false,
},
{
name: "target HTTP request dial refusal",
errStr: "Get \"http://127.0.0.1:8080\": dial tcp 127.0.0.1:8080: connect: connection refused",
expected: false,
},
{
name: "HTTP error",
errStr: "HTTP 500: internal server error",
Expand Down
10 changes: 4 additions & 6 deletions pkg/model/provider/gemini/schema_boolean_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package gemini

import (
"testing"

"github.com/stretchr/testify/require"
)

// A tool input schema containing a boolean sub-schema — the shape a JSON Schema
Expand Down Expand Up @@ -30,12 +32,8 @@ func TestConvertParametersToSchema_BooleanSubSchema(t *testing.T) {
}

schema, err := ConvertParametersToSchema(params)
if err != nil {
t.Fatalf("ConvertParametersToSchema: %v", err)
}
if schema == nil {
t.Fatal("nil schema")
}
require.NoError(t, err)
require.NotNil(t, schema)
if _, ok := schema.Properties["count"]; !ok {
t.Errorf("count property dropped; got %v", schema.Properties)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/selfupdate/exec_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ func swapBinary(dst, src string) error {
if cpErr := atomicWriteFromFile(dst, src); cpErr != nil {
// Roll back so we never leave the install without a binary.
if rbErr := os.Rename(old, dst); rbErr != nil {
return fmt.Errorf("installing new binary: %w (copy fallback failed: %v; rollback also failed: %v)", err, cpErr, rbErr)
return fmt.Errorf("installing new binary: %w (copy fallback failed: %w; rollback also failed: %w)", err, cpErr, rbErr)
}
return fmt.Errorf("installing new binary: %w (copy fallback failed: %v)", err, cpErr)
return fmt.Errorf("installing new binary: %w (copy fallback failed: %w)", err, cpErr)
}
_ = os.Remove(src)
}
Expand All @@ -48,7 +48,7 @@ func reExecProcess(path string, args, env []string) error {
childArgs = args[1:]
}

cmd := exec.Command(path, childArgs...) //nolint:gosec // path is our own freshly installed binary
cmd := exec.Command(path, childArgs...) //nolint:noctx // path is our own freshly installed binary
cmd.Env = env
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
Expand Down
4 changes: 2 additions & 2 deletions pkg/tools/builtin/backgroundjobs/cmd_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ func createProcessGroup(proc *os.Process) (*processGroup, error) {
if _, err := windows.SetInformationJobObject(
job,
windows.JobObjectExtendedLimitInformation,
uintptr(unsafe.Pointer(&info)),
uintptr(unsafe.Pointer(&info)), //nolint:gosec // C-style struct pointer conversion required for Windows API
uint32(unsafe.Sizeof(info))); err != nil {
_ = windows.CloseHandle(job)
return nil, err
}

handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(proc.Pid))
handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(proc.Pid)) //nolint:gosec // Process PID conversion to uint32
if err != nil {
_ = windows.CloseHandle(job)
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions pkg/tools/builtin/shell/cmd_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ func createProcessGroup(proc *os.Process) (*processGroup, error) {
if _, err := windows.SetInformationJobObject(
job,
windows.JobObjectExtendedLimitInformation,
uintptr(unsafe.Pointer(&info)),
uintptr(unsafe.Pointer(&info)), //nolint:gosec // C-style struct pointer conversion required for Windows API
uint32(unsafe.Sizeof(info))); err != nil {
_ = windows.CloseHandle(job)
return nil, err
}

handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(proc.Pid))
handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(proc.Pid)) //nolint:gosec // Process PID conversion to uint32
if err != nil {
_ = windows.CloseHandle(job)
return nil, err
Expand Down
Loading