Add CONTAINER_WORKSPACE_MOUNT_MODE to container_shell (no default change)

src/seclab_taskflows/mcp_servers/container_shell.py

@@ -8,6 +8,13 @@
 
 - ``CONTAINER_IMAGE`` — image to run (required).
 - ``CONTAINER_WORKSPACE`` — host path bind-mounted at ``/workspace`` (optional).
+- ``CONTAINER_WORKSPACE_MOUNT_MODE`` — bind-mount mode for that path, ``rw`` (default)
+  or ``ro``. The default preserves the historical writable mount, because the
+  shipped source-access and SAST prompts build symbol indexes in-tree
+  (``ctags -R .``, ``cscope -R -b``, ``gtags``). Set it to ``ro`` when the
+  workspace should be left pristine — for example when the caller collects the
+  tree afterwards as an artifact and wants a faithful copy of the input. Only a
+  literal ``ro`` selects read-only; any other value mounts read-write.
 - ``CONTAINER_TIMEOUT`` — default per-command timeout in seconds (default 30).
 - ``CONTAINER_PERSIST`` — reuse a deterministic container across runs when truthy.
 - ``CONTAINER_PERSIST_KEY`` — extra key to distinguish persistent containers.
@@ -66,6 +73,18 @@
 
 CONTAINER_IMAGE = os.environ.get("CONTAINER_IMAGE", "")
 CONTAINER_WORKSPACE = os.environ.get("CONTAINER_WORKSPACE", "")
+# Bind-mount mode for CONTAINER_WORKSPACE. Defaults to "rw", which preserves
+# the historical behaviour: the shipped source-access and SAST prompts tell the
+# model to build symbol indexes in-tree (`ctags -R .`, `cscope -R -b`, `gtags`),
+# which write into the workspace. Setting this to "ro" is worth doing whenever
+# the caller collects the workspace afterwards as an artifact, since a writable
+# mount makes that artifact agent-influenced rather than a faithful copy of the
+# input. Flipping the default would require redirecting those index outputs
+# first. Only a literal "ro" selects read-only, so a blank or unrecognized
+# value cannot accidentally break a workflow that needs to write.
+CONTAINER_WORKSPACE_MOUNT_MODE = (
+    "ro" if os.environ.get("CONTAINER_WORKSPACE_MOUNT_MODE", "").strip().lower() == "ro" else "rw"
+)
 CONTAINER_TIMEOUT = int(os.environ.get("CONTAINER_TIMEOUT", "30"))
 CONTAINER_PERSIST = os.environ.get("CONTAINER_PERSIST", "").lower() in ("1", "true", "yes")
 CONTAINER_PERSIST_KEY = os.environ.get("CONTAINER_PERSIST_KEY", "")
@@ -103,8 +122,19 @@ def _persistent_name() -> str:
     network (e.g. the default "none") never reuses a persistent container that
     was created with a different, more permissive network (e.g. "bridge"),
     which would otherwise silently re-enable egress.
