Give the test suite a devpod namespace of its own

test/conftest.py

@@ -27,6 +27,7 @@
     local_git_repo_with_devcontainer,
     real_managers,
 )
+from devpod_scoping import scope_devpod_to_this_run  # noqa: E402
 from fixtures.devpod_mock import DevPodMock, mock_devpod  # noqa: E402
 from fixtures.e2e_helpers import dl_no_ide, devpod_cleanup  # noqa: E402
 
@@ -85,7 +86,16 @@ def fresh_workspace_list_cache():
 
 
 def pytest_configure(config):
-    """Register custom markers."""
+    """Scope this run's devpod state, then register custom markers.
+
+    The scoping happens here, before collection, rather than in a fixture: a
+    fixture is something a test has to ask for, and the test that must not
+    forget is the one nobody has written yet. Everything the session spawns
+    inherits this process's environment, so one assignment covers the whole
+    suite -- including the `devpod list` that decides what `dl --purge` deletes.
+    """
+    scope_devpod_to_this_run()
+
     config.addinivalue_line(
         "markers",
         "unit: Pure logic tests with no external commands. Fast, runs everywhere.",

test/devpod_scoping.py

@@ -0,0 +1,62 @@
+"""Keep the test suite's devpod state off the developer's machine.
+
+devpod's workspace namespace lives in `~/.devpod`, and no `XDG_*` variable
+moves it. That matters because `dl --purge` deletes *every* workspace `devpod
+list` returns and an e2e test exercises exactly that, for real: run the e2e
+suite on a machine with real workspaces on it and they are gone. The
+`-m 'not e2e'` default in pyproject is the only thing standing in the way, and
+a default is not a safeguard.
+
+So the suite gives itself a devpod home of its own and points devpod at it
+through the process environment, before any test runs. Two variables are needed
+rather than one -- `DEVPOD_HOME` does not cover the ssh config, which devpod
+resolves against the real `$HOME` regardless.
+
+The two variables do not have the same reach, and the difference is worth
+knowing. `--devpod-home` is a *persistent root* flag, so `DEVPOD_HOME` applies
+to every subcommand. `--ssh-config` is registered on `devpod up` alone
+(measured on devpod v0.26.1: `up` has it, `ssh`/`delete`/`stop` do not), so
+`DEVPOD_SSH_CONFIG` only redirects the subcommand that *writes* ssh config.
+That is the one that does the damage, so the suite is covered -- but a reader
+should not take `DEVPOD_SSH_CONFIG` for a global.
+
+Scoping rather than guarding is deliberate. A guard is an assertion some future
+test can forget to make; this applies to every subprocess the session spawns,
+including the `devpod list` inside `dl --purge`, because they all inherit this
+environment. It also makes the suite's hardcoded `e2e-test-*` workspace ids
+private to the run, so two concurrent runs stop force-deleting each other's
+workspaces -- the directory is new every time.
+"""
+
+import os
+import tempfile
+from pathlib import Path
+
+DEVPOD_HOME_VAR = "DEVPOD_HOME"
+DEVPOD_SSH_CONFIG_VAR = "DEVPOD_SSH_CONFIG"
+
+
+def scope_devpod_to_this_run() -> Path:
+    """Point every devpod subprocess in this process at a private namespace.
+
+    Returns the devpod home it created, which is a *new* directory on every
+    call. That is load-bearing rather than incidental: the suite's e2e
+    workspace ids are hardcoded constants, so a namespace shared between two
+    concurrent runs means each run's teardown force-deletes the other's
+    workspaces.
+
+    The directory is deliberately not cleaned up afterwards. It holds the
+    metadata devpod needs to find and delete the containers a run created, so
+    deleting it after a crashed run would leave those containers orphaned with
+    no way to reach them -- worse than a stale directory the OS will reap.
+
+    Two honest caveats on that. Most runs are unit runs that never spawn devpod
+    at all, so what they leave behind is an empty directory rather than
+    recoverable state. And the reaping this leans on is itself the orphaning
+    event it is trying to avoid, just deferred: a devpod home that outlives its
+    containers by long enough gets collected with them still running.
+    """
+    devpod_home = Path(tempfile.mkdtemp(prefix="devlaunch-testrun-"))
+    os.environ[DEVPOD_HOME_VAR] = str(devpod_home)
+    os.environ[DEVPOD_SSH_CONFIG_VAR] = str(devpod_home / "ssh_config")
+    return devpod_home

test/e2e/conftest.py

@@ -0,0 +1,52 @@
+"""Make the run's private devpod home usable by real devpod commands.
+
+The root conftest points `DEVPOD_HOME` at a directory created for this run. A
+brand new devpod home has no providers in it at all, so nothing can be brought
+up there until one is installed. Doing that here, autouse and session-scoped,
+means every test in this directory gets it without asking and no other kind of
+test pays for it.
+"""
+
+import os
+import subprocess
+
+import pytest
+
+from devpod_scoping import DEVPOD_HOME_VAR
+from fixtures.e2e_helpers import devpod_available
+
+
+@pytest.fixture(scope="session", autouse=True)
+def docker_provider_in_scoped_devpod_home():
+    """Install the docker provider into this run's devpod home.
+
+    `--use` rewrites the *default provider* of whichever devpod home is live, so
+    this is the one unconditional write to a devpod home the suite performs. Its
+    safety rests entirely on the scoping the root conftest sets up, which makes
+    it the one place worth asserting that scoping rather than assuming it: if
+    DEVPOD_HOME ever stops being set, this fixture is what would reach into the
+    developer's own ~/.devpod.
+
+    The install is a precondition, not a teardown, so a failure is raised rather
+    than swallowed -- otherwise every later e2e test fails against a
+    provider-less devpod home with an unrelated error, while the stderr that
+    explains it was captured and discarded.
+    """
+    assert os.environ.get(DEVPOD_HOME_VAR), (
+        "refusing to add a devpod provider: DEVPOD_HOME is unset, so `--use` "
+        "would rewrite the default provider in the developer's real ~/.devpod"
+    )
+
+    if not devpod_available():
+        return
+
+    result = subprocess.run(
+        ["devpod", "provider", "add", "docker", "--use"],
+        capture_output=True,
+        text=True,
+        check=False,
+    )
+    if result.returncode != 0:
+        pytest.fail(
+            f"could not install the docker provider into {os.environ[DEVPOD_HOME_VAR]}:\n{result.stderr}"
+        )

test/e2e/test_full_workflow.py

@@ -13,21 +13,63 @@
 import json
 import os
 import subprocess
+from pathlib import Path
 
 import pytest
 
+from fixtures.e2e_helpers import devpod_available
+
+
+def real_devpod_workspace_ids() -> set:
+    """Workspace ids in the developer's own ~/.devpod, read straight off disk.
+
+    Read from the filesystem rather than from `devpod list`, because the whole
+    point of the assertion below is that `devpod list` no longer answers for
+    that directory.
+    """
+    contexts = Path.home() / ".devpod" / "contexts"
+    if not contexts.is_dir():
+        return set()
+    return {
+        workspace.name
+        for context in contexts.iterdir()
+        for workspace in (context / "workspaces").glob("*")
+        if workspace.is_dir()
+    }
+
+
+@pytest.mark.e2e
+class TestSuiteIsolationE2E:
+    """The destructive half of this suite must not be able to reach real state."""
+
+    def test_devpod_in_this_session_cannot_see_the_developers_workspaces(self):
+        """Proves the scoping is live in the session that could do the damage.
+
+        Every devpod call in this file -- including the one inside `dl --purge`
+        -- inherits this process's environment, so what `devpod list` reports
+        here is exactly what `--purge` would delete.
+
+        Any set at all is disjoint from an empty one, so where there is no real
+        devpod state to be disjoint from -- CI, or a fresh DinD container --
+        this skips rather than passing on nothing.
+        """
+        if not devpod_available():
+            pytest.skip("DevPod not available")
+
+        real_ids = real_devpod_workspace_ids()
+        if not real_ids:
+            pytest.skip("no workspaces in ~/.devpod on this host; nothing to be isolated from")
 
-def devpod_available() -> bool:
-    """Check if DevPod is available."""
-    try:
         result = subprocess.run(
-            ["devpod", "version"],
+            ["devpod", "list", "--output", "json"],
             capture_output=True,
+            text=True,
             check=False,
         )
-        return result.returncode == 0
-    except FileNotFoundError:
-        return False
+        assert result.returncode == 0
+
+        visible = {ws.get("id", "") for ws in json.loads(result.stdout or "[]")}
+        assert visible.isdisjoint(real_ids)
 
 
 @pytest.mark.e2e

test/fixtures/e2e_helpers.py

@@ -12,6 +12,24 @@
 import pytest
 
 
+def devpod_available() -> bool:
+    """Whether real devpod commands can run at all.
+
+    The single answer to that question for the whole e2e directory: an
+    installed-but-unrunnable devpod and a missing one are the same thing to a
+    test, and two checks that can disagree are worse than one that cannot.
+    """
+    try:
+        result = subprocess.run(
+            ["devpod", "version"],
+            capture_output=True,
+            check=False,
+        )
+        return result.returncode == 0
+    except FileNotFoundError:
+        return False
+
+
 class DLRunner:
     """Helper to run dl commands safely without launching IDE.
 

test/unit/test_devpod_scoping.py

@@ -0,0 +1,86 @@
+"""The suite must never be able to reach the developer's real devpod state.
+
+`dl --purge` deletes every workspace `devpod list` returns, and an e2e test runs
+it for real. `devpod list` reads `~/.devpod`, which no `XDG_*` variable
+relocates, so the suite's existing XDG isolation buys nothing against it: a
+`pytest -m e2e` on a developer's machine destroys their whole workspace list.
+
+What stands between the two is the process environment every subprocess in this
+session inherits, set once before collection. These tests run on the *default*
+suite -- not under `-m e2e` -- so removing the scoping breaks the ordinary test
+run rather than waiting for the run that would do the damage.
+"""
+
+import os
+import tempfile
+from pathlib import Path
+
+from devpod_scoping import (
+    DEVPOD_HOME_VAR,
+    DEVPOD_SSH_CONFIG_VAR,
+    scope_devpod_to_this_run,
+)
+
+
+def test_devpod_commands_are_pointed_away_from_the_developers_devpod_home():
+    """DEVPOD_HOME is what scopes `devpod list`, and so `dl --purge`'s blast radius."""
+    scoped = os.environ.get(DEVPOD_HOME_VAR)
+
+    assert scoped, "the suite must set DEVPOD_HOME; without it devpod reads ~/.devpod"
+
+    scoped_path = Path(scoped).resolve()
+    home = Path.home().resolve()
+    assert scoped_path != home / ".devpod"
+    assert home not in scoped_path.parents
+
+
+def test_devpod_up_is_pointed_away_from_the_developers_ssh_config():
+    """`devpod up` rewrites ~/.ssh/config unless DEVPOD_SSH_CONFIG redirects it.
+
+    DEVPOD_HOME does not cover this: devpod resolves the ssh config against the
+    real home either way.
+
+    The claim is deliberately narrower than the variable's name suggests.
+    `--ssh-config` is registered on `devpod up` alone -- measured on devpod
+    v0.26.1, `ssh`, `delete` and `stop` do not accept it and still read the
+    developer's real `~/.ssh/config`. `up` is the subcommand that *writes*, so
+    the harm is covered; nothing here claims the variable is global.
+    """
+    scoped = os.environ.get(DEVPOD_SSH_CONFIG_VAR)
+
+    assert scoped, (
+        "the suite must set DEVPOD_SSH_CONFIG; without it `devpod up` edits ~/.ssh/config"
+    )
+
+    scoped_path = Path(scoped).resolve()
+    home = Path.home().resolve()
+    assert scoped_path != home / ".ssh" / "config"
+    assert home not in scoped_path.parents
+
+
+def test_each_run_gets_a_devpod_home_of_its_own(tmp_path, monkeypatch):
+    """Two concurrent runs must not share a namespace.
+
+    The e2e workspace ids are hardcoded constants, so a namespace shared between
+    runs means each run's teardown force-deletes the other's workspaces. A
+    per-run directory is what makes those constants private to the run.
+
+    This drives `scope_devpod_to_this_run` itself, twice, rather than the
+    directory-making it happens to use -- so it goes red if the per-run
+    directory is ever "simplified" into a fixed path, which is the regression it
+    exists to catch. `tempfile.tempdir` sends the two throwaway homes under
+    `tmp_path`, and `monkeypatch.setenv` restores the session's real scoping
+    afterwards, since the function under test writes to `os.environ` by design.
+    """
+    monkeypatch.setattr(tempfile, "tempdir", str(tmp_path))
+    monkeypatch.setenv(DEVPOD_HOME_VAR, "")
+    monkeypatch.setenv(DEVPOD_SSH_CONFIG_VAR, "")
+
+    first = scope_devpod_to_this_run()
+    first_env = os.environ[DEVPOD_HOME_VAR]
+    second = scope_devpod_to_this_run()
+    second_env = os.environ[DEVPOD_HOME_VAR]
+
+    assert first != second
+    assert first_env != second_env
+    assert os.environ[DEVPOD_SSH_CONFIG_VAR] == str(second / "ssh_config")