[dongjiang1989]

Summary

Adds AgentSandboxProvider, a ContainerProvider that runs an environment server in a kubernetes-sigs/agent-sandbox sandbox claimed from a pre-created SandboxWarmPool, using the hand-written k8s-agent-sandbox SDK. Traffic flows through a Gateway/sandbox-router ingress; start_container() returns a loopback proxy URL that injects the X-Sandbox-* routing headers into every HTTP request and WebSocket handshake (same pattern as HFSandboxProvider), so any EnvClient connects unchanged. Optional extra pip install openenv[agent-sandbox] with a lazy SDK import; not re-exported, same as KubernetesProvider. Follows the RFC 002 "Cloud Sandbox Providers" amendment: https enforced on the ingress leg (allow_insecure=True opt-out for local Kind), bounded lifetime via shutdown_after_seconds=3600, ingress URLs kept out of error messages.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation
• New environment
• Refactoring

Alignment Checklist

Before submitting, verify:

• I have read .claude/docs/PRINCIPLES.md and this PR aligns with our principles
• I have checked .claude/docs/INVARIANTS.md and no invariants are violated
• I have run /pre-submit-pr (or bash .claude/hooks/lint.sh and tests) and addressed all issues

RFC Status

• Not required (implements the existing ContainerProvider contract per the proposed RFC 002 cloud-sandbox amendment; no new core concepts)

Test Plan

• PYTHONPATH=src:envs uv run pytest tests/test_core/test_agent_sandbox_provider.py: 31 passed, no cluster or SDK account needed. Tests use a fake SDK module and a loopback fake router, covering header injection over HTTP and WebSocket end-to-end, gateway/direct connection configs, create_sandbox argument forwarding (warmpool, namespace, env merge, TTL), https enforcement with scheme rewrite and the allow_insecure opt-out, URL non-leakage in errors, image/port/kwargs rejection, failed-start claim cleanup, idempotent and retried stop, wait_for_ready health/timeout/inactive paths, and context-manager exit.
• PYTHONPATH=src:envs uv run pytest tests/test_core/: 617 passed, 16 skipped (4 pre-existing local failures in test_generic_client.py caused by Docker occupying port 8000; they reproduce on main).
• ruff check passes repo-wide; usort check / ruff format --check only report the pre-existing main failures (tests/envs/test_julia_env.py, tests/envs/test_grid_world.py, src/openenv/core/harness/README.md), none in files touched here.
• Live run against a cluster:

  kubectl apply -f sandboxtemplate.yaml -f sandboxwarmpool.yaml   # OpenEnv echo image, port 8000
  pip install "openenv[agent-sandbox]"
  PYTHONPATH=src:envs uv run python examples/agent_sandbox_echo_env.py openenv-echo-pool external-http-gateway
  # Expected: claim becomes Ready, /health returns 200, echo_message round-trips "Hello, World!", claim deleted at exit.

Claude Code Review

Developed with Claude Code (brainstorming -> spec -> plan -> TDD execution). N/A for /alignment-review output.

---

Note

Medium Risk
New isolated runtime path with a local HTTP/WebSocket proxy and cluster claim lifecycle; security-sensitive routing headers and TLS rules, but scoped to an opt-in extra with extensive unit tests and no changes to core default providers.

Overview
Adds AgentSandboxProvider, a new runtime backend that claims OpenEnv servers from a pre-created SandboxWarmPool (kubernetes-sigs agent-sandbox) and exposes them through a local loopback proxy that injects X-Sandbox-* routing headers—same ergonomic pattern as HFSandboxProvider, with stricter ingress handling (HTTPS by default, no HTTP/WS redirect following, sanitized upstream errors).

Ships as optional pip install openenv[agent-sandbox] (k8s-agent-sandbox, Python 3.11+), plus docs, reference autodoc, an examples/agent_sandbox_echo_env.py script, and a large mocked test suite (fake SDK + fake router) covering lifecycle, env merge, TTL, claim rollback, and proxy behavior.

^{Reviewed by Cursor Bugbot for commit fa059d7. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Stale Bugbot comment from a previous run.

[burtenshaw]

Changes requested at eb016bd2f1730932b83d9b163067f31298b8160a.

1. [P1] Restrict upstream redirects. src/openenv/core/containers/runtime/agent_sandbox_provider.py:114 and src/openenv/core/containers/runtime/agent_sandbox_provider.py:185: automatic redirects bypass the initial ingress check. Reproduction: an HTTPS router returns HTTP 307 to a plaintext receiver. The proxy replays the POST body and a configured X-Router-Key to that receiver without allow_insecure=True. A WSS HTTP 302 to a different TLS origin also forwards router_headers["Authorization"] to that origin. Reject upstream redirects with a generic proxy error, or validate the scheme and origin of every hop before forwarding bodies or credentials. Add HTTP downgrade and WebSocket cross-origin regression tests.

2. [P2] Preserve claim ownership when provisioning rollback fails. src/openenv/core/containers/runtime/agent_sandbox_provider.py:416: create_sandbox() runs before the cleanup block and before the provider records a claim identity. Reproduction with the real 1.0.6 SDK: claim readiness times out and the SDK's rollback DELETE raises once. The claim remains, provider._sandbox is None, close() issues no retry, and the SDK's atexit registry contains no handle. A second start creates another claim. Retain an owned claim identity before provisioning, keep it until deletion succeeds, retry deletion through close(), and reject another start while cleanup is pending. The one-hour TTL bounds this leak but does not make cleanup retryable.

3. [P2] Keep the proxy handle until its thread stops. src/openenv/core/containers/runtime/agent_sandbox_provider.py:213 and src/openenv/core/containers/runtime/agent_sandbox_provider.py:474: the five-second join can expire, after which both the proxy's thread/server handles and the provider's proxy handle are discarded. Reproduction: keep an upstream HTTP response open, call stop(), and inspect the saved thread. It is still alive after stop() returns, while _thread and _server are None. Bound shutdown of in-flight requests, and retain the handles and report failure if the thread has not stopped so cleanup can retry.

4. [P2] Sanitize WebSocket handshake failures. src/openenv/core/containers/runtime/agent_sandbox_provider.py:139 and src/openenv/core/containers/runtime/agent_sandbox_provider.py:195: raw handshake exceptions escape the route and are logged by Uvicorn. Reproduction: a TLS router redirects the handshake to ws://127.0.0.1:9/session?token=review-url-secret. The final ASGI traceback contains that complete URL and token. Handle handshake failures inside the route and return a generic denial or close without logging the upstream exception chain. Add a log-capture regression test. This violates the credential rule in .claude/docs/INVARIANTS.md and RFC 002 S4.

Keep corresponding proxy fixes aligned with src/openenv/core/containers/runtime/hf_sandbox_provider.py, as required by the copy note at agent_sandbox_provider.py:75.

Suggestions:

• src/openenv/core/containers/runtime/agent_sandbox_provider.py:105 and src/openenv/core/containers/runtime/agent_sandbox_provider.py:127: request and response bodies are fully buffered. A server-sent event is withheld until the upstream stream ends. Stream these bodies or document this limitation.
• src/openenv/core/containers/runtime/agent_sandbox_provider.py:199 and src/openenv/core/containers/runtime/agent_sandbox_provider.py:157: binary uploads raise KeyError: 'text', and upstream close code 1001 becomes downstream code 1006. Relay binary frames and close codes, or document the narrower transport support. JSON text and downstream binary frames passed with eight concurrent clients.
• tests/test_core/test_agent_sandbox_provider.py:192 and tests/test_core/test_agent_sandbox_provider.py:211: use strict SDK signatures in the fake or a wheel-backed contract check. The current **kwargs configurations and MagicMock client accept unsupported SDK calls.
• docs/source/guides/runtime-providers.md:204: use registry.hf.space/openenv-echo-env:latest, the image in the getting-started guide. The documented GHCR image returned manifest HTTP 404; the HF registry image returned HTTP 200.
• docs/source/guides/runtime-providers.md:200: show resource limits and automountServiceAccountToken: false in the template, and document the template's environment-variable injection policy when using env_vars.

The two commits previously pushed from burtenshaw's account should remain. d2bbe6906ecbf7664357962ee615223b04c25ce8 reads Sandbox.is_active as a property; the former call reproduces TypeError: 'bool' object is not callable, and the corrected call passes with a real SDK handle. eb016bd2f1730932b83d9b163067f31298b8160a raises the SDK floor because 1.0.5.post1 rejects SandboxClient(api_client=...) and 1.0.6 accepts it. Verified against the downloaded 1.0.5.post1 wheel and 1.0.6 wheel. No additional commits were pushed in this review.

All SDK calls and kwargs used by the provider exist in 1.0.6. The real SDK retries failed handle termination and rolls back a timed-out claim when deletion succeeds. The default lifecycle sets deletion one hour from claim creation. Lazy import and the missing-extra installation hint pass. The proxy binds to 127.0.0.1.

Validation: the 32 provider tests passed. Provider and related cleanup tests passed with non-loopback socket connections blocked: 427 passed, 4 skipped. Twelve additional offline probes verified the lifecycle and proxy behaviors above. The new provider/test files pass usort, Ruff formatting, and Ruff lint; repository Ruff lint passes. The full lint hook reports existing formatting failures outside this PR. No live cluster run was performed. At the check snapshot, lint, package smoke tests, docs, discovery, lock validation, thinkingbox, tau2, and Bugbot had passed; Python 3.11 and 3.12 tests were still running.

The November 4 Bugbot HTTPS-rewrite thread is already resolved. The author's d408e639 change rejects plaintext gateway ingress unless explicitly opted in; its regression tests pass. There are no unresolved Bugbot threads in the fetched review threads.

[dongjiang1989]

Thanks @burtenshaw Pleaese recheck it

All four fixed (rebased on main, your two commits kept):

• Redirects: HTTP now uses allow_redirects=False and turns 3xx into a generic 502; WS disables redirect following via process_redirect (websockets 16 does replay additional_headers cross-origin, confirmed). Regression tests for both.
• Claim ownership: claim_name is pre-generated and kept until deletion succeeds; a failed rollback is retryable through stop_container(), and a new start is refused while cleanup is pending.
• Proxy thread: stop() sets force_exit and raises if the thread survives the join, keeping the handles for retry. The claim is still terminated first.
• WS handshake: failures are caught in the route — generic warning, no exception chain in logs, plain deny to the client.

Same proxy fixes applied to hf_sandbox_provider. Docs now use the registry.hf.space echo image, and the template YAML has resource limits + automountServiceAccountToken: false.

40 provider tests pass; tests/test_core 629 passed (4 pre-existing local failures from Docker on port 8000, reproducible on main).

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 77a25a5. Configure here.}

[burtenshaw]

NEEDS AUTHOR CHANGES.

Reviewed head: 77a25a54e0074d20f4a902299db009b0cfc06eee. The final PR check returned isDraft: false and the same head.

Previous blockers:

• PASS, P1 redirects. Both proxies reject an HTTPS HTTP 307 to a plaintext recorder and a WSS handshake HTTP 302 to a different TLS origin. The recorder receives zero requests, bodies, and credential headers. Tested with websockets 15.0.1 and 16.0. A control using the default WebSocket connector follows the same redirect and replays Authorization. Evidence: src/openenv/core/containers/runtime/agent_sandbox_provider.py:120, src/openenv/core/containers/runtime/agent_sandbox_provider.py:215, src/openenv/core/containers/runtime/hf_sandbox_provider.py:148, and src/openenv/core/containers/runtime/hf_sandbox_provider.py:243; inline transport probe below.
• FAIL, P2 claim lifecycle. Recording ownership and explicit cleanup retries pass with real SDK 1.0.6: a failed DELETE retains the identity, another start is refused, and close treats 404 as success. Supplying claim_name disables the SDK's rollback and exit deletion. The provider does not replace either operation. A readiness timeout now issues zero DELETEs, and successful provisioning followed by SDK exit cleanup also issues zero DELETEs. See finding 1 and the inline lifecycle probe.
• FAIL, P2 proxy handles. stop bounds its join, raises with handles retained, and stop_container terminates the sandbox before re-raising. A blocked real upstream response verifies the five-second bound and successful retry after release. Both providers still accept a new start after that failure and overwrite the retained proxy. See finding 2 and the inline probes.
• PASS, P2 handshake logging. Both routes deny a failed handshake with a generic warning. Captured log records and formatted exception chains contain no upstream URL or synthetic token for redirects and an injected URL-bearing SecurityError. Evidence: src/openenv/core/containers/runtime/agent_sandbox_provider.py:152 and src/openenv/core/containers/runtime/hf_sandbox_provider.py:180; inline transport probe below.

1. [P2] Restore provider-owned rollback and exit deletion for pre-named claims.

src/openenv/core/containers/runtime/agent_sandbox_provider.py:448, src/openenv/core/containers/runtime/agent_sandbox_provider.py:459, src/openenv/core/containers/runtime/agent_sandbox_provider.py:460, and src/openenv/core/containers/runtime/agent_sandbox_provider.py:481.

SDK 1.0.6 treats an explicit claim_name as caller-owned. create_sandbox skips rollback for these claims, and the registered exit callback closes the connection without deleting the claim. The provider's create_sandbox call remains outside its exception cleanup block. A failed start therefore retains a claim without attempting deletion, and a normal process exit after a successful start leaves the claim running until its TTL. cleanup=True no longer provides the deletion promised by the comment. The control below verifies one exit DELETE for an SDK-generated name and zero for this provider's name.

Attempt provider-owned deletion on provisioning failure and register exit cleanup for provider-owned claims. Keep the recorded identity when deletion fails so stop_container and close retain their retry behavior. Do not rely on SDK automatic cleanup for explicitly named claims.

2. [P2] Reject restart while a proxy still awaits cleanup.

src/openenv/core/containers/runtime/agent_sandbox_provider.py:400, src/openenv/core/containers/runtime/agent_sandbox_provider.py:471, src/openenv/core/containers/runtime/hf_sandbox_provider.py:318, and src/openenv/core/containers/runtime/hf_sandbox_provider.py:347.

After proxy.stop raises, stop_container deletes the sandbox and retains the proxy. The start guards check sandbox or claim state but omit proxy state. The next start allocates another sandbox and replaces self._proxy. A later stop cannot reach the old proxy thread. The lifecycle probe prints retained proxy overwritten = True for both providers. Include a pending proxy in both start guards before allocating another sandbox, and test stop failure followed by restart.

HF proxy verdict: the redirect and handshake-log fixes pass. Normal authenticated HTTP POST traffic, query strings, WebSocket text, and downstream binary frames pass with both tested websockets versions. Shutdown reports a stuck thread and retains handles for retry; sandbox.kill still runs when proxy shutdown raises. Finding 2 remains. The HF changes have no dependency on AgentSandboxProvider or the new SDK extra and can be split into a small PR touching hf_sandbox_provider.py plus dedicated regression tests. Include the pending-proxy start guard in that PR. No committed HF sandbox provider test file or HF provider references exist under tests at this head, so there was no existing HF provider suite to run. The inline probes supply that verification.

Suggestions checked:

• Addressed: docs/source/guides/runtime-providers.md:207 uses registry.hf.space/openenv-echo-env:latest; :203 disables service-account token mounting; :211 supplies CPU and memory requests and limits; :261 explains template env merging and cold starts.
• Addressed: buffering, upstream text-frame support, missing close-code forwarding, and rejected redirects are documented at src/openenv/core/containers/runtime/agent_sandbox_provider.py:78 and src/openenv/core/containers/runtime/hf_sandbox_provider.py:105.
• Partly addressed: config fakes have strict signatures at tests/test_core/test_agent_sandbox_provider.py:217 and :228, and create_sandbox rejects unknown keyword names at :248. SandboxClient remains an unspecced MagicMock at :255. Suggest giving its constructor a strict signature too.
• Bugbot's example finding holds: examples/agent_sandbox_echo_env.py:56 calls start_container() before the try at line 57, so a failed start never reaches stop_container() at line 69. With the provider now keeping the claim identity for retry, move the call inside the try so the example releases the claim.
• Suggestion: usort requests collapsing the import at examples/agent_sandbox_echo_env.py:34. Examples are outside the standard usort scope; this is not a blocker.

Validation:

• uv sync --all-extras completed. The environment resolved SDK 1.0.9 and websockets 15.0.1; the lifecycle probe separately loaded real SDK 1.0.6, and the transport probe also ran with websockets 16.0.
• PYTHONPATH=src:envs uv run pytest tests/test_core/test_agent_sandbox_provider.py tests/core -q: 975 passed, 9 skipped. All 40 agent-provider tests passed.
• PYTHONPATH=src:envs uv run pytest tests/test_core -q: 630 passed, 19 skipped, 23 subtests passed. These runs overlap; the counts are not additive.
• Changed source and test files pass usort. All four changed Python files pass Ruff formatting and lint. Repository src/tests Ruff lint passes. The full lint hook fails on 47 unchanged format files and the two unchanged usort files tests/envs/test_grid_world.py and tests/envs/test_julia_env.py. The debug hook reports no new provider matches.
• The rebased maintainer fixes remain: Sandbox.is_active is read as a property at src/openenv/core/containers/runtime/agent_sandbox_provider.py:562, and the SDK floor is 1.0.6 at pyproject.toml:69.
• No live Kubernetes cluster or HF sandbox was used. SDK Kubernetes API operations and the HF pool were mocked; transport probes used local TLS servers.

The lifecycle reproduction below uses the real 1.0.6 client, helper, and sandbox handle. It substitutes Kubernetes API responses and a proxy thread that reports itself alive. It exits with status 1 on this head and reports the missing exit deletion and both overwritten proxies.

PYTHONPATH=src:envs uv run --with k8s-agent-sandbox==1.0.6 python - <<'PY'
from importlib.metadata import version
from types import SimpleNamespace
from unittest.mock import Mock, patch

from kubernetes import client as kube
from kubernetes.client.exceptions import ApiException
from k8s_agent_sandbox.k8s_helper import K8sHelper

from openenv.core.containers.runtime import agent_sandbox_provider as agent
from openenv.core.containers.runtime import hf_sandbox_provider as hf


assert version("k8s-agent-sandbox") == "1.0.6"
failures = []


def create_claim(self, **kwargs):
    manifest = kwargs["body"]
    manifest["metadata"].update(uid="review-uid", resourceVersion="1")
    return manifest


def restart_check(provider, label):
    provider.start_container()
    old = provider._proxy
    sandbox = provider._sandbox
    old._server = Mock()
    old._thread = Mock()
    old._thread.is_alive.return_value = True
    try:
        provider.stop_container()
        raise AssertionError("stuck stop did not raise")
    except RuntimeError:
        pass
    assert provider._proxy is old
    assert provider._sandbox is None
    if isinstance(provider, hf.HFSandboxProvider):
        sandbox.kill.assert_called_once()
    else:
        assert sandbox.claim_name is None and provider._claim is None
    print("PASS", label, "sandbox terminated despite proxy stop failure")
    try:
        provider.start_container()
    except RuntimeError:
        print("PASS", label, "restart refused while proxy cleanup is pending")
    else:
        lost = provider._proxy is not old
        assert lost and old._thread.is_alive()
        failures.append(label + " restart overwrites retained proxy")
        print("FAIL", label, "restart accepted; retained proxy overwritten =", lost)
    finally:
        old._server = old._thread = None
        provider.close()


with kube.ApiClient() as api, patch.object(
    kube.CustomObjectsApi, "create_namespaced_custom_object",
    autospec=True, side_effect=create_claim,
), patch.object(
    kube.CustomObjectsApi, "delete_namespaced_custom_object", autospec=True,
) as delete, patch.object(
    K8sHelper, "wait_for_claim_ready", autospec=True, return_value="sbx-review",
) as ready, patch.object(agent._LocalAuthProxy, "start", return_value="http://127.0.0.1:1"):
    def provider():
        return agent.AgentSandboxProvider(
            warmpool="review-pool", namespace="review",
            router_url="https://router.example.invalid", api_client=api,
        )

    p = provider()
    ready.side_effect = TimeoutError("claim not ready")
    try:
        p.start_container()
        raise AssertionError("provisioning unexpectedly succeeded")
    except TimeoutError:
        pass
    claim = p._claim
    assert claim is not None and p._sandbox is None
    assert delete.call_count == 0
    try:
        p.start_container()
        raise AssertionError("pending-claim restart accepted")
    except RuntimeError:
        pass
    delete.side_effect = [ApiException(status=500), ApiException(status=404)]
    try:
        p.stop_container()
        raise AssertionError("failed DELETE did not raise")
    except ApiException:
        pass
    assert p._claim == claim
    p.close()
    assert p._claim is None and delete.call_count == 2
    calls = delete.call_args_list
    assert all(c.kwargs["name"] == claim[0] and c.kwargs["namespace"] == claim[1] for c in calls)
    p.close()
    assert delete.call_count == 2
    print("PASS SDK 1.0.6: pending claim retained, restart refused, DELETE 500 retained, close DELETE 404 cleared, idempotent")
    print("NOTE SDK 1.0.6: explicit claim_name disables SDK rollback; DELETE attempts after failed start = 0")

    ready.side_effect = None
    delete.side_effect = None
    p = provider()
    p.start_container()
    before = delete.call_count
    p._client._delete_automatic_sandboxes()
    delta = delete.call_count - before
    print("SDK 1.0.6 exit cleanup: DELETE attempts =", delta, "claim retained =", p._claim is not None)
    if delta == 0:
        failures.append("explicit names disable SDK exit deletion")
    automatic = p._client.create_sandbox(warmpool="review-pool")
    before = delete.call_count
    p._client._delete_automatic_sandboxes()
    assert delete.call_count - before == 1
    assert automatic.claim_name is None
    print("CONTROL SDK-generated name: exit cleanup DELETE attempts = 1")
    p.close()
    restart_check(provider(), "AgentSandboxProvider")

pool = SimpleNamespace(create=lambda env: SimpleNamespace(
    proxy_url_for=lambda port, path: "https://sandbox.example.invalid",
    proxy_headers={"Authorization": "Bearer review-hf-secret", "X-Sandbox-Token": "review-sandbox-secret"},
    run=Mock(), kill=Mock(),
))
with patch.object(hf, "_get_pool", return_value=pool), patch.object(
    hf._LocalAuthProxy, "start", return_value="http://127.0.0.1:1",
):
    p = hf.HFSandboxProvider(image="review-image")
    p.start_container()
    sandbox = p._sandbox
    p.stop_container()
    sandbox.kill.assert_called_once()
    p.close()
    print("PASS HF provider normal create/start/stop/idempotent close")
    restart_check(hf.HFSandboxProvider(image="review-image"), "HFSandboxProvider")

print("FAILURES:", failures)
raise SystemExit(bool(failures))

PY

The transport reproduction below runs both proxy implementations against an HTTPS router, a plaintext HTTP recorder, and a second TLS recorder. It confirms the upstream handshake is HTTP 302 before testing rejection, verifies the default connector replays Authorization as a control, checks normal traffic and captured handshake logs, and checks both simulated and real blocked-request shutdown. It exits with status 0 for each websockets version on this head. The simulated and forced shutdowns emit cancellation diagnostics; the handshake log assertions pass.

for ws_version in 15.0.1 16.0; do
  PYTHONPATH=src:envs uv run --with "websockets==$ws_version" python - <<'PY'
import asyncio
import logging
import os
import socket
import subprocess
import tempfile
import threading
import time
import traceback
from contextlib import contextmanager
from pathlib import Path
from unittest.mock import Mock, patch

import requests
import uvicorn
import websockets
from fastapi import FastAPI, Request, Response, WebSocket
from websockets.asyncio.client import connect
from websockets.exceptions import InvalidStatus, SecurityError

from openenv.core.containers.runtime import agent_sandbox_provider as agent
from openenv.core.containers.runtime import hf_sandbox_provider as hf


@contextmanager
def serve(app, cert=None, key=None):
    sock = socket.socket()
    sock.bind(("127.0.0.1", 0))
    port = sock.getsockname()[1]
    server = uvicorn.Server(uvicorn.Config(
        app, log_config=None, access_log=False, log_level="warning",
        ssl_certfile=cert, ssl_keyfile=key,
    ))
    thread = threading.Thread(target=server.run, kwargs={"sockets": [sock]}, daemon=True)
    thread.start()
    deadline = time.monotonic() + 5
    while not server.started:
        assert thread.is_alive() and time.monotonic() < deadline
        time.sleep(0.01)
    try:
        yield f"{'https' if cert else 'http'}://127.0.0.1:{port}"
    finally:
        server.should_exit = True
        thread.join(3)
        assert not thread.is_alive()
        sock.close()


def ws_url(url):
    return url.replace("https://", "wss://").replace("http://", "ws://")


async def denied(url):
    try:
        async with connect(url):
            raise AssertionError("handshake was accepted")
    except InvalidStatus as exc:
        assert exc.response.status_code == 403


async def confirm_redirect(url):
    connector = connect(url)
    connector.process_redirect = lambda exc: exc
    try:
        await connector
        raise AssertionError("upstream accepted redirect probe")
    except InvalidStatus as exc:
        assert exc.response.status_code == 302
        assert "review-url-secret" in exc.response.headers["Location"]


async def normal_ws(url):
    async with connect(url) as ws:
        await ws.send("ping")
        assert await ws.recv() == "echo:ping"
        await ws.send("binary-response")
        assert await ws.recv() == b"binary-response"


async def follow_redirect(url):
    async with connect(url, additional_headers={"Authorization": "Bearer control-secret"}):
        pass


def check(module, target, source_requests, recorder_requests):
    credentials = {
        "Authorization": "Bearer review-hf-secret",
        "X-Sandbox-Token": "review-sandbox-secret",
        "X-Router-Key": "review-router-secret",
    }
    proxy = module._LocalAuthProxy(target_url=target, headers=credentials)
    base = proxy.start()
    records = []
    original_handle = logging.Logger.handle

    def capture(logger, record):
        records.append(record)
        return original_handle(logger, record)

    try:
        with patch.object(logging.Logger, "handle", capture):
            result = requests.post(base + "/echo?x=1", data=b"review-body", timeout=10)
            assert result.status_code == 201
            assert result.content == b"review-body"
            assert result.headers["X-Query"] == "x=1"
            assert source_requests[-1][1] == b"review-body"
            for key, value in credentials.items():
                assert source_requests[-1][2][key.lower()] == value
            result = requests.post(base + "/redirect", data=b"review-body", timeout=10)
            assert result.status_code == 502
            assert "Location" not in result.headers
            assert b"review-url-secret" not in result.content
            asyncio.run(denied(ws_url(base) + "/redirect"))
            assert recorder_requests == [], recorder_requests
            asyncio.run(normal_ws(ws_url(base) + "/ws"))
            for key, value in credentials.items():
                assert source_requests[-1][2][key.lower()] == value
            with patch.object(module, "ws_connect", side_effect=SecurityError(
                "wss://127.0.0.1:9/private?token=review-url-secret "
                "Bearer review-hf-secret"
            )):
                asyncio.run(denied(ws_url(base) + "/ws"))
        assert any("handshake failed" in r.getMessage() for r in records)
        for record in records:
            rendered = record.getMessage()
            if record.exc_info:
                rendered += "".join(traceback.format_exception(*record.exc_info))
            for secret in (*credentials.values(), "review-url-secret", target):
                assert secret not in rendered, rendered
        print(module.__name__, "HTTP 307 / WSS 302 rejected; recorder=0; normal HTTP/WS/binary downstream; logs sanitized")
        old_thread, old_server = proxy._thread, proxy._server
        stuck = Mock()
        stuck.is_alive.return_value = True
        proxy._thread = stuck
        try:
            try:
                proxy.stop()
                raise AssertionError("stuck-thread stop did not raise")
            except RuntimeError:
                pass
            assert proxy._thread is stuck and proxy._server is old_server
            assert old_server.force_exit and old_server.should_exit
            stuck.join.assert_called_once_with(timeout=5.0)
        finally:
            proxy._thread = old_thread
        proxy.stop()
        assert not old_thread.is_alive()
        assert proxy._thread is None and proxy._server is None
        print(module.__name__, "stuck-thread handles retained; retry stopped real proxy")
        proxy = module._LocalAuthProxy(target_url=target, headers=credentials)
        base = proxy.start()
        entered.clear()
        release.clear()

        def request_in_flight():
            try:
                requests.get(base + "/hang", timeout=15)
            except requests.RequestException:
                pass

        caller = threading.Thread(target=request_in_flight, daemon=True)
        caller.start()
        assert entered.wait(5)
        old_thread, old_server = proxy._thread, proxy._server
        started = time.monotonic()
        raised = False
        try:
            proxy.stop()
        except RuntimeError:
            raised = True
            assert proxy._thread is old_thread and proxy._server is old_server
        assert time.monotonic() - started < 6
        release.set()
        caller.join(3)
        old_thread.join(3)
        proxy.stop()
        assert not old_thread.is_alive() and proxy._thread is None
        print(module.__name__, "in-flight stop bounded; reported live thread =", raised, "; release and retry cleared handles")
    finally:
        release.set()
        proxy.stop()


with tempfile.TemporaryDirectory() as temp:
    cert, key = str(Path(temp) / "cert.pem"), str(Path(temp) / "key.pem")
    subprocess.run([
        "openssl", "req", "-x509", "-newkey", "rsa:2048", "-nodes", "-days", "1",
        "-keyout", key, "-out", cert, "-subj", "/CN=localhost",
        "-addext", "subjectAltName=IP:127.0.0.1",
    ], check=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
    recorder_requests, source_requests = [], []
    entered, release = threading.Event(), threading.Event()
    recorder, source = FastAPI(), FastAPI()

    @recorder.api_route("/{path:path}", methods=["GET", "POST"])
    async def record_http(path: str, request: Request):
        recorder_requests.append((path, await request.body(), dict(request.headers)))
        return Response(b"recorded")

    @recorder.websocket("/{path:path}")
    async def record_ws(path: str, websocket: WebSocket):
        recorder_requests.append((path, b"", dict(websocket.headers)))
        await websocket.accept()
        await websocket.close()

    @source.api_route("/{path:path}", methods=["GET", "POST"])
    async def source_http(path: str, request: Request):
        body = await request.body()
        source_requests.append((path, body, dict(request.headers)))
        if path == "redirect":
            return Response(status_code=307, headers={
                "Location": plain_recorder + "/steal?token=review-url-secret"
            })
        if path == "hang":
            entered.set()
            await asyncio.to_thread(release.wait, 15)
        return Response(body, status_code=201, headers={"X-Query": request.url.query})

    @source.websocket("/{path:path}")
    async def source_ws(path: str, websocket: WebSocket):
        source_requests.append((path, b"", dict(websocket.headers)))
        if path == "redirect":
            await websocket.send({
                "type": "websocket.http.response.start", "status": 302,
                "headers": [(b"location", (ws_url(tls_recorder) + "/steal?token=review-url-secret").encode())],
            })
            await websocket.send({"type": "websocket.http.response.body", "body": b""})
            return
        await websocket.accept()
        assert await websocket.receive_text() == "ping"
        await websocket.send_text("echo:ping")
        assert await websocket.receive_text() == "binary-response"
        await websocket.send_bytes(b"binary-response")
        await websocket.close()

    with patch.dict(os.environ, {"REQUESTS_CA_BUNDLE": cert, "SSL_CERT_FILE": cert}):
        with serve(recorder) as plain_recorder, serve(recorder, cert, key) as tls_recorder, serve(source, cert, key) as target:
            print("websockets", websockets.__version__)
            asyncio.run(confirm_redirect(ws_url(target) + "/redirect"))
            asyncio.run(follow_redirect(ws_url(target) + "/redirect"))
            assert len(recorder_requests) == 1
            assert recorder_requests[0][2]["authorization"] == "Bearer control-secret"
            recorder_requests.clear()
            print("positive control: default WebSocket connector follows 302 and replays Authorization")
            for module in (agent, hf):
                check(module, target, source_requests, recorder_requests)

PY
done

[dongjiang1989]

Both findings fixed (the example/atexit items were already covered by 16f5d194, pushed minutes after your review):

1. Named-claim rollback: start_container now deletes the claim itself when create_sandbox fails — the identity is only kept (and starts refused) when that DELETE fails, so stop_container()/close() still retry. Dropped cleanup=True; the provider registers its own atexit stop instead, since the SDK hook never deletes named claims.
2. Pending proxy: both start guards now include _proxy is not None, so a stuck proxy thread can't be overwritten by a restart. Same guard added to HFSandboxProvider.

Also gave the fake SandboxClient a strict 1.0.6 constructor signature (asserts cleanup is never passed).

Verified with your lifecycle probe adapted to the real SDK 1.0.6: immediate rollback DELETE on failed start, failed-rollback retention + restart refusal + close retry (404 clears), SDK exit cleanup skipping named claims, and restart refusal while proxy cleanup is pending — all pass for both providers. 43 provider tests and tests/test_core (632 passed) green; the 4 local test_generic_client failures are Docker-on-port-8000 and reproduce on main.

Happy to split the HF proxy changes into their own PR if you prefer — they're self-contained (redirect rejection, handshake sanitization, stop retention, start guard) with no dependency on the new extra.

[burtenshaw]

Thanks. Both items are fixed in 0eda21ab: a failed create_sandbox now deletes the named claim through _delete_pending_claim() and keeps the identity only when that delete fails, and both providers refuse a new start while _proxy still awaits cleanup.

Yes, please split the hf_sandbox_provider.py changes into their own PR, with the redirect rejection, handshake sanitization, stop retention and the pending-proxy start guard, plus their regression tests. That fix applies to code already on main and can land on its own. This PR then stays focused on the new provider, which still needs a maintainer decision on whether core carries another sandbox provider.

[dongjiang1989]

Split done: the HF proxy hardening (redirect rejection, handshake sanitization, stop retention, pending-proxy start guard) now lives in #1558 against main, with its own regression suite (tests/test_core/test_hf_sandbox_provider.py, 10 tests, faked pool + loopback upstream — there was no HF provider test file before).

This branch drops its hf_sandbox_provider.py edits (fa059d72), so the diff here is just the new provider, its tests, the example, docs and the extra. Once #1558 lands, the proxy copy in agent_sandbox_provider.py matches it line for line.

[dongjiang1989]

cc @burtenshaw Please recheck it, thx

[burtenshaw]

Rechecked at fa059d72. All four review blockers and both Bugbot findings are fixed, the HF proxy hardening is merged as #1558, and CI passes. There are no open review findings on the code.

What remains is a maintainer decision on whether core carries another sandbox provider under the proposed RFC 002 Cloud Sandbox Providers amendment. We will follow up here once that is decided.