fix(api)!: key the health route and keep server internals from user keys

/api/health needs X-Secret-Key but stays outside the rate limit. Only the admin key gets load_errors from /api/services, default error details no longer name the exception class, and a prepare signal replaces aiohttp's Server banner on every response while the CDM routes keep their own.
2026-11-11 19:45:06 +00:00 · 2026-11-03 11:53:55 -07:00
parent b6429ecdfd
commit 87ccd6666c
16 changed files with 105 additions and 47 deletions
@@ -16,7 +16,7 @@ You start the API with the [`unshackle serve`](../../guide/cli-reference.md) com

 1. Add an `api_secret` to the `serve` section of your `unshackle.yaml`.
 2. Start the server with `unshackle serve`.
-3. Send that secret in the `X-Secret-Key` header on every request except `GET /api/health`.
+3. Send that secret in the `X-Secret-Key` header on every request, `GET /api/health` included.

 ```bash title="A minimal authenticated request"
 curl -H "X-Secret-Key: your-secret-key-here" \
@@ -152,12 +152,12 @@ The value must match one of the configured API keys exactly (`api_secret`, or on
 !!! tip "The reference client sets it for you"
    unshackle's own [remote download](../../guide/downloading.md) client (`RemoteService`) sends `X-Secret-Key` automatically, reading the API key from your `remote_services` config. It also sends a `User-Agent` of `unshackle/<version>`. You only deal with the header directly when writing your own client.

-### The health check is always open
+### The health check needs an API key too

-One endpoint is exempt from authentication: `GET /api/health`. It never requires an API key, so you can use it for liveness and version probes without embedding a secret in your monitoring:
+`GET /api/health` reports the running version and commit, so it sits behind the same `X-Secret-Key` check as every other route. A monitor that probes it sends an API key; a request without one gets the usual `401` body, which still proves the server is up:

-```bash title="No key needed"
-curl http://127.0.0.1:8786/api/health
+```bash title="Liveness probe"
+curl -H "X-Secret-Key: your-secret-key-here" http://127.0.0.1:8786/api/health
 ```

 ```json title="Response"
@@ -307,7 +307,6 @@ With `--caddy`, unshackle also starts `caddy run` using the `Caddyfile` located
 | Auth header | `X-Secret-Key` |
 | Query-parameter alternative | `?secret_key=` on `GET /api/download/jobs/{job_id}/events` only |
 | Default bind address | `127.0.0.1:8786` |
-| Exempt endpoint | `GET /api/health` |
 | Failure status | `401 Unauthorized`, or `429 Too Many Requests` for a key over its rate limit |
 | Missing-header body | `{"status": 401, "message": "Secret Key is Empty."}` |
 | Invalid-key body | `{"status": 401, "message": "Secret Key is Invalid."}` |
@@ -326,7 +325,7 @@ With `--caddy`, unshackle also starts `caddy run` using the `Caddyfile` located

 The header and config are identical between modes, but the code that enforces them differs:

- **API-only server** (`--api-only`, or `--remote-only` which forces API-only). An `api_key_authentication` middleware runs on every request. It lets `GET /api/health` through unconditionally, then reads `X-Secret-Key`. An empty header returns the *"Secret Key is Empty."* body. A header that is not a configured user returns *"Secret Key is Invalid."*. In this mode the accepted-keys map holds your `api_secret` (labelled `api_user`, with no devices) plus every key under `serve.users`. A per-user key therefore authenticates here exactly as it does on the integrated server, and that user's `services` allowlist applies as usual. With `--no-key`, this middleware is not installed and the users map is empty.
+- **API-only server** (`--api-only`, or `--remote-only` which forces API-only). An `api_key_authentication` middleware runs on every request. It reads `X-Secret-Key` on every request, `GET /api/health` included. An empty header returns the *"Secret Key is Empty."* body. A header that is not a configured user returns *"Secret Key is Invalid."*. In this mode the accepted-keys map holds your `api_secret` (labelled `api_user`, with no devices) plus every key under `serve.users`. A per-user key therefore authenticates here exactly as it does on the integrated server, and that user's `services` allowlist applies as usual. With `--no-key`, this middleware is not installed and the users map is empty.

 - **Integrated server** (default). A `serve_authentication` middleware wraps pywidevine's authentication (and pyplayready's for `/playready*` paths), and the REST routes live in the same application, guarded by the same check. Here the accepted-keys map is your full `serve` config: `api_secret` (auto-added with access to all loaded devices) plus every entry under `users`, each with its declared `devices` / `playready_devices`.

@@ -199,7 +199,7 @@ here, so use `id` as the identity and `label` only for display.
 | `bytes_out` | Response bodies only. An event stream reports no body length, so it counts as zero. |
 | `last_seen` | `null` until the API key makes its first request. |

-Only a configured API key gets a row of counters. A request on an open route such as `/api/health` with an unknown key counts as `anonymous`, so an unauthenticated caller cannot grow the table.
+Only a configured API key gets a row of counters. A request that answers with an unknown key counts as `anonymous`, so an unauthenticated caller cannot grow the table.

 Every API key the server counts has a row here, the dashboard key included, so a
 `requests_by_key` bucket other than `anonymous` always matches one. A "traffic by key" panel
@@ -8,11 +8,11 @@ Start the server with [`unshackle serve`](../../guide/cli-reference.md). By defa
    All examples below assume a base URL of `http://127.0.0.1:8786`. Replace the host and port so that they match your own `unshackle serve -h <host> -p <port>` invocation.

 !!! tip "Interactive docs"
-    The server mounts a live Swagger UI at **`/api/docs/`** whenever it does not operate in `--remote-only` mode. Its title is "Unshackle REST API" and its version tracks the installed package version. In API-only mode with an API key configured, the Swagger UI itself sits behind `X-Secret-Key`. Only `/api/health` is exempt.
+    The server mounts a live Swagger UI at **`/api/docs/`** whenever it does not operate in `--remote-only` mode. Its title is "Unshackle REST API" and its version tracks the installed package version. In API-only mode with an API key configured, the Swagger UI itself sits behind `X-Secret-Key`, like every other route.

 ## Conventions

- **Authentication.** Every request except `GET /api/health` requires the `X-Secret-Key` header when you configure an API key. `--no-key` disables the check entirely. See [Authentication](authentication.md).
+- **Authentication.** Every request, `GET /api/health` included, requires the `X-Secret-Key` header when you configure an API key. `--no-key` disables the check entirely. See [Authentication](authentication.md).
 - **Content type.** Request bodies are JSON (`Content-Type: application/json`). All success responses are JSON unless this page documents a `204 No Content`.
 - **CORS.** Every response carries permissive CORS headers, and the server answers `OPTIONS` preflight requests automatically.
 - **Compression.** JSON responses of 256 bytes or more are gzip-compressed when the client sends `Accept-Encoding: gzip`. A [slow response](#slow-responses) is not compressed.
@@ -30,12 +30,12 @@ These endpoints report server state and enumerate what is available. `GET /api/h

 ### `GET /api/health`

-Health check. **This is the only route exempt from authentication**: you can call it without an `X-Secret-Key` header. It reports the running version and a best-effort update check.
+Health check. It reports the running version and commit and a best-effort update check, so it needs the `X-Secret-Key` header like every other route. A request without an API key gets a `401`, which still shows the server is up.

 === "Request"

    ```bash
-    curl http://127.0.0.1:8786/api/health
+    curl -H "X-Secret-Key: $SECRET" http://127.0.0.1:8786/api/health
    ```

 === "Response `200`"
@@ -68,7 +68,7 @@ If the update check fails (for example, no network), `update_available` and `lat

 ### `GET /api/services`

-Show the streaming services available on this server, filtered by your allowlist. Each entry gives the service's tag, matching rules, capability flags, and its CLI parameters (useful when you make a UI that accepts service-specific options). `load_errors` lists the services the server skipped because they failed to import, at startup or after a service repository refresh; the server also writes each one to its log. An entry marked `pending_update` has newer code staged: it keeps its current code until its running or queued jobs and its live remote sessions finish.
+Show the streaming services available on this server, filtered by your allowlist. Each entry gives the service's tag, matching rules, capability flags, and its CLI parameters (useful when you make a UI that accepts service-specific options). For the admin key, `load_errors` lists the services the server skipped because they failed to import, at startup or after a service repository refresh; the server also writes each one to its log. Any other key gets an empty list; with `--no-key` every caller is the admin. An entry marked `pending_update` has newer code staged: it keeps its current code until its running or queued jobs and its live remote sessions finish.

 === "Request"

@@ -221,8 +221,7 @@ body that uses an integer `status` and no `error_code` or `timestamp`.
 !!! warning "Handle both shapes"
    A client must expect two error formats: the standard structured error
    (string `status` of `"error"`, with `error_code`), and this authentication error
-    (integer `status`, no `error_code`). The `GET /api/health` endpoint is exempt
-    from authentication, so it never returns the key-error shape.
+    (integer `status`, no `error_code`).

 ## Late failures with status `200`

@@ -126,12 +126,12 @@ health check is `GET http://127.0.0.1:8786/api/health`.
    available at [`/api/docs/`](http://127.0.0.1:8786/api/docs/). The server generates
    it from the same route table it serves, so it always matches the version you
    operate. Note that in `--api-only` mode the docs page is itself behind the
-    API key. Only `/api/health` is exempt from authentication.
+    API key, like every other route.

-A quick smoke test that needs no API key:
+A quick smoke test:

 ```shell title="Confirm the server is up"
-curl http://127.0.0.1:8786/api/health
+curl -H "X-Secret-Key: your-api-secret" http://127.0.0.1:8786/api/health
 ```

 ```json title="Response"
@@ -156,7 +156,7 @@ request and response in full.

 | Method | Path | Purpose |
 |---|---|---|
-| `GET` | `/api/health` | Liveness and version check. Never requires an API key. |
+| `GET` | `/api/health` | Liveness and version check. |
 | `GET` | `/api/services` | List the services available on this server. |
 | `POST` | `/api/search` | Run a service's search for a query. |
 | `POST` | `/api/list-titles` | List the titles behind a service title ID. |
@@ -220,7 +220,7 @@ request and response in full.
 ## Authentication in one line

 With the default configuration, send your API secret in the `X-Secret-Key` header on
-every request except `/api/health`:
+every request:

 ```shell title="An authenticated request"
 curl -X POST http://127.0.0.1:8786/api/search \
@@ -93,7 +93,7 @@ Swagger UI available at http://127.0.0.1:8786/api/docs/
 ```

 !!! note "Swagger UI is also behind auth"
-    In `--api-only` mode (without `--no-key`), the Swagger UI at `/api/docs/` requires the `X-Secret-Key` header, the same as every other route. Only `/api/health` is exempt from authentication.
+    In `--api-only` mode (without `--no-key`), the Swagger UI at `/api/docs/` requires the `X-Secret-Key` header, the same as every other route.

 For the rest of this page we will assume the server is reachable at `http://127.0.0.1:8786` and store the secret in an environment variable:

@@ -106,10 +106,10 @@ export BASE="http://127.0.0.1:8786"

 ## Step 3: Make sure that the server is alive

-The health endpoint is the one route that never requires an API key. Use it to make sure that the server operates, and to read its version:
+Use the health endpoint to make sure that the server operates, and to read its version. It needs the API key like every other route:

 ```shell
-curl "$BASE/api/health"
+curl -H "X-Secret-Key: $SECRET" "$BASE/api/health"
 ```

 ```json title="200 OK"
@@ -575,7 +575,7 @@ unshackle serve [OPTIONS]
 | `--debug` | off | Enable debug logging for API operations. |
 | `--remote-only` | off | Expose only the remote service session endpoints (health, services, search, session). Implies `--api-only`. |

-WVD files in the WVDs directory and PRD files in the PRDs directory are auto-loaded. The REST API lives under `http://<host>:<port>/api/`, with Swagger UI at `/api/docs/` and a health check at `/api/health` (exempt from auth).
+WVD files in the WVDs directory and PRD files in the PRDs directory are auto-loaded. The REST API lives under `http://<host>:<port>/api/`, with Swagger UI at `/api/docs/` and a health check at `/api/health`.

 !!! warning "Configure `api_secret` first"
    Unless you pass `--no-key`, `serve.api_secret` must be set in your config. Requests authenticate with the `X-Secret-Key` header. `--no-key` disables authentication entirely. Only use it on a trusted, private network.
@@ -71,13 +71,13 @@ def free_port() -> int:
        return s.getsockname()[1]


-def wait_for_health(url: str, timeout: float = 60.0) -> bool:
+def wait_for_health(url: str, secret_key: str, timeout: float = 60.0) -> bool:
    import requests

    deadline = time.monotonic() + timeout
    while time.monotonic() < deadline:
        try:
-            r = requests.get(f"{url}/api/health", timeout=2)
+            r = requests.get(f"{url}/api/health", headers={"X-Secret-Key": secret_key}, timeout=2)
            if r.status_code == 200:
                return True
        except requests.RequestException:
@@ -112,6 +112,7 @@ def server_url(request: pytest.FixtureRequest):
    explicit = str(cfg.getoption("--server-url")).rstrip("/")
    mode = cfg.getoption("--spawn-serve")
    is_live = cfg.getoption("--live")
+    key = str(cfg.getoption("--secret-key"))

    if not is_live:
        yield explicit or "http://localhost:8786"
@@ -120,13 +121,13 @@ def server_url(request: pytest.FixtureRequest):
    if mode == "never":
        if not explicit:
            pytest.fail("--spawn-serve=never requires --server-url")
-        if not wait_for_health(explicit, timeout=10):
+        if not wait_for_health(explicit, key, timeout=10):
            pytest.fail(f"External serve not reachable at {explicit}")
        yield explicit
        return

    if mode == "auto" and explicit:
-        if not wait_for_health(explicit, timeout=10):
+        if not wait_for_health(explicit, key, timeout=10):
            pytest.fail(f"External serve not reachable at {explicit}")
        yield explicit
        return
@@ -153,7 +154,7 @@ def server_url(request: pytest.FixtureRequest):
    )
    url = f"http://127.0.0.1:{port}"
    try:
-        if not wait_for_health(url, timeout=60):
+        if not wait_for_health(url, key, timeout=60):
            proc.terminate()
            pytest.fail(f"Spawned serve at {url} did not become healthy within 60s")
        yield url
@@ -24,12 +24,12 @@ def test_services_endpoint(http_session, server_url: str) -> None:
    assert isinstance(body["services"], list)


-def test_health_does_not_require_secret_key(server_url: str) -> None:
-    """Health bypasses auth even when the server runs with a key."""
+def test_health_requires_secret_key(server_url: str, secret_key: str) -> None:
+    """Health needs the API key like every other route."""
    import requests

    r = requests.get(f"{server_url}/api/health", timeout=10)
-    assert r.status_code == 200
+    assert r.status_code == (401 if secret_key else 200)


 def test_unknown_route_returns_404(http_session, server_url: str) -> None:
@@ -249,7 +249,7 @@ async def test_since_cursor_and_dashboard_key(aiohttp_client, serve_app) -> None
    assert (await client.get("/api/dashboard/cdm-logs")).status == 401
    assert (await client.get("/api/dashboard/cdm-logs", headers=USER)).status == 401
    # Only the device routes are CDM calls: not the API, not the ping, not this route.
-    await client.get("/api/health")
+    await client.get("/api/health", headers=USER)
    await client.get("/", headers=USER)

    page = await cdm_log(client, since=1)
@@ -255,6 +255,8 @@ async def test_rate_limited_request_returns_429(aiohttp_client, monkeypatch) ->
    assert (await client.get("/api/services", headers={"X-Secret-Key": "tier-key"})).status == 200
    resp = await client.get("/api/services", headers={"X-Secret-Key": "tier-key"})
    assert resp.status == 429 and int(resp.headers["Retry-After"]) > 0
+    assert (await client.get("/api/health", headers={"X-Secret-Key": "tier-key"})).status == 200
+    assert (await client.get("/api/health")).status == 401
    # The dashboard key leaves the middleware before the limit check, so it always answers.
    assert (await client.get("/api/dashboard/status", headers={"X-Secret-Key": "dash-secret"})).status == 200

@@ -400,7 +402,7 @@ async def test_dashboard_health(aiohttp_client, dashboard_cfg, monkeypatch) -> N


 async def test_unknown_key_on_open_route_does_not_grow_key_stats(aiohttp_client, monkeypatch) -> None:
-    """/api/health answers 200 to any header, so counters keyed by the raw header are a memory leak."""
+    """Counters keyed by the raw header of an unknown key would grow without bound."""
    monkeypatch.setitem(config.serve, "users", {"real": {}})
    stats.keys.clear()
    client = await aiohttp_client(make_app())
@@ -95,6 +95,12 @@ def test_build_error_response_from_generic_exception() -> None:
    assert body["message"] == f"oops (ref {body['ref']})"


+def test_categorize_fallback_names_no_exception_class() -> None:
+    err = categorize_exception(RuntimeError("oops"), context={"op": "x"})
+    assert err.error_code == APIErrorCode.INTERNAL_ERROR
+    assert err.details == {"op": "x"}
+
+
 def test_build_error_response_debug_mode_includes_traceback() -> None:
    try:
        raise ValueError("kaboom")
@@ -40,13 +40,10 @@ def make_app():


 def no_key_required_auth():
-    """Mirror serve.py's api_key_authentication middleware: required X-Secret-Key
-    on every endpoint except /api/health."""
+    """Mirror serve.py's api_key_authentication middleware: a required X-Secret-Key on every endpoint."""

    @web.middleware
    async def mw(request, handler):
-        if request.path == "/api/health":
-            return await handler(request)
        secret = request.headers.get("X-Secret-Key")
        if not secret:
            return web.json_response({"status": 401, "message": "Secret Key is Empty."}, status=401)
@@ -307,11 +304,44 @@ async def test_health_endpoint_responds_ok(make_app, aiohttp_client) -> None:
    assert "version" in body


-async def test_health_bypasses_api_key_auth_middleware(make_app, aiohttp_client) -> None:
-    app = make_app(remote_only=True, with_auth_middleware=True)
+async def test_every_response_carries_a_generic_server_header(make_app, aiohttp_client) -> None:
+    """Returned, raised (404, 405) and streamed responses all lose aiohttp's version banner."""
+
+    async def stream(request: web.Request) -> web.StreamResponse:
+        response = web.StreamResponse()
+        await response.prepare(request)
+        await response.write(b"x")
+        return response
+
+    app = make_app(remote_only=True)
+    app.router.add_get("/stream", stream)
    client = await aiohttp_client(app)
-    resp = await client.get("/api/health")
-    assert resp.status == 200  # health bypasses auth
+    for method, path in [("GET", "/api/health"), ("DELETE", "/api/health"), ("GET", "/api/nope"), ("GET", "/stream")]:
+        resp = await client.request(method, path)
+        assert resp.headers["Server"] == "unshackle", (method, path, resp.status)
+
+
+async def test_services_load_errors_go_to_the_admin_key_only(
+    make_app, aiohttp_client, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    from unshackle.core import services as services_module
+    from unshackle.core.config import config
+
+    monkeypatch.setattr(services_module, "LOAD_ERRORS", ["BAD: failed to import - ImportError: boom"])
+    monkeypatch.setattr(config, "serve", {"api_secret": "admin", "users": {"user": {"services": ["EXAMPLE"]}}})
+    client = await aiohttp_client(make_app(remote_only=True))
+    user = await (await client.get("/api/services", headers={"X-Secret-Key": "user"})).json()
+    admin = await (await client.get("/api/services", headers={"X-Secret-Key": "admin"})).json()
+    assert user["load_errors"] == []
+    assert admin["load_errors"] == ["BAD: failed to import - ImportError: boom"]
+
+
+async def test_health_needs_an_api_key(make_app, aiohttp_client) -> None:
+    app = make_app(remote_only=True, with_auth_middleware=True)
+    app["config"]["users"]["good-key"] = {}
+    client = await aiohttp_client(app)
+    assert (await client.get("/api/health")).status == 401
+    assert (await client.get("/api/health", headers={"X-Secret-Key": "good-key"})).status == 200


 async def test_auth_middleware_rejects_missing_key(make_app, aiohttp_client) -> None:
@@ -303,7 +303,7 @@ def categorize_exception(
    return APIError(
        error_code=APIErrorCode.INTERNAL_ERROR,
        message=f"An unexpected error occurred: {redact_secrets(str(exc))}",
-        details={**context, "exception_type": exc_type},
+        details=context,
        retryable=False,
    )

@@ -1417,11 +1417,12 @@ async def api_key_authentication(request: web.Request, handler: Any) -> web.Stre
    """Gate every non-dashboard route behind an API key in ``app["config"]["users"]``, then rate limit it.

    Runs after :func:`dashboard_authentication`, which has already answered the dashboard
-    routes, so the dashboard key never reaches the limiter.
+    routes, so the dashboard key never reaches the limiter. Health is never limited: a client
+    polls it while it waits for a restarted server.
    """
    import hmac

-    if request.path == "/api/health" or request.path.startswith(DASHBOARD_PREFIX):
+    if request.path.startswith(DASHBOARD_PREFIX):
        return await handler(request)
    secret_key = request_secret_key(request)
    if not secret_key:
@@ -1430,6 +1431,8 @@ async def api_key_authentication(request: web.Request, handler: Any) -> web.Stre
    # recovered byte-by-byte from response timing.
    if not any(hmac.compare_digest(secret_key, k) for k in request.app["config"]["users"]):
        return web.json_response({"status": 401, "message": "Secret Key is Invalid."}, status=401)
+    if request.path == "/api/health":
+        return await handler(request)
    return rate_limit_response(secret_key) or await handler(request)


@@ -8,6 +8,7 @@ from typing import Any, Optional

 import click
 from aiohttp import web
+from aiohttp.http import SERVER_SOFTWARE
 from aiohttp_swagger3 import SwaggerDocs, SwaggerInfo, SwaggerUiSettings

 from unshackle.core import __code_hash__, __version__
@@ -48,6 +49,7 @@ from unshackle.core.api.handlers import (
    env_check_handler,
    get_allowed_services,
    get_download_job_handler,
+    is_admin,
    list_download_jobs_handler,
    list_titles_handler,
    list_tracks_handler,
@@ -428,7 +430,7 @@ async def services(request: web.Request) -> web.Response:
              properties:
                load_errors:
                  type: array
-                  description: Services skipped because they failed to import (startup or repo refresh)
+                  description: Services skipped because they failed to import (startup or repo refresh); admin key only
                  items:
                    type: string
                services:
@@ -526,7 +528,12 @@ async def services(request: web.Request) -> web.Response:

            services_info.append(service_data)

-        load_errors = [redact_path(re.sub(r" \([^()]*\)$", "", err)) for err in services_module.LOAD_ERRORS]
+        # an import error names services outside the caller's allowlist
+        load_errors = (
+            [redact_path(re.sub(r" \([^()]*\)$", "", err)) for err in services_module.LOAD_ERRORS]
+            if is_admin(request)
+            else []
+        )
        return web.json_response({"services": services_info, "load_errors": load_errors})
    except Exception as e:
        log.exception("Error listing services")
@@ -3413,9 +3420,20 @@ def setup_routes(
        if remote_only and not remote:
            continue
        add[method](path, handler)
+    app.on_response_prepare.append(generic_server_header)
    app.on_shutdown.append(close_tunnels)


+async def generic_server_header(request: web.Request, response: web.StreamResponse) -> None:
+    """Replace aiohttp's default Server header, which names the Python and aiohttp versions.
+
+    A prepare signal also reaches raised and streamed responses, which a middleware never sees.
+    API routes drop the CDM middleware's stamp too; the CDM routes keep it for their clients.
+    """
+    if request.path.startswith("/api/") or response.headers.get("Server") == SERVER_SOFTWARE:
+        response.headers["Server"] = "unshackle"
+
+
 def swagger_docs(app: web.Application) -> SwaggerDocs:
    """The Swagger UI at /api/docs/, which registers the routes added through it."""
    return SwaggerDocs(