-
Notifications
You must be signed in to change notification settings - Fork 980
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
[META] Draft support in the current ("legacy") upload API #17230
Comments
I worry that displaying draft releases in the project release timeline would confuse the end-users (i.e., not the project developers). |
Thanks, I'll review this in more detail when I get a chance. But just to let you know, I'm almost done with my PEP 694 rewrite. Keep in mind that PR will still go through one more pass on my side, but then I plan on rebooting the DPO thread. |
That's a good point -- putting it in the timeline might be unnecessarily confusing. At the same time, I think we should render the draft release itself as a page, for a few reasons:
As a middle ground here, we could leave drafted projects out of the timeline but add a flash/element to the top of |
This pass includes updating the PR to align with the current state of the resumable upload RFC draft which formed the original basis for PEP 694. |
Asking as a |
I think the draft feature here would not affect the current behavior: files are unique at the index level, so draft releases would have immutable (In other words, drafts wouldn't be for iterating on a release's files; they're only for staging a release so that all files become public in one atomic step + allowing projects to share a new release before it becomes resolvable by default.) |
How important is (b) ?. In my mind, draft releases are a tool for maintainers to:
Those could be achieved without making draft releases public to users, and without the need to expose a temporary draft index. Then draft releases would be only visible to maintainers in the web UI, as a tool to double-check and more neatly publish a release. So my question is, how common is the problem that (b) is trying to solve, and is it worth the added complexity? With private draft releases, only publishing tools need to implement support for uploading draft releases, whereas if we make them public, they're suddenly a concern for any tool that downloads packages from the index. |
I don't have good numbers here, but my understanding is that (b) is a recurring request from projects with long or complicated release procedures (e.g. due to complex native builds or their own build farms): having to do a pre-release means having to run their release processes twice, and it means more downstream test churn (since users looking to evaluate the pre-release need to upgrade twice, rather than opting into the draft and then rolling forwards).
I think this is ameliorated by the split-index design: drafts won't appear in Given that IMO the complexity here is pretty constrained: nothing will change in ordinary installation flows, and anything that uses the draft indices should interoperate fine, given that the indices will be the standard PEP 503/691 ones. |
Some thoughts:
|
No objection 🙂 -- I agree that's a better term.
I'm a little bit murky on the value of this: is the idea to allow uploaders to hide their staged release until they want it made fully public? Given that features like attestations (and longer term, things like index-wide transparency logging) will publicly disclosure releases as they're made, I think the obfuscatory effect here is pretty limited. (I think this is also true especially for the CI/CD context: projects that do public staged releases via GitHub or GitLab are going to have their staged release URLs in their public run logs.) I'm also a little murky on optional client-controlled randomness: I think, if the staged release needs to be obfuscated, then it makes sense to always do it, and have the server handle the thing we're calling the staging token. My reasoning there is that clients will struggle to make informed decisions around how to turn this knob, so if we want to give them obfuscated staging URLs then we should do it for them. But as part of that, I don't want to accidentally produce an unenforceable security boundary here, per above 🙂
No objection, although I think we should state explicitly whether this is for user confusion reasons (a sufficient reason IMO!) or obfuscation reasons (which IMO are harder to formalize, per the transparency and CI/CD leakiness points above). More generally, I think PyPI should retain the (currently implicit, but maybe it should be explicit) assumption that once you put something on the index, it should be considered disclosed. Obfuscation can be an OK feature for misuse-resistance purposes in that context, but I think it can't be a security boundary here. (I'll update the top-level comment to adjust these parts, since I think you're 100% right about drafts being confusing in the web UI.)
I'm not inherently opposed to mutable staging but I think it encourages a pretty different model than the one that Python packaging (IMO) is moving towards as a whole, where versions are "cheap" and the mapping between a package's identity (= distribution filename) and its contents (= strong digest) is immutable. That model is the one that PEP 740 (and other cryptographic packaging PEPs, like 458) assumes. It's also (separately) the one that PyPI is architecturally rigged for, since PyPI already has permanent, unique (filename, digest) pairs baked into the codebase. Or perhaps as an alternate framing: I think the distinction between pre-releases and staged releases collapses too much if pre-releases become a mutable feature 😅 -- I think right now a lot of projects use pre-releases as a low-impact place to experiment with their uploads since they don't get resolved by default, and I'm not sure it makes sense for PyPI to have multiple ways to accomplish similar packaging tasks, versus having pre-releases be the "sandbox" feature and staging be the "make a release atomic" feature. |
To tack onto this: one weird (not necessarily insecure, but not ideal) consequence to mutable staged releases is that a given The consequence of this would be some potentially useful (although admittedly narrow!) flexibility for an attacker: if |
Maybe the key difference in how we're thinking about stages is whether you would call the stage "a release" (and/or "pre-release" - a term we should probably avoid so as not to confuse with pre-release version numbers). The way I'm thinking about them, they really aren't a release, hence the term "stage"! This is not a release until the stage is actually published, at which point it becomes a release, along with all the constraints and guarantees you mention. It seems clear that
Agreed that if you really care about keeping artifacts completely secret, you can't use the staging feature. That said, I still think the optional obfuscation is useful as a middle ground between completely public and completely secret. And because the nonce in my proposal is optional, it's totally up to the client to decide. For example:
By "pre-releases" here, you mean actual published releases with a pre-release version number. And all of what you say is fine, but it's not a substitute for a testable, pre-published, final release. Failures can and do happen between say the last rc and the final release, and it's that use case that I want to support.
If we're treating stages as not-yet-published, then I don't think this is a realistic scenario. Yes, the @ sha123 attestation gets uploaded, but if we The original PEP 694 was based on https://tus.io which then got turned into an Internet Draft. The draft has been updated since then, and I am in the process of aligning my PEP 694 update to the latest draft version. Although stages aren't part of that protocol, the ability to delete or overwrite artifacts are, and I think that has valid use cases that should be supported, at least up until the actual publish step. |
Support for drafted uploads is a long-standing feature request, dating back (at least) to 2015 with #726.
Since then there's been been a proposed PEP, PEP 694, which defines a new upload API. This API includes support for drafted uploads, among other larger features (like upload session management, resumption, and batched uploading).
In the interest of expedience, @DarkaMau, @facutuesca, and I (@woodruffw) are looking at adding drafting support to just the legacy API, in such a way that a future implementation of PEP 694 (such as one built on #8941 or #16277) can reuse the building blocks within a new endpoint.
To keep things tractable (and keep PRs small + reviewable), here's the units of work we're proposing:
Add
published: datetime | None = datetime.now
to theRelease
model, with a schema migration. This can be picked/adapted from PEP 694 (resolved merge conflicts from PR #8941) #16277Add a PyPI-specific metadata field (or HTTP request header) to the upload endpoint, with the following semantics:
published = None
The current work in Draft Release support in PyPI #8941/PEP 694 (resolved merge conflicts from PR #8941) #16277 uses the
Is-Draft: true | false
header, but I propose we prefix this withX-PyPI-
to avoid standard header conflicts, so we'd checkX-PyPI-Is-Draft
or similar.When
Release.published is None
, we should not present it in the standard indices/APIs. Instead, we'll serve it at a temporary draft index. Draft Release support in PyPI #8941 has this as/simple/draft/<draft_hash>
, but I propose we drop the hash and do this with a URL that mirrors the existing hierarchy:/simple/draft/<name>/<version>/
or even just/simple/draft/<name>
with all drafts for the project being aggregated.When
Release.published is None
, the given release should not appear as the latest release on the web view. Instead, it should have presentation behavior similar to yanked releases: we can show it on the/project/<name>/#history
timeline, but with a badge indicating that it's a draft. When navigating to/project/<name>/<version>
, draft versions should be rendered like normal versions but with two differences:pip install ...
instructions should be specialized to show the draft index, per the step aboveAll told, I think this gives us a set of independent tasks to accomplish drafting. Our proposal is that we try and use as much of #8941 and #16277 here as possible, but do so via independent PRs to keep things as small as possible and reduce reviewer burden.
Separately, as client-side/tooling changes that'll need to occur:
twine
and similar uploading tools will ideally learn a--draft
or similar flag that sets the appropriate headerCC @di for thoughts, and thanks @warsaw and @alanbato for the work you've done on this already!
The text was updated successfully, but these errors were encountered: