fix(server): address runtime auth review feedback

abhinav-galileo · abhinav-galileo · commit 09cb2895ee65 · 2026-05-07T23:07:04.000+05:30
diff --git a/sdks/typescript/src/generated/funcs/auth-runtime-token-exchange.ts b/sdks/typescript/src/generated/funcs/auth-runtime-token-exchange.ts
@@ -32,11 +32,10 @@ import { Result } from "../types/fp.js";
  * @remarks
  * Mint a short-lived runtime token for the requested target.
  *
- * The caller's credential is authenticated and authorized by the
- * installed default authorizer; the resulting :class:`Principal`
- * supplies the actor identity and (when the upstream surfaces it)
- * the grant scopes and expiry. This endpoint then mints a local HS256
- * token whose lifetime cannot outlive the upstream grant.
+ * The caller's credential is authenticated and authorized before a
+ * :class:`Principal` supplies the actor identity, grant scopes, and
+ * expiry. This endpoint then mints a local HS256 token whose lifetime
+ * cannot outlive the grant.
  *
  * Runtime auth must be enabled via
  * ``AGENT_CONTROL_RUNTIME_TOKEN_SECRET``; otherwise the endpoint
diff --git a/sdks/typescript/src/generated/funcs/control-bindings-create.ts b/sdks/typescript/src/generated/funcs/control-bindings-create.ts
@@ -32,8 +32,8 @@ import { Result } from "../types/fp.js";
  * @remarks
  * Attach a control to an opaque external target.
  *
- * Each binding row is scoped to the request namespace as resolved by
- * the active authorizer.
+ * Each binding row is scoped to the namespace associated with the
+ * authenticated request.
  */
 export function controlBindingsCreate(
   client: AgentControlSDKCore,
diff --git a/sdks/typescript/src/generated/funcs/control-bindings-delete.ts b/sdks/typescript/src/generated/funcs/control-bindings-delete.ts
@@ -36,7 +36,7 @@ import { Result } from "../types/fp.js";
  * See the GET-by-id docstring for the authorization scope: this route
  * is namespace-wide because the target identifiers are not available
  * before the binding is loaded. Use ``POST /by-key:delete`` for
- * target-scoped detach that forwards the target to the authorizer.
+ * target-scoped detach that includes the target in the request context.
  */
 export function controlBindingsDelete(
   client: AgentControlSDKCore,
diff --git a/sdks/typescript/src/generated/funcs/control-bindings-get.ts b/sdks/typescript/src/generated/funcs/control-bindings-get.ts
@@ -34,12 +34,11 @@ import { Result } from "../types/fp.js";
  * Read a single control binding by surrogate ID.
  *
  * Authorization is namespace-wide: the binding's target identifiers
- * are not forwarded to the upstream because they are only discoverable
- * after the row is loaded, and ``require_operation`` is single-pass.
+ * are not available until after the row is loaded.
  * Callers whose authorization model requires per-target permissions
  * should use the natural-key endpoints (``PUT /by-key``,
  * ``POST /by-key:delete``) and the target-filtered list endpoint, all
- * of which forward ``(target_type, target_id)`` to the authorizer.
+ * of which include ``(target_type, target_id)`` in the request context.
  */
 export function controlBindingsGet(
   client: AgentControlSDKCore,
diff --git a/sdks/typescript/src/generated/funcs/control-bindings-list.ts b/sdks/typescript/src/generated/funcs/control-bindings-list.ts
@@ -35,7 +35,7 @@ import { Result } from "../types/fp.js";
  * cursor-based pagination. Bindings are ordered by ID descending
  * (newest first). The cursor is opaque to clients: pass back the
  * ``next_cursor`` value verbatim to fetch the following page. The
- * storage namespace is resolved by the active authorizer.
+ * storage namespace is resolved from the authenticated request.
  */
 export function controlBindingsList(
   client: AgentControlSDKCore,
diff --git a/sdks/typescript/src/generated/funcs/control-bindings-update.ts b/sdks/typescript/src/generated/funcs/control-bindings-update.ts
@@ -36,7 +36,7 @@ import { Result } from "../types/fp.js";
  * See the GET-by-id docstring for the authorization scope: this route
  * is namespace-wide because the target identifiers are not available
  * before the binding is loaded. Use ``PUT /by-key`` for target-scoped
- * upserts that forward the target to the authorizer.
+ * upserts that include the target in the request context.
  */
 export function controlBindingsUpdate(
   client: AgentControlSDKCore,
diff --git a/sdks/typescript/src/generated/sdk/auth.ts b/sdks/typescript/src/generated/sdk/auth.ts
@@ -14,11 +14,10 @@ export class Auth extends ClientSDK {
    * @remarks
    * Mint a short-lived runtime token for the requested target.
    *
-   * The caller's credential is authenticated and authorized by the
-   * installed default authorizer; the resulting :class:`Principal`
-   * supplies the actor identity and (when the upstream surfaces it)
-   * the grant scopes and expiry. This endpoint then mints a local HS256
-   * token whose lifetime cannot outlive the upstream grant.
+   * The caller's credential is authenticated and authorized before a
+   * :class:`Principal` supplies the actor identity, grant scopes, and
+   * expiry. This endpoint then mints a local HS256 token whose lifetime
+   * cannot outlive the grant.
    *
    * Runtime auth must be enabled via
    * ``AGENT_CONTROL_RUNTIME_TOKEN_SECRET``; otherwise the endpoint
diff --git a/sdks/typescript/src/generated/sdk/control-bindings.ts b/sdks/typescript/src/generated/sdk/control-bindings.ts
@@ -23,7 +23,7 @@ export class ControlBindings extends ClientSDK {
    * cursor-based pagination. Bindings are ordered by ID descending
    * (newest first). The cursor is opaque to clients: pass back the
    * ``next_cursor`` value verbatim to fetch the following page. The
-   * storage namespace is resolved by the active authorizer.
+   * storage namespace is resolved from the authenticated request.
    */
   async list(
     request?:
@@ -44,8 +44,8 @@ export class ControlBindings extends ClientSDK {
    * @remarks
    * Attach a control to an opaque external target.
    *
-   * Each binding row is scoped to the request namespace as resolved by
-   * the active authorizer.
+   * Each binding row is scoped to the namespace associated with the
+   * authenticated request.
    */
   async create(
     request: models.CreateControlBindingRequest,
@@ -104,7 +104,7 @@ export class ControlBindings extends ClientSDK {
    * See the GET-by-id docstring for the authorization scope: this route
    * is namespace-wide because the target identifiers are not available
    * before the binding is loaded. Use ``POST /by-key:delete`` for
-   * target-scoped detach that forwards the target to the authorizer.
+   * target-scoped detach that includes the target in the request context.
    */
   async delete(
     request:
@@ -125,12 +125,11 @@ export class ControlBindings extends ClientSDK {
    * Read a single control binding by surrogate ID.
    *
    * Authorization is namespace-wide: the binding's target identifiers
-   * are not forwarded to the upstream because they are only discoverable
-   * after the row is loaded, and ``require_operation`` is single-pass.
+   * are not available until after the row is loaded.
    * Callers whose authorization model requires per-target permissions
    * should use the natural-key endpoints (``PUT /by-key``,
    * ``POST /by-key:delete``) and the target-filtered list endpoint, all
-   * of which forward ``(target_type, target_id)`` to the authorizer.
+   * of which include ``(target_type, target_id)`` in the request context.
    */
   async get(
     request:
@@ -153,7 +152,7 @@ export class ControlBindings extends ClientSDK {
    * See the GET-by-id docstring for the authorization scope: this route
    * is namespace-wide because the target identifiers are not available
    * before the binding is loaded. Use ``PUT /by-key`` for target-scoped
-   * upserts that forward the target to the authorizer.
+   * upserts that include the target in the request context.
    */
   async update(
     request:
diff --git a/server/src/agent_control_server/auth_framework/core.py b/server/src/agent_control_server/auth_framework/core.py
@@ -52,11 +52,9 @@ class Operation(StrEnum):
     POLICIES_READ = "policies.read"
     POLICIES_CREATE = "policies.create"
     POLICIES_UPDATE = "policies.update"
-    POLICIES_DELETE = "policies.delete"
     AGENTS_READ = "agents.read"
     AGENTS_CREATE = "agents.create"
     AGENTS_UPDATE = "agents.update"
-    AGENTS_DELETE = "agents.delete"
     RUNTIME_USE = "runtime.use"
 
 
diff --git a/server/src/agent_control_server/auth_framework/providers/header.py b/server/src/agent_control_server/auth_framework/providers/header.py
@@ -45,11 +45,9 @@ class AccessLevel(Enum):
     Operation.POLICIES_READ: AccessLevel.AUTHENTICATED,
     Operation.POLICIES_CREATE: AccessLevel.ADMIN,
     Operation.POLICIES_UPDATE: AccessLevel.ADMIN,
-    Operation.POLICIES_DELETE: AccessLevel.ADMIN,
     Operation.AGENTS_READ: AccessLevel.AUTHENTICATED,
     Operation.AGENTS_CREATE: AccessLevel.AUTHENTICATED,
     Operation.AGENTS_UPDATE: AccessLevel.ADMIN,
-    Operation.AGENTS_DELETE: AccessLevel.ADMIN,
     Operation.RUNTIME_TOKEN_EXCHANGE: AccessLevel.AUTHENTICATED,
     Operation.RUNTIME_USE: AccessLevel.AUTHENTICATED,
 }
diff --git a/server/src/agent_control_server/endpoints/auth.py b/server/src/agent_control_server/endpoints/auth.py
@@ -2,8 +2,8 @@
 
 The runtime auth flow is two-phase: this endpoint is phase one. The
 caller presents a long-lived credential plus ``(target_type,
-target_id)``; the default authorizer authenticates the credential and
-authorizes the implied
+target_id)``; the configured authorization provider authenticates the
+credential and authorizes the implied
 :data:`Operation.RUNTIME_TOKEN_EXCHANGE`. On success, this endpoint
 mints a short-lived local runtime token bound to the supplied target
 and returns it. Subsequent target-bearing runtime calls present the
@@ -56,7 +56,7 @@ class RuntimeTokenExchangeResponse(BaseModel):
 
 
 async def _exchange_context(request: Request) -> dict[str, Any]:
-    """Surface target identifiers to the authorizer's context.
+    """Surface target identifiers to the authorization context.
 
     Reads the request body once. FastAPI caches the parsed body, so the
     endpoint's own Pydantic body model still binds normally.
@@ -89,11 +89,10 @@ async def runtime_token_exchange(
 ) -> RuntimeTokenExchangeResponse:
     """Mint a short-lived runtime token for the requested target.
 
-    The caller's credential is authenticated and authorized by the
-    installed default authorizer; the resulting :class:`Principal`
-    supplies the actor identity and (when the upstream surfaces it)
-    the grant scopes and expiry. This endpoint then mints a local HS256
-    token whose lifetime cannot outlive the upstream grant.
+    The caller's credential is authenticated and authorized before a
+    :class:`Principal` supplies the actor identity, grant scopes, and
+    expiry. This endpoint then mints a local HS256 token whose lifetime
+    cannot outlive the grant.
 
     Runtime auth must be enabled via
     ``AGENT_CONTROL_RUNTIME_TOKEN_SECRET``; otherwise the endpoint
diff --git a/server/src/agent_control_server/endpoints/control_bindings.py b/server/src/agent_control_server/endpoints/control_bindings.py
@@ -35,12 +35,11 @@
 
 
 async def _binding_body_context(request: Request) -> dict[str, Any]:
-    """Surface ``(target_type, target_id)`` to the authorizer's context.
+    """Surface ``(target_type, target_id)`` to the authorization context.
 
     The body-bearing binding endpoints carry the target identifiers in
-    the request payload. Upstream authorizers that make target-scoped
-    decisions need those identifiers; without them the upstream may
-    reject the request.
+    the request payload. Authorization providers can use those
+    identifiers when a request needs target-scoped access checks.
 
     FastAPI caches the parsed body, so the endpoint's own Pydantic
     request model still binds normally.
@@ -58,13 +57,12 @@ async def _binding_body_context(request: Request) -> dict[str, Any]:
 
 
 async def _binding_list_context(request: Request) -> dict[str, Any]:
-    """Surface optional target query parameters to the authorizer.
+    """Surface optional target query parameters to authorization context.
 
     When the GET list endpoint is called with ``target_type`` and
     ``target_id`` query params, the request is target-scoped and the
-    upstream needs the identifiers to make a project-level decision.
-    When neither is present the request is namespace-wide and forwards
-    no target context (upstream may then reject if it requires one).
+    request context includes those identifiers. When neither is present
+    the request is namespace-wide and forwards no target context.
     """
     target_type = request.query_params.get("target_type")
     target_id = request.query_params.get("target_id")
@@ -104,8 +102,8 @@ async def create_control_binding(
 ) -> CreateControlBindingResponse:
     """Attach a control to an opaque external target.
 
-    Each binding row is scoped to the request namespace as resolved by
-    the active authorizer.
+    Each binding row is scoped to the namespace associated with the
+    authenticated request.
     """
     service = ControlBindingsService(db)
     binding = await service.create_binding(
@@ -155,7 +153,7 @@ async def list_control_bindings(
     cursor-based pagination. Bindings are ordered by ID descending
     (newest first). The cursor is opaque to clients: pass back the
     ``next_cursor`` value verbatim to fetch the following page. The
-    storage namespace is resolved by the active authorizer.
+    storage namespace is resolved from the authenticated request.
     """
     parsed_cursor: int | None
     if cursor is None:
@@ -203,12 +201,11 @@ async def get_control_binding(
     """Read a single control binding by surrogate ID.
 
     Authorization is namespace-wide: the binding's target identifiers
-    are not forwarded to the upstream because they are only discoverable
-    after the row is loaded, and ``require_operation`` is single-pass.
+    are not available until after the row is loaded.
     Callers whose authorization model requires per-target permissions
     should use the natural-key endpoints (``PUT /by-key``,
     ``POST /by-key:delete``) and the target-filtered list endpoint, all
-    of which forward ``(target_type, target_id)`` to the authorizer.
+    of which include ``(target_type, target_id)`` in the request context.
     """
     service = ControlBindingsService(db)
     binding = await service.get_binding_or_404(
@@ -234,7 +231,7 @@ async def patch_control_binding(
     See the GET-by-id docstring for the authorization scope: this route
     is namespace-wide because the target identifiers are not available
     before the binding is loaded. Use ``PUT /by-key`` for target-scoped
-    upserts that forward the target to the authorizer.
+    upserts that include the target in the request context.
     """
     service = ControlBindingsService(db)
     binding = await service.set_enabled(
@@ -262,7 +259,7 @@ async def delete_control_binding(
     See the GET-by-id docstring for the authorization scope: this route
     is namespace-wide because the target identifiers are not available
     before the binding is loaded. Use ``POST /by-key:delete`` for
-    target-scoped detach that forwards the target to the authorizer.
+    target-scoped detach that includes the target in the request context.
     """
     service = ControlBindingsService(db)
     await service.delete_binding(namespace_key=principal.namespace_key, binding_id=binding_id)
diff --git a/server/src/agent_control_server/endpoints/controls.py b/server/src/agent_control_server/endpoints/controls.py
@@ -793,12 +793,12 @@ async def set_control_data(
     summary="Validate control configuration",
     response_description="Validation result",
 )
-# Authorized as CONTROLS_READ: validate exercises the materialization
-# path but does not mutate stored control data.
+# Authorized as CONTROLS_CREATE: validate exercises the same materialization
+# path as create/update authoring flows, even though it does not save.
 async def validate_control_data(
     request: ValidateControlDataRequest,
     db: AsyncSession = Depends(get_async_db),
-    _principal: Principal = Depends(require_operation(Operation.CONTROLS_READ)),
+    _principal: Principal = Depends(require_operation(Operation.CONTROLS_CREATE)),
 ) -> ValidateControlDataResponse:
     """
     Validate control configuration data without saving it.
diff --git a/server/src/agent_control_server/namespace.py b/server/src/agent_control_server/namespace.py
diff --git a/server/src/agent_control_server/services/controls.py b/server/src/agent_control_server/services/controls.py
@@ -20,7 +20,6 @@
 
 from ..errors import APIValidationError, NotFoundError
 from ..models import (
-    DEFAULT_NAMESPACE_KEY,
     Control,
     ControlBinding,
     ControlVersion,
@@ -100,7 +99,7 @@ def __init__(self, db: AsyncSession) -> None:
     def create_control(
         self,
         *,
-        namespace_key: str = DEFAULT_NAMESPACE_KEY,
+        namespace_key: str,
         name: str,
         data: dict[str, Any],
     ) -> Control:
@@ -190,7 +189,7 @@ async def active_control_name_exists(
         self,
         name: str,
         *,
-        namespace_key: str = DEFAULT_NAMESPACE_KEY,
+        namespace_key: str,
         exclude_control_id: int | None = None,
     ) -> bool:
         """Return whether an active control already uses the provided name."""
@@ -537,7 +536,7 @@ async def list_active_control_counts_by_agent(
         self,
         agent_names: Sequence[str],
         *,
-        namespace_key: str = DEFAULT_NAMESPACE_KEY,
+        namespace_key: str,
     ) -> dict[str, int]:
         """Return active control counts keyed by agent name."""
         if not agent_names:
diff --git a/server/tests/test_controls_auth.py b/server/tests/test_controls_auth.py
@@ -19,10 +19,9 @@
 
 import uuid
 
-from fastapi.testclient import TestClient
-
 from agent_control_server.auth_framework import set_authorizer
 from agent_control_server.auth_framework.providers import NoAuthProvider
+from fastapi.testclient import TestClient
 
 from .utils import VALID_CONTROL_PAYLOAD
 
@@ -213,21 +212,20 @@ def test_non_admin_cannot_delete_control(
     assert resp.status_code == 403, resp.text
 
 
-def test_non_admin_can_validate_control_data(
+def test_non_admin_cannot_validate_control_data(
     non_admin_client: TestClient,
 ) -> None:
-    """``/controls/validate`` is wired to ``CONTROLS_READ`` because it
-    does not mutate stored control data.
+    """``/controls/validate`` is wired to ``CONTROLS_CREATE`` because it
+    exercises the authoring materialization path.
     """
     # When: a non-admin attempts to validate a draft payload
     resp = non_admin_client.post(
         f"{_CONTROLS_URL}/validate",
         json={"data": VALID_CONTROL_PAYLOAD},
     )
 
-    # Then: validation is allowed for authenticated non-admin callers
-    assert resp.status_code == 200, resp.text
-    assert resp.json()["success"] is True
+    # Then: validation is admin-only
+    assert resp.status_code == 403, resp.text
 
 
 def test_non_admin_cannot_render_template(non_admin_client: TestClient) -> None:
diff --git a/server/tests/test_runtime_token_exchange_endpoint.py b/server/tests/test_runtime_token_exchange_endpoint.py
diff --git a/server/tests/test_services_controls.py b/server/tests/test_services_controls.py
