-
Notifications
You must be signed in to change notification settings - Fork 197
chore: Added packages.lock to csharp sdk #1620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Generated a project locklife for the csharp sdk to lock down package dependencies. This was done to remediate an internal vulnerability scan ticket and to allow Google to manage our csharp supply chain vulnerabilities. Currently afaict, we dont have any CI running for csharp given its community supported. So there's nothing in the repo that relies on the lock file. The lockfile shouldn't need to be updated when generating a csharp sdk as we only generate api/model definitions which shouldn't depend on the csharp sdk's packages.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @jeremytchang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a packages.lock.json file for the C# SDK, which is a great step for reproducible builds and improving supply chain security. The changes to enable package locking are correct.
My review comments highlight that many of the locked dependencies are significantly outdated. Since a key motivation for this change is to address security vulnerabilities, it's crucial to update these packages to their latest stable versions. This will help mitigate existing vulnerabilities and prevent future ones.
Additionally, to maintain the integrity of the lock file, I recommend adding a CI step to verify that it is in sync with the project's dependencies. This can be done by running dotnet restore --locked-mode, which will fail the build if the lock file is out of date. This ensures that any changes to dependencies are intentionally committed to the lock file.
| "coverlet.collector": { | ||
| "type": "Direct", | ||
| "requested": "[1.2.0, )", | ||
| "resolved": "1.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "Microsoft.NET.Test.Sdk": { | ||
| "type": "Direct", | ||
| "requested": "[16.5.0, )", | ||
| "resolved": "16.5.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The resolved version for Microsoft.NET.Test.Sdk is 16.5.0, which was released in early 2020 and is outdated. Using old package versions can expose the project to known security vulnerabilities and bugs. To improve the security posture, please consider updating the direct dependencies in csharp.csproj to their latest stable versions. For example, the latest version of Microsoft.NET.Test.Sdk is 17.9.0. Updating this and other direct dependencies will also bring in updated transitive dependencies.
| "xunit": { | ||
| "type": "Direct", | ||
| "requested": "[2.4.1, )", | ||
| "resolved": "2.4.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "xunit.runner.visualstudio": { | ||
| "type": "Direct", | ||
| "requested": "[2.4.2, )", | ||
| "resolved": "2.4.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Used personal account by accident. Closing PR and will open a new one. |
Generated a project lock file for the csharp sdk to lock down package dependencies.
This was done to remediate an internal vulnerability scan ticket and to allow Google to scan/manage our csharp supply chain vulnerabilities.
Currently afaict, we dont have any CI running for csharp given its community supported, so there's nothing in the repo that relies on the lock file. The lockfile shouldn't need to be updated when generating a csharp sdk as we only generate api/model definitions which shouldn't depend on the csharp sdk's packages. Example generated PR: https://github.com/looker-open-source/sdk-codegen/pull/1612/files