Wrap sigstore isntance in sigstore options

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>

Author: puerco

options/sigstore.go

@@ -4,147 +4,60 @@
 package options
 
 import (
-	"errors"
 	"fmt"
-	"net/url"
 
 	"github.com/spf13/cobra"
 
 	"github.com/carabiner-dev/signer/internal/tuf"
+	"github.com/carabiner-dev/signer/sigstore"
 )
 
 // Sigstore options to control how signer handles signing with sigstore
 type Sigstore struct {
-	// Embed the tuf options struct
-	tuf.TufOptions
-
-	Timestamp bool
-
-	// AppendToRekor controls if the signing operation is recorded into the
-	// transparency log.
-	AppendToRekor bool `json:"rekor-append"`
-	DisableSTS    bool
-
-	// FulcioURL url of the Fulcio CA (defaults to the public good instance)
-	FulcioURL string `json:"fulcio-url"`
-
-	// RekorURL url of the Rekor transparency log (defaults to the public good instance)
-	RekorURL string `json:"rekor-url"`
-
-	// Hide the OIDC options in the CLI --help
-	HideOIDCOptions bool
-	// FlagPrefix adds a prefix to the CLI strings, these help grouping them
-	FlagPrefix string
-
-	// OidcRedirectURL defines the URL that the browser will redirect to.
-	// if the port is set to 0, bind will randomize it to a high number
-	// port before starting the OIDC flow.
-	OidcRedirectURL string `json:"oidc-redirect-url"`
-
-	// OIDC token issuer endpoint
-	OidcIssuer string `json:"oidc-issuer"`
-
-	// Client ID to stamp on the tokens
-	OidcClientID string `json:"oidc-client-id"`
-
-	// Client secret to pass in OIDC calls
-	OidcClientSecret string `json:"oidc-client-secret"`
-
-	// Time stamp verification options
-
-	// Look for a signed timestamp in the cert and verify with the CTLog Auth
-	RequireCTlog bool `json:"require-ct-log"`
-	// Verify the cert validity in the transparency log
-	RequireTlog bool `json:"require-tlog"`
-	// Verify the certificate validity time with a signed timestamp
-	RequireSignedTimestamps bool `json:"require-signed-timestamps"`
-	// Allow no timestamp, for keys instead of certs
-	RequireObserverTimestamp bool `json:"require-observer-timestamp"`
+	sigstore.Instance
 }
 
 // Ensure the options have the required OIDC fields
 func (s *Sigstore) ValidateOIDC() error {
-	errs := []error{}
-	if s.OidcClientID == "" {
-		errs = append(errs, errors.New("OIDC client ID is missing"))
-	}
-
-	if s.OidcIssuer == "" {
-		errs = append(errs, errors.New("OIDC issuer URL missing"))
-	}
-
-	if s.OidcRedirectURL == "" {
-		errs = append(errs, errors.New("OIDC redirect URL missing"))
-	}
-	return errors.Join(errs...)
+	return s.Instance.ValidateOIDC()
 }
 
 var DefaultSigstore = Sigstore{
-	Timestamp:     true,
-	AppendToRekor: true,
-
-	TufOptions: tuf.TufOptions{
-		TufRootURL: "https://tuf-repo-cdn.sigstore.dev",
+	Instance: sigstore.Instance{
+		Timestamp:     true,
+		AppendToRekor: true,
+
+		TufOptions: tuf.TufOptions{
+			TufRootURL: "https://tuf-repo-cdn.sigstore.dev",
+		},
+
+		HideOIDCOptions: true,
+		OidcRedirectURL: "http://localhost:0/auth/callback",
+		OidcIssuer:      "https://oauth2.sigstore.dev/auth",
+		OidcClientID:    "sigstore",
+
+		// URLs default the public good instances
+		FulcioURL: "https://fulcio.sigstore.dev",
+		RekorURL:  "https://rekor.sigstore.dev",
+
+		RequireCTlog:             true,
+		RequireTlog:              true,
+		RequireObserverTimestamp: true,
 	},
-
-	HideOIDCOptions: true,
-	OidcRedirectURL: "http://localhost:0/auth/callback",
-	OidcIssuer:      "https://oauth2.sigstore.dev/auth",
-	OidcClientID:    "sigstore",
-
-	// URLs default the public good instances
-	FulcioURL: "https://fulcio.sigstore.dev",
-	RekorURL:  "https://rekor.sigstore.dev",
-
-	RequireCTlog:             true,
-	RequireTlog:              true,
-	RequireObserverTimestamp: true,
 }
 
 // ValidateTimestamps
 func (s *Sigstore) ValidateTimestamps() error {
-	if !s.RequireCTlog && !s.RequireTlog && !s.RequireObserverTimestamp && !s.RequireSignedTimestamps {
-		return errors.New("at least one method to check timestamps must be set")
-	}
-	return nil
+	return s.Instance.ValidateTimestamps()
 }
 
 // ValidateSigner check the options required to sign
 func (s *Sigstore) ValidateSigner() error {
-	errs := []error{
-		s.ValidateOIDC(),
-	}
-	if s.RekorURL == "" {
-		if s.AppendToRekor {
-			errs = append(errs, errors.New("rekor url not set (and append to rekor is set)"))
-		}
-	} else {
-		if _, err := url.Parse(s.RekorURL); err != nil {
-			errs = append(errs, fmt.Errorf("invalid Rekor URL"))
-		}
-	}
-	if s.FulcioURL == "" {
-		errs = append(errs, errors.New("fulcio url not set"))
-	} else {
-		if _, err := url.Parse(s.RekorURL); err != nil {
-			errs = append(errs, fmt.Errorf("invalid Rekor URL"))
-		}
-	}
-	return errors.Join(errs...)
+	return s.Instance.ValidateSigner()
 }
 
 func (s *Sigstore) ValidateVerifier() error {
-	errs := []error{
-		s.ValidateTimestamps(),
-	}
-	if s.FulcioURL == "" {
-		errs = append(errs, errors.New("fulcio url not set"))
-	} else {
-		if _, err := url.Parse(s.RekorURL); err != nil {
-			errs = append(errs, fmt.Errorf("invalid Rekor URL value"))
-		}
-	}
-	return errors.Join(errs...)
+	return s.Instance.ValidateVerifier()
 }
 
 // Validate checks the integrity of the sigstore options

options/sigstore_test.go

@@ -0,0 +1,65 @@
+// SPDX-FileCopyrightText: Copyright 2025 Carabiner Systems, Inc
+// SPDX-License-Identifier: Apache-2.0
+
+package options
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/require"
+
+	"github.com/carabiner-dev/signer/sigstore"
+)
+
+// TestEnsureDefaultSigstore checks that the default sigstore
+// options match the first instance in the roots file.
+func TestEnsureDefaultSigstore(t *testing.T) {
+	conf, err := sigstore.ParseRoots(DefaultRoots)
+	require.NoError(t, err)
+
+	moded := DefaultSigstore
+	// Timestamp is not exposed in json :/
+	moded.Timestamp = false
+	moded.HideOIDCOptions = false
+
+	conf.Roots[0].RootData = nil
+	require.Empty(t, cmp.Diff(conf.Roots[0].Instance, moded.Instance))
+}
+
+// TestDefaultRoots checks that the default roots are valid and that the first
+// root is sign-capable
+func TestDefaultRoots(t *testing.T) {
+	t.Parallel()
+	for _, tt := range []struct {
+		name     string
+		getRoots func(t *testing.T) *sigstore.SigstoreRoots
+	}{
+		{"top-level-file", func(t *testing.T) *sigstore.SigstoreRoots {
+			t.Helper()
+			roots, err := sigstore.ParseRootsFile("sigstore-roots.json")
+			require.NoError(t, err)
+			return roots
+		}},
+		{"options-embed", func(t *testing.T) *sigstore.SigstoreRoots {
+			t.Helper()
+			roots, err := sigstore.ParseRoots(DefaultRoots)
+			require.NoError(t, err)
+			return roots
+		}},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			roots := tt.getRoots(t)
+
+			// Require at least one root
+			require.GreaterOrEqual(t, len(roots.Roots), 1)
+			require.NoError(t, roots.Roots[0].ValidateSigner())
+
+			// Verify all returned sets are valid
+			for _, r := range roots.Roots {
+				require.NoError(t, r.ValidateVerifier())
+			}
+		})
+	}
+}

signer.go

@@ -29,7 +29,7 @@ func NewSigner() *Signer {
 	opts := options.DefaultSigner
 	roots, err := sigstore.ParseRoots(opts.SigstoreRootsData)
 	if err == nil && len(roots.Roots) > 0 {
-		opts.Sigstore = roots.Roots[0].Sigstore
+		opts.Instance = roots.Roots[0].Instance
 	} else {
 		// This is a fatal err. This should never happen as the package embeds
 		// the roots information and we have unit tests to check they are valid

signer_test.go

@@ -35,7 +35,7 @@ func TestSignStatement(t *testing.T) {
 	// Parse the roots
 	roots, err := sigstore.ParseRoots(opts.SigstoreRootsData)
 	require.NoError(t, err)
-	opts.Sigstore = roots.Roots[0].Sigstore
+	opts.Instance = roots.Roots[0].Instance
 
 	for _, tt := range []struct {
 		name      string
@@ -110,7 +110,7 @@ func TestSignMessage(t *testing.T) {
 	opts := options.DefaultSigner
 	roots, err := sigstore.ParseRoots(opts.SigstoreRootsData)
 	require.NoError(t, err)
-	opts.Sigstore = roots.Roots[0].Sigstore
+	opts.Instance = roots.Roots[0].Instance
 
 	for _, tt := range []struct {
 		name      string