Skip to content

[PM-34828] Fix BW lite sqlite bulk import failure for org imports#7468

Open
mimartin12 wants to merge 4 commits intomainfrom
PM-34828-lite-sqlite-bulk-import
Open

[PM-34828] Fix BW lite sqlite bulk import failure for org imports#7468
mimartin12 wants to merge 4 commits intomainfrom
PM-34828-lite-sqlite-bulk-import

Conversation

@mimartin12
Copy link
Copy Markdown
Contributor

@mimartin12 mimartin12 commented Apr 14, 2026

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-34828

📔 Objective

Fixes a 500 error when logging in with an org account and when importing a vault on SQLite environments.

Cause

EventRepository.CreateManyAsync and two cases of CipherRepository.CreateAsync call
LinqToDB's BulkCopyAsync. When called against a SQLite context, LinqToDB auto-detects which SQLite
adapter to use by checking for System.Data.SQLite.dll or Microsoft.Data.Sqlite.dll on disk. Since the
server publishes as a single-file bundle (PublishSingleFile=true), managed DLLs are embedded in the
executable and not present on disk (example).

This surfaced after a change in the build process for the Bitwarden Lite container, which now pulls directly from our existing built containers.
bitwarden/self-host@08922d4

Changes in this PR

Added IsSqlite() guards to three methods to route SQLite through EF Core's AddRangeAsync/SaveChangesAsync instead of LinqToDB's BulkCopyAsync:

  • EventRepository.CreateManyAsync
  • CipherRepository.CreateAsync(userId, ciphers, folders) — personal vault import with folders
  • CipherRepository.CreateAsync(ciphers, collections, collectionCiphers, collectionUsers) — org vault import with collections

📸 Screenshots

@mimartin12 mimartin12 added the ai-review Request a Claude code review label Apr 14, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 14, 2026

Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR extends the SQLite BulkCopyAsync workaround to the remaining call sites: EventRepository.CreateManyAsync and both CipherRepository.CreateAsync overloads for personal and organization vault imports. The approach routes SQLite through EF Core's AddRangeAsync with a single trailing SaveChangesAsync while leaving BulkCopyAsync intact for MSSQL, PostgreSQL, and MySQL — matching the pattern established in prior commits (559452ec1, 1a74cd29f). The missing await dbContext.SaveChangesAsync(); in the non-SQLite else branch of EventRepository.CreateManyAsync (flagged by reviewer Banrion) was addressed in commit 0c7941e52. No unguarded BulkCopyAsync calls remain.

Code Review Details

No findings.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 14, 2026

Logo
Checkmarx One – Scan Summary & Details67072b3b-44dc-4890-bcce-f8f132510d38


New Issues (2) Checkmarx found the following issues in this Pull Request
# Severity Issue Source File / Package Checkmarx Insight
1 MEDIUM CSRF src/Api/AdminConsole/Controllers/GroupsController.cs: 289
detailsMethod at line 289 of /src/Api/AdminConsole/Controllers/GroupsController.cs gets a parameter from a user request from orgUserId. This parameter ...
Attack Vector
2 MEDIUM CSRF src/Api/Vault/Controllers/CiphersController.cs: 287
detailsMethod at line 287 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows t...
Attack Vector

Fixed Issues (2) Great job! The following issues were fixed in this Pull Request
Severity Issue Source File / Package
MEDIUM CSRF src/Api/AdminConsole/Controllers/GroupsController.cs: 275
MEDIUM CSRF src/Identity/Controllers/AccountsController.cs: 138

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 89.58333% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.61%. Comparing base (7c4fa75) to head (0c7941e).
⚠️ Report is 85 commits behind head on main.

Files with missing lines Patch % Lines
...tityFramework/Dirt/Repositories/EventRepository.cs 44.44% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7468      +/-   ##
==========================================
+ Coverage   58.56%   63.61%   +5.05%     
==========================================
  Files        2063     2077      +14     
  Lines       91188    91881     +693     
  Branches     8123     8182      +59     
==========================================
+ Hits        53401    58450    +5049     
+ Misses      35878    31412    -4466     
- Partials     1909     2019     +110     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud
Copy link
Copy Markdown

@mimartin12 mimartin12 marked this pull request as ready for review April 14, 2026 16:11
@mimartin12 mimartin12 requested review from a team as code owners April 14, 2026 16:11
@mimartin12 mimartin12 requested a review from jengstrom-bw April 14, 2026 16:11
jengstrom-bw
jengstrom-bw previously approved these changes Apr 14, 2026
@mimartin12 mimartin12 enabled auto-merge (squash) April 14, 2026 20:12
Copy link
Copy Markdown
Member

@justindbaur justindbaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm cautious about this change, there could be many other places that are broken and it forces us to do more and more branching when the goal is that we don't have to branch for EF unless absolutely needed.

I wonder if we could configure the provider explicitly like this:

diff --git a/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs b/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs
index e2227167a6..af66106e81 100644
--- a/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs
+++ b/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs
@@ -1,4 +1,5 @@
-using Bit.Core.AdminConsole.Repositories;
+using System.Diagnostics;
+using Bit.Core.AdminConsole.Repositories;
 using Bit.Core.Auth.Repositories;
 using Bit.Core.Billing.Organizations.Repositories;
 using Bit.Core.Billing.Providers.Repositories;
@@ -25,6 +26,7 @@ using Bit.Infrastructure.EntityFramework.Repositories;
 using Bit.Infrastructure.EntityFramework.SecretsManager.Repositories;
 using Bit.Infrastructure.EntityFramework.Tools.Repositories;
 using Bit.Infrastructure.EntityFramework.Vault.Repositories;
+using LinqToDB;
 using LinqToDB.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.DependencyInjection;
@@ -40,9 +42,6 @@ public static class EntityFrameworkServiceCollectionExtensions
             throw new Exception($"Database provider type {provider} was selected but no connection string was found.");
         }
 
-        // TODO: We should move away from using LINQ syntax for EF (TDL-48).
-        LinqToDBForEFTools.Initialize();
-
         services.AddAutoMapper(typeof(UserRepository));
         services.AddDbContext<DatabaseContext>(options =>
         {
@@ -65,6 +64,35 @@ public static class EntityFrameworkServiceCollectionExtensions
             {
                 options.UseSqlServer(connectionString);
             }
+
+            // Configure LinqToDB with the types of drivers we use for each DB type, this allows to us avoid it searching
+            // for assemblies and gives us the ability to publish as a single file.
+            options.UseLinqToDB(linq =>
+            {
+                linq.AddCustomOptions(linqOptions =>
+                {
+                    if (provider == SupportedDatabaseProviders.Postgres)
+                    {
+                        return linqOptions.UsePostgreSQL();
+                    }
+                    else if (provider == SupportedDatabaseProviders.MySql)
+                    {
+                        return linqOptions.UseMySqlConnector();
+                    }
+                    else if (provider == SupportedDatabaseProviders.Sqlite)
+                    {
+                        return linqOptions.UseSQLiteMicrosoft();
+                    }
+                    else if (provider == SupportedDatabaseProviders.SqlServer)
+                    {
+                        return linqOptions.UseSqlServer(provider: LinqToDB.DataProvider.SqlServer.SqlServerProvider.MicrosoftDataSqlClient);
+                    }
+
+                    Debug.Fail("Unknown database provider.");
+
+                    return linqOptions;
+                });
+            });
         });
     }

Hopefully by manually telling it the provider we avoid having to have it search for it. If this doesn't work and we need to start branching then we especially need to make sure that these differences are now covered by tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants