[AppSec] Add packages.lock.json files for dependency locking#7725
[AppSec] Add packages.lock.json files for dependency locking#7725aikido-autofix[bot] wants to merge 3 commits into
Conversation
aikido-autofix
Bot
requested review from
amorask-bitwarden,
dani-garcia,
ike-kottlowski and
mimartin12
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7725 +/- ##
==========================================
+ Coverage 60.55% 65.01% +4.46%
==========================================
Files 2142 2145 +3
Lines 94847 95028 +181
Branches 8487 8504 +17
==========================================
+ Hits 57430 61786 +4356
+ Misses 35401 31126 -4275
- Partials 2016 2116 +100 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Testing Aikido and it doesn't have a draft mode -- hang tight! |
…props Apply the lockfile-restore setting solution-wide via Directory.Build.props instead of repeating it in every csproj. Inheritance covers the whole solution, so the setting now applies to all current and future projects uniformly.
🤖 Bitwarden Claude Code ReviewOverall Assessment: REQUEST CHANGES This PR adds Code Review Details
Dependency ChangesNo package version changes — this PR only adds lock files and a solution-wide MSBuild property. |
withinfocus
removed request for
a team,
amorask-bitwarden,
dani-garcia,
ike-kottlowski and
mimartin12
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| <Nullable Condition="'$(Nullable)' == '' and '$(IsTestProject)' != 'true'">enable</Nullable> | ||
| <TreatWarningsAsErrors Condition="'$(TreatWarningsAsErrors)' == ''">true</TreatWarningsAsErrors> | ||
| <NuGetAuditLevel>critical</NuGetAuditLevel> | ||
| <RestorePackagesWithLockFile>true</RestorePackagesWithLockFile> |
There was a problem hiding this comment.
packages.lock.json and will not gain reproducible-restore or deterministic dependency-scan coverage.
Details and fix
RestorePackagesWithLockFile=true here applies to every project that inherits from this Directory.Build.props (no Directory.Build.targets or per-project override exists). The PR added 49 lock files but the repo contains 57 csproj files; the following 8 projects have no packages.lock.json:
src/Admin/Admin.csprojsrc/Identity/Identity.csprojsrc/Events/Events.csprojsrc/EventsProcessor/EventsProcessor.csprojbitwarden_license/src/Scim/Scim.csprojutil/RustSdk/RustSdk.csprojutil/SeederApi/SeederApi.csprojutil/Server/Server.csproj
A NuGet lock file locks the direct + transitive graph of its own project — referencing Core from a covered project does not transitively lock Admin's closure. The PR description's stated benefits ("Reproducible builds", "Prevents supply chain attacks by pinning specific dependency versions", "Deterministic child dependency scanning") are therefore not realized for these projects, several of which are production services (Admin, Identity, Events, EventsProcessor, Scim).
In addition, with the setting enabled solution-wide, the next dotnet restore on a clean checkout will silently generate these missing lock files locally, leaving the working tree dirty.
Fix: run dotnet restore against each of the 8 projects and commit the generated packages.lock.json files alongside the 49 already added.
There was a problem hiding this comment.
Yep, Aikido Endpoint Protection ...
There was a problem hiding this comment.
@claude apply this suggestion by running dotnet restore against the entire solution
There was a problem hiding this comment.
Claude finished @withinfocus's task in 2m 36s —— View job
Cannot Run dotnet restore in This Environment
I attempted to run dotnet restore bitwarden-server.slnx to generate the 8 missing packages.lock.json files, but the dotnet binary is not in the allowed tool list for this Claude Code action. Each invocation returns This command requires approval and the runtime blocks it.
- Verify the 8 missing projects (
Admin,Identity,Events,EventsProcessor,Scim,RustSdk,SeederApi,Server) -
Run— blocked:dotnet restoreagainst the solutiondotnetnot inallowed_tools - Verify the 8 missing
packages.lock.jsonfiles were generated - Commit and push the new lock files
How to unblock:
Either:
-
Run locally and push (recommended — produces hash-verified lock files):
git fetch origin fix/security-dotnet-40388333-pt2b git checkout fix/security-dotnet-40388333-pt2b dotnet restore bitwarden-server.slnx git add src/Admin/packages.lock.json src/Identity/packages.lock.json \ src/Events/packages.lock.json src/EventsProcessor/packages.lock.json \ bitwarden_license/src/Scim/packages.lock.json \ util/RustSdk/packages.lock.json util/SeederApi/packages.lock.json util/Server/packages.lock.json git commit -m "fix(security): add missing packages.lock.json files" git push -
Update the reusable workflow at
bitwarden/gh-actions/.github/workflows/_respond.ymlto extendallowed_tools(orclaude_args) withBash(dotnet restore:*), then re-run@claude apply this suggestion ….
Lock files cannot be hand-authored — they contain content hashes from the NuGet feed that only the restore process can compute correctly, so I can't substitute another approach from here.
· Branch: fix/security-dotnet-40388333-pt2b
| <Nullable Condition="'$(Nullable)' == '' and '$(IsTestProject)' != 'true'">enable</Nullable> | ||
| <TreatWarningsAsErrors Condition="'$(TreatWarningsAsErrors)' == ''">true</TreatWarningsAsErrors> | ||
| <NuGetAuditLevel>critical</NuGetAuditLevel> | ||
| <RestorePackagesWithLockFile>true</RestorePackagesWithLockFile> |
There was a problem hiding this comment.
packages.lock.json files are committed as empty stubs ("net10.0": {}) — no dependencies are actually locked.
Details and fix
The latest commit ("Try to force a lock generation, at least partially") added lock files to the projects called out in the earlier review, but the resulting files contain no locked dependencies:
src/Admin/packages.lock.json(5 lines)src/Identity/packages.lock.json(5 lines)src/Events/packages.lock.json(5 lines)src/EventsProcessor/packages.lock.json(5 lines)bitwarden_license/src/Scim/packages.lock.json(5 lines)util/RustSdk/packages.lock.json(5 lines)util/SeederApi/packages.lock.json(5 lines)util/Server/packages.lock.json(5 lines)src/Sql/packages.lock.json(5 lines) — new
Compare with siblings that were properly restored: src/Api/packages.lock.json is 1,238 lines, src/SharedWeb/packages.lock.json is 1,398 lines, bitwarden_license/src/Sso/packages.lock.json is 1,202 lines. All of the affected projects pull in Microsoft.NET.Sdk.Web and/or Bitwarden.Server.Sdk plus several ProjectReferences, so their lock files should contain hundreds of resolved entries — not zero.
With RestorePackagesWithLockFile=true set solution-wide and these stubs in place, the next clean restore will either regenerate them (leaving the working tree dirty) or, in CI with --locked-mode, fail because the actual dependency graph does not match the empty lock. The PR description's stated benefits (reproducible builds, supply chain pinning, deterministic dependency scanning) are not realized for these projects — five of which are production services (Admin, Identity, Events, EventsProcessor, Scim).
Separately, src/Sql/Sql.sqlproj is a SQL Database project (Microsoft.Build.Sql SDK) with no PackageReference items — it does not participate in NuGet lock file generation. Adding src/Sql/packages.lock.json is incorrect and should be removed.
Fix: regenerate the eight .csproj lock files via a real dotnet restore (in an environment that can resolve the full graph for each project — Admin/Scim need the OSS configuration consideration since their conditional ProjectReferences depend on it) and delete src/Sql/packages.lock.json.
|
withinfocus
changed the title
1 participant
.NET Lockfile Creation
This PR adds packages.lock.json files to ensure reproducible builds and secure dependency management.
Benefits:
The lockfiles were generated using
dotnet restorecommand.