Skip to content

Add Docker sandbox host mappings#488

Open
Dawn-Fighter wants to merge 2 commits into
usestrix:mainfrom
Dawn-Fighter:fix-docker-sandbox-extra-hosts
Open

Add Docker sandbox host mappings#488
Dawn-Fighter wants to merge 2 commits into
usestrix:mainfrom
Dawn-Fighter:fix-docker-sandbox-extra-hosts

Conversation

@Dawn-Fighter
Copy link
Copy Markdown

Summary

  • Adds STRIX_SANDBOX_EXTRA_HOSTS for Docker sandbox /etc/hosts mappings.
  • Preserves the existing host.docker.internal -> host-gateway mapping.
  • Adds runtime tests and README documentation for local/internal target domains.

Why

Fixes #481 by allowing users to resolve local or internal hostnames from inside the Docker sandbox without rebuilding the sandbox image. For example:

export STRIX_SANDBOX_EXTRA_HOSTS="test.internal.lan=host-gateway"

Validation

  • docker run --rm hello-world
  • docker run --rm --add-host test.internal.lan:host-gateway alpine getent hosts test.internal.lan
  • uv run pytest tests/runtime/test_docker_runtime.py
  • uv run ruff format --check strix/config/config.py strix/runtime/docker_runtime.py tests/runtime/test_docker_runtime.py
  • uv run ruff check strix/config/config.py strix/runtime/docker_runtime.py tests/runtime/test_docker_runtime.py

Note: I also ran make check-all; it currently fails on existing repo-wide lint issues outside this PR's touched files.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 14, 2026

Greptile Summary

This PR introduces STRIX_SANDBOX_EXTRA_HOSTS, a new environment variable that lets users inject custom /etc/hosts mappings into the Docker sandbox without rebuilding the image, while always preserving the existing host.docker.internal → host-gateway entry.

  • _get_extra_hosts() parses the comma-separated hostname=address entries and merges them with the default gateway mapping before passing them to containers.run.
  • Several lazy local imports (tarfile, BytesIO, subprocess, urlparse, get_global_tracer) are hoisted to the top of the module, removing the defensive ImportError guard that the original _get_scan_id relied on.
  • Four unit tests cover the default, merge, invalid-format, and integration cases for the new feature.

Confidence Score: 3/5

The 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 get_global_tracer to a top-level import silently invalidates the except (ImportError, AttributeError) guard in _get_scan_id - any import failure now breaks the entire docker_runtime module rather than falling back gracefully. Separately, a malformed STRIX_SANDBOX_EXTRA_HOSTS raises ValueError inside the container creation retry loop where only Docker-specific exceptions are caught, so the error bypasses retry handling and surfaces to callers who expect SandboxInitializationError.

strix/runtime/docker_runtime.py - the import promotion and error propagation path need a second look before merging.

Important Files Changed

Filename Overview
strix/runtime/docker_runtime.py Adds _get_extra_hosts() for user-configurable /etc/hosts entries and moves several lazy imports to the top level; the promotion of get_global_tracer removes the existing ImportError guard, and ValueError from invalid config propagates uncaught through the container creation retry loop.
strix/config/config.py Adds strix_sandbox_extra_hosts = None as a new nullable config field; straightforward and consistent with existing config patterns.
tests/runtime/test_docker_runtime.py New test file covering the four main cases for _get_extra_hosts and _create_container host injection; tests use DockerRuntime.__new__ to avoid __init__ side effects, which is appropriate.
README.md Adds one-line documentation for STRIX_SANDBOX_EXTRA_HOSTS in the env-var export block; accurate and consistent with the existing examples.
Prompt To Fix All With AI
Fix 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

Comment thread strix/runtime/docker_runtime.py Outdated
Comment thread strix/runtime/docker_runtime.py
Comment thread strix/runtime/docker_runtime.py Outdated
@Dawn-Fighter
Copy link
Copy Markdown
Author

i have commit the changes suggested by the greptile .. please review the code and issue .. comment if any changes are needed .. thank you

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.

Failed to query DNS

1 participant