fix(dotnet): close SSRF gaps in WebhookApprover endpoint validation

[chopmob-cloud]

Summary

Addresses microsoft#3369, filed against this branch during review: WebhookApprover.ValidateEndpoint is advertised as an SSRF control but its blocklist is partial.

Problem

ValidateEndpoint only checked a small named blocklist (cloud metadata hosts) and IPv4 link-local addresses. It did not block:

• loopback (127.0.0.0/8, ::1)
• RFC 1918 private ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16)
• IPv6 unique-local (fc00::/7)
• 0.0.0.0

It also only ran against literal IP endpoints. A hostname endpoint bypassed validation entirely, since DNS resolution happened later, inside HttpClient.SendAsync, outside this check.

Fix

• Extends the address check (IsBlockedAddress) to cover loopback, the missing private ranges, IPv6 unique-local, and 0.0.0.0, alongside the existing link-local and named-host checks.
• Unwraps IPv4-mapped IPv6 addresses (::ffff:a.b.c.d) before checking, so a mapped address cannot smuggle a blocked IPv4 address past the IPv6 branch.
• Adds a SocketsHttpHandler.ConnectCallback that resolves the host and validates the resolved address at actual connect time, used when this class creates its own HttpClient (the common path, httpClient argument omitted). This closes the DNS rebinding gap a constructor-time-only check leaves open: there is no second, independent resolution between "the address that was checked" and "the address that was connected to", because they are the same operation. Verified empirically that this also covers redirects: ConnectCallback fires once per connection the handler opens, including a connection needed to follow a redirect to a different host.
• If a caller supplies their own HttpClient, this class does not control its transport and cannot enforce the connect-time guard; documented on the class per the issue's acceptance criterion 4, rather than silently overclaiming coverage.

Scope note

This repository's AgentGovernance assembly is strong-name signed, and InternalsVisibleTo is not set up for the test project (existing doc comments elsewhere in the codebase discuss why that is friction under strong-naming). Following the existing convention, the new validation logic is private and exercised entirely through the public WebhookApprover constructor in the added tests, rather than unit-tested directly.

Tests

WebhookApproverSecurityTests.cs, 17 cases through the public constructor:

• 12 cases confirming a blocked literal endpoint throws (the 9 new categories plus the 3 pre-existing ones, as a regression check).
• 4 cases confirming addresses just outside each blocked range are still allowed.
• 1 case confirming a hostname endpoint is not rejected outright (correct: it cannot be range-checked before DNS resolution; the connect-time guard is the real enforcement point for hostnames).

Validation

Isolated .NET 8 SDK build, run twice independently (SHA-256-verified against a fresh fetch of this branch both times):

• Pre-fix: 9 of the 17 new tests fail, exactly the newly-covered categories (loopback, private ranges, unique-local, 0.0.0.0); the 8 already-covered cases still pass. Confirms the tests reproduce a real gap, not a false positive.
• Post-fix: 17/17 pass.
• Full existing suite: 823/823 pass, no regressions.
• A standalone probe against real HttpListeners confirmed ConnectCallback fires per connection, including on a redirect to a different host:port, so the connect-time guard is not limited to the initial request.

Happy to adjust the fix shape, split it, or fold it directly into microsoft#3363 if you'd rather carry it there instead of as a follow-up PR.

[github-actions]

Welcome to the Agent Governance Toolkit! Thanks for your first pull request.
Please ensure tests pass, code follows style (ruff check), and you have signed the CLA.
See our Contributing Guide.

[github-actions]

🤖 AI Agent: contributor-guide — 🌟 What You Did Well

Hi @carloshvp! 👋 Welcome to the Agent Governance Toolkit community, and thank you for your first contribution! 🎉 We're thrilled to have you here, and you've tackled an important issue with a thoughtful and comprehensive approach. Let's dive into your pull request together!

---

🌟 What You Did Well

1. Thorough Problem Analysis: Your PR description is incredibly detailed, clearly outlining the SSRF vulnerabilities, the gaps in the existing implementation, and the rationale behind your proposed solution. This level of clarity makes it easy for reviewers to understand the context and importance of your changes.

2. Comprehensive Fix: You've addressed multiple SSRF attack vectors (e.g., loopback, private ranges, DNS rebinding) and implemented a robust ConnectCallback mechanism to validate resolved addresses at connect time. This is a well-thought-out and security-conscious approach.

3. Testing Excellence: The new test suite in WebhookApproverSecurityTests.cs is thorough, covering both blocked and allowed cases, as well as edge cases like hostnames. Your validation steps (pre- and post-fix) demonstrate a strong commitment to quality.

4. Documentation: The inline comments and XML documentation in your code are detailed and helpful, explaining not just what the code does but why certain decisions were made. This is a great example of maintainable code!

---

🔍 Suggestions for Improvement

While your contribution is excellent, there are a few areas where we can align it more closely with the project's conventions and best practices:

1. Linting with Ruff:

   • This project uses Ruff for linting, specifically targeting E, F, and W error codes. While your code is in C# and not directly affected by Ruff, it's worth ensuring that any Python-related changes (e.g., tests or scripts) adhere to this standard. If you haven't already, you can check the CONTRIBUTING.md for more details.

2. Test Placement:

   • Tests for this project are typically organized under packages/{name}/tests/. Since your tests are in the .NET folder, this convention doesn't directly apply, but it's worth noting for any future contributions involving Python code.

3. Commit Style:

   • We follow the Conventional Commits standard. Your commit message (fix(dotnet): close SSRF gaps in WebhookApprover endpoint validation) is already well-aligned with this convention. Great job! 🎉

4. Security-Sensitive Code:

   • Since this PR addresses a security-sensitive area (SSRF mitigation), it will undergo extra scrutiny during the review process. You've already done a fantastic job documenting the scope and limitations of your changes (e.g., the note about user-supplied HttpClient), which will help reviewers assess the security implications.

---

📚 Helpful Resources

Here are some resources to help you navigate the contribution process:

• CONTRIBUTING.md: Guidelines for contributing to this project.
• QUICKSTART.md: A quick guide to getting started with the project.

---

🚀 Next Steps

1. Review Feedback: The maintainers will review your PR and may provide additional feedback or request changes. Keep an eye on notifications for updates.

2. Address Comments: If changes are requested, don't hesitate to ask questions or seek clarification. We're here to help!

3. Approval and Merge: Once the PR is approved, a maintainer will merge it into the main branch. 🎉

---

Thank you again for your contribution, @carloshvp! Your work is a fantastic example of how thoughtful, well-documented code can make a big impact. We're excited to have you as part of the community and look forward to collaborating with you on this project. 😊

[chopmob-cloud]

Rebased onto the current dotnet-approval-chain-parity tip to clear the merge conflict from the base rewrite. Our redundant parity commit was dropped (base already carries parity), so the branch is now a single commit and the diff against base is exactly the WebhookApprover hardening plus its tests. Base's own additions (approval lifecycle hardening, redirect refusal, stricter approve-response identity check) are preserved, not reverted.

The validator is now a resolve-once, connect-time guard: when WebhookApprover owns its HttpClient it resolves the host and connects the socket directly to the validated address via SocketsHttpHandler.ConnectCallback, closing the DNS-rebinding gap a constructor-only check leaves open. AllowAutoRedirect stays false, so a remote endpoint cannot bounce the request onto another target.

The blocked-address set was expanded to cover the ranges the earlier check missed: on IPv4, loopback, 0.0.0.0, RFC 1918, 169.254/16 (incl. 169.254.169.254 metadata), 100.64.0.0/10 CGNAT (incl. Alibaba 100.100.100.200), 198.18.0.0/15, 224.0.0.0/4 multicast, and 240.0.0.0/4 reserved (incl. 255.255.255.255); on IPv6, link-local, fc00::/7 unique-local, fec0::/10 site-local, and the 64:ff9b::/96 NAT64 prefix, with IPv4-mapped and IPv4-compatible addresses unwrapped before the check. Checks use AddressFamily branches and byte comparisons, no string parsing. New xUnit cases assert each newly blocked range plus boundary and public addresses. One base test that pointed the owned client at a loopback server was converted to a network-free handler assertion, since the guard now rejects loopback targets by design. Full suite is green (843 tests).