Skip to content

.NET: [BREAKING] fix(dotnet): require allowed HTTP hosts - #9841

Merged
Vincent Biret (baywet) merged 18 commits into
mainfrom
fix/http-request-destinations
Nov 4, 2026
Merged

Vincent Biret (baywet) merged 18 commits into
mainfrom
fix/http-request-destinations

Conversation

@baywet

@baywet Vincent Biret (baywet) commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

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

  • What are the major changes? Require an explicit, copied host-and-port allowlist and an application-owned 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 an HttpClientHandler, or on .NET a SocketsHttpHandler, 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.
  • What is the impact of these changes? Breaking API and behavior: remove the parameterless, direct-client, and callback constructors and the disposal method. Migrate to new DefaultHttpRequestHandler(["api.example.com"], serviceProvider.GetRequiredService<IHttpMessageHandlerFactory>(), "WorkflowHttp"). Register a named pipeline through AddHttpClient with .ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler { AllowAutoRedirect = false, UseCookies = false, CheckCertificateRevocationList = true }) and a finite handler lifetime. IHttpClientFactory client 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 allow api.example.com:80 for ordinary HTTP or [::1]:8080 for an IPv6 destination with a port. Ports must be from 1 to 65535. Both HTTP and HTTPS are supported, so api.example.com also permits http://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.
  • What do you want reviewers to focus on? Allowlist normalization and checks before pipeline selection, per-hop transport validation, factory-owned handler lifetime, and migration from opaque clients to inspectable handler pipelines. Explicit redirect restrictions, workflow-header removal on redirects, per-request timeouts, and response handling remain covered.

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 revision fb791b540 because 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 revision e257eb39f after 100 seconds with TaskCanceledException identifying HttpClient.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.
  • Both source and unit-test project formatting commands above passed again after the timeout follow-up.

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

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: Copilot assisted with implementation, regression tests, documentation, validation, and this description.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d9d71be9-dc2c-411c-a0d7-ce04bcbda9c9
@baywet Vincent Biret (baywet) added the breaking change Usage: [PRs], Target: all PRs that introduce changes that are not backward compatible label Oct 29, 2026
Copilot AI balanced review requested due to automatic review settings October 29, 2026 15:05
@github-actions github-actions Bot changed the title fix(dotnet): validate default HTTP destinations [BREAKING] fix(dotnet): validate default HTTP destinations Oct 29, 2026
@agent-framework-automation agent-framework-automation Bot added documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs .NET Usage: [Issues, PRs], Target: .Net workflows Usage: [Issues, PRs], Target: Workflows labels Oct 29, 2026
@github-actions github-actions Bot changed the title [BREAKING] fix(dotnet): validate default HTTP destinations .NET: [BREAKING] fix(dotnet): validate default HTTP destinations Oct 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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
This was referenced Nov 9, 2026

This branch was previously deployed

2 inactive deployments
github-app-auth — 69c31996 Deployed Nov 4, 2026 by Copilot via add_label #27118
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Usage: [PRs], Target: all PRs that introduce changes that are not backward compatible documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs .NET Usage: [Issues, PRs], Target: .Net workflows Usage: [Issues, PRs], Target: Workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants