Powershell Integration - System.Security.Cryptography.Xml Vulnerability#1288
Open
Omnideth wants to merge 3 commits intoCommunityToolkit:mainfrom
Open
Powershell Integration - System.Security.Cryptography.Xml Vulnerability#1288Omnideth wants to merge 3 commits intoCommunityToolkit:mainfrom
Omnideth wants to merge 3 commits intoCommunityToolkit:mainfrom
Conversation
I am unclear if this test was flaky before or not as it refused to build with the Cryptography vulnerability. Versions of the Powershell SDK above 4.10 don't support .NET 8 anymore. Even the newest Powershell SDK which doesn't support Net 9 anymore only has Cryptography.XML at 10.0.5 which is still pinged with high vuln. Had to pull it in manually to pin it at 10.0.6 for now. I will contact Oising to see if that test is just failing for me or if he's broken too. It might be that the scripts aren't checked in that are to be running in this test: ScriptsExecuteSuccessfully()
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.sh | bash -s -- 1288Or
iex "& { $(irm https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.ps1) } 1288" |
Contributor
There was a problem hiding this comment.
Pull request overview
Pins System.Security.Cryptography.Xml to a non-vulnerable version for the PowerShell hosting integration to restore/build cleanly under the repo’s “warnings as errors” policy, addressing CI/build breaks reported in #1287.
Changes:
- Add an explicit
System.Security.Cryptography.Xmlpackage reference to the PowerShell hosting project. - Centrally pin
System.Security.Cryptography.Xmlto10.0.6inDirectory.Packages.props. - Ignore
*.lscacheartifacts via.gitignore.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/CommunityToolkit.Aspire.Hosting.PowerShell/CommunityToolkit.Aspire.Hosting.PowerShell.csproj | Adds a direct reference to System.Security.Cryptography.Xml to ensure the pinned version is used during restore. |
| Directory.Packages.props | Centrally sets System.Security.Cryptography.Xml version to 10.0.6 for vulnerability mitigation. |
| .gitignore | Adds ignore pattern for *.lscache generated files. |
…/ why: Summary Root cause: A race between the PowerShellRunspacePoolResource and its child PowerShellScriptResource. In DistributedApplicationBuilderExtensions.AddPowerShell, the pool's OnInitializeResource fires _ = res.StartAsync(...) and returns. Aspire 13.2 then transitions ps toward Running (the "Waiting for resource 'ps' to enter Running" log from script1 appears but the wait effectively resolves immediately). Script1's StartAsync assigns _ps.RunspacePool = Parent.Pool — but Parent.Pool is still null (or in Opening state) because the pool's RunspaceFactory.CreateRunspacePool(...) / BeginOpen hasn't run/completed yet. With a null runspace pool, InvokeAsync hangs forever — scripts never reach Finished, and the test times out after 90s. Fixes applied: DistributedApplicationBuilderExtensions.cs:84: await res.StartAsync(...) instead of fire-and-forget so pool initialization blocks until open. PowerShellScriptResource.cs:131-141: Defensive poll in StartAsync for Parent.Pool to exist and be in RunspacePoolState.Opened before binding. This is the decisive fix because Aspire's WaitFor enforcement wasn't blocking as expected. Verified: Both tests in CommunityToolkit.Aspire.Hosting.PowerShell.Tests now pass (~31s).
…ing the System.Security.Cryptography.Xml library reference to the group labeled for System packages.
Contributor
Author
|
@oising heads up here are the changes I mentioned in the discord. Please let me know if this messes you up anywhere, but now I've got the test passing and the CI issues resolved. |
7 tasks
oising
reviewed
Apr 20, 2026
| _ = res.StartAsync(sessionState, notificationService, poolLogger, hostLifetime, ct); | ||
| // Await pool open so the resource isn't marked Running before the RunspacePool exists. | ||
| // Otherwise dependent scripts that WaitFor(pool) can race and hit a null Parent.Pool. | ||
| await res.StartAsync(sessionState, notificationService, poolLogger, hostLifetime, ct); |
Contributor
There was a problem hiding this comment.
Good catch. I had done some refactoring and clearly messed up. This absolutely needs to be awaited.
| // the pool has been created AND opened by the parent resource before binding the PowerShell | ||
| // instance to it. Without this, _ps.RunspacePool can be null (hangs InvokeAsync) or in | ||
| // 'Opening' state (throws InvalidRunspacePoolStateException). | ||
| while ((Parent.Pool is null || Parent.Pool.RunspacePoolStateInfo.State != System.Management.Automation.Runspaces.RunspacePoolState.Opened) |
Contributor
There was a problem hiding this comment.
We don't need this after having added the await earlier, right? Unless there's some other issue implied?
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.
**Closes #1287 **
This change is the minimum required to get the project building again.
I am unclear if this test was flaky before or not as it refused to build with the Cryptography vulnerability.
Versions of the Powershell SDK above 4.10 don't support .NET 8 anymore.
Even the newest Powershell SDK which doesn't support Net 9 anymore only has Cryptography.XML at 10.0.5 which is still pinged with high vuln. Had to pull it in manually to pin it at 10.0.6 for now.
I will contact Oising to see if that test is just failing for me or if he's broken too.
It might be that the scripts aren't checked in that are to be running in this test: ScriptsExecuteSuccessfully()
PR Checklist
I don't think I broke it, but one of the tests is failing after getting the project to build.
Other information