Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions server/src/agent_control_server/endpoints/control_bindings.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,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 resolve the target's
owning project (e.g., Galileo's ``check_management_access``) need
those identifiers to make a project-level decision; without them the
upstream returns 400.
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.
Expand All @@ -60,13 +58,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")
Expand Down
32 changes: 23 additions & 9 deletions server/src/agent_control_server/endpoints/controls.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
from sqlalchemy.exc import IntegrityError
from sqlalchemy.ext.asyncio import AsyncSession

from ..auth import require_admin_key
from ..auth_framework import Operation, Principal, require_operation
from ..db import get_async_db
from ..errors import (
APIValidationError,
Expand Down Expand Up @@ -443,9 +443,11 @@ async def _validate_control_definition(
summary="Render a control template preview",
response_description="Rendered control preview",
)
# Rendering is part of the authoring flow, so require create access.
async def render_control_template(
request: RenderControlTemplateRequest,
db: AsyncSession = Depends(get_async_db),
_principal: Principal = Depends(require_operation(Operation.CONTROLS_CREATE)),
) -> RenderControlTemplateResponse:
"""Render a template-backed control without persisting it."""
control_def = await _render_and_validate_template_input(
Expand All @@ -461,13 +463,14 @@ async def render_control_template(

@router.put(
"",
dependencies=[Depends(require_admin_key)],
response_model=CreateControlResponse,
summary="Create a new control",
response_description="Created control ID",
)
async def create_control(
request: CreateControlRequest, db: AsyncSession = Depends(get_async_db)
request: CreateControlRequest,
db: AsyncSession = Depends(get_async_db),
_principal: Principal = Depends(require_operation(Operation.CONTROLS_CREATE)),
) -> CreateControlResponse:
"""
Create a new control with a unique name.
Expand Down Expand Up @@ -549,6 +552,7 @@ async def create_control(
summary="Get control definition JSON schema",
response_description="JSON schema for ControlDefinition",
)
# Public schema metadata: no tenant state, no auth operation.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: GET /controls/schema is now anonymous. Previously gated by require_api_key at the router. The diff intentionally drops auth and test_schema_endpoint_reachable_without_credentials pins it. Schema content is ControlDefinition.model_json_schema() (no tenant data), so likely fine, but worth explicit security sign-off because it changes the "every /api/v1/controls/* route requires creds" invariant operators may have depended on.

async def get_control_schema() -> GetControlSchemaResponse:
"""Return the canonical JSON schema for ControlDefinition."""
return GetControlSchemaResponse(
Expand All @@ -563,7 +567,9 @@ async def get_control_schema() -> GetControlSchemaResponse:
response_description="Control metadata and configuration",
)
async def get_control(
control_id: int, db: AsyncSession = Depends(get_async_db)
control_id: int,
db: AsyncSession = Depends(get_async_db),
_principal: Principal = Depends(require_operation(Operation.CONTROLS_READ)),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The endpoint binds _principal for the auth side-effect but never threads _principal.namespace_key into the ControlService calls. list_controls_page doesn't even accept a namespace; get_active_control_or_404 accepts one but isn't called with it. Today this is fine because HeaderAuthProvider always returns DEFAULT_NAMESPACE_KEY, but the moment any other provider lands (and the framework is explicitly built for that), controls become cross-tenant readable/writable here.

Recommended fix in this PR (cheap, defensive): add a small _require_default_namespace(principal) helper in this module and call it from every endpoint that captures _principal. That converts a silent forward-compat hazard into a loud deploy-time failure the day a non-default namespace shows up. Threading it properly through the service layer can be a follow-up PR.

) -> GetControlResponse:
"""
Retrieve a control by ID including its name and configuration data.
Expand Down Expand Up @@ -600,7 +606,9 @@ async def get_control(
response_description="Control data payload",
)
async def get_control_data(
control_id: int, db: AsyncSession = Depends(get_async_db)
control_id: int,
db: AsyncSession = Depends(get_async_db),
_principal: Principal = Depends(require_operation(Operation.CONTROLS_READ)),
) -> GetControlDataResponse:
"""
Retrieve the configuration data for a control.
Expand Down Expand Up @@ -640,6 +648,7 @@ async def list_control_versions(
),
limit: int = Query(_DEFAULT_PAGINATION_LIMIT, ge=1, le=_MAX_PAGINATION_LIMIT),
db: AsyncSession = Depends(get_async_db),
_principal: Principal = Depends(require_operation(Operation.CONTROLS_READ)),
) -> ListControlVersionsResponse:
"""List control versions ordered newest-first using cursor-based pagination."""
page = await ControlService(db).list_versions(control_id, cursor=cursor, limit=limit)
Expand Down Expand Up @@ -673,6 +682,7 @@ async def get_control_version(
control_id: int,
version_num: int,
db: AsyncSession = Depends(get_async_db),
_principal: Principal = Depends(require_operation(Operation.CONTROLS_READ)),
) -> GetControlVersionResponse:
"""Return a specific control version, including its raw persisted snapshot."""
version = await ControlService(db).get_version_or_404(control_id, version_num)
Expand All @@ -687,7 +697,6 @@ async def get_control_version(

@router.put(
"/{control_id}/data",
dependencies=[Depends(require_admin_key)],
response_model=SetControlDataResponse,
summary="Update control configuration data",
response_description="Success confirmation",
Expand All @@ -696,6 +705,7 @@ async def set_control_data(
control_id: int,
request: SetControlDataRequest,
db: AsyncSession = Depends(get_async_db),
_principal: Principal = Depends(require_operation(Operation.CONTROLS_UPDATE)),
) -> SetControlDataResponse:
"""
Update the configuration data for a control.
Expand Down Expand Up @@ -757,8 +767,11 @@ async def set_control_data(
summary="Validate control configuration",
response_description="Validation result",
)
# Validation uses the authoring path, so require create access.
async def validate_control_data(
request: ValidateControlDataRequest, db: AsyncSession = Depends(get_async_db)
request: ValidateControlDataRequest,
db: AsyncSession = Depends(get_async_db),
_principal: Principal = Depends(require_operation(Operation.CONTROLS_CREATE)),
) -> ValidateControlDataResponse:
"""
Validate control configuration data without saving it.
Expand Down Expand Up @@ -798,6 +811,7 @@ async def list_controls(
execution: str | None = Query(None, description="Filter by execution ('server' or 'sdk')"),
tag: str | None = Query(None, description="Filter by tag"),
db: AsyncSession = Depends(get_async_db),
_principal: Principal = Depends(require_operation(Operation.CONTROLS_READ)),
) -> ListControlsResponse:
"""
List all controls with optional filtering and cursor-based pagination.
Expand Down Expand Up @@ -884,7 +898,6 @@ async def list_controls(

@router.delete(
"/{control_id}",
dependencies=[Depends(require_admin_key)],
response_model=DeleteControlResponse,
summary="Delete a control",
response_description="Deletion confirmation with dissociation info",
Expand All @@ -897,6 +910,7 @@ async def delete_control(
"If false, fail if control is associated with any policy or agent.",
),
db: AsyncSession = Depends(get_async_db),
_principal: Principal = Depends(require_operation(Operation.CONTROLS_DELETE)),
) -> DeleteControlResponse:
"""
Delete a control by ID.
Expand Down Expand Up @@ -1035,7 +1049,6 @@ async def delete_control(

@router.patch(
"/{control_id}",
dependencies=[Depends(require_admin_key)],
response_model=PatchControlResponse,
summary="Update control metadata",
response_description="Updated control information",
Expand All @@ -1044,6 +1057,7 @@ async def patch_control(
control_id: int,
request: PatchControlRequest,
db: AsyncSession = Depends(get_async_db),
_principal: Principal = Depends(require_operation(Operation.CONTROLS_UPDATE)),
) -> PatchControlResponse:
"""
Update control metadata (name and/or enabled status).
Expand Down
6 changes: 4 additions & 2 deletions server/src/agent_control_server/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,10 @@ async def attach_version_header(request, call_next): # type: ignore[no-untyped-
dependencies=[Depends(require_api_key)],
)
app.include_router(
# Endpoint dependencies handle auth; this advertises X-API-Key.
control_router,
prefix=api_v1_prefix,
dependencies=[Depends(require_api_key)],
dependencies=[Depends(get_api_key_from_header)],
)
app.include_router(
# The auth framework on each endpoint owns authentication and
Expand All @@ -300,9 +301,10 @@ async def attach_version_header(request, call_next): # type: ignore[no-untyped-
dependencies=[Depends(get_api_key_from_header)],
)
app.include_router(
# Endpoint dependencies handle auth; this advertises X-API-Key.
control_template_router,
prefix=api_v1_prefix,
dependencies=[Depends(require_api_key)],
dependencies=[Depends(get_api_key_from_header)],
)
app.include_router(
evaluation_router,
Expand Down
Loading
Loading