Secure IP Address Filtering - #881
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the Feature Management IP address filter to prevent bypass via untrusted forwarding headers by relying only on trusted connection metadata (the remote address as determined by the hosting stack / forwarded headers middleware).
Changes:
- Update
IPAddressFilterto evaluate feature gating using onlyHttpContext.Connection.RemoteIpAddress(noX-Forwarded-Forparsing and no “local request” bypass). - Remove forwarded-IP parsing helpers from
HttpContextExtensionsand theX-Forwarded-Forheader constant fromRequestParameters. - Refresh unit tests and documentation to reflect the trusted-proxy /
ForwardedHeadersMiddlewarerequirement.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/FeatureManagement.UnitTests/Filters/IPAddressFilterTests.cs | Updates coverage to ensure forwarded headers are ignored and only RemoteIpAddress drives evaluation. |
| tests/FeatureManagement.UnitTests/Extensions/HttpContextExtensionsTests.cs | Removes tests for deleted forwarded-IP and local-request helpers. |
| src/FeatureManagement/README.md | Documents the trusted-proxy requirement and clarifies RemoteIpAddress behavior. |
| src/FeatureManagement/Filters/IPAddressFilter.cs | Switches filter evaluation to trusted RemoteIpAddress only. |
| src/FeatureManagement/Extensions/HttpContextExtensions.cs | Removes forwarded-IP parsing and IsLocal logic now out of scope. |
| src/FeatureManagement/Constants/RequestParameters.cs | Drops the X-Forwarded-For header constant. |
| .github/copilot-instructions.md | Adds repo guidance to avoid X-Forwarded-For parsing and local bypass in IPAddressFilter. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
tests/FeatureManagement.UnitTests/Filters/IPAddressFilterTests.cs:297
- This test is meant to validate that spoofed X-Forwarded-For is ignored; using the framework constant for the header name helps ensure the test is actually setting the correct header (a typo would still pass otherwise).
m_httpContext.Request.Headers["X-Forwarded-For"] = forwardedIp.ToString();
tests/FeatureManagement.UnitTests/Filters/IPAddressFilterTests.cs:410
- Using the framework constant for the X-Forwarded-For header name avoids tests passing due to an accidental misspelling (since the filter ignores the header either way) and keeps the test aligned with ASP.NET Core conventions.
m_httpContext.Request.Headers["X-Forwarded-For"] = "10.1.1.1,203.0.113.10";
| m_context.Parameters = configuration; | ||
|
|
||
| m_httpContext.Request.Headers[RequestParameters.Header.ForwardedFor] = "127.0.0.1"; | ||
| m_httpContext.Request.Headers["X-Forwarded-For"] = IPAddress.Loopback.ToString(); |
|
Replaced by #883, which contains the same commits on a branch hosted directly in microsoft/Omex. |
Purpose
Prevent clients from bypassing IP-based feature controls through untrusted forwarding data.
Impact
IP-gated features now rely on trusted connection information. Updated guidance and regression coverage reduce the risk of reintroducing the bypass.