Skip to content

Commit 960305e

Browse files
julienldclaude
andcommitted
test(internal): address Gemini review feedback on host detection and port allocation
- Replace subprocess.run(["docker", "run", ...]) with docker SDK containers.run() in _detect_docker_host() — avoids shell dependency - Eliminate TOCTOU port race: bind HTTPServer directly to port 0 and read srv.server_address[1] instead of pre-allocating with a socket - Remove now-unused socket import Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 39417ff commit 960305e

1 file changed

Lines changed: 10 additions & 18 deletions

File tree

tests/src/e2e/conftest.py

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import logging
2323
import os
2424
import shutil
25-
import socket
2625
import sys
2726
import tempfile
2827
import threading
@@ -205,20 +204,16 @@ def _detect_docker_host() -> dict:
205204
- ``hostname`` – hostname that Docker containers use to reach the host
206205
- ``extra_hosts`` – dict passed to ``container.with_kwargs`` (may be empty)
207206
"""
208-
import subprocess
209-
210207
try:
211-
result = subprocess.run(
212-
[
213-
"docker", "run", "--rm", "alpine",
214-
"sh", "-c",
215-
"getent hosts host.docker.internal 2>/dev/null | awk '{print $1}'",
216-
],
217-
capture_output=True,
218-
text=True,
219-
timeout=30,
208+
import docker as docker_sdk
209+
210+
client = docker_sdk.from_env()
211+
output = client.containers.run(
212+
"alpine",
213+
["sh", "-c", "getent hosts host.docker.internal 2>/dev/null | awk '{print $1}'"],
214+
remove=True,
220215
)
221-
if result.stdout.strip():
216+
if output.strip():
222217
# Docker Desktop DNS resolved the name — use hostname, no override needed
223218
logger.info("🔍 Docker Desktop DNS detected — using host.docker.internal as-is")
224219
return {"hostname": "host.docker.internal", "extra_hosts": {}}
@@ -242,16 +237,13 @@ def _blueprint_http_server():
242237
"""
243238
env = _detect_docker_host()
244239

245-
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
246-
s.bind(("", 0))
247-
port = s.getsockname()[1]
248-
249240
assets_dir = Path(__file__).parent.parent.parent / "assets" / "blueprints"
250241
assets_dir.mkdir(parents=True, exist_ok=True)
251242

252243
handler = partial(http.server.SimpleHTTPRequestHandler, directory=str(assets_dir))
253244
handler.log_message = lambda *args: None # type: ignore[method-assign]
254-
srv = http.server.HTTPServer(("0.0.0.0", port), handler)
245+
srv = http.server.HTTPServer(("0.0.0.0", 0), handler)
246+
port = srv.server_address[1]
255247
t = threading.Thread(target=srv.serve_forever, daemon=True)
256248
t.start()
257249

0 commit comments

Comments
 (0)