-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat(pep621): surface uv.lock transitive deps for osvVulnerabilityAlerts #44262
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
64b938b
99979a6
9a2579f
6284112
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -219,17 +219,29 @@ export const PdmLockfile = Toml.pipe( | |
| ) | ||
| .transform((lock) => ({ lock })); | ||
|
|
||
| export interface UvLockedPackage { | ||
| version: string; | ||
| // The index a package was resolved from, if any. Absent for the workspace | ||
| // root and for virtual/editable/path/git packages, which cannot be | ||
| // remediated by `uv lock --upgrade-package`. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use jsdoc comment
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| registryUrl?: string; | ||
| } | ||
|
|
||
| export const UvLockfile = Toml.pipe( | ||
| z.object({ | ||
| package: LooseArray( | ||
| z.object({ | ||
| name: z.string(), | ||
| version: z.string(), | ||
| source: z.object({ registry: z.string() }).partial().optional(), | ||
| }), | ||
| ), | ||
| }), | ||
| ).transform(({ package: pkg }) => | ||
| Object.fromEntries( | ||
| pkg.map(({ name, version }): [string, string] => [name, version]), | ||
| pkg.map(({ name, version, source }): [string, UvLockedPackage] => [ | ||
| name, | ||
| { version, registryUrl: source?.registry }, | ||
| ]), | ||
| ), | ||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1127,6 +1127,9 @@ describe('workers/repository/process/vulnerabilities', () => { | |
| matchPackageNames: ['stdlib'], | ||
| matchCurrentVersion: '1.7.5', | ||
| allowedVersions: '>= 1.7.6', | ||
| // remediation must override a disabled dependency, e.g. a transitive | ||
| // dep surfaced from a lockfile | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. jsdoc comment
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| enabled: true, | ||
| isVulnerabilityAlert: true, | ||
| }, | ||
| ]); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -600,6 +600,10 @@ export class Vulnerabilities { | |
| matchCurrentVersion: depVersion, | ||
| versioning, | ||
| allowedVersions: fixedVersion, | ||
| // Remediate even when updates are otherwise disabled for the dependency, | ||
| // e.g. transitive deps surfaced from a lockfile. Consistent with how | ||
| // vulnerability alerts already override schedule and minimumReleaseAge. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. jsdoc comment
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| enabled: true, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I can see how this can be regarded as too broad. One way to make this cleaner could be to add a dedicated |
||
| isVulnerabilityAlert: true, | ||
| vulnerabilitySeverity: severityDetails.severityLevel, | ||
| prBodyNotes: this.generatePrBodyNotes(vulnerability, affected), | ||
|
|
||
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.
only used once, inline this
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.
6284112