debugger: scrub API Security schema tags from exception replay span approvals - #7543
Merged
Merged
Conversation
…pprovals API Security samples schemas per (route, method, status) within a time window (30s by default), the same order as the test's own retry interval, so whether a given request carries `_dd.appsec.s.*` is not deterministic. Remove those keys like the `_dd.appsec.fp.*` fingerprints already are.
Contributor
|
|
![datadog-datadog-prod-us1-2[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The prefix filter removes nondeterministic API Security schema tags from both supported span formats before approval comparison, while it keeps unrelated validation unchanged.
🤖 Datadog Autotest · Commit a605ce2 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
dromanol
enabled auto-merge (squash)
August 20, 2026 09:05
dromanol
added a commit
to DataDog/dd-trace-dotnet
that referenced
this pull request
Sep 9, 2026
…the request addresses (#9082) ## Summary of changes - `server.response.status` reaches the WAF with the real status code: it is never sent during the request-phase scans, and it is sent once the real response status is known. On ASP.NET Core requests that reach the end of the pipeline without an endpoint, both `Scan(lastTime: true)` and the later response scan can carry it — the WAF doesn't report the same match twice. - On ASP.NET Core the request address set is collected once per request instead of on every run. ASP.NET Framework keeps refreshing it on each `BeginRequest`, including the second pipeline a `Server.TransferRequest` creates, which is deliberate. - Responses the WAF never saw get a report-only scan at the end of the pipeline: HTTP.sys, which never fires the instrumented response start hook, and server-generated error responses on Kestrel. ## Reason for change The status came from the request-phase address set, where `HttpContext.Response.StatusCode` is still the default `200`, 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()`, after `application.ProcessRequestAsync()` has returned, so the scope set by the diagnostic observer inside that call is no longer visible in the `AsyncLocal`: `FireOnStarting` finds 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`, both driving the Core path. Persistent addresses live for the whole WAF context, so later runs re-evaluate rules, schemas and fingerprints against the stored values. `server.request.cookies` on ASP.NET Framework is the one address the end-of-request run re-reads. ASP.NET puts `ASP.NET_SessionId` in `Request.Cookies` when the session id is read, which `SessionStateModule` normally does at `AcquireRequestState`, well before `EndRequest`; the explicit `SessionID` read in `GetEndRequestArgsForWaf` is the guard for hosts where the cookie isn't visible yet, and it has to happen before the cookies are snapshotted. Without the re-read the cookie halves of `_dd.appsec.fp.session` go 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. Coordinator-level tests for the one-shot address collection, the omission of the status during the request phase and the real status at the end of the request, on both Core and Framework. 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 the `DEBUGGER_EXCEPTION_REPLAY` span 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(...)` from `BlockingMiddleware` would work on every server and let HTTP.sys block, but it means removing a calltarget and redoing the blocking and ordering story. --------- 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
2 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.
Motivation
Test_Debugger_Exception_Replay::test_exception_replay_rockpaperscissorscompares theaspnet_core.requestspans against a stored approval, and the .NET tracer is about to start emitting API Security schemas (_dd.appsec.s.*) on error responses (dd-trace-dotnet#9082 makes the response phase reach the WAF for server-generated 500s).Those tags cannot be pinned in an approval: API Security samples schemas once per (route, method, status) per
DD_API_SECURITY_SAMPLE_DELAY(30s by default), which is the same order of magnitude as this test's own retry interval (_timeout_next = 30). Which of the three rock/paper/scissors requests carries the schemas is therefore a race — in the failing run it was therockspan, and it could just as easily be another one or none.Changes
Drop every
_dd.appsec.s.*key in_validate_spans, the same treatment the_dd.appsec.fp.*fingerprints already get. No approval file currently contains those keys, so no expected data changes.Workflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
tests/ormanifests/is modified ? I have the approval from R&P teambuild-XXX-imagelabel is present