feat(appsec): add Forwarded as a source for client IP#3911
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the Forwarded header as a source for client IP detection in the AppSec module. The Forwarded header uses a different format than other IP headers, requiring specialized parsing logic to extract IP addresses from semicolon-delimited directives.
- Replaces
x-forwardedwithforwardedin the default IP headers list - Implements parsing logic for the RFC 7239
Forwardedheader format - Adds comprehensive test coverage for various
Forwardedheader scenarios
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/appsec/listener/httpsec/request.go | Updates the default IP headers list to include forwarded instead of x-forwarded |
| internal/appsec/listener/httpsec/clientip.go | Implements parseForwardedHeader function with specialized parsing logic for the Forwarded header format |
| internal/appsec/listener/httpsec/clientip_test.go | Adds comprehensive test cases for Forwarded header parsing and excludes it from generic IP header tests |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
7b73009 to
49d57f1
Compare
BenchmarksBenchmark execution time: 2025-08-25 13:44:43 Comparing candidate commit 75e125e in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 24 metrics, 0 unstable metrics. |
The `Forwarded` header is used by API Gateway to inform handlers about the actual forwarding route. This header has a somewhat different format compared to other IP headers (it's a semicolon-delimited sequence of directives, not a comma-separated list of IP addresses) that requires its own parsing logic.
49d57f1 to
6af4f59
Compare
eliottness
left a comment
eliottness
left a comment
There was a problem hiding this comment.
Looks good except minor tweaks and maybe what looks like an overly complex implementation compared to this one for example: https://github.com/slashid/httpforwarded/blob/0bf9cfcea9fa/parse.go#L70
|
NB -- This is pending a ST update that enacts removal of the |
Following implementation in DataDog/dd-trace-go#3911
What does this PR do?
Adds the
Forwardedheader as a source of information for the Client IP detection.Motivation
The
Forwardedheader is used by API Gateway to inform handlers about the actual forwarding route. This header has a somewhat different format compared to other IP headers (it's a semicolon-delimited sequence of directives, not a comma-separated list of IP addresses) that requires its own parsing logic.Reviewer's Checklist
./scripts/lint.shlocally.Unsure? Have a question? Request a review!