Add tests for --allow-docker-gateway behavior

pkg/container/docker/client_deploy_test.go

@@ -312,6 +312,40 @@ func TestDeployWorkload_AllowDockerGateway_ForwardedToEgress(t *testing.T) {
 	assert.True(t, fops.egressAllowDockerGW, "AllowDockerGateway must be forwarded to createEgressSquidContainer")
 }
 
+// TestDeployWorkload_AllowDockerGateway_DefaultsToNotForwarded guards against a
+// default flip: when the caller does not opt in, the egress proxy must keep its
+// Docker-gateway deny rules. It also confirms the DNS container is spawned on the
+// isolation path (the MCP server's resolver).
+func TestDeployWorkload_AllowDockerGateway_DefaultsToNotForwarded(t *testing.T) {
+	t.Parallel()
+
+	fops := &fakeDeployOps{dnsIP: "172.18.0.10"}
+	c := newClientWithOps(fops)
+
+	opts := runtime.NewDeployWorkloadOptions()
+	opts.AttachStdio = true
+	// AllowDockerGateway intentionally left at its zero value (false).
+
+	_, err := c.DeployWorkload(
+		t.Context(),
+		"ghcr.io/example/mcp:latest",
+		"app",
+		[]string{"serve"},
+		map[string]string{},
+		map[string]string{},
+		&permissions.Profile{},
+		"stdio",
+		opts,
+		true, // isolateNetwork required for egress container to be created
+	)
+	require.NoError(t, err)
+
+	require.True(t, fops.egressCalled, "egress container must be created when isolateNetwork=true")
+	assert.False(t, fops.egressAllowDockerGW,
+		"AllowDockerGateway must default to false so the gateway deny rules stay in place")
+	assert.True(t, fops.dnsCalled, "DNS container must be created on the isolation path")
+}
+
 func TestDeployWorkload_UnsupportedTransport_PropagatesError(t *testing.T) {
 	t.Parallel()
 

pkg/container/docker/squid_test.go

@@ -358,6 +358,45 @@ func TestCreateTempEgressSquidConf_DockerGatewayBlocking(t *testing.T) {
 				"http_access allow allowed_ports allowed_dsts",
 			},
 		},
+		{
+			// Listing host.docker.internal in allow_host is NOT sufficient on its
+			// own: without the opt-in the gateway deny is still written, and
+			// because Squid is first-match-wins the deny (asserted to precede the
+			// allow below) blocks the request before the allowed_dsts allow is
+			// reached. Reaching the gateway requires BOTH the flag and the host.
+			name: "host.docker.internal in allow_host without opt-in is still blocked",
+			permissions: &permissions.NetworkPermissions{
+				Outbound: &permissions.OutboundNetworkPermissions{
+					AllowHost: []string{"host.docker.internal"},
+					AllowPort: []int{8080},
+				},
+			},
+			allowDockerGateway: false,
+			expectDenyRule:     true,
+			expectAllowAll:     false,
+			expectContains: []string{
+				"acl allowed_dsts dstdomain host.docker.internal",
+				"http_access allow allowed_ports allowed_dsts",
+			},
+		},
+		{
+			// With the opt-in the deny is dropped and the ACL allow for
+			// host.docker.internal takes effect.
+			name: "host.docker.internal in allow_host with opt-in is allowed via ACL",
+			permissions: &permissions.NetworkPermissions{
+				Outbound: &permissions.OutboundNetworkPermissions{
+					AllowHost: []string{"host.docker.internal"},
+					AllowPort: []int{8080},
+				},
+			},
+			allowDockerGateway: true,
+			expectDenyRule:     false,
+			expectAllowAll:     false,
+			expectContains: []string{
+				"acl allowed_dsts dstdomain host.docker.internal",
+				"http_access allow allowed_ports allowed_dsts",
+			},
+		},
 	}
 
 	for _, tt := range tests {

test/e2e/network_isolation_test.go

@@ -6,11 +6,16 @@ package e2e_test
 import (
 	"context"
 	"fmt"
+	"io"
+	"net"
 	"net/http"
 	"os"
+	"os/exec"
 	"path/filepath"
+	"strings"
 	"time"
 
+	"github.com/mark3labs/mcp-go/mcp"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
@@ -141,4 +146,166 @@ var _ = Describe("NetworkIsolation", Label("proxy", "network", "isolation", "e2e
 			verifyNetworkRestrictions("default")
 		})
 	})
+
+	Describe("Reaching the host with --allow-docker-gateway", func() {
+		// startFetchServer runs the fetch MCP server under network isolation with
+		// the given extra run args (e.g. --allow-docker-gateway), waits for it to
+		// be running, and returns its workload name. An empty profileJSON uses the
+		// default (allow-all) network profile.
+		startFetchServer := func(nameSuffix, profileJSON string, extraRunArgs ...string) string {
+			serverName := fmt.Sprintf("ni-gw-%s-%d", nameSuffix, GinkgoRandomSeed())
+			DeferCleanup(func() {
+				if config.CleanupAfter {
+					// Best-effort: a test may have already removed this server to
+					// avoid running two isolation stacks at once.
+					_ = e2e.StopAndRemoveMCPServer(config, serverName)
+				}
+			})
+
+			runArgs := append([]string{"run", "--name", serverName}, extraRunArgs...)
+			if profileJSON != "" {
+				profilePath := filepath.Join(permissionProfileDir, nameSuffix+".json")
+				err := os.WriteFile(profilePath, []byte(profileJSON), 0644)
+				Expect(err).ToNot(HaveOccurred(), "Should be able to write permission profile")
+				runArgs = append(runArgs, "--permission-profile", profilePath)
+			}
+			runArgs = append(runArgs, "fetch")
+
+			e2e.NewTHVCommand(config, runArgs...).ExpectSuccess()
+			// The fetch server can take ~70s to become healthy; allow generous
+			// headroom so the test does not flake.
+			err := e2e.WaitForMCPServer(config, serverName, 120*time.Second)
+			Expect(err).ToNot(HaveOccurred(), "Server should be running within 120 seconds")
+			return serverName
+		}
+
+		// retireServer tears a server down mid-test so only one network-isolation
+		// stack (MCP + dns + egress + ingress containers) runs at a time, avoiding
+		// resource contention that slows the next server's startup.
+		retireServer := func(serverName string) {
+			if config.CleanupAfter {
+				Expect(e2e.StopAndRemoveMCPServer(config, serverName)).To(Succeed())
+			}
+		}
+
+		// fetchThrough drives the fetch tool against the given server and returns
+		// the tool result. A denied request comes back as an error result
+		// (result.IsError); a successful one carries the fetched body.
+		fetchThrough := func(serverName, targetURL string) *mcp.CallToolResult {
+			serverURL, err := e2e.GetMCPServerURL(config, serverName)
+			Expect(err).ToNot(HaveOccurred(), "Should be able to get server URL")
+			err = e2e.WaitForMCPServerReady(config, serverURL, "streamable-http", 60*time.Second)
+			Expect(err).ToNot(HaveOccurred(), "Server should be ready")
+
+			mcpClient, err := e2e.NewMCPClientForStreamableHTTP(config, serverURL)
+			Expect(err).ToNot(HaveOccurred(), "Should be able to create MCP client")
+			defer mcpClient.Close()
+
+			ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+			defer cancel()
+			Expect(mcpClient.Initialize(ctx)).To(Succeed(), "Should be able to initialize MCP client")
+
+			result, err := mcpClient.CallTool(ctx, "fetch", map[string]interface{}{"url": targetURL})
+			Expect(err).ToNot(HaveOccurred(), "CallTool should complete without transport error")
+			return result
+		}
+
+		// resultText concatenates the text content of a tool result.
+		resultText := func(result *mcp.CallToolResult) string {
+			var sb strings.Builder
+			for _, c := range result.Content {
+				if tc, ok := mcp.AsTextContent(c); ok {
+					sb.WriteString(tc.Text)
+				}
+			}
+			return sb.String()
+		}
+
+		// dockerBridgeGatewayIP returns the host gateway IP of the default Docker
+		// bridge — the same address the egress proxy denies by default, and the
+		// address a container uses to reach the host on Linux.
+		dockerBridgeGatewayIP := func() string {
+			//nolint:gosec // fixed, test-controlled arguments
+			out, err := exec.Command("docker", "network", "inspect", "bridge",
+				"-f", "{{range .IPAM.Config}}{{.Gateway}}{{end}}").Output()
+			Expect(err).ToNot(HaveOccurred(), "Should be able to inspect the docker bridge network")
+			return strings.TrimSpace(string(out))
+		}
+
+		It("denies the bridge gateway IP by default and, where routable, reaches it with the flag", func() {
+			By("Starting a host service reachable from containers")
+			listener, err := net.Listen("tcp", ":0") //nolint:gosec // binds an ephemeral port for the test
+			Expect(err).ToNot(HaveOccurred(), "Should be able to listen on an ephemeral port")
+			port := listener.Addr().(*net.TCPAddr).Port
+			srv := &http.Server{
+				Handler:           http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = io.WriteString(w, "host-service-ok") }),
+				ReadHeaderTimeout: 5 * time.Second,
+			}
+			go func() { _ = srv.Serve(listener) }()
+			DeferCleanup(func() { _ = srv.Close() })
+
+			// Target the bridge gateway IP, not host.docker.internal: an IP needs no
+			// DNS resolution, so a blocked fetch unambiguously means the egress
+			// `docker_gateway_ip` deny fired (see #5640 — the isolated resolver
+			// cannot resolve host.docker.internal, which would confound a
+			// hostname-based assertion). The hostname deny rule is pinned by the
+			// config test below instead.
+			gatewayIP := dockerBridgeGatewayIP()
+			target := fmt.Sprintf("http://%s:%d/", gatewayIP, port)
+
+			By("Confirming the gateway IP is blocked by the egress proxy by default")
+			denyServer := startFetchServer("deny", "")
+			Expect(fetchThrough(denyServer, target).IsError).To(BeTrue(),
+				"the egress proxy must deny the bridge gateway IP without --allow-docker-gateway")
+			retireServer(denyServer)
+
+			By("Confirming --allow-docker-gateway removes the deny so the host is reachable")
+			allowServer := startFetchServer("allow", "", "--allow-docker-gateway")
+			result := fetchThrough(allowServer, target)
+			if result.IsError {
+				// The deny removal is pinned deterministically by the config test
+				// below; whether the bridge gateway actually routes to a host
+				// service is environment-specific — it works on Linux Docker Engine
+				// (where the bridge gateway is the host), but on Docker Desktop the
+				// host lives behind host.docker.internal, not the bridge gateway. Skip
+				// the positive reachability leg where the gateway is not host-routable.
+				Skip("docker bridge gateway is not routable to the host in this environment (e.g. Docker Desktop)")
+			}
+			Expect(resultText(result)).To(ContainSubstring("host-service-ok"),
+				"with --allow-docker-gateway the fetch must reach the host service through the egress proxy")
+		})
+
+		// This test pins the egress ACL rules deterministically on a real, deployed
+		// container — both the hostname deny (which the traffic test above cannot
+		// exercise without the host.docker.internal DNS confounder of #5640) and the
+		// direct-IP deny. It complements, rather than duplicates, the unit test for
+		// createTempEgressSquidConf: it proves thv actually generates and mounts the
+		// config into the running egress proxy, alongside the real-traffic assertion above.
+		It("carries both gateway deny rules in the egress config by default and drops them with the flag", func() {
+			squidConf := func(serverName string) string {
+				//nolint:gosec // container name is test-controlled
+				out, err := exec.Command("docker", "exec", serverName+"-egress",
+					"cat", "/etc/squid/squid.conf").CombinedOutput()
+				Expect(err).ToNot(HaveOccurred(), "Should be able to read egress squid.conf: %s", string(out))
+				return string(out)
+			}
+
+			By("Default profile: both the hostname and direct-IP deny rules are present")
+			denyServer := startFetchServer("cfg-deny", "")
+			denyConf := squidConf(denyServer)
+			Expect(denyConf).To(ContainSubstring("http_access deny docker_gateway_hosts"),
+				"default config must deny the docker gateway hostnames")
+			Expect(denyConf).To(ContainSubstring("dstdomain host.docker.internal gateway.docker.internal"))
+			Expect(denyConf).To(ContainSubstring("http_access deny docker_gateway_ip"),
+				"default config must deny the docker gateway IP (the DNS-bypass path)")
+			retireServer(denyServer)
+
+			By("With --allow-docker-gateway: both deny rules are absent")
+			allowConf := squidConf(startFetchServer("cfg-allow", "", "--allow-docker-gateway"))
+			Expect(allowConf).ToNot(ContainSubstring("docker_gateway_hosts"),
+				"--allow-docker-gateway must remove the gateway hostname deny rule")
+			Expect(allowConf).ToNot(ContainSubstring("docker_gateway_ip"),
+				"--allow-docker-gateway must remove the gateway IP deny rule")
+		})
+	})
 })