Skip to content

Commit ad586bb

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 4fc551e commit ad586bb

7 files changed

Lines changed: 15 additions & 75 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: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -443,17 +443,15 @@ async def _validate_control_definition(
443443
summary="Render a control template preview",
444444
response_description="Rendered control preview",
445445
)
446+
# Authorized as CONTROLS_CREATE: render is part of the create flow
447+
# (its output feeds the create / set-data endpoints); a caller who
448+
# cannot create controls has no use for the materialized output.
446449
async def render_control_template(
447450
request: RenderControlTemplateRequest,
448451
db: AsyncSession = Depends(get_async_db),
449452
_principal: Principal = Depends(require_operation(Operation.CONTROLS_CREATE)),
450453
) -> 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-
"""
454+
"""Render a template-backed control without persisting it."""
457455
control_def = await _render_and_validate_template_input(
458456
TemplateControlInput(
459457
template=request.template,
@@ -556,15 +554,13 @@ async def create_control(
556554
summary="Get control definition JSON schema",
557555
response_description="JSON schema for ControlDefinition",
558556
)
557+
# Public schema metadata: intentionally has no ``require_operation``
558+
# dependency. The response is static, derived from the model class, and
559+
# exposes no tenant state. Mounting it under the framework would force
560+
# every authorizer to handle a meta-only operation with no permission
561+
# semantics.
559562
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-
"""
563+
"""Return the canonical JSON schema for ControlDefinition."""
568564
return GetControlSchemaResponse(
569565
schema=ControlDefinition.model_json_schema(by_alias=True)
570566
)
@@ -777,6 +773,9 @@ async def set_control_data(
777773
summary="Validate control configuration",
778774
response_description="Validation result",
779775
)
776+
# Authorized as CONTROLS_CREATE rather than CONTROLS_READ: validate
777+
# exercises the same materialization path as create / set-data and is
778+
# only useful to a caller who can act on the result.
780779
async def validate_control_data(
781780
request: ValidateControlDataRequest,
782781
db: AsyncSession = Depends(get_async_db),
@@ -785,11 +784,6 @@ async def validate_control_data(
785784
"""
786785
Validate control configuration data without saving it.
787786
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-
793787
Args:
794788
request: Control configuration data to validate
795789
db: Database session (injected)

server/src/agent_control_server/main.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,8 @@ async def attach_version_header(request, call_next): # type: ignore[no-untyped-
278278
# ``control_binding_router`` rationale below for the
279279
# ``get_api_key_from_header`` mounting pattern. The single route on
280280
# this router without ``require_operation`` is ``GET /controls/schema``,
281-
# which is intentionally public meta — see its endpoint docstring.
281+
# which is intentionally public meta — see the comment above its
282+
# handler in ``endpoints/controls.py``.
282283
control_router,
283284
prefix=api_v1_prefix,
284285
dependencies=[Depends(get_api_key_from_header)],

server/tests/test_controls_auth.py

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -338,28 +338,3 @@ def test_no_auth_mode_allows_writes_without_credentials(
338338
assert "control_id" in resp.json()
339339

340340

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)