diff --git a/dotnet/eng/MSBuild/Shared.props b/dotnet/eng/MSBuild/Shared.props index 4e126e4be6..10db0a47ef 100644 --- a/dotnet/eng/MSBuild/Shared.props +++ b/dotnet/eng/MSBuild/Shared.props @@ -38,4 +38,7 @@ + + + diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry/ClientHeaderValidation.cs b/dotnet/src/Microsoft.Agents.AI.Foundry/ClientHeaderValidation.cs index 0dfd116eb3..10a7315ceb 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry/ClientHeaderValidation.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry/ClientHeaderValidation.cs @@ -21,20 +21,20 @@ public static void Validate(string name, string value) } // Reject transport delimiters before using the name in exception text or a transport API. - if (ContainsProhibitedCharacter(name)) - { - throw new ArgumentException("Header name must not contain NUL, carriage-return, or line-feed characters.", nameof(name)); - } + HttpHeaderValidation.ValidateNoProhibitedCharacters( + name, + nameof(name), + "Header name must not contain NUL, carriage-return, or line-feed characters."); if (value.Length == 0) { throw new ArgumentException("Header value must not be empty.", nameof(value)); } - if (ContainsProhibitedCharacter(value)) - { - throw new ArgumentException("Header value must not contain NUL, carriage-return, or line-feed characters.", nameof(value)); - } + HttpHeaderValidation.ValidateNoProhibitedCharacters( + value, + nameof(value), + "Header value must not contain NUL, carriage-return, or line-feed characters."); if (!name.StartsWith(ClientHeaderPrefix, StringComparison.OrdinalIgnoreCase)) { @@ -43,7 +43,4 @@ public static void Validate(string name, string value) nameof(name)); } } - - private static bool ContainsProhibitedCharacter(string value) => - value.IndexOf('\0') >= 0 || value.IndexOf('\r') >= 0 || value.IndexOf('\n') >= 0; } diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry/FoundryAgentSessionExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Foundry/FoundryAgentSessionExtensions.cs index 3383001f48..a00aa146f8 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry/FoundryAgentSessionExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry/FoundryAgentSessionExtensions.cs @@ -108,6 +108,10 @@ internal set { _ = Throw.IfNull(session); _ = Throw.IfNullOrWhitespace(value); + HttpHeaderValidation.ValidateNoProhibitedCharacters( + value, + "userIdentity", + "User identity must not contain NUL, carriage-return, or line-feed characters."); session.StateBag.SetValue(FoundryHostedAgentUserIdentityKey, value); } } diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry/Microsoft.Agents.AI.Foundry.csproj b/dotnet/src/Microsoft.Agents.AI.Foundry/Microsoft.Agents.AI.Foundry.csproj index 5959b69eec..981245c48f 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry/Microsoft.Agents.AI.Foundry.csproj +++ b/dotnet/src/Microsoft.Agents.AI.Foundry/Microsoft.Agents.AI.Foundry.csproj @@ -6,6 +6,7 @@ related per-agent endpoint surface). Flip back to IsReleased=true once Azure.AI.Projects ships a stable 3.0.0. --> true + true true $(NoWarn);AAIP001;AAIP002;OPENAI001;SCME0001 diff --git a/dotnet/src/Microsoft.Agents.AI.Foundry/UserIdentityPolicy.cs b/dotnet/src/Microsoft.Agents.AI.Foundry/UserIdentityPolicy.cs index 4fa86673df..ca801911de 100644 --- a/dotnet/src/Microsoft.Agents.AI.Foundry/UserIdentityPolicy.cs +++ b/dotnet/src/Microsoft.Agents.AI.Foundry/UserIdentityPolicy.cs @@ -3,6 +3,7 @@ using System.ClientModel.Primitives; using System.Collections.Generic; using System.Threading.Tasks; +using Microsoft.Shared.Diagnostics; namespace Microsoft.Agents.AI.Foundry; @@ -33,6 +34,18 @@ public override ValueTask ProcessAsync(PipelineMessage message, IReadOnlyList - value.IndexOfAny(['\r', '\n', '\0']) >= 0; - private static HttpClient CreateOwnedHttpClient() { HttpClientHandler handler = new() diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Microsoft.Agents.AI.Workflows.Declarative.csproj b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Microsoft.Agents.AI.Workflows.Declarative.csproj index 27cfe95f29..e0264efc14 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Microsoft.Agents.AI.Workflows.Declarative.csproj +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Microsoft.Agents.AI.Workflows.Declarative.csproj @@ -6,6 +6,7 @@ + true true true true diff --git a/dotnet/src/Shared/Http/HttpHeaderValidation.cs b/dotnet/src/Shared/Http/HttpHeaderValidation.cs new file mode 100644 index 0000000000..c319deb5cb --- /dev/null +++ b/dotnet/src/Shared/Http/HttpHeaderValidation.cs @@ -0,0 +1,17 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; + +namespace Microsoft.Shared.Diagnostics; + +/// Validates values before they reach HTTP header transport APIs. +internal static class HttpHeaderValidation +{ + public static void ValidateNoProhibitedCharacters(string value, string parameterName, string message) + { + if (value.IndexOfAny(['\r', '\n', '\0']) >= 0) + { + throw new ArgumentException(message, parameterName); + } + } +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Foundry.UnitTests/FoundryHostedRequestTests.cs b/dotnet/tests/Microsoft.Agents.AI.Foundry.UnitTests/FoundryHostedRequestTests.cs index aee7a5000b..db955a69b1 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Foundry.UnitTests/FoundryHostedRequestTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Foundry.UnitTests/FoundryHostedRequestTests.cs @@ -101,6 +101,169 @@ await Assert.ThrowsAsync( () => agent.CreateFoundryHostedAgentSessionAsync(userIdentity: " ")); } + [Theory] + [InlineData("\0")] + [InlineData("\r")] + [InlineData("\n")] + public async Task CreateFoundryHostedAgentSessionAsync_ProhibitedUserIdentityCharacter_ThrowsAsync(string prohibitedCharacter) + { + // Arrange + FoundryAgent agent = CreateFoundryAgent(); + + // Act + ArgumentException exception = await Assert.ThrowsAsync( + () => agent.CreateFoundryHostedAgentSessionAsync(userIdentity: $"before{prohibitedCharacter}after")); + + // Assert + Assert.Equal("userIdentity", exception.ParamName); + } + + [Theory] + [InlineData("\0")] + [InlineData("\r")] + [InlineData("\n")] + [InlineData("before\0after")] + [InlineData("before\rafter")] + [InlineData("before\nafter")] + [InlineData(" \r ")] + [InlineData(" \n ")] + public void UserIdentityPolicy_Process_ProhibitedUserIdentityCharacter_Throws(string invalidIdentity) + { + // Arrange + using var handler = new RecordingHandler(MinimalResponseJson()); +#pragma warning disable CA5399 + using var http = new HttpClient(handler, disposeHandler: false); +#pragma warning restore CA5399 + ClientPipeline pipeline = ClientPipeline.Create( + new ClientPipelineOptions { Transport = new HttpClientPipelineTransport(http) }, + perCallPolicies: default, + perTryPolicies: default, + beforeTransportPolicies: default); + PipelineMessage message = CreatePipelineMessage(pipeline); + UserIdentityScope.Current = invalidIdentity; + + try + { + // Act + ArgumentException exception = Assert.Throws( + () => UserIdentityPolicy.Instance.Process( + message, + [UserIdentityPolicy.Instance, TerminalPolicy.Instance], + 0)); + + // Assert + Assert.Equal("userIdentity", exception.ParamName); + } + finally + { + UserIdentityScope.Current = null; + } + } + + [Theory] + [InlineData("\0")] + [InlineData("\r")] + [InlineData("\n")] + [InlineData("before\0after")] + [InlineData("before\rafter")] + [InlineData("before\nafter")] + [InlineData(" \r ")] + [InlineData(" \n ")] + public async Task UserIdentityPolicy_ProcessAsync_ProhibitedUserIdentityCharacter_ThrowsAsync(string invalidIdentity) + { + // Arrange + using var handler = new RecordingHandler(MinimalResponseJson()); +#pragma warning disable CA5399 + using var http = new HttpClient(handler, disposeHandler: false); +#pragma warning restore CA5399 + ClientPipeline pipeline = ClientPipeline.Create( + new ClientPipelineOptions { Transport = new HttpClientPipelineTransport(http) }, + perCallPolicies: default, + perTryPolicies: default, + beforeTransportPolicies: default); + PipelineMessage message = CreatePipelineMessage(pipeline); + UserIdentityScope.Current = invalidIdentity; + + try + { + // Act + ArgumentException exception = await Assert.ThrowsAsync( + async () => await UserIdentityPolicy.Instance.ProcessAsync( + message, + [UserIdentityPolicy.Instance, TerminalPolicy.Instance], + 0)); + + // Assert + Assert.Equal("userIdentity", exception.ParamName); + } + finally + { + UserIdentityScope.Current = null; + } + } + + [Theory] + [InlineData("\0")] + [InlineData("\r")] + [InlineData("\n")] + [InlineData("before\0after")] + [InlineData("before\rafter")] + [InlineData("before\nafter")] + [InlineData(" \r ")] + [InlineData(" \n ")] + public async Task RunAsync_RestoredProhibitedUserIdentity_DoesNotReachTransportAsync(string invalidIdentity) + { + // Arrange + using var handler = new RecordingHandler(MinimalResponseJson()); + (FoundryHostedRequestAgent agent, AgentSession session, HttpClient http) = await CreateEndToEndAgentAsync(handler); + using (http) + { + SetRawUserIdentity(session, invalidIdentity); + + // Act + ArgumentException exception = await Assert.ThrowsAsync( + () => agent.RunAsync("hi", session)); + + // Assert + Assert.Equal("userIdentity", exception.ParamName); + Assert.Empty(handler.Requests); + } + } + + [Theory] + [InlineData("\0")] + [InlineData("\r")] + [InlineData("\n")] + [InlineData("before\0after")] + [InlineData("before\rafter")] + [InlineData("before\nafter")] + [InlineData(" \r ")] + [InlineData(" \n ")] + public async Task RunStreamingAsync_RestoredProhibitedUserIdentity_DoesNotReachTransportAsync(string invalidIdentity) + { + // Arrange + using var handler = new RecordingHandler(MinimalResponseJson()); + (FoundryHostedRequestAgent agent, AgentSession session, HttpClient http) = await CreateEndToEndAgentAsync(handler); + using (http) + { + SetRawUserIdentity(session, invalidIdentity); + + // Act + ArgumentException exception = await Assert.ThrowsAsync( + async () => + { + await foreach (AgentResponseUpdate _ in agent.RunStreamingAsync("hi", session)) + { + // Drain the stream so transport validation executes. + } + }); + + // Assert + Assert.Equal("userIdentity", exception.ParamName); + Assert.Empty(handler.Requests); + } + } + [Fact] public async Task Conflict_SessionAndOptionsHostedIdsDiffer_ThrowsAsync() { @@ -382,6 +545,42 @@ private static FoundryAgent CreateFoundryAgent() => model: "gpt-4o-mini", instructions: "Test"); + private static PipelineMessage CreatePipelineMessage(ClientPipeline pipeline) + { + PipelineMessage message = pipeline.CreateMessage(); + message.Request.Method = "POST"; + message.Request.Uri = new Uri("https://example.test/"); + return message; + } + + private static async Task<(FoundryHostedRequestAgent Agent, AgentSession Session, HttpClient HttpClient)> CreateEndToEndAgentAsync( + RecordingHandler handler) + { +#pragma warning disable CA5399 + var http = new HttpClient(handler, disposeHandler: false); +#pragma warning restore CA5399 + var openAIClient = new OpenAIClient( + new ApiKeyCredential("fake"), + new OpenAIClientOptions { Transport = new HttpClientPipelineTransport(http) }); + IChatClient chatClient = openAIClient.GetResponsesClient().AsIChatClient(); + +#pragma warning disable MEAI001 + OpenAIRequestPolicies policies = chatClient.GetService()!; + OpenAIRequestPoliciesReflection.AddPolicyIfMissing(policies, UserIdentityPolicy.Instance); +#pragma warning restore MEAI001 + + var chatAgent = new ChatClientAgent(chatClient); + AgentSession session = await chatAgent.CreateSessionAsync(); + return (new FoundryHostedRequestAgent(chatAgent), session, http); + } + + private static void SetRawUserIdentity(AgentSession session, string userIdentity) + { + // Session deserialization writes state directly, so the final transport boundary must + // reject invalid restored values even when the public session factory was not used. + session.StateBag.SetValue("Microsoft.Agents.AI.Foundry.UserIdentity", userIdentity); + } + private static string MinimalResponseJson() => """ { "id":"resp_1","object":"response","created_at":1700000000,"status":"completed", @@ -391,6 +590,18 @@ private static string MinimalResponseJson() => """ private sealed class TestSession : AgentSession; + private sealed class TerminalPolicy : PipelinePolicy + { + public static TerminalPolicy Instance { get; } = new(); + + public override void Process(PipelineMessage message, IReadOnlyList pipeline, int currentIndex) + { + } + + public override ValueTask ProcessAsync(PipelineMessage message, IReadOnlyList pipeline, int currentIndex) => + default; + } + private sealed class ProbeAgent : AIAgent { private readonly Action? _onRun;