diff --git a/.github/workflows/on-pull-request.yml b/.github/workflows/on-pull-request.yml index 51172f0..a25431b 100644 --- a/.github/workflows/on-pull-request.yml +++ b/.github/workflows/on-pull-request.yml @@ -187,6 +187,18 @@ jobs: docker logs drukbox-api-test exit 1 + - name: Start the secrets exchange + run: | + docker exec --detach drukbox-api-test \ + /bin/sh -c '.venv/bin/python -m secrets_exchange > /tmp/secrets_exchange.log 2>&1' + for _ in $(seq 1 60); do + curl -fsS http://127.0.0.1:8781/healthz >/dev/null 2>&1 && exit 0 + sleep 1 + done + echo "secrets exchange failed to start" >&2 + docker exec drukbox-api-test cat /tmp/secrets_exchange.log + exit 1 + - name: Set up Node uses: actions/setup-node@v6 with: @@ -211,7 +223,9 @@ jobs: - name: Dump drukbox log on failure if: failure() - run: docker logs drukbox-api-test || true + run: | + docker logs drukbox-api-test || true + docker exec drukbox-api-test cat /tmp/secrets_exchange.log || true - name: Remove drukbox container if: always() diff --git a/Makefile b/Makefile index 1d56caa..5df2bdb 100644 --- a/Makefile +++ b/Makefile @@ -7,4 +7,4 @@ LOCAL_ENV = \ .PHONY: dev dev: $(LOCAL_ENV) uv run alembic upgrade head - $(LOCAL_ENV) uv run uvicorn api.app:app + $(LOCAL_ENV) uv run python -m secrets_exchange & $(LOCAL_ENV) uv run uvicorn api.app:app diff --git a/api-tests/tests/full-api.spec.js b/api-tests/tests/full-api.spec.js index cc8123d..2a4c871 100644 --- a/api-tests/tests/full-api.spec.js +++ b/api-tests/tests/full-api.spec.js @@ -21,6 +21,7 @@ const EXPECTED_OPENAPI_OPERATIONS = [ "POST /http-proxies/{name}/hosts/{host_id}", "POST /hosts", "POST /hosts/{host_id}/renew", + "POST /hosts/{host_id}/secrets/{service}/refresh", "POST /templates", "POST /service-accounts", ]; @@ -136,6 +137,12 @@ test.describe("Drukbox API", () => { } }); + test("doctor reports all dependencies healthy", async () => { + const report = await expectJson(await api.get("/doctor"), 200); + expect(report.ok).toBe(true); + expect(report.checks.find((check) => check.name === "exchange").status).toBe("ok"); + }); + test("GET /hosts requires auth and returns hosts with service auth", async () => { await expectStatus(await publicApi.get("/hosts"), 401); @@ -207,6 +214,14 @@ test.describe("Drukbox API", () => { expect(missing.detail).toBe("host not found"); }); + test("refresh requires auth and returns exchange errors", async () => { + const path = `/hosts/${createdHost.id}/secrets/anthropic/refresh`; + await expectStatus(await publicApi.post(path), 401); + await expectStatus(await badTokenApi.post(path), 403); + await expectStatus(await api.post(path), 409); + await expectStatus(await api.post(`/hosts/${createdHost.id}/secrets/missing/refresh`), 404); + }); + test("created host is observably active", async () => { test.setTimeout(config.hostActiveTimeoutMs); diff --git a/deploy/proxy/swap.py b/deploy/proxy/swap.py index 41e8d63..88e0cbc 100644 --- a/deploy/proxy/swap.py +++ b/deploy/proxy/swap.py @@ -1,6 +1,6 @@ """A mitmproxy addon: swaps a sandbox's placeholder for the real credential. - mitmdump -s /addon/swap.py --set exchange_url=http://exchange:8781 + mitmdump -s /addon/swap.py --set exchange_url=http://127.0.0.1:8781 TLS is terminated for hosts with a registered secret only. A loopback, private, link-local, or metadata destination is refused, for HTTP and CONNECT. diff --git a/docs/api.md b/docs/api.md index 11a2dae..d282604 100644 --- a/docs/api.md +++ b/docs/api.md @@ -31,6 +31,7 @@ migration, has no token, and cannot be removed. Admin keys act as it. `DELETE /templates/{id}` - `POST /http-proxies` · `DELETE /http-proxies/{name}` · `POST|DELETE /http-proxies/{name}/hosts/{host_id}` +- `POST /hosts/{host_id}/secrets/{service}/refresh` — refresh one secret - `GET /doctor` — read-only dependency diagnostics - `GET /healthz` — unauthenticated liveness probe @@ -39,11 +40,24 @@ or claimed the host, `admin` for an admin key, or `null` for an unclaimed warm host. Callers cannot set it. An `Idempotency-Key` belongs to the service account that first used it. Another one reusing it gets `409`. +## Refresh a host secret + +`POST /hosts/{host_id}/secrets/{service}/refresh` makes the exchange drop +its value for that secret and fetch a new one. A provider that stores the +value receives it at once. The response is `204` with no body. The API +sends no `Authorization` header to the exchange. + +- `404` with `NOT_FOUND`: The host or the secret does not exist. +- `409` with `SECRET_STATIC`: The secret has a static value. +- `503` with `SECRET_REFRESH` and `Retry-After`: The exchange did not + answer, or the issuer or provider did not supply a value. + ## The secrets exchange -The exchange is a second process, `python -m secrets_exchange`, on a private -port with no bearer token. Only the proxy and an issuer inside the deployment -reach it. See [Architecture](architecture.md) for the flow. +The exchange is a second process, `python -m secrets_exchange`, on loopback +with no bearer token. The API and proxy share its network namespace. +Remote callers use the API refresh route. See [Architecture](architecture.md) +for the flow. - `GET /upstreams` — the hosts the proxy terminates TLS for - `GET /authorize` — the proxy's question: the header and the real credential diff --git a/docs/architecture.md b/docs/architecture.md index b312537..8085fcb 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -131,10 +131,12 @@ A host that is gone is forgotten on the next pass. An issuer can end a value before its expiry, as an OAuth provider does when it revokes the previous token at a refresh. The issuer then orders a refresh -at `POST /refresh//`. The exchange forgets the held value, -fetches now, and on a provider that holds the value pushes at once. It answers -`200` after that, and `503` with `Retry-After` when the issuer gave nothing -usable. The order carries no value: the exchange only asks the issuer again. +through the API at `POST /hosts/{host_id}/secrets/{service}/refresh` with a +bearer token. The API passes the order to the exchange on loopback. The +exchange forgets the held value, fetches now, and on a provider that holds +the value pushes at once. The API answers `204` after that, and `503` with +`Retry-After` when the issuer gave nothing usable. The order carries no +value: the exchange only asks the issuer again. Provisioning mints a placeholder per secret. The placeholder names the host and the service, `drk...`. The entry keeps only a @@ -204,7 +206,8 @@ customizes its host — `image`, `env`, `template`, `instance_type`, or ## Diagnostics `GET /doctor` runs one cheap, non-mutating probe per dependency — -database, active provider, Tailscale when enabled — in parallel with a +database, active provider, secrets exchange, Tailscale when enabled — in +parallel with a per-probe timeout. Providers own their probe (`diagnose()`) and their remediation slug (`diagnose_hint`); the endpoint stays a thin orchestrator. It always returns 200; health is the `ok` field in the diff --git a/docs/deploy.md b/docs/deploy.md index 27c0afa..620276a 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -16,7 +16,10 @@ with the same tags. IMAGE=ghcr.io/czpython/drukbox:latest # API (port 8780; /healthz for liveness probes) -docker run --rm -p 8780:8780 --env-file drukbox.env "$IMAGE" +docker run --rm --name drukbox -p 8780:8780 --env-file drukbox.env "$IMAGE" + +# Secrets exchange (loopback, in the API network namespace) +docker run --rm --network container:drukbox --env-file drukbox.env "$IMAGE" .venv/bin/python -m secrets_exchange # Migrations (one-off, before first start and on upgrades) docker run --rm --env-file drukbox.env "$IMAGE" .venv/bin/alembic upgrade head @@ -296,23 +299,40 @@ placeholder for the real credential on the way out. Two pieces run this: - **The exchange process** runs as `python -m secrets_exchange` from this image. The proxy asks it which hosts to terminate, and, for a request with a placeholder, for the header the upstream reads and the real credential. - Its answer is the credential, so bind it where only the proxy can reach it. + Its answer contains a credential. Keep its listener on loopback. The API + carries remote refresh requests to it and checks its health. + +The API, exchange, and proxy must share a network namespace. This Compose +example uses the API's namespace for the other two processes. Only the API +and proxy ports are published: ```yaml services: + api: + image: ghcr.io/czpython/drukbox:latest + env_file: drukbox.env + environment: + SECRETS_PROXY_URL: http://proxy.example:8880 + SECRETS_PROXY_CA_FILE: /secrets-proxy-ca/mitmproxy-ca-cert.pem + ports: + - "8780:8780" + - "8880:8880" + volumes: + - secrets-proxy-ca:/secrets-proxy-ca:ro + exchange: image: ghcr.io/czpython/drukbox:latest command: [".venv/bin/python", "-m", "secrets_exchange"] + network_mode: "service:api" env_file: drukbox.env environment: - SECRETS_EXCHANGE_BIND_HOST: 0.0.0.0 + SECRETS_EXCHANGE_BIND_HOST: 127.0.0.1 proxy: image: ghcr.io/czpython/drukbox/proxy:latest + network_mode: "service:api" environment: - SECRETS_EXCHANGE_URL: http://exchange:8781 - ports: - - "8880:8880" + SECRETS_EXCHANGE_URL: http://127.0.0.1:8781 volumes: - secrets-proxy-ca:/home/mitmproxy/.mitmproxy @@ -320,21 +340,25 @@ volumes: secrets-proxy-ca: ``` -The exchange binds `0.0.0.0` inside the compose network and publishes no -port, so only the proxy reaches it. The API reads the public certificate of -the CA from the same volume, at `SECRETS_PROXY_CA_FILE`, and hands it to -every sandbox with secrets: +A recreated `api` container gets a new network namespace and the other two +stay in the old one. After a change to `api`, recreate all three: +`docker compose up -d --force-recreate api exchange proxy`. -```yaml - api: - image: ghcr.io/czpython/drukbox:latest - env_file: drukbox.env - environment: - SECRETS_PROXY_URL: http://proxy.example:8880 - SECRETS_PROXY_CA_FILE: /secrets-proxy-ca/mitmproxy-ca-cert.pem - volumes: - - secrets-proxy-ca:/secrets-proxy-ca:ro -``` +Use Postgres for the shared database. Set `SECRETS_PROXY_URL` to the proxy +address that sandboxes can contact. Apply the deployment's API and proxy +access rules to the published ports. Do not publish port 8781 or bind the +exchange to a public, bridge, or tailnet address. + +On a host-network deployment, all three processes use the host namespace. +Keep `SECRETS_EXCHANGE_BIND_HOST=127.0.0.1`. The API reads +`SECRETS_EXCHANGE_BIND_HOST` and `SECRETS_EXCHANGE_PORT` from the same env +file as the exchange. The proxy reads `SECRETS_EXCHANGE_URL`. If you change +the exchange port, set it in both places. + +Remote callers refresh a secret with +`POST /hosts/{host_id}/secrets/{service}/refresh` on the API. They never +connect to the exchange. The API reads the public CA certificate from the +shared volume and gives it to each sandbox with secrets. A sandbox with secrets gets the certificate in `SECRETS_PROXY_CA`, base64, and installs it at boot with `update-ca-certificates`. `SSL_CERT_FILE`, @@ -401,9 +425,9 @@ takes no secrets: a request with secrets always provisions a new sandbox. curl -fsS -H "Authorization: Bearer $TOKEN" http://localhost:8780/doctor ``` -`/doctor` runs one read-only probe per dependency (database, active -provider, Tailscale when enabled) and reports per-check status, -latency, and a remediation hint on failures. It always returns 200 — +`/doctor` runs one read-only probe per dependency: database, active +provider, secrets exchange, and Tailscale when enabled. It reports per-check +status, latency, and a remediation hint on failures. It always returns 200 — health is the `ok` field. `GET /healthz` is the unauthenticated liveness probe. @@ -449,8 +473,8 @@ Secrets exchange: | --- | --- | --- | | `SECRETS_PROXY_URL` | — | Proxy a sandbox sends its HTTPS through. Required to create a host with secrets on every provider but docker-sbx. | | `SECRETS_PROXY_CA_FILE` | — | Path of the proxy's public CA certificate, from the proxy's volume. Required with `SECRETS_PROXY_URL`. | -| `SECRETS_EXCHANGE_BIND_HOST` | `127.0.0.1` | Interface the exchange process binds. Bind it where only the proxy can reach it. | -| `SECRETS_EXCHANGE_PORT` | `8781` | Port the exchange process listens on. | +| `SECRETS_EXCHANGE_BIND_HOST` | `127.0.0.1` | Loopback listener for the exchange. The API reads it to reach the exchange. | +| `SECRETS_EXCHANGE_PORT` | `8781` | Port the exchange process listens on. The API reads it to reach the exchange. | Tailscale (required when `TAILSCALE_ENABLED=true`): diff --git a/docs/security.md b/docs/security.md index 9215af9..1302e76 100644 --- a/docs/security.md +++ b/docs/security.md @@ -105,10 +105,12 @@ API through it. It logs no credential. The real value is encrypted in the database. It passes through the exchange and the proxy for one request, and the exchange keeps an issuer's value in -memory. An issuer that ends a value before its expiry orders a refresh with -`POST /refresh//` on the exchange's private port. The order -carries no value and no token. It makes the exchange ask the issuer again, so -a stray order costs one fetch and nothing else. On docker-sbx the value lives +memory. An issuer that ends a value before its expiry orders a refresh +through the API at `POST /hosts/{host_id}/secrets/{service}/refresh` with an +admin key or service account token. The exchange listens on loopback beside +the API and takes the order from the API only. The order carries no value. +It makes the exchange ask the issuer again, so a stray order costs one fetch +and nothing else. On docker-sbx the value lives in sbx's own store, scoped to that sandbox, and drukbox runs no proxy there. Host deletion removes the sandbox's secrets and value files before the VM goes. The lease in `expires_at` schedules that diff --git a/src/api/app.py b/src/api/app.py index 32cf159..f563d97 100644 --- a/src/api/app.py +++ b/src/api/app.py @@ -76,7 +76,7 @@ async def app_exception_handler(_request: Request, exc: AppException) -> JSONRes payload: dict[str, str] = {"detail": exc.detail} if exc.error_code: payload["error_code"] = exc.error_code - return JSONResponse(status_code=exc.status_code, content=payload) + return JSONResponse(status_code=exc.status_code, content=payload, headers=exc.headers) @app.get("/healthz", include_in_schema=False) diff --git a/src/core/exceptions.py b/src/core/exceptions.py index f966309..246f7e3 100644 --- a/src/core/exceptions.py +++ b/src/core/exceptions.py @@ -12,6 +12,7 @@ class AppException(RuntimeError): status_code: ClassVar[int] = 500 error_code: ClassVar[str | None] = None + headers: ClassVar[dict[str, str]] = {} def __init__(self, detail: str) -> None: super().__init__(detail) diff --git a/src/diagnostics/api.py b/src/diagnostics/api.py index 12e67d1..7d52b1d 100644 --- a/src/diagnostics/api.py +++ b/src/diagnostics/api.py @@ -2,7 +2,7 @@ from typing import Annotated from fastapi import APIRouter, Depends, Request -from pydantic import BaseModel +from pydantic import BaseModel, ConfigDict from sqlalchemy import text from sqlalchemy.ext.asyncio import AsyncSession @@ -12,11 +12,14 @@ from hosts.auth import require_auth from networking.tailscale import Tailscale from providers.registry import get_default_vm_provider +from secrets_exchange.client import SecretsExchange router = APIRouter(prefix="/doctor", tags=["doctor"], dependencies=[Depends(require_auth)]) class CheckOut(BaseModel): + model_config = ConfigDict(from_attributes=True) + name: str status: CheckStatus detail: str | None @@ -69,6 +72,13 @@ async def _tailscale_probe() -> str: asyncio.ensure_future( run_check("provider", _provider_probe, hint=provider_hint, timeout=provider_timeout), ), + asyncio.ensure_future( + run_check( + "exchange", + SecretsExchange.from_settings().diagnose, + hint=SecretsExchange.diagnose_hint, + ), + ), ] if settings.tailscale_enabled: tasks.append( @@ -82,16 +92,7 @@ async def _tailscale_probe() -> str: ok=ok, active_provider=settings.default_host_provider, tailscale_enabled=settings.tailscale_enabled, - checks=[ - CheckOut( - name=check.name, - status=check.status, - detail=check.detail, - latency_ms=check.latency_ms, - hint=check.hint, - ) - for check in checks - ], + checks=[CheckOut.model_validate(check) for check in checks], ) diff --git a/src/diagnostics/tests/test_doctor.py b/src/diagnostics/tests/test_doctor.py index df10cca..9ca2ed2 100644 --- a/src/diagnostics/tests/test_doctor.py +++ b/src/diagnostics/tests/test_doctor.py @@ -2,11 +2,15 @@ import contextlib from unittest.mock import AsyncMock, MagicMock, patch +import httpx import pytest +from api.app import app +from core import settings as settings_module from networking.tailscale import Tailscale from providers.exe.provider import ExeProvider from providers.registry import reset_vm_provider_cache +from secrets_exchange.client import SecretsExchange @pytest.fixture(autouse=True) @@ -15,8 +19,6 @@ def _reset_doctor_state() -> None: # clear cached singletons + the lifespan-installed tailscale slot so a # previous test's bindings don't leak in. The FastAPI app is module-scoped # so app.state survives between tests. - from api.app import app - reset_vm_provider_cache() with contextlib.suppress(KeyError): del app.state.tailscale @@ -53,7 +55,12 @@ async def test_doctor_reports_ok_when_all_probes_pass(client) -> None: assert body["ok"] is True assert body["active_provider"] == "exe" assert body["tailscale_enabled"] is True - assert [check["name"] for check in body["checks"]] == ["db", "provider", "tailscale"] + assert [check["name"] for check in body["checks"]] == [ + "db", + "provider", + "exchange", + "tailscale", + ] assert all(check["hint"] is None for check in body["checks"]) provider = next(check for check in body["checks"] if check["name"] == "provider") assert provider["detail"] == "exe ok" @@ -84,8 +91,6 @@ async def test_doctor_propagates_failure_with_owner_hint(client) -> None: async def test_doctor_omits_tailscale_when_disabled(client, monkeypatch) -> None: """With TAILSCALE_ENABLED=false there is no tailscale row at all.""" - from core import settings as settings_module - monkeypatch.setenv("TAILSCALE_ENABLED", "false") settings_module.get_settings.cache_clear() @@ -97,7 +102,7 @@ async def test_doctor_omits_tailscale_when_disabled(client, monkeypatch) -> None body = response.json() assert body["tailscale_enabled"] is False - assert [check["name"] for check in body["checks"]] == ["db", "provider"] + assert [check["name"] for check in body["checks"]] == ["db", "provider", "exchange"] settings_module.get_settings.cache_clear() @@ -195,3 +200,26 @@ async def slow_diagnose(self) -> str: provider = next(check for check in response.json()["checks"] if check["name"] == "provider") assert provider["status"] == "ok" assert provider["detail"] == "slow but healthy" + + +@pytest.fixture(autouse=True) +def exchange_health(monkeypatch: pytest.MonkeyPatch) -> AsyncMock: + diagnose = AsyncMock(return_value="exchange healthy") + monkeypatch.setattr(SecretsExchange, "diagnose", diagnose) + return diagnose + + +async def test_doctor_reports_exchange_failure(client, exchange_health: AsyncMock) -> None: + exchange_health.side_effect = httpx.ConnectError("connection refused") + + with ( + patch.object(ExeProvider, "diagnose", new=AsyncMock(return_value="exe ok")), + patch.object(Tailscale, "diagnose", new=AsyncMock(return_value="tailnet ok")), + ): + response = await client.get("/doctor", headers={"Authorization": "Bearer service-token"}) + + assert response.status_code == 200 + assert response.json()["ok"] is False + exchange = next(check for check in response.json()["checks"] if check["name"] == "exchange") + assert exchange["status"] == "fail" + assert exchange["hint"] == SecretsExchange.diagnose_hint diff --git a/src/host_secrets/exceptions.py b/src/host_secrets/exceptions.py index 8b43221..6f6b201 100644 --- a/src/host_secrets/exceptions.py +++ b/src/host_secrets/exceptions.py @@ -1,6 +1,19 @@ +from typing import ClassVar + from core.exceptions import AppException class SecretsProxyNotConfiguredError(AppException): status_code = 409 error_code = "SECRETS_PROXY_NOT_CONFIGURED" + + +class SecretStaticError(AppException): + status_code = 409 + error_code = "SECRET_STATIC" + + +class SecretRefreshError(AppException): + status_code = 503 + error_code = "SECRET_REFRESH" + headers: ClassVar[dict[str, str]] = {"Retry-After": "5"} diff --git a/src/hosts/api.py b/src/hosts/api.py index 86de2f5..fef7d79 100644 --- a/src/hosts/api.py +++ b/src/hosts/api.py @@ -2,9 +2,10 @@ import uuid from typing import Annotated -from fastapi import APIRouter, Body, Depends, Header, HTTPException, Response, status +from fastapi import APIRouter, Body, Depends, Header, HTTPException, Path, Response, status from sqlalchemy.exc import SQLAlchemyError +from host_secrets.schemas import SECRET_NAME_PATTERN from hosts.auth import require_auth from hosts.deps import get_host_service from hosts.exceptions import HostTeardownError @@ -99,6 +100,16 @@ async def renew_host( return await service.renew_host(host_id, expires_at=expires_at) +@router.post("/{host_id}/secrets/{service}/refresh", status_code=status.HTTP_204_NO_CONTENT) +async def refresh_secret( + host_id: uuid.UUID, + secret: Annotated[str, Path(alias="service", pattern=SECRET_NAME_PATTERN)], + service: HostServiceDep, +) -> Response: + await service.refresh_secret(host_id, secret) + return Response(status_code=status.HTTP_204_NO_CONTENT) + + @router.delete("/{host_id}", status_code=status.HTTP_204_NO_CONTENT) async def delete_host(host_id: uuid.UUID, service: HostServiceDep) -> Response: try: diff --git a/src/hosts/service.py b/src/hosts/service.py index 49c1ca3..b282c5f 100644 --- a/src/hosts/service.py +++ b/src/hosts/service.py @@ -18,7 +18,7 @@ from core.settings import Settings, get_settings from gateway.settings import GatewaySettings from host_secrets import catalog -from host_secrets.exceptions import SecretsProxyNotConfiguredError +from host_secrets.exceptions import SecretsProxyNotConfiguredError, SecretStaticError from host_secrets.placeholder import Placeholder from hosts.exceptions import HostStateError, IdempotencyKeyConflictError, ProvisioningFailedError from hosts.models import Host, HostStatus, IdempotencyKey @@ -38,6 +38,7 @@ UnsupportedSizingError, ) from providers.registry import get_provider_names, get_vm_provider +from secrets_exchange.client import SecretsExchange from secrets_exchange.secrets import IssuerError, Secret from templates.exceptions import TemplateNotAvailableError, UnknownTemplateError from templates.models import Template, TemplateStatus @@ -439,6 +440,20 @@ async def renew_host(self, host_id: uuid.UUID, *, expires_at: datetime | None = await self.session.refresh(host) return host + async def refresh_secret(self, host_id: uuid.UUID, service: str) -> None: + host = await self.get_host(host_id) + + if not host: + raise ResourceNotFoundError("host not found") + + if service not in host.secrets: + raise ResourceNotFoundError("secret not found") + + if "value" in host.secrets[service]: + raise SecretStaticError(f"secret {service} has a static value") + + await SecretsExchange.from_settings().refresh(host.id, service) + async def delete_host( self, host_id: uuid.UUID, diff --git a/src/hosts/tests/test_refresh_secret.py b/src/hosts/tests/test_refresh_secret.py new file mode 100644 index 0000000..db8c534 --- /dev/null +++ b/src/hosts/tests/test_refresh_secret.py @@ -0,0 +1,139 @@ +import uuid +from collections.abc import AsyncIterator +from unittest.mock import AsyncMock + +import httpx +import pytest +import respx +from httpx import ASGITransport, AsyncClient + +from core.database import async_session_factory +from host_secrets.exceptions import SecretRefreshError +from host_secrets.placeholder import Placeholder +from hosts.models import Host, HostStatus +from hosts.service import utc_now +from secrets_exchange.app import app as exchange_app +from secrets_exchange.client import SecretsExchange +from secrets_exchange.secrets import Secrets + +ADMIN = {"Authorization": "Bearer service-token"} +ISSUER_URL = "https://issuer.test/github" +ISSUER: dict[str, object] = {"issuer": {"url": ISSUER_URL, "headers": {}, "refresh": "1h"}} +STATIC: dict[str, object] = {"value": "static-credential"} + + +async def create_host(entry: dict[str, object]) -> tuple[uuid.UUID, Placeholder]: + host_id = uuid.uuid4() + placeholder = Placeholder.mint(host_id, "github") + + async with async_session_factory() as session: + session.add( + Host( + id=host_id, + name=f"sb-{host_id.hex[:12]}", + image="test:image", + status=HostStatus.ACTIVE.value, + secrets={"github": {**entry, "placeholder_fingerprint": placeholder.fingerprint}}, + created_at=utc_now(), + updated_at=utc_now(), + ) + ) + await session.commit() + + return host_id, placeholder + + +@pytest.fixture +def exchange(monkeypatch: pytest.MonkeyPatch) -> AsyncMock: + refresh = AsyncMock() + monkeypatch.setattr(SecretsExchange, "refresh", refresh) + return refresh + + +async def test_refresh_orders_the_exchange(client: AsyncClient, exchange: AsyncMock) -> None: + host_id, _ = await create_host(ISSUER) + response = await client.post(f"/hosts/{host_id}/secrets/github/refresh", headers=ADMIN) + assert response.status_code == 204 + assert not response.content + exchange.assert_awaited_once_with(host_id, "github") + + +@pytest.mark.parametrize( + "entry, service, expected, error_code", + [ + (None, "github", 404, "NOT_FOUND"), + (ISSUER, "anthropic", 404, "NOT_FOUND"), + (STATIC, "github", 409, "SECRET_STATIC"), + ], +) +async def test_refresh_answers_before_the_exchange( + client: AsyncClient, + exchange: AsyncMock, + entry: dict[str, object] | None, + service: str, + expected: int, + error_code: str, +) -> None: + host_id = (await create_host(entry))[0] if entry else uuid.uuid4() + response = await client.post(f"/hosts/{host_id}/secrets/{service}/refresh", headers=ADMIN) + assert response.status_code == expected + assert response.json()["error_code"] == error_code + exchange.assert_not_awaited() + + +async def test_refresh_failure_carries_retry_after( + client: AsyncClient, exchange: AsyncMock +) -> None: + exchange.side_effect = SecretRefreshError("secrets exchange unavailable") + host_id, _ = await create_host(ISSUER) + response = await client.post(f"/hosts/{host_id}/secrets/github/refresh", headers=ADMIN) + assert response.status_code == 503 + assert response.headers["Retry-After"] == "5" + assert response.json() == { + "detail": "secrets exchange unavailable", + "error_code": "SECRET_REFRESH", + } + + +@pytest.mark.parametrize( + "headers, expected", [({}, 401), ({"Authorization": "Bearer invalid"}, 403)] +) +async def test_refresh_requires_a_token( + client: AsyncClient, exchange: AsyncMock, headers: dict[str, str], expected: int +) -> None: + response = await client.post(f"/hosts/{uuid.uuid4()}/secrets/github/refresh", headers=headers) + assert response.status_code == expected + exchange.assert_not_awaited() + + +@pytest.fixture +async def live_exchange() -> AsyncIterator[respx.MockRouter]: + async with httpx.AsyncClient() as issuer_client: + exchange_app.state.secrets = Secrets(issuer_client) + + with respx.mock as router: + router.route(url__startswith=SecretsExchange.from_settings().url).mock( + side_effect=respx.ASGIHandler(exchange_app) + ) + yield router + + +async def test_refresh_replaces_the_exchange_value( + client: AsyncClient, live_exchange: respx.MockRouter +) -> None: + host_id, placeholder = await create_host(ISSUER) + issuer = live_exchange.get(ISSUER_URL).respond(json={"value": "credential-one"}) + headers = {"Authorization": f"Bearer {placeholder}", "X-Forwarded-Host": "api.github.com"} + + async with AsyncClient( + transport=ASGITransport(app=exchange_app), base_url=SecretsExchange.from_settings().url + ) as edge: + response = await edge.get("/authorize", headers=headers) + assert response.headers["X-Upstream-Credential"] == "Bearer credential-one" + issuer.respond(json={"value": "credential-two"}) + response = await client.post(f"/hosts/{host_id}/secrets/github/refresh", headers=ADMIN) + assert response.status_code == 204 + response = await edge.get("/authorize", headers=headers) + assert response.headers["X-Upstream-Credential"] == "Bearer credential-two" + + assert issuer.call_count == 2 diff --git a/src/secrets_exchange/client.py b/src/secrets_exchange/client.py new file mode 100644 index 0000000..117a653 --- /dev/null +++ b/src/secrets_exchange/client.py @@ -0,0 +1,33 @@ +import uuid +from typing import Self + +import httpx + +from host_secrets.exceptions import SecretRefreshError +from secrets_exchange.settings import SecretsExchangeSettings + + +class SecretsExchange: + diagnose_hint = "start_secrets_exchange_beside_the_api" + + def __init__(self, url: str) -> None: + self.url = url + + @classmethod + def from_settings(cls) -> Self: + return cls(SecretsExchangeSettings().url) + + async def refresh(self, host_id: uuid.UUID, service: str) -> None: + try: + async with httpx.AsyncClient(base_url=self.url, timeout=30, trust_env=False) as client: + response = await client.post(f"/refresh/{host_id}/{service}") + except httpx.RequestError as exc: + raise SecretRefreshError("secrets exchange unavailable") from exc + if response.status_code == httpx.codes.SERVICE_UNAVAILABLE: + raise SecretRefreshError("the issuer or provider did not supply a value") + response.raise_for_status() + + async def diagnose(self) -> str: + async with httpx.AsyncClient(base_url=self.url, trust_env=False) as client: + (await client.get("/healthz")).raise_for_status() + return "exchange healthy" diff --git a/src/secrets_exchange/settings.py b/src/secrets_exchange/settings.py index 007beef..635907b 100644 --- a/src/secrets_exchange/settings.py +++ b/src/secrets_exchange/settings.py @@ -13,7 +13,11 @@ class SecretsExchangeSettings(BaseSettings): bind_host: str = Field( default="127.0.0.1", description=( - "Interface the exchange process binds. Bind it where only the proxy can reach it." + "Interface the exchange process binds. Keep it on loopback beside the API and proxy." ), ) port: int = Field(default=8781, description="Port the exchange process listens on.") + + @property + def url(self) -> str: + return f"http://{self.bind_host}:{self.port}" diff --git a/src/secrets_exchange/tests/test_client.py b/src/secrets_exchange/tests/test_client.py new file mode 100644 index 0000000..20d16a5 --- /dev/null +++ b/src/secrets_exchange/tests/test_client.py @@ -0,0 +1,61 @@ +import uuid +from collections.abc import Callable + +import httpx +import pytest +import respx + +from host_secrets.exceptions import SecretRefreshError +from secrets_exchange.client import SecretsExchange + +URL = "http://127.0.0.1:8781" + + +async def test_refresh_sends_no_token_and_no_body() -> None: + host_id = uuid.uuid4() + + with respx.mock as router: + route = router.post(f"{URL}/refresh/{host_id}/github").respond(200) + await SecretsExchange(URL).refresh(host_id, "github") + + request = route.calls.last.request + assert not request.content + assert "Authorization" not in request.headers + + +@pytest.mark.parametrize( + "fail", + [ + lambda route: route.respond(503, headers={"Retry-After": "5"}), + lambda route: route.mock(side_effect=httpx.ConnectError("refused")), + lambda route: route.mock(side_effect=httpx.ReadTimeout("slow")), + ], +) +async def test_refresh_raises_when_the_exchange_cannot_refresh( + fail: Callable[[respx.Route], object], +) -> None: + host_id = uuid.uuid4() + + with respx.mock as router: + fail(router.post(f"{URL}/refresh/{host_id}/github")) + + with pytest.raises(SecretRefreshError): + await SecretsExchange(URL).refresh(host_id, "github") + + +async def test_diagnose_reports_a_healthy_exchange() -> None: + with respx.mock as router: + router.get(f"{URL}/healthz").respond(json={"status": "ok"}) + assert await SecretsExchange(URL).diagnose() == "exchange healthy" + + +async def test_diagnose_raises_on_an_unhealthy_exchange() -> None: + with respx.mock as router: + router.get(f"{URL}/healthz").respond(503) + with pytest.raises(httpx.HTTPStatusError): + await SecretsExchange(URL).diagnose() + + +def test_from_settings_reads_the_exchange_settings(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("SECRETS_EXCHANGE_PORT", "9876") + assert SecretsExchange.from_settings().url == "http://127.0.0.1:9876" diff --git a/src/service_accounts/tests/test_api.py b/src/service_accounts/tests/test_api.py index 54a1e14..7170a63 100644 --- a/src/service_accounts/tests/test_api.py +++ b/src/service_accounts/tests/test_api.py @@ -53,6 +53,10 @@ async def test_token_works_on_each_router(client: AsyncClient, use_admin: bool) with ( patch("providers.exe.provider.ExeProvider.diagnose", new=AsyncMock(return_value="exe ok")), patch("networking.tailscale.Tailscale.diagnose", new=AsyncMock(return_value="tailnet ok")), + patch( + "secrets_exchange.client.SecretsExchange.diagnose", + new=AsyncMock(return_value="exchange healthy"), + ), patch( "http_proxies.service.HTTPProxyService.create_http_proxy", new=AsyncMock() ) as create_proxy,