PEP 694: Amend scanning in staged releases and add legacy API changes to use staged releases - #5070
PEP 694: Amend scanning in staged releases and add legacy API changes to use staged releases#5070cjames23 wants to merge 5 commits into
Conversation
…API changes to use staged releases
|
All commit authors signed the Contributor License Agreement. |
Documentation build overview
35 files changed ·
|
hugovk
changed the title
hugovk
changed the title
warsaw
left a comment
warsaw
left a comment
There was a problem hiding this comment.
This is a great addition to the PEP. I have some comments for a few things that need clarification, but otherwise +1. And welcome aboard as a co-author!
| **MAY** treat it as though it had completed without an adverse result and publish the session, so that a | ||
| backlogged or unavailable review system does not indefinitely prevent publication. If the review produces an | ||
| adverse result, the session resolves to ``error`` with the reason reported in the session's ``notices``; the | ||
| server **MAY** decline to publish such a session on any subsequent retry, in which case it is eventually |
There was a problem hiding this comment.
This section (if not covered below), should provide details on what happens if files in the session are deleted and reuploaded. It could be:
- Continue to await human review
- Rescan newly uploaded files
- Reset the entire malware scan state
It probably would also be helpful to say something about denial of service attacks on the scanner. E.g. bad actor starts an upload session, uploads known malware knowing it will get flagged. Bad Actor deletes the files and reuploads them to trigger another scan. Rinse and repeat until the scanning service is overwhelmed.
warsaw
left a comment
warsaw
left a comment
There was a problem hiding this comment.
I really like this addition to the PEP. It provides a fantastic transition period from legacy to upload-2.0. I have a few questions and suggestions.
| system alone can neither publish a release nor discard a pending one. | ||
|
|
||
| Cancellation is protected because it is destructive: it discards the staged files and frees the name-version | ||
| pair. An index that left it at the upload-permission level would let a compromised upload credential delete a |
There was a problem hiding this comment.
Cancelation also frees any temporary new project name reservation, which would allow an attacker with the compromised credential to "steal" the name. So that's yet another reason why an index might want to separate these permissions.
| contribute to it through the *other* path **MUST** be rejected with a ``409 Conflict``: a legacy | ||
| ``staged=true`` upload for a pair that already has an open session created through the Upload 2.0 API is | ||
| rejected, and an Upload 2.0 request that would add to a session created through a legacy upload is likewise | ||
| rejected. |
There was a problem hiding this comment.
I'm confused by the requirements here. I would totally expect rejection for a path where a legacy upload without the staged=true flag when an Upload 2.0 session is already created for the name-version pair, because that's mixing legacy-legacy with upload-2.0. But legacy+staged into an Upload 2.0 stage could be allowed without any semantic confusion -- I think! Or is that also a problem? And vice versa of course (legacy+staged followed by Upload 2.0 ... okay or not okay?). If I'm confused maybe some additional rationale is warranted?
…etween legacy and 2.0, minor fixes for other sections.
2 participants
Amendments based on DPO discussion https://discuss.python.org/t/pre-pep-staged-releases-separated-from-pep-694/107804/59
@warsaw - here are my proprosed amendments which I would like your sign off on as well.