feat: use trusted publishing to adapt UI #14657
Conversation
miketheman
added
HTML
|
Screen.Recording.2023-09-29.at.15.29.23.mov |
| A Release can be considered published via a trusted publisher if | ||
| **all** the Files in the release are published via a trusted publisher. | ||
| """ | ||
| files = self.files.all() # type: ignore[attr-defined] |
There was a problem hiding this comment.
Since self.files returns an AppenderQuery, it returns True until we resolve the query by fetching the rows. If there's a better way to do this, I'm all ears!
| @@ -603,6 +603,17 @@ def has_meta(self): | |||
| ] | |||
| ) | |||
|
|
|||
| @property | |||
| def trusted_published(self) -> bool: | |||
There was a problem hiding this comment.
Thought: If this is too cumbersome a query pattern, we could explore creating a database trigger to update the Release's value on upload.
| @@ -69,6 +70,7 @@ <h3 class="sidebar-section__title">{% trans %}Statistics{% endtrans %}</h3> | |||
| </li> | |||
| </ul> | |||
| </div> | |||
| {% if not release.trusted_published %}</details>{% endif %} | |||
There was a problem hiding this comment.
I think this is probably not worth doing this way, for a few reasons, and we should Do It Right™ instead.
As-is, we're not actually checking that the URL used for getting repo statistics is the same as the URL used for trusted publishing, so this can be subverted easily by using trusted publishing for something that is attempting to do "starjacking".
Additionally, it also doesn't indicate to the user or maintainer why sometimes statistics are hidden, and why sometimes they are not, which will make a more confusing UI.
I think we should remove this from the PR for now, with the plan to eventually replace it with a new statistics sidebar that does the following:
- if trusted publishing from GitHub has been used for the release:
- include the URL of the upstream repository (Show repository name, in the GitHub statistics section #10917)
- fetch statistics for the repository that trusted publishing is configured for
- mark the statistics as "verified via Trusted Publishing", with a link
- if trusted publishing was not used, and a GitHub URL was provided in the metadata (e.g., we would currently show statistics):
- don't fetch the statistics
- instead, include a message + provide a link that indicates that trusted publishing should be used to enable GitHub statistics
There was a problem hiding this comment.
@di Thanks for the feedback!
I've gone ahead and removed this change from scope so we can ship the other enhancements in the meantime.
I'll move the rest of the notes to a new issue so we can discuss the stats sidebar.
Signed-off-by: Mike Fiedler <[email protected]>
Signed-off-by: Mike Fiedler <[email protected]>
Signed-off-by: Mike Fiedler <[email protected]>
miketheman
force-pushed
the
miketheman/tp
branch
from
ada7d33 to
0d245b8
Compare
Projects using Trusted Publishing from GitHub have a strong attestation that the repo is the one they came from.
Using the metadata we have from the upload events, we can determine how the file got there, and bubble that up to the Release to control some aspects of the UI.
We could also display something in the Download Files if we wanted to.
Wrapping the sidebar statistics with the HTML
<details>accomplishes preserving the sidebar metadata for projects, but making it more prominent for those using a Trusted Publishing workflow.Refs: StarJacking
Addresses #8462 (partially) and probably replaces #10917