Skip to content

Commit b9fd1b3

Browse files
andyleejordanCopilot
andyleejordan
and
Copilot
authored
Match strong-name identity when resolving PSES dependencies (#2303)
`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>
1 parent 6ad4f46 commit b9fd1b3

4 files changed

Lines changed: 210 additions & 3 deletions

File tree

‎src/PowerShellEditorServices.Hosting/Internal/PsesLoadContext.cs‎

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,72 @@ private static bool IsSatisfyingAssembly(AssemblyName requiredAssemblyName, stri
7676
return false;
7777
}
7878

79-
AssemblyName asmToLoadName = AssemblyName.GetAssemblyName(assemblyPath);
79+
return IsSatisfyingAssembly(requiredAssemblyName, AssemblyName.GetAssemblyName(assemblyPath));
80+
}
81+
82+
// Internal (rather than private) purely so it can be unit tested with constructed
83+
// AssemblyName instances; it has no file-system dependency of its own.
84+
internal static bool IsSatisfyingAssembly(AssemblyName requiredAssemblyName, AssemblyName asmToLoadName)
85+
{
86+
// The simple name must match (case-insensitively, as assembly names are).
87+
if (!string.Equals(asmToLoadName.Name, requiredAssemblyName.Name, StringComparison.OrdinalIgnoreCase))
88+
{
89+
return false;
90+
}
91+
92+
// The candidate must be at least the requested version. We still accept newer
93+
// versions, since shared framework and $PSHOME assemblies are generally
94+
// forward-compatible via the runtime's binding.
95+
if (asmToLoadName.Version < requiredAssemblyName.Version)
96+
{
97+
return false;
98+
}
99+
100+
// The strong-name identity must match. Previously only the simple name and version
101+
// were compared, so a same-named assembly with a *different* public key token (i.e.
102+
// a genuinely different assembly) was treated as a drop-in replacement and would then
103+
// fail at runtime with a FileLoadException/TypeLoadException. Requiring the public key
104+
// token to match means we only short-circuit to a $PSHOME/Common assembly that can
105+
// actually satisfy the reference; otherwise we fall through and let the default load
106+
// context resolve it with its own (laxer) rules.
107+
if (!PublicKeyTokensMatch(requiredAssemblyName, asmToLoadName))
108+
{
109+
return false;
110+
}
111+
112+
// The culture must match so we never substitute a satellite resource assembly for the
113+
// neutral one (or vice versa).
114+
return string.Equals(
115+
asmToLoadName.CultureName ?? string.Empty,
116+
requiredAssemblyName.CultureName ?? string.Empty,
117+
StringComparison.OrdinalIgnoreCase);
118+
}
119+
120+
private static bool PublicKeyTokensMatch(AssemblyName requiredAssemblyName, AssemblyName candidateAssemblyName)
121+
{
122+
byte[] requiredToken = requiredAssemblyName.GetPublicKeyToken();
123+
124+
// A reference to a non-strong-named assembly imposes no public key token requirement.
125+
if (requiredToken is null || requiredToken.Length == 0)
126+
{
127+
return true;
128+
}
129+
130+
byte[] candidateToken = candidateAssemblyName.GetPublicKeyToken();
131+
if (candidateToken is null || candidateToken.Length != requiredToken.Length)
132+
{
133+
return false;
134+
}
135+
136+
for (int i = 0; i < requiredToken.Length; i++)
137+
{
138+
if (requiredToken[i] != candidateToken[i])
139+
{
140+
return false;
141+
}
142+
}
80143

81-
return string.Equals(asmToLoadName.Name, requiredAssemblyName.Name, StringComparison.OrdinalIgnoreCase)
82-
&& asmToLoadName.Version >= requiredAssemblyName.Version;
144+
return true;
83145
}
84146
}
85147
}

‎src/PowerShellEditorServices.Hosting/PowerShellEditorServices.Hosting.csproj‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@
1111
<DefineConstants>$(DefineConstants);CoreCLR</DefineConstants>
1212
</PropertyGroup>
1313

14+
<ItemGroup>
15+
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo">
16+
<_Parameter1>Microsoft.PowerShell.EditorServices.Test</_Parameter1>
17+
</AssemblyAttribute>
18+
</ItemGroup>
19+
1420
<ItemGroup>
1521
<PackageReference Include="PowerShellStandard.Library" PrivateAssets="all" />
1622
<PackageReference Include="System.IO.Pipes.AccessControl" />

‎test/PowerShellEditorServices.Test/PowerShellEditorServices.Test.csproj‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616
<ProjectReference Include="..\PowerShellEditorServices.Test.Shared\PowerShellEditorServices.Test.Shared.csproj" />
1717
</ItemGroup>
1818

19+
<!-- The Hosting assembly (and thus PsesLoadContext) only exists on .NET Core. -->
20+
<ItemGroup Condition=" '$(TargetFramework)' == 'net8.0' ">
21+
<ProjectReference Include="..\..\src\PowerShellEditorServices.Hosting\PowerShellEditorServices.Hosting.csproj" />
22+
</ItemGroup>
23+
1924
<!-- PowerShell 7.4.x -->
2025
<ItemGroup Condition=" '$(TargetFramework)' == 'net8.0' ">
2126
<PackageReference Include="Microsoft.PowerShell.SDK" />

‎test/PowerShellEditorServices.Test/Session/PsesLoadContextTests.cs‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
#if CoreCLR
5+
6+
using System;
7+
using System.Globalization;
8+
using System.Reflection;
9+
using Microsoft.PowerShell.EditorServices.Hosting;
10+
using Xunit;
11+
12+
namespace PowerShellEditorServices.Test.Session
13+
{
14+
[Trait("Category", "PsesLoadContext")]
15+
public class PsesLoadContextTests
16+
{
17+
// Two distinct, realistic public key tokens: Newtonsoft.Json's and the ECMA/Microsoft one.
18+
private static readonly byte[] s_tokenA = { 0x30, 0xad, 0x4f, 0xe6, 0xb2, 0xa6, 0xae, 0xed };
19+
private static readonly byte[] s_tokenB = { 0xb0, 0x3f, 0x5f, 0x7f, 0x11, 0xd5, 0x0a, 0x3a };
20+
21+
private static AssemblyName MakeName(
22+
string name,
23+
string version = "1.0.0.0",
24+
byte[] publicKeyToken = null,
25+
string culture = "")
26+
{
27+
AssemblyName assemblyName = new(name)
28+
{
29+
Version = new Version(version),
30+
CultureInfo = string.IsNullOrEmpty(culture)
31+
? CultureInfo.InvariantCulture
32+
: new CultureInfo(culture),
33+
};
34+
35+
assemblyName.SetPublicKeyToken(publicKeyToken);
36+
return assemblyName;
37+
}
38+
39+
[Fact]
40+
public void IsSatisfyingWhenIdentityMatchesExactly()
41+
{
42+
AssemblyName required = MakeName("Contoso.Lib", "2.0.0.0", s_tokenA);
43+
AssemblyName candidate = MakeName("Contoso.Lib", "2.0.0.0", s_tokenA);
44+
45+
Assert.True(PsesLoadContext.IsSatisfyingAssembly(required, candidate));
46+
}
47+
48+
[Fact]
49+
public void IsSatisfyingWhenCandidateVersionIsNewer()
50+
{
51+
AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA);
52+
AssemblyName candidate = MakeName("Contoso.Lib", "2.5.0.0", s_tokenA);
53+
54+
Assert.True(PsesLoadContext.IsSatisfyingAssembly(required, candidate));
55+
}
56+
57+
[Fact]
58+
public void IsNotSatisfyingWhenCandidateVersionIsOlder()
59+
{
60+
AssemblyName required = MakeName("Contoso.Lib", "2.0.0.0", s_tokenA);
61+
AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA);
62+
63+
Assert.False(PsesLoadContext.IsSatisfyingAssembly(required, candidate));
64+
}
65+
66+
[Fact]
67+
public void IsNotSatisfyingWhenSimpleNameDiffers()
68+
{
69+
AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA);
70+
AssemblyName candidate = MakeName("Fabrikam.Lib", "1.0.0.0", s_tokenA);
71+
72+
Assert.False(PsesLoadContext.IsSatisfyingAssembly(required, candidate));
73+
}
74+
75+
[Fact]
76+
public void IsSatisfyingWhenSimpleNameDiffersOnlyByCase()
77+
{
78+
AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA);
79+
AssemblyName candidate = MakeName("contoso.lib", "1.0.0.0", s_tokenA);
80+
81+
Assert.True(PsesLoadContext.IsSatisfyingAssembly(required, candidate));
82+
}
83+
84+
// This is the core fix: matching name and version but a different strong-name identity
85+
// must NOT be treated as a drop-in replacement, since binding to it would fail at runtime.
86+
[Fact]
87+
public void IsNotSatisfyingWhenPublicKeyTokenDiffers()
88+
{
89+
AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA);
90+
AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", s_tokenB);
91+
92+
Assert.False(PsesLoadContext.IsSatisfyingAssembly(required, candidate));
93+
}
94+
95+
[Fact]
96+
public void IsNotSatisfyingWhenRequiredIsStrongNamedButCandidateIsNot()
97+
{
98+
AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA);
99+
AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", publicKeyToken: null);
100+
101+
Assert.False(PsesLoadContext.IsSatisfyingAssembly(required, candidate));
102+
}
103+
104+
// A reference to a non-strong-named assembly imposes no public key token requirement.
105+
[Fact]
106+
public void IsSatisfyingWhenRequiredIsNotStrongNamedRegardlessOfCandidateToken()
107+
{
108+
AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", publicKeyToken: null);
109+
AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA);
110+
111+
Assert.True(PsesLoadContext.IsSatisfyingAssembly(required, candidate));
112+
}
113+
114+
[Fact]
115+
public void IsNotSatisfyingWhenCultureDiffers()
116+
{
117+
AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA, culture: "");
118+
AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA, culture: "fr");
119+
120+
Assert.False(PsesLoadContext.IsSatisfyingAssembly(required, candidate));
121+
}
122+
123+
[Fact]
124+
public void IsSatisfyingWhenCultureMatches()
125+
{
126+
AssemblyName required = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA, culture: "fr");
127+
AssemblyName candidate = MakeName("Contoso.Lib", "1.0.0.0", s_tokenA, culture: "fr");
128+
129+
Assert.True(PsesLoadContext.IsSatisfyingAssembly(required, candidate));
130+
}
131+
}
132+
}
133+
134+
#endif

0 commit comments

Comments
 (0)