feat: Implement 'env' package to handle environment variables in cosign

.golangci.yml

@@ -20,6 +20,7 @@ linters:
   - depguard
   - errcheck
   - errorlint
+  - forbidigo
   - gofmt
   - goimports
   - gosec
@@ -33,6 +34,15 @@ linters:
   - unconvert
   - unparam
   - whitespace
+linters-settings:
+  forbidigo:
+    # Forbid using os.Getenv and os.LookupEnv with COSIGN_ variables in favor of
+    # pkg/cosign/env package
+    # Reference: https://github.com/sigstore/cosign/issues/2236
+    forbid:
+      - 'os\.Getenv.*'
+      - 'os\.LookupEnv.*'
+    exclude_godoc_examples: false
 output:
   uniq-by-line: false
 issues:
@@ -41,6 +51,14 @@ issues:
     linters:
     - errcheck
     - gosec
+    # We want to allow using os.Getenv and os.Setenv in tests because it
+    # might be easier (and needed in some cases)
+    - forbidigo
+  # pkg/cosign/env implements working with environment variables in cosign
+  # and is based on os.Getenv and os.LookupEnv
+  - path: pkg/cosign/env
+    linters:
+    - forbidigo
   max-issues-per-linter: 0
   max-same-issues: 0
 run:

cmd/cosign/cli/env.go

@@ -18,41 +18,106 @@ package cli
 import (
 	"fmt"
 	"os"
+	"sort"
 	"strings"
 
 	"github.com/spf13/cobra"
+
+	"github.com/sigstore/cosign/cmd/cosign/cli/options"
+	"github.com/sigstore/cosign/pkg/cosign/env"
 )
 
 func Env() *cobra.Command {
-	return &cobra.Command{
+	o := &options.EnvOptions{}
+
+	cmd := &cobra.Command{
 		Use:   "env",
 		Short: "Prints Cosign environment variables",
 		Args:  cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			for _, e := range getEnv() {
-				fmt.Println(e)
-			}
+			envVars := env.EnvironmentVariables()
+			printEnv(envVars, o.ShowDescriptions, o.ShowSensitiveValues)
+
 			return nil
 		},
 	}
+
+	o.AddFlags(cmd)
+	return cmd
 }
 
-func getEnv() []string {
-	out := []string{}
+// NB: printEnv intentionally takes map of env vars to make it easier to unit test it
+func printEnv(envVars map[env.Variable]env.VariableOpts, showDescription, showSensitive bool) {
+	// Sort keys to print them in a predictable order
+	keys := sortEnvKeys(envVars)
+
+	// Print known/registered environment variables
+	for _, env := range keys {
+		opts := envVars[env]
+
+		// Get value of environment variable
+		// NB: This package works with environment variables, so we ignore forbidigo linter.
+		// We could use env.Getenv here, but that would make unit testing too complicated
+		// as unit tests are using non-registered variables
+		val := os.Getenv(env.String()) //nolint:forbidigo
+
+		// If showDescription is set, print description for that variable
+		if showDescription {
+			fmt.Printf("# %s %s\n", env.String(), opts.Description)
+			fmt.Printf("# Expects: %s\n", opts.Expects)
+		}
+
+		// If variable is sensitive, and we don't want to show sensitive values,
+		// print environment variable name and some asterisk symbols.
+		// If sensitive variable isn't set or doesn't have any value, we'll just
+		// print like non-sensitive variable
+		if opts.Sensitive && !showSensitive && val != "" {
+			fmt.Printf("%s=\"******\"\n", env.String())
+		} else {
+			fmt.Printf("%s=%q\n", env.String(), val)
+		}
+	}
+
+	// Print not registered environment variables
+	var nonRegEnv []string
 	for _, e := range os.Environ() {
 		// Prefixes to look for. err on the side of showing too much rather
 		// than too little. We'll only output things that have values set.
 		for _, prefix := range []string{
+			// We want to print eventually non-registered cosign variables (even if this shouldn't happen)
 			"COSIGN_",
 			// Can modify Sigstore/TUF client behavior - https://github.com/sigstore/sigstore/blob/35d6a82c15183f7fe7a07eca45e17e378aa32126/pkg/tuf/client.go#L52
 			"SIGSTORE_",
 			"TUF_",
 		} {
 			if strings.HasPrefix(e, prefix) {
-				out = append(out, e)
-				continue
+				// Skip registered environment variables (those are already printed above).
+				// os.Environ returns key=value pairs, so we use split by '=' to get the key
+				key := strings.Split(e, "=")[0]
+				if _, ok := envVars[env.Variable(key)]; ok {
+					continue
+				}
+				nonRegEnv = append(nonRegEnv, e)
 			}
 		}
 	}
-	return out
+	if len(nonRegEnv) > 0 && showDescription {
+		fmt.Println("# Environment variables below are not registered with cosign,\n# but might still influence cosign's behavior.")
+	}
+	for _, e := range nonRegEnv {
+		fmt.Println(e)
+	}
+}
+
+func sortEnvKeys(envVars map[env.Variable]env.VariableOpts) []env.Variable {
+	keys := []env.Variable{}
+	for k := range envVars {
+		keys = append(keys, k)
+	}
+
+	sort.Slice(keys, func(i, j int) bool {
+		return strings.Compare(keys[i].String(), keys[j].String()) < 0
+	})
+
+	return keys
 }

cmd/cosign/cli/env_test.go

@@ -16,22 +16,204 @@
 package cli
 
 import (
+	"io"
 	"os"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
+	"github.com/sigstore/cosign/pkg/cosign/env"
 )
 
-func TestGetEnv(t *testing.T) {
-	os.Setenv("COSIGN_EXPERIMENTAL", "foo")
-	os.Setenv("TUF_ROOT", "bar")
-	got := getEnv()
-	want := []string{
-		"COSIGN_EXPERIMENTAL=foo",
-		"TUF_ROOT=bar",
+const (
+	VariableTest1 env.Variable = "COSIGN_TEST1"
+	VariableTest2 env.Variable = "COSIGN_TEST2"
+
+	expectedWithoutDescription = `COSIGN_TEST1="abcd"
+COSIGN_TEST2=""
+`
+	expectedWithDescription = `# COSIGN_TEST1 is the first test variable
+# Expects: test1 value
+COSIGN_TEST1="abcd"
+# COSIGN_TEST2 is the second test variable
+# Expects: test2 value
+COSIGN_TEST2=""
+`
+
+	expectedWithHiddenSensitive = `# COSIGN_TEST1 is the first test variable
+# Expects: test1 value
+COSIGN_TEST1="abcd"
+# COSIGN_TEST2 is the second test variable
+# Expects: test2 value
+COSIGN_TEST2="******"
+`
+
+	expectedWithSensitive = `# COSIGN_TEST1 is the first test variable
+# Expects: test1 value
+COSIGN_TEST1="abcd"
+# COSIGN_TEST2 is the second test variable
+# Expects: test2 value
+COSIGN_TEST2="1234"
+`
+
+	expectedSensitiveWithoutDescription = `COSIGN_TEST1="abcd"
+COSIGN_TEST2="1234"
+`
+
+	expectedWithNonRegisteredEnv = `# COSIGN_TEST1 is the first test variable
+# Expects: test1 value
+COSIGN_TEST1="abcd"
+# COSIGN_TEST2 is the second test variable
+# Expects: test2 value
+COSIGN_TEST2=""
+# Environment variables below are not registered with cosign,
+# but might still influence cosign's behavior.
+COSIGN_TEST3=abcd
+`
+
+	expectedWithNonRegisteredEnvNoDesc = `COSIGN_TEST1="abcd"
+COSIGN_TEST2=""
+COSIGN_TEST3=abcd
+`
+)
+
+func TestPrintEnv(t *testing.T) {
+	variables := map[env.Variable]env.VariableOpts{
+		VariableTest1: {
+			Description: "is the first test variable",
+			Expects:     "test1 value",
+			Sensitive:   false,
+		},
+		VariableTest2: {
+			Description: "is the second test variable",
+			Expects:     "test2 value",
+			Sensitive:   true,
+		},
 	}
 
-	if diff := cmp.Diff(got, want); diff != "" {
-		t.Error(diff)
+	tests := []struct {
+		name                 string
+		environmentVariables map[string]string
+		registeredVariables  map[env.Variable]env.VariableOpts
+		showDescriptions     bool
+		showSensitiveValues  bool
+		expectedOutput       string
+	}{
+		{
+			name: "no descriptions and sensitive variables",
+			environmentVariables: map[string]string{
+				"COSIGN_TEST1": "abcd",
+				"COSIGN_TEST2": "",
+			},
+			registeredVariables: variables,
+			showDescriptions:    false,
+			showSensitiveValues: false,
+			expectedOutput:      expectedWithoutDescription,
+		},
+		{
+			name: "descriptions but sensitive variable is unset",
+			environmentVariables: map[string]string{
+				"COSIGN_TEST1": "abcd",
+				"COSIGN_TEST2": "",
+			},
+			registeredVariables: variables,
+			showDescriptions:    true,
+			showSensitiveValues: false,
+			expectedOutput:      expectedWithDescription,
+		},
+		{
+			name: "sensitive variable is non-empty but show sensitive variables is disabled",
+			environmentVariables: map[string]string{
+				"COSIGN_TEST1": "abcd",
+				"COSIGN_TEST2": "1234",
+			},
+			registeredVariables: variables,
+			showDescriptions:    true,
+			showSensitiveValues: false,
+			expectedOutput:      expectedWithHiddenSensitive,
+		},
+		{
+			name: "sensitive variable is empty",
+			environmentVariables: map[string]string{
+				"COSIGN_TEST1": "abcd",
+				"COSIGN_TEST2": "",
+			},
+			registeredVariables: variables,
+			showDescriptions:    true,
+			showSensitiveValues: true,
+			expectedOutput:      expectedWithDescription,
+		},
+		{
+			name: "sensitive variable is non-empty and show sensitive variables is enabled",
+			environmentVariables: map[string]string{
+				"COSIGN_TEST1": "abcd",
+				"COSIGN_TEST2": "1234",
+			},
+			registeredVariables: variables,
+			showDescriptions:    true,
+			showSensitiveValues: true,
+			expectedOutput:      expectedWithSensitive,
+		},
+		{
+			name: "sensitive variable is non-empty but show descriptions is disabled",
+			environmentVariables: map[string]string{
+				"COSIGN_TEST1": "abcd",
+				"COSIGN_TEST2": "1234",
+			},
+			registeredVariables: variables,
+			showDescriptions:    false,
+			showSensitiveValues: true,
+			expectedOutput:      expectedSensitiveWithoutDescription,
+		},
+		{
+			name: "print unregistered variable with description",
+			environmentVariables: map[string]string{
+				"COSIGN_TEST1": "abcd",
+				"COSIGN_TEST2": "",
+				"COSIGN_TEST3": "abcd",
+			},
+			registeredVariables: variables,
+			showDescriptions:    true,
+			showSensitiveValues: false,
+			expectedOutput:      expectedWithNonRegisteredEnv,
+		},
+		{
+			name: "print unregistered variable without description",
+			environmentVariables: map[string]string{
+				"COSIGN_TEST1": "abcd",
+				"COSIGN_TEST2": "",
+				"COSIGN_TEST3": "abcd",
+			},
+			registeredVariables: variables,
+			showDescriptions:    false,
+			showSensitiveValues: false,
+			expectedOutput:      expectedWithNonRegisteredEnvNoDesc,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Set needed environment variables
+			for k, v := range tt.environmentVariables {
+				os.Setenv(k, v)
+			}
+
+			orgStdout := os.Stdout
+			r, w, _ := os.Pipe()
+			os.Stdout = w
+
+			printEnv(tt.registeredVariables, tt.showDescriptions, tt.showSensitiveValues)
+
+			w.Close()
+			out, _ := io.ReadAll(r)
+			os.Stdout = orgStdout
+
+			if tt.expectedOutput != string(out) {
+				t.Errorf("Expected to get %q\n, but got %q", tt.expectedOutput, string(out))
+			}
+
+			// Unset needed environment variables to ensure clean state between tests
+			for k := range tt.environmentVariables {
+				os.Unsetenv(k)
+			}
+		})
 	}
 }

cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go

@@ -28,6 +28,7 @@ import (
 	ctx509 "github.com/google/certificate-transparency-go/x509"
 	"github.com/google/certificate-transparency-go/x509util"
 	"github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioverifier/ctutil"
+	"github.com/sigstore/cosign/pkg/cosign/env"
 
 	"github.com/sigstore/sigstore/pkg/cryptoutils"
 	"github.com/sigstore/sigstore/pkg/tuf"
@@ -36,10 +37,6 @@ import (
 // This is the CT log public key target name
 var ctPublicKeyStr = `ctfe.pub`
 
-// Setting this env variable will over ride what is used to validate
-// the SCT coming back from Fulcio.
-const altCTLogPublicKeyLocation = "SIGSTORE_CT_LOG_PUBLIC_KEY_FILE"
-
 // logIDMetadata holds information for mapping a key ID hash (log ID) to associated data.
 type logIDMetadata struct {
 	pubKey crypto.PublicKey
@@ -74,7 +71,7 @@ func ContainsSCT(cert []byte) (bool, error) {
 func VerifySCT(ctx context.Context, certPEM, chainPEM, rawSCT []byte) error {
 	// fetch SCT verification key
 	pubKeys := make(map[[sha256.Size]byte]logIDMetadata)
-	rootEnv := os.Getenv(altCTLogPublicKeyLocation)
+	rootEnv := env.Getenv(env.VariableSigstoreCTLogPublicKeyFile)
 	if rootEnv == "" {
 		tufClient, err := tuf.NewFromEnv(ctx)
 		if err != nil {