From 0c0e9da85009f569c016d9bda6ba1bcb2af0c3ee Mon Sep 17 00:00:00 2001 From: Andy Jordan <2226434+andyleejordan@users.noreply.github.com> Date: Fri, 12 Jun 2026 15:42:33 -0700 Subject: [PATCH] Match strong-name identity when resolving PSES dependencies `PsesLoadContext.IsSatisfyingAssembly` decided whether a candidate DLL in `$PSHOME` or our bundled `Common` directory could satisfy a dependency request using only the simple name and `Version >=`. That ignores the rest of the assembly identity, so a same-named assembly with a different public key token (i.e. a genuinely different assembly) was treated as a drop-in replacement. When the runtime then bound against it, the mismatch surfaced later as a `FileLoadException`/`TypeLoadException` rather than being declined up front. Patrick and I had suspected for a while that the version-only matching was inadequate, so this is a focused trial of tightening it. We now also require the public key token and culture to match: - If the requested reference is strong-named, the candidate's public key token must match exactly; a non-strong-named reference imposes no token requirement. - The culture must match, so we never substitute a satellite resource assembly for the neutral one (or vice versa). The check can only return `false` in more cases than before, and only for assemblies that could not have satisfied the reference anyway. On a token mismatch we now decline to short-circuit and fall through to the default load context's own (laxer) resolution instead of forcing a copy that fails at load. Measured against a current build, no presently-bundled dependency changes resolution under the new rules, so this is purely added protection. I pulled the pure comparison into an `internal` overload taking two `AssemblyName`s and added `PsesLoadContextTests` covering the version, name, public key token, and culture cases. The Hosting assembly (and thus `PsesLoadContext`) is .NET Core only, so the project reference and tests are guarded to `net8.0`/`CoreCLR`. Drafted by Copilot (Claude Opus 4.8). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Internal/PsesLoadContext.cs | 68 ++++++++- .../PowerShellEditorServices.Hosting.csproj | 6 + .../PowerShellEditorServices.Test.csproj | 5 + .../Session/PsesLoadContextTests.cs | 134 ++++++++++++++++++ 4 files changed, 210 insertions(+), 3 deletions(-) create mode 100644 test/PowerShellEditorServices.Test/Session/PsesLoadContextTests.cs diff --git a/src/PowerShellEditorServices.Hosting/Internal/PsesLoadContext.cs b/src/PowerShellEditorServices.Hosting/Internal/PsesLoadContext.cs index da6588f7e..77de15bd1 100644 --- a/src/PowerShellEditorServices.Hosting/Internal/PsesLoadContext.cs +++ b/src/PowerShellEditorServices.Hosting/Internal/PsesLoadContext.cs @@ -76,10 +76,72 @@ private static bool IsSatisfyingAssembly(AssemblyName requiredAssemblyName, stri return false; } - AssemblyName asmToLoadName = AssemblyName.GetAssemblyName(assemblyPath); + return IsSatisfyingAssembly(requiredAssemblyName, AssemblyName.GetAssemblyName(assemblyPath)); + } + + // Internal (rather than private) purely so it can be unit tested with constructed + // AssemblyName instances; it has no file-system dependency of its own. + internal static bool IsSatisfyingAssembly(AssemblyName requiredAssemblyName, AssemblyName asmToLoadName) + { + // The simple name must match (case-insensitively, as assembly names are). + if (!string.Equals(asmToLoadName.Name, requiredAssemblyName.Name, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + // The candidate must be at least the requested version. We still accept newer + // versions, since shared framework and $PSHOME assemblies are generally + // forward-compatible via the runtime's binding. + if (asmToLoadName.Version < requiredAssemblyName.Version) + { + return false; + } + + // The strong-name identity must match. Previously only the simple name and version + // were compared, so a same-named assembly with a *different* public key token (i.e. + // a genuinely different assembly) was treated as a drop-in replacement and would then + // fail at runtime with a FileLoadException/TypeLoadException. Requiring the public key + // token to match means we only short-circuit to a $PSHOME/Common assembly that can + // actually satisfy the reference; otherwise we fall through and let the default load + // context resolve it with its own (laxer) rules. + if (!PublicKeyTokensMatch(requiredAssemblyName, asmToLoadName)) + { + return false; + } + + // The culture must match so we never substitute a satellite resource assembly for the + // neutral one (or vice versa). + return string.Equals( + asmToLoadName.CultureName ?? string.Empty, + requiredAssemblyName.CultureName ?? string.Empty, + StringComparison.OrdinalIgnoreCase); + } + + private static bool PublicKeyTokensMatch(AssemblyName requiredAssemblyName, AssemblyName candidateAssemblyName) + { + byte[] requiredToken = requiredAssemblyName.GetPublicKeyToken(); + + // A reference to a non-strong-named assembly imposes no public key token requirement. + if (requiredToken is null || requiredToken.Length == 0) + { + return true; + } + + byte[] candidateToken = candidateAssemblyName.GetPublicKeyToken(); + if (candidateToken is null || candidateToken.Length != requiredToken.Length) + { + return false; + } + + for (int i = 0; i < requiredToken.Length; i++) + { + if (requiredToken[i] != candidateToken[i]) + { + return false; + } + } - return string.Equals(asmToLoadName.Name, requiredAssemblyName.Name, StringComparison.OrdinalIgnoreCase) - && asmToLoadName.Version >= requiredAssemblyName.Version; + return true; } } } diff --git a/src/PowerShellEditorServices.Hosting/PowerShellEditorServices.Hosting.csproj b/src/PowerShellEditorServices.Hosting/PowerShellEditorServices.Hosting.csproj index 1c30a93df..a41911e14 100644 --- a/src/PowerShellEditorServices.Hosting/PowerShellEditorServices.Hosting.csproj +++ b/src/PowerShellEditorServices.Hosting/PowerShellEditorServices.Hosting.csproj @@ -11,6 +11,12 @@ $(DefineConstants);CoreCLR + + + <_Parameter1>Microsoft.PowerShell.EditorServices.Test + + + diff --git a/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj b/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj index 6161a113e..0566b8cf2 100644 --- a/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj +++ b/test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj @@ -16,6 +16,11 @@ + + + + + diff --git a/test/PowerShellEditorServices.Test/Session/PsesLoadContextTests.cs b/test/PowerShellEditorServices.Test/Session/PsesLoadContextTests.cs new file mode 100644 index 000000000..ace268854 --- /dev/null +++ b/test/PowerShellEditorServices.Test/Session/PsesLoadContextTests.cs @@ -0,0 +1,134 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#if CoreCLR + +using System; +using System.Globalization; +using System.Reflection; +using Microsoft.PowerShell.EditorServices.Hosting; +using Xunit; + +namespace PowerShellEditorServices.Test.Session +{ + [Trait("Category", "PsesLoadContext")] + public class PsesLoadContextTests + { + // Two distinct, realistic public key tokens: Newtonsoft.Json's and the ECMA/Microsoft one. + private static readonly byte[] s_tokenA = { 0x30, 0xad, 0x4f, 0xe6, 0xb2, 0xa6, 0xae, 0xed }; + private static readonly byte[] s_tokenB = { 0xb0, 0x3f, 0x5f, 0x7f, 0x11, 0xd5, 0x0a, 0x3a }; + + private static AssemblyName MakeName( + string name, + string version = "1.0.0.0", + byte[] publicKeyToken = null, + string culture = "") + { + AssemblyName assemblyName = new(name) + { + Version = new Version(version), + CultureInfo = string.IsNullOrEmpty(culture) + ? CultureInfo.InvariantCulture + : new CultureInfo(culture), + }; + + assemblyName.SetPublicKeyToken(publicKeyToken); + return assemblyName; + } + + [Fact] + public void IsSatisfyingWhenIdentityMatchesExactly() + { + AssemblyName required = MakeName("Contoso.Lib", "2.0.0.0", s_tokenA); + AssemblyName candidate = MakeName("Contoso.Lib", "2.0.0.0", s_tokenA); + + Assert.True(PsesLoadContext.IsSatisfyingAssembly(required, candidate)); + } + + [Fact] + public void IsSatisfyingWhenCandidateVersionIsNewer() + { + AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA); + AssemblyName candidate = MakeName("Contoso.Lib", "2.5.0.0", s_tokenA); + + Assert.True(PsesLoadContext.IsSatisfyingAssembly(required, candidate)); + } + + [Fact] + public void IsNotSatisfyingWhenCandidateVersionIsOlder() + { + AssemblyName required = MakeName("Contoso.Lib", "2.0.0.0", s_tokenA); + AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA); + + Assert.False(PsesLoadContext.IsSatisfyingAssembly(required, candidate)); + } + + [Fact] + public void IsNotSatisfyingWhenSimpleNameDiffers() + { + AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA); + AssemblyName candidate = MakeName("Fabrikam.Lib", "1.0.0.0", s_tokenA); + + Assert.False(PsesLoadContext.IsSatisfyingAssembly(required, candidate)); + } + + [Fact] + public void IsSatisfyingWhenSimpleNameDiffersOnlyByCase() + { + AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA); + AssemblyName candidate = MakeName("contoso.lib", "1.0.0.0", s_tokenA); + + Assert.True(PsesLoadContext.IsSatisfyingAssembly(required, candidate)); + } + + // This is the core fix: matching name and version but a different strong-name identity + // must NOT be treated as a drop-in replacement, since binding to it would fail at runtime. + [Fact] + public void IsNotSatisfyingWhenPublicKeyTokenDiffers() + { + AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA); + AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", s_tokenB); + + Assert.False(PsesLoadContext.IsSatisfyingAssembly(required, candidate)); + } + + [Fact] + public void IsNotSatisfyingWhenRequiredIsStrongNamedButCandidateIsNot() + { + AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA); + AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", publicKeyToken: null); + + Assert.False(PsesLoadContext.IsSatisfyingAssembly(required, candidate)); + } + + // A reference to a non-strong-named assembly imposes no public key token requirement. + [Fact] + public void IsSatisfyingWhenRequiredIsNotStrongNamedRegardlessOfCandidateToken() + { + AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", publicKeyToken: null); + AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA); + + Assert.True(PsesLoadContext.IsSatisfyingAssembly(required, candidate)); + } + + [Fact] + public void IsNotSatisfyingWhenCultureDiffers() + { + AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA, culture: ""); + AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA, culture: "fr"); + + Assert.False(PsesLoadContext.IsSatisfyingAssembly(required, candidate)); + } + + [Fact] + public void IsSatisfyingWhenCultureMatches() + { + AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA, culture: "fr"); + AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA, culture: "fr"); + + Assert.True(PsesLoadContext.IsSatisfyingAssembly(required, candidate)); + } + } +} + +#endif