From bb6c474950a0c75a2540b6f884ef076f3e46f10a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20He=CC=81ritier?= Date: Sat, 12 Sep 2026 23:34:12 +0200 Subject: [PATCH] fix(record): scrub encrypted config from captures Require Docker Desktop authentication only for HTTPS docker.com gateways while retaining loopback trust for forwarded encrypted configuration. Scrub encrypted config and its digest independently from recording captures, and remove both before forwarding to untrusted or direct upstreams. Fixes #4250 --- cmd/root/doctor.go | 2 +- cmd/root/models_test.go | 27 ++---- docs/features/cli/index.md | 8 +- pkg/config/config.go | 2 +- pkg/config/config_test.go | 2 +- pkg/environment/docker-desktop.go | 16 +++- pkg/environment/docker_desktop_test.go | 26 ++++++ pkg/fake/proxy.go | 37 ++++++++ pkg/fake/proxy_gateway_test.go | 109 ++++++++++++++++++++++++ pkg/fake/streaming_recorder.go | 35 ++++++-- pkg/httpclient/client.go | 42 ++++++++- pkg/httpclient/client_test.go | 32 +++++++ pkg/model/provider/base/gateway.go | 18 ++-- pkg/model/provider/base/gateway_test.go | 12 +++ pkg/modelsgateway/discovery_test.go | 30 +++++-- 15 files changed, 341 insertions(+), 57 deletions(-) diff --git a/cmd/root/doctor.go b/cmd/root/doctor.go index ae9292d3bb..2a111cac32 100644 --- a/cmd/root/doctor.go +++ b/cmd/root/doctor.go @@ -289,7 +289,7 @@ func (f *doctorFlags) buildReport(ctx context.Context, agentRef string) (*doctor autoStatus.Note = "credentials are supplied by the models gateway" // Mirrors the run-time preflight: the Docker AI Gateway authenticates // with the Docker Desktop JWT, not per-provider API keys. - if environment.IsTrustedDockerURL(f.runConfig.ModelsGateway) { + if environment.IsDockerDomainURL(f.runConfig.ModelsGateway) { if _, ok := findSource(ctx, sources, environment.DockerDesktopTokenEnv); !ok { autoStatus.Usable = false autoIssues = append(autoIssues, diff --git a/cmd/root/models_test.go b/cmd/root/models_test.go index e259179e63..91456adc29 100644 --- a/cmd/root/models_test.go +++ b/cmd/root/models_test.go @@ -532,9 +532,8 @@ func newGatewayServer(t *testing.T, body string) (*httptest.Server, *atomic.Valu return server, &lastAuth } -// gatewayTestEnv is the hermetic env for gateway tests: httptest binds to -// 127.0.0.1, which IsTrustedDockerURL treats as trusted, so discovery -// requires the Docker Desktop token. +// gatewayTestEnv is the hermetic env for gateway tests: loopback gateways are +// trusted to receive an available Docker token but do not require one. func gatewayTestEnv(extra map[string]string) map[string]string { env := map[string]string{environment.DockerDesktopTokenEnv: "test-docker-token"} maps.Copy(env, extra) @@ -567,7 +566,7 @@ func TestModelsListCommand_GatewayProviderFilter(t *testing.T) { require.Len(t, rows, 1, "--provider must filter the live gateway results") assert.Equal(t, "google", rows[0].Provider) assert.Equal(t, "mock-gemini", rows[0].Model) - assert.Equal(t, "Bearer test-docker-token", lastAuth.Load(), "a trusted Docker gateway must be queried with the Docker token") + assert.Equal(t, "Bearer test-docker-token", lastAuth.Load(), "a trusted loopback gateway may receive the Docker token") } // TestModelsListCommand_GatewayNormalizesAndSorts covers the full live @@ -689,10 +688,9 @@ func TestModelsListCommand_GatewayFallback(t *testing.T) { t.Parallel() tests := []struct { - name string - handler http.HandlerFunc - env map[string]string - wantNotQueried bool + name string + handler http.HandlerFunc + env map[string]string }{ { name: "endpoint not found", @@ -714,15 +712,11 @@ func TestModelsListCommand_GatewayFallback(t *testing.T) { env: gatewayTestEnv(map[string]string{"ANTHROPIC_API_KEY": "test-key"}), }, { - // httptest is localhost, hence Docker-trusted: without the token - // the live request must not even be attempted, but the auth - // failure must not remove directly usable providers. name: "missing Docker token", handler: func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`{"object":"list","data":[{"id":"openai/mock-gpt"}]}`)) + http.Error(w, "unavailable", http.StatusServiceUnavailable) }, - env: map[string]string{"ANTHROPIC_API_KEY": "test-key"}, - wantNotQueried: true, + env: map[string]string{"ANTHROPIC_API_KEY": "test-key"}, }, } @@ -730,9 +724,7 @@ func TestModelsListCommand_GatewayFallback(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - var queried atomic.Bool gateway := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - queried.Store(true) tt.handler(w, r) })) t.Cleanup(gateway.Close) @@ -756,9 +748,6 @@ func TestModelsListCommand_GatewayFallback(t *testing.T) { assert.Contains(t, output, "claude-sonnet-5", "the direct anthropic provider must survive the gateway failure") assert.Contains(t, output, catalogOnlyModel, "the catalog fallback must be read") assert.Contains(t, output, "corp-model-a", "a usable custom provider must survive the gateway failure") - if tt.wantNotQueried { - assert.False(t, queried.Load(), "a trusted Docker gateway must not be queried without the Docker token") - } }) } } diff --git a/docs/features/cli/index.md b/docs/features/cli/index.md index eb20ebe65c..c57db6075b 100644 --- a/docs/features/cli/index.md +++ b/docs/features/cli/index.md @@ -60,7 +60,7 @@ $ docker agent run [config] [message...] [flags] | `--env-from-file ` | Load environment variables from file (repeatable) | | `--flavor ` | Enable a config flavor, a YAML patch defined under the config's `flavors` section (repeatable, applied in order). See [Flavors](../../configuration/flavors/index.md). | | `--code-mode-tools` | Provide a single tool to call other tools via JavaScript (forces code-mode tools globally) | -| `--models-gateway ` | Route model traffic through a gateway. Also reads `DOCKER_AGENT_MODELS_GATEWAY` (legacy `CAGENT_MODELS_GATEWAY`) env var. | +| `--models-gateway ` | Route model traffic through a gateway. Docker Desktop sign-in is required only for HTTPS `docker.com` gateways; loopback and third-party gateways need no Docker token. Also reads `DOCKER_AGENT_MODELS_GATEWAY` (legacy `CAGENT_MODELS_GATEWAY`) env var. | | `--hook-pre-tool-use ` | Add a pre-tool-use hook command (repeatable). See [Hooks](../../configuration/hooks/index.md). | | `--hook-post-tool-use ` | Add a post-tool-use hook command (repeatable) | | `--hook-session-start ` | Add a session-start hook command (repeatable) | @@ -69,7 +69,7 @@ $ docker agent run [config] [message...] [flags] | `--hook-stop ` | Add a stop hook command, fired when the model finishes responding (repeatable) | | `--fake ` | Replay AI responses from a cassette file (for testing). Mutually exclusive with `--record`. | | `--fake-stream [ms]` | When replaying with `--fake`, simulate streaming with a delay between chunks (defaults to 15ms when given without a value). | -| `--record [path]` | Record AI API interactions to a cassette file and generate a TUI e2e test from the session (auto-generates filename if no path given). Routes through `--models-gateway` when one is configured. | +| `--record [path]` | Record AI API interactions to a cassette file and generate a TUI e2e test from the session (auto-generates filename if no path given). Routes through `--models-gateway` when one is configured. Encrypted agent config and its digest are never stored in cassettes. | | `-d, --debug` | Enable debug logging | | `--log-file ` | Custom debug log location | | `-o, --otel` | Enable OpenTelemetry observability: traces, metrics, and logs. Requires `OTEL_EXPORTER_OTLP_ENDPOINT` to export to a collector. | @@ -211,7 +211,7 @@ $ docker agent models --provider openai $ docker agent models --format json | jq ``` -When a models gateway is configured (`--models-gateway`, `DOCKER_AGENT_MODELS_GATEWAY`, or the user config), the command first queries the gateway's `/v1/models` endpoint. A non-empty response is authoritative for the models routed through the gateway: the listing shows the models the gateway serves (`--provider` filters within it), alongside any custom providers you have configured, which serve their models from their own endpoints rather than through the gateway. If the gateway cannot be queried or serves no usable model (endpoint not implemented, empty list, invalid response, timeout, missing authentication), the command falls back to the providers you have configured directly — provider API keys, provider aliases, and custom providers — plus the model catalog; a failure of one source never prevents the others from being listed. The Docker Desktop token is only sent (and required) when the gateway targets a trusted Docker URL. +When a models gateway is configured (`--models-gateway`, `DOCKER_AGENT_MODELS_GATEWAY`, or the user config), the command first queries the gateway's `/v1/models` endpoint. A non-empty response is authoritative for the models routed through the gateway: the listing shows the models the gateway serves (`--provider` filters within it), alongside any custom providers you have configured, which serve their models from their own endpoints rather than through the gateway. If the gateway cannot be queried or serves no usable model (endpoint not implemented, empty list, invalid response, timeout, missing authentication), the command falls back to the providers you have configured directly — provider API keys, provider aliases, and custom providers — plus the model catalog; a failure of one source never prevents the others from being listed. Docker Desktop authentication is required only for HTTPS `docker.com` gateways. An available Docker Desktop token may also be sent to trusted loopback gateways, but is never sent to third-party gateways. ### `docker agent toolsets` @@ -288,7 +288,7 @@ $ docker agent serve api || [flags] | `-s, --session-db ` | `session.db` | Path to the SQLite session database (relative paths resolve against the working directory). | | `--pull-interval `| `0` | Periodically re-pull OCI/URL references and refresh the agent definition. `0` disables auto-pull. | | `--fake ` | (none) | Replay AI responses from a cassette file (for testing). Mutually exclusive with `--record`. | -| `--record ` | (none) | Record AI API interactions to a cassette file. Routes through `--models-gateway` when one is configured. | +| `--record ` | (none) | Record AI API interactions to a cassette file. Routes through `--models-gateway` when one is configured; encrypted agent config and its digest are omitted from cassettes. | | `--mcp-oauth-redirect-uri ` | (none) | OAuth redirect URI for the unmanaged MCP OAuth flow in server mode. When set, the runtime drives PKCE and code exchange in-process and sends the full authorize URL to the client via elicitation. See [Remote MCP](../remote-mcp/index.md) for details. | > **Diagnostics:** Set `CAGENT_PPROF_ADDR=127.0.0.1:6060` (or `--pprof-addr`, a hidden flag) to start a live Go pprof server at `/debug/pprof/`. Use a loopback address; a non-loopback binding logs a security warning. diff --git a/pkg/config/config.go b/pkg/config/config.go index b3d0761108..f85a851011 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -163,7 +163,7 @@ func readInstructionFiles(parentDir string, paths []string) (string, error) { // // This allows exiting early with a proper error message instead of failing later when trying to use a model or tool. func CheckRequiredEnvVars(ctx context.Context, cfg *latest.Config, modelsGateway string, env environment.Provider) error { - if modelsGateway != "" && environment.IsTrustedDockerURL(modelsGateway) { + if modelsGateway != "" && environment.IsDockerDomainURL(modelsGateway) { if jwt, _ := env.Get(ctx, environment.DockerDesktopTokenEnv); jwt == "" { return errors.New("sorry, you first need to sign in Docker Desktop to use the Docker AI Gateway") } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index c1ad29819a..1b8612087a 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -333,7 +333,7 @@ func TestCheckRequiredEnvVarsWithModelGateway(t *testing.T) { require.NoError(t, err) err = CheckRequiredEnvVars(t.Context(), cfg, "http://localhost:8080", &noEnvProvider{}) - require.ErrorContains(t, err, "sign in Docker Desktop") + require.NoError(t, err) }) t.Run("localhost gateway with token", func(t *testing.T) { diff --git a/pkg/environment/docker-desktop.go b/pkg/environment/docker-desktop.go index 5e5fe9489b..7d0510190c 100644 --- a/pkg/environment/docker-desktop.go +++ b/pkg/environment/docker-desktop.go @@ -14,6 +14,17 @@ const ( DockerDesktopTokenEnv = "DOCKER_TOKEN" ) +// IsDockerDomainURL reports whether rawURL targets docker.com or one of its +// subdomains over HTTPS. +func IsDockerDomainURL(rawURL string) bool { + u, err := url.Parse(rawURL) + if err != nil || u.Scheme != "https" { + return false + } + host := strings.ToLower(u.Hostname()) + return host == "docker.com" || strings.HasSuffix(host, ".docker.com") +} + // IsTrustedDockerURL checks if the URL targets a domain trusted to receive // the Docker Desktop JWT. It matches: // - "docker.com" and any subdomain (e.g. "desktop.docker.com") over HTTPS only @@ -32,10 +43,7 @@ func IsTrustedDockerURL(rawURL string) bool { if host == "localhost" || host == "127.0.0.1" || host == "::1" { return true } - if u.Scheme != "https" { - return false - } - return host == "docker.com" || strings.HasSuffix(host, ".docker.com") + return IsDockerDomainURL(rawURL) } type DockerDesktopProvider struct{} diff --git a/pkg/environment/docker_desktop_test.go b/pkg/environment/docker_desktop_test.go index 4a2ad65e71..2569f3d015 100644 --- a/pkg/environment/docker_desktop_test.go +++ b/pkg/environment/docker_desktop_test.go @@ -8,6 +8,32 @@ import ( "github.com/docker/docker-agent/pkg/environment" ) +func TestIsDockerDomainURL(t *testing.T) { + t.Parallel() + + tests := []struct { + url string + want bool + }{ + {"https://docker.com", true}, + {"https://api.docker.com/models", true}, + {"https://DOCKER.COM", true}, + {"http://docker.com", false}, + {"https://docker.com.evil.com", false}, + {"https://notdocker.com", false}, + {"http://localhost:8080", false}, + {"https://127.0.0.1:8080", false}, + {"not-a-url", false}, + } + + for _, tt := range tests { + t.Run(tt.url, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, environment.IsDockerDomainURL(tt.url)) + }) + } +} + func TestIsTrustedDockerURL(t *testing.T) { t.Parallel() diff --git a/pkg/fake/proxy.go b/pkg/fake/proxy.go index 965fe09969..ef60f26ecb 100644 --- a/pkg/fake/proxy.go +++ b/pkg/fake/proxy.go @@ -24,6 +24,7 @@ import ( "gopkg.in/dnaeon/go-vcr.v4/pkg/recorder" "github.com/docker/docker-agent/pkg/environment" + "github.com/docker/docker-agent/pkg/httpclient" ) // ProxyOptions configures the fake proxy behavior. @@ -93,6 +94,16 @@ func StartStreamingRecordingProxy( cassettePath string, upstreamGateway string, headerUpdater func(host string, req *http.Request), +) (string, func() error, error) { + return startStreamingRecordingProxy(ctx, cassettePath, upstreamGateway, headerUpdater, http.DefaultTransport) +} + +func startStreamingRecordingProxy( + ctx context.Context, + cassettePath string, + upstreamGateway string, + headerUpdater func(host string, req *http.Request), + transport http.RoundTripper, ) (string, func() error, error) { // Fail fast on a bad gateway URL instead of returning 500s per request. if upstreamGateway != "" { @@ -106,6 +117,27 @@ func StartStreamingRecordingProxy( return "", nil, fmt.Errorf("failed to create streaming recorder: %w", err) } + streamRec.transport = transport + streamRec.SetCaptureRequest(func(req *http.Request) ([]byte, error) { + recorded := req.Clone(req.Context()) + if req.GetBody != nil { + body, err := req.GetBody() + if err != nil { + return nil, err + } + recorded.Body = body + } + if err := httpclient.RemoveEncryptedConfig(recorded); err != nil { + return nil, err + } + req.Header.Del(httpclient.EncryptedConfigDigestHeader) + if recorded.Body == nil || recorded.Body == http.NoBody { + return nil, nil + } + defer recorded.Body.Close() + return io.ReadAll(recorded.Body) + }) + e := echo.New() e.HideBanner = true e.HidePort = true @@ -448,6 +480,11 @@ func Handle(transport http.RoundTripper, headerUpdater func(host string, req *ht if headerUpdater != nil { headerUpdater(host, req) } + if !environment.IsTrustedDockerURL(options.UpstreamGateway) { + if err := httpclient.RemoveEncryptedConfig(req); err != nil { + return echo.NewHTTPError(http.StatusInternalServerError, "Failed to scrub encrypted agent config") + } + } client := &http.Client{ Timeout: 0, // no timeout, let ctx control it diff --git a/pkg/fake/proxy_gateway_test.go b/pkg/fake/proxy_gateway_test.go index 45681054a7..23b408014a 100644 --- a/pkg/fake/proxy_gateway_test.go +++ b/pkg/fake/proxy_gateway_test.go @@ -11,8 +11,117 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/dnaeon/go-vcr.v4/pkg/cassette" + + "github.com/docker/docker-agent/pkg/httpclient" ) +func TestStartRecordingProxy_EncryptedConfigSecrecy(t *testing.T) { + const ( + encrypted = "ENCRYPTED-AGENT-CONFIG" + digest = "sha256:DIGEST-SECRET" + ) + + tests := []struct { + name string + upstreamTrustURL string + wantUpstreamField bool + }{ + {name: "untrusted upstream", upstreamTrustURL: "https://gateway.example.com"}, + {name: "trusted loopback upstream", upstreamTrustURL: "http://localhost:8080", wantUpstreamField: true}, + {name: "trusted Docker upstream", upstreamTrustURL: "https://models.docker.com", wantUpstreamField: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var upstreamBody []byte + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var err error + upstreamBody, err = io.ReadAll(r.Body) + assert.NoError(t, err) + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer upstream.Close() + + cassettePath := t.TempDir() + "/recording" + transport := hostRewriteRoundTripper{target: upstream.URL} + proxyURL, cleanup, err := startStreamingRecordingProxy(t.Context(), cassettePath, tt.upstreamTrustURL, + gatewayAuthHeaderUpdater(tt.upstreamTrustURL), transport) + require.NoError(t, err) + t.Cleanup(func() { _ = cleanup() }) + + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, proxyURL+"/v1/chat/completions", + strings.NewReader(`{"model":"gpt-4o","encrypted_agent_config":"`+encrypted+`"}`)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Cagent-Forward", "https://api.openai.com/v1") + req.Header.Set(httpclient.EncryptedConfigDigestHeader, digest) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + resp.Body.Close() + require.NoError(t, cleanup()) + + if tt.wantUpstreamField { + assert.Contains(t, string(upstreamBody), encrypted) + } else { + assert.NotContains(t, string(upstreamBody), encrypted) + } + + data, err := os.ReadFile(cassettePath + ".yaml") + require.NoError(t, err) + assert.NotContains(t, string(data), encrypted) + assert.NotContains(t, string(data), digest) + }) + } +} + +type hostRewriteRoundTripper struct { + target string +} + +func (t hostRewriteRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + target, err := http.NewRequestWithContext(req.Context(), req.Method, t.target+req.URL.RequestURI(), req.Body) + if err != nil { + return nil, err + } + target.Header = req.Header.Clone() + return http.DefaultTransport.RoundTrip(target) +} + +func TestStartRecordingProxy_NoUpstreamScrubsEncryptedConfig(t *testing.T) { + const encrypted = "NO-UPSTREAM-SECRET" + + var upstreamBody []byte + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var err error + upstreamBody, err = io.ReadAll(r.Body) + assert.NoError(t, err) + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer upstream.Close() + + cassettePath := t.TempDir() + "/recording" + proxyURL, cleanup, err := StartStreamingRecordingProxy(t.Context(), cassettePath, "", nil) + require.NoError(t, err) + t.Cleanup(func() { _ = cleanup() }) + + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, proxyURL+"/v1/chat/completions", + strings.NewReader(`{"model":"gpt-4o","encrypted_agent_config":"`+encrypted+`"}`)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Cagent-Forward", upstream.URL) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + resp.Body.Close() + require.NoError(t, cleanup()) + + assert.NotContains(t, string(upstreamBody), encrypted) + data, err := os.ReadFile(cassettePath + ".yaml") + require.NoError(t, err) + assert.NotContains(t, string(data), encrypted) +} + func TestGatewayTargetURL(t *testing.T) { t.Parallel() diff --git a/pkg/fake/streaming_recorder.go b/pkg/fake/streaming_recorder.go index b556b9fe9b..be7b35260d 100644 --- a/pkg/fake/streaming_recorder.go +++ b/pkg/fake/streaming_recorder.go @@ -25,11 +25,14 @@ func WithRecordURL(ctx context.Context, url string) context.Context { // Unlike the standard VCR recorder which buffers entire responses, // this recorder tees the response body so it can be streamed to the client // while simultaneously being captured for recording. +type captureRequestFunc func(*http.Request) ([]byte, error) + type StreamingRecorder struct { - transport http.RoundTripper - cassette *cassette.Cassette - cassettePath string - mu sync.Mutex + transport http.RoundTripper + cassette *cassette.Cassette + cassettePath string + captureRequest captureRequestFunc + mu sync.Mutex } // NewStreamingRecorder creates a new streaming recorder that will save @@ -46,11 +49,17 @@ func NewStreamingRecorder(cassettePath string) (*StreamingRecorder, error) { }, nil } +// SetCaptureRequest sets an optional request-body sanitizer for cassette +// capture. The forwarded request is never modified. +func (r *StreamingRecorder) SetCaptureRequest(capture captureRequestFunc) { + r.captureRequest = capture +} + // RoundTrip implements http.RoundTripper. It makes the actual HTTP request, // tees the response body for recording, and returns immediately so the // response can be streamed to the client. func (r *StreamingRecorder) RoundTrip(req *http.Request) (*http.Response, error) { - // Read and buffer the request body for recording + // Buffer the forwarded body, then independently derive the cassette body. var reqBody []byte if req.Body != nil && req.Body != http.NoBody { var err error @@ -60,14 +69,26 @@ func (r *StreamingRecorder) RoundTrip(req *http.Request) (*http.Response, error) } req.Body.Close() req.Body = io.NopCloser(bytes.NewReader(reqBody)) + req.GetBody = func() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader(reqBody)), nil + } } - // Make the actual HTTP request + // Make the actual HTTP request before sanitizing the independent capture. resp, err := r.transport.RoundTrip(req) if err != nil { return nil, err } + recordedBody := reqBody + if r.captureRequest != nil { + recordedBody, err = r.captureRequest(req) + if err != nil { + resp.Body.Close() + return nil, err + } + } + // Create a buffer to capture the response body var respBodyBuf bytes.Buffer @@ -78,7 +99,7 @@ func (r *StreamingRecorder) RoundTrip(req *http.Request) (*http.Response, error) origBody: resp.Body, recorder: r, req: req, - reqBody: reqBody, + reqBody: recordedBody, resp: resp, respBuf: &respBodyBuf, } diff --git a/pkg/httpclient/client.go b/pkg/httpclient/client.go index d4b1233173..96060b4767 100644 --- a/pkg/httpclient/client.go +++ b/pkg/httpclient/client.go @@ -328,10 +328,10 @@ func (u *userAgentTransport) RoundTrip(req *http.Request) (*http.Response, error // can replay the request. Callers must pass a request clone; the body reader is // consumed. func injectEncryptedConfigBody(req *http.Request, enc string) error { - if req.Body == nil { + if ct := req.Header.Get("Content-Type"); !strings.HasPrefix(strings.ToLower(ct), "application/json") { return nil } - if ct := req.Header.Get("Content-Type"); !strings.HasPrefix(strings.ToLower(ct), "application/json") { + if req.Body == nil { return nil } @@ -359,6 +359,44 @@ func injectEncryptedConfigBody(req *http.Request, enc string) error { return nil } +// RemoveEncryptedConfig removes encrypted agent configuration from a JSON +// request body and deletes its digest header. Bodies without the field remain +// byte-for-byte unchanged. +func RemoveEncryptedConfig(req *http.Request) error { + req.Header.Del(EncryptedConfigDigestHeader) + if req.Body == nil { + return nil + } + if ct := req.Header.Get("Content-Type"); !strings.HasPrefix(strings.ToLower(ct), "application/json") { + return nil + } + + raw, err := io.ReadAll(req.Body) + _ = req.Body.Close() + if err != nil { + return fmt.Errorf("read request body: %w", err) + } + + var payload map[string]json.RawMessage + if err := json.Unmarshal(raw, &payload); err != nil { + resetBody(req, raw) + return nil + } + if _, ok := payload[EncryptedConfigBodyField]; !ok { + resetBody(req, raw) + return nil + } + delete(payload, EncryptedConfigBodyField) + + rewritten, err := json.Marshal(payload) + if err != nil { + resetBody(req, raw) + return fmt.Errorf("encode JSON body: %w", err) + } + resetBody(req, rewritten) + return nil +} + // resetBody points req at a fresh, replayable body backed by b. func resetBody(req *http.Request, b []byte) { req.Body = io.NopCloser(bytes.NewReader(b)) diff --git a/pkg/httpclient/client_test.go b/pkg/httpclient/client_test.go index 0ce49ad090..c756bc65ad 100644 --- a/pkg/httpclient/client_test.go +++ b/pkg/httpclient/client_test.go @@ -266,6 +266,38 @@ func TestEncryptedConfigBodyInjection(t *testing.T) { } } +func TestRemoveEncryptedConfig(t *testing.T) { + t.Parallel() + + t.Run("removes field and digest", func(t *testing.T) { + t.Parallel() + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, "https://example.com", strings.NewReader(`{"model":"gpt-4o","encrypted_agent_config":"secret"}`)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set(EncryptedConfigDigestHeader, "sha256:secret") + + require.NoError(t, RemoveEncryptedConfig(req)) + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + assert.JSONEq(t, `{"model":"gpt-4o"}`, string(body)) + assert.Empty(t, req.Header.Get(EncryptedConfigDigestHeader)) + }) + + t.Run("leaves non-JSON body unchanged", func(t *testing.T) { + t.Parallel() + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, "https://example.com", strings.NewReader("opaque")) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/octet-stream") + req.Header.Set(EncryptedConfigDigestHeader, "sha256:secret") + + require.NoError(t, RemoveEncryptedConfig(req)) + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + assert.Equal(t, "opaque", string(body)) + assert.Empty(t, req.Header.Get(EncryptedConfigDigestHeader)) + }) +} + func TestContextWithSessionID_RoundTrip(t *testing.T) { t.Parallel() diff --git a/pkg/model/provider/base/gateway.go b/pkg/model/provider/base/gateway.go index 9bf666f768..ccf9e9df18 100644 --- a/pkg/model/provider/base/gateway.go +++ b/pkg/model/provider/base/gateway.go @@ -14,12 +14,12 @@ import ( "github.com/docker/docker-agent/pkg/model/provider/options" ) -// VerifyDockerGatewayAuth fails fast when gateway targets a trusted Docker -// domain but Docker Desktop's auth token is unavailable. Provider clients -// call it at construction time so a missing sign-in surfaces before the -// first request. Non-Docker gateways need no Desktop token and always pass. +// VerifyDockerGatewayAuth fails fast when gateway targets a Docker domain but +// Docker Desktop's auth token is unavailable. Provider clients call it at +// construction time so a missing sign-in surfaces before the first request. +// Loopback and non-Docker gateways need no Desktop token and always pass. func VerifyDockerGatewayAuth(ctx context.Context, env environment.Provider, gateway string) error { - if !environment.IsTrustedDockerURL(gateway) { + if !environment.IsDockerDomainURL(gateway) { return nil } if token, _ := env.Get(ctx, environment.DockerDesktopTokenEnv); token == "" { @@ -28,15 +28,15 @@ func VerifyDockerGatewayAuth(ctx context.Context, env environment.Provider, gate return nil } -// GatewayAuthToken returns a fresh Docker Desktop auth token when gateway -// targets a trusted Docker domain, or "" for other gateways. Gateway clients -// call it on every request because Desktop tokens are short-lived. +// GatewayAuthToken returns a fresh Docker Desktop auth token for trusted +// gateways. Docker domains require a token; loopback gateways may proceed +// without one. Other gateways receive no Docker token. func GatewayAuthToken(ctx context.Context, env environment.Provider, gateway string) (string, error) { if !environment.IsTrustedDockerURL(gateway) { return "", nil } token, _ := env.Get(ctx, environment.DockerDesktopTokenEnv) - if token == "" { + if token == "" && environment.IsDockerDomainURL(gateway) { return "", errors.New(NoDesktopTokenErrorMessage) } return token, nil diff --git a/pkg/model/provider/base/gateway_test.go b/pkg/model/provider/base/gateway_test.go index 68a06d9be6..a2a821f6ce 100644 --- a/pkg/model/provider/base/gateway_test.go +++ b/pkg/model/provider/base/gateway_test.go @@ -30,6 +30,11 @@ func TestVerifyDockerGatewayAuth(t *testing.T) { assert.NoError(t, VerifyDockerGatewayAuth(t.Context(), fakeEnv{}, "https://gateway.example.com")) }) + t.Run("loopback gateway needs no token", func(t *testing.T) { + t.Parallel() + assert.NoError(t, VerifyDockerGatewayAuth(t.Context(), fakeEnv{}, "http://localhost:8080")) + }) + t.Run("trusted docker gateway with token", func(t *testing.T) { t.Parallel() env := fakeEnv{environment.DockerDesktopTokenEnv: "jwt"} @@ -53,6 +58,13 @@ func TestGatewayAuthToken(t *testing.T) { assert.Empty(t, token) }) + t.Run("loopback gateway returns empty token", func(t *testing.T) { + t.Parallel() + token, err := GatewayAuthToken(t.Context(), fakeEnv{}, "http://127.0.0.1:8080") + require.NoError(t, err) + assert.Empty(t, token) + }) + t.Run("trusted docker gateway returns fresh token", func(t *testing.T) { t.Parallel() env := fakeEnv{environment.DockerDesktopTokenEnv: "jwt"} diff --git a/pkg/modelsgateway/discovery_test.go b/pkg/modelsgateway/discovery_test.go index c191d35097..2e30f48bc3 100644 --- a/pkg/modelsgateway/discovery_test.go +++ b/pkg/modelsgateway/discovery_test.go @@ -13,9 +13,8 @@ import ( "github.com/docker/docker-agent/pkg/environment" ) -// Note: httptest servers listen on 127.0.0.1, which IsTrustedDockerURL -// treats as trusted, so every test against them exercises the Docker -// token auth path. +// httptest servers use loopback URLs, so they exercise the no-Docker-token +// path unless a test supplies a Docker hostname with a rewriting transport. func tokenEnv() environment.Provider { return environment.NewMapEnvProvider(map[string]string{ @@ -107,15 +106,20 @@ func TestListModels_InvalidJSON(t *testing.T) { func TestListModels_MissingDockerToken(t *testing.T) { t.Parallel() - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + var gotAuth string + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") _, _ = w.Write([]byte(`{"data":[{"id":"gpt-4o"}]}`)) })) defer server.Close() - _, err := ListModels(t.Context(), server.URL, environment.NewMapEnvProvider(nil)) + client := server.Client() + client.Transport = hostRewriteTransport{host: server.Listener.Addr().String(), base: client.Transport} + _, err := listModelsWith(t.Context(), "https://models.docker.com", environment.NewMapEnvProvider(nil), client) require.Error(t, err) assert.Contains(t, err.Error(), "Docker Desktop") + assert.Empty(t, gotAuth) } func TestListModels_UnreachableGateway(t *testing.T) { @@ -137,14 +141,22 @@ func TestListModels_InvalidURL(t *testing.T) { // hostRewriteTransport redirects every request to a local test server, // letting tests exercise non-trusted hostnames without touching the network. type hostRewriteTransport struct { - host string + host string + base http.RoundTripper + scheme string } func (t hostRewriteTransport) RoundTrip(req *http.Request) (*http.Response, error) { r2 := req.Clone(req.Context()) - r2.URL.Scheme = "http" + if t.scheme != "" { + r2.URL.Scheme = t.scheme + } r2.URL.Host = t.host - return http.DefaultTransport.RoundTrip(r2) + base := t.base + if base == nil { + base = http.DefaultTransport + } + return base.RoundTrip(r2) } func TestListModels_GenericGatewayNeedsNoDockerToken(t *testing.T) { @@ -159,7 +171,7 @@ func TestListModels_GenericGatewayNeedsNoDockerToken(t *testing.T) { // models.example.com is not a trusted Docker URL, so discovery must // neither require the Docker token nor send any Authorization header. - client := &http.Client{Transport: hostRewriteTransport{host: server.Listener.Addr().String()}} + client := &http.Client{Transport: hostRewriteTransport{host: server.Listener.Addr().String(), scheme: "http"}} ids, err := listModelsWith(t.Context(), "https://models.example.com", environment.NewMapEnvProvider(nil), client) require.NoError(t, err)