Add Docker sandbox host mappings#488
Conversation
Greptile SummaryThis PR introduces
Confidence Score: 3/5The core host-mapping feature is straightforward, but the import refactor changes a previously guarded failure into a hard module-load failure, and invalid config surfaces as a raw ValueError rather than the expected SandboxInitializationError. Hoisting strix/runtime/docker_runtime.py - the import promotion and error propagation path need a second look before merging. Important Files Changed
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
strix/runtime/docker_runtime.py:21
**Top-level import removes the `ImportError` safety net**
The original code imported `get_global_tracer` inside `_get_scan_id` specifically so that the surrounding `except (ImportError, AttributeError)` could catch a missing or broken tracer module and fall back gracefully. Promoting it to a module-level import means that if `strix.telemetry.tracer` fails to import in any deployment environment, the entire `docker_runtime` module fails to load — not just `_get_scan_id`. The `except (ImportError, AttributeError)` on line 58 is now dead code for the `ImportError` case, because any `ImportError` would surface at module load time before the function is ever called.
### Issue 2 of 3
strix/runtime/docker_runtime.py:114-138
**`ValueError` from `_get_extra_hosts` bypasses retry logic and surfaces as an unhandled exception**
`_get_extra_hosts()` is called inside the retry loop of `_create_container`, but the loop's exception handler only catches `DockerException`, `RequestsConnectionError`, and `RequestsTimeout`. A `ValueError` raised here propagates uncaught through `_create_container`, `_get_or_create_container`, and `create_sandbox`. Callers of `create_sandbox` receive a raw `ValueError` rather than a `SandboxInitializationError`, breaking the expected error contract for callers who catch only that type.
### Issue 3 of 3
strix/runtime/docker_runtime.py:130-131
Entries with multiple `=` signs — e.g. `foo==bar` — will parse with `hostname="foo"` and `address="=bar"` (the leading `=` is preserved), which passes the non-empty check but produces an invalid address string that Docker will reject at container creation time with an opaque error. Validating that exactly two parts result would surface this earlier with a clearer message.
```suggestion
parts = [part.strip() for part in host_entry.split("=")]
if len(parts) != 2:
raise ValueError(
"STRIX_SANDBOX_EXTRA_HOSTS entries must use hostname=address format"
)
hostname, address = parts
if not hostname or not address:
```
Reviews (1): Last reviewed commit: "Add Docker sandbox host mappings" | Re-trigger Greptile |
|
i have commit the changes suggested by the greptile .. please review the code and issue .. comment if any changes are needed .. thank you |
Summary
STRIX_SANDBOX_EXTRA_HOSTSfor Docker sandbox/etc/hostsmappings.host.docker.internal -> host-gatewaymapping.Why
Fixes #481 by allowing users to resolve local or internal hostnames from inside the Docker sandbox without rebuilding the sandbox image. For example:
Validation
docker run --rm hello-worlddocker run --rm --add-host test.internal.lan:host-gateway alpine getent hosts test.internal.lanuv run pytest tests/runtime/test_docker_runtime.pyuv run ruff format --check strix/config/config.py strix/runtime/docker_runtime.py tests/runtime/test_docker_runtime.pyuv run ruff check strix/config/config.py strix/runtime/docker_runtime.py tests/runtime/test_docker_runtime.pyNote: I also ran
make check-all; it currently fails on existing repo-wide lint issues outside this PR's touched files.