docker-sbx: sbx's own secrets, per sandbox, and teardown

docs/architecture.md

@@ -146,7 +146,9 @@ the proxy. Every connection goes to the address the proxy checked, and the
 upstream certificate is checked against the CONNECT host. A request whose
 `Host` differs from the CONNECT host is refused. On docker-sbx the sandbox gets only the placeholder. Drukbox puts
 the value in sbx's own secret store for that sandbox, and sbx's proxy swaps
-the placeholder on the way out.
+the placeholder on the way out. Host deletion calls `delete_secrets` for the
+box before the VM goes, so nothing the seam put anywhere outlives the box. It
+never reads the row's secrets, so a lost key cannot block a teardown.
 
 A template is a persistent provider image keyed by provider, base image,
 and setup-script hash. `POST /templates` creates a `building` record and

docs/deploy.md

@@ -364,8 +364,14 @@ from the sandbox tag to the proxy host:
 A sandbox then reaches that port on that host and nothing else there. A
 Docker Sandboxes sandbox dials nothing. Drukbox puts each value in sbx's own
 secret store for that sandbox, and sbx's proxy swaps the placeholder. The
-value files that sbx reads live in a `secrets` directory under
-`DOCKER_SBX_WORKSPACE_ROOT`, beside the workspaces and never inside one.
+`github` service is sbx's own `github` secret, which covers git and gh. Any
+other service, and a custom entry that names a host of its own, is a custom
+secret on its hosts. The value files that sbx reads live in a `secrets`
+directory under `DOCKER_SBX_WORKSPACE_ROOT`, beside the workspaces and never
+inside one. sbx keeps a sandbox's secrets after the sandbox is removed, so
+host deletion removes every secret in the sandbox's scope and the files. Do
+not set a global sbx secret for a destination drukbox manages. sbx applies
+the global one first, and drukbox's value never reaches the sandbox.
 
 Give secrets to `POST /hosts`. Provisioning delivers the placeholders in the
 sandbox's boot environment, on every provider, the same way as `env`. A

src/hosts/service.py

@@ -461,8 +461,11 @@ async def delete_host(
                 host.tailscale_device_id = None
                 host.updated_at = utc_now()
                 await self.session.commit()
+            # Secrets go before the VM. A provider failure keeps the row for a retry.
+            vm = get_vm_provider(host.provider)
+            await vm.secrets.delete_secrets(vm=host.name)
             try:
-                await get_vm_provider(host.provider).delete_vm(host.name)
+                await vm.delete_vm(host.name)
             except ProviderNotFoundError:
                 # VM already absent at the provider — exe.dev may have evicted
                 # it, or a previous delete partially succeeded. Treat as done

src/hosts/tests/test_secrets.py

@@ -5,6 +5,7 @@
 import pytest
 import respx
 from sqlalchemy import select, text
+from sqlalchemy_encrypted_field import SecretDecryptError
 
 from core.database import async_session_factory
 from core.settings import Settings, get_settings
@@ -16,6 +17,7 @@
 from hosts.tests.conftest import StubVMProvider
 from providers.base import VMCreateResult
 from providers.capabilities import SecretInjectionCapability
+from providers.exceptions import ProviderTransportError
 
 ISSUER = {"url": "https://mint.test/box/github", "headers": {"X-Key": "k"}, "refresh": "1h"}
 SECRETS = {
@@ -37,15 +39,16 @@ class RecordingInjection(SecretInjectionCapability):
 
     def __init__(self) -> None:
         self.values: dict[str, str] = {}
+        self.deleted: list[str] = []
 
     async def put_secret(
         self, *, vm: str, service: Service, placeholder: Placeholder, value: str
     ) -> dict[str, str]:
         self.values[placeholder.service] = value
         return {service.auth_variable: str(placeholder)}
 
-    async def delete_secret(self, *, vm: str, placeholder: Placeholder) -> None:
-        return
+    async def delete_secrets(self, *, vm: str) -> None:
+        self.deleted.append(vm)
 
 
 @pytest.fixture
@@ -220,6 +223,67 @@ async def test_a_wrong_issuer_answer_at_boot_never_reaches_the_log_or_the_host(
     assert "secret-xyz" not in str(failure.value)
 
 
+async def test_teardown_removes_the_secrets_before_the_vm(
+    settings: Settings, create_vm: AsyncMock, stub_provider: StubVMProvider
+) -> None:
+    recording = RecordingInjection()
+    stub_provider.secrets = recording
+    async with async_session_factory() as session:
+        host = await HostService(session, settings=settings).create_host(
+            env={}, secrets={"anthropic": {"value": "sk-ant-real"}}, image=None, provider="stub"
+        )
+
+    async with async_session_factory() as session:
+        assert await HostService(session, settings=settings).delete_host(host.id)
+
+    assert recording.deleted == [host.name]
+    assert stub_provider.deleted == [host.name]
+
+
+async def test_a_secret_that_cannot_be_removed_keeps_the_host_for_a_retry(
+    settings: Settings, create_vm: AsyncMock, stub_provider: StubVMProvider
+) -> None:
+    recording = RecordingInjection()
+    stub_provider.secrets = recording
+    async with async_session_factory() as session:
+        host = await HostService(session, settings=settings).create_host(
+            env={}, secrets={"anthropic": {"value": "sk-ant-real"}}, image=None, provider="stub"
+        )
+    recording.delete_secrets = AsyncMock(side_effect=ProviderTransportError("sbx is down"))
+
+    async with async_session_factory() as session:
+        service = HostService(session, settings=settings)
+        with pytest.raises(ProviderTransportError):
+            await service.delete_host(host.id)
+        assert await service.get_host(host.id)
+    assert stub_provider.deleted == []
+
+
+async def test_teardown_needs_no_secrets_key(
+    settings: Settings,
+    create_vm: AsyncMock,
+    stub_provider: StubVMProvider,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """A row whose secrets no longer decrypt still goes, VM and all."""
+    stub_provider.secrets = RecordingInjection()
+    async with async_session_factory() as session:
+        host = await HostService(session, settings=settings).create_host(
+            env={}, secrets={"anthropic": {"value": "sk-ant-real"}}, image=None, provider="stub"
+        )
+    monkeypatch.setenv("SECRETS_KEY", "MTExMTExMTExMTExMTExMTExMTExMTExMTExMTExMTE=")
+    get_settings.cache_clear()
+
+    async with async_session_factory() as session:
+        service = HostService(session, settings=settings)
+        stored = await service.get_host(host.id)
+        assert stored
+        with pytest.raises(SecretDecryptError):
+            dict(stored.secrets)
+        assert await service.delete_host(host.id)
+    assert stub_provider.deleted == [host.name]
+
+
 async def test_secrets_on_a_provider_that_holds_the_value_need_no_proxy(
     client, monkeypatch: pytest.MonkeyPatch, stub_provider: StubVMProvider
 ) -> None:

src/providers/capabilities.py

@@ -30,7 +30,8 @@ def resolve_capability(provider, capability: type[CapabilityT]) -> CapabilityT:
 
 class SecretInjectionCapability(abc.ABC):
     """How a secret reaches one provider's boxes. ``put_secret`` returns the
-    environment the box needs."""
+    environment the box needs. ``delete_secrets`` gets the box alone, so
+    teardown never reads the host row."""
 
     # sbx keeps the value in its own store. The proxy needs only the placeholder.
     needs_value: ClassVar[bool]
@@ -46,7 +47,7 @@ async def put_secret(
     ) -> dict[str, str]: ...
 
     @abc.abstractmethod
-    async def delete_secret(self, *, vm: str, placeholder: Placeholder) -> None: ...
+    async def delete_secrets(self, *, vm: str) -> None: ...
 
 
 class ProxyInjection(SecretInjectionCapability):
@@ -76,7 +77,7 @@ async def put_secret(
             "NODE_EXTRA_CA_CERTS": environment.PROXY_CA_PATH,
         }
 
-    async def delete_secret(self, *, vm: str, placeholder: Placeholder) -> None:
+    async def delete_secrets(self, *, vm: str) -> None:
         return
 
     def get_public_certificate(self) -> bytes:

src/providers/docker_sbx/api.py

@@ -86,6 +86,21 @@ async def set_custom_secret(
             command,
         )
 
+    async def custom_placeholders(self, *, sandbox: str) -> list[str]:
+        """``sbx secret ls`` has no JSON. The custom rows follow a ``CUSTOM
+        SECRETS`` header, with the placeholder in the fourth column."""
+        output = await self._run("secret", "ls", "--sandbox", sandbox)
+        placeholders: list[str] = []
+        custom = False
+        for line in output.splitlines():
+            if line.startswith("CUSTOM SECRETS"):
+                custom = True
+                continue
+            columns = re.split(r"\s{2,}", line.strip())
+            if custom and len(columns) >= 4 and columns[0] == sandbox:
+                placeholders.append(columns[3])
+        return placeholders
+
     async def remove_secret(self, service: str, *, sandbox: str) -> None:
         # Without -f the CLI waits for a confirmation.
         await self._run("secret", "rm", "-f", service, "--sandbox", sandbox)

src/providers/docker_sbx/secrets.py

@@ -1,17 +1,18 @@
 import shlex
+import shutil
 from pathlib import Path
 
-from host_secrets.catalog import Service
+from host_secrets.catalog import CATALOG, Service
 from host_secrets.placeholder import Placeholder
 from providers.capabilities import SecretInjectionCapability
 from providers.exceptions import ProviderTransportError
 
 from .api import SbxCLI
 from .exceptions import DockerSbxProviderError
 
-# sbx's own secret for these covers git and gh. Every other service is a
-# custom secret on its host.
-_NATIVE_SERVICES = frozenset({"github"})
+# sbx's own secret for these covers git and gh, when the entry reaches the
+# service itself. Every other entry is a custom secret on its hosts.
+_NATIVE_SERVICES = {"github": CATALOG["github"]}
 
 
 class SbxInjection(SecretInjectionCapability):
@@ -41,7 +42,7 @@ async def put_secret(
         path.write_text(value)
         command = f"cat {shlex.quote(str(path))}"
         try:
-            if placeholder.service in _NATIVE_SERVICES:
+            if _NATIVE_SERVICES.get(placeholder.service) == service:
                 await self.api.set_secret(placeholder.service, sandbox=vm, command=command)
             else:
                 await self.api.set_custom_secret(
@@ -55,15 +56,17 @@ async def put_secret(
             raise ProviderTransportError(str(exc)) from exc
         return {service.auth_variable: str(placeholder)}
 
-    async def delete_secret(self, *, vm: str, placeholder: Placeholder) -> None:
+    async def delete_secrets(self, *, vm: str) -> None:
+        """sbx keeps a sandbox's secrets after the sandbox is removed, and
+        answers a missing one with success, so this can run again."""
         try:
-            if placeholder.service in _NATIVE_SERVICES:
-                await self.api.remove_secret(placeholder.service, sandbox=vm)
-            else:
-                await self.api.remove_custom_secret(sandbox=vm, placeholder=str(placeholder))
+            for name in _NATIVE_SERVICES:
+                await self.api.remove_secret(name, sandbox=vm)
+            for placeholder in await self.api.custom_placeholders(sandbox=vm):
+                await self.api.remove_custom_secret(sandbox=vm, placeholder=placeholder)
         except DockerSbxProviderError as exc:
             raise ProviderTransportError(str(exc)) from exc
-        self.value_path(vm, placeholder.service).unlink()
+        shutil.rmtree(self.secrets_root / vm, ignore_errors=True)
 
     def value_path(self, vm: str, service: str) -> Path:
         return self.secrets_root / vm / service

src/providers/docker_sbx/tests/test_api.py

@@ -106,6 +106,44 @@ async def fake_exec(*args, **kwargs):
     )
 
 
+LISTING = """SCOPE  TYPE  NAME  SECRET
+sb-test  service  github  (stored)
+
+CUSTOM SECRETS
+SCOPE  TARGETS  ENV  PLACEHOLDER  SECRET
+sb-test  api.anthropic.com  ANTHROPIC_AUTH_TOKEN  drk.0123.anthropic.abc  cmd (on-demand)
+sb-test  api.acme.test, acme.example  ACME_TOKEN  drk.0123.acme.def  cmd (on-demand)
+"""
+
+
+@pytest.mark.asyncio
+async def test_custom_placeholders_reads_the_placeholder_of_each_custom_row(
+    monkeypatch,
+):
+    captured: dict = {}
+
+    async def fake_exec(*args, **kwargs):
+        captured["args"] = args
+        return _process(stdout=LISTING.encode())
+
+    monkeypatch.setattr("providers.docker_sbx.api.asyncio.create_subprocess_exec", fake_exec)
+
+    placeholders = await SbxCLI().custom_placeholders(sandbox="sb-test")
+
+    assert captured["args"] == ("sbx", "secret", "ls", "--sandbox", "sb-test")
+    assert placeholders == ["drk.0123.anthropic.abc", "drk.0123.acme.def"]
+
+
+@pytest.mark.asyncio
+async def test_custom_placeholders_of_an_empty_scope(monkeypatch):
+    async def fake_exec(*args, **kwargs):
+        return _process(stdout=b'No secrets found for scope "sb-test".\n')
+
+    monkeypatch.setattr("providers.docker_sbx.api.asyncio.create_subprocess_exec", fake_exec)
+
+    assert await SbxCLI().custom_placeholders(sandbox="sb-test") == []
+
+
 @pytest.mark.asyncio
 async def test_sandbox_count_reads_the_listing(monkeypatch):
     listing = b'{"sandboxes": [{"name": "sb-a"}, {"name": "sb-b"}]}'