Skip to content

Make ilverify respect IgnoresAccessChecksToAttribute#130978

Draft
JulieLeeMSFT with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-ilverify-ignore-access-checks-to
Draft

Make ilverify respect IgnoresAccessChecksToAttribute#130978
JulieLeeMSFT with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-ilverify-ignore-access-checks-to

Conversation

Copilot AI commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

ilverify only honored InternalsVisibleToAttribute for cross-assembly access checks, so IL that legitimately accesses another assembly's internal/private members via [assembly: IgnoresAccessChecksTo("...")] was incorrectly reported as invalid. This aligns ilverify with CoreCLR runtime behavior, where an assembly declaring IgnoresAccessChecksTo may bypass all access checks (private, family, assembly) to the named target — stronger than InternalsVisibleTo.

Changes

  • AccessVerificationHelpers.cs

    • Added IgnoresAccessChecksTo(this ModuleDesc accessing, ModuleDesc accessed) which decodes System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute on the accessing assembly and matches the accessed assembly by simple name — mirroring the runtime's FriendAssemblyDescriptor::IsAssemblyOnList / RefMatchesDef (name-only match when the declared name omits a public key token).
    • Added a bypass in the type-level CanAccess (after the generic-instantiation checks, so third-assembly instantiation arguments are still validated) and in CanAccessMember, covering type, method, and field access.
  • Tests — new AccessTestsIgnoreChecks.il / .ilproj test assembly declaring [IgnoresAccessChecksTo("AccessTests")] (defining the attribute type itself, as real consumers like IgnoresAccessChecksToGenerator do), with 11 cases exercising private/family/assembly classes, nested types, methods, and fields. Confirmed the cases fail without the fix and pass with it.

// Accessing assembly waives all access checks to the target assembly.
if (currentTypeDef.Module.IgnoresAccessChecksTo(targetTypeDef.Module))
    return true;

Copilot AI review requested due to automatic review settings July 17, 2026 16:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI linked an issue Jul 17, 2026 that may be closed by this pull request
Co-authored-by: JulieLeeMSFT <63486087+JulieLeeMSFT@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 17, 2026 16:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Co-authored-by: JulieLeeMSFT <63486087+JulieLeeMSFT@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 17, 2026 16:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI changed the title [WIP] Fix ilverify to respect IgnoreAccessChecksTo Make ilverify respect IgnoresAccessChecksToAttribute Jul 17, 2026
Copilot AI requested a review from JulieLeeMSFT July 17, 2026 16:29
@MichalPetryka

Copy link
Copy Markdown
Contributor

The test can be in C# but using separate ref and impl assemblies differing by member accessibility, it might be cleaner that way.

@github-actions github-actions Bot added the area-Tools-ILVerification Issues related to ilverify tool and IL verification in general label Jul 17, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/jit-contrib
See info in area-owners.md if you want to be subscribed.

var currentTypeDef = (MetadataType)currentClass.GetTypeDefinition();
var targetTypeDef = (EcmaType)targetClass.GetTypeDefinition();

// The accessing assembly can waive all access checks to the target assembly via [IgnoresAccessChecksTo].

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very rare case - check of last resort, and it is somewhat expensive due to custom attribute lookup and parsing. It should be done last when all other ways to prove accessibility failed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does ILVerify performance matter to this extent? I'd have assumed the overhead to be fine enough for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Tools-ILVerification Issues related to ilverify tool and IL verification in general

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ilverify doesn't respect IgnoreAccessChecksTo

5 participants