Skip to content

Use Docker Desktop proxy when available#2222

Merged
dgageot merged 5 commits intodocker:mainfrom
gtardif:use_desktop_proxy_when_up
Mar 23, 2026
Merged

Use Docker Desktop proxy when available#2222
dgageot merged 5 commits intodocker:mainfrom
gtardif:use_desktop_proxy_when_up

Conversation

@gtardif
Copy link
Contributor

@gtardif gtardif commented Mar 23, 2026

  • Check if Desktop is running and uses Desktop proxy for all download operations if yes
  • timeout checking if desktop is running
  • memoize it

gtardif added 2 commits March 23, 2026 08:57
Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>
@gtardif gtardif requested a review from a team as a code owner March 23, 2026 08:57
@gtardif gtardif changed the title Use Docker Desktop proxy when up Use Docker Desktop proxy when available Mar 23, 2026
Copy link

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

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

Assessment: 🔴 CRITICAL

This PR introduces Docker Desktop proxy support but has several implementation issues that need to be addressed.

@gtardif gtardif force-pushed the use_desktop_proxy_when_up branch from 44c2f53 to 84b20f9 Compare March 23, 2026 09:19
@gtardif gtardif marked this pull request as draft March 23, 2026 09:24
gtardif added 2 commits March 23, 2026 10:32
Updates fetch, openapi, api tools, gateway catalog, skills cache, and models.dev store to use remote.NewTransport for Desktop proxy support when downloading external content.

Assisted-By: docker-agent
Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>
@gtardif gtardif force-pushed the use_desktop_proxy_when_up branch 2 times, most recently from b63b305 to d8845b2 Compare March 23, 2026 09:38
@gtardif gtardif marked this pull request as ready for review March 23, 2026 09:39
Copy link

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

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

Assessment: 🔴 CRITICAL

Found critical bugs in the Docker Desktop proxy integration:

🔴 Critical: Missing ProxySocket for WSL2 and Native Linux

File: pkg/desktop/sockets_linux.go

The PR adds ProxySocket to DockerDesktopPaths and updates the LinuxKit case (line 15 in diff), but forgets to update the WSL2 case (line 26 in file) and native Linux case (line 40 in file).

Current code (WSL2):

return DockerDesktopPaths{
    BackendSocket: "/mnt/wsl/docker-desktop/shared-sockets/host-services/backend.sock",
    // ProxySocket is missing!
}, nil

Current code (native Linux):

return DockerDesktopPaths{
    BackendSocket: filepath.Join(home, ".docker", "desktop", "backend.sock"),
    // ProxySocket is missing!
}, nil

Impact: When NewTransport() calls desktop.Paths().ProxySocket, it will get an empty string on WSL2 and native Linux, causing socket.DialUnix() to fail. All HTTP operations will break on these platforms.

Fix: Add ProxySocket to both cases:

// WSL2 case:
return DockerDesktopPaths{
    BackendSocket: "/mnt/wsl/docker-desktop/shared-sockets/host-services/backend.sock",
    ProxySocket:   "/mnt/wsl/docker-desktop/shared-sockets/host-services/httpproxy.sock",
}, nil

// Native Linux case:
return DockerDesktopPaths{
    BackendSocket: filepath.Join(home, ".docker", "desktop", "backend.sock"),
    ProxySocket:   filepath.Join(home, ".docker", "desktop", "httpproxy.sock"),
}, nil

⚠️ Medium: Context handling in memoizer

File: pkg/remote/transport.go line 26

The memoizer caches IsDockerDesktopRunning(ctx) for 1 minute. If the first call's context is cancelled during the check, an incorrect result (false negative) gets cached.

Recommendation: Use a shorter cache duration or validate context state before using cached results.

⚠️ Medium: Context cancellation in IsDockerDesktopRunning

File: pkg/desktop/running.go line 9

The function creates a new 3-second timeout context derived from the passed context. This is actually correct - context.WithTimeout(ctx, ...) properly propagates cancellation from the parent context. However, the memoization means this check only runs once per minute, so context cancellation won't be respected for cached results.


Summary: The critical issue is that ProxySocket is missing for WSL2 and native Linux. This will cause the proxy integration to fail on these platforms. Please add ProxySocket to all return statements in getDockerDesktopPaths().

@gtardif gtardif force-pushed the use_desktop_proxy_when_up branch from d8845b2 to 499872a Compare March 23, 2026 09:53
Copy link

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

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

Assessment: 🔴 CRITICAL

Found 2 HIGH and 3 MEDIUM severity issues in the Desktop proxy implementation.

Critical Issues:

  • Missing ProxySocket configuration for WSL2 and native Linux environments will cause proxy routing to fail completely on these platforms

Medium Issues:

  • Context handling issues in memoization and HTTP requests

…ansport

Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>
@gtardif gtardif force-pushed the use_desktop_proxy_when_up branch from 499872a to 725de01 Compare March 23, 2026 11:24
@dgageot dgageot merged commit 9a3e077 into docker:main Mar 23, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants