Skip to content

Commit 806eb68

Browse files
fix(sdk): the 502 in api (#265)
## Summary - Fixed GET /api/v1/controls authorization context handling. - Agent Control now skips target-binding authorization unless both attachment target filters are present and valid. - Incomplete filter pairs return a local HTTP 422 instead of sending invalid context upstream. - This prevents the upstream validation failure that caused intermittent 422/502 responses in staging. Related Jira: https://splunk.atlassian.net/browse/SAO-17310 ## Scope User-facing/API changes: - Incomplete attachment target filters are rejected with HTTP 422. - Org-level controls remain supported without target bindings. - Target attachments are returned only when complete target filters are supplied. Internal changes: - Updated controls endpoint authorization dependency and validation. - Added regression coverage for no filters, complete filters, and incomplete filters. - Preserved diagnosability of unexpected upstream 4xx responses. ## Risk and Rollout Risk level: low Rollback plan:- Revert the Agent Control changes and redeploy the previous version. ## Testing - [ x] Added or updated automated tests - [ ] Ran `make check` (or explained why not) - [ ] Manually verified behavior ## Checklist - Linked issue/spec: Incident #51954 - Updated inline API description for target attachment behavior - [ ] Included any required follow-up tasks
1 parent 9d12df0 commit 806eb68

4 files changed

Lines changed: 121 additions & 78 deletions

File tree

‎sdks/typescript/src/generated/models/operations/list-controls-api-v1-controls-get.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export type ListControlsApiV1ControlsGetRequest = {
4444
*/
4545
tag?: string | null | undefined;
4646
/**
47-
* When true, include direct agent associations, policy associations, and target bindings for each listed control.
47+
* When true, include direct agent associations, policy associations, and, when target filters are supplied, target bindings for each listed control.
4848
*/
4949
includeAttachments?: boolean | undefined;
5050
/**

‎server/src/agent_control_server/endpoints/controls.py‎

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@
4747
from ..errors import (
4848
APIError,
4949
APIValidationError,
50-
AuthenticationError,
5150
ConflictError,
5251
DatabaseError,
5352
ForbiddenError,
@@ -143,19 +142,18 @@ async def _clone_and_bind_context(request: Request) -> dict[str, Any]:
143142
}
144143

145144

146-
def _attachment_target_context(request: Request) -> dict[str, str]:
147-
context: dict[str, str] = {}
145+
def _attachment_target_context(request: Request) -> dict[str, str] | None:
146+
"""Return target context only when both attachment identifiers are valid."""
148147
target_type = request.query_params.get("attachment_target_type")
149148
target_id = request.query_params.get("attachment_target_id")
150-
if target_type is not None:
151-
if not _is_target_context_value(target_type):
152-
return {}
153-
context["target_type"] = target_type
154-
if target_id is not None:
155-
if not _is_target_context_value(target_id):
156-
return {}
157-
context["target_id"] = target_id
158-
return context
149+
if (
150+
target_type is None
151+
or target_id is None
152+
or not _is_target_context_value(target_type)
153+
or not _is_target_context_value(target_id)
154+
):
155+
return None
156+
return {"target_type": target_type, "target_id": target_id}
159157

160158

161159
async def _optional_attachment_target_principal(request: Request) -> Principal | None:
@@ -165,20 +163,13 @@ async def _optional_attachment_target_principal(request: Request) -> Principal |
165163
if include_attachments.lower() not in _TRUE_QUERY_VALUES:
166164
return None
167165
target_context = _attachment_target_context(request)
168-
try:
169-
return await get_authorizer(Operation.CONTROL_BINDINGS_READ).authorize(
170-
request,
171-
Operation.CONTROL_BINDINGS_READ,
172-
target_context,
173-
)
174-
except (AuthenticationError, ForbiddenError, NotFoundError):
175-
if target_context:
176-
raise
177-
return None
178-
except APIError:
179-
if target_context:
180-
raise
166+
if target_context is None:
181167
return None
168+
return await get_authorizer(Operation.CONTROL_BINDINGS_READ).authorize(
169+
request,
170+
Operation.CONTROL_BINDINGS_READ,
171+
target_context,
172+
)
182173

183174

184175
def _generated_clone_name(source_id: int, source_name: str) -> str:
@@ -235,6 +226,31 @@ def _validate_attachment_filters(
235226
attachment_target_id: str | None,
236227
) -> None:
237228
if include_attachments:
229+
if (attachment_target_type is None) != (attachment_target_id is None):
230+
raise APIValidationError(
231+
error_code=ErrorCode.VALIDATION_ERROR,
232+
detail="Attachment target filters must be supplied together.",
233+
resource="Control",
234+
hint=(
235+
"Set both attachment_target_type and attachment_target_id, "
236+
"or remove both filters."
237+
),
238+
errors=[
239+
ValidationErrorItem(
240+
resource="Control",
241+
field=(
242+
"attachment_target_type"
243+
if attachment_target_type is None
244+
else "attachment_target_id"
245+
),
246+
code="missing_required_parameter",
247+
message=(
248+
"Set attachment_target_type and attachment_target_id "
249+
"together."
250+
),
251+
)
252+
],
253+
)
238254
return
239255
if attachment_target_type is None and attachment_target_id is None:
240256
return
@@ -1157,7 +1173,8 @@ async def list_controls(
11571173
False,
11581174
description=(
11591175
"When true, include direct agent associations, policy associations, "
1160-
"and target bindings for each listed control."
1176+
"and, when target filters are supplied, target bindings for each "
1177+
"listed control."
11611178
),
11621179
),
11631180
attachment_target_type: str | None = Query(

‎server/tests/test_auth_framework.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,7 @@ async def test_http_upstream_fails_closed_on_5xx():
476476
@pytest.mark.asyncio
477477
@pytest.mark.parametrize("status", [400, 422])
478478
async def test_http_upstream_unexpected_4xx_reports_upstream_rejection(status):
479+
"""Unexpected upstream 4xx responses retain the contract failure signal."""
479480
provider = _build_upstream(lambda req: httpx.Response(status, text="bad request"))
480481

481482
with pytest.raises(APIError) as exc_info:
@@ -484,6 +485,7 @@ async def test_http_upstream_unexpected_4xx_reports_upstream_rejection(status):
484485
assert exc_info.value.status_code == 502
485486
assert exc_info.value.error_code == "AUTH_UPSTREAM_REJECTED"
486487
assert str(status) in exc_info.value.detail
488+
assert "request shape" in exc_info.value.hint
487489

488490

489491
@pytest.mark.asyncio

‎server/tests/test_controls_additional.py‎

Lines changed: 75 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111
import pytest
1212
from agent_control_evaluators import RegexEvaluatorConfig
1313
from agent_control_models import ConditionNode
14-
from agent_control_models.errors import ErrorCode, ErrorReason
14+
from agent_control_models.errors import ErrorCode
1515
from agent_control_server.auth_framework import Operation, Principal, set_authorizer
1616
from agent_control_server.db import get_async_db
1717
from agent_control_server.endpoints import controls as controls_module
18-
from agent_control_server.errors import APIError, BadRequestError, ForbiddenError
18+
from agent_control_server.errors import BadRequestError, ForbiddenError
1919
from agent_control_server.main import app
2020
from agent_control_server.models import (
2121
DEFAULT_NAMESPACE_KEY,
@@ -1355,20 +1355,18 @@ def test_list_controls_expands_filtered_control_attachments(
13551355
}
13561356

13571357

1358-
def test_list_controls_caps_inline_target_attachments(
1358+
def test_list_controls_omits_target_attachments_without_filter(
13591359
client: TestClient,
13601360
) -> None:
13611361
control_id, control_name = _create_control(client, name=f"Attachments-{uuid.uuid4()}")
13621362
_set_control_data(client, control_id, deepcopy(VALID_CONTROL_PAYLOAD))
1363-
binding_ids = [
1363+
for index in range(25):
13641364
_create_target_binding(
13651365
client,
13661366
control_id=control_id,
13671367
target_type="log_stream",
13681368
target_id=f"ls-{index}",
13691369
)
1370-
for index in range(25)
1371-
]
13721370

13731371
resp = client.get(
13741372
"/api/v1/controls",
@@ -1380,15 +1378,12 @@ def test_list_controls_caps_inline_target_attachments(
13801378

13811379
assert resp.status_code == 200, resp.text
13821380
attachments = resp.json()["controls"][0]["attachments"]
1383-
assert len(attachments["targets"]) == 20
1384-
assert attachments["targets_total"] == 25
1385-
assert attachments["targets_truncated"] is True
1386-
assert [target["binding_id"] for target in attachments["targets"]] == list(
1387-
reversed(binding_ids[-20:])
1388-
)
1381+
assert attachments["targets"] == []
1382+
assert attachments["targets_total"] == 0
1383+
assert attachments["targets_truncated"] is False
13891384

13901385

1391-
def test_list_controls_omits_targets_without_binding_read_authorization(
1386+
def test_list_controls_skips_target_authorization_without_filter(
13921387
client: TestClient,
13931388
) -> None:
13941389
control_id, control_name = _create_control(client, name=f"Attachments-{uuid.uuid4()}")
@@ -1435,59 +1430,84 @@ async def authorize(
14351430
"targets_total": 0,
14361431
"targets_truncated": False,
14371432
}
1438-
assert (Operation.CONTROL_BINDINGS_READ, {}) in calls
1433+
assert not any(operation == Operation.CONTROL_BINDINGS_READ for operation, _ in calls)
14391434

14401435

1441-
def test_list_controls_omits_targets_when_broad_binding_read_upstream_rejects(
1436+
@pytest.mark.parametrize(
1437+
("params", "expected_context"),
1438+
[
1439+
({"include_attachments": "true"}, None),
1440+
(
1441+
{
1442+
"include_attachments": "true",
1443+
"attachment_target_type": "log_stream",
1444+
"attachment_target_id": "ls-prod",
1445+
},
1446+
{"target_type": "log_stream", "target_id": "ls-prod"},
1447+
),
1448+
],
1449+
)
1450+
def test_list_controls_builds_complete_attachment_auth_context(
14421451
client: TestClient,
1452+
params: dict[str, str],
1453+
expected_context: dict[str, str] | None,
14431454
) -> None:
1444-
control_id, control_name = _create_control(client, name=f"Attachments-{uuid.uuid4()}")
1445-
_set_control_data(client, control_id, deepcopy(VALID_CONTROL_PAYLOAD))
1446-
_create_target_binding(
1447-
client,
1448-
control_id=control_id,
1449-
target_type="log_stream",
1450-
target_id="ls-prod",
1451-
)
14521455
calls: list[tuple[Operation, dict[str, Any] | None]] = []
14531456

1454-
class BindingReadRejectAuthorizer:
1457+
class RecordingAuthorizer:
14551458
async def authorize(
14561459
self,
14571460
request: Any,
14581461
operation: Operation,
14591462
context: dict[str, Any] | None = None,
14601463
) -> Principal:
14611464
calls.append((operation, context))
1462-
if operation == Operation.CONTROL_BINDINGS_READ:
1463-
raise APIError(
1464-
status_code=502,
1465-
error_code=ErrorCode.AUTH_UPSTREAM_REJECTED,
1466-
reason=ErrorReason.INTERNAL_ERROR,
1467-
detail="Authorization service rejected the authorization check.",
1468-
)
14691465
return Principal(namespace_key=DEFAULT_NAMESPACE_KEY, is_admin=True)
14701466

1471-
set_authorizer(BindingReadRejectAuthorizer())
1467+
set_authorizer(RecordingAuthorizer())
14721468

1473-
resp = client.get(
1474-
"/api/v1/controls",
1475-
params={
1476-
"name": control_name,
1477-
"include_attachments": "true",
1478-
},
1479-
)
1469+
resp = client.get("/api/v1/controls", params=params)
14801470

14811471
assert resp.status_code == 200, resp.text
1482-
controls = resp.json()["controls"]
1483-
assert controls[0]["attachments"] == {
1484-
"agents": [],
1485-
"policies": [],
1486-
"targets": [],
1487-
"targets_total": 0,
1488-
"targets_truncated": False,
1489-
}
1490-
assert (Operation.CONTROL_BINDINGS_READ, {}) in calls
1472+
binding_contexts = [
1473+
context for operation, context in calls if operation == Operation.CONTROL_BINDINGS_READ
1474+
]
1475+
if expected_context is None:
1476+
assert binding_contexts == []
1477+
else:
1478+
assert binding_contexts == [expected_context]
1479+
1480+
1481+
@pytest.mark.parametrize(
1482+
"params",
1483+
[
1484+
{"include_attachments": "true", "attachment_target_type": "log_stream"},
1485+
{"include_attachments": "true", "attachment_target_id": "ls-prod"},
1486+
],
1487+
)
1488+
def test_list_controls_rejects_incomplete_attachment_filters_without_auth_call(
1489+
client: TestClient,
1490+
params: dict[str, str],
1491+
) -> None:
1492+
calls: list[tuple[Operation, dict[str, Any] | None]] = []
1493+
1494+
class RecordingAuthorizer:
1495+
async def authorize(
1496+
self,
1497+
request: Any,
1498+
operation: Operation,
1499+
context: dict[str, Any] | None = None,
1500+
) -> Principal:
1501+
calls.append((operation, context))
1502+
return Principal(namespace_key=DEFAULT_NAMESPACE_KEY, is_admin=True)
1503+
1504+
set_authorizer(RecordingAuthorizer())
1505+
1506+
resp = client.get("/api/v1/controls", params=params)
1507+
1508+
assert resp.status_code == 422, resp.text
1509+
assert resp.json()["error_code"] == "VALIDATION_ERROR"
1510+
assert not any(operation == Operation.CONTROL_BINDINGS_READ for operation, _ in calls)
14911511

14921512

14931513
def test_list_controls_rejects_target_filter_without_binding_read_authorization(
@@ -1538,19 +1558,22 @@ async def authorize(
15381558
) in calls
15391559

15401560

1541-
def test_list_controls_rejects_attachment_namespace_mismatch(
1561+
def test_list_controls_does_not_require_target_authorization_without_filter(
15421562
client: TestClient,
15431563
) -> None:
15441564
control_id, control_name = _create_control(client, name=f"Attachments-{uuid.uuid4()}")
15451565
_set_control_data(client, control_id, deepcopy(VALID_CONTROL_PAYLOAD))
15461566

1567+
calls: list[Operation] = []
1568+
15471569
class MismatchedBindingReadAuthorizer:
15481570
async def authorize(
15491571
self,
15501572
request: Any,
15511573
operation: Operation,
15521574
context: dict[str, Any] | None = None,
15531575
) -> Principal:
1576+
calls.append(operation)
15541577
namespace_key = (
15551578
"other-namespace"
15561579
if operation == Operation.CONTROL_BINDINGS_READ
@@ -1568,8 +1591,9 @@ async def authorize(
15681591
},
15691592
)
15701593

1571-
assert resp.status_code == 403
1572-
assert resp.json()["error_code"] == "AUTH_INSUFFICIENT_PRIVILEGES"
1594+
assert resp.status_code == 200, resp.text
1595+
assert resp.json()["controls"][0]["attachments"]["targets"] == []
1596+
assert Operation.CONTROL_BINDINGS_READ not in calls
15731597

15741598

15751599
def test_list_controls_rejects_attachment_filters_without_expansion(

0 commit comments

Comments
 (0)