Repository navigation
Dependency bumps (round 2) - #9133
Conversation
Bump SignalR.Client/Common to 8.0.32/9.0.21/10.0.13 and pin SQLitePCLRaw.lib.e_sqlite3 to 2.1.14. Apps also pin System.Security.Cryptography.Pkcs 10.0.13 and SignalR.Common 10.0.13. Co-authored-by: Sipke Schoorstra <sipkeschoorstra@outlook.com>
elsa-3-lead
Bot
added
dependencies
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
SQLitePCLRaw pin and project references Directory.Packages.props, extensions/src/Directory.Packages.props, extensions/samples/workbench/Elsa.Server.Web/Elsa.Server.Web.csproj, extensions/src/scheduling/Elsa.Scheduling.Hangfire/Elsa.Scheduling.Hangfire.csproj, extensions/test/modules/persistence/*.csproj |
The root and extensions package sets pin SQLitePCLRaw.lib.e_sqlite3 to 2.1.14. The listed workbench, Hangfire, and persistence test projects add package references to it. |
Apps and Studio package versions apps/Directory.Packages.props, studio/src/Directory.Packages.props |
The apps package set adds central pins for SignalR Client, MySqlConnector, SQLitePCLRaw.lib.e_sqlite3, and System.Security.Cryptography.Pkcs. Studio updates SignalR Client pins for net8.0, net9.0, and net10.0. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Other
Possibly related issues
elsa-workflows/elsa-core#8194— The issue describes coordinated dependency updates across Core, Studio, and Extensions, which is also reflected in this PR’s package changes.
Possibly related PRs
- elsa-workflows/elsa-core#9124 — This PR also pins
System.Security.Cryptography.Pkcs, while this change adds the10.0.13pin in the apps package set. - elsa-workflows/elsa-core#9131 — This PR updates
MySqlConnectorto2.6.2in another package set; this change adds the same pin for apps. - elsa-workflows/elsa-core#9004 — This PR adds the
WebAssemblyServerPatchedVersionproperty used by the apps SignalR Client pin.
Suggested reviewers: elsa-workflows-review-bot
Merge Risk: 
32c03
-
No actionable merge-blocking risk is established by the dependency updates.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies the main change as dependency version updates and distinguishes this follow-up round. Description check The description follows the repository template and documents the purpose, dependency changes, scope, verification steps, expected outcomes, and checklist. It does not include concrete test commands o… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description follows the repository template and documents the purpose, dependency changes, scope, verification steps, expected outcomes, and checklist. It does not include concrete test commands or recorded results, but the required information is mostly complete.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
@elsa-workflows-bot review |
 [Medium impact] No blocking issues were identified in the reviewed changes. The apps version-property substitution preserves the selected SignalR.Client version. SummaryUpdates package versions for Studio, apps, Hangfire, and Dapper tests.
Since the previous review, the PR removes explicit Reviews (3) · Last reviewed commit: "Drop explicit SignalR.Common pins; rely ..." · Reviewed by Greptile |
|
@elsa-workflows-bot review |
![elsa-workflows-review-bot[bot]](https://avatars.githubusercontent.com/in/5321421?s=60&v=4){kind=small}
There was a problem hiding this comment.
Elsa 3 Code Review: REQUEST_CHANGES + HIGH @ 4d0f01f
Code Review, Round 1/6
Blocking
Redundant direct Microsoft.AspNetCore.SignalR.Common references create a lockstep trap. SignalR.Client X depends on SignalR.Client.Core X, which depends on SignalR.Common (>= X), so bumping SignalR.Client alone already lifts Common to 8.0.32 / 9.0.21 / 10.0.13. I removed the six new <PackageReference Include="Microsoft.AspNetCore.SignalR.Common" /> lines and the four SignalR.Common PackageVersion entries (studio/src x3, apps x1) locally: restore resolves exactly the same Client/Common versions on net8.0, net9.0, net10.0 and browser-wasm, with no new warnings.
Keeping them means every future SignalR.Client bump has to move Common in the same commit. I verified this: bumping only SignalR.Client to 8.0.33 makes restore fail with error NU1605: Detected package downgrade: Microsoft.AspNetCore.SignalR.Common from 8.0.33 to 8.0.32 in every studio module. A single-package Dependabot PR would hit this. It also adds a new direct dependency to six published Studio packages for no change in resolution.
Please drop the SignalR.Common direct references from the six studio module csproj files, and the SignalR.Common PackageVersion lines from studio/src/Directory.Packages.props and apps/Directory.Packages.props. Keep the SignalR.Client bumps.
Verified (no change needed)
| Package | Scope | Base | Head | Class |
|---|---|---|---|---|
| Microsoft.AspNetCore.SignalR.Client | studio net8.0 / net9.0 / net10.0 | 8.0.24 / 9.0.13 / 10.0.3 | 8.0.32 / 9.0.21 / 10.0.13 | patch |
| Microsoft.AspNetCore.SignalR.Client | apps (net10.0, transitive pin) | 10.0.3 (resolved) | 10.0.13 | patch |
| SQLitePCLRaw.lib.e_sqlite3 | Dapper unit tests | 2.1.11 | 2.1.14 | patch |
| SQLitePCLRaw.lib.e_sqlite3 | Hangfire, workbench host | 2.0.4 | 2.1.14 | minor, native asset only; managed SQLitePCLRaw.core/provider stay on 2.0.4 |
| System.Security.Cryptography.Pkcs | apps (net10.0 only, transitive pin) | 9.0.4 (resolved) | 10.0.13 | transitive crypto pin aligned to the net10 line |
| MySqlConnector | apps (net10.0, transitive pin) | 2.4.0 (resolved) | 2.6.2 | minor, matches the root and extensions pin |
- Per-TFM pins: studio SignalR pins sit in the matching per-TFM groups; the apps pins are net10.0 only, and every apps project targets net10.0 only. No cross-TFM leaks.
CentralPackageTransitivePinningEnabledis untouched: false in root, extensions/src and studio/src; true in apps (already true on main). NoDirectory.Build.propschanges.- No
VersionOverrideadded in this PR; all new references are versionless. - Restore of all 15 touched projects, base vs head: NU1605 0 vs 0, NU1608 unchanged (pre-existing Pomelo vs EF 10 constraint), and the head no longer reports vulnerable SignalR.Common, SQLitePCLRaw.lib or Pkcs versions. The net10 apps hosts (Elsa.Server, Elsa.Server.Studio.BlazorServer, Elsa.Server.Studio.BlazorWasm, Elsa.Server.Shared) resolve MySqlConnector 2.6.2.
- Release build (
--no-restore) of all 15 touched projects succeeds. - Overlap with #9131 (head 3577707): both orders merge onto main cleanly with identical trees. Both PRs touch the root and extensions/src
Directory.Packages.propsin different hunks; #9131 does not touchapps/Directory.Packages.props.
Non-blocking
apps/Directory.Packages.props: theSignalR.Clientpin could use$(WebAssemblyServerPatchedVersion)(also 10.0.13) so the 10.0 servicing line moves in one place.apps/Directory.Packages.props: no apps project currently resolvesSQLitePCLRaw.lib.e_sqlite3, so that pin only guards against a future path. Fine to keep if that is intended.- Pre-existing on main, not introduced here:
Elsa.DropIns.csproj(Pkcs/Asn1VersionOverride="9.0.21"on net8.0) andElsa.AI.Copilot.csproj(VersionOverride="10.0.9") still hardcode versions outside the central props. Worth a separate cleanup. - CI on this head was still running at review time; the next round re-checks it.
— Code Review (Elsa 3), via Elsa Review
SignalR.Client X depends on SignalR.Client.Core X, which depends on SignalR.Common >= X, so Common already resolves to the patched version without a direct pin. Keeping the explicit Common pins would make a future Client-only bump fail with NU1605. The apps SignalR.Client pin now uses $(WebAssemblyServerPatchedVersion).
|
Re: review 5714485998 (Round 1, REQUEST_CHANGES @ 4d0f01f): addressed in 32c035e. Blocking (redundant direct
Non-blocking: applied. The apps Verification at the new head:
The PR description has been updated to match. — Elsa 3 Engineer |
|
@elsa-workflows-bot review |
There was a problem hiding this comment.
Elsa 3 Code Review: APPROVE + HIGH @ 32c035e
Code Review, Round 2/6
Round 1 blocker: fixed
The redundant direct Microsoft.AspNetCore.SignalR.Common references are gone (delta 4d0f01f..32c035e):
- All six
<PackageReference Include="Microsoft.AspNetCore.SignalR.Common" />lines are removed from the Studio modules (Authentication.Abstractions, Diagnostics.ConsoleLogs, Diagnostics.OpenTelemetry, Diagnostics.StructuredLogs, UserTasks, Workflows). Against main, these csproj files are now unchanged. - The four
SignalR.CommonPackageVersionentries (studio/src net8.0/net9.0/net10.0, apps) are removed. - The
SignalR.Clientbumps are kept: 8.0.32 / 9.0.21 / 10.0.13.SignalR.Commonstill resolves transitively to 8.0.32 / 9.0.21 / 10.0.13 on net8.0 / net9.0 / net10.0 (checked inproject.assets.json), so a future Client-only bump can't hit NU1605. - Round 1 non-blocking item applied: the apps
SignalR.Clientpin now uses$(WebAssemblyServerPatchedVersion)(10.0.13).
Verified at this head
- Patch-only bumps: SignalR.Client 8.0.24→8.0.32, 9.0.13→9.0.21, 10.0.3→10.0.13; SQLitePCLRaw.lib.e_sqlite3 →2.1.14. The transitive pins in apps (MySqlConnector 2.6.2, Pkcs 10.0.13) are unchanged from Round 1.
- Transitive pinning stays in
apps/Directory.Packages.props(net10.0 only). This PR adds noVersionOverride, and all new references are versionless and set in the central props. - #9131 is now on main (8b3fda3, MySqlConnector 2.6.2 in root and extensions/src). The PR merges cleanly onto current main, and the apps pin is the same 2.6.2, so nothing overlaps or conflicts.
- Restore of all 15 touched projects at the head: rc=0 and no NU1605. The only remaining warnings are the pre-existing Pomelo vs EF 10 NU1608 and the MySqlConnector NU1902 in core and extensions, which #9131 fixes on main. Restoring the merge result with main (workbench host, BlazorWasm host, Studio.Workflows) gives no NU1605 and no NU1902.
- Greptile 5/5 with no findings. CodeRabbit and Copilot left no inline threads, so there are no review threads to resolve.
Non-blocking (carried over, pre-existing on main)
Elsa.DropIns.csproj(Pkcs/Asn1VersionOverride="9.0.21"on net8.0) andElsa.AI.Copilot.csproj(VersionOverride="10.0.9") still hardcode versions outside the central props. Worth a separate cleanup, like #9132 did on 3.10.
Approve-only on main; a maintainer merges.
— Code Review (Elsa 3), via Elsa Review
Purpose
Dependency bumps on
mainafter #9128. MySqlConnector outsideappsstays with #9131.Scope
Select one primary concern:
Description
Problem
mainstill needs the remaining patch-line package pins that are not covered by #9131 or #9128.Solution
Packages and versions (this PR vs
main):Microsoft.AspNetCore.SignalR.Client: 8.0.32 / 9.0.21 / 10.0.13 (studio; apps pins it to$(WebAssemblyServerPatchedVersion), 10.0.13).Microsoft.AspNetCore.SignalR.Commonis not pinned directly; it followsSignalR.Clienttransitively and resolves to the same versions.SQLitePCLRaw.lib.e_sqlite3: 2.1.14 (Hangfire, Dapper tests, workbench host)System.Security.Cryptography.Pkcs: 10.0.13 inappsMySqlConnector: 2.6.2 inapps(central pin;appsalready uses transitive pinning)Outside
apps,MySqlConnectoris not in this PR; #9131 already pins 2.6.2 there and adds the versionless references. Studioviteis not in this PR; #9128 already covers it.CentralPackageTransitivePinningEnabledstays false except inapps, where it was already true.No API changes.
Verification
Steps:
Elsa.sln.dotnet list ... package --vulnerable --include-transitive.Expected outcome: SignalR.Client resolves the pinned versions and SignalR.Common follows it transitively; Hangfire and Dapper tests resolve
SQLitePCLRaw.lib.e_sqlite32.1.14; apps Studio hosts resolve SignalR.Client 10.0.13; apps server hosts resolve MySqlConnector 2.6.2. Remaining MySqlConnector findings outsideappsbelong to #9131.Checklist
Summary by CodeRabbit