sanitise the invocation id path param with a safe fallback#47485
Open
alhudz wants to merge 1 commit into
Open
Conversation
alhudz
requested review from
RaviPidaparthi,
ankitbko and
vangarp
as code owners
github-actions
Bot
added
Community Contribution
Contributor
|
Thank you for your contribution @alhudz! We will review the pull request and get back to you soon. |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Prevents malformed invocation_id path parameters from being reflected into the x-agent-invocation-id response header by sanitizing to a generated fallback, and adds regression tests plus a version/changelog bump.
Changes:
- Sanitize
invocation_idfrom path params using a generated UUID fallback in traced invocation endpoints. - Add async tests asserting malformed/overlong IDs are not echoed into response headers.
- Bump package version to
1.0.0b6and document the fix inCHANGELOG.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| sdk/agentserver/azure-ai-agentserver-invocations/azure/ai/agentserver/invocations/_invocation.py | Switches path invocation_id sanitization fallback to a generated UUID to prevent reflection of malformed IDs. |
| sdk/agentserver/azure-ai-agentserver-invocations/tests/test_invocation_id_sanitization.py | Adds regression coverage for invocation id sanitization in response headers. |
| sdk/agentserver/azure-ai-agentserver-invocations/azure/ai/agentserver/invocations/_version.py | Bumps the prerelease version to 1.0.0b6. |
| sdk/agentserver/azure-ai-agentserver-invocations/CHANGELOG.md | Adds an Unreleased 1.0.0b6 entry describing the fix. |
Comment on lines
+39
to
+40
| @pytest.mark.asyncio | ||
| async def test_invalid_char_id_not_reflected_in_header(): |
Comment on lines
+34
to
+36
| async with AsyncClient(transport=transport, base_url="http://testserver") as client: | ||
| resp = await client.get(f"/invocations/{path_id}") | ||
| return resp.headers[_HEADER] |
|
|
||
| from azure.ai.agentserver.invocations import InvocationAgentServerHost | ||
| from azure.ai.agentserver.invocations._constants import InvocationConstants | ||
| from azure.ai.agentserver.invocations._invocation import _MAX_ID_LENGTH, _VALID_ID_RE |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
GET /invocations/{id}and thePOST /invocations/{id}/cancelvariant both run through_traced_invocation_endpoint, which is meant to validate the id with_sanitize_idbefore it reaches thex-agent-invocation-idresponse header and the request-scoped log and span fields.Repro: request
GET /invocations/bad~id(a character outside_VALID_ID_RE), or an id longer than_MAX_ID_LENGTH; the raw value comes straight back in thex-agent-invocation-idresponse header.Cause: the call is
_sanitize_id(raw_invocation_id, raw_invocation_id), so the rejected value is its own fallback and the length and character checks can never drop anything._create_invocation_endpointpasses a generated UUID instead.Fix: fall back to a fresh UUID, matching the create path, so a malformed id never reaches the header or the logs. Added a regression test for the invalid-character and over-length cases plus a valid-id passthrough.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines