Repository navigation
.NET: [BREAKING] fix(dotnet): require allowed HTTP hosts - #9841
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9d71be9-dc2c-411c-a0d7-ce04bcbda9c9
There was a problem hiding this comment.
🟡 Changes recommended
The added tests do not verify single DNS resolution, connection to validated addresses, or rejection of mixed public/private DNS answers in the production callback, leaving these protections vulnerable to undetected regressions.
1 open issue
What changed in this PR
This PR hardens declarative HTTP actions by restricting the internally owned client to public destinations while preserving caller-supplied clients for explicitly configured access.
Changes:
- Require HTTP(S) URLs and validate resolved addresses at connection time, including after redirects.
- Disable system proxies for the internal client and require supplied clients on older target frameworks.
- Add destination-policy regression tests and update consumer guidance.
| File | Description |
|---|---|
skills/agent-framework-builder-dotnet/references/declarative.md |
Documents destination restrictions, proxy behavior, and framework requirements. |
dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.UnitTests/DefaultHttpRequestHandlerTests.cs |
Adds destination and scheme validation tests; adapts cookie tests to the restricted transport. |
dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/DefaultHttpRequestHandler.cs |
Adds public-address validation and connects directly to validated addresses. |
Review effort: Balanced
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (1 commit(s)): 8bf3e789adaf
Model: gpt-5.6-sol
Overview
The PR moves the internally owned transport to a direct SocketsHttpHandler connection path, validates every resolved address at connection time, and revalidates redirect destinations while preserving explicitly supplied clients. The regression suite covers literal and DNS-resolved non-public destinations, redirect and header behavior, framework-specific fallback requirements, timeout propagation, and owned-client disposal. Those guards and tests prevented a supported Critical, High, or Medium issue introduced by this change.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9d71be9-dc2c-411c-a0d7-ce04bcbda9c9
There was a problem hiding this comment.
🔵 Needs a closer look
The custom IP classifier and DNS/socket handling establish a new SSRF security boundary requiring security-owner review and cross-platform validation beyond mocked connection tests.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9d71be9-dc2c-411c-a0d7-ce04bcbda9c9

Motivation & Context
Make declarative HTTP destinations an explicit application decision. The host knows which services a workflow may contact; public/private address classification cannot express that policy across different DNS, proxy, and routing configurations.
Description & Review Guide
IHttpMessageHandlerFactory. Check canonical HTTP(S) destinations before factory invocation and on every redirect. Reject workflow-supplied Host headers. Inspect each factory-provided pipeline before sending: require anHttpClientHandler, or on .NET aSocketsHttpHandler, with automatic redirects and cookies disabled. Reject unsupported primary handlers and incomplete pipelines rather than silently changing application settings. Create and dispose a client per hop without disposing pooled handlers. Update tests, sample and integration callers, all five API baselines, compatibility records, proposed ADR 0049, and consumer guidance.new DefaultHttpRequestHandler(["api.example.com"], serviceProvider.GetRequiredService<IHttpMessageHandlerFactory>(), "WorkflowHttp"). Register a named pipeline throughAddHttpClientwith.ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler { AllowAutoRedirect = false, UseCookies = false, CheckCertificateRevocationList = true })and a finite handler lifetime.IHttpClientFactoryclient settings (default headers, base address, timeout) do not apply; configure headers and origin-bound authentication in delegating handlers. These application-owned handlers must preserve the validated destination and must not follow redirects themselves. Empty allowlists reject all requests. Host matching is exact, case-insensitive, and normalizes IDN and trailing DNS dots; wildcards and implicit subdomains are unsupported. Omitted ports default to 443; explicitly allowapi.example.com:80for ordinary HTTP or[::1]:8080for an IPv6 destination with a port. Ports must be from 1 to 65535. Both HTTP and HTTPS are supported, soapi.example.comalso permitshttp://api.example.com:443; applications requiring HTTPS must enforce that additional policy. DNS, proxies, and routing remain transport responsibilities; intentionally allowed internal services remain supported.Transport-validation and type-pattern validation (before the timeout follow-up), from the repository root:
dotnet run --project dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore -- --filter-method '*SendAsyncRejectsAutomaticRedirectsBeforeTransportAsync': failed against unchanged PR revisionfb791b540because the default factory sent the request without throwing; passed after transport validation. Before implementation edits, the diff contained only this regression test.dotnet run --project dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore -- --filter-class '*DefaultHttpRequestHandlerTests': passed, 105 tests.dotnet run --project dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net472 --no-restore -- --filter-class '*DefaultHttpRequestHandlerTests': passed, 102 tests. Both handler-suite commands and the source formatting command below passed again after switching transport checks to type patterns.dotnet run --project dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore: passed, 1,322 tests.dotnet run --project dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net472 --no-restore: passed, 1,319 tests.dotnet build dotnet\src\Microsoft.Agents.AI.Workflows.Declarative -c Release --no-restore --tl:off: passed across all five targets, including package compatibility validation; no warnings or errors.dotnet build dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests --no-restore --tl:off: passed on both targets; no warnings or errors.dotnet build dotnet\samples\03-workflows\Declarative\InvokeHttpRequest --no-restore --tl:off: passed; no warnings or errors.dotnet format dotnet\src\Microsoft.Agents.AI.Workflows.Declarative --no-restore: passed.dotnet format dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests --no-restore: passed.dotnet format dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests --no-restore --include dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.IntegrationTests\InvokeToolWorkflowTest.cs: passed after restoring missing references.dotnet format dotnet\samples\03-workflows\Declarative\InvokeHttpRequest --no-restore --include dotnet\samples\03-workflows\Declarative\InvokeHttpRequest\Program.cs: passed after restoring missing references.dotnet build dotnet\agent-framework-dotnet.slnx --no-restore --tl:off -m:2: blocked by unrestored unrelated projects (NETSDK1004/MSB3644). Repository-wide unit tests remain unverified locally. External-service integration tests and sample execution were not run.The timeout follow-up disables the client's independent timeout only when a positive workflow timeout is supplied, so the existing linked cancellation token governs the whole operation. Without a positive workflow timeout, the default client timeout remains unchanged.
dotnet run --project dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore -- --filter-method '*SendAsyncHonorsTimeoutLongerThanDefaultClientTimeoutAsync': failed against unchanged revisione257eb39fafter 100 seconds withTaskCanceledExceptionidentifyingHttpClient.Timeout; passed after the fix, returning headers after 101 seconds. Before implementation edits, the diff contained only the new regression test.dotnet run --project dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --no-restore -- --filter-method '*SendAsyncTimeout*': passed, 3 cancellation tests.dotnet run --project dotnet\tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net472 --no-restore -- --filter-method '*Timeout*': passed, 8 tests, including the long-deadline regression.Related Issue
N/A - maintainer-directed bugfix; no public issue is linked. The follow-up updates this existing pull request rather than opening a competing pull request.
AI Assistance
AI assistance details: Copilot assisted with implementation, regression tests, documentation, validation, and this description.
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.