Suppress ERR997 for IL-only managed assemblies#1174
Open
danmoseley wants to merge 7 commits into
Open
Conversation
Remove EnforcePdbLoadForManagedAssemblies from the ERR997 gate condition so that IL-only managed assemblies (including satellite resource assemblies) no longer trigger ERR997.ExceptionLoadingPdb. These assemblies typically have no PDB, and no current rule requires PDB data for IL-only binaries. Native and mixed-mode binaries still trigger ERR997 as before. The two PDB-using rules applicable to IL-only assemblies (BA2004, BA2027) already handle null PDB gracefully via LogPdbLoadException => false. Mark EnforcePdbLoadForManagedAssemblies as [Obsolete] since it is public virtual but no longer referenced in the gate logic. Fixes microsoft#1173 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
Member
Author
|
For our dotnet/dotnet repo, while this doesn't produce noise in Guardian/S360, it is super noisy in local logs, and makes SARIF much bigger — 63,702 notification entries bloat the SARIF to 80MB. With this fix it is 50MB |
…Assemblies - New property defaults to false (opt-in instead of opt-out) - Add IsManagedResourceOnly unconditional skip for satellite DLLs - Future rules can override RequiresPdbForManagedAssemblies => true to get the ERR997 safety net for IL-only managed assemblies Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Changed to the alternative approach suggested by @martin-reznik -- note, this version is a public API breaking change, which I had avoided in the first change. I do not know whether that is a problem. Description updated above. |
…o false Restore the original property name to avoid a breaking public API change. The default is changed from true to false so IL-only managed assemblies no longer trigger ERR997. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sasinkas
reviewed
Apr 22, 2026
Member
Author
Member
Author
|
@Sasinkas how can I get this merged? I've pinged Martin offline but he's quite busy. Can we merge? |
Member
Author
|
Oh - I see you just signed of today! thanks. Should I click merge? |
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.
Fixes #1173
Note
This is an AI-assisted draft.
Important
Breaking change:
EnforcePdbLoadForManagedAssembliesis replaced byRequiresPdbForManagedAssemblieswith inverted default (falseinstead oftrue). Any rule or external subclass that relied on the old property name or itstruedefault will need to be updated.Problem
ERR997.ExceptionLoadingPdbfires for IL-only managed assemblies even thoughno current BinSkim rule requires PDB data for IL-only binaries. This creates
significant noise — for example, scanning dotnet/dotnet macOS artifacts produces
thousands of ERR997 entries on IL-only assemblies (including satellite resource
assemblies), none actionable.
Note — this makes the raw logs cleaner. ERR997 is not going to Guardian or S360
so it's just a tidiness thing.
Root cause
In
WindowsBinaryAndPdbSkimmerBase.Analyze(), the oldEnforcePdbLoadForManagedAssembliesdefaulted totrueand no rule overrodeit, so the ERR997 condition always evaluated to true for IL-only assemblies
with no PDB.
Fix
Replace
EnforcePdbLoadForManagedAssemblies(defaulttrue) withRequiresPdbForManagedAssemblies(defaultfalse). This is a public APIbreaking change — the old property is removed, not obsoleted.
IL-only managed assemblies with no PDB now proceed to
AnalyzePortableExecutableAndPdb()instead of emitting ERR997 and returningearly. Resource-only assemblies (
IsManagedResourceOnly) are also excludedfrom the PDB requirement.
Rules that genuinely need PDB data for IL-only managed assemblies can override
RequiresPdbForManagedAssembliesto returntrue.Why this is safe
Verified all 17
WindowsBinaryAndPdbSkimmerBasesubclasses:CanAnalyzePE(directly or viaStackProtectionUtilities) — they never reachAnalyze()for IL-onlyassemblies.
already null-check
Pdbat the top ofAnalyzePortableExecutableAndPdb()and return early when it is null.
Native and mixed-mode binaries still trigger ERR997 as before.