feat: Implement 'env' package to handle environment variables in cosi…

…gn (#2322) * Implement 'env' package to handle environment variables Signed-off-by: Marko Mudrinić <[email protected]> * Make 'cosign env' use the env package Signed-off-by: Marko Mudrinić <[email protected]> * Make packages use env instead os.Getenv/LookupEnv Signed-off-by: Marko Mudrinić <[email protected]> * Fix oci unit test Signed-off-by: Marko Mudrinić <[email protected]> * Fix E2E tests Signed-off-by: Marko Mudrinić <[email protected]> * Address review comments Signed-off-by: Marko Mudrinić <[email protected]> * Update docgen Signed-off-by: Marko Mudrinić <[email protected]> * Integrate forbidigo linter for os.Getenv/LookupEnv Signed-off-by: Marko Mudrinić <[email protected]> * Document all environment variables used by cosign Signed-off-by: Marko Mudrinić <[email protected]> * Address review comments - part 2 Signed-off-by: Marko Mudrinić <[email protected]> * Fix issue with tests conflicting with OS envs Signed-off-by: Marko Mudrinić <[email protected]> * Address review comments - part 3 Signed-off-by: Marko Mudrinić <[email protected]> * Fix gocritic lint errors Signed-off-by: Marko Mudrinić <[email protected]> Signed-off-by: Marko Mudrinić <[email protected]>
sigstore · Oct 17, 2022 · 285459e · 285459e
1 parent d1c6336
commit 285459e
Show file tree

Hide file tree

Showing 27 changed files with 791 additions and 97 deletions.
diff --git a/.golangci.yml b/.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:

diff --git a/cmd/cosign/cli/env.go b/cmd/cosign/cli/env.go
@@ -18,41 +18,129 @@ 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, getEnv(), getEnviron(), o.ShowDescriptions, o.ShowSensitiveValues)
+
 			return nil
 		},
 	}
+
+	o.AddFlags(cmd)
+	return cmd
+}
+
+// NB: the purpose of those types and functions is to make it possible to swap function for testing purposes
+type envGetter func(env.Variable) string
+type environGetter func() []string
+
+func getEnv() envGetter {
+	return env.Getenv
+}
+func getEnviron() environGetter {
+	return os.Environ
 }
 
-func getEnv() []string {
-	out := []string{}
-	for _, e := range os.Environ() {
+// NB: printEnv intentionally takes map of env vars to make it easier to unit test it
+func printEnv(envVars map[env.Variable]env.VariableOpts,
+	envGet envGetter,
+	environGet environGetter,
+	showDescription, showSensitive bool) {
+	// Sort keys to print them in a predictable order
+	keys := sortEnvKeys(envVars)
+
+	// Print known/registered environment variables
+	for _, e := range keys {
+		opts := envVars[e]
+
+		// Get value of environment variable
+		val := envGet(e)
+
+		// If showDescription is set, print description for that variable
+		if showDescription {
+			fmt.Printf("# %s %s\n", e.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", e.String())
+		} else {
+			fmt.Printf("%s=%s\n", e.String(), val)
+		}
+	}
+
+	// Print not registered environment variables
+	nonRegEnv := map[string]string{}
+	for _, e := range environGet() {
 		// 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
+				// os.Environ returns key=value pairs, so we split by =
+				envSplit := strings.Split(e, "=")
+				key := envSplit[0]
+
+				// Skip registered environment variables (those are already printed above)
+				if _, ok := envVars[env.Variable(key)]; ok {
+					continue
+				}
+
+				val := ""
+				if len(envSplit) == 2 {
+					val = envSplit[1]
+				}
+
+				nonRegEnv[key] = val
 			}
 		}
 	}
-	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 key, val := range nonRegEnv {
+		if !showSensitive && val != "" {
+			fmt.Printf("%s=******\n", key)
+		} else {
+			fmt.Printf("%s=%s\n", key, val)
+		}
+	}
+}
+
+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
 }