Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions strix/runtime/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,17 @@ async def _docker_backend(
backend don't need the docker-py library installed.

``session.start()`` is what materializes the manifest entries
(LocalDir copies and manifest-declared volume/FUSE mounts) into the
running container — the SDK's ``client.create()`` only builds the inner
session object without applying the manifest. ``async with session:``
would call it too, but Strix manages session lifetime explicitly via
``client.delete()`` so we trigger ``start()`` ourselves.
(manifest-declared volume/FUSE mounts) into the running container — the
SDK's ``client.create()`` only builds the inner session object without
applying the manifest. ``async with session:`` would call it too, but
Strix manages session lifetime explicitly via ``client.delete()`` so we
trigger ``start()`` ourselves. Local source trees are copied in separately
after ``start()`` via a single tar ``put_archive`` (see
``session_manager._import_local_sources``), not through the manifest.

``bind_mounts`` are host directories (e.g. large repos passed via
``--mount``) bind-mounted read-only; unlike manifest entries they are
applied by Docker at container-create time, not by ``start()``.
``--mount``) bind-mounted read-only; unlike copied sources they are
applied by Docker at container-create time, not after ``start()``.
"""
import docker
from agents.sandbox.sandboxes.docker import DockerSandboxClientOptions
Expand Down
2 changes: 1 addition & 1 deletion strix/runtime/docker_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ async def _create_container(
extra_hosts["host.docker.internal"] = "host-gateway"

# Strix injection: host bind mounts (e.g. large repos passed via --mount)
# that bypass the SDK's file-by-file LocalDir copy.
# that bypass the in-container source copy entirely.
bind_mounts = getattr(self, "strix_bind_mounts", ())
if bind_mounts:
mounts = create_kwargs.setdefault("mounts", [])
Expand Down
187 changes: 171 additions & 16 deletions strix/runtime/session_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

from __future__ import annotations

import asyncio
import io
import logging
import os
import tarfile
from pathlib import Path
from typing import Any

from agents.sandbox.entries import BaseEntry, LocalDir
from agents.sandbox.manifest import Environment, Manifest

from strix.config import load_settings
Expand All @@ -23,27 +26,46 @@

_SESSION_CACHE: dict[str, dict[str, Any]] = {}

# Manifest root inside the container; entry keys hang off this path.
# Workspace root inside the container; sources land at ``<root>/<workspace_subdir>``.
_WORKSPACE_ROOT = "/workspace"

# Container user that runs the agent / shell tools (from the image).
_CONTAINER_USER = "pentester"

def build_session_entries(

def _is_safe_workspace_subdir(ws_subdir: str) -> bool:
"""Reject subdirs that would escape ``/workspace`` when joined.

``ws_subdir`` becomes both a tar member prefix and a ``chown`` target, so a
value containing ``..`` (or an absolute path) could write outside the
intended ``/workspace/<subdir>`` tree. Callers pass values with surrounding
slashes already stripped.
"""
if not ws_subdir:
return False
return not any(part in ("..", "") for part in ws_subdir.split("/"))


def split_local_sources(
local_sources: list[dict[str, Any]],
) -> tuple[dict[str | Path, BaseEntry], list[dict[str, Any]]]:
"""Split local sources into copied manifest entries and host bind mounts.
) -> tuple[list[dict[str, str]], list[dict[str, Any]]]:
"""Split local sources into tar-copied entries and host bind mounts.

Sources flagged ``mount`` are bind-mounted read-only at
``/workspace/<workspace_subdir>`` (not added to the manifest, so the SDK
does not stream them in file-by-file). Every other source becomes a
``LocalDir`` entry copied into the container as before.
``/workspace/<workspace_subdir>`` (applied by Docker at container-create
time). Every other source is copied into the container after start via a
single tar ``put_archive`` (see :func:`_import_local_sources`).
"""
entries: dict[str | Path, BaseEntry] = {}
copied: list[dict[str, str]] = []
bind_mounts: list[dict[str, Any]] = []
for src in local_sources:
ws_subdir = src.get("workspace_subdir") or ""
ws_subdir = (src.get("workspace_subdir") or "").strip("/")
Comment thread
aYoung-CS marked this conversation as resolved.
host_path = src.get("source_path") or ""
if not ws_subdir or not host_path:
continue
if not _is_safe_workspace_subdir(ws_subdir):
logger.warning("Skipping local source with unsafe workspace_subdir: %r", ws_subdir)
continue
resolved = Path(host_path).expanduser().resolve()
if src.get("mount"):
bind_mounts.append(
Expand All @@ -54,8 +76,134 @@ def build_session_entries(
}
)
else:
entries[ws_subdir] = LocalDir(src=resolved)
return entries, bind_mounts
copied.append({"source_path": str(resolved), "workspace_subdir": ws_subdir})
return copied, bind_mounts


def _build_source_tar(src_root: Path, arc_prefix: str) -> tuple[bytes, int, int]:
"""Pack ``src_root`` into an in-memory tar rooted at ``arc_prefix``.

Returns ``(tar_bytes, added, skipped)``. Regular files and directories —
including dotfiles such as ``.git`` and directories with no files — are
packed as-is so source-aware and git-diff analysis keep working and
committed empty dirs survive. Symlinks are skipped (and counted) rather
than followed, avoiding host path escapes and the dangling links a naive
copy would create inside the container.
"""
buf = io.BytesIO()
added = 0
skipped = 0
with tarfile.open(fileobj=buf, mode="w") as tar:
for dirpath, dirnames, filenames in os.walk(src_root, followlinks=False):
kept_dirs: list[str] = []
for name in dirnames:
if Path(dirpath, name).is_symlink():
skipped += 1
continue
kept_dirs.append(name)
dirnames[:] = kept_dirs

dir_abs = Path(dirpath)
rel = dir_abs.relative_to(src_root).as_posix()
dir_arcname = arc_prefix if rel == "." else f"{arc_prefix}/{rel}"
tar.add(str(dir_abs), arcname=dir_arcname, recursive=False)

for name in filenames:
full = dir_abs / name
if full.is_symlink() or not full.is_file():
skipped += 1
continue
arcname = f"{arc_prefix}/{full.relative_to(src_root).as_posix()}"
tar.add(str(full), arcname=arcname, recursive=False)
added += 1
Comment thread
aYoung-CS marked this conversation as resolved.
return buf.getvalue(), added, skipped


def _container_of(session: Any) -> Any:
"""Reach the underlying docker-py ``Container`` from an SDK session.

The SDK wraps the backend session in an outer object; the docker backend
exposes the real container as ``_inner._container``. Pinned to
openai-agents==0.14.6 — re-check these private attrs on SDK bumps.
"""
inner = getattr(session, "_inner", session)
container = getattr(inner, "_container", None)
if container is None:
raise RuntimeError("could not locate docker container on sandbox session")
return container


def _run_checked(container: Any, cmd: list[str]) -> None:
"""Run ``cmd`` in the container as root and raise on non-zero exit.

``exec_run`` reports failures only via its result; an ignored ``chown``
failure would leave sources root-owned and unwritable while session setup
still "succeeds", so surface it here with the command's own diagnostics.
"""
result = container.exec_run(cmd, user="root")
if result.exit_code:
output = result.output
detail = output.decode("utf-8", "replace").strip() if isinstance(output, bytes) else output
raise RuntimeError(f"container command {cmd!r} failed (exit {result.exit_code}): {detail}")


async def _import_local_sources(
session: Any,
copied_sources: list[dict[str, str]],
) -> None:
"""Copy each host source tree into the container via a single tar import.

Replaces the SDK's per-file ``LocalDir`` materialization, which issues
several ``docker exec`` calls per file. On large trees that serializes into
thousands of exec round-trips against a non-thread-safe docker-py client,
causing multi-minute hangs (the "stuck on loading" symptom) and occasional
``ExecTransportError``. ``put_archive`` lands the whole tree in one shot
(sub-second for thousands of files).

Safe here because the copy path attaches no Docker volume-driver mounts —
the SDK avoids ``put_archive`` only to sidestep volume plugins that re-run
mount setup during archive ops (docker.py:709). ``--mount`` sources use
plain read-only bind mounts at a different subdir, which have no such
driver.
"""
container = _container_of(session)
loop = asyncio.get_running_loop()

for src in copied_sources:
ws_subdir = src["workspace_subdir"]
src_root = Path(src["source_path"])
if not src_root.is_dir():
logger.warning("Skipping non-directory local source: %s", src_root)
continue

tar_bytes, added, skipped = await loop.run_in_executor(
None, _build_source_tar, src_root, ws_subdir
)
logger.info(
"Importing %d files into %s/%s (skipped %d symlink entries)",
added,
_WORKSPACE_ROOT,
ws_subdir,
skipped,
)

def _put(tar_bytes: bytes = tar_bytes, ws_subdir: str = ws_subdir) -> None:
_run_checked(container, ["mkdir", "-p", _WORKSPACE_ROOT])
if not container.put_archive(_WORKSPACE_ROOT, tar_bytes):
raise RuntimeError(f"put_archive failed for {_WORKSPACE_ROOT}/{ws_subdir}")
# put_archive unpacks as root with the tar's host uids; hand the
# tree to the agent's runtime user so tools can read/write it.
_run_checked(
container,
[
"chown",
"-R",
f"{_CONTAINER_USER}:{_CONTAINER_USER}",
f"{_WORKSPACE_ROOT}/{ws_subdir}",
],
)

await loop.run_in_executor(None, _put)


async def create_or_reuse(
Expand All @@ -67,24 +215,29 @@ async def create_or_reuse(
"""Return the existing session bundle for ``scan_id`` or create a new one.

Each ``local_sources`` entry exposes its host ``source_path`` at
``/workspace/<workspace_subdir>`` inside the container — copied in, or
bind-mounted read-only when the entry is flagged ``mount``.
``/workspace/<workspace_subdir>`` inside the container — copied in via a
single tar ``put_archive`` after start, or bind-mounted read-only when the
entry is flagged ``mount``.
"""
cached = _SESSION_CACHE.get(scan_id)
if cached is not None:
logger.info("Reusing existing sandbox session for scan %s", scan_id)
return cached

entries, bind_mounts = build_session_entries(local_sources)
copied_sources, bind_mounts = split_local_sources(local_sources)

# Copied source trees are imported after start() via a single tar
# ``put_archive`` (see ``_import_local_sources``) instead of the SDK's
# per-file ``LocalDir`` exec loop, which hangs on large trees. So the
# manifest itself carries no source entries.
# Caido runs as an in-container sidecar; HTTP(S) traffic from any
# process started via ``session.exec`` (the SDK's Shell tool, etc.)
# picks up these env vars automatically. ``NO_PROXY`` keeps the
# agent-browser CDP daemon's localhost traffic from looping back
# through Caido.
container_caido_url = f"http://127.0.0.1:{_CONTAINER_CAIDO_PORT}"
manifest = Manifest(
entries=entries,
entries={},
environment=Environment(
value={
"PYTHONUNBUFFERED": "1",
Expand Down Expand Up @@ -113,6 +266,8 @@ async def create_or_reuse(
bind_mounts=bind_mounts,
)

await _import_local_sources(session, copied_sources)

caido_endpoint = await session.resolve_exposed_port(_CONTAINER_CAIDO_PORT)
host_caido_url = f"http://{caido_endpoint.host}:{caido_endpoint.port}"
logger.debug("Caido host endpoint resolved: %s", host_caido_url)
Expand Down
Loading