[Vincent Biret (baywet)]

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.

[Copilot]

🟡 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

• Add seam tests for single DNS resolution and private-address rejection · New

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]

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.

[Copilot]

🔵 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

• Add seam tests for single DNS resolution and private-address rejection

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The host-allowlist security boundary, delegated DNS and transport behavior, and breaking constructor migration require maintainer/security sign-off on the proposed trust model.

No open issues 🎉

_{Review effort: Balanced}

[github-actions]

MAF Automated Review — Iteration 3

Result: Findings reported
Scope: 5 net-new commit(s): 37ee7c4280eb, fb9786677be3, b1df6dfffb6e, d9ff0f305417, 090c01a238ea
Model: gpt-5.6-sol

Overview

The PR adds deny-by-default exact-host authorization, canonicalization, pre-provider checks, and redirect coverage for the internally owned transport, backed by focused regression tests. Those guards are effective for handler-managed requests, but supplied HttpClient instances remain able to follow unobserved redirects or override authority through default headers. Host-only authorization also leaves authenticated supplied clients exposed to workflow-selected schemes and ports.

Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
3 verified findings remained after source verification (1 high, 2 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/DefaultHttpRequestHandler.cs

[Copilot]

🟡 Changes recommended

The PR description and migration guidance inaccurately describe host-only authorization, while the implementation requires exact host-and-port matches and defaults omitted ports to 443.

1 open issue

• Clarify that host authorization requires an exact port match · New

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Maintainer/security review is needed to confirm the breaking authorization contract’s trust boundaries, especially scheme-independent host/port permissions and caller-enforced redirect policy for supplied transports.

No open issues 🎉

1 resolved since last review

• Clarify that host authorization requires an exact port match

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

The new client’s default 100-second timeout overrides longer workflow deadlines, so positive per-request timeouts need to disable the client timeout and rely on the linked cancellation token.

1 open issue

• Default HttpClient timeout overrides longer per-request deadlines · New

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Maintainer sign-off on ADR 0049 is needed to accept the breaking security model’s scheme-independent host/port authorization and trust in DNS, proxies, and application-owned handler pipelines across runtimes.

No open issues 🎉

1 resolved since last review

• Default HttpClient timeout overrides longer per-request deadlines

_{Review effort: Balanced}