[AppSec] Send the real response status to the WAF and scan each request once - #9082
Open
dromanol wants to merge 2 commits into
Open
[AppSec] Send the real response status to the WAF and scan each request once#9082dromanol wants to merge 2 commits into
dromanol wants to merge 2 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98f2a72c31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9082) and master. ✅ No regressions detected |
BenchmarksBenchmark execution time: 2026-08-20 15:11:23 Comparing candidate commit ebd367b in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 70 metrics, 0 unstable metrics, 64 known flaky benchmarks, 62 flaky benchmarks without significant changes.
|
5 tasks
dromanol
force-pushed
the
dani/aap/waf-response-status-repro
branch
from
August 20, 2026 14:22
29b1e45 to
ebd367b
Compare
…st once server.response.status came from the request-phase address set, where HttpContext.Response.StatusCode is still the default 200. It is now sent only on the last run of the request, together with the response headers. The request addresses are supplied once: the WAF keeps them for the life of its context, so a later run re-evaluates rules and processors against the stored values instead of paying for the whole set again. ASP.NET's session cookie is the exception, since it only lands in Request.Cookies once the session id is read, so the end of request run re-reads the cookies. Servers without the instrumented response start hook (HTTP.sys) never saw a real status at all; they now get a report-only scan at the end of the pipeline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A Server.TransferRequest pipeline shares the trace context, so the one-shot flag swallowed the transferred request's addresses. EndRequest starts from an empty set instead. Also pins that a re-supplied status can't report twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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
1 participant
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.
Summary of changes
server.response.statusreaches the WAF with the real status code, on the last run of the request only.Reason for change
The status came from the request-phase address set, where
HttpContext.Response.StatusCodeis still the default200, so status rules were fed a fabricated value — and on HTTP.sys that was the only status the WAF ever saw. Resending every request address on the later runs also re-evaluated the whole request rule set for no new information.The same gap existed on Kestrel for responses the server writes itself. On an unhandled exception the 500 is produced by
ProduceEnd(), afterapplication.ProcessRequestAsync()has returned, so the scope set by the diagnostic observer inside that call is no longer visible in theAsyncLocal:FireOnStartingfinds no active span and skips the WAF entirely. Those requests reached the WAF without a response status or response headers at all.Implementation details
Two one-shot flags on
AppSecRequestContext. Persistent addresses live for the whole WAF context, so later runs re-evaluate rules, schemas and fingerprints against the stored values.The one exception is
server.request.cookieson ASP.NET Framework: ASP.NET insertsASP.NET_SessionIdintoRequest.Cookieswhen the session id is read, which the end-of-request run does itself, so a cookie-less request has none atOnBeginRequestand one atOnEndRequest. Without the re-read the cookie halves of_dd.appsec.fp.sessiongo empty.Test coverage
New unit tests against the real WAF: status rule matching per phase, schemas surviving a run without resupplied addresses, session fingerprint vs late cookies, server detection. Green locally: Security unit tests net8.0/net48, Security integration AspNetCore net8.0 + AspNetMvc5 net48, and system-tests
APPSEC_BLOCKING,APPSEC_API_SECURITY,APPSEC_WAF_TELEMETRY,APPSEC_RASP,APPSEC_AUTO_EVENTS_EXTENDED,DEFAULT.Because error responses now reach the last WAF run, API Security samples them too and error spans can carry
_dd.appsec.s.*schemas. That breaks theDEBUGGER_EXCEPTION_REPLAYspan approvals, where the schemas land on a different request every run (the sampler's 30s window against the test's 30s retry interval): DataDog/system-tests#7543 scrubs them like the_dd.appsec.fp.*fingerprints already are.Other details
Replaces #8856 (rewritten from scratch, not rebased). Left out on purpose: registering
Response.OnStarting(...)fromBlockingMiddlewarewould work on every server and let HTTP.sys block, but it means removing a calltarget and redoing the blocking and ordering story.