Skip to content

Commit 0a2092d

Browse files
fix(server): keep public docstrings API-level on migrated controls routes
Move auth-framework rationale on /controls/schema, /controls/validate, and /control-templates/render from route docstrings into normal code comments. The docstrings flow into the generated TypeScript SDK as public API documentation, so internal terminology like ``require_operation`` and "upstream authorizer" should not appear there. Function-level comments preserve the rationale for readers of the source. Also remove the skipped placeholder test for the project-scoped credential deny scenario; that scenario depends on a deployment-side provider configuration that is not part of the OSS server, so tracking it as a permanent skipped test in this repo was the wrong home for it. Regenerate the TypeScript SDK to drop the leaked rationale lines.
1 parent 5e6811b commit 0a2092d

7 files changed

Lines changed: 13 additions & 115 deletions

File tree

sdks/typescript/src/generated/funcs/controls-get-schema.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,6 @@ import { Result } from "../types/fp.js";
2727
*
2828
* @remarks
2929
* Return the canonical JSON schema for ControlDefinition.
30-
*
31-
* Intentionally has no ``require_operation`` dependency: the schema is
32-
* static metadata derived from the model class and exposes no tenant
33-
* state. Routing it through the auth framework would force callers
34-
* (and the upstream authorizer) to handle a meta-only operation that
35-
* has no permission semantics.
3630
*/
3731
export function controlsGetSchema(
3832
client: AgentControlSDKCore,

sdks/typescript/src/generated/funcs/controls-render-template.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@ import { Result } from "../types/fp.js";
3131
*
3232
* @remarks
3333
* Render a template-backed control without persisting it.
34-
*
35-
* Authorized as ``controls.create``: rendering is part of the authoring
36-
* flow (the result feeds the create / update endpoints), so a caller
37-
* who cannot create controls has no use for the materialized output.
3834
*/
3935
export function controlsRenderTemplate(
4036
client: AgentControlSDKCore,

sdks/typescript/src/generated/funcs/controls-validate-data.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,6 @@ import { Result } from "../types/fp.js";
3232
* @remarks
3333
* Validate control configuration data without saving it.
3434
*
35-
* Authorized as ``controls.create`` rather than ``controls.read``:
36-
* validation exercises the full create / update materialization path
37-
* and exists to support authoring, so a caller who cannot create
38-
* controls has no use for the result.
39-
*
4035
* Args:
4136
* request: Control configuration data to validate
4237
* db: Database session (injected)

sdks/typescript/src/generated/sdk/controls.ts

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,6 @@ export class Controls extends ClientSDK {
2525
*
2626
* @remarks
2727
* Render a template-backed control without persisting it.
28-
*
29-
* Authorized as ``controls.create``: rendering is part of the authoring
30-
* flow (the result feeds the create / update endpoints), so a caller
31-
* who cannot create controls has no use for the materialized output.
3228
*/
3329
async renderTemplate(
3430
request: models.RenderControlTemplateRequest,
@@ -114,12 +110,6 @@ export class Controls extends ClientSDK {
114110
*
115111
* @remarks
116112
* Return the canonical JSON schema for ControlDefinition.
117-
*
118-
* Intentionally has no ``require_operation`` dependency: the schema is
119-
* static metadata derived from the model class and exposes no tenant
120-
* state. Routing it through the auth framework would force callers
121-
* (and the upstream authorizer) to handle a meta-only operation that
122-
* has no permission semantics.
123113
*/
124114
async getSchema(
125115
options?: RequestOptions,
@@ -136,11 +126,6 @@ export class Controls extends ClientSDK {
136126
* @remarks
137127
* Validate control configuration data without saving it.
138128
*
139-
* Authorized as ``controls.create`` rather than ``controls.read``:
140-
* validation exercises the full create / update materialization path
141-
* and exists to support authoring, so a caller who cannot create
142-
* controls has no use for the result.
143-
*
144129
* Args:
145130
* request: Control configuration data to validate
146131
* db: Database session (injected)

server/src/agent_control_server/endpoints/controls.py

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -443,17 +443,13 @@ async def _validate_control_definition(
443443
summary="Render a control template preview",
444444
response_description="Rendered control preview",
445445
)
446+
# Rendering is part of the authoring flow, so require create access.
446447
async def render_control_template(
447448
request: RenderControlTemplateRequest,
448449
db: AsyncSession = Depends(get_async_db),
449450
_principal: Principal = Depends(require_operation(Operation.CONTROLS_CREATE)),
450451
) -> RenderControlTemplateResponse:
451-
"""Render a template-backed control without persisting it.
452-
453-
Authorized as ``controls.create``: rendering is part of the authoring
454-
flow (the result feeds the create / update endpoints), so a caller
455-
who cannot create controls has no use for the materialized output.
456-
"""
452+
"""Render a template-backed control without persisting it."""
457453
control_def = await _render_and_validate_template_input(
458454
TemplateControlInput(
459455
template=request.template,
@@ -556,15 +552,9 @@ async def create_control(
556552
summary="Get control definition JSON schema",
557553
response_description="JSON schema for ControlDefinition",
558554
)
555+
# Public schema metadata: no tenant state, no auth operation.
559556
async def get_control_schema() -> GetControlSchemaResponse:
560-
"""Return the canonical JSON schema for ControlDefinition.
561-
562-
Intentionally has no ``require_operation`` dependency: the schema is
563-
static metadata derived from the model class and exposes no tenant
564-
state. Routing it through the auth framework would force callers
565-
(and the upstream authorizer) to handle a meta-only operation that
566-
has no permission semantics.
567-
"""
557+
"""Return the canonical JSON schema for ControlDefinition."""
568558
return GetControlSchemaResponse(
569559
schema=ControlDefinition.model_json_schema(by_alias=True)
570560
)
@@ -777,6 +767,7 @@ async def set_control_data(
777767
summary="Validate control configuration",
778768
response_description="Validation result",
779769
)
770+
# Validation uses the authoring path, so require create access.
780771
async def validate_control_data(
781772
request: ValidateControlDataRequest,
782773
db: AsyncSession = Depends(get_async_db),
@@ -785,11 +776,6 @@ async def validate_control_data(
785776
"""
786777
Validate control configuration data without saving it.
787778
788-
Authorized as ``controls.create`` rather than ``controls.read``:
789-
validation exercises the full create / update materialization path
790-
and exists to support authoring, so a caller who cannot create
791-
controls has no use for the result.
792-
793779
Args:
794780
request: Control configuration data to validate
795781
db: Database session (injected)

server/src/agent_control_server/main.py

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -273,12 +273,7 @@ async def attach_version_header(request, call_next): # type: ignore[no-untyped-
273273
dependencies=[Depends(require_api_key)],
274274
)
275275
app.include_router(
276-
# ``/controls`` CRUD goes through the auth framework on each
277-
# endpoint (``require_operation(Operation.CONTROLS_*)``); see the
278-
# ``control_binding_router`` rationale below for the
279-
# ``get_api_key_from_header`` mounting pattern. The single route on
280-
# this router without ``require_operation`` is ``GET /controls/schema``,
281-
# which is intentionally public meta — see its endpoint docstring.
276+
# Endpoint dependencies handle auth; this advertises X-API-Key.
282277
control_router,
283278
prefix=api_v1_prefix,
284279
dependencies=[Depends(get_api_key_from_header)],
@@ -306,9 +301,7 @@ async def attach_version_header(request, call_next): # type: ignore[no-untyped-
306301
dependencies=[Depends(get_api_key_from_header)],
307302
)
308303
app.include_router(
309-
# Control templates: ``/render`` is on the auth framework via
310-
# ``require_operation(Operation.CONTROLS_CREATE)``; same mounting
311-
# pattern as the controls and control-bindings routers.
304+
# Endpoint dependencies handle auth; this advertises X-API-Key.
312305
control_template_router,
313306
prefix=api_v1_prefix,
314307
dependencies=[Depends(get_api_key_from_header)],

server/tests/test_controls_auth.py

Lines changed: 6 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,4 @@
1-
"""HTTP-level coverage for the auth seam on ``/controls`` and
2-
``/control-templates``.
3-
4-
These tests exercise the wiring of ``require_operation`` on each route
5-
through the default ``HeaderAuthProvider``: read operations require any
6-
valid credential (``CONTROLS_READ`` -> ``AUTHENTICATED``), write
7-
operations require an admin credential
8-
(``CONTROLS_CREATE`` / ``CONTROLS_UPDATE`` / ``CONTROLS_DELETE`` ->
9-
``ADMIN``), and ``GET /controls/schema`` is intentionally outside the
10-
framework so it stays publicly reachable.
11-
12-
The provider primitives themselves are exercised in
13-
``tests/test_auth_framework.py``; this file focuses on each endpoint
14-
calling the right ``Operation`` so a future change to the operation
15-
mapping is caught at the route level.
16-
"""
1+
"""HTTP-level auth coverage for ``/controls`` and ``/control-templates``."""
172

183
from __future__ import annotations
194

@@ -42,7 +27,7 @@ def _create_control(client: TestClient, name: str | None = None) -> int:
4227

4328

4429
# ---------------------------------------------------------------------------
45-
# /controls/schema is intentionally public meta — no require_operation.
30+
# /controls/schema is intentionally public metadata.
4631
# ---------------------------------------------------------------------------
4732

4833

@@ -165,7 +150,7 @@ def test_non_admin_cannot_create_control(non_admin_client: TestClient) -> None:
165150
},
166151
)
167152

168-
# Then: the request is forbidden by the auth seam
153+
# Then: the request is forbidden
169154
assert resp.status_code == 403, resp.text
170155

171156

@@ -217,12 +202,7 @@ def test_non_admin_cannot_delete_control(
217202
def test_non_admin_cannot_validate_control_data(
218203
non_admin_client: TestClient,
219204
) -> None:
220-
"""``/controls/validate`` is wired to ``CONTROLS_CREATE`` rather than
221-
``CONTROLS_READ`` because validation exercises the create / update
222-
materialization path; a caller who cannot create has no use for the
223-
result. This pins that decision so it can't drift to ``READ``
224-
accidentally.
225-
"""
205+
"""``/controls/validate`` requires ``CONTROLS_CREATE``."""
226206
# When: a non-admin attempts to validate a draft payload
227207
resp = non_admin_client.post(
228208
f"{_CONTROLS_URL}/validate",
@@ -234,19 +214,14 @@ def test_non_admin_cannot_validate_control_data(
234214

235215

236216
def test_non_admin_cannot_render_template(non_admin_client: TestClient) -> None:
237-
"""``/control-templates/render`` is wired to ``CONTROLS_CREATE`` for
238-
the same reason as ``/validate``: rendering is part of the authoring
239-
flow. The 422 path is not exercised here — only the auth gate is
240-
asserted, so the request shape need not validate.
241-
"""
217+
"""``/control-templates/render`` requires ``CONTROLS_CREATE``."""
242218
# When: a non-admin attempts to render a template
243219
resp = non_admin_client.post(
244220
f"{_TEMPLATES_URL}/render",
245221
json={"template": {}, "template_values": {}},
246222
)
247223

248-
# Then: rendering is admin-only — the auth gate fires before body
249-
# validation reaches the materialization path
224+
# Then: rendering is admin-only
250225
assert resp.status_code == 403, resp.text
251226

252227

@@ -337,29 +312,3 @@ def test_no_auth_mode_allows_writes_without_credentials(
337312
assert resp.status_code == 200, resp.text
338313
assert "control_id" in resp.json()
339314

340-
341-
# ---------------------------------------------------------------------------
342-
# Project-scoped API key deny — pending header forwarding follow-up.
343-
# ---------------------------------------------------------------------------
344-
345-
346-
@pytest.mark.skip(
347-
reason=(
348-
"Requires the upstream auth provider to forward an additional "
349-
"configurable credential header. The default forward set is "
350-
"fixed to (X-API-Key, Authorization, Cookie); deployments that "
351-
"use a different credential header can't surface a "
352-
"project-scoped credential to upstream until that becomes "
353-
"configurable. Re-enable when the follow-up PR adds an "
354-
"operator-configurable extra forward list."
355-
)
356-
)
357-
def test_project_scoped_credential_denied_on_org_scoped_controls() -> None:
358-
"""Stub for the deny-test promised by the upstream provider's
359-
response contract: a project-scoped credential calling an
360-
org-scoped operation (``controls.*``) should resolve to a 403 from
361-
the upstream. The end-to-end path is unreachable today because the
362-
provider's credential-forward list is not configurable; tracked as
363-
the next follow-up after this PR.
364-
"""
365-
pytest.fail("test stub — see skip reason")

0 commit comments

Comments
 (0)