review-topic-auth-and-tenant-security

# Review topic: Auth, tenant-boundary & secret handling ## When this applies Trigger on CONTENT/behaviour, not just directory. Load when the diff: - Touches the auth/usage middleware in `inference/core/interfaces/http/http_api.py` (`check_authorization_serverless`, `check_authorization`) or the unauthenticated-route allowlist inside them. - Calls `get_serverless_usage_check_async` or branches on its `status_code`. - Resolves/compares api-keys, workspaces, or projects — `get_roboflow_workspace(_async)`, the `cached_api_keys` map / `AuthorizationCacheKey`, `WORKSPACES_WHITELISTED_FOR_LOCAL_DEPLOYMENT`, `DEDICATED_DEPLOYMENT_WORKSPACE_URL`. - Adds/reads assume-identity plumbing — `_add_assume_identity_headers`, `ASSUME_IDENTITY_ACCESS_TOKEN_HEADER` (`x-assume-identity-access-token`), `assume_identity_authorised_workspace_db_id`, `ROBOFLOW_ASSUME_IDENTITY_SERVICE_ACCESS_TOKEN`. - Adds/forwards secrets or tokens — a new `*_SECRET`/`*_TOKEN`/`API_KEY` in `inference/core/env.py`, or a request/URL/header that may carry `api_key=` reaching a log, exception, response, metric label, or usage payload. ## Review checklist Severity-tag every finding. Reference the numbered Standards below. - **BLOCK** — Ambiguous/unexpected serverless usage-check outcome falls through to `call_next` (fail-open). The `status_code` branches cover 200/401/402; any other value (upstream 5xx, timeout, unmapped code) must deny and MUST NOT be cached. [S1] - **BLOCK** — Missing `api_key` (query params AND JSON body) does not return 401 via `_authorization_error_response` / `_unauthorized_response`. [S2] - **BLOCK** — An ambiguous/error auth result is written into `cached_api_keys` under a reusable key, so it can later be served as an allow. Only definitive outcomes are cached. [S3] - **BLOCK** — A raw secret/api-key/token reaches a log line, exception `str()`, HTTP response, Prometheus label, or persisted usage payload in clear form. [S6] - **BLOCK** — A revert re-opens a previously closed fail-open hole without a compensating control. [S1] - **FLAG** — Auth cache keyed too broadly (not on `(api_key, enforce_credits_verification)` in the serverless path / `api_key` in the local path), risking one tenant's grant served to another. [S3] - **FLAG** — A data/inference route added to the unauthenticated allowlist, or a prefix-match where an exact-match is required. [S4] - **FLAG** — Workspace boundary not enforced against `DEDICATED_DEPLOYMENT_WORKSPACE_URL` / `WORKSPACES_WHITELISTED_FOR_LOCAL_DEPLOYMENT`, or api-key resolvable to a workspace other than the caller's. [S5] - **FLAG** — Assume-identity headers injected without the `ROBOFLOW_ASSUME_IDENTITY_SERVICE_ACCESS_TOKEN` + authorised-workspace gate. [S5] - **FLAG** — Redaction seam (`api_key_safe_raise_for_status(_aiohttp)`) skipped on a Roboflow-API egress whose URL carries `api_key=`. [S6] - **NIT** — New secret env var without a `None`/unset default, or exposed via an `/info`-style introspection endpoint. [S7] ### Not blocking - Do NOT demand redaction refactors on paths that never carry a credential (internal URLs, static assets). - Do NOT flag a correctly-keyed auth cache purely for its TTL choice — `AUTH_CACHE_TTL_SECONDS` (grants) vs `SHORT_AUTH_CACHE_TTL_SECONDS` (denials) is a deliberate split, not a bug. - Do NOT require assume-identity gating on codepaths that never set `assume_identity_authorised_workspace_db_id`. - The `deduct_api_key` convention deliberately reveals a 2-char prefix+postfix for keys ≥ 8 chars (`***` only, below that) — that is the intended redaction, not a leak. - SSRF / user-supplied-URL validation is owned by `review-topic-input-boundary-security` — cross-ref it, do not review it here. ## Standards **S1 — Fail-closed authorization.** In `check_authorization_serverless`, an ambiguous outcome (unexpected `get_serverless_usage_check_async.status_code`, upstream 5xx, timeout, missing field) must deny the request, not admit it. The current branches handle `200` (authorize), `401` (unauthorized), `402` (credits) and return explicitly; any other status falls through to `call_next` — a fail-open bug that silently grants free inference and is invisible in normal traffic (PR #2528 restored the deny-and-no-cache contract; PR #2529 reverted it, so treat reverts here as High-risk needing explicit justification). **S2 — Missing credential ⇒ 401.** `api_key` is read from both query params and JSON body; when `None`, return 401 immediately (`_authorization_error_response(401, ...)` / `_unauthorized_response(...)`), never a pass-through, and never store/act on usage for a missing key (PR #772). **S3 — Cache correctness.** The serverless path keys `cached_api_keys` on `AuthorizationCacheKey = (api_key, enforce_credits_verification)`; the local-deployment path keys on `api_key`. Never widen to a key that could serve tenant A's grant to tenant B. Denials are cached under `SHORT_AUTH_CACHE_TTL_SECONDS`; ambiguous/error results are NOT cached at all (PR #772). **S4 — Unauthenticated-route allowlist.** The middlewares exclude a fixed list of paths (`/`, `/docs`, `/info`, `/healthz`, `/readiness`, `/metrics`, `/openapi.json`, `/model/registry`, and `/workflows/blocks/describe` only for GET / bodyless requests). Match is done against the raw ASGI `request.scope["path"]`, NOT `request.url.path`, so a malicious `Host` header cannot poison the path and slip an authenticated route in (CVE-2026-48710). Any addition must be genuinely safe to expose unauthenticated; prefer exact-match over prefix-match; never add a data/inference route. **S5 — Tenant/workspace isolation.** A caller's api-key must only resolve to and act on its own workspace. `get_roboflow_workspace(_async)` fails (`WorkspaceLoadError`) on an empty workspace. In `check_authorization`, the resolved `workspace_id` must be in `allowed_workspaces` built from `DEDICATED_DEPLOYMENT_WORKSPACE_URL` + `WORKSPACES_WHITELISTED_FOR_LOCAL_DEPLOYMENT`, else 401 (PR #988 simplified this to a pure workspace-ID match). `_add_assume_identity_headers` injects `ASSUME_IDENTITY_ACCESS_TOKEN_HEADER` / `ASSUME_IDENTITY_AUTHORISED_WORKSPACE_HEADER` only when BOTH `ROBOFLOW_ASSUME_IDENTITY_SERVICE_ACCESS_TOKEN` is set AND `assume_identity_authorised_workspace_db_id` is present. **S6 — No secret exposure.** Secrets, api-keys, and tokens must never be logged, echoed in responses/exceptions, or persisted in clear form. Roboflow-API egress whose URL may carry `api_key=` must pass through `api_key_safe_raise_for_status` / `api_key_safe_raise_for_status_aiohttp` (they apply `API_KEY_PATTERN` → `deduct_api_key`) before raising/logging (PR #140, #255, #248, #188). Usage-tracking hashes keys via `_calculate_api_key_hash` and never stores them raw; do not mangle the hash used for usage identity (PR #772, #1831). **S7 — New secret env vars.** A new `*_SECRET`/`*_TOKEN`/api-key in `inference/core/env.py` must default to `None`/unset (never a literal value), must not be printed at startup, and must not be returned by any `/info`-style introspection endpoint. ## Key files & reference PRs - `inference/core/interfaces/http/http_api.py` — `check_authorization_serverless` (canonical fail-closed serverless path: 401 on missing/unauthorized key, explicit 401/402 branches, credential-scoped `cached_api_keys` on `AuthorizationCacheKey`) and `check_authorization` (workspace-boundary enforcement + minimal allowlist). TTLs: `AUTH_CACHE_TTL_SECONDS`, `SHORT_AUTH_CACHE_TTL_SECONDS`. - `inference/core/roboflow_api.py` — `get_serverless_usage_check_async`, `get_roboflow_workspace(_async)`, `_add_assume_identity_headers`, `assume_identity_authorised_workspace_db_id`, `ASSUME_IDENTITY_ACCESS_TOKEN_HEADER`. - `inference/core/utils/requests.py` — `api_key_safe_raise_for_status`, `api_key_safe_raise_for_status_aiohttp`, `deduct_api_key`, `API_KEY_PATTERN`. - `inference/core/env.py` — secret/token declarations (`ROBOFLOW_INTERNAL_SERVICE_SECRET`, `ROBOFLOW_ASSUME_IDENTITY_SERVICE_ACCESS_TOKEN`, `MODAL_TOKEN_SECRET`, `PRELOAD_API_KEY`, `WORKSPACES_WHITELISTED_FOR_LOCAL_DEPLOYMENT`) — all default to `None`/unset; pattern-match new secrets against these. - `inference/usage_tracking/collector.py` — `_calculate_api_key_hash`, `_hashed_api_keys`. - Reference PRs: #2528 / #2529 (serverless fail-open fix + revert), #772 / #1831 (missing-key usage + hash identity), #140 / #255 / #248 / #188 (api-key redaction), #988 (workspace-ID match), #2417 (self-hosted allowlist).