Skip to content

docs: allowed_service_ids is silently ignored when allow_all_services defaults to true #512

@eanzhao

Description

@eanzhao

Summary

POST /api/v1/api-keys silently ignores a populated allowed_service_ids list unless the caller also sends allow_all_services: false. The current API doc (docs/API.md:847-849) and the OpenAPI-exposed CreateApiKeyRequest struct (backend/src/handlers/api_keys.rs:94-119) do not surface this dependency, which makes "create a scoped key" payloads quietly broad and only surface as a downstream proxy 403 well after key creation.

Concrete bite

aevatar PR aevatarAI/aevatar#418 (issue aevatarAI/aevatar#417) was bitten by this exact footgun:

  • aevatar sent POST /api/v1/api-keys with { "name": "...", "scopes": "proxy", "allowed_service_ids": [<UserService.id>, ...] } and no allow_all_services field.
  • #[serde(default = "default_true")] on CreateApiKeyRequest.allow_all_services (api_keys.rs:105) → stored allow_all_services = true.
  • proxy enforcement (proxy.rs:1030) is gated on !auth_user.allow_all_services, so the populated allowed_service_ids was never consulted.
  • The fix on aevatar side was to add "allow_all_services": false to the payload — but the SDK had no signal that this was required.

The aevatar fix shipped, but the next consumer who reads docs/API.md and writes the same naive payload will repeat the same mistake.

What the docs say today

docs/API.md:847-849:

| `allowed_service_ids` | string[] | No | UserService IDs this key can proxy (empty = use `allow_all_services`) |
| `allowed_node_ids`    | string[] | No | Node IDs this key can route through (empty = use `allow_all_nodes`) |
| `allow_all_services`  | boolean  | No | If true, key can access all user's external services (default: true) |

The (empty = use allow_all_services) parenthetical is misleading: it implies that providing values is itself sufficient to constrain the key. It is not. allow_all_services is checked first, regardless of whether allowed_service_ids is empty.

The Rust struct CreateApiKeyRequest (backend/src/handlers/api_keys.rs:94-119) carries no doc comments on these fields, so OpenAPI / utoipa::ToSchema consumers see only the bare types. Notably models/api_key.rs:33 does have the right note ("Only checked when allow_all_services is false.") — but that struct is the storage model, not the request DTO.

What I'd like the docs to say

In docs/API.md POST /api/v1/api-keys section:

  1. Replace the misleading (empty = use allow_all_services) with an explicit precondition: "Only enforced when allow_all_services: false. Providing values without setting allow_all_services: false will silently store the list but not scope the key."

  2. Add a "Scoped key example" curl block alongside the current broad-key example:

    {
      "name": "Scoped Key",
      "scopes": "proxy",
      "allow_all_services": false,
      "allowed_service_ids": ["<UserService.id>", "..."]
    }
  3. Callout box: "To create a scoped API key you must set BOTH allow_all_services: false AND a non-empty allowed_service_ids. Either alone is a no-op."

In backend/src/handlers/api_keys.rs:94-119 (CreateApiKeyRequest):

  1. Add doc comments on allowed_service_ids, allowed_node_ids, allow_all_services, allow_all_nodes mirroring the model-level note. These flow into the OpenAPI/Swagger output via ToSchema automatically.

Optional follow-up at the API layer (not required for this issue)

If allowed_service_ids is non-empty AND allow_all_services is true (or default), the request is internally inconsistent — the caller is asking for a scoped key but submitting a broad-key payload. Two ways to make this impossible:

  • (a) Reject with 400 ValidationError: "Cannot specify allowed_service_ids when allow_all_services is true. Set allow_all_services: false to scope the key."
  • (b) Auto-flip: if allowed_service_ids is non-empty, treat allow_all_services as false unless explicitly set to true.

I'd lean toward (a) since it preserves explicit-intent semantics. But docs alone are a valid resolution — the bare minimum is making the precondition discoverable.

Source pointers (verified against HEAD 3279d9c95249e1ca159c66bec5754ba35281c783)

  • Default: backend/src/handlers/api_keys.rs:105#[serde(default = "default_true")]
  • Handler pass-through: backend/src/handlers/api_keys.rs:1111Some(body.allow_all_services) straight to key_service::create_api_key (no parent inheritance, no delegation forcing)
  • Validation gate: backend/src/services/key_service.rs:183if !all_svcs { validate_service_ids(...) }
  • Enforcement gate: backend/src/handlers/proxy.rs:1030-1033
  • Existing (correct) model-level note: backend/src/models/api_key.rs:33
  • Doc to fix: docs/API.md:847-849
  • Struct to add doc-comments to: backend/src/handlers/api_keys.rs:94-119

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentation

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions