diff --git a/docs/architecture.md b/docs/architecture.md index 38a06e4..b1a3865 100644 --- a/docs/architecture.md +++ b/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 diff --git a/docs/deploy.md b/docs/deploy.md index e757f8b..4ff00b4 100644 --- a/docs/deploy.md +++ b/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 diff --git a/src/hosts/service.py b/src/hosts/service.py index 30602f0..fd79e78 100644 --- a/src/hosts/service.py +++ b/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 diff --git a/src/hosts/tests/test_secrets.py b/src/hosts/tests/test_secrets.py index 35312bc..8f1ed3d 100644 --- a/src/hosts/tests/test_secrets.py +++ b/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,6 +39,7 @@ 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 @@ -44,8 +47,8 @@ async def put_secret( 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: diff --git a/src/providers/capabilities.py b/src/providers/capabilities.py index 4eb5dc4..42ae2eb 100644 --- a/src/providers/capabilities.py +++ b/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: diff --git a/src/providers/docker_sbx/api.py b/src/providers/docker_sbx/api.py index b15bd60..a29b6bc 100644 --- a/src/providers/docker_sbx/api.py +++ b/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) diff --git a/src/providers/docker_sbx/secrets.py b/src/providers/docker_sbx/secrets.py index 045e5b8..756cd8b 100644 --- a/src/providers/docker_sbx/secrets.py +++ b/src/providers/docker_sbx/secrets.py @@ -1,7 +1,8 @@ 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 @@ -9,9 +10,9 @@ 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 diff --git a/src/providers/docker_sbx/tests/test_api.py b/src/providers/docker_sbx/tests/test_api.py index 938ab2a..fa438b4 100644 --- a/src/providers/docker_sbx/tests/test_api.py +++ b/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"}]}' diff --git a/src/providers/docker_sbx/tests/test_secrets.py b/src/providers/docker_sbx/tests/test_secrets.py index 0c2692a..fdb7ec1 100644 --- a/src/providers/docker_sbx/tests/test_secrets.py +++ b/src/providers/docker_sbx/tests/test_secrets.py @@ -5,7 +5,7 @@ import pytest -from host_secrets.catalog import CATALOG +from host_secrets.catalog import CATALOG, Service, Upstream from host_secrets.placeholder import Placeholder from providers.docker_sbx.exceptions import DockerSbxTransportError from providers.docker_sbx.secrets import SbxInjection @@ -18,6 +18,7 @@ def _api_mock() -> MagicMock: api.set_custom_secret = AsyncMock() api.remove_secret = AsyncMock() api.remove_custom_secret = AsyncMock() + api.custom_placeholders = AsyncMock(return_value=[]) return api @@ -76,35 +77,58 @@ async def test_a_new_value_replaces_the_file_and_the_secret(tmp_path: Path) -> N assert api.set_custom_secret.await_count == 2 -async def test_delete_removes_a_custom_secret_by_placeholder_and_its_file(tmp_path: Path) -> None: +async def test_a_custom_entry_named_github_is_a_custom_secret_on_its_host(tmp_path: Path) -> 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" - ) + placeholder = Placeholder.mint(uuid.uuid4(), "github") + enterprise = Service("GH_ENTERPRISE_TOKEN", (Upstream("github.enterprise.example"),)) - await injection.delete_secret(vm="sb-one", placeholder=placeholder) + environment = await SbxInjection(api, tmp_path).put_secret( + vm="sb-one", service=enterprise, placeholder=placeholder, value="ghe_real" + ) - api.remove_custom_secret.assert_awaited_once_with( - sandbox="sb-one", placeholder=str(placeholder) + api.set_custom_secret.assert_awaited_once_with( + sandbox="sb-one", + hosts=["github.enterprise.example"], + env="GH_ENTERPRISE_TOKEN", + placeholder=str(placeholder), + command=f"cat {tmp_path / 'sb-one' / 'github'}", ) - assert not (tmp_path / "sb-one" / "anthropic").exists() + api.set_secret.assert_not_awaited() + assert environment == {"GH_ENTERPRISE_TOKEN": str(placeholder)} -async def test_delete_removes_a_github_secret_by_service_name_and_its_file(tmp_path: Path) -> None: +async def test_delete_secrets_removes_the_scope_and_the_files(tmp_path: Path) -> None: api = _api_mock() injection = SbxInjection(api, tmp_path) - placeholder = Placeholder.mint(uuid.uuid4(), "github") + anthropic = Placeholder.mint(uuid.uuid4(), "anthropic") + github = Placeholder.mint(uuid.uuid4(), "github") await injection.put_secret( - vm="sb-one", service=CATALOG["github"], placeholder=placeholder, value="ghs_real" + vm="sb-one", service=CATALOG["anthropic"], placeholder=anthropic, value="sk-ant-real" ) + await injection.put_secret( + vm="sb-one", service=CATALOG["github"], placeholder=github, value="ghs_real" + ) + api.custom_placeholders.return_value = [str(anthropic), "drk.x.other.y"] + + await injection.delete_secrets(vm="sb-one") + + api.remove_secret.assert_awaited_once_with("github", sandbox="sb-one") + api.custom_placeholders.assert_awaited_once_with(sandbox="sb-one") + assert [call.kwargs for call in api.remove_custom_secret.await_args_list] == [ + {"sandbox": "sb-one", "placeholder": str(anthropic)}, + {"sandbox": "sb-one", "placeholder": "drk.x.other.y"}, + ] + assert not (tmp_path / "sb-one").exists() + + +async def test_delete_secrets_can_run_again_after_a_partial_teardown(tmp_path: Path) -> None: + api = _api_mock() - await injection.delete_secret(vm="sb-one", placeholder=placeholder) + await SbxInjection(api, tmp_path).delete_secrets(vm="sb-one") api.remove_secret.assert_awaited_once_with("github", sandbox="sb-one") api.remove_custom_secret.assert_not_awaited() - assert not (tmp_path / "sb-one" / "github").exists() + assert not (tmp_path / "sb-one").exists() @pytest.mark.parametrize("service", ["github", "anthropic"]) @@ -113,7 +137,6 @@ async def test_a_cli_failure_is_a_provider_error(tmp_path: Path, service: str) - api.set_secret.side_effect = DockerSbxTransportError("daemon unavailable") api.set_custom_secret.side_effect = DockerSbxTransportError("daemon unavailable") api.remove_secret.side_effect = DockerSbxTransportError("daemon unavailable") - api.remove_custom_secret.side_effect = DockerSbxTransportError("daemon unavailable") injection = SbxInjection(api, tmp_path) placeholder = Placeholder.mint(uuid.uuid4(), service) @@ -122,4 +145,6 @@ async def test_a_cli_failure_is_a_provider_error(tmp_path: Path, service: str) - vm="sb-one", service=CATALOG[service], placeholder=placeholder, value="v" ) with pytest.raises(ProviderTransportError, match="daemon unavailable"): - await injection.delete_secret(vm="sb-one", placeholder=placeholder) + await injection.delete_secrets(vm="sb-one") + # The value file stays for the retry. + assert (tmp_path / "sb-one" / service).exists() diff --git a/src/providers/tests/test_capabilities.py b/src/providers/tests/test_capabilities.py index 810f713..547e226 100644 --- a/src/providers/tests/test_capabilities.py +++ b/src/providers/tests/test_capabilities.py @@ -143,9 +143,7 @@ async def test_the_proxy_injection_hands_out_a_public_certificate_and_nothing_el async def test_the_proxy_injection_holds_no_value_and_has_nothing_to_delete() -> None: assert ProxyInjection.needs_value is False - await ProxyInjection().delete_secret( - vm="sb-one", placeholder=Placeholder.mint(uuid.uuid4(), "anthropic") - ) + await ProxyInjection().delete_secrets(vm="sb-one") @pytest.mark.parametrize(