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: