[IAST] Add Class field to vulnerability Location and repurpose Path to source file - #8930
Merged
Conversation
Contributor
Snapshots difference summaryThe following differences have been observed in committed snapshots. It is meant to help the reviewer. 215 occurrences of : - "path": "Samples.Security.AspNetCore5.Controllers.IastController",
+ "class": "Samples.Security.AspNetCore5.Controllers.IastController",
1 occurrences of : - "path": "Samples.Security.AspNetCore5.Startup",
+ "class": "Samples.Security.AspNetCore5.Startup",
1 occurrences of : - "path": "Samples.Security.AspNetCore5.Helpers.EmailHelper",
+ "class": "Samples.Security.AspNetCore5.Helpers.EmailHelper",
2 occurrences of : - "path": "Samples.Security.AspNetCore5.Controllers.IastController+"
+ "class": "Samples.Security.AspNetCore5.Controllers.IastController+"
1 occurrences of : - "path": "Samples.Security.AspNetCore5.Controllers.IastController+<>c__DisplayClass28_0",
+ "class": "Samples.Security.AspNetCore5.Controllers.IastController+<>c__DisplayClass28_0",
2 occurrences of : - "path": "Samples.Security.Data.NHibernateHelper",
+ "class": "Samples.Security.Data.NHibernateHelper",
9 occurrences of : - "path": "AspNetCore.Views_Iast_Xss+<<ExecuteAsync>b__8_1>d",
+ "class": "AspNetCore.Views_Iast_Xss+<<ExecuteAsync>b__8_1>d",
1 occurrences of : - "path": "ASP._Page_Views_Iast_ReflectedXss_cshtml",
+ "path": "ReflectedXss.cshtml",
+ "class": "ASP._Page_Views_Iast_ReflectedXss_cshtml",
2 occurrences of : - "path": "Samples.Security.AspNetCore2.Pages.DataRazorIastPageModel",
+ "class": "Samples.Security.AspNetCore2.Pages.DataRazorIastPageModel",
2 occurrences of : - "path": "Samples.Security.AspNetCore5.DataRazorIastPageModel",
+ "class": "Samples.Security.AspNetCore5.DataRazorIastPageModel",
1 occurrences of : - "path": "Samples.Security.AspNetCore5.Startup+<>c__DisplayClass4_0",
+ "class": "Samples.Security.AspNetCore5.Startup+<>c__DisplayClass4_0",
2 occurrences of : - "path": "Iast_Print",
+ "path": "print.aspx.cs",
+ "class": "Iast_Print",
6 occurrences of : - "path": "Samples.WeakCipher.Program",
+ "class": "Samples.WeakCipher.Program",
8 occurrences of : - "path": "Samples.Deduplication.Program",
+ "class": "Samples.Deduplication.Program",
|
![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: ca126fbc34
ℹ️ 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".
BenchmarksBenchmark execution time: 2026-07-28 16:41:44 Comparing candidate commit e850350 in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 70 metrics, 0 unstable metrics, 59 known flaky benchmarks, 67 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8930) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8930) - mean (75ms) : 70, 80
master - mean (74ms) : 69, 78
section Bailout
This PR (8930) - mean (77ms) : 73, 82
master - mean (78ms) : 74, 82
section CallTarget+Inlining+NGEN
This PR (8930) - mean (1,099ms) : 1041, 1156
master - mean (1,092ms) : 1042, 1141
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8930) - mean (114ms) : 106, 121
master - mean (114ms) : 109, 120
section Bailout
This PR (8930) - mean (111ms) : 109, 114
master - mean (112ms) : 108, 116
section CallTarget+Inlining+NGEN
This PR (8930) - mean (790ms) : 762, 818
master - mean (785ms) : 762, 808
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8930) - mean (98ms) : 95, 102
master - mean (97ms) : 95, 100
section Bailout
This PR (8930) - mean (99ms) : 97, 100
master - mean (102ms) : 95, 109
section CallTarget+Inlining+NGEN
This PR (8930) - mean (949ms) : 909, 989
master - mean (943ms) : 903, 984
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8930) - mean (101ms) : 95, 107
master - mean (98ms) : 93, 103
section Bailout
This PR (8930) - mean (98ms) : 94, 101
master - mean (97ms) : 96, 99
section CallTarget+Inlining+NGEN
This PR (8930) - mean (823ms) : 781, 864
master - mean (829ms) : 781, 877
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8930) - mean (218ms) : 202, 234
master - mean (210ms) : 201, 219
section Bailout
This PR (8930) - mean (221ms) : 211, 232
master - mean (212ms) : 205, 219
section CallTarget+Inlining+NGEN
This PR (8930) - mean (1,279ms) : 1216, 1341
master - mean (1,246ms) : 1194, 1297
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8930) - mean (314ms) : 296, 333
master - mean (304ms) : 290, 317
section Bailout
This PR (8930) - mean (322ms) : 301, 343
master - mean (308ms) : 290, 327
section CallTarget+Inlining+NGEN
This PR (8930) - mean (1,026ms) : 992, 1060
master - mean (1,002ms) : 976, 1027
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8930) - mean (310ms) : 285, 335
master - mean (298ms) : 281, 315
section Bailout
This PR (8930) - mean (299ms) : 286, 312
master - mean (292ms) : 280, 305
section CallTarget+Inlining+NGEN
This PR (8930) - mean (1,203ms) : 1156, 1249
master - mean (1,189ms) : 1147, 1231
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8930) - mean (304ms) : 282, 327
master - mean (292ms) : 277, 307
section Bailout
This PR (8930) - mean (305ms) : 286, 324
master - mean (301ms) : 283, 319
section CallTarget+Inlining+NGEN
This PR (8930) - mean (1,152ms) : 1033, 1271
master - mean (1,095ms) : 985, 1205
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Split the vulnerability Location so that the declaring type and the source file are reported in distinct fields: - New `class` field always carries the declaring type (`DeclaringType.FullName`) — this is what `path` used to hold. - `method` continues to carry the method name. - `path` is repurposed to hold the source file name (basename of `StackFrame.GetFileName()`), present only when debug info (PDBs) is available. - `line` remains present only with debug info. Deduplication is unchanged: `Location.GetHashCode()` now hashes `Class`+`Method` (previously `Path`+`Method`, where `Path` was the type), so the hash is stable and independent of PDB availability. Updates the MessagePack meta-struct emission, the vulnerability JSON schema, unit tests, and regenerates the IAST integration snapshots. The three snapshots with debug info (Razor/WebForms compiled types) were regenerated locally in Release to capture the real runtime source-file `path` values. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ename
The Location resemantization moved the declaring type from `path` to the
new `class` field. Update the test infrastructure that was still keyed to
the old layout:
- AspNetCore5IastTests: move the type-normalizing scrubbers (Razor
`AspNetCoreGeneratedDocument`, compiler display classes, Program/Startup)
from `path` to `class` so unstable generated names stay scrubbed.
- GrpcDotNetTests: widen the `location` block scrubber from {0,4} to {0,6}
inner lines so the (now 5-field) block is still fully removed.
- Samples.InstrumentedTests: reflect `Location.Class` instead of `Path`
for the LocationIsOk type-containment check.
- Location: split source-file basename on both `/` and `\` so a
Windows-built PDB read on Unix does not leak the full build path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dromanol
force-pushed
the
dani/iast/enhanced_location
branch
from
July 28, 2026 15:51
64ee155 to
e850350
Compare
dromanol
enabled auto-merge (squash)
July 29, 2026 07:05
e-n-0
approved these changes
Jul 29, 2026
andrewlock
approved these changes
Jul 29, 2026
| internal readonly struct Location | ||
| { | ||
| // Both Windows ('\') and Unix ('/') separators, because PDBs produced on one OS may be read on another. | ||
| private static readonly char[] PathSeparators = ['/', '\\']; |
Member
There was a problem hiding this comment.
nit: we have a Separators class that we use in various places, could we move this to there so we can reuse it in other places too?
|
|
||
| // Add scrub for the location data, as using APM sample, we won't disable symbols on their sample | ||
| (Regex RegexPattern, string Replacement) locationMsgRegex = (new Regex(@"(\S)*""location"": {(\r|\n){1,2}(.*(\r|\n){1,2}){0,4}(\s)*},"), string.Empty); | ||
| (Regex RegexPattern, string Replacement) locationMsgRegex = (new Regex(@"(\S)*""location"": {(\r|\n){1,2}(.*(\r|\n){1,2}){0,6}(\s)*},"), string.Empty); |
Member
There was a problem hiding this comment.
Why did this need to change? 🤔 should it be 5 for class? 😅
Contributor
Author
There was a problem hiding this comment.
I suppose Claude wanted to be extra sure XD
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.
Summary of changes
Resemantizes the IAST vulnerability
Locationso the declaring type and the source file are reported in distinct fields:classfield always carries the declaring type (DeclaringType.FullName) — this is whatpathused to hold.methodcontinues to carry the method name (unchanged).pathis repurposed to hold the source file name (basename ofStackFrame.GetFileName()), present only when debug info (PDBs) is available.lineremains present only with debug info (as before).Reason for change
Previously
pathconflated two concepts: it held the type name, never the actual source file, and left file/line information (which depends on PDBs) unused. This split makesclassa stable, always-present identifier and letspathcarry genuinely useful source-file information when PDBs are present.Implementation details
Iast/Location.cs: addedClassproperty; runtime ctor now setsClass = method?.DeclaringType?.FullNameandPath = Path.GetFileName(stackFrame?.GetFileName())(filename only, matchingStackReporterand avoiding leaking build-machine paths). String and test ctors updated accordingly.Location.GetHashCode()now hashesClass+Method(previouslyPath+Method, wherePathwas the type). Deduplication is unchanged — same inputs, same hash — and is now independent of PDB availability.AppSec/Rasp/MetaStructHelper.cs: emitsclassin the MessagePack meta-struct. The_dd.iast.jsonspan tag emitsclassautomatically via the reflection-based camelCase serializer (NullValueHandling.Ignore).vulnerability_schema.json: addedclass;pathre-documented as "source file name (only available with debug info)".Test coverage
LocationTestsandVulnerabilityBatchTestsunit tests.path→classrename (the normal integration environment does not expose PDBs for sample code).path:Iast.ReflectedXss.AspNetMvc5.IastEnabled→path: "ReflectedXss.cshtml"Security.AspNetWebForms.Classic/Integrated…TestQueryParameterNameVulnerability→path: "print.aspx.cs"Other details
classfield and the resemantizedpathbefore merging.