Skip to content
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

feat(python): add support for uv #8080

Merged
merged 18 commits into from
Dec 19, 2024
Merged

Conversation

nikpivkin
Copy link
Contributor

@nikpivkin nikpivkin commented Dec 11, 2024

Description

This PR adds support for retrieving the dependency list from uv.lock of the uv package manager.

Example:

❯ uv init
❯ uv add django==4.2.16
❯ ./trivy fs uv.lock -d --dependency-tree
uv.lock (uv)

Total: 2 (UNKNOWN: 0, LOW: 0, MEDIUM: 1, HIGH: 1, CRITICAL: 0)

┌─────────┬────────────────┬──────────┬────────┬───────────────────┬───────────────────────┬────────────────────────────────────────────────────────┐
│ Library │ Vulnerability  │ Severity │ Status │ Installed Version │     Fixed Version     │                         Title                          │
├─────────┼────────────────┼──────────┼────────┼───────────────────┼───────────────────────┼────────────────────────────────────────────────────────┤
│ django  │ CVE-2024-53908 │ HIGH     │ fixed  │ 4.2.16            │ 5.0.10, 5.1.4, 4.2.17 │ django: Potential SQL injection in HasKey(lhs, rhs) on │
│         │                │          │        │                   │                       │ Oracle                                                 │
│         │                │          │        │                   │                       │ https://avd.aquasec.com/nvd/cve-2024-53908             │
│         ├────────────────┼──────────┤        │                   ├───────────────────────┼────────────────────────────────────────────────────────┤
│         │ CVE-2024-53907 │ MEDIUM   │        │                   │ 5.1.4, 4.2.17, 5.0.10 │ django: Potential denial-of-service in                 │
│         │                │          │        │                   │                       │ django.utils.html.strip_tags()                         │
│         │                │          │        │                   │                       │ https://avd.aquasec.com/nvd/cve-2024-53907             │
└─────────┴────────────────┴──────────┴────────┴───────────────────┴───────────────────────┴────────────────────────────────────────────────────────┘

Dependency Origin Tree (Reversed)
=================================
uv.lock
└── [email protected], (UNKNOWN: 0, LOW: 0, MEDIUM: 1, HIGH: 1, CRITICAL: 0)

Related issues

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@nikpivkin
Copy link
Contributor Author

nikpivkin commented Dec 12, 2024

@knqyf263 @DmitriyLewen Many package managers are now moving towards PEP-735 support, which introduces the concept of dependency groups, and uv is no exception. I have now implemented skipping all groups except from the dependencies field, as groups can include dependencies for testing, linting or building documentation. Here's what it might look like:

[dependency-groups]
test = ["pytest", "coverage"]
docs = ["sphinx", "sphinx-rtd-theme"]
typing = ["mypy", "types-requests"]

The uv documentation mentions dev dependencies separately from dependency groups, but dev dependencies just belong to a dev group. It also says:

Eventually, the dev-dependencies field will be deprecated and removed.

Is it worth skipping the dev group exclusively?

BTW, I did some tests with poetry (1.8.5). I added a dev dependency poetry add pytest --dev which added to the dev group:

[tool.poetry.group.dev.dependencies]
pytest = "^8.3.4"

But Trivy didn't skip it because now the package in the lockfile doesn't contain the category field as before, based on which Trivy determines the dev dependencies. This is unusual because the lock version remains unchanged and matches the one used in our tests: lock-version = '2.0'.

❯ trivy fs . -f json --list-all-pkgs | grep pytest
2024-12-12-12T17:03:28+06:00 INFO [vuln] Vulnerability scanning is enabled
2024-12-12-12T17:03:28+06:00 INFO [secret] Secret scanning is enabled
2024-12-12-12T17:03:28+06:00 INFO [secret] INFO [secret] If your scanning is slow, please try '--scanners vuln' to disable secret scanning
2024-12-12-12T17:03:28+06:00 INFO [secret] Please see also https://aquasecurity.github.io/trivy/v0.58/docs/scanner/secret#recommendation for faster secret detection
2024-12-12-12T17:03:03:28+06:00 INFO Number of language-specific files num=1
2024-12-12-12T17:03:28+06:00 INFO [poetry] Detecting vulnerabilities...
          “ID": ‘[email protected]’,
          “Name": ‘pytest’,
            “PURL": ‘pkg:pypi/[email protected]’,
```

@nikpivkin nikpivkin marked this pull request as ready for review December 12, 2024 11:20
Copy link
Contributor

@DmitriyLewen DmitriyLewen left a comment

Choose a reason for hiding this comment

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

Is it worth skipping the dev group exclusively?

I think we also need to skip dev group.

But Trivy didn't skip it because now the package in the lockfile doesn't contain the category field as before

Create new issue for this case. please

@nikpivkin i left comments, take a look, please

pkg/dependency/parser/python/uv/parser.go Outdated Show resolved Hide resolved
pkg/dependency/parser/python/uv/parser.go Outdated Show resolved Hide resolved
pkg/dependency/parser/python/uv/parser.go Outdated Show resolved Hide resolved
pkg/dependency/parser/python/uv/parser.go Outdated Show resolved Hide resolved
pkg/dependency/parser/python/uv/parser.go Outdated Show resolved Hide resolved
pkg/dependency/parser/python/uv/parser.go Outdated Show resolved Hide resolved
pkg/dependency/parser/python/uv/parser.go Outdated Show resolved Hide resolved
pkg/fanal/types/const.go Show resolved Hide resolved
integration/repo_test.go Outdated Show resolved Hide resolved
@nikpivkin
Copy link
Contributor Author

Create new issue for this case. please

Created #8096

@nikpivkin
Copy link
Contributor Author

I think we also need to skip dev group.

The dev group is already skipped. I wanted to discuss what to do with other groups such as test or docs that inherently contain dev dependencies?

Copy link
Contributor

@DmitriyLewen DmitriyLewen left a comment

Choose a reason for hiding this comment

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

looks good.
left small comments

pkg/dependency/parser/python/uv/parse.go Outdated Show resolved Hide resolved
pkg/dependency/parser/python/uv/parse.go Outdated Show resolved Hide resolved
pkg/dependency/parser/python/uv/parse_testcase.go Outdated Show resolved Hide resolved
pkg/fanal/analyzer/language/python/uv/uv.go Outdated Show resolved Hide resolved
pkg/fanal/analyzer/language/python/uv/uv.go Outdated Show resolved Hide resolved
pkg/dependency/parser/python/uv/parse.go Outdated Show resolved Hide resolved
@DmitriyLewen
Copy link
Contributor

The dev group is already skipped. I wanted to discuss what to do with other groups such as test or docs that inherently contain dev dependencies?

Does the dependencies field contain these dependencies?
If so, is there a marker for them

@nikpivkin
Copy link
Contributor Author

dependencies does not contain these dependencies.

Here's an example:

[dependency-groups]
test = ["pytest", "coverage"]
docs = ["sphinx", "sphinx-rtd-theme"]
typing = ["mypy", "types-requests"]

@DmitriyLewen
Copy link
Contributor

IIUC your logic skips these (doc, test, development etc) dependencies .
I think we can leave this logic and wait for feedback from users and then think about it if users need these dependencies.

Signed-off-by: nikpivkin <[email protected]>
Signed-off-by: nikpivkin <[email protected]>
Copy link
Contributor

@DmitriyLewen DmitriyLewen left a comment

Choose a reason for hiding this comment

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

LGTM

cc. @knqyf263

Copy link
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

LGTM. Left small comments.

pkg/dependency/parser/python/uv/parse.go Outdated Show resolved Hide resolved
pkg/dependency/parser/python/uv/parse_testcase.go Outdated Show resolved Hide resolved
pkg/dependency/parser/python/uv/parse.go Show resolved Hide resolved
@nikpivkin nikpivkin requested a review from knqyf263 December 18, 2024 15:29
@knqyf263
Copy link
Collaborator

I think we also need to skip dev group.

The dev group is already skipped. I wanted to discuss what to do with other groups such as test or docs that inherently contain dev dependencies?

Is this about pyproject.toml, not uv.lock? I didn't find the code parsing pyproject.toml in this PR. What am I missing? I'm sorry I was not following the discussion.

@nikpivkin
Copy link
Contributor Author

nikpivkin commented Dec 19, 2024

@knqyf263 This is about uv.lock. It contains information about all dependencies and groups of the root package. Example:

[package.dev-dependencies]
docs = [
    { name = "mkdocs" },
]
test = [
    { name = "pytest" },
]

For uv we only need to parse the lockfile, since it contains all the information we need.

@knqyf263
Copy link
Collaborator

Isn't dependency-groups you mentioned above about pyproject.toml?
#8080 (comment)

Does it mean dependency-groups are copied into uv.lock?

@nikpivkin
Copy link
Contributor Author

That's right, it's the same dependency groups

@knqyf263
Copy link
Collaborator

Thanks for clarifying. Is it possible to identify the dependency graph for development dependencies? We can do that in another PR, though.
#8080 (comment)

@nikpivkin
Copy link
Contributor Author

Yes, I've already started working on this so we can implement support in this PR.

@knqyf263
Copy link
Collaborator

It's better to keep the PR small. Please let me merge this one.

@knqyf263 knqyf263 added this pull request to the merge queue Dec 19, 2024
@nikpivkin
Copy link
Contributor Author

@knqyf263 OK

Merged via the queue into aquasecurity:main with commit c4a4a5f Dec 19, 2024
17 checks passed
@nikpivkin nikpivkin deleted the feat/py-uv branch December 19, 2024 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(python): Add support for astral UV
3 participants