Fix the dead plugin version gate and stop leaking host assemblies into plugin folders - #760
Merged
Merged
Conversation
PluginInfoAttribute computed SupportedVersion in its constructor from Assembly.GetExecutingAssembly(). That constructor runs inside Grand.Infrastructure, so it always returned Grand.Infrastructure's own version - never the plugin's - even though the attribute is read off the plugin DLL in PluginManager.PreparePluginInfo. GrandVersion.SupportedPluginVersion is derived from the same assembly, so both sides of every compatibility check were equal by construction. The gates in PluginManager.Load, PluginController.Install and PluginController.UploadPlugin could not reject anything: a plugin built against 1.0 loaded and installed as readily as one built against 2.4. SupportedVersion is now a plain optional property on the attribute, and PluginVersionResolver derives it from the plugin assembly's own Grand.Infrastructure reference when the plugin does not declare one. Every shipped plugin carries that reference at 2.4, so their resolved value is unchanged and none of them changes load behaviour. A plugin that cannot be tied to a core version resolves to null and is treated as incompatible rather than silently accepted. No Manifest.cs changes: plugins keep declaring only Version, and may now declare SupportedVersion explicitly if they need to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every plugin listed the same seven core projects with Private=false so the host's assemblies are compiled against but not copied into the plugin folder. The list was duplicated 16 times, and that duplication had already failed: Grand.Mediator arrived with the MediatR replacement (#753), was added to none of them, and MSBuild copied Grand.Mediator.dll and its .pdb into all 16 plugin output folders. It was the only Grand assembly leaking; the other seven were correctly excluded. That matters beyond tidiness. Plugins load into the default AssemblyLoadContext, so a duplicate of a host assembly beside a plugin is a hazard: if the plugin's copy is ever the one loaded, IRequestHandler<T> from it is not the same type as the host's and handler resolution fails in ways that are hard to trace. Private=false does not carry to a reference's own dependencies, which is why each project has to be named rather than left to the transitive graph. So the list stays, but now in one file that plugins import next to Grand.Common.props. Adding a core project is one edit instead of sixteen that are easy to miss. Paths are anchored with $(MSBuildThisFileDirectory) so a plugin can sit at any depth. Theme.Modern keeps its own Grand.Web reference - it is the only plugin with one, and referencing the host project is a separate problem. Verified by deleting all 16 output folders and rebuilding: no Grand.*.dll in any of them, plugin-specific third-party files (Stripe.net, Newtonsoft.Json) still copied, and all 16 assemblies still carry their Grand.Infrastructure 2.4 reference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The base-plugin template still spelled out the seven core project references, so a plugin scaffolded from it would reintroduce the duplication the shared props file removes - and the template's own list was already missing Grand.Mediator, which is how the leak spread in the first place. Also updates the two places that generate a new .csproj: the create-plugin prompt and the project-references section of the dependencies standard. The per-kind plugin skills are left alone; "mark GrandNode references Private=false" is still correct there, and now applies to whatever a plugin adds on top of the shared set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…NuGet cache
Four plugins reached into the NuGet cache with a hand-written CopyFile target
to get their third-party assemblies into the output folder:
<CopyFiles Include="$(NuGetPackageRoot)\stripe.net\51.1.0\lib\net9.0\*.*" />
Six such lines existed, each pinning a package version and a target framework
that Directory.Packages.props also controls. Bumping a package there left the
plugin shipping the old assembly, or nothing at all once that cache folder no
longer existed.
They were also incomplete, because a hand-written list cannot follow a
dependency graph. Payments.StripeCheckout shipped Stripe.net and
Newtonsoft.Json but not System.Configuration.ConfigurationManager,
System.Diagnostics.EventLog or System.Security.Cryptography.ProtectedData,
which Stripe.net needs; Payments.BrainTree never copied Braintree's
Newtonsoft.Json.
The reason the copies were needed at all: a plugin is a library, and the SDK
does not copy NuGet assemblies into a library's output - it leaves the runtime
to resolve them through deps.json, which does not happen for an assembly
side-loaded from a plugin folder.
Grand.Plugin.props now sets CopyLocalLockFileAssemblies, paired with
ExcludeAssets=runtime on each host reference. Without that pairing the switch
copies the host's whole package graph - 62 assemblies for a plugin that needs
one. This is the combination the projects in src/Modules already use.
Each plugin now gets exactly its own closure: one assembly for the Facebook
and Google providers, five for Stripe, three for BrainTree, nothing for the
twelve that reference no packages.
The same Grand.Mediator omission described in the previous commit also applied
to all four modules, which were copying Grand.Mediator.dll into their output
folders. Added there too.
Verified by deleting all 16 plugin and 4 module output folders and rebuilding.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Type: bugfix
Three defects in how the host and its plugins relate to each other. All were invisible: one made a safety check pass unconditionally, one quietly copied a host assembly into every plugin folder, and one shipped some plugins without their own dependencies.
Issue
1. The plugin compatibility gate could never reject anything
PluginInfoAttributecomputedSupportedVersionin its constructor:GetExecutingAssembly()returns the assembly holding the executing code. That constructor is compiled intoGrand.Infrastructure, so it returned Grand.Infrastructure's version no matter whose DLL was being inspected — even though the attribute is read off the plugin inPluginManager.PreparePluginInfo:GrandVersion.SupportedPluginVersionis derived from that same assembly, so both sides of every comparison were equal by construction. Three gates were dead code:PluginManager.Load— theIncompatible pluginbranchPluginController.Install— "You can't install unsupported version of plugin"PluginController.UploadPluginA plugin built against 1.0 loaded and installed exactly like one built against 2.4, then failed later with
TypeLoadExceptionorMissingMethodExceptionfrom plugin code instead of one clean log line. The admin upload screen showed the running installation's version in the "supported version" column rather than the uploaded file's.No
Manifest.cssetsSupportedVersionexplicitly, so there was no way to work around it.2.
Grand.Mediator.dllwas copied into all 16 plugin foldersPlugins name each core project with
Private=false, so the host's assemblies are compiled against but not copied.Grand.Mediatorarrived with the MediatR replacement (#753) and was added to none of the 16 lists, so it came in transitively —Private=falsedoes not carry to a reference's own dependencies — and MSBuild copied it.Grand.Mediator.dlland its.pdbsat in every plugin folder, and in all four module folders for the same reason. It was the only Grand assembly leaking; the other seven were correctly excluded.Plugins load into the default
AssemblyLoadContext, so a duplicate of a host assembly next to a plugin is a hazard rather than dead weight: if the plugin's copy is ever the one loaded,IRequestHandler<T>from it is not the same type as the host's and handler resolution fails in ways that are hard to trace.The reference list had been duplicated 16 times, which is why one addition to the solution could be missed everywhere at once.
3. Plugins hand-copied their NuGet assemblies, and got it wrong
Four plugins reached into the NuGet cache with a hand-written target:
Six such lines, each pinning a package version and a target framework that
Directory.Packages.propsalso controls. Bumping a package there leaves the plugin shipping the old assembly, or nothing once that cache folder no longer exists, and the build does not complain.A hand-written list also cannot follow a dependency graph.
Payments.StripeCheckoutshippedStripe.netandNewtonsoft.Jsonbut notSystem.Configuration.ConfigurationManager,System.Diagnostics.EventLogorSystem.Security.Cryptography.ProtectedData, whichStripe.netneeds.Payments.BrainTreenever copied Braintree'sNewtonsoft.Json.The copies were needed because a plugin is a library, and the SDK does not copy NuGet assemblies into a library's output — it leaves the runtime to resolve them through
deps.json, which does not happen for an assembly side-loaded from a plugin folder.Solution
Version gate.
SupportedVersionbecomes a plain optional property.PluginVersionResolverderives it from the plugin assembly's ownGrand.Infrastructurereference when the plugin does not declare one — the compiler records that version in the plugin's metadata, so it is the honest answer to "what was this built against". A plugin that cannot be tied to a core version resolves tonulland is treated as incompatible instead of silently accepted. NoManifest.cschanges; a plugin may still declareSupportedVersionitself.Reference list. New
src/Build/Grand.Plugin.propsholds the eight shared host references once. Plugins import it next toGrand.Common.propsand drop their own copies, so adding a core project is one edit instead of sixteen that are easy to miss. Paths use$(MSBuildThisFileDirectory)so a plugin can sit at any depth.Theme.Modernkeeps its ownGrand.Webreference — it is the only plugin with one, and referencing the host project is a separate problem. The four modules keep their individual lists but get the missingGrand.Mediatorentry.Package assemblies. The props file sets
CopyLocalLockFileAssemblies, paired withExcludeAssets=runtimeon each host reference. The pairing is the point: on its own the switch copies the host's entire package graph, 62 assemblies for a plugin that needs one. This is the combination the projects insrc/Modulesalready use. Each plugin now gets exactly its own closure and the fourCopyFiletargets are gone, so versions come only fromDirectory.Packages.props.Docs. The base-plugin template spelled out the same reference list — and was itself already missing
Grand.Mediator— so it would have reintroduced the duplication. Updated together with the create-plugin prompt and the dependencies standard.Breaking changes
Plugins compiled against an older GrandNode are now correctly reported as incompatible instead of loading. That is the gate working, but it is a visible change for anyone who was relying on the broken behaviour.
Nothing changes for the bundled plugins. All 16 built assemblies were read directly from their metadata: each carries a
Grand.Infrastructurereference at 2.4, so each resolves to"2.4"and loads exactly as before. Stripe and BrainTree now ship more than they did, not less.Testing
dotnet test ./src/Tests/Grand.Infrastructure.Tests/Grand.Infrastructure.Tests.csproj— 94 pass, including 7 newPluginVersionResolverTests.Grand.Module.Api.Tests— 20 pass.PluginInfoAttribute_DoesNotSelfAssignSupportedVersionfails on the previous code — the old constructor always assigned a value. That is the regression proof.Delete every folder under
src/Web/Grand.Web/Plugins/exceptbin, and every folder undersrc/Web/Grand.Web/Modules/, thendotnet build ./GrandNode.sln. Deleting first matters:OutputPathis a fixed folder the build never prunes, so stale copies survive an ordinary rebuild and hide the result.After that rebuild, no
Grand.*.dllin any of the 16 plugin folders or 4 module folders, and each plugin holds only its own packages:Manual: admin panel → Configuration → Plugins. Every bundled plugin still lists as compatible and installs. Exercise the paths that need the copied packages — sign in with Facebook and with Google, and run a Stripe and a BrainTree payment — since those are what a missing assembly would break.
🤖 Generated with Claude Code