Improve unexpected success handling (#108)

* Add raises() contextmanager

This allows managing the "unexpected success" case, and printing
the same details as in "unexpected failure" case.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>

* Change the failure output slightly

Try to print a usable shell command:
 * use str() so that we don't get "PosixPath()" in the args list
 * join the arguments into a single string

This means that I can now copy paste the command and reproduce the
problem by just being in the same directory with assets.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>

---------

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>

test/client.py

@@ -1,4 +1,5 @@
 from __future__ import annotations
+from contextlib import contextmanager
 
 import os
 import subprocess
@@ -12,9 +13,7 @@
 CERTIFICATE_OIDC_ISSUER = "https://token.actions.githubusercontent.com"
 
 _CLIENT_ERROR_MSG = """
-!!! CLIENT FAILURE !!!
-
-Arguments: {args}
+Command: {command}
 Exit code: {exitcode}
 
 !!! STDOUT !!!
@@ -33,6 +32,10 @@ class ClientFail(Exception):
     pass
 
 
+class ClientUnexpectedSuccess(Exception):
+    pass
+
+
 class VerificationMaterials:
     """
     A wrapper around verification materials. Materials can be either bundles
@@ -114,13 +117,15 @@ def __init__(self, entrypoint: str, identity_token: str) -> None:
         """
         self.entrypoint = entrypoint
         self.identity_token = identity_token
+        self.completed_process: subprocess.CompletedProcess | None = None
 
     def run(self, *args) -> None:
         """
         Execute a command against the Sigstore client.
         """
+        self.completed_process = None
         try:
-            subprocess.run(
+            self.completed_process = subprocess.run(
                 [self.entrypoint, *args],
                 text=True,
                 stdout=subprocess.PIPE,
@@ -130,12 +135,28 @@ def run(self, *args) -> None:
         except subprocess.CalledProcessError as cpe:
             msg = _CLIENT_ERROR_MSG.format(
                 exitcode=cpe.returncode,
-                args=cpe.args,
+                command=" ".join(map(str, cpe.args)),
                 stdout=cpe.stdout,
                 stderr=cpe.stderr,
             )
             raise ClientFail(msg)
 
+    @contextmanager
+    def raises(self):
+        try:
+            yield
+        except ClientFail:
+            pass
+        else:
+            assert self.completed_process
+            msg = _CLIENT_ERROR_MSG.format(
+                exitcode=self.completed_process.returncode,
+                command=" ".join(map(str, self.completed_process.args)),
+                stdout=self.completed_process.stdout,
+                stderr=self.completed_process.stderr,
+            )
+            raise ClientUnexpectedSuccess(msg)
+
     @singledispatchmethod
     def sign(self, materials: VerificationMaterials, artifact: os.PathLike) -> None:
         """

test/test_bundle.py

@@ -1,5 +1,5 @@
 from pathlib import Path
-from test.client import BundleMaterials, ClientFail, SigstoreClient
+from test.client import BundleMaterials, SigstoreClient
 from test.conftest import _MakeMaterialsByType
 
 import pytest  # type: ignore
@@ -29,7 +29,7 @@ def test_verify_rejects_root(
     materials: BundleMaterials
     input_path, materials = make_materials_by_type("has_root_in_chain.txt", BundleMaterials)
 
-    with pytest.raises(ClientFail):
+    with client.raises():
         client.verify(materials, input_path)
 
 
@@ -77,7 +77,7 @@ def test_verify_rejects_staging_cert(
     input_path, materials = make_materials_by_type("a.txt", BundleMaterials)
     materials.bundle = Path("a.txt.staging.sigstore")
 
-    with pytest.raises(ClientFail):
+    with client.raises():
         client.verify(materials, input_path)
 
 
@@ -93,7 +93,7 @@ def test_verify_rejects_invalid_set(
     input_path, materials = make_materials_by_type("a.txt", BundleMaterials)
     materials.bundle = Path("a.txt.invalid_set.sigstore")
 
-    with pytest.raises(ClientFail):
+    with client.raises():
         client.verify(materials, input_path)
 
 
@@ -108,7 +108,7 @@ def test_verify_rejects_invalid_signature(
     input_path, materials = make_materials_by_type("a.txt", BundleMaterials)
     materials.bundle = Path("a.txt.invalid_signature.sigstore")
 
-    with pytest.raises(ClientFail):
+    with client.raises():
         client.verify(materials, input_path)
 
 
@@ -124,7 +124,7 @@ def test_verify_rejects_invalid_key(
     input_path, materials = make_materials_by_type("a.txt", BundleMaterials)
     materials.bundle = Path("a.txt.invalid_key.sigstore")
 
-    with pytest.raises(ClientFail):
+    with client.raises():
         client.verify(materials, input_path)
 
 
@@ -139,7 +139,7 @@ def test_verify_rejects_invalid_inclusion_proof(
     input_path, materials = make_materials_by_type("a.txt", BundleMaterials)
     materials.bundle = Path("a.txt.invalid_inclusion_proof.sigstore")
 
-    with pytest.raises(ClientFail):
+    with client.raises():
         client.verify(materials, input_path)
 
 
@@ -154,5 +154,5 @@ def test_verify_rejects_different_materials(
     input_path, materials = make_materials_by_type("b.txt", BundleMaterials)
     materials.bundle = Path("a.txt.good.sigstore")
 
-    with pytest.raises(ClientFail):
+    with client.raises():
         client.verify(materials, input_path)

test/test_certificate_verify.py

@@ -2,7 +2,7 @@
 
 import pytest  # type: ignore
 
-from .client import ClientFail, SignatureCertificateMaterials, SigstoreClient
+from .client import SignatureCertificateMaterials, SigstoreClient
 
 
 def test_verify_invalid_certificate_chain(client: SigstoreClient) -> None:
@@ -22,5 +22,5 @@ def test_verify_invalid_certificate_chain(client: SigstoreClient) -> None:
     materials.certificate = certificate_path
     materials.signature = signature_path
 
-    with pytest.raises(ClientFail):
+    with client.raises():
         client.verify(materials, artifact_path)

test/test_signature_verify.py

@@ -3,7 +3,7 @@
 
 import pytest  # type: ignore
 
-from .client import ClientFail, SignatureCertificateMaterials, SigstoreClient
+from .client import SignatureCertificateMaterials, SigstoreClient
 
 
 @pytest.mark.signing
@@ -26,7 +26,7 @@ def test_verify_empty(client: SigstoreClient, make_materials: _MakeMaterials) ->
     blank_path.touch()
 
     # Verify with an empty artifact.
-    with pytest.raises(ClientFail):
+    with client.raises():
         client.verify(materials, blank_path)
 
     # Verify with correct inputs.
@@ -55,7 +55,7 @@ def test_verify_mismatch(client: SigstoreClient, make_materials: _MakeMaterials)
     assert b_materials.exists()
 
     # Verify with a mismatching artifact.
-    with pytest.raises(ClientFail):
+    with client.raises():
         client.verify(a_materials, b_artifact_path)
 
     # Verify with correct inputs.
@@ -82,31 +82,31 @@ def test_verify_sigcrt(
     blank_path.touch()
 
     # Verify with an empty signature.
-    with pytest.raises(ClientFail):
+    with client.raises():
         blank_sig = SignatureCertificateMaterials()
         blank_sig.signature = blank_path
         blank_sig.certificate = a_materials.certificate
 
         client.verify(blank_sig, a_artifact_path)
 
     # Verify with an empty certificate.
-    with pytest.raises(ClientFail):
+    with client.raises():
         blank_crt = SignatureCertificateMaterials()
         blank_crt.signature = a_materials.signature
         blank_crt.certificate = blank_path
 
         client.verify(blank_crt, a_artifact_path)
 
     # Verify with a mismatching certificate.
-    with pytest.raises(ClientFail):
+    with client.raises():
         crt_mismatch = SignatureCertificateMaterials()
         crt_mismatch.certificate = b_materials.certificate
         crt_mismatch.signature = a_materials.signature
 
         client.verify(crt_mismatch, a_artifact_path)
 
     # Verify with a mismatching signature.
-    with pytest.raises(ClientFail):
+    with client.raises():
         sig_mismatch = SignatureCertificateMaterials()
         sig_mismatch.certificate = a_materials.certificate
         sig_mismatch.signature = b_materials.signature