Forget a deleted host in the exchange, and prove the janitor's teardown

docs/architecture.md

@@ -127,6 +127,7 @@ value is still valid. With nothing valid in memory it answers `503`.
 A provider that holds the value never asks the exchange. For it, a timer
 fetches a fresh value when less than a minute of the pushed one remains and
 hands it to `push_secret`. A push that fails waits like a fetch that fails.
+A host that is gone is forgotten on the next pass.
 
 Provisioning mints a placeholder per secret. The placeholder names the host
 and the service, `drk.<host id>.<service>.<random>`. The entry keeps only a
@@ -153,7 +154,8 @@ the value in sbx's own secret store for that sandbox, and sbx's proxy swaps
 the placeholder on the way out. sbx reads the value file at each use, so a
 pushed value is a rewritten file. 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.
+never reads the row's secrets, so a lost key cannot block a teardown. The
+janitor deletes an expired host through the same path.
 
 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

@@ -370,9 +370,12 @@ 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.
+host deletion removes every secret in the sandbox's scope and the files. The
+janitor deletes an expired host the same way. Remove a sandbox through
+drukbox, never with `sbx rm`, or its secrets stay in sbx's store until its
+row expires. 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/tests/test_janitor.py

@@ -1,12 +1,13 @@
 from datetime import UTC, datetime, timedelta
-from unittest.mock import AsyncMock
+from unittest.mock import AsyncMock, MagicMock
 
 from uuid6 import uuid7
 
 from core.database import async_session_factory
 from hosts.janitor import reap_expired_hosts
 from hosts.models import Host, HostStatus
 from hosts.service import HostService, utc_now
+from hosts.tests.conftest import StubVMProvider
 from providers.exe.settings import ExeSettings
 
 
@@ -16,13 +17,14 @@ async def _create_host(
     status: str,
     expires_at: datetime | None,
     tailscale_device_id: str | None = None,
+    provider: str = "exe",
 ) -> Host:
     now = utc_now()
     host = Host(
         id=uuid7(),
         name=name,
         status=status,
-        provider="exe",
+        provider=provider,
         image=ExeSettings().default_image,  # pyright: ignore[reportCallIssue]
         env={},
         internal_ssh_host=f"{name}.example.ts.net",
@@ -60,6 +62,23 @@ async def test_janitor_deletes_host_past_expires_at(monkeypatch):
         assert await session.get(Host, host.id) is None
 
 
+async def test_janitor_removes_the_secrets_of_an_expired_host_before_its_vm(
+    stub_provider: StubVMProvider,
+) -> None:
+    stub_provider.secrets = MagicMock(delete_secrets=AsyncMock())
+    host = await _create_host(
+        name="sb-expired",
+        status=HostStatus.ACTIVE.value,
+        expires_at=datetime.now(UTC) - timedelta(minutes=1),
+        provider="stub",
+    )
+
+    assert await reap_expired_hosts() == [host.id]
+
+    stub_provider.secrets.delete_secrets.assert_awaited_once_with(vm="sb-expired")
+    assert stub_provider.deleted == ["sb-expired"]
+
+
 async def test_janitor_leaves_unexpired_hosts_alone(monkeypatch):
     mocked_delete_device = AsyncMock()
     mocked_delete_vm = AsyncMock()

src/providers/docker_sbx/secrets.py

@@ -71,7 +71,12 @@ async def delete_secrets(self, *, vm: str) -> None:
                 await self.api.remove_custom_secret(sandbox=vm, placeholder=placeholder)
         except DockerSbxProviderError as exc:
             raise ProviderTransportError(str(exc)) from exc
-        shutil.rmtree(self.secrets_root / vm, ignore_errors=True)
+        try:
+            shutil.rmtree(self.secrets_root / vm)
+        except FileNotFoundError:
+            return
+        except OSError as exc:
+            raise ProviderCommandError(f"cannot remove the value files: {exc}") from exc
 
     def write_value(self, vm: str, name: str, value: str) -> Path:
         """Replace the value file whole, so sbx never reads a half-written one."""

src/providers/docker_sbx/tests/test_secrets.py

@@ -153,6 +153,25 @@ async def test_delete_secrets_removes_the_scope_and_the_files(tmp_path: Path) ->
     assert not (tmp_path / "sb-one").exists()
 
 
+async def test_value_files_that_cannot_be_removed_keep_the_secrets_for_a_retry(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    api = _api_mock()
+    injection = SbxInjection(api, tmp_path)
+    placeholder = Placeholder.mint(uuid.uuid4(), "anthropic")
+    await injection.put_secret(
+        vm="sb-one", service=CATALOG["anthropic"], placeholder=placeholder, value="sk-ant-real"
+    )
+
+    def refuse(path: Path) -> None:
+        raise PermissionError(f"{path}: operation not permitted")
+
+    monkeypatch.setattr("providers.docker_sbx.secrets.shutil.rmtree", refuse)
+
+    with pytest.raises(ProviderCommandError, match="value files"):
+        await injection.delete_secrets(vm="sb-one")
+
+
 async def test_delete_secrets_can_run_again_after_a_partial_teardown(tmp_path: Path) -> None:
     api = _api_mock()
 

src/secrets_exchange/app.py

@@ -54,11 +54,13 @@ async def push_on_expiry(secrets: Secrets) -> None:
 
 
 async def push_active_hosts(secrets: Secrets) -> None:
-    """Side by side, so one slow issuer delays no other host."""
+    """Side by side, so one slow issuer delays no other host. A host that is
+    gone is forgotten first."""
     async with async_session_factory() as session:
-        active = select(Host).where(Host.status == HostStatus.ACTIVE.value)
-        hosts = (await session.execute(active)).scalars().all()
-    await asyncio.gather(*(push_to_host(secrets, host) for host in hosts))
+        hosts = (await session.execute(select(Host))).scalars().all()
+    secrets.forget_deleted_hosts({host.id for host in hosts})
+    active = [host for host in hosts if host.status == HostStatus.ACTIVE.value]
+    await asyncio.gather(*(push_to_host(secrets, host) for host in active))
 
 
 async def push_to_host(secrets: Secrets, host: Host) -> None:

src/secrets_exchange/secrets.py

@@ -137,6 +137,11 @@ def __init__(self, client: httpx.AsyncClient) -> None:
         self._client = client
         self._refreshable: dict[tuple[uuid.UUID, str], RefreshableSecret] = {}
 
+    def forget_deleted_hosts(self, existing: set[uuid.UUID]) -> None:
+        self._refreshable = {
+            key: secret for key, secret in self._refreshable.items() if key[0] in existing
+        }
+
     async def current(self, host_id: uuid.UUID, service: str, entry: dict[str, Any]) -> Secret:
         if "value" in entry:
             return Secret(value=entry["value"])

src/secrets_exchange/tests/test_app.py

@@ -246,6 +246,28 @@ async def test_the_timer_pushes_issuer_values_to_a_provider_that_holds_them(edge
     )
 
 
+@respx.mock
+@pytest.mark.usefixtures("stub_provider")
+async def test_the_timer_forgets_a_deleted_host(edge) -> None:
+    injection = MagicMock(needs_value=True, push_secret=AsyncMock())
+    get_vm_provider("stub").secrets = injection
+    host_id = uuid.uuid4()
+    await _create_host(
+        host_id, {"anthropic": {"issuer": ISSUER, "placeholder_fingerprint": "a"}}, provider="stub"
+    )
+    respx.get(ISSUER["url"]).respond(json={"value": "sk-ant-fresh"})
+    await push_active_hosts(app.state.secrets)
+    assert (host_id, "anthropic") in app.state.secrets._refreshable
+
+    async with async_session_factory() as session:
+        await session.delete(await session.get(Host, host_id))
+        await session.commit()
+    await push_active_hosts(app.state.secrets)
+
+    assert (host_id, "anthropic") not in app.state.secrets._refreshable
+    injection.push_secret.assert_awaited_once()
+
+
 @respx.mock
 @pytest.mark.usefixtures("stub_provider")
 async def test_one_host_in_trouble_costs_no_other_host_its_value(edge, caplog) -> None: