From f3373edf38a8705ab5011ee3bd7116a23ced3d02 Mon Sep 17 00:00:00 2001 From: Paulo Date: Sun, 6 Sep 2026 18:24:20 +0200 Subject: [PATCH] Forget a deleted host in the exchange, and prove the janitor's teardown The exchange drops the secrets of every host that has no row on each pass of its timer, so no fetch runs for a dead box. A test proves that the janitor removes an expired host's secrets before its VM, through the same path as an API delete. On docker-sbx a value directory that cannot be removed at teardown is a provider error now, so the row stays for a retry and no value file outlives its row. A missing directory is a removed one. The docs say that the janitor deletes an expired host the same way, and that a sandbox must be removed through drukbox, never with sbx rm, since its secrets would stay in sbx's store until its row expires. --- docs/architecture.md | 4 +++- docs/deploy.md | 9 +++++--- src/hosts/tests/test_janitor.py | 23 +++++++++++++++++-- src/providers/docker_sbx/secrets.py | 7 +++++- .../docker_sbx/tests/test_secrets.py | 19 +++++++++++++++ src/secrets_exchange/app.py | 10 ++++---- src/secrets_exchange/secrets.py | 5 ++++ src/secrets_exchange/tests/test_app.py | 22 ++++++++++++++++++ 8 files changed, 88 insertions(+), 11 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 61aef90..98ae7f9 100644 --- a/docs/architecture.md +++ b/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...`. 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 diff --git a/docs/deploy.md b/docs/deploy.md index e92dc12..941f8e9 100644 --- a/docs/deploy.md +++ b/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 diff --git a/src/hosts/tests/test_janitor.py b/src/hosts/tests/test_janitor.py index 6507df1..4e64d2d 100644 --- a/src/hosts/tests/test_janitor.py +++ b/src/hosts/tests/test_janitor.py @@ -1,5 +1,5 @@ from datetime import UTC, datetime, timedelta -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, MagicMock from uuid6 import uuid7 @@ -7,6 +7,7 @@ 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() diff --git a/src/providers/docker_sbx/secrets.py b/src/providers/docker_sbx/secrets.py index 921f180..304226c 100644 --- a/src/providers/docker_sbx/secrets.py +++ b/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.""" diff --git a/src/providers/docker_sbx/tests/test_secrets.py b/src/providers/docker_sbx/tests/test_secrets.py index 4f81b4f..8e36706 100644 --- a/src/providers/docker_sbx/tests/test_secrets.py +++ b/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() diff --git a/src/secrets_exchange/app.py b/src/secrets_exchange/app.py index 8cec027..3c9ac9b 100644 --- a/src/secrets_exchange/app.py +++ b/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: diff --git a/src/secrets_exchange/secrets.py b/src/secrets_exchange/secrets.py index a844a2c..e427c37 100644 --- a/src/secrets_exchange/secrets.py +++ b/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"]) diff --git a/src/secrets_exchange/tests/test_app.py b/src/secrets_exchange/tests/test_app.py index 75cae6d..0dfb227 100644 --- a/src/secrets_exchange/tests/test_app.py +++ b/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: