diff --git a/coding-agent/docs/action_safety/CURRENT_STATE.md b/coding-agent/docs/action_safety/CURRENT_STATE.md index e82162c43..6b10a7a5e 100644 --- a/coding-agent/docs/action_safety/CURRENT_STATE.md +++ b/coding-agent/docs/action_safety/CURRENT_STATE.md @@ -47,9 +47,9 @@ Current safety behavior: - validates explicit `cwd` against the workspace root, - rejects absolute path arguments outside the workspace in `sandbox_mode = "none"`, - supports structured stdout/stderr/exit-code results inside its structured-results scope, -- delegates to `coding_agent.tools.sandbox` for `none`, `nsjail`, or `docker` sandbox modes. +- delegates to `coding_agent.tools.sandbox` for `none`, `native`, `podman`, or `docker` sandbox modes. -`src/coding_agent/tools/sandbox.py` provides local runner implementations and guards for cwd, Docker env names, resource limits, and command path escape detection. Docker and nsjail modes are present but this phase explicitly does not implement a full Docker sandbox product. +`src/coding_agent/tools/sandbox.py` provides local runner implementations and guards for cwd, Docker/Podman env names, resource limits, and command path escape detection. Docker and Podman are explicit container modes; `native` resolves to macOS Seatbelt or Linux bubblewrap and `nsjail` is rejected. `src/coding_agent/environment/cloud.py` defines a separate cloud `bash_run` wrapper. It supports `cd` and `export` session-style responses, validates cwd under the cloud workspace default cwd, and delegates the raw command string to `CloudWorkspaceClient.run_command`. It does not use the local `_parse_command`, local shell-metacharacter rejection, local sandbox modes, local absolute-path argument escape checks, or local structured shell result scope. diff --git a/coding-agent/docs/adr/0060-native-platform-sandbox-backends.md b/coding-agent/docs/adr/0060-native-platform-sandbox-backends.md index e3cbcc5de..cff806ea4 100644 --- a/coding-agent/docs/adr/0060-native-platform-sandbox-backends.md +++ b/coding-agent/docs/adr/0060-native-platform-sandbox-backends.md @@ -5,13 +5,14 @@ ## Context -`bash_run` resolves command isolation through a single `sandbox_mode` selector -that maps one-to-one to a backend: `none`, `nsjail`, or `docker` -(`src/coding_agent/tools/sandbox.py`, `src/coding_agent/tools/shell.py`). This -couples the user-visible policy to a specific implementation and offers no -host-native isolation: `none` runs commands directly under host-execution -guards, `nsjail` is Linux-only and rarely deployed, and `docker` requires a -container runtime. On macOS there is no isolation option at all beyond `none`. +Before this ADR, `bash_run` resolved command isolation through a single +`sandbox_mode` selector that mapped one-to-one to a backend: `none`, `nsjail`, +or `docker` (`src/coding_agent/tools/sandbox.py`, +`src/coding_agent/tools/shell.py`). That coupled the user-visible policy to a +specific implementation and offered no host-native isolation: `none` ran +commands directly under host-execution guards, `nsjail` was Linux-only and +rarely deployed, and `docker` required a container runtime. On macOS there was +no isolation option at all beyond `none`. ADR-0058 established that sandbox is a wrapper policy applied to an executor environment derived from `RunTarget.isolation`, not an environment type. This @@ -65,8 +66,7 @@ no-new-privileges, rootless preferred). `nsjail` is dropped from the allowed `sandbox_mode` values. Configs that set `sandbox_mode = "nsjail"` are now rejected at validation. `NsjailSandboxRunner` -may remain in `sandbox.py` temporarily but is unreachable through config and is -slated for removal in a follow-up; it is not extended. +and the Helm nsjail sidecar template were removed in the follow-up cleanup. Native backends preserve the existing safety contract used by `none`/`docker`: workspace is read-write, required system paths are read-only, network is denied @@ -87,7 +87,8 @@ the default to `native`. Implemented. The user-visible `sandbox_mode` values are now converged to `none`, `native`, `podman`, and `docker`; `nsjail` is rejected by validation. Native sandbox resolution fails closed when the required platform backend is -unavailable, and the default is now `native`. +unavailable, and the default is now `native`. The temporary nsjail runner and +Helm sidecar scaffolding have been removed. The default switch is staged behind a structured retry signal: when `structured_results` is enabled, `bash_run` classifies native sandbox failures @@ -109,8 +110,8 @@ criteria below. intentionally copying. - Keep `nsjail` as an accepted legacy `sandbox_mode` value — rejected in favor of dropping it from validation now; keeping a fourth half-supported backend in - the user-visible surface contradicts the convergence goal. The runner class is - left temporarily only to stage its removal. + the user-visible surface contradicts the convergence goal. The runner class + was retained temporarily only to stage removal; that cleanup is now complete. - Silently fall back to `none` when a native backend binary is missing — rejected because it defeats the isolation contract; runners must fail closed. - Flip the default to `native` in the same change that adds the backends — diff --git a/coding-agent/helm/templates/configmap-sandbox.yaml b/coding-agent/helm/templates/configmap-sandbox.yaml deleted file mode 100644 index b09e8e2c1..000000000 --- a/coding-agent/helm/templates/configmap-sandbox.yaml +++ /dev/null @@ -1,11 +0,0 @@ -{{- if .Values.sandbox.sidecar.enabled }} -apiVersion: v1 -kind: ConfigMap -metadata: - name: {{ include "coding-agent.fullname" . }}-sandbox-config - labels: - {{- include "coding-agent.labels" . | nindent 4 }} -data: - nsjail.cfg: | -{{ .Values.sandbox.sidecar.config | indent 4 }} -{{- end }} diff --git a/coding-agent/helm/templates/deployment.yaml b/coding-agent/helm/templates/deployment.yaml index 6f12a41d1..edd16227b 100644 --- a/coding-agent/helm/templates/deployment.yaml +++ b/coding-agent/helm/templates/deployment.yaml @@ -69,10 +69,6 @@ spec: - name: AGENT_SESSION_REDIS_URL value: {{ default (printf "redis://127.0.0.1:%v/0" .Values.redis.sidecar.port) .Values.redis.url | quote }} {{- end }} - {{- if .Values.sandbox.sidecar.enabled }} - - name: SANDBOX_NSJAIL_CONFIG_PATH - value: {{ .Values.sandbox.sidecar.configPath | quote }} - {{- end }} {{- range .Values.agent.extraEnv }} - name: {{ .name }} value: {{ .value | quote }} @@ -97,11 +93,6 @@ spec: - name: data-storage mountPath: {{ .Values.persistence.data.mountPath }} {{- end }} - {{- if .Values.sandbox.sidecar.enabled }} - - name: sandbox-config - mountPath: /etc/nsjail - readOnly: true - {{- end }} readinessProbe: httpGet: path: /readyz @@ -141,17 +132,6 @@ spec: {{- toYaml . | nindent 12 }} {{- end }} {{- end }} - {{- if .Values.sandbox.sidecar.enabled }} - - name: sandbox - image: {{ .Values.sandbox.sidecar.image | quote }} - imagePullPolicy: IfNotPresent - command: - {{- toYaml .Values.sandbox.sidecar.command | nindent 12 }} - volumeMounts: - - name: sandbox-config - mountPath: /etc/nsjail - readOnly: true - {{- end }} volumes: - name: agent-config configMap: @@ -166,8 +146,3 @@ spec: persistentVolumeClaim: claimName: {{ default (printf "%s-data" (include "coding-agent.fullname" .)) .Values.persistence.data.existingClaim }} {{- end }} - {{- if .Values.sandbox.sidecar.enabled }} - - name: sandbox-config - configMap: - name: {{ include "coding-agent.fullname" . }}-sandbox-config - {{- end }} diff --git a/coding-agent/helm/values-orbstack-kimi.yaml b/coding-agent/helm/values-orbstack-kimi.yaml index 99a675c6e..2e73fd9f0 100644 --- a/coding-agent/helm/values-orbstack-kimi.yaml +++ b/coding-agent/helm/values-orbstack-kimi.yaml @@ -22,7 +22,3 @@ agent: redis: enabled: false - -sandbox: - sidecar: - enabled: false diff --git a/coding-agent/helm/values-orbstack.yaml b/coding-agent/helm/values-orbstack.yaml index fe37f0bc6..2818a32ea 100644 --- a/coding-agent/helm/values-orbstack.yaml +++ b/coding-agent/helm/values-orbstack.yaml @@ -10,6 +10,3 @@ server: enabled: false redis: enabled: false -sandbox: - sidecar: - enabled: false diff --git a/coding-agent/helm/values.yaml b/coding-agent/helm/values.yaml index f4c6b2b02..6778cd205 100644 --- a/coding-agent/helm/values.yaml +++ b/coding-agent/helm/values.yaml @@ -92,19 +92,3 @@ redis: - --appendonly - "no" resources: {} - -sandbox: - sidecar: - enabled: false - image: busybox:1.36 - command: - - sh - - -c - - sleep infinity - configPath: /etc/nsjail/nsjail.cfg - config: | - name: "coding-agent" - mode: o - cwd: "/workspace" - bindmount: ["/workspace:/workspace"] - time_limit: 10 diff --git a/coding-agent/src/coding_agent/tools/sandbox.py b/coding-agent/src/coding_agent/tools/sandbox.py index 6072af3db..0a121471b 100644 --- a/coding-agent/src/coding_agent/tools/sandbox.py +++ b/coding-agent/src/coding_agent/tools/sandbox.py @@ -230,60 +230,6 @@ def _bwrap_command(self, request: SandboxRequest, cwd: Path) -> list[str]: return command -class NsjailSandboxRunner: - # ADR-0060: nsjail is dropped from accepted sandbox_mode values and is no - # longer reachable through build_sandbox(). This class is retained only to - # stage its removal and must not be extended. - def __init__(self, config: SandboxConfig) -> None: - self._config: SandboxConfig = config - - def run(self, request: SandboxRequest) -> subprocess.CompletedProcess[str]: - if platform.system() != "Linux": - raise SandboxUnavailableError( - "nsjail sandbox mode is only supported on Linux" - ) - if which("nsjail") is None: - raise SandboxUnavailableError("nsjail binary not found on PATH") - - cwd = _validate_cwd( - request.cwd, - self._config.workspace_root, - additional_roots=self._config.additional_roots, - ) - command = self._nsjail_command(request, cwd) - return subprocess.run( - command, - shell=False, - capture_output=True, - text=True, - timeout=request.timeout_seconds, - env=request.env, - ) - - def _nsjail_command(self, request: SandboxRequest, cwd: Path) -> list[str]: - command = [ - "nsjail", - "--mode", - "o", - "--cwd", - str(cwd), - "--bindmount", - f"{self._config.workspace_root}:{self._config.workspace_root}", - "--disable_proc", - "--iface_no_lo", - "--", - ] - if self._config.limits.cpu_limit_seconds is not None: - command[1:1] = ["--time_limit", str(self._config.limits.cpu_limit_seconds)] - if self._config.limits.memory_limit_mb is not None: - command[1:1] = [ - "--rlimit_as", - str(self._config.limits.memory_limit_mb * 1024 * 1024), - ] - command.extend(request.args) - return command - - class DockerSandboxRunner: def __init__(self, config: SandboxConfig) -> None: self._config: SandboxConfig = config diff --git a/coding-agent/tests/deploy/test_helm_chart.py b/coding-agent/tests/deploy/test_helm_chart.py index 546138cbd..5773de56d 100644 --- a/coding-agent/tests/deploy/test_helm_chart.py +++ b/coding-agent/tests/deploy/test_helm_chart.py @@ -62,6 +62,10 @@ def _service(docs: list[dict[str, Any]]) -> dict[str, Any]: return _object_named(docs, "Service", "coding-agent") +def _objects_of_kind(docs: list[dict[str, Any]], kind: str) -> list[dict[str, Any]]: + return [doc for doc in docs if doc.get("kind") == kind] + + def _agent_toml(docs: list[dict[str, Any]]) -> dict[str, Any]: config = _object_named(docs, "ConfigMap", "-agent-config") return tomllib.loads(config["data"]["agent.toml"]) @@ -200,6 +204,27 @@ def test_helm_data_mount_override_updates_sqlite_bundle_path() -> None: _assert_storage(docs, "/data2") +def test_helm_chart_ignores_legacy_sandbox_sidecar_values() -> None: + docs = _render("--set", "sandbox.sidecar.enabled=true") + assert all( + not doc.get("metadata", {}).get("name", "").endswith("-sandbox-config") + for doc in _objects_of_kind(docs, "ConfigMap") + ) + + pod_spec = _deployment(docs)["spec"]["template"]["spec"] + assert [container["name"] for container in pod_spec["containers"]] == [ + "coding-agent" + ] + main = _container(docs) + assert all( + env.get("name") != "SANDBOX_NSJAIL_CONFIG_PATH" for env in main.get("env", []) + ) + assert all( + mount.get("name") != "sandbox-config" for mount in main.get("volumeMounts", []) + ) + assert all(volume.get("name") != "sandbox-config" for volume in pod_spec["volumes"]) + + def test_runtime_image_installs_native_linux_sandbox_binary() -> None: dockerfile = (ROOT / "Dockerfile").read_text() install_block = dockerfile.split("apt-get install", maxsplit=1)[1].split( diff --git a/coding-agent/tests/tools/test_shell.py b/coding-agent/tests/tools/test_shell.py index 95797cd89..caee8e6b0 100644 --- a/coding-agent/tests/tools/test_shell.py +++ b/coding-agent/tests/tools/test_shell.py @@ -627,6 +627,11 @@ def test_nsjail_mode_is_rejected_by_validation(self): with pytest.raises(ValueError, match="(?i)unsupported sandbox mode"): _sandbox_mode({"sandbox_mode": "nsjail"}) + def test_nsjail_runner_is_removed(self): + sandbox_module = importlib.import_module("coding_agent.tools.sandbox") + + assert not hasattr(sandbox_module, "NsjailSandboxRunner") + def test_existing_none_and_docker_modes_unchanged(self, tmp_path: Path): from coding_agent.tools.shell import _sandbox_mode