fix(appsec): keep first redirect hop for redirection derivatives#4972
Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 1 commit intoJul 1, 2026
Merged
Conversation
Merge WAF attribute derivatives with first-write-wins in AbsorbDerivatives so a uniform-status redirect chain reports the first hop's appsec.api.redirection.* value instead of the last. Since #4938 moved per-hop downstream request/response WAF evaluation to ephemeral subcontexts, the same attribute rule re-fires on every hop (the persistent context used to dedup it) and last-write-wins kept the final hop, regressing system-tests Test_API10_redirect_status. Adds a uniform-302 regression test (the existing TestAppsecHTTP30X used distinct 308/307 per hop so it never collided on one key), AbsorbDerivatives unit tests, and the APPSEC_RASP_NON_BLOCKING system-tests CI scenario.
github-actions
Bot
added
the
apm:ecosystem
Contributor
Config Audit |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 8c6171c | Docs | Datadog PR Page | Give us feedback! |
Contributor
Author
|
@codex review |
BenchmarksBenchmark execution time: 2026-07-01 13:42:04 Comparing candidate commit 8c6171c in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 322 metrics, 2 unstable metrics, 1 flaky benchmarks without significant changes.
|
RomainMuller
approved these changes
Jul 1, 2026
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: queued
labels
gh-worker-dd-mergequeue-cf854d
Bot
merged commit 12105c7
into
main
222 of 225 checks passed
gh-worker-dd-mergequeue-cf854d
Bot
deleted the
eliottness/fix-api10-redirect-first-hop
branch
darccio
pushed a commit
that referenced
this pull request
Jul 1, 2026
### What does this PR do? Merges WAF attribute derivatives with **first-write-wins** in `AbsorbDerivatives` so a uniform-status redirect chain reports the **first** hop's `appsec.api.redirection.*` value instead of the last. It also enables the `APPSEC_RASP_NON_BLOCKING` scenario in the system-tests CI matrix (across all weblog variants, matching `APPSEC_RASP`) so this stays covered. ### Motivation #4938 moved per-hop downstream request/response WAF evaluation from the persistent request context to per-roundtrip **ephemeral subcontexts**. The persistent context deduplicates attribute generation within a request, so a redirection attribute rule matching the same status (e.g. 302) on every hop produced its derivative only once (the first hop). Ephemeral subcontexts don't share that dedup, so the rule re-fires on every hop and `AbsorbDerivatives`' last-write-wins kept the **final** hop. This regressed system-tests `Test_API10_redirect_status::test_api10_redirect` on `golang@2.10.0-dev` (bisected to `40280d189`): `appsec.api.redirection.move_target` became `/mirror/200` (last hop) instead of `/redirect?totalRedirects=2` (first hop). The fix restores the pre-#4938 "first discovery wins" invariant at the derivative merge boundary while preserving #4938's per-hop ephemeral evaluation (RASP SSRF still fires on every hop). The existing blocked-response-schema skip is kept ahead of the new guard. ### Testing - New `TestAppsecHTTP30XRedirectChainKeepsFirstHop` (uniform-302 chain) — verified it FAILS without the fix and PASSES with it. The existing `TestAppsecHTTP30X` used distinct 308/307 per hop, so it never collided on a single key. - New `AbsorbDerivatives` unit tests (first-write-wins + blocked-response-schema skip preserved). - Full `contrib/net/http` appsec suite and `internal/appsec/emitter/waf` package pass; `go vet` clean. ### Reviewer's Checklist - [x] Changed code has unit tests for its functionality at or near 100% coverage. - [x] [System-Tests](https://github.com/DataDog/system-tests/) covering this feature have been enabled (`APPSEC_RASP_NON_BLOCKING` added to the CI matrix). - [ ] There is a benchmark for any new code, or changes to existing code. - [ ] If this interacts with the agent in a new way, a system test has been added. - [x] New code is free of linting errors. - [x] New code doesn't break existing tests. - [ ] Add an appropriate team label so this PR gets put in the right place for the release notes. - [x] All generated files are up to date. - [ ] Non-trivial go.mod changes reviewed by @DataDog/dd-trace-go-guild (no go.mod changes). Co-authored-by: eliott.bouhana <eliott.bouhana@datadoghq.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Merges WAF attribute derivatives with first-write-wins in
AbsorbDerivativesso a uniform-status redirect chain reports the first hop'sappsec.api.redirection.*value instead of the last.It also enables the
APPSEC_RASP_NON_BLOCKINGscenario in the system-tests CI matrix (across all weblog variants, matchingAPPSEC_RASP) so this stays covered.Motivation
#4938 moved per-hop downstream request/response WAF evaluation from the persistent request context to per-roundtrip ephemeral subcontexts. The persistent context deduplicates attribute generation within a request, so a redirection attribute rule matching the same status (e.g. 302) on every hop produced its derivative only once (the first hop). Ephemeral subcontexts don't share that dedup, so the rule re-fires on every hop and
AbsorbDerivatives' last-write-wins kept the final hop.This regressed system-tests
Test_API10_redirect_status::test_api10_redirectongolang@2.10.0-dev(bisected to40280d189):appsec.api.redirection.move_targetbecame/mirror/200(last hop) instead of/redirect?totalRedirects=2(first hop).The fix restores the pre-#4938 "first discovery wins" invariant at the derivative merge boundary while preserving #4938's per-hop ephemeral evaluation (RASP SSRF still fires on every hop). The existing blocked-response-schema skip is kept ahead of the new guard.
Testing
TestAppsecHTTP30XRedirectChainKeepsFirstHop(uniform-302 chain) — verified it FAILS without the fix and PASSES with it. The existingTestAppsecHTTP30Xused distinct 308/307 per hop, so it never collided on a single key.AbsorbDerivativesunit tests (first-write-wins + blocked-response-schema skip preserved).contrib/net/httpappsec suite andinternal/appsec/emitter/wafpackage pass;go vetclean.Reviewer's Checklist
APPSEC_RASP_NON_BLOCKINGadded to the CI matrix).