.NET: Scope OpenAI Responses conversation access to the owning agent#6892
Conversation
The in-memory Responses hosting service resolved conversation history by a client-supplied conversation identifier and read/appended to it without verifying the conversation belonged to the agent scope serving the request. Because each agent has its own responses endpoint but shares the conversation store, a request could reference a conversation owned by a different agent. Reject a conversation identifier whose stored owner (metadata "agent_id") does not match the endpoint's agent scope, before any read or append. Unknown or unowned conversations keep their existing behavior. Add unit tests covering cross-agent, same-agent, unowned, and no-scope cases.
There was a problem hiding this comment.
Pull request overview
This PR tightens the in-memory OpenAI Responses hosting implementation so that a client-supplied conversation ID can only be used on the agent-scoped endpoint that owns it (based on metadata["agent_id"]), preventing silent cross-agent conversation reuse.
Changes:
- Added an optional
ownerAgentIdscope toInMemoryResponsesServiceand enforced an ownership check duringValidateRequestAsync. - Wired per-agent
MapOpenAIResponsesto pass the trusted route agent name as the scope. - Added unit tests covering cross-agent mismatch, same-agent, unowned, and no-scope scenarios.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/InMemoryResponsesService.cs | Adds owner scoping and a validation-time check rejecting conversations owned by a different agent. |
| dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/EndpointRouteBuilderExtensions.Responses.cs | Passes agent.Name as the trusted owner scope when mapping per-agent Responses endpoints. |
| dotnet/tests/Microsoft.Agents.AI.Hosting.OpenAI.UnitTests/InMemoryResponsesServiceTests.cs | Introduces tests for the new conversation ownership validation behavior. |
| if (conversation is not null | ||
| && conversation.Metadata.TryGetValue(AgentIdMetadataKey, out var ownerAgentId) | ||
| && !string.IsNullOrEmpty(ownerAgentId) | ||
| && !string.Equals(ownerAgentId, this._ownerAgentId, StringComparison.Ordinal)) | ||
| { |
| // Assert | ||
| Assert.NotNull(error); |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 90%
✓ Correctness
The PR correctly implements conversation access scoping for the InMemoryResponsesService. The logic in ValidateConversationScopeAsync is sound: it properly null-checks the owner scope, conversation storage, conversation ID, and the fetched conversation before performing the ordinal comparison. The Conversation.Metadata property is initialized with an empty dictionary (never null), making the TryGetValue call safe. The constructor chaining preserves backward compatibility. The tests adequately cover the key scenarios (cross-agent rejection, same-agent pass, unowned pass, no-scope pass). No correctness issues found.
✓ Security Reliability
This PR adds proper conversation access scoping to prevent cross-agent conversation access. The implementation is sound: validation correctly rejects conversations owned by other agents, the PreviousResponseId path is implicitly scoped by the per-instance response cache, null checks are consistent between validation and execution paths, and error messages avoid leaking sensitive details. No significant security or reliability issues found.
✓ Test Coverage
The test suite covers the four primary validation paths (cross-agent rejection, same-agent pass, unowned pass, no-scope pass) but omits a test for the explicitly-documented case-sensitive comparison behavior and a test for nonexistent conversation IDs. Assertions in the cross-agent test only check non-null without verifying the error code/message.
✓ Failure Modes
The PR correctly implements conversation ownership scoping. The validation logic is properly positioned in the request flow, handles null/empty cases appropriately, and the fail-open behavior for unowned conversations is intentional and well-documented. No concrete silent-failure paths or operational failure modes were identified in the changed code.
✗ Design Approach
I found one blocking design issue: the new ownership check relies on
conversation.Metadata["agent_id"], but that field remains fully client-mutable through the public conversation update route, so the new scoping can be bypassed by reassigning a conversation's owner before calling another agent's responses endpoint.
Flagged Issues
- The ownership check trusts
conversation.Metadata["agent_id"], butConversationsHttpHandler.UpdateConversationAsyncreplacesMetadatafrom the request (dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Conversations/ConversationsHttpHandler.cs:159-164). A client can rewrite a conversation's owner from agent B to agent A and then satisfy the new validation inInMemoryResponsesService, defeating the stated goal of scoping conversation access to the owning agent.
Automated review by rogerbarreto's agents
| // keyed-service keys, so case-only-different names are distinct agents. Compare ordinally | ||
| // to stay consistent and avoid treating a different agent as the same owner. | ||
| if (conversation is not null | ||
| && conversation.Metadata.TryGetValue(AgentIdMetadataKey, out var ownerAgentId) |
There was a problem hiding this comment.
This trusts conversation.Metadata["agent_id"] as the authoritative owner, but that value is client-mutable: ConversationsHttpHandler.UpdateConversationAsync overwrites Metadata = request.Metadata. A caller can update a conversation created for agent B so its metadata says agent A, then this check passes and /{agentA}/v1/responses will read/append that history. Ownership needs to come from server-controlled state or agent_id updates must be rejected.
|
Flagged issue The ownership check trusts Source: automated DevFlow PR review |
Motivation & Context
The in-memory OpenAI Responses hosting service resolved conversation history from a
client-supplied conversation identifier and read/appended to that conversation without
checking that it belongs to the agent scope serving the request. Each agent is mapped to its
own responses endpoint (
/{agentName}/v1/responses) while the conversation store is a sharedsingleton, so a request sent to one agent''s endpoint could reference a conversation owned by
a different agent and be served silently. This tightens conversation access to the owning
agent scope as a defense-in-depth / consistency improvement.
Description & Review Guide
What are the major changes?
InMemoryResponsesServicenow accepts an optional owning-agent scope via a new,backward-compatible constructor overload (existing constructors and DI registration pass
nulland are unaffected).ValidateRequestAsyncperforms a fail-closed ownership check before any conversationread/append: a supplied conversation whose stored owner (
metadata["agent_id"]) does notmatch the endpoint''s agent scope is rejected. Unknown or unowned conversations keep their
existing behavior.
MapOpenAIResponsesoverload wires the trusted routeagent.Nameas theowner scope (the client-supplied
agentfield is not used for scoping).InMemoryResponsesServiceTestscovering cross-agent, same-agent, unowned, andno-scope cases.
What is the impact of these changes?
validation path) instead of being served. No public API breaks; the new constructor
parameter is optional.
elsewhere in this subsystem (the per-agent conversation index uses ordinal
MemoryCachekeys and agents resolve via ordinal keyed-service keys), so case-only-different agent names
remain distinct owners.
What do you want reviewers to focus on?
to the existing execution path as done here.
Related Issue
No public issue is linked. This is a self-contained defense-in-depth / access-scoping
consistency change; it is tracked outside the public tracker. Author to attach an internal
tracking reference if applicable.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.