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

Add backend for pip-audit #16288

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add backend for pip-audit #16288

wants to merge 3 commits into from

Conversation

jyggen
Copy link
Member

@jyggen jyggen commented Jul 24, 2022

This PR adds a backend for pip-audit during the check goal.

Fixes #13770.

)
requirements_digest = await Get(
Digest,
CreateDigest([FileContent("requirements.txt", requirements_str.encode())]),
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if this file will cause collisions in Pants' cache when multiple partitions are used or if the build root has a file with the same name?

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks so much for prototyping this! Extremely useful functionality.

This PR raises some big design questions for Pants:

  1. Should this live in check or lint? According to Slack discussion led by @thejcannon and me last week, we think that the definition of check should likely be "lightest form of compilation, or type checking". Everything else is lint. So, this should be lint technically -- but that's annoying to run this heavy-weight thing every time you do ./pants lint ::...
  2. This runs on individual targets, and it will also leave off transitive dependencies + hashes like you mention. Really, what I think we want to run on is the entire lockfile for a resolve.

The simplest way we could do this is a goal like experimental-pip-audit, which similar to generate-lockfiles does not take Specs and instead has a --resolve option (with default to all).

But we should decide if we want pip-audit to live instead in a goal like lint or check? We could so something like "running on one target belonging to a resolve -> check the whole resolve".

Note that others have requested things like #15723. There is demand for a "resolve-specific lint/check interface"

src/python/pants/backend/python/check/pip_audit/rules.py Outdated Show resolved Hide resolved
class SkipPipAuditField(BoolField):
alias = "skip_pip_audit"
default = False
help = "If true, don't run pip-audit on this target's code."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
help = "If true, don't run pip-audit on this target's code."
help = "If true, don't run pip-audit on this requirement."

@thejcannon
Copy link
Member

Can we maybe look into optional goals? So a goal only appears if you have installed a relevant plugin?

In that case I could see an audit or similar goal, which doesn't show up unless something is installed.

It's not a fast code-related-tool goal, but a heavier metadata-and-friends goal.

Audit your deps, maybe do expensive code scanning, etc...

@Eric-Arellano
Copy link
Contributor

Can we maybe look into optional goals? So a goal only appears if you have installed a relevant plugin?

We have that already :)

@classmethod
def activated(cls, union_membership: UnionMembership) -> bool:
return TestFieldSet in union_membership

@thejcannon
Copy link
Member

there it is

[ci skip-rust]

[ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add vulnerability audit support using pip-audit
4 participants