Skip to content

Commit

Permalink
Remove unneeded alias and unneeded CLI flags for attest command
Browse files Browse the repository at this point in the history
Signed-off-by: Facundo Tuesca <[email protected]>
  • Loading branch information
facutuesca committed Sep 13, 2024
1 parent 67104f7 commit b6cedd0
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 152 deletions.
16 changes: 1 addition & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,7 @@ usage: sigstore attest [-h] [-v] --predicate FILE --predicate-type TYPE
[--identity-token TOKEN] [--oidc-client-id ID]
[--oidc-client-secret SECRET]
[--oidc-disable-ambient-providers] [--oidc-issuer URL]
[--oauth-force-oob] [--no-default-files]
[--signature FILE] [--certificate FILE] [--bundle FILE]
[--output-directory DIR] [--overwrite]
[--oauth-force-oob] [--bundle FILE] [--overwrite]
FILE [FILE ...]

positional arguments:
Expand Down Expand Up @@ -227,20 +225,8 @@ OpenID Connect options:
False)

Output options:
--no-default-files Don't emit the default output files
({input}.sigstore.json) (default: False)
--signature FILE, --output-signature FILE
Write a single signature to the given file; does not
work with multiple input files (default: None)
--certificate FILE, --output-certificate FILE
Write a single certificate to the given file; does not
work with multiple input files (default: None)
--bundle FILE Write a single Sigstore bundle to the given file; does
not work with multiple input files (default: None)
--output-directory DIR
Write default outputs to the given directory
(conflicts with --signature, --certificate, --bundle)
(default: None)
--overwrite Overwrite preexisting signature and certificate
outputs, if present (default: False)
```
Expand Down
255 changes: 126 additions & 129 deletions sigstore/_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
import logging
import os
import sys
from dataclasses import dataclass
from pathlib import Path
from typing import NoReturn, Optional, TextIO, Union
from typing import Dict, NoReturn, Optional, TextIO, Union

from cryptography.hazmat.primitives.serialization import Encoding
from cryptography.x509 import load_pem_x509_certificate
Expand All @@ -30,6 +31,7 @@
from sigstore_protobuf_specs.dev.sigstore.bundle.v1 import (
Bundle as RawBundle,
)
from typing_extensions import TypeAlias

from sigstore import __version__, dsse
from sigstore._internal.fulcio.client import ExpiredCertificate
Expand Down Expand Up @@ -74,6 +76,17 @@
_package_logger.setLevel(os.environ.get("SIGSTORE_LOGLEVEL", "INFO").upper())


@dataclass(frozen=True)
class SigningOutputs:
signature: Optional[Path] = None
certificate: Optional[Path] = None
bundle: Optional[Path] = None


# Map of inputs -> outputs for signing operations
OutputMap: TypeAlias = Dict[Path, SigningOutputs]


def _fatal(message: str) -> NoReturn:
"""
Logs a fatal condition and exits.
Expand Down Expand Up @@ -281,32 +294,6 @@ def _parser() -> argparse.ArgumentParser:
_add_shared_oidc_options(oidc_options)

output_options = attest.add_argument_group("Output options")
output_options.add_argument(
"--no-default-files",
action="store_true",
default=_boolify_env("SIGSTORE_NO_DEFAULT_FILES"),
help="Don't emit the default output files ({input}.sigstore.json)",
)
output_options.add_argument(
"--signature",
"--output-signature",
metavar="FILE",
type=Path,
default=os.getenv("SIGSTORE_OUTPUT_SIGNATURE"),
help=(
"Write a single signature to the given file; does not work with multiple input files"
),
)
output_options.add_argument(
"--certificate",
"--output-certificate",
metavar="FILE",
type=Path,
default=os.getenv("SIGSTORE_OUTPUT_CERTIFICATE"),
help=(
"Write a single certificate to the given file; does not work with multiple input files"
),
)
output_options.add_argument(
"--bundle",
metavar="FILE",
Expand All @@ -317,21 +304,11 @@ def _parser() -> argparse.ArgumentParser:
"files"
),
)
output_options.add_argument(
"--output-directory",
metavar="DIR",
type=Path,
default=os.getenv("SIGSTORE_OUTPUT_DIRECTORY"),
help=(
"Write default outputs to the given directory (conflicts with --signature, --certificate"
", --bundle)"
),
)
output_options.add_argument(
"--overwrite",
action="store_true",
default=_boolify_env("SIGSTORE_OVERWRITE"),
help="Overwrite preexisting signature and certificate outputs, if present",
help="Overwrite preexisting bundle outputs, if present",
)

# `sigstore sign`
Expand Down Expand Up @@ -613,7 +590,9 @@ def main(args: list[str] | None = None) -> None:
e.log_and_exit(_logger, args.verbose >= 1)


def _sign_common(args: argparse.Namespace, predicate: Predicate | None) -> None:
def _sign_common(
args: argparse.Namespace, output_map: OutputMap, predicate: Predicate | None
) -> None:
"""
Signing logic for both `sigstore sign` and `sigstore attest`
Expand All @@ -623,84 +602,6 @@ def _sign_common(args: argparse.Namespace, predicate: Predicate | None) -> None:
present, it will generate an in-toto statement and wrap it in a DSSE envelope. If
not, it will use a hashedrekord.
"""
has_sig = bool(args.signature)
has_crt = bool(args.certificate)
has_bundle = bool(args.bundle)

# `--no-default-files` has no effect on `--bundle`, but we forbid it because
# it indicates user confusion.
if args.no_default_files and has_bundle:
_invalid_arguments(
args, "--no-default-files may not be combined with --bundle."
)

# Fail if `--signature` or `--certificate` is specified *and* we have more
# than one input.
if (has_sig or has_crt or has_bundle) and len(args.files) > 1:
_invalid_arguments(
args,
"Error: --signature, --certificate, and --bundle can't be used with "
"explicit outputs for multiple inputs.",
)

if args.output_directory and (has_sig or has_crt or has_bundle):
_invalid_arguments(
args,
"Error: --signature, --certificate, and --bundle can't be used with "
"an explicit output directory.",
)

# Fail if either `--signature` or `--certificate` is specified, but not both.
if has_sig ^ has_crt:
_invalid_arguments(
args, "Error: --signature and --certificate must be used together."
)

# Build up the map of inputs -> outputs ahead of any signing operations,
# so that we can fail early if overwriting without `--overwrite`.
output_map: dict[Path, dict[str, Path | None]] = {}
for file in args.files:
if not file.is_file():
_invalid_arguments(args, f"Input must be a file: {file}")

sig, cert, bundle = (
args.signature,
args.certificate,
args.bundle,
)

output_dir = args.output_directory if args.output_directory else file.parent
if output_dir.exists() and not output_dir.is_dir():
_invalid_arguments(
args, f"Output directory exists and is not a directory: {output_dir}"
)
output_dir.mkdir(parents=True, exist_ok=True)

if not bundle and not args.no_default_files:
bundle = output_dir / f"{file.name}.sigstore.json"

if not args.overwrite:
extants = []
if sig and sig.exists():
extants.append(str(sig))
if cert and cert.exists():
extants.append(str(cert))
if bundle and bundle.exists():
extants.append(str(bundle))

if extants:
_invalid_arguments(
args,
"Refusing to overwrite outputs without --overwrite: "
f"{', '.join(extants)}",
)

output_map[file] = {
"cert": cert,
"sig": sig,
"bundle": bundle,
}

# Select the signing context to use.
if args.staging:
_logger.debug("sign: staging instances requested")
Expand Down Expand Up @@ -773,27 +674,27 @@ def _sign_common(args: argparse.Namespace, predicate: Predicate | None) -> None:
)

sig_output: TextIO
if outputs["sig"] is not None:
sig_output = outputs["sig"].open("w")
if outputs.signature is not None:
sig_output = outputs.signature.open("w")
else:
sig_output = sys.stdout

signature = base64.b64encode(
result._inner.message_signature.signature
).decode()
print(signature, file=sig_output)
if outputs["sig"] is not None:
print(f"Signature written to {outputs['sig']}")
if outputs.signature is not None:
print(f"Signature written to {outputs.signature}")

if outputs["cert"] is not None:
with outputs["cert"].open(mode="w") as io:
if outputs.certificate is not None:
with outputs.certificate.open(mode="w") as io:
print(cert_pem, file=io)
print(f"Certificate written to {outputs['cert']}")
print(f"Certificate written to {outputs.certificate}")

if outputs["bundle"] is not None:
with outputs["bundle"].open(mode="w") as io:
if outputs.bundle is not None:
with outputs.bundle.open(mode="w") as io:
print(result.to_json(), file=io)
print(f"Sigstore bundle written to {outputs['bundle']}")
print(f"Sigstore bundle written to {outputs.bundle}")


def _attest(args: argparse.Namespace) -> None:
Expand All @@ -817,11 +718,107 @@ def _attest(args: argparse.Namespace) -> None:
args, f'Unable to parse predicate of type "{args.predicate_type}": {e}'
)

_sign_common(args, predicate=predicate)
# Build up the map of inputs -> outputs ahead of any signing operations,
# so that we can fail early if overwriting without `--overwrite`.
output_map: OutputMap = {}
for file in args.files:
if not file.is_file():
_invalid_arguments(args, f"Input must be a file: {file}")

bundle = args.bundle
output_dir = file.parent

if not bundle:
bundle = output_dir / f"{file.name}.sigstore.json"

if bundle and bundle.exists() and not args.overwrite:
_invalid_arguments(
args,
"Refusing to overwrite outputs without --overwrite: " f"{bundle}",
)
output_map[file] = SigningOutputs(bundle=bundle)

_sign_common(args, output_map=output_map, predicate=predicate)


def _sign(args: argparse.Namespace) -> None:
_sign_common(args, predicate=None)
has_sig = bool(args.signature)
has_crt = bool(args.certificate)
has_bundle = bool(args.bundle)

# `--no-default-files` has no effect on `--bundle`, but we forbid it because
# it indicates user confusion.
if args.no_default_files and has_bundle:
_invalid_arguments(
args, "--no-default-files may not be combined with --bundle."
)

# Fail if `--signature` or `--certificate` is specified *and* we have more
# than one input.
if (has_sig or has_crt or has_bundle) and len(args.files) > 1:
_invalid_arguments(
args,
"Error: --signature, --certificate, and --bundle can't be used with "
"explicit outputs for multiple inputs.",
)

if args.output_directory and (has_sig or has_crt or has_bundle):
_invalid_arguments(
args,
"Error: --signature, --certificate, and --bundle can't be used with "
"an explicit output directory.",
)

# Fail if either `--signature` or `--certificate` is specified, but not both.
if has_sig ^ has_crt:
_invalid_arguments(
args, "Error: --signature and --certificate must be used together."
)

# Build up the map of inputs -> outputs ahead of any signing operations,
# so that we can fail early if overwriting without `--overwrite`.
output_map: OutputMap = {}
for file in args.files:
if not file.is_file():
_invalid_arguments(args, f"Input must be a file: {file}")

sig, cert, bundle = (
args.signature,
args.certificate,
args.bundle,
)

output_dir = args.output_directory if args.output_directory else file.parent
if output_dir.exists() and not output_dir.is_dir():
_invalid_arguments(
args, f"Output directory exists and is not a directory: {output_dir}"
)
output_dir.mkdir(parents=True, exist_ok=True)

if not bundle and not args.no_default_files:
bundle = output_dir / f"{file.name}.sigstore.json"

if not args.overwrite:
extants = []
if sig and sig.exists():
extants.append(str(sig))
if cert and cert.exists():
extants.append(str(cert))
if bundle and bundle.exists():
extants.append(str(bundle))

if extants:
_invalid_arguments(
args,
"Refusing to overwrite outputs without --overwrite: "
f"{', '.join(extants)}",
)

output_map[file] = SigningOutputs(
signature=sig, certificate=cert, bundle=bundle
)

_sign_common(args, output_map=output_map, predicate=None)


def _collect_verification_state(
Expand Down
9 changes: 1 addition & 8 deletions sigstore/dsse/_predicate.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
from typing import Any, Dict, List, Literal, Optional, TypeVar, Union

from pydantic import (
AliasChoices,
BaseModel,
ConfigDict,
Field,
Expand Down Expand Up @@ -120,13 +119,7 @@ class Metadata(_SLSAConfigBase):
The Metadata object used by SLSAPredicateV0_2
"""

# We add a manual alias here because some provenance generators
# (like `slsa-github-generator`) incorrectly use BuildInvocationID
# instead of BuildInvocationId (ID vs Id)
build_invocation_id: Optional[StrictStr] = Field(
default=None,
validation_alias=AliasChoices("buildInvocationId", "buildInvocationID"),
)
build_invocation_id: Optional[StrictStr] = None
build_started_on: Optional[StrictStr] = None
build_finished_on: Optional[StrictStr] = None
completeness: Optional[Completeness] = None
Expand Down

0 comments on commit b6cedd0

Please sign in to comment.