-
Notifications
You must be signed in to change notification settings - Fork 966
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
Improve file-details with certificate claims #17145
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # warehouse/templates/includes/file-details.html
@@ -220,7 +220,7 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ | |||
set -x \ | |||
&& apt-get update \ | |||
&& apt-get install --no-install-recommends -y \ | |||
libpq5 libxml2 libxslt1.1 libcurl4 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just flagging for review: this needs to be removed before merge.
{% set claims = attestation.certificate_claims %} | ||
Source repository: | ||
<ul> | ||
<!-- 1.3.6.1.4.1.57264.1.12 | Source Repository URI--> | ||
<!-- 1.3.6.1.4.1.57264.1.13 | Source Repository Digest--> | ||
<li>Permalink: <a href="{{ repo_url_with_reference(claims.get("1.3.6.1.4.1.57264.1.12"), claims.get("1.3.6.1.4.1.57264.1.13")) }}"> | ||
<code>{{ repository_name(bundle.publisher) }}#{{ claims.get("1.3.6.1.4.1.57264.1.13") }}</code> | ||
</a></li> | ||
<!-- 1.3.6.1.4.1.57264.1.14 | Source Repository Ref--> | ||
<li>Branch / Tag: <a href="{{ repo_url_with_reference(claims.get("1.3.6.1.4.1.57264.1.12"), claims.get("1.3.6.1.4.1.57264.1.14")) }} "> | ||
<code>{{ claims.get("1.3.6.1.4.1.57264.1.14") }}</code> | ||
</a></li> | ||
<!-- 1.3.6.1.4.1.57264.1.15 | Source Repository Identifier--> | ||
<!-- 1.3.6.1.4.1.57264.1.16 | Source Repository Owner URI--> | ||
<li>Owner: <a href="{{ claims.get("1.3.6.1.4.1.57264.1.16") }}">{{ claims.get("1.3.6.1.4.1.57264.1.16") }}</a></li> | ||
<!-- 1.3.6.1.4.1.57264.1.17 | Source Repository Owner Identifier--> | ||
<!-- 1.3.6.1.4.1.57264.1.22 | Source Repository Visibility At Signing--> | ||
<li>Access: <code>{{ claims.get("1.3.6.1.4.1.57264.1.22") }}</code></li> | ||
</ul> | ||
Build information: | ||
<ul> | ||
<!-- 1.3.6.1.4.1.57264.1.8 | Issuer--> | ||
<!-- 1.3.6.1.4.1.57264.1.9 | Build Signer URI--> | ||
<!-- 1.3.6.1.4.1.57264.1.10 | Build Signer Digest--> | ||
<li>Token Issuer: <code>{{ claims.get("1.3.6.1.4.1.57264.1.8") }}</code></li> | ||
<!-- 1.3.6.1.4.1.57264.1.11 | Runner Environment--> | ||
<li>Build Environment: <code>{{ claims.get("1.3.6.1.4.1.57264.1.11") }}</code></li> | ||
<!-- 1.3.6.1.4.1.57264.1.18 | Build Config URI--> | ||
<!-- 1.3.6.1.4.1.57264.1.19 | Build Config Digest--> | ||
<li>Build Configuration File: | ||
<a href="{{ workflow_url(bundle.publisher, claims) }}"> | ||
<code>{{ workflow_filename(bundle.publisher) }}#{{ claims.get("1.3.6.1.4.1.57264.1.19") }}</code> | ||
</a> | ||
</li> | ||
<!-- 1.3.6.1.4.1.57264.1.20 | Build Trigger--> | ||
<li>Trigger Event: <code>{{ claims.get("1.3.6.1.4.1.57264.1.20") }}</code></li> | ||
<!-- 1.3.6.1.4.1.57264.1.21 | Run Invocation URI--> | ||
</ul> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing these mappings/selections within the template is likely going to be brittle/hard to test going forwards -- I think we'd be better off defining a Jinja2 function or other helper within the Python codebase to do these transforms.
(I'm not 100% sure how best to do this, since the way we render each claim varies based on its content/expected format, e.g. URLs. Maybe @di has thoughts?)
<!-- 1.3.6.1.4.1.57264.1.12 | Source Repository URI--> | ||
<!-- 1.3.6.1.4.1.57264.1.13 | Source Repository Digest--> | ||
<li>Permalink: <a href="{{ repo_url_with_reference(claims.get("1.3.6.1.4.1.57264.1.12"), claims.get("1.3.6.1.4.1.57264.1.13")) }}"> | ||
<code>{{ repository_name(bundle.publisher) }}#{{ claims.get("1.3.6.1.4.1.57264.1.13") }}</code> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stylistic nitpick: I've seen @
used more commonly as a convention for referencing SHAs:
<code>{{ repository_name(bundle.publisher) }}#{{ claims.get("1.3.6.1.4.1.57264.1.13") }}</code> | |
<code>{{ repository_name(bundle.publisher) }}@{{ claims.get("1.3.6.1.4.1.57264.1.13") }}</code> |
Permanent source link: <a href="{{ repo_url_with_reference(claims.get('1.3.6.1.4.1.57264.1.12'),claims.get('1.3.6.1.4.1.57264.1.13')) }}"> | ||
<i class="fa-brands fa-{{ brand }}" aria-hidden="true"></i> | ||
{{ repository_name(publ) }}#{{ claims.get("1.3.6.1.4.1.57264.1.13") }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is going to get confusing when we start to have build & publish attestations, since there's no guarantee that they come from the same hash. I think we should just push deep links like this down into the individual attestations instead.
<!-- 1.3.6.1.4.1.57264.1.22 | Source Repository Visibility At Signing--> | ||
<li>Access: <code>{{ claims.get("1.3.6.1.4.1.57264.1.22") }}</code></li> | ||
</ul> | ||
Build information: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Build information" for a publish attestation is confusing, we shouldn't really be using the word "build" in the context of a "publish" attestation I think.
Expose Certificate Claims in UI
This PR implements the UI display of Sigstore certificate claims, providing a more comprehensive view of package attestations.
Implementation Details
pypi-attestation
pypi-attestation
returns adict[str, str]
mapping OIDs (dotted strings) to their decoded valuesDependencies
claims
toAttestation
trailofbits/pypi-attestations#70pypi-attestation
(to be removed before merge)Closes #17122
/cc @di @woodruffw @facutuesca