Address review comments

Author: facutuesca

warehouse/forklift/legacy.py

@@ -1087,28 +1087,37 @@ def file_upload(request):
                 attestations = TypeAdapter(list[Attestation]).validate_json(
                     request.POST["attestations"]
                 )
+                verification_policy = publisher.publisher_verification_policy(
+                    request.oidc_claims
+                )
                 for attestation_model in attestations:
                     # For now, attestations are not stored, just verified
-                    verification_policy = publisher.publisher_verification_policy(
-                        request.oidc_claims
-                    )
                     attestation_model.verify(
                         Verifier.production(),
                         verification_policy,
                         Path(temporary_filename),
                     )
+                # Log successful attestation upload
+                metrics.increment("warehouse.upload.attestations.ok")
             except (ValidationError, InvalidAttestationError) as e:
+                # Log invalid (malformed) attestation upload
+                metrics.increment("warehouse.upload.attestations.malformed")
                 raise _exc_with_message(
                     HTTPBadRequest,
                     f"Error while decoding the included attestation: {e}",
                 )
             except VerificationError as e:
+                # Log invalid (failed verification) attestation upload
+                metrics.increment("warehouse.upload.attestations.failed_verify")
                 raise _exc_with_message(
                     HTTPBadRequest,
                     f"Could not verify the uploaded artifact using the included "
                     f"attestation: {e}",
                 )
             except Exception as e:
+                sentry_sdk.capture_message(
+                    f"Unexpected error while verifying attestation: {e}"
+                )
                 raise _exc_with_message(
                     HTTPBadRequest,
                     f"Unknown error while trying to verify included attestations: {e}",

warehouse/oidc/models/github.py

@@ -16,7 +16,7 @@
     AllOf,
     AnyOf,
     OIDCBuildConfigURI,
-    OIDCSourceRepositoryURI,
+    OIDCSourceRepositoryDigest,
 )
 from sqlalchemy import ForeignKey, String, UniqueConstraint
 from sqlalchemy.dialects.postgresql import UUID
@@ -242,6 +242,20 @@ def publisher_url(self, claims=None):
         return base
 
     def publisher_verification_policy(self, claims):
+        """
+        Get the policy used to verify attestations signed with GitHub Actions.
+
+        This policy checks the certificate in an attestation against the following
+        claims:
+        - OIDCBuildConfigURI (e.g:
+        https://github.com/org/repo/.github/workflows/workflow.yml@REF})
+        - OIDCSourceRepositoryDigest (the commit SHA corresponding to the version of
+        the repo used)
+
+        Note: the Build Config URI might end with either a ref (i.e: refs/heads/main)
+        or with a commit SHA, so we allow either by using the `AnyOf` policy and
+        grouping both possibilities together.
+        """
         sha = claims.get("sha") if claims else None
         ref = claims.get("ref") if claims else None
         if not (ref or sha):
@@ -255,7 +269,7 @@ def publisher_verification_policy(self, claims):
 
         return AllOf(
             [
-                OIDCSourceRepositoryURI(f"https://github.com/{self.repository}"),
+                OIDCSourceRepositoryDigest(sha),
                 AnyOf(expected_build_configs),
             ],
         )