Skip to content

Remove internal malware infrastructure/checks #13647

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

Merged
merged 11 commits into from
May 23, 2023

Conversation

ewdurbin
Copy link
Member

@ewdurbin ewdurbin commented May 11, 2023

This languished.

There was some discussion of re-using this infrastructure for other use cases such as scanning uploads for secrets to report them to providers (similar to GitHub token scanning).

I propose #13596 combined with some form of "what's new!" feed to build out a more generic interface for internal and external "third parties" to perform these kinds of scans and report them to PyPI.

Closes #12412.

@ewdurbin ewdurbin requested a review from a team as a code owner May 11, 2023 12:18
@ewdurbin ewdurbin force-pushed the remove_internal_malware_infrastructure branch 4 times, most recently from cb251c3 to a40f739 Compare May 11, 2023 12:36
@ewdurbin ewdurbin force-pushed the remove_internal_malware_infrastructure branch from a40f739 to c7ac1dc Compare May 11, 2023 12:37
Copy link
Member

@miketheman miketheman left a comment

Choose a reason for hiding this comment

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

nitpick: ‏‏Looks to be a typo, triggered my malware searches:

Manage the Warehouse Malware Checks.

suggestion: ‏Remove leftovers from tests/common/checks/__init__.py and associated classes/modules

thoughts: ‏I didn't review the removed modules in depth, since they were removed in their entirety. Same applies for most of the test code. I figured with other protections in place, there's not a high degree of potential breakage. I focused predominantly on the integration parts of this code and the rest of the stack.

Other than a few smaller items included inline and above, this should be a relatively simple re-review once addressed.

@ewdurbin ewdurbin requested a review from miketheman May 19, 2023 13:50
Copy link
Member

@miketheman miketheman left a comment

Choose a reason for hiding this comment

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

I added a commit removing test config I had missed earlier.

@ewdurbin
Copy link
Member Author

Gonna wait for a 👍🏼 from @di.

@ewdurbin ewdurbin requested a review from di May 19, 2023 14:06
Copy link
Member

@di di left a comment

Choose a reason for hiding this comment

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

I think it's clear that supporting 3rd party reports from trusted reporters is a better path towards actually removing malware here.

@ewdurbin ewdurbin enabled auto-merge (squash) May 23, 2023 13:14
@ewdurbin ewdurbin merged commit 319cbc0 into main May 23, 2023
@ewdurbin ewdurbin deleted the remove_internal_malware_infrastructure branch May 23, 2023 13:17
.. _celery crontab: https://docs.celeryq.dev/en/latest/reference/celery.schedules.html#celery.schedules.crontab
.. _prepare classmethod: https://github.com/pypi/warehouse/blob/main/warehouse/malware/checks/base.py
.. _MalwareVerdict model: https://github.com/pypi/warehouse/blob/main/warehouse/malware/models.py
.. |CheckLifecycle| image:: ../_static/check-lifecycle.png
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the two images referenced here were not removed by the PR

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.

Repurpose 'malware' checks / YARA scanning for secret scanning
4 participants