+
+    The workspace mount mode is included for the same reason, but only when it
+    differs from the "rw" default. Appending it unconditionally would change the
+    derived name for every persistent container that already exists, orphaning
+    each one on upgrade: _start_container() only ever inspects or removes the
+    name it computes, and persistent containers are normally left running, so a
+    non-forcing ``docker rm`` would not collect them either. Omitting the field
+    at the default keeps those names byte-identical, while an explicit "ro" run
+    still gets a distinct name and so cannot reuse a writable container.
     """
     key_material = f"{CONTAINER_IMAGE}:{CONTAINER_WORKSPACE}:net={CONTAINER_NETWORK}"
+    if CONTAINER_WORKSPACE_MOUNT_MODE != "rw":
+        key_material += f":ws={CONTAINER_WORKSPACE_MOUNT_MODE}"
     if CONTAINER_PERSIST_KEY:
         key_material += f":{CONTAINER_PERSIST_KEY}"
     digest = hashlib.sha256(key_material.encode()).hexdigest()[:12]
@@ -172,7 +202,7 @@ def _start_container() -> str:
     if not CONTAINER_PERSIST:
         cmd.append("--rm")
     if CONTAINER_WORKSPACE:
-        cmd += ["-v", f"{CONTAINER_WORKSPACE}:/workspace"]
+        cmd += ["-v", f"{CONTAINER_WORKSPACE}:/workspace:{CONTAINER_WORKSPACE_MOUNT_MODE}"]
     cmd += [CONTAINER_IMAGE, "tail", "-f", "/dev/null"]
     logging.debug(f"Starting container: {' '.join(cmd)}")
     result = subprocess.run(cmd, capture_output=True, text=True, timeout=_DOCKER_TIMEOUT)

src/seclab_taskflows/toolboxes/container_shell_base.yaml

@@ -12,6 +12,7 @@ server_params:
   env:
     CONTAINER_IMAGE: "ghcr.io/githubsecuritylab/seclab-shell-base:latest"
     CONTAINER_WORKSPACE: "{{ env('CONTAINER_WORKSPACE', required=False) }}"
+    CONTAINER_WORKSPACE_MOUNT_MODE: "{{ env('CONTAINER_WORKSPACE_MOUNT_MODE', 'rw') }}"
     CONTAINER_TIMEOUT: "{{ env('CONTAINER_TIMEOUT', '30') }}"
     CONTAINER_PERSIST: "{{ env('CONTAINER_PERSIST', required=False) }}"
     CONTAINER_PERSIST_KEY: "{{ env('CONTAINER_PERSIST_KEY', required=False) }}"

src/seclab_taskflows/toolboxes/container_shell_malware_analysis.yaml

@@ -12,6 +12,7 @@ server_params:
   env:
     CONTAINER_IMAGE: "ghcr.io/githubsecuritylab/seclab-shell-malware-analysis:latest"
     CONTAINER_WORKSPACE: "{{ env('CONTAINER_WORKSPACE', required=False) }}"
+    CONTAINER_WORKSPACE_MOUNT_MODE: "{{ env('CONTAINER_WORKSPACE_MOUNT_MODE', 'rw') }}"
     CONTAINER_TIMEOUT: "{{ env('CONTAINER_TIMEOUT', '60') }}"
     CONTAINER_PERSIST: "{{ env('CONTAINER_PERSIST', required=False) }}"
     CONTAINER_PERSIST_KEY: "{{ env('CONTAINER_PERSIST_KEY', required=False) }}"

src/seclab_taskflows/toolboxes/container_shell_network_analysis.yaml

@@ -12,6 +12,7 @@ server_params:
   env:
     CONTAINER_IMAGE: "ghcr.io/githubsecuritylab/seclab-shell-network-analysis:latest"
     CONTAINER_WORKSPACE: "{{ env('CONTAINER_WORKSPACE', required=False) }}"
+    CONTAINER_WORKSPACE_MOUNT_MODE: "{{ env('CONTAINER_WORKSPACE_MOUNT_MODE', 'rw') }}"
     CONTAINER_TIMEOUT: "{{ env('CONTAINER_TIMEOUT', '30') }}"
     CONTAINER_PERSIST: "{{ env('CONTAINER_PERSIST', required=False) }}"
     CONTAINER_PERSIST_KEY: "{{ env('CONTAINER_PERSIST_KEY', required=False) }}"

src/seclab_taskflows/toolboxes/container_shell_sast.yaml

@@ -12,6 +12,12 @@ server_params:
   env:
     CONTAINER_IMAGE: "ghcr.io/githubsecuritylab/seclab-shell-sast:latest"
     CONTAINER_WORKSPACE: "{{ env('CONTAINER_WORKSPACE', required=False) }}"
+    # Writable: the documented workflow for this toolbox builds symbol indexes
+    # in-tree (`ctags -R .`, `cscope -R -b`, `gtags`), which write tags,
+    # cscope.out and GTAGS/GRTAGS into the workspace. Set this to "ro" and
+    # redirect those outputs (e.g. `ctags -f /tmp/tags`) if you need the
+    # workspace left pristine.
+    CONTAINER_WORKSPACE_MOUNT_MODE: "{{ env('CONTAINER_WORKSPACE_MOUNT_MODE', 'rw') }}"
     CONTAINER_TIMEOUT: "{{ env('CONTAINER_TIMEOUT', '60') }}"
     CONTAINER_PERSIST: "{{ env('CONTAINER_PERSIST', required=False) }}"
     CONTAINER_PERSIST_KEY: "{{ env('CONTAINER_PERSIST_KEY', required=False) }}"

src/seclab_taskflows/toolboxes/container_shell_source_access.yaml

@@ -12,6 +12,10 @@ server_params:
   env:
     CONTAINER_IMAGE: "ghcr.io/githubsecuritylab/seclab-shell-source-access:latest"
     CONTAINER_WORKSPACE: "{{ env('CONTAINER_WORKSPACE', required=False) or env('DATA_DIR') ~ '/repo_under_test' }}"
+    # The prompt for this toolbox builds symbol indexes in-tree (`ctags -R .`,
+    # `gtags`), so the mount must stay writable by default. Set this to "ro"
+    # if you redirect those outputs and want the workspace left pristine.
+    CONTAINER_WORKSPACE_MOUNT_MODE: "{{ env('CONTAINER_WORKSPACE_MOUNT_MODE', 'rw') }}"
     CONTAINER_TIMEOUT: "{{ env('CONTAINER_TIMEOUT', '60') }}"
     CONTAINER_PERSIST: "{{ env('CONTAINER_PERSIST', 'true') }}"
     CONTAINER_PERSIST_KEY: "{{ env('CONTAINER_PERSIST_KEY', required=False) }}"

tests/test_container_shell.py

@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: MIT
 
 import atexit
+import hashlib
 import importlib
 import os
 import subprocess
@@ -67,6 +68,7 @@ def test_start_container_success(self):
         with (
             patch.object(cs_mod, "CONTAINER_IMAGE", "test-image:latest"),
             patch.object(cs_mod, "CONTAINER_WORKSPACE", "/host/workspace"),
+            patch.object(cs_mod, "CONTAINER_WORKSPACE_MOUNT_MODE", "rw"),
             patch("subprocess.run", return_value=_make_proc(returncode=0)) as mock_run,
         ):
             name = cs_mod._start_container()
@@ -76,7 +78,7 @@ def test_start_container_success(self):
             assert "run" in cmd
             assert "--name" in cmd
             assert "-v" in cmd
-            assert "/host/workspace:/workspace" in cmd
+            assert "/host/workspace:/workspace:rw" in cmd
             assert "test-image:latest" in cmd
             assert "tail" in cmd
 
@@ -91,6 +93,49 @@ def test_start_container_no_workspace(self):
             cmd = mock_run.call_args[0][0]
             assert "-v" not in cmd
 
+    def test_start_container_workspace_writable_by_default(self):
+        """The default preserves the historical writable mount."""
+        with (
+            patch.object(cs_mod, "CONTAINER_IMAGE", "test-image:latest"),
+            patch.object(cs_mod, "CONTAINER_WORKSPACE", "/host/workspace"),
+            patch.object(cs_mod, "CONTAINER_WORKSPACE_MOUNT_MODE", "rw"),
+            patch("subprocess.run", return_value=_make_proc(returncode=0)) as mock_run,
+        ):
+            cs_mod._start_container()
+            cmd = mock_run.call_args[0][0]
+            assert "/host/workspace:/workspace:rw" in cmd
+
+    def test_start_container_workspace_ro_opt_in(self):
+        """CONTAINER_WORKSPACE_MOUNT_MODE=ro mounts the workspace read-only."""
+        with (
+            patch.object(cs_mod, "CONTAINER_IMAGE", "test-image:latest"),
+            patch.object(cs_mod, "CONTAINER_WORKSPACE", "/host/workspace"),
+            patch.object(cs_mod, "CONTAINER_WORKSPACE_MOUNT_MODE", "ro"),
+            patch("subprocess.run", return_value=_make_proc(returncode=0)) as mock_run,
+        ):
+            cs_mod._start_container()
+            cmd = mock_run.call_args[0][0]
+            assert "/host/workspace:/workspace:ro" in cmd
+
+    def test_persistent_name_unchanged_at_default_mode(self):
+        """Upgrading must not rename existing persistent containers.
+
+        The pre-CONTAINER_WORKSPACE_MOUNT_MODE name is the one derived without a
+        ws= field, so the default must reproduce it exactly or every existing
+        persistent container is orphaned on upgrade.
+        """
+        with (
+            patch.object(cs_mod, "CONTAINER_IMAGE", "test-image:latest"),
+            patch.object(cs_mod, "CONTAINER_WORKSPACE", "/host/workspace"),
+            patch.object(cs_mod, "CONTAINER_NETWORK", "none"),
+            patch.object(cs_mod, "CONTAINER_WORKSPACE_MOUNT_MODE", "rw"),
+            patch.object(cs_mod, "CONTAINER_PERSIST_KEY", ""),
+        ):
+            pre_mode = hashlib.sha256(
+                b"test-image:latest:/host/workspace:net=none"
+            ).hexdigest()[:12]
+            assert cs_mod._persistent_name() == f"seclab-persist-{pre_mode}"
+
     def test_start_container_failure(self):
         with (
             patch.object(cs_mod, "CONTAINER_IMAGE", "missing-image:latest"),
@@ -140,6 +185,39 @@ def test_start_container_opt_in_network(self):
             assert "--network" in cmd
             assert cmd[cmd.index("--network") + 1] == "bridge"
 
+    def test_workspace_mode_defaults_to_rw_when_unset(self, monkeypatch):
+        """Unset preserves the historical writable mount."""
+        original = os.environ.get("CONTAINER_WORKSPACE_MOUNT_MODE")
+        monkeypatch.delenv("CONTAINER_WORKSPACE_MOUNT_MODE", raising=False)
+        try:
+            reloaded = _reload_cs()
+            assert reloaded.CONTAINER_WORKSPACE_MOUNT_MODE == "rw"
+        finally:
+            _restore_env_and_reload("CONTAINER_WORKSPACE_MOUNT_MODE", original)
+
+    @pytest.mark.parametrize(
+        "value", ["", "   ", "\t", "bogus", "readonly", "rw", "ro; rm -rf /", "ro rw"]
+    )
+    def test_workspace_mode_falls_back_to_rw(self, monkeypatch, value):
+        """Only a literal "ro" opts in; anything else stays writable."""
+        original = os.environ.get("CONTAINER_WORKSPACE_MOUNT_MODE")
+        monkeypatch.setenv("CONTAINER_WORKSPACE_MOUNT_MODE", value)
+        try:
+            reloaded = _reload_cs()
+            assert reloaded.CONTAINER_WORKSPACE_MOUNT_MODE == "rw"
+        finally:
+            _restore_env_and_reload("CONTAINER_WORKSPACE_MOUNT_MODE", original)
+
+    @pytest.mark.parametrize("value", ["ro", "RO", " ro "])
+    def test_workspace_mode_ro_opt_in(self, monkeypatch, value):
+        original = os.environ.get("CONTAINER_WORKSPACE_MOUNT_MODE")
+        monkeypatch.setenv("CONTAINER_WORKSPACE_MOUNT_MODE", value)
+        try:
+            reloaded = _reload_cs()
+            assert reloaded.CONTAINER_WORKSPACE_MOUNT_MODE == "ro"
+        finally:
+            _restore_env_and_reload("CONTAINER_WORKSPACE_MOUNT_MODE", original)
+
     def test_network_defaults_to_none_when_unset(self, monkeypatch):
         original = os.environ.get("CONTAINER_NETWORK")
         monkeypatch.delenv("CONTAINER_NETWORK", raising=False)
@@ -301,6 +379,19 @@ def test_persistent_name_differs_for_different_workspaces(self):
                 name_b = cs_mod._persistent_name()
             assert name_a != name_b
 
+    def test_persistent_name_varies_with_workspace_mode(self):
+        """A "ro" run must not reuse a container created with a writable mount."""
+        with (
+            patch.object(cs_mod, "CONTAINER_IMAGE", "test-image:latest"),
+            patch.object(cs_mod, "CONTAINER_WORKSPACE", "/source/tree"),
+            patch.object(cs_mod, "CONTAINER_PERSIST_KEY", ""),
+        ):
+            with patch.object(cs_mod, "CONTAINER_WORKSPACE_MOUNT_MODE", "ro"):
+                name_ro = cs_mod._persistent_name()
+            with patch.object(cs_mod, "CONTAINER_WORKSPACE_MOUNT_MODE", "rw"):
+                name_rw = cs_mod._persistent_name()
+            assert name_ro != name_rw
+
     def test_persistent_name_varies_with_network(self):
         with (
             patch.object(cs_mod, "CONTAINER_IMAGE", "test-image:latest"),
@@ -343,13 +434,18 @@ def test_start_persistent_no_rm_flag(self):
             patch.object(cs_mod, "CONTAINER_WORKSPACE", ""),
             patch.object(cs_mod, "CONTAINER_PERSIST", True),
             patch.object(cs_mod, "CONTAINER_PERSIST_KEY", ""),
-            patch("subprocess.run", side_effect=[inspect_proc, rm_proc, run_proc]) as mock_run,
+            patch(
+                "subprocess.run",
+                side_effect=[inspect_proc, rm_proc, run_proc],
+            ) as mock_run,
         ):
             name = cs_mod._start_container()
             assert name.startswith("seclab-persist-")
-            # The docker run call is the third one
-            run_cmd = mock_run.call_args_list[2][0][0]
-            assert "--rm" not in run_cmd
+            # Locate the docker run call rather than indexing, so inserting
+            # another docker invocation ahead of it does not break this test.
+            run_cmds = [c[0][0] for c in mock_run.call_args_list if c[0][0][:2] == ["docker", "run"]]
+            assert len(run_cmds) == 1
+            assert "--rm" not in run_cmds[0]
 
     def test_stop_skips_persistent_container(self):
         cs_mod._container_name = "seclab-persist-abc123"