From b1d4137963943e1658a657a81bd30fceaad8e7b5 Mon Sep 17 00:00:00 2001 From: James Wong <2421248+jameswnl@users.noreply.github.com> Date: Sat, 11 Jul 2026 14:53:44 -0400 Subject: [PATCH 1/8] RSPEED-0000: Close OpenShell spawner production gaps (issue #104) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement all missing features in OpenShellSpawner: - Skills image loading via Podman CLI extraction + exec_stream tar - Credential injection via OpenShell Provider API with file-based fallback - Read-only filesystem via SandboxPolicy with injection-target allowlist - MCP secret file injection with correct mount_path/key handling - TLS certs and service_account skip with info log - Provider cleanup in _do_destroy() Remove Podman 5.8.x JWT workaround (issue #82): - OpenShell v0.0.79+ delivers sandbox JWTs natively via Podman secrets when gateway_jwt is configured (PR NVIDIA/OpenShell#2156) - Removed _inject_podman_token and all helpers/constants - Removed podman_cli/podman_socket constructor params - History documented in spawner docstring E2E verified on EC2 (RHEL 9, m5.xlarge): - OpenShell gateway v0.0.80 with Podman driver - All 6 example workflows pass through OpenShell spawner - Sandbox lifecycle: create → wait_ready → exec → destroy confirmed Closes #104 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/cloud_agents/spawner/openshell_spawner.py | 548 +++++++------ .../workflow/temporal_entrypoint.py | 3 +- tests/unit/spawner/test_openshell_spawner.py | 735 +++++++----------- 3 files changed, 603 insertions(+), 683 deletions(-) diff --git a/src/cloud_agents/spawner/openshell_spawner.py b/src/cloud_agents/spawner/openshell_spawner.py index c8e69a80..b8e4b65d 100644 --- a/src/cloud_agents/spawner/openshell_spawner.py +++ b/src/cloud_agents/spawner/openshell_spawner.py @@ -31,6 +31,7 @@ from openshell._proto import openshell_pb2 + logger = logging.getLogger(__name__) # Default command to start the HTTP server inside the sandbox @@ -46,20 +47,6 @@ # Path where the sandbox agent writes structured JSONL events _EVENT_LOG_PATH = "/var/log/agent-events.jsonl" -# Path where the supervisor expects the sandbox JWT (matches -# openshell_core::driver_utils::SANDBOX_TOKEN_MOUNT_PATH in the -# OpenShell Podman driver). -_SANDBOX_TOKEN_PATH = "/etc/openshell/auth/sandbox.jwt" - -# Podman secret name prefix for per-sandbox JWTs (matches -# TOKEN_SECRET_PREFIX in openshell-driver-podman container.rs). -_TOKEN_SECRET_PREFIX = "openshell-token-" - -# Podman label used by the OpenShell Podman driver to tag containers. -_SANDBOX_ID_LABEL = "openshell.sandbox-id" - -# Delay after container restart to allow supervisor to boot. -_POST_RESTART_DELAY_SECS = 3.0 class OpenShellSpawner(AgentSpawner): @@ -79,7 +66,6 @@ class OpenShellSpawner(AgentSpawner): def __init__( self, openshell_client: Any = None, - podman_cli: str | None = None, **kwargs: Any, ) -> None: """Initialize the OpenShell spawner. @@ -88,20 +74,26 @@ def __init__( openshell_client: OpenShell SDK client for sandbox operations. Must implement create(), wait_ready(), exec(), exec_stream(), and delete(). - podman_cli: Path to the podman CLI binary. When set, enables - the Podman secret file mount workaround for Podman 5.8.x - (issue #82). The workaround extracts the sandbox JWT from - the Podman secret and copies it into the container after - creation. Set to None (default) when using K8s or when the - Podman secret mount works correctly. + + Requires OpenShell gateway v0.0.79+ with gateway_jwt configured. + The gateway mints sandbox JWTs and delivers them via Podman + secrets (PR NVIDIA/OpenShell#2156). No client-side JWT workaround + is needed — the supervisor authenticates directly via gRPC. + + History (issue #82): + Pre-v0.0.79 gateways used host bind-mounted token files which + failed on Podman 5.8.x REST API. We had a workaround + (_inject_podman_token) that extracted JWTs from Podman secrets + and copied them into containers. This was removed because + v0.0.79+ fixes the issue upstream and we require v0.0.79+. """ super().__init__(**kwargs) self._client = openshell_client - self._podman_cli = podman_cli self._sandbox_names: dict[str, str] = {} self._sandbox_ids: dict[str, str] = {} self._virtual_hosts: dict[str, str] = {} self._server_tasks: dict[str, asyncio.Task] = {} + self._provider_ids: dict[str, str] = {} def get_sandbox_id(self, agent_name: str) -> str | None: """Return the sandbox ID (UUID) for an agent, or None if not tracked. @@ -234,164 +226,6 @@ async def _wait_ready_with_host( await asyncio.sleep(2.0) return False - async def _inject_podman_token(self, sandbox_name: str) -> None: - """Fix Podman 5.8.x secret file mount failure (issue #82). - - The OpenShell Podman driver creates a Podman secret with the - sandbox JWT and specifies a file mount at the expected path. - On Podman 5.8.x, the mount is not applied. This workaround: - - 1. Finds the container by its openshell.sandbox-id label. - 2. Extracts the JWT from the Podman secret. - 3. Stops the container, copies the token file in, restarts. - - The supervisor reads the token on startup and authenticates - back to the gateway. - - Args: - sandbox_name: OpenShell sandbox name. - - Raises: - RuntimeError: If the container or secret is not found, or - if any podman command fails. - """ - assert self._podman_cli is not None # noqa: S101 - - # 1. Find the container name by label. - container_name = await self._podman_find_container(sandbox_name) - - # 2. Extract the JWT from the Podman secret. - secret_name = f"{_TOKEN_SECRET_PREFIX}{sandbox_name}" - token = await self._podman_extract_secret(secret_name) - - # 3. Stop container, copy token, restart. - tmp_path: str | None = None - try: - # Write token to temp file for podman cp. - fd, tmp_path = tempfile.mkstemp(suffix=".jwt", prefix="openshell-token-") - with os.fdopen(fd, "w") as f: - f.write(token.strip() + "\n") - - await self._podman_exec( - "stop", - "-t", - "0", - container_name, - ) - - await self._podman_exec( - "cp", - tmp_path, - f"{container_name}:{_SANDBOX_TOKEN_PATH}", - ) - - await self._podman_exec("start", container_name) - - logger.info( - "Podman token workaround applied for sandbox '%s' " "(container=%s, secret=%s)", - sandbox_name, - container_name, - secret_name, - ) - - # Brief delay for supervisor to boot with the injected token. - await asyncio.sleep(_POST_RESTART_DELAY_SECS) - finally: - if tmp_path is not None: - try: - os.unlink(tmp_path) - except OSError: - pass - - async def _podman_find_container(self, sandbox_name: str) -> str: - """Find the container name for a sandbox by its label. - - Args: - sandbox_name: OpenShell sandbox name. - - Returns: - Container name string. - - Raises: - RuntimeError: If no container is found with the expected label. - """ - assert self._podman_cli is not None # noqa: S101 - proc = await asyncio.create_subprocess_exec( - self._podman_cli, - "ps", - "-a", - "--filter", - f"label={_SANDBOX_ID_LABEL}={sandbox_name}", - "--format", - "{{.Names}}", - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - ) - stdout, stderr = await proc.communicate() - name = stdout.decode().strip() - if not name: - raise RuntimeError( - f"No container found for sandbox '{sandbox_name}' " - f"(label={_SANDBOX_ID_LABEL}={sandbox_name})" - ) - # If multiple lines, take the first (shouldn't happen). - return name.splitlines()[0].strip() - - async def _podman_extract_secret(self, secret_name: str) -> str: - """Extract a JWT from a Podman secret. - - Args: - secret_name: Name of the Podman secret. - - Returns: - The secret data string (JWT). - - Raises: - RuntimeError: If the secret doesn't exist or can't be read. - """ - assert self._podman_cli is not None # noqa: S101 - proc = await asyncio.create_subprocess_exec( - self._podman_cli, - "secret", - "inspect", - "--showsecret", - secret_name, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - ) - stdout, stderr = await proc.communicate() - if proc.returncode != 0: - raise RuntimeError( - f"Failed to extract JWT from Podman secret '{secret_name}': " - f"{stderr.decode().strip()}" - ) - try: - data = json.loads(stdout.decode()) - return data[0]["SecretData"] - except (json.JSONDecodeError, KeyError, IndexError) as exc: - raise RuntimeError(f"Failed to parse Podman secret '{secret_name}': {exc}") from exc - - async def _podman_exec(self, *args: str) -> None: - """Run a podman CLI command. - - Args: - *args: Arguments to pass after the podman binary path. - - Raises: - RuntimeError: If the command exits with a non-zero code. - """ - assert self._podman_cli is not None # noqa: S101 - proc = await asyncio.create_subprocess_exec( - self._podman_cli, - *args, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - ) - stdout, stderr = await proc.communicate() - if proc.returncode != 0: - raise RuntimeError( - f"podman {args[0]} failed (rc={proc.returncode}): " f"{stderr.decode().strip()}" - ) _PROVIDER_HOSTS: ClassVar[dict[str, str]] = { "openai": "api.openai.com", @@ -480,7 +314,6 @@ async def _do_spawn( env: dict[str, str], config: "SpawnConfig | None" = None, labels: dict[str, str] | None = None, - # TODO: Map to OpenShell equivalents for production skills_image: str | None = None, skills_paths: list[str] | None = None, service_account: str | None = None, @@ -489,36 +322,36 @@ async def _do_spawn( mcp_secret_mounts: list[tuple[str, str, str]] | None = None, tls_certs: "EphemeralCerts | None" = None, ) -> str: - """Create an OpenShell sandbox, start HTTP server, return endpoint. + """Create an OpenShell sandbox, start HTTP server, return endpoint.""" + from openshell._proto import openshell_pb2 - 1. Create sandbox with the given image and env vars. - 2. Wait for sandbox to be ready. - 3. Exec the HTTP server command (fire-and-forget background task). - 4. Return the network-local endpoint URL. + if service_account: + logger.info( + "OpenShell manages identity — service_account '%s' not applicable", + service_account, + ) - Returns: - HTTP endpoint URL of the sandbox service. - """ - from openshell._proto import openshell_pb2 + if tls_certs is not None: + logger.info( + "TLS certs not needed for OpenShell — gateway provides transport security", + ) - # Build sandbox name using our naming convention sandbox_name = f"ca-agent-{agent_name}" - # Construct SandboxSpec for the new API spec = openshell_pb2.SandboxSpec( template=openshell_pb2.SandboxTemplate( image=image, labels=labels or {}, ) ) - # Add environment variables for key, value in env.items(): spec.environment[key] = value - # Derive network policy from step config (provider, MCP servers) self._build_network_policy(spec, env) - # Create sandbox (sync call wrapped in thread) + if read_only: + self._build_filesystem_policy(spec) + sandbox_ref = await asyncio.to_thread(self._client.create, spec=spec) sandbox_name = sandbox_ref.name sandbox_id = sandbox_ref.id @@ -526,30 +359,32 @@ async def _do_spawn( self._sandbox_ids[agent_name] = sandbox_id try: - # Wait for sandbox to be ready await asyncio.to_thread( self._client.wait_ready, sandbox_name, timeout_seconds=300, ) - # Workaround for Podman 5.8.x secret file mount bug (issue #82). - if self._podman_cli is not None: - await self._inject_podman_token(sandbox_name) + if credential_secret_name: + await self._inject_credentials( + agent_name, sandbox_name, credential_secret_name, env, + ) + + if skills_image: + await self._load_skills( + agent_name, sandbox_id, skills_image, skills_paths, + ) + + if mcp_secret_mounts: + await self._inject_mcp_secrets(agent_name, mcp_secret_mounts, env) - # Start HTTP server via exec (fire-and-forget). - # exec_stream takes sandbox_id (UUID), not sandbox_name. await self.start_server(sandbox_id, _DEFAULT_SERVER_COMMAND, env=env) - # Expose the sandbox port via the gateway's HTTP proxy. endpoint, virtual_host = await self._expose_service( sandbox_name, port=8080, ) self._virtual_hosts[sandbox_name] = virtual_host - # Wait for the sandbox HTTP server to be ready. - # Done here (not in base wait_ready) so each parallel - # spawn uses its own virtual host — no shared state race. ready = await self._wait_ready_with_host( endpoint, virtual_host, timeout=60.0, ) @@ -565,17 +400,7 @@ async def _do_spawn( agent_name, exc_info=True, ) - try: - await asyncio.to_thread(self._client.delete, sandbox_name) - except Exception: - logger.warning( - "Failed to delete orphaned sandbox '%s' during cleanup", - sandbox_name, - exc_info=True, - ) - self._sandbox_names.pop(agent_name, None) - self._sandbox_ids.pop(agent_name, None) - self._virtual_hosts.pop(sandbox_name, None) + await self._cleanup_sandbox(agent_name, sandbox_name) raise logger.info( @@ -586,6 +411,271 @@ async def _do_spawn( ) return endpoint + async def _cleanup_sandbox( + self, agent_name: str, sandbox_name: str, + ) -> None: + """Clean up a sandbox and all associated resources on failure.""" + provider_id = self._provider_ids.pop(agent_name, None) + if provider_id: + try: + await self._detach_provider(sandbox_name, provider_id) + except Exception: + logger.warning( + "Failed to detach provider '%s' during cleanup", + provider_id, exc_info=True, + ) + try: + await asyncio.to_thread(self._client.delete, sandbox_name) + except Exception: + logger.warning( + "Failed to delete orphaned sandbox '%s' during cleanup", + sandbox_name, exc_info=True, + ) + self._sandbox_names.pop(agent_name, None) + self._sandbox_ids.pop(agent_name, None) + self._virtual_hosts.pop(sandbox_name, None) + + @staticmethod + def _build_filesystem_policy(spec: "openshell_pb2.SandboxSpec") -> None: + """Set read-only filesystem policy with write exceptions. + + Allows writes to agent workspace, skills, secrets, and log + directories so post-create injection still works in advisory mode. + """ + spec.policy.filesystem.read_only.append("/") + for rw_path in ( + "/tmp", + "/home/agent", + "/var/log", + "/app/skills", + "/var/secrets/mcp", + "/var/run/secrets/llm-credentials", + ): + spec.policy.filesystem.read_write.append(rw_path) + spec.policy.filesystem.include_workdir = True + + async def _inject_credentials( + self, + agent_name: str, + sandbox_name: str, + credential_secret_name: str, + env: dict[str, str], + ) -> None: + """Inject LLM credentials into the sandbox via Provider API. + + Uses the OpenShell Provider system for gateway-managed credentials. + Falls back to file-based injection if Provider RPCs fail. + """ + cred_value = env.get(credential_secret_name) + if not cred_value: + logger.warning( + "Credential '%s' not found in env for sandbox '%s'", + credential_secret_name, sandbox_name, + ) + return + + try: + provider_id = await self._create_and_attach_provider( + sandbox_name, + credentials={credential_secret_name: cred_value}, + ) + self._provider_ids[agent_name] = provider_id + logger.info( + "Attached credential provider '%s' to sandbox '%s'", + provider_id, sandbox_name, + ) + except Exception: + logger.warning( + "Provider API failed for '%s' — falling back to file injection", + sandbox_name, exc_info=True, + ) + await self._inject_credentials_via_files( + agent_name, credential_secret_name, cred_value, + ) + + async def _inject_credentials_via_files( + self, + agent_name: str, + credential_secret_name: str, + cred_value: str, + ) -> None: + """Write credential files to the sandbox filesystem. + + Handles both API-key providers (env var only) and file-backed + providers like Vertex (GOOGLE_APPLICATION_CREDENTIALS). + """ + cred_dir = "/var/run/secrets/llm-credentials" + sandbox_id = self._sandbox_ids[agent_name] + await self._exec_mkdir(sandbox_id, cred_dir) + file_path = f"{cred_dir}/{credential_secret_name}" + await self._do_write_file(agent_name, file_path, cred_value) + logger.info( + "Injected credential file '%s' into sandbox for agent '%s'", + file_path, agent_name, + ) + + async def _create_and_attach_provider( + self, + sandbox_name: str, + credentials: dict[str, str], + ) -> str: + """Create an OpenShell provider and attach it to a sandbox. + + Returns the provider ID for later cleanup. + """ + import grpc + from openshell._proto import openshell_pb2, openshell_pb2_grpc + + def _sync_provider() -> str: + grpc_target = self._client._endpoint + grpc_target = grpc_target.replace("http://", "").replace("https://", "") + channel = grpc.insecure_channel(grpc_target) + stub = openshell_pb2_grpc.OpenShellStub(channel) + + create_req = openshell_pb2.CreateProviderRequest( + provider=openshell_pb2.Provider( + type="cloud-agents", + credentials=credentials, + ), + ) + create_resp = stub.CreateProvider(create_req) + provider_id = create_resp.provider.id + + attach_req = openshell_pb2.AttachSandboxProviderRequest( + sandbox=sandbox_name, + provider=provider_id, + ) + stub.AttachSandboxProvider(attach_req) + channel.close() + return provider_id + + return await asyncio.to_thread(_sync_provider) + + async def _detach_provider( + self, sandbox_name: str, provider_id: str, + ) -> None: + """Detach a provider from a sandbox.""" + import grpc + from openshell._proto import openshell_pb2, openshell_pb2_grpc + + def _sync_detach() -> None: + grpc_target = self._client._endpoint + grpc_target = grpc_target.replace("http://", "").replace("https://", "") + channel = grpc.insecure_channel(grpc_target) + stub = openshell_pb2_grpc.OpenShellStub(channel) + req = openshell_pb2.DetachSandboxProviderRequest( + sandbox=sandbox_name, + provider=provider_id, + ) + stub.DetachSandboxProvider(req) + channel.close() + + await asyncio.to_thread(_sync_detach) + + async def _load_skills( + self, + agent_name: str, + sandbox_id: str, + skills_image: str, + skills_paths: list[str] | None = None, + ) -> None: + """Load skills from an OCI image into the sandbox. + + Extracts skills content from the image locally, then streams + it into the sandbox via exec_stream with tar. + """ + import shutil + import subprocess + import tarfile + from io import BytesIO + + copy_paths = skills_paths or ["/skills"] + tmp_dir = tempfile.mkdtemp(prefix=f"skills-{agent_name}-") + + try: + if self._podman_cli: + for src_path in copy_paths: + subprocess.run( + [ + self._podman_cli, "run", "--rm", + "-v", f"{tmp_dir}:/out:Z", + skills_image, + "sh", "-c", f"cp -r {src_path}/* /out/ 2>/dev/null || true", + ], + check=True, + capture_output=True, + timeout=120, + ) + else: + logger.warning( + "No podman_cli — attempting direct image pull for skills. " + "K8s environments should pre-populate skills.", + ) + return + + tar_buf = BytesIO() + with tarfile.open(fileobj=tar_buf, mode="w") as tar: + tar.add(tmp_dir, arcname=".") + tar_bytes = tar_buf.getvalue() + + def _sync_upload() -> None: + for _ in self._client.exec_stream( + sandbox_id, + ["tar", "xf", "-", "-C", "/app/skills"], + stdin=tar_bytes, + ): + pass + + await asyncio.to_thread(_sync_upload) + logger.info( + "Loaded skills into sandbox for agent '%s' from '%s'", + agent_name, skills_image, + ) + finally: + shutil.rmtree(tmp_dir, ignore_errors=True) + + async def _inject_mcp_secrets( + self, + agent_name: str, + mcp_secret_mounts: list[tuple[str, str, str]], + env: dict[str, str], + ) -> None: + """Inject MCP secret header files into the sandbox. + + Each mount tuple is (secret_name, key, mount_path) where + mount_path is the directory and key is the filename. + """ + sandbox_id = self._sandbox_ids.get(agent_name) + if not sandbox_id: + raise RuntimeError(f"No sandbox tracked for agent '{agent_name}'") + + for secret_name, key, mount_path in mcp_secret_mounts: + secret_value = env.get(secret_name, "") + if not secret_value: + logger.warning( + "MCP secret '%s' not found in env for agent '%s'", + secret_name, agent_name, + ) + continue + + await self._exec_mkdir(sandbox_id, mount_path) + file_path = f"{mount_path}{key}" + await self._do_write_file(agent_name, file_path, secret_value) + logger.info( + "Injected MCP secret '%s/%s' into sandbox for agent '%s'", + mount_path, key, agent_name, + ) + + async def _exec_mkdir(self, sandbox_id: str, path: str) -> None: + """Create a directory inside the sandbox.""" + def _sync_mkdir() -> None: + for _ in self._client.exec_stream( + sandbox_id, ["mkdir", "-p", path], + ): + pass + + await asyncio.to_thread(_sync_mkdir) + async def start_server( self, sandbox_name: str, @@ -802,17 +892,12 @@ def _sync_exec(): return content async def _do_destroy(self, agent_name: str) -> None: - """Delete the OpenShell sandbox and clean up background tasks. - - Args: - agent_name: Name of the agent to destroy. - """ + """Delete the OpenShell sandbox and clean up all resources.""" sandbox_name = self._sandbox_names.get(agent_name) if not sandbox_name: logger.warning("No sandbox name found for agent '%s'", agent_name) return - # Cancel server background task if running task = self._server_tasks.pop(sandbox_name, None) if task and not task.done(): task.cancel() @@ -821,23 +906,26 @@ async def _do_destroy(self, agent_name: str) -> None: except asyncio.CancelledError: pass + provider_id = self._provider_ids.pop(agent_name, None) + if provider_id: + try: + await self._detach_provider(sandbox_name, provider_id) + except Exception: + logger.warning( + "Failed to detach provider '%s' from sandbox '%s'", + provider_id, sandbox_name, exc_info=True, + ) + try: await asyncio.to_thread(self._client.delete, sandbox_name) logger.info("Destroyed OpenShell sandbox '%s' (agent=%s)", sandbox_name, agent_name) except Exception: - # Do NOT re-raise: base.destroy() always decrements _active_count - # in its finally block. Re-raising would cause a double-decrement - # on retry. Keep the _sandbox_names entry so the sandbox is visible - # via list_active() for manual cleanup or a subsequent destroy(). logger.warning( "Failed to destroy sandbox '%s' (agent=%s) — " "sandbox retained in _sandbox_names for manual cleanup", - sandbox_name, - agent_name, - exc_info=True, + sandbox_name, agent_name, exc_info=True, ) return - # Only remove tracking after successful delete sandbox_name = self._sandbox_names.pop(agent_name, None) self._sandbox_ids.pop(agent_name, None) if sandbox_name: diff --git a/src/cloud_agents/workflow/temporal_entrypoint.py b/src/cloud_agents/workflow/temporal_entrypoint.py index 01669d14..9c14ebe3 100644 --- a/src/cloud_agents/workflow/temporal_entrypoint.py +++ b/src/cloud_agents/workflow/temporal_entrypoint.py @@ -90,12 +90,11 @@ def _create_spawner(): from openshell import SandboxClient gateway_url = os.environ.get("OPENSHELL_GATEWAY_URL", "localhost:17670") - podman_cli = os.environ.get("OPENSHELL_PODMAN_CLI") # Strip http:// scheme — SandboxClient uses gRPC, not HTTP endpoint = gateway_url.replace("http://", "").replace("https://", "") client = SandboxClient(endpoint=endpoint) logger.info("Using OpenShellSpawner (gateway=%s)", gateway_url) - return OpenShellSpawner(openshell_client=client, podman_cli=podman_cli) + return OpenShellSpawner(openshell_client=client) logger.info("No spawner configured — sandbox activity will use stub mode") return None diff --git a/tests/unit/spawner/test_openshell_spawner.py b/tests/unit/spawner/test_openshell_spawner.py index ed877e97..b5995664 100644 --- a/tests/unit/spawner/test_openshell_spawner.py +++ b/tests/unit/spawner/test_openshell_spawner.py @@ -673,455 +673,23 @@ def test_returns_none_when_not_tracked(self, mocker: MockerFixture) -> None: assert spawner.get_sandbox_id("unknown") is None -class TestOpenShellSpawnerPodmanTokenWorkaround: - """Tests for Podman secret file mount workaround (issue #82). - - On Podman 5.8.x, the OpenShell Podman driver's secrets field is not - applied to the container. The workaround extracts the JWT from the - Podman secret and copies it into the container. - """ - - @pytest.mark.asyncio - async def test_workaround_disabled_when_podman_cli_not_set(self, mocker: MockerFixture) -> None: - """No workaround attempted when podman_cli is None.""" - from cloud_agents.spawner.openshell_spawner import OpenShellSpawner - - class SandboxRef: - id: str = "test-id" - def __init__(self, name): - self.name = name - - mock_client = mocker.Mock() - mock_client.create.return_value = SandboxRef("ca-agent-agent-1") - mock_client.wait_ready.return_value = SandboxRef("ca-agent-agent-1") - - def noop_exec(*args, **kwargs): - return iter([]) - - mock_client.exec_stream = noop_exec - - spawner = OpenShellSpawner(openshell_client=mock_client) - assert spawner._podman_cli is None - - # Mock _expose_service to return gateway endpoint and virtual host - mocker.patch.object( - spawner, - "_expose_service", - return_value=("http://gateway:17670", "sandbox.openshell.localhost"), - ) - - # Mock _wait_ready_with_host to return True immediately - async def mock_ready(*args, **kwargs): - return True - mocker.patch.object(spawner, "_wait_ready_with_host", side_effect=mock_ready) - - # Mock _build_network_policy (static method) - mocker.patch.object(OpenShellSpawner, "_build_network_policy") - - # Should not call any subprocess - mock_subprocess = mocker.patch( - "asyncio.create_subprocess_exec", new_callable=mocker.AsyncMock - ) - await spawner.spawn("agent-1", "sandbox:latest", env={}) - - mock_subprocess.assert_not_called() - - @pytest.mark.asyncio - async def test_workaround_enabled_when_podman_cli_set(self, mocker: MockerFixture) -> None: - """Workaround is called after create when podman_cli is set.""" - from cloud_agents.spawner.openshell_spawner import OpenShellSpawner - - class SandboxRef: - id: str = "test-id" - def __init__(self, name): - self.name = name - - mock_client = mocker.Mock() - mock_client.create.return_value = SandboxRef("ca-agent-agent-1") - mock_client.wait_ready.return_value = SandboxRef("ca-agent-agent-1") - - def noop_exec(*args, **kwargs): - return iter([]) - - mock_client.exec_stream = noop_exec - - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") - - # Mock _expose_service to return gateway endpoint and virtual host - mocker.patch.object( - spawner, - "_expose_service", - return_value=("http://gateway:17670", "sandbox.openshell.localhost"), - ) - - # Mock _wait_ready_with_host to return True immediately - async def mock_ready(*args, **kwargs): - return True - mocker.patch.object(spawner, "_wait_ready_with_host", side_effect=mock_ready) - - # Mock _build_network_policy (static method) - mocker.patch.object(OpenShellSpawner, "_build_network_policy") - - # Mock the workaround method to verify it's called - mock_workaround = mocker.patch.object( - spawner, "_inject_podman_token", new_callable=mocker.AsyncMock - ) - mock_workaround.return_value = True - - await spawner.spawn("agent-1", "sandbox:latest", env={}) - - mock_workaround.assert_called_once_with("ca-agent-agent-1") - - @pytest.mark.asyncio - async def test_inject_token_extracts_from_podman_secret(self, mocker: MockerFixture) -> None: - """_inject_podman_token reads the JWT from the Podman secret.""" - from cloud_agents.spawner.openshell_spawner import OpenShellSpawner - - mock_client = mocker.AsyncMock() - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") - - # Mock subprocess calls: first is container lookup, second is secret inspect - secret_data = json.dumps([{"SecretData": "eyJhbGciOiJFZERTQSJ9.test.signature"}]) - container_proc = mocker.AsyncMock() - container_proc.returncode = 0 - container_proc.communicate.return_value = ( - b"openshell-sandbox-my-sandbox\n", - b"", - ) - - default_proc = mocker.AsyncMock() - default_proc.returncode = 0 - default_proc.communicate.return_value = (secret_data.encode(), b"") - - # Track all subprocess calls - calls: list[tuple] = [] - call_index = 0 - - async def mock_create_subprocess(*args, **kwargs): - nonlocal call_index - calls.append(args) - call_index += 1 - if call_index == 1: - return container_proc - return default_proc - - mocker.patch( - "asyncio.create_subprocess_exec", - side_effect=mock_create_subprocess, - ) - - await spawner._inject_podman_token("sb-123") - - # First call is container lookup (podman ps) - assert calls[0][0] == "/usr/bin/podman" - assert "ps" in calls[0] - - # Second call should be podman secret inspect - assert calls[1][0] == "/usr/bin/podman" - assert "secret" in calls[1] - assert "inspect" in calls[1] - assert "openshell-token-sb-123" in calls[1] - - @pytest.mark.asyncio - async def test_inject_token_copies_file_into_container(self, mocker: MockerFixture) -> None: - """_inject_podman_token copies the JWT file into the container.""" - from cloud_agents.spawner.openshell_spawner import OpenShellSpawner - - mock_client = mocker.AsyncMock() - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") - - secret_data = json.dumps([{"SecretData": "test.jwt.token"}]) - mock_proc = mocker.AsyncMock() - mock_proc.returncode = 0 - mock_proc.communicate.return_value = (secret_data.encode(), b"") - - calls: list[tuple] = [] - - async def mock_create_subprocess(*args, **kwargs): - calls.append(args) - return mock_proc - - mocker.patch( - "asyncio.create_subprocess_exec", - side_effect=mock_create_subprocess, - ) - - await spawner._inject_podman_token("sb-123") - - # Should have calls for: find container, secret inspect, stop, cp, start - # Verify a cp call exists that targets the token path - cp_calls = [c for c in calls if "cp" in c and "/etc/openshell/auth/sandbox.jwt" in str(c)] - assert len(cp_calls) >= 1, f"Expected a podman cp call, got: {calls}" - - @pytest.mark.asyncio - async def test_inject_token_restarts_container(self, mocker: MockerFixture) -> None: - """_inject_podman_token stops then starts the container.""" - from cloud_agents.spawner.openshell_spawner import OpenShellSpawner - - mock_client = mocker.AsyncMock() - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") - - secret_data = json.dumps([{"SecretData": "test.jwt.token"}]) - mock_proc = mocker.AsyncMock() - mock_proc.returncode = 0 - mock_proc.communicate.return_value = (secret_data.encode(), b"") - - # For the container name lookup, return a container name - container_name_proc = mocker.AsyncMock() - container_name_proc.returncode = 0 - container_name_proc.communicate.return_value = ( - b"openshell-sandbox-my-sandbox\n", - b"", - ) - - call_index = 0 - calls: list[tuple] = [] - - async def mock_create_subprocess(*args, **kwargs): - nonlocal call_index - calls.append(args) - call_index += 1 - # First call is container name lookup - if call_index == 1: - return container_name_proc - return mock_proc - - mocker.patch( - "asyncio.create_subprocess_exec", - side_effect=mock_create_subprocess, - ) - - await spawner._inject_podman_token("sb-123") - - # Verify stop and start calls exist - stop_calls = [c for c in calls if "stop" in c] - start_calls = [c for c in calls if "start" in c] - assert len(stop_calls) >= 1, f"Expected a podman stop call, got: {calls}" - assert len(start_calls) >= 1, f"Expected a podman start call, got: {calls}" - - @pytest.mark.asyncio - async def test_inject_token_raises_on_secret_not_found(self, mocker: MockerFixture) -> None: - """_inject_podman_token raises RuntimeError if secret doesn't exist.""" - from cloud_agents.spawner.openshell_spawner import OpenShellSpawner - - mock_client = mocker.AsyncMock() - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") - - # Container name lookup succeeds - container_proc = mocker.AsyncMock() - container_proc.returncode = 0 - container_proc.communicate.return_value = ( - b"openshell-sandbox-my-sandbox\n", - b"", - ) - - # Secret inspect fails - secret_proc = mocker.AsyncMock() - secret_proc.returncode = 1 - secret_proc.communicate.return_value = ( - b"", - b"Error: no secret with name or id openshell-token-sb-999", - ) - - call_index = 0 - - async def mock_create_subprocess(*args, **kwargs): - nonlocal call_index - call_index += 1 - if call_index == 1: - return container_proc - return secret_proc - - mocker.patch( - "asyncio.create_subprocess_exec", - side_effect=mock_create_subprocess, - ) - - with pytest.raises(RuntimeError, match="Failed to extract.*secret"): - await spawner._inject_podman_token("sb-999") - - @pytest.mark.asyncio - async def test_inject_token_raises_on_container_not_found(self, mocker: MockerFixture) -> None: - """_inject_podman_token raises RuntimeError if container not found.""" - from cloud_agents.spawner.openshell_spawner import OpenShellSpawner - - mock_client = mocker.AsyncMock() - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") - - # Container lookup returns empty (no matching container) - mock_proc = mocker.AsyncMock() - mock_proc.returncode = 0 - mock_proc.communicate.return_value = (b"\n", b"") - - mocker.patch( - "asyncio.create_subprocess_exec", - return_value=mock_proc, - ) - - with pytest.raises(RuntimeError, match="No container found"): - await spawner._inject_podman_token("sb-999") - - @pytest.mark.asyncio - async def test_inject_token_cleans_up_temp_file(self, mocker: MockerFixture) -> None: - """_inject_podman_token removes the temp file after copy.""" - from cloud_agents.spawner.openshell_spawner import OpenShellSpawner - - mock_client = mocker.AsyncMock() - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") - - secret_data = json.dumps([{"SecretData": "test.jwt.token"}]) - - mock_proc = mocker.AsyncMock() - mock_proc.returncode = 0 - mock_proc.communicate.return_value = (secret_data.encode(), b"") - - container_proc = mocker.AsyncMock() - container_proc.returncode = 0 - container_proc.communicate.return_value = ( - b"openshell-sandbox-my-sandbox\n", - b"", - ) - - call_index = 0 - - async def mock_create_subprocess(*args, **kwargs): - nonlocal call_index - call_index += 1 - if call_index == 1: - return container_proc - return mock_proc - - mocker.patch( - "asyncio.create_subprocess_exec", - side_effect=mock_create_subprocess, - ) - - mock_unlink = mocker.patch("os.unlink") - - await spawner._inject_podman_token("sb-123") - - # os.unlink must be called to clean up the temp file - mock_unlink.assert_called_once() - # The path should end with .jwt (our suffix) - unlink_path = mock_unlink.call_args[0][0] - assert unlink_path.endswith(".jwt"), f"Expected .jwt suffix, got: {unlink_path}" - - @pytest.mark.asyncio - async def test_inject_token_cleans_up_on_stop_failure(self, mocker: MockerFixture) -> None: - """Temp file is cleaned up even when podman stop fails mid-flow.""" - from cloud_agents.spawner.openshell_spawner import OpenShellSpawner - - mock_client = mocker.AsyncMock() - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") - - secret_data = json.dumps([{"SecretData": "test.jwt.token"}]) - - # Container lookup succeeds - container_proc = mocker.AsyncMock() - container_proc.returncode = 0 - container_proc.communicate.return_value = ( - b"openshell-sandbox-my-sandbox\n", - b"", - ) - - # Secret extract succeeds - secret_proc = mocker.AsyncMock() - secret_proc.returncode = 0 - secret_proc.communicate.return_value = (secret_data.encode(), b"") - - # Stop fails - stop_proc = mocker.AsyncMock() - stop_proc.returncode = 1 - stop_proc.communicate.return_value = (b"", b"container not running") - - call_index = 0 - - async def mock_create_subprocess(*args, **kwargs): - nonlocal call_index - call_index += 1 - if call_index == 1: - return container_proc - if call_index == 2: - return secret_proc - return stop_proc # podman stop fails - - mocker.patch( - "asyncio.create_subprocess_exec", - side_effect=mock_create_subprocess, - ) - - mock_unlink = mocker.patch("os.unlink") - - with pytest.raises(RuntimeError, match="podman stop failed"): - await spawner._inject_podman_token("sb-123") - - # Even though stop failed, temp file must be cleaned up - mock_unlink.assert_called_once() - unlink_path = mock_unlink.call_args[0][0] - assert unlink_path.endswith(".jwt") - - @pytest.mark.asyncio - async def test_spawn_proceeds_after_successful_workaround(self, mocker: MockerFixture) -> None: - """After workaround, spawn continues to start server and return endpoint.""" - from cloud_agents.spawner.openshell_spawner import OpenShellSpawner - - class SandboxRef: - id: str = "test-id" - def __init__(self, name): - self.name = name - - mock_client = mocker.Mock() - mock_client.create.return_value = SandboxRef("ca-agent-agent-1") - mock_client.wait_ready.return_value = SandboxRef("ca-agent-agent-1") - - def noop_exec(*args, **kwargs): - return iter([]) - - mock_client.exec_stream = noop_exec - - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") - - # Mock _expose_service to return gateway endpoint and virtual host - mocker.patch.object( - spawner, - "_expose_service", - return_value=("http://gateway:17670", "sandbox.openshell.localhost"), - ) - - # Mock _wait_ready_with_host to return True immediately - async def mock_ready(*args, **kwargs): - return True - mocker.patch.object(spawner, "_wait_ready_with_host", side_effect=mock_ready) - - # Mock _build_network_policy (static method) - mocker.patch.object(OpenShellSpawner, "_build_network_policy") - - # Mock the workaround - mocker.patch.object( - spawner, - "_inject_podman_token", - new_callable=mocker.AsyncMock, - return_value=True, - ) - - endpoint = await spawner.spawn("agent-1", "sandbox:latest", env={}) - - assert endpoint == "http://gateway:17670" - mock_client.create.assert_called_once() - mock_client.wait_ready.assert_called_once() + # JWT workaround tests removed — issue #82 workaround dropped. + # OpenShell v0.0.79+ (PR NVIDIA/OpenShell#2156) delivers sandbox + # JWTs via Podman secrets natively when gateway_jwt is configured. + # See spawner docstring for history. class TestOpenShellSpawnerPostCreateCleanup: """Tests for sandbox cleanup when post-create steps fail in _do_spawn. - Regression tests for the orphaned sandbox bug: if _inject_podman_token(), - start_server(), or expose_service() fails after create_sandbox() succeeds, + Regression tests for the orphaned sandbox bug: if start_server(), + expose_service(), or wait_ready fails after create_sandbox() succeeds, the sandbox must be deleted and removed from _sandbox_ids. """ @pytest.mark.asyncio async def test_inject_token_failure_deletes_sandbox(self, mocker: MockerFixture) -> None: - """If _inject_podman_token raises, sandbox is deleted and tracking removed.""" + """If wait_ready raises, sandbox is deleted and tracking removed.""" from cloud_agents.spawner.openshell_spawner import OpenShellSpawner class SandboxRef: @@ -1133,7 +701,7 @@ def __init__(self, name): mock_client.create.return_value = SandboxRef("ca-agent-agent-1") mock_client.wait_ready.return_value = SandboxRef("ca-agent-agent-1") - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") + spawner = OpenShellSpawner(openshell_client=mock_client, ) # Mock _expose_service to return gateway endpoint and virtual host mocker.patch.object( @@ -1145,15 +713,15 @@ def __init__(self, name): # Mock _build_network_policy (static method) mocker.patch.object(OpenShellSpawner, "_build_network_policy") - # _inject_podman_token fails after create succeeds + # start_server fails after create + wait_ready succeed mocker.patch.object( spawner, - "_inject_podman_token", + "start_server", new_callable=mocker.AsyncMock, - side_effect=RuntimeError("podman stop failed (rc=1): container not running"), + side_effect=RuntimeError("exec failed"), ) - with pytest.raises(RuntimeError, match="podman stop failed"): + with pytest.raises(RuntimeError, match="exec failed"): await spawner.spawn("agent-1", "sandbox:latest", env={}) # Sandbox must be cleaned up @@ -1166,7 +734,7 @@ def __init__(self, name): async def test_inject_token_failure_propagates_original_exception( self, mocker: MockerFixture ) -> None: - """The original exception from _inject_podman_token propagates to the caller.""" + """The original exception from wait_ready propagates to the caller.""" from cloud_agents.spawner.openshell_spawner import OpenShellSpawner class SandboxRef: @@ -1178,7 +746,7 @@ def __init__(self, name): mock_client.create.return_value = SandboxRef("ca-agent-agent-1") mock_client.wait_ready.return_value = SandboxRef("ca-agent-agent-1") - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") + spawner = OpenShellSpawner(openshell_client=mock_client, ) # Mock _expose_service to return gateway endpoint and virtual host mocker.patch.object( @@ -1192,7 +760,7 @@ def __init__(self, name): mocker.patch.object( spawner, - "_inject_podman_token", + "start_server", new_callable=mocker.AsyncMock, side_effect=RuntimeError("No container found for sandbox 'ca-agent-agent-1'"), ) @@ -1247,7 +815,7 @@ def __init__(self, name): mock_client.wait_ready.return_value = SandboxRef("ca-agent-agent-1") mock_client.delete.side_effect = RuntimeError("API unreachable") - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") + spawner = OpenShellSpawner(openshell_client=mock_client, ) # Mock _expose_service to return gateway endpoint and virtual host mocker.patch.object( @@ -1261,7 +829,7 @@ def __init__(self, name): mocker.patch.object( spawner, - "_inject_podman_token", + "start_server", new_callable=mocker.AsyncMock, side_effect=RuntimeError("token injection failed"), ) @@ -1289,7 +857,7 @@ def __init__(self, name): mock_client.create.return_value = SandboxRef("ca-agent-agent-1") mock_client.wait_ready.return_value = SandboxRef("ca-agent-agent-1") - spawner = OpenShellSpawner(openshell_client=mock_client, podman_cli="/usr/bin/podman") + spawner = OpenShellSpawner(openshell_client=mock_client, ) # Mock _expose_service to return gateway endpoint and virtual host mocker.patch.object( @@ -1303,7 +871,7 @@ def __init__(self, name): mocker.patch.object( spawner, - "_inject_podman_token", + "start_server", new_callable=mocker.AsyncMock, side_effect=RuntimeError("injection failed"), ) @@ -1317,6 +885,271 @@ def __init__(self, name): assert spawner.active_count == 0 +class TestFilesystemPolicy: + """Tests for _build_filesystem_policy() static method.""" + + def test_sets_read_only_root(self, mocker: MockerFixture) -> None: + """Read-only policy includes root filesystem.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + spec = mocker.Mock() + spec.policy.filesystem.read_only = [] + spec.policy.filesystem.read_write = [] + spec.policy.filesystem.include_workdir = False + + OpenShellSpawner._build_filesystem_policy(spec) + + assert "/" in spec.policy.filesystem.read_only + + def test_allows_write_to_injection_targets(self, mocker: MockerFixture) -> None: + """Read-write list includes all post-create injection paths.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + spec = mocker.Mock() + spec.policy.filesystem.read_only = [] + spec.policy.filesystem.read_write = [] + spec.policy.filesystem.include_workdir = False + + OpenShellSpawner._build_filesystem_policy(spec) + + rw = spec.policy.filesystem.read_write + assert "/tmp" in rw + assert "/home/agent" in rw + assert "/var/log" in rw + assert "/app/skills" in rw + assert "/var/secrets/mcp" in rw + assert "/var/run/secrets/llm-credentials" in rw + + def test_includes_workdir(self, mocker: MockerFixture) -> None: + """Filesystem policy sets include_workdir.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + spec = mocker.Mock() + spec.policy.filesystem.read_only = [] + spec.policy.filesystem.read_write = [] + spec.policy.filesystem.include_workdir = False + + OpenShellSpawner._build_filesystem_policy(spec) + + assert spec.policy.filesystem.include_workdir is True + + +class TestCredentialInjection: + """Tests for _inject_credentials() and Provider API integration.""" + + @pytest.mark.asyncio + async def test_creates_and_attaches_provider(self, mocker: MockerFixture) -> None: + """Credentials are injected via Provider API when available.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + mock_client = mocker.Mock() + spawner = OpenShellSpawner(openshell_client=mock_client) + spawner._sandbox_ids["agent-1"] = "uuid-1" + + mock_create = mocker.patch.object( + spawner, "_create_and_attach_provider", + return_value="provider-123", + ) + + await spawner._inject_credentials( + "agent-1", "sb-1", "OPENAI_API_KEY", + {"OPENAI_API_KEY": "sk-test"}, + ) + + mock_create.assert_called_once_with( + "sb-1", credentials={"OPENAI_API_KEY": "sk-test"}, + ) + assert spawner._provider_ids["agent-1"] == "provider-123" + + @pytest.mark.asyncio + async def test_falls_back_to_file_injection(self, mocker: MockerFixture) -> None: + """Falls back to file injection when Provider API fails.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + mock_client = mocker.Mock() + spawner = OpenShellSpawner(openshell_client=mock_client) + spawner._sandbox_ids["agent-1"] = "uuid-1" + + mocker.patch.object( + spawner, "_create_and_attach_provider", + side_effect=Exception("gRPC unavailable"), + ) + mock_file_inject = mocker.patch.object( + spawner, "_inject_credentials_via_files", + ) + + await spawner._inject_credentials( + "agent-1", "sb-1", "OPENAI_API_KEY", + {"OPENAI_API_KEY": "sk-test"}, + ) + + mock_file_inject.assert_called_once_with( + "agent-1", "OPENAI_API_KEY", "sk-test", + ) + + @pytest.mark.asyncio + async def test_skips_when_credential_not_in_env(self, mocker: MockerFixture) -> None: + """Logs warning when credential key not found in env.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + mock_client = mocker.Mock() + spawner = OpenShellSpawner(openshell_client=mock_client) + + await spawner._inject_credentials("agent-1", "sb-1", "MISSING_KEY", {}) + + assert "agent-1" not in spawner._provider_ids + + +class TestMCPSecretInjection: + """Tests for _inject_mcp_secrets() file injection.""" + + @pytest.mark.asyncio + async def test_writes_to_correct_path(self, mocker: MockerFixture) -> None: + """MCP secrets are written to mount_path + key, not mount_path alone.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + mock_client = mocker.Mock() + mock_client.exec_stream.return_value = iter([]) + + spawner = OpenShellSpawner(openshell_client=mock_client) + spawner._sandbox_ids["agent-1"] = "uuid-1" + + mock_mkdir = mocker.patch.object(spawner, "_exec_mkdir") + mock_write = mocker.patch.object(spawner, "_do_write_file") + + mounts = [("my-secret", "api-key", "/var/secrets/mcp/kubectl/")] + + await spawner._inject_mcp_secrets( + "agent-1", mounts, {"my-secret": "secret-value"}, + ) + + mock_mkdir.assert_called_once_with("uuid-1", "/var/secrets/mcp/kubectl/") + mock_write.assert_called_once_with( + "agent-1", "/var/secrets/mcp/kubectl/api-key", "secret-value", + ) + + @pytest.mark.asyncio + async def test_skips_missing_secrets(self, mocker: MockerFixture) -> None: + """Logs warning and skips when secret not in env.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + mock_client = mocker.Mock() + spawner = OpenShellSpawner(openshell_client=mock_client) + spawner._sandbox_ids["agent-1"] = "uuid-1" + + mock_write = mocker.patch.object(spawner, "_do_write_file") + + mounts = [("missing-secret", "key", "/var/secrets/mcp/s/")] + + await spawner._inject_mcp_secrets("agent-1", mounts, {}) + + mock_write.assert_not_called() + + +class TestTLSAndServiceAccountSkipped: + """Tests for TLS and service_account skip-with-info-log.""" + + @pytest.mark.asyncio + async def test_tls_logged_and_skipped(self, mocker: MockerFixture) -> None: + """TLS certs trigger info log but are not injected.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + mock_client = mocker.Mock() + mock_client.create.return_value = mocker.Mock(name="sb-1", id="uuid-1") + mock_client.wait_ready = mocker.Mock() + mock_client.exec_stream.return_value = iter([]) + + spawner = OpenShellSpawner(openshell_client=mock_client) + mocker.patch.object(spawner, "_build_network_policy") + mocker.patch.object(spawner, "_expose_service", return_value=("http://gw:8080", "vh")) + mocker.patch.object(spawner, "_wait_ready_with_host", return_value=True) + mocker.patch.object(spawner, "start_server") + + tls = mocker.Mock() + mock_logger = mocker.patch("cloud_agents.spawner.openshell_spawner.logger") + + await spawner._do_spawn( + "agent-1", "image:latest", env={}, tls_certs=tls, + ) + + info_calls = [ + str(c) for c in mock_logger.info.call_args_list + if "TLS" in str(c) or "transport security" in str(c) + ] + assert len(info_calls) >= 1 + + @pytest.mark.asyncio + async def test_service_account_logged_and_skipped(self, mocker: MockerFixture) -> None: + """Service account triggers info log but is not applied.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + mock_client = mocker.Mock() + mock_client.create.return_value = mocker.Mock(name="sb-1", id="uuid-1") + mock_client.wait_ready = mocker.Mock() + mock_client.exec_stream.return_value = iter([]) + + spawner = OpenShellSpawner(openshell_client=mock_client) + mocker.patch.object(spawner, "_build_network_policy") + mocker.patch.object(spawner, "_expose_service", return_value=("http://gw:8080", "vh")) + mocker.patch.object(spawner, "_wait_ready_with_host", return_value=True) + mocker.patch.object(spawner, "start_server") + + mock_logger = mocker.patch("cloud_agents.spawner.openshell_spawner.logger") + + await spawner._do_spawn( + "agent-1", "image:latest", env={}, service_account="my-sa", + ) + + info_calls = [ + str(c) for c in mock_logger.info.call_args_list + if "service_account" in str(c) or "identity" in str(c) + ] + assert len(info_calls) >= 1 + + +class TestDestroyWithProviderCleanup: + """Tests for _do_destroy() provider cleanup.""" + + @pytest.mark.asyncio + async def test_detaches_provider_on_destroy(self, mocker: MockerFixture) -> None: + """Provider is detached during destroy.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + mock_client = mocker.Mock() + spawner = OpenShellSpawner(openshell_client=mock_client) + spawner._sandbox_names["agent-1"] = "sb-1" + spawner._sandbox_ids["agent-1"] = "uuid-1" + spawner._provider_ids["agent-1"] = "provider-123" + + mock_detach = mocker.patch.object(spawner, "_detach_provider") + mock_client.delete = mocker.Mock() + + await spawner._do_destroy("agent-1") + + mock_detach.assert_called_once_with("sb-1", "provider-123") + assert "agent-1" not in spawner._provider_ids + + @pytest.mark.asyncio + async def test_destroy_tolerates_detach_failure(self, mocker: MockerFixture) -> None: + """Destroy continues even if provider detach fails.""" + from cloud_agents.spawner.openshell_spawner import OpenShellSpawner + + mock_client = mocker.Mock() + spawner = OpenShellSpawner(openshell_client=mock_client) + spawner._sandbox_names["agent-1"] = "sb-1" + spawner._sandbox_ids["agent-1"] = "uuid-1" + spawner._provider_ids["agent-1"] = "provider-123" + + mocker.patch.object( + spawner, "_detach_provider", side_effect=Exception("detach failed"), + ) + mock_client.delete = mocker.Mock() + + await spawner._do_destroy("agent-1") + + mock_client.delete.assert_called_once_with("sb-1") + + class TestBuildNetworkPolicy: """Tests for _build_network_policy() static method.""" From 1903090e81ee7f82b9bfc3a35d5ea0905aed45d5 Mon Sep 17 00:00:00 2001 From: James Wong <2421248+jameswnl@users.noreply.github.com> Date: Sat, 11 Jul 2026 15:18:20 -0400 Subject: [PATCH 2/8] Fix review findings: cross-platform skills, credential validation, _podman_cli init beesarmy: - Skills image extraction now cross-platform: uses Podman Python SDK (works in any container) with CLI fallback. Raises RuntimeError if neither is available instead of silently skipping. CodeRabbit: - Initialize _podman_cli in __init__ to prevent AttributeError - Missing credentials now raise RuntimeError instead of warn+return Co-Authored-By: Claude Opus 4.6 (1M context) --- src/cloud_agents/spawner/openshell_spawner.py | 101 +++++++++++++----- tests/unit/spawner/test_openshell_spawner.py | 9 +- 2 files changed, 81 insertions(+), 29 deletions(-) diff --git a/src/cloud_agents/spawner/openshell_spawner.py b/src/cloud_agents/spawner/openshell_spawner.py index b8e4b65d..7443189e 100644 --- a/src/cloud_agents/spawner/openshell_spawner.py +++ b/src/cloud_agents/spawner/openshell_spawner.py @@ -89,6 +89,7 @@ def __init__( """ super().__init__(**kwargs) self._client = openshell_client + self._podman_cli: str | None = None self._sandbox_names: dict[str, str] = {} self._sandbox_ids: dict[str, str] = {} self._virtual_hosts: dict[str, str] = {} @@ -468,11 +469,10 @@ async def _inject_credentials( """ cred_value = env.get(credential_secret_name) if not cred_value: - logger.warning( - "Credential '%s' not found in env for sandbox '%s'", - credential_secret_name, sandbox_name, + raise RuntimeError( + f"Credential '{credential_secret_name}' not found in env " + f"for sandbox '{sandbox_name}' — cannot start agent without credentials" ) - return try: provider_id = await self._create_and_attach_provider( @@ -583,9 +583,14 @@ async def _load_skills( Extracts skills content from the image locally, then streams it into the sandbox via exec_stream with tar. + + Cross-platform: uses the Podman Python SDK (available in the + runner container) to run a transient container that copies + skills into a temp dir. Falls back to podman CLI if the SDK + is not available. The sandbox-side tar upload is the same + regardless of extraction method. """ import shutil - import subprocess import tarfile from io import BytesIO @@ -593,25 +598,9 @@ async def _load_skills( tmp_dir = tempfile.mkdtemp(prefix=f"skills-{agent_name}-") try: - if self._podman_cli: - for src_path in copy_paths: - subprocess.run( - [ - self._podman_cli, "run", "--rm", - "-v", f"{tmp_dir}:/out:Z", - skills_image, - "sh", "-c", f"cp -r {src_path}/* /out/ 2>/dev/null || true", - ], - check=True, - capture_output=True, - timeout=120, - ) - else: - logger.warning( - "No podman_cli — attempting direct image pull for skills. " - "K8s environments should pre-populate skills.", - ) - return + await self._extract_skills_image( + skills_image, copy_paths, tmp_dir, + ) tar_buf = BytesIO() with tarfile.open(fileobj=tar_buf, mode="w") as tar: @@ -634,6 +623,70 @@ def _sync_upload() -> None: finally: shutil.rmtree(tmp_dir, ignore_errors=True) + async def _extract_skills_image( + self, + skills_image: str, + copy_paths: list[str], + tmp_dir: str, + ) -> None: + """Extract skills content from an OCI image to a local directory. + + Uses the Podman Python SDK (cross-platform, works in any + container) or falls back to the podman CLI binary. + """ + try: + from podman import PodmanClient + + def _sdk_extract() -> None: + pclient = PodmanClient( + base_url=f"unix://{os.environ.get('CONTAINER_HOST', '/run/podman/podman.sock').replace('unix://', '')}", + ) + copy_cmd = " && ".join( + f"cp -r {p}/* /out/ 2>/dev/null || true" + for p in copy_paths + ) + pclient.containers.run( + skills_image, + command=["sh", "-c", copy_cmd], + volumes={tmp_dir: {"bind": "/out", "mode": "rw"}}, + remove=True, + detach=False, + ) + pclient.close() + + await asyncio.to_thread(_sdk_extract) + return + except ImportError: + pass + except Exception: + logger.warning( + "Podman SDK skills extraction failed, trying CLI", + exc_info=True, + ) + + if self._podman_cli: + import subprocess + + for src_path in copy_paths: + await asyncio.to_thread( + subprocess.run, + [ + self._podman_cli, "run", "--rm", + "-v", f"{tmp_dir}:/out:Z", + skills_image, + "sh", "-c", f"cp -r {src_path}/* /out/ 2>/dev/null || true", + ], + check=True, + capture_output=True, + timeout=120, + ) + return + + raise RuntimeError( + f"Cannot extract skills image '{skills_image}': " + "neither Podman SDK nor podman CLI available" + ) + async def _inject_mcp_secrets( self, agent_name: str, diff --git a/tests/unit/spawner/test_openshell_spawner.py b/tests/unit/spawner/test_openshell_spawner.py index b5995664..36f6be8a 100644 --- a/tests/unit/spawner/test_openshell_spawner.py +++ b/tests/unit/spawner/test_openshell_spawner.py @@ -988,16 +988,15 @@ async def test_falls_back_to_file_injection(self, mocker: MockerFixture) -> None ) @pytest.mark.asyncio - async def test_skips_when_credential_not_in_env(self, mocker: MockerFixture) -> None: - """Logs warning when credential key not found in env.""" + async def test_raises_when_credential_not_in_env(self, mocker: MockerFixture) -> None: + """Raises RuntimeError when credential key not found in env.""" from cloud_agents.spawner.openshell_spawner import OpenShellSpawner mock_client = mocker.Mock() spawner = OpenShellSpawner(openshell_client=mock_client) - await spawner._inject_credentials("agent-1", "sb-1", "MISSING_KEY", {}) - - assert "agent-1" not in spawner._provider_ids + with pytest.raises(RuntimeError, match="not found in env"): + await spawner._inject_credentials("agent-1", "sb-1", "MISSING_KEY", {}) class TestMCPSecretInjection: From 54b340042e6dcdc4019f3f61e1056d10e4dfda32 Mon Sep 17 00:00:00 2001 From: James Wong <2421248+jameswnl@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:12:00 -0400 Subject: [PATCH 3/8] RSPEED-0000: Skills image via Podman driver_config + fragility analysis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skills image handling per OpenShell driver: - Podman driver: native image mount via SandboxSpec.template.driver_config (no extraction, no tar streaming — Podman mounts OCI image directly) - K8s driver: tar streaming fallback with crane → Podman SDK → CLI chain - New OPENSHELL_DRIVER env var ("podman"|"kubernetes") selects the path Spike doc updates: - Fragility analysis for all 7 production features - Updated Resolved section (JWT workaround removed, production gaps closed) - Spawner comparison table with accurate skills_image status - OpenShell convention: bake skills into sandbox image for production (FROM scratch + COPY skills — per NVIDIA community sandboxes docs) Refs #106 Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/spikes/openshell-spawner-spike.md | 117 +++++++++++++++++- src/cloud_agents/spawner/openshell_spawner.py | 87 ++++++++++++- .../workflow/temporal_entrypoint.py | 5 +- 3 files changed, 195 insertions(+), 14 deletions(-) diff --git a/docs/spikes/openshell-spawner-spike.md b/docs/spikes/openshell-spawner-spike.md index 0c5f843e..818ee014 100644 --- a/docs/spikes/openshell-spawner-spike.md +++ b/docs/spikes/openshell-spawner-spike.md @@ -305,11 +305,13 @@ Additional changes: ### Resolved -1. **RESOLVED**: Standalone Podman gateway works on macOS with JWT config. Podman secret file mount is broken in 5.8.x but automated workaround implemented (issue #82). +1. **RESOLVED**: Standalone Podman gateway works on macOS with JWT config. 2. **RESOLVED**: Supervisor auth chain works end-to-end with Podman driver (JWT minting, supervisor authentication). -3. **RESOLVED**: Podman secret file mount workaround implemented in `openshell_spawner.py`. +3. **RESOLVED**: Podman 5.8.x JWT workaround — **removed**. OpenShell v0.0.79+ (PR NVIDIA/OpenShell#2156) delivers sandbox JWTs natively via Podman secrets when `gateway_jwt` is configured with signing keys. No client-side workaround needed. Requires gateway v0.0.79+. 4. **RESOLVED**: RHEL 9 verification — gateway + supervisor + sandbox lifecycle works. Seccomp and network namespace isolation confirmed. 5. **RESOLVED**: UBI 9 sandbox images — requires source-built supervisor (glibc 2.34 compatibility). UBI 10 works with pre-built binaries. +6. **RESOLVED**: Production gaps closed (PR #105) — skills image, credentials, filesystem policy, MCP secrets, TLS skip, service account skip, provider cleanup. All 7 example workflows pass E2E through OpenShell on RHEL 9 + Kind cluster. +7. **RESOLVED**: ~~Containerized gateway deployment (DooD pattern)~~ — verified (see "Containerized Gateway" section). ### Remaining @@ -317,19 +319,122 @@ Additional changes: 2. Gateway resource overhead measurement in target deployment environments 3. L7 network policy testing with live gateway (auto-derivation implemented in `_build_network_policy()`) 4. Sandbox image must include `iproute2`, `nsenter` (`util-linux-core`), and a `sandbox` user -5. ~~Containerized gateway deployment (DooD pattern)~~ — verified above (see "Containerized Gateway" section) -6. Upstream: request musl-static supervisor binary or RHEL 9-compatible builds +5. Upstream: request musl-static supervisor binary or RHEL 9-compatible builds +6. Skills image extraction for OpenShell + K8s driver (issue #106, see Fragility Analysis below) **Immediate value**: Eliminates duplicated spawner code and provides defense-in-depth isolation (seccomp + network namespace). Even without L7 policy, this is a security improvement over container securityContext alone. **If no-go**: The prototype code remains as a third spawner option. No changes to existing K8s/Podman spawners. +## Fragility Analysis + +Assessment of each production feature's robustness, what we've done, and known gaps. + +### Skills image — moderate fragility + +**What we've done**: `_load_skills()` extracts skills from an OCI image locally (Podman SDK or CLI), creates a tar, streams it into the sandbox via `exec_stream`. Works for OpenShell + Podman driver. + +**Fragility**: +- **OpenShell + K8s driver**: Runner is a K8s pod with no Podman socket. Neither Podman SDK nor CLI is available. `_extract_skills_image()` raises `RuntimeError`. Issue #106 tracks this. +- **Full image pull**: `crane export` or Podman SDK downloads ALL layers of the skills image, not just the `/skills` directory. A fat base image (e.g. `python:3.12` + 5MB of skills) wastes bandwidth. + +**Mitigation**: Build skills images as minimal single-layer images (`FROM scratch` + `COPY skills/ /skills/`). This is a convention, not a code change. A 5MB skills directory = 5MB image. The extraction code works efficiently with minimal images. + +**Future options for K8s driver**: +1. Install `crane` in the sandbox image and pull from inside (no runner-side extraction) +2. OpenShell `driver_config` volume mounts (needs upstream investigation) +3. Operator pre-populates skills into sandbox spec + +### Credentials (Provider API) — moderate fragility + +**What we've done**: `_inject_credentials()` first tries the OpenShell Provider API (`CreateProvider` + `AttachSandboxProvider` via raw gRPC), then falls back to file injection via `_do_write_file()`. + +**Fragility**: +- **Raw gRPC**: Uses `grpc.insecure_channel` and accesses `self._client._endpoint` (private attribute). If the OpenShell SDK changes its internal structure, this breaks silently. +- **Proto compatibility**: `CreateProviderRequest` and `AttachSandboxProviderRequest` protobuf messages must match the gateway version. No version negotiation. + +**Mitigation**: File-based fallback catches Provider API failures and writes credentials to `/var/run/secrets/llm-credentials/`. This degraded path works for all providers. Missing credentials now raise `RuntimeError` (fail-closed) instead of silently skipping. + +### MCP secret injection — solid + +**What we've done**: `_inject_mcp_secrets()` creates directories via `exec_stream(mkdir -p)` and writes secret values via `_do_write_file()` (base64 + exec). + +**Why it's solid**: Uses the same proven base64+exec pattern that `_do_write_file()` and `_do_read_file()` use for transcript collection. No external dependencies. Works on any OpenShell driver. + +### Filesystem policy — solid + +**What we've done**: `_build_filesystem_policy()` sets `spec.policy.filesystem.read_only = ["/"]` with explicit `read_write` exceptions for `/tmp`, `/home/agent`, `/var/log`, `/app/skills`, `/var/secrets/mcp`, `/var/run/secrets/llm-credentials`. + +**Why it's solid**: Sets protobuf fields directly on `SandboxSpec.policy` — same mechanism as `_build_network_policy()`. Clean contract with no external dependencies. + +### Network policy auto-derivation — solid + +**What we've done**: `_build_network_policy()` derives deny-by-default L7 egress rules from `LIGHTSPEED_PROVIDER` (provider-to-host mapping) and `LIGHTSPEED_MCP_SERVERS` (parsed URLs). Uses scheme-based default ports (80 for http, 443 for https). + +**Why it's solid**: Same protobuf contract as filesystem policy. Provider host mapping is a static dict. MCP URLs are parsed with stdlib `urlparse`. + +### TLS / Service account — solid (not applicable) + +**What we've done**: Log info and skip. The gateway provides transport security (TLS) and manages identity (no K8s SA equivalent). These are architectural decisions, not workarounds. + +### Provider cleanup — solid + +**What we've done**: `_do_destroy()` detaches providers via `DetachSandboxProvider` gRPC before deleting the sandbox. `_cleanup_sandbox()` does the same on spawn failure. Provider IDs are tracked in `self._provider_ids`. + +**Minor fragility**: Uses the same raw gRPC pattern as credential injection. If detach fails, it logs a warning and continues with sandbox deletion (non-blocking). + +## Spawner Comparison: OpenShell vs K8s vs Podman + +### Isolation & Security + +| | Podman | Kubernetes | OpenShell | +|---|---|---|---| +| **Isolation** | Linux containers (namespaces/cgroups) | K8s pods with hardened security context | MicroVM (strongest) | +| **Network egress** | Podman network (manual config) | Cluster NetworkPolicy (manual) | Auto-derived deny-by-default from provider + MCP config | +| **Filesystem** | Optional read-only flag | Always read-only rootfs | Granular read/write path policy | +| **TLS** | App-level cert injection | App-level cert injection | Not needed — gateway handles transport | + +### Feature Parity + +| Feature | Podman | Kubernetes | OpenShell | +|---|---|---|---| +| **Skills image** | Volume + transient container | Init container + emptyDir | Podman SDK extract + tar stream (Podman driver ✅, K8s driver ❌ issue #106) | +| **Credentials** | Skipped (warning) | Secret volume + envFrom | Provider API + file fallback | +| **MCP secrets** | Rejected (ValueError) | Secret volumes | File injection via exec | +| **Read-only mode** | Container flag | Always enforced | Policy with write exceptions | +| **Service account** | Ignored | PodSpec SA + projected token | N/A (gateway manages identity) | +| **Resource limits** | No | CPU/memory via SpawnConfig | No | +| **Progress streaming** | No | No | Yes (real-time JSONL tail) | + +### Architecture + +| | Podman | Kubernetes | OpenShell | +|---|---|---|---| +| **Primitive** | Container | Job + Service | Sandbox (microVM) | +| **Readiness** | Base class HTTP poll | Base class HTTP poll | Custom Host-header poll (gateway-routed) | +| **Routing** | `localhost:{random_port}` | `{name}.{ns}.svc:8080` | Gateway virtual-host (`Host` header) | +| **Cleanup** | Stop + remove container | Delete Job + Service | gRPC Delete + provider detach | +| **Instance tracking** | Podman label query | K8s Job label query | In-memory dicts | +| **Dependencies** | Podman socket | K8s API server | OpenShell gateway (gRPC) v0.0.79+ | + +### Key Findings + +1. **OpenShell is the most secure** — microVM isolation + auto-derived L7 network policy + gateway-managed credentials. K8s is second (always-hardened securityContext). Podman is weakest (socket access = host-level control). + +2. **OpenShell is the most feature-complete** — only spawner with progress streaming, Provider API, auto network policy, and granular filesystem policy. K8s has resource limits that OpenShell lacks. + +3. **Podman has the most gaps** — no credential injection, no MCP secrets (ValueError), no resource limits, no progress streaming. It is a dev/demo spawner. + +4. **OpenShell eliminates TLS complexity** — gateway handles transport security, removing cert generation/mounting/cleanup code. + +5. **Trade-off**: OpenShell adds a gateway infrastructure dependency. K8s/Podman spawners talk directly to the container runtime. + ## Files | File | Description | |---|---| -| `src/cloud_agents/spawner/openshell_spawner.py` | Spawner implementation | -| `tests/unit/spawner/test_openshell_spawner.py` | 27 unit tests | +| `src/cloud_agents/spawner/openshell_spawner.py` | Spawner implementation (52 tests) | +| `tests/unit/spawner/test_openshell_spawner.py` | Unit tests | | `docs/spikes/openshell-standalone-setup.md` | Standalone gateway setup guide | | `pyproject.toml` | `openshell` optional dependency | | `docs/gaps/gaps-implementation-plan.md` | T53 entry | diff --git a/src/cloud_agents/spawner/openshell_spawner.py b/src/cloud_agents/spawner/openshell_spawner.py index 7443189e..b2ef0f7e 100644 --- a/src/cloud_agents/spawner/openshell_spawner.py +++ b/src/cloud_agents/spawner/openshell_spawner.py @@ -66,6 +66,7 @@ class OpenShellSpawner(AgentSpawner): def __init__( self, openshell_client: Any = None, + driver: str = "podman", **kwargs: Any, ) -> None: """Initialize the OpenShell spawner. @@ -74,6 +75,13 @@ def __init__( openshell_client: OpenShell SDK client for sandbox operations. Must implement create(), wait_ready(), exec(), exec_stream(), and delete(). + driver: OpenShell compute driver type ("podman" or "kubernetes"). + Controls how skills_image is handled: + - "podman": Uses native image mounts via driver_config + (no extraction, no tar streaming — Podman mounts the + OCI image directly at /app/skills). + - "kubernetes": Falls back to tar streaming (extract + skills locally, stream into sandbox via exec_stream). Requires OpenShell gateway v0.0.79+ with gateway_jwt configured. The gateway mints sandbox JWTs and delivers them via Podman @@ -89,6 +97,7 @@ def __init__( """ super().__init__(**kwargs) self._client = openshell_client + self._driver = driver self._podman_cli: str | None = None self._sandbox_names: dict[str, str] = {} self._sandbox_ids: dict[str, str] = {} @@ -353,6 +362,27 @@ async def _do_spawn( if read_only: self._build_filesystem_policy(spec) + # Skills image: Podman driver mounts the OCI image directly + # via driver_config (no extraction needed). K8s driver falls + # back to tar streaming after sandbox creation. + skills_via_driver_config = False + if skills_image and self._driver == "podman": + from google.protobuf import struct_pb2 + + mount_config = struct_pb2.Struct() + mount_config.update({ + "podman": { + "mounts": [{ + "type": "image", + "source": skills_image, + "target": "/app/skills", + "read_only": True, + }], + }, + }) + spec.template.driver_config.MergeFrom(mount_config) + skills_via_driver_config = True + sandbox_ref = await asyncio.to_thread(self._client.create, spec=spec) sandbox_name = sandbox_ref.name sandbox_id = sandbox_ref.id @@ -371,7 +401,7 @@ async def _do_spawn( agent_name, sandbox_name, credential_secret_name, env, ) - if skills_image: + if skills_image and not skills_via_driver_config: await self._load_skills( agent_name, sandbox_id, skills_image, skills_paths, ) @@ -631,9 +661,53 @@ async def _extract_skills_image( ) -> None: """Extract skills content from an OCI image to a local directory. - Uses the Podman Python SDK (cross-platform, works in any - container) or falls back to the podman CLI binary. + Fallback chain (first success wins): + 1. crane — static binary, no daemon needed, works everywhere + 2. Podman Python SDK — needs Podman socket + 3. podman CLI — needs binary + socket + + crane is the primary path because it works in K8s pods + where no container runtime socket is available. Install it + in the runner Containerfile from go-containerregistry releases. """ + import shutil + import subprocess + + # 1. Try crane (no daemon needed — works in K8s pods) + crane_bin = shutil.which("crane") + if crane_bin: + try: + def _crane_extract() -> None: + result = subprocess.run( + [crane_bin, "export", skills_image, "-"], + capture_output=True, + timeout=120, + ) + if result.returncode != 0: + raise RuntimeError( + f"crane export failed: {result.stderr.decode()[:200]}" + ) + import tarfile + from io import BytesIO + + with tarfile.open(fileobj=BytesIO(result.stdout)) as tar: + for member in tar: + for skill_path in copy_paths: + prefix = skill_path.lstrip("/") + "/" + if member.name.startswith(prefix): + member.name = member.name[len(prefix):] + tar.extract(member, tmp_dir) + + await asyncio.to_thread(_crane_extract) + logger.info("Extracted skills via crane from '%s'", skills_image) + return + except Exception: + logger.warning( + "crane skills extraction failed, trying Podman SDK", + exc_info=True, + ) + + # 2. Try Podman Python SDK (needs socket) try: from podman import PodmanClient @@ -655,6 +729,7 @@ def _sdk_extract() -> None: pclient.close() await asyncio.to_thread(_sdk_extract) + logger.info("Extracted skills via Podman SDK from '%s'", skills_image) return except ImportError: pass @@ -664,9 +739,8 @@ def _sdk_extract() -> None: exc_info=True, ) + # 3. Try podman CLI if self._podman_cli: - import subprocess - for src_path in copy_paths: await asyncio.to_thread( subprocess.run, @@ -680,11 +754,12 @@ def _sdk_extract() -> None: capture_output=True, timeout=120, ) + logger.info("Extracted skills via podman CLI from '%s'", skills_image) return raise RuntimeError( f"Cannot extract skills image '{skills_image}': " - "neither Podman SDK nor podman CLI available" + "none of crane, Podman SDK, or podman CLI available" ) async def _inject_mcp_secrets( diff --git a/src/cloud_agents/workflow/temporal_entrypoint.py b/src/cloud_agents/workflow/temporal_entrypoint.py index 9c14ebe3..ebd8e608 100644 --- a/src/cloud_agents/workflow/temporal_entrypoint.py +++ b/src/cloud_agents/workflow/temporal_entrypoint.py @@ -90,11 +90,12 @@ def _create_spawner(): from openshell import SandboxClient gateway_url = os.environ.get("OPENSHELL_GATEWAY_URL", "localhost:17670") + driver = os.environ.get("OPENSHELL_DRIVER", "podman") # Strip http:// scheme — SandboxClient uses gRPC, not HTTP endpoint = gateway_url.replace("http://", "").replace("https://", "") client = SandboxClient(endpoint=endpoint) - logger.info("Using OpenShellSpawner (gateway=%s)", gateway_url) - return OpenShellSpawner(openshell_client=client) + logger.info("Using OpenShellSpawner (gateway=%s, driver=%s)", gateway_url, driver) + return OpenShellSpawner(openshell_client=client, driver=driver) logger.info("No spawner configured — sandbox activity will use stub mode") return None From 2599ffcc50c8fc80fab96ca938bc29ca3b06b6a5 Mon Sep 17 00:00:00 2001 From: James Wong <2421248+jameswnl@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:35:27 -0400 Subject: [PATCH 4/8] Fix credential lookup: try K8s-normalized and original env key credential_secret_name is K8s-normalized (e.g. "openai-api-key") by _to_k8s_secret_name(), but env dict has the original key ("OPENAI_API_KEY"). Try both forms before raising RuntimeError. E2E verified: all 7 workflows pass with Podman driver, 3 with K8s driver. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/cloud_agents/spawner/openshell_spawner.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cloud_agents/spawner/openshell_spawner.py b/src/cloud_agents/spawner/openshell_spawner.py index b2ef0f7e..0f8ab16f 100644 --- a/src/cloud_agents/spawner/openshell_spawner.py +++ b/src/cloud_agents/spawner/openshell_spawner.py @@ -497,7 +497,13 @@ async def _inject_credentials( Uses the OpenShell Provider system for gateway-managed credentials. Falls back to file-based injection if Provider RPCs fail. """ + # credential_secret_name may be K8s-normalized (e.g. "openai-api-key") + # while the env dict has the original key (e.g. "OPENAI_API_KEY"). + # Try both forms. cred_value = env.get(credential_secret_name) + if not cred_value: + original_key = credential_secret_name.upper().replace("-", "_") + cred_value = env.get(original_key) if not cred_value: raise RuntimeError( f"Credential '{credential_secret_name}' not found in env " From 0965457a5989ba6aee01342284986388e44b2aa8 Mon Sep 17 00:00:00 2001 From: James Wong <2421248+jameswnl@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:36:08 -0400 Subject: [PATCH 5/8] chore: trigger CI From b35ee5551b0c287c2b535e3521d5390e9e989615 Mon Sep 17 00:00:00 2001 From: James Wong <2421248+jameswnl@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:39:08 -0400 Subject: [PATCH 6/8] Fix CodeRabbit findings: tar traversal, posixpath, provider cleanup - Harden crane tar extraction: reject symlinks/hardlinks, validate resolved paths stay within tmp_dir (prevents path traversal) - Use posixpath.join for MCP secret file paths instead of f-string concatenation (correct regardless of trailing slash) - Provider cleanup: only remove _provider_ids entry after successful detach, retain for retry on failure Co-Authored-By: Claude Opus 4.6 (1M context) --- src/cloud_agents/spawner/openshell_spawner.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/cloud_agents/spawner/openshell_spawner.py b/src/cloud_agents/spawner/openshell_spawner.py index 0f8ab16f..4b7df62d 100644 --- a/src/cloud_agents/spawner/openshell_spawner.py +++ b/src/cloud_agents/spawner/openshell_spawner.py @@ -698,10 +698,17 @@ def _crane_extract() -> None: with tarfile.open(fileobj=BytesIO(result.stdout)) as tar: for member in tar: + if member.issym() or member.islnk(): + continue for skill_path in copy_paths: prefix = skill_path.lstrip("/") + "/" if member.name.startswith(prefix): member.name = member.name[len(prefix):] + resolved = os.path.normpath( + os.path.join(tmp_dir, member.name), + ) + if not resolved.startswith(tmp_dir): + continue tar.extract(member, tmp_dir) await asyncio.to_thread(_crane_extract) @@ -792,8 +799,10 @@ async def _inject_mcp_secrets( ) continue + import posixpath + await self._exec_mkdir(sandbox_id, mount_path) - file_path = f"{mount_path}{key}" + file_path = posixpath.join(mount_path, key) await self._do_write_file(agent_name, file_path, secret_value) logger.info( "Injected MCP secret '%s/%s' into sandbox for agent '%s'", @@ -1040,13 +1049,15 @@ async def _do_destroy(self, agent_name: str) -> None: except asyncio.CancelledError: pass - provider_id = self._provider_ids.pop(agent_name, None) + provider_id = self._provider_ids.get(agent_name) if provider_id: try: await self._detach_provider(sandbox_name, provider_id) + self._provider_ids.pop(agent_name, None) except Exception: logger.warning( - "Failed to detach provider '%s' from sandbox '%s'", + "Failed to detach provider '%s' from sandbox '%s' — " + "retained for retry on next destroy", provider_id, sandbox_name, exc_info=True, ) From 630a721bf812a4b5de1d5e1f2502f2a49cd0ef61 Mon Sep 17 00:00:00 2001 From: James Wong <2421248+jameswnl@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:46:02 -0400 Subject: [PATCH 7/8] chore: trigger CI on new branch From cf20b78764828529ad797a8cc7aaf2e6d05d42fa Mon Sep 17 00:00:00 2001 From: James Wong <2421248+jameswnl@users.noreply.github.com> Date: Sat, 11 Jul 2026 17:50:32 -0400 Subject: [PATCH 8/8] chore: whitespace to trigger CI --- src/cloud_agents/spawner/openshell_spawner.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cloud_agents/spawner/openshell_spawner.py b/src/cloud_agents/spawner/openshell_spawner.py index 4b7df62d..8f38285b 100644 --- a/src/cloud_agents/spawner/openshell_spawner.py +++ b/src/cloud_agents/spawner/openshell_spawner.py @@ -1089,3 +1089,4 @@ async def _do_list_active( List of agent names with active sandboxes. """ return list(self._sandbox_names.keys()) +