From 04e699c413ec21d9d37e9beacd87360de5f2c35a Mon Sep 17 00:00:00 2001 From: Sam Bostock Date: Fri, 17 Feb 2023 22:30:54 -0500 Subject: [PATCH 1/2] Use descriptive test fixture names --- env_test.go | 12 ++++++------ secrets_test.go | 2 +- ...json => test-environment-string-not-object.ejson} | 0 testdata/{test.ejson => test-expected-usage.ejson} | 0 ...4.ejson => test-leading-underscore-env-key.ejson} | 0 testdata/{test2.ejson => test-public-key-only.ejson} | 0 6 files changed, 7 insertions(+), 7 deletions(-) rename testdata/{test3.ejson => test-environment-string-not-object.ejson} (100%) rename testdata/{test.ejson => test-expected-usage.ejson} (100%) rename testdata/{test4.ejson => test-leading-underscore-env-key.ejson} (100%) rename testdata/{test2.ejson => test-public-key-only.ejson} (100%) diff --git a/env_test.go b/env_test.go index 2dbbb8c..208db2e 100644 --- a/env_test.go +++ b/env_test.go @@ -14,7 +14,7 @@ func formatInvalid(received, expected string) string { func TestLoadSecrets(t *testing.T) { - rawValues, err := readSecrets("testdata/test.ejson", "./key", TestKeyValue) + rawValues, err := readSecrets("testdata/test-expected-usage.ejson", "./key", TestKeyValue) if nil != err { t.Fatal(err) } @@ -31,7 +31,7 @@ func TestLoadSecrets(t *testing.T) { func TestLoadNoEnvSecrets(t *testing.T) { - rawValues, err := readSecrets("testdata/test2.ejson", "./key", TestKeyValue) + rawValues, err := readSecrets("testdata/test-public-key-only.ejson", "./key", TestKeyValue) if nil != err { t.Fatal(err) } @@ -49,7 +49,7 @@ func TestLoadNoEnvSecrets(t *testing.T) { func TestLoadBadEnvSecrets(t *testing.T) { - rawValues, err := readSecrets("testdata/test3.ejson", "./key", TestKeyValue) + rawValues, err := readSecrets("testdata/test-environment-string-not-object.ejson", "./key", TestKeyValue) if nil != err { t.Fatal(err) } @@ -67,7 +67,7 @@ func TestLoadBadEnvSecrets(t *testing.T) { func TestLoadUnderscoreEnvSecrets(t *testing.T) { - rawValues, err := readSecrets("testdata/test4.ejson", "./key", TestKeyValue) + rawValues, err := readSecrets("testdata/test-leading-underscore-env-key.ejson", "./key", TestKeyValue) if nil != err { t.Fatal(err) } @@ -90,13 +90,13 @@ func TestInvalidEnvironments(t *testing.T) { }, } - testBad := map[string]interface{}{ + testBadNonMap := map[string]interface{}{ "environment": "bad", } var testNoEnv map[string]interface{} - _, err := ExtractEnv(testBad) + _, err := ExtractEnv(testBadNonMap) if nil == err { t.Errorf("no error when passed a non-map environment") } else if errEnvNotMap != err { diff --git a/secrets_test.go b/secrets_test.go index dedb813..8ad925b 100644 --- a/secrets_test.go +++ b/secrets_test.go @@ -46,7 +46,7 @@ func TestReadAndExportEnv(t *testing.T) { } for _, test := range tests { - err := ReadAndExportEnv("testdata/test.ejson", "./key", TestKeyValue, test.exportFunc) + err := ReadAndExportEnv("testdata/test-expected-usage.ejson", "./key", TestKeyValue, test.exportFunc) if nil != err { t.Errorf("testing %s failed: %s", test.name, err) continue diff --git a/testdata/test3.ejson b/testdata/test-environment-string-not-object.ejson similarity index 100% rename from testdata/test3.ejson rename to testdata/test-environment-string-not-object.ejson diff --git a/testdata/test.ejson b/testdata/test-expected-usage.ejson similarity index 100% rename from testdata/test.ejson rename to testdata/test-expected-usage.ejson diff --git a/testdata/test4.ejson b/testdata/test-leading-underscore-env-key.ejson similarity index 100% rename from testdata/test4.ejson rename to testdata/test-leading-underscore-env-key.ejson diff --git a/testdata/test2.ejson b/testdata/test-public-key-only.ejson similarity index 100% rename from testdata/test2.ejson rename to testdata/test-public-key-only.ejson From 8de550703593574783adf50899a78f029bc7c83e Mon Sep 17 00:00:00 2001 From: Sam Bostock Date: Fri, 17 Feb 2023 22:31:43 -0500 Subject: [PATCH 2/2] Disallow invalid identifiers as `environment` keys Since the keys under `environment` will end up as the identifiers in `export` lines, we should ensure they are valid identifiers. export not valid=value export worse; echo dangerous; _='uh oh' --- env.go | 9 +++++++++ env_test.go | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/env.go b/env.go index 5876579..1c118e3 100644 --- a/env.go +++ b/env.go @@ -3,11 +3,14 @@ package ejson2env import ( "errors" "fmt" + "regexp" ) var errNoEnv = errors.New("environment is not set in ejson") var errEnvNotMap = errors.New("environment is not a map[string]interface{}") +var validIdentifierPattern = regexp.MustCompile(`\A[a-zA-Z_][a-zA-Z0-9_]*\z`) + // ExtractEnv extracts the environment values from the map[string]interface{} // containing all secrets, and returns a map[string]string containing the // key value pairs. If there's an issue (the environment key doesn't exist, for @@ -26,6 +29,12 @@ func ExtractEnv(secrets map[string]interface{}) (map[string]string, error) { envSecrets := make(map[string]string, len(envMap)) for key, rawValue := range envMap { + // Reject keys that would be invalid environment variable identifiers + if !validIdentifierPattern.MatchString(key) { + err := fmt.Errorf("invalid identifier as key in environment: %q", key) + + return nil, err + } // Only export values that convert to strings properly. if value, ok := rawValue.(string); ok { diff --git a/env_test.go b/env_test.go index 208db2e..a0f6384 100644 --- a/env_test.go +++ b/env_test.go @@ -94,6 +94,12 @@ func TestInvalidEnvironments(t *testing.T) { "environment": "bad", } + testBadInvalidKey := map[string]interface{}{ + "environment": map[string]interface{}{ + "invalid key": "test_value", + }, + } + var testNoEnv map[string]interface{} _, err := ExtractEnv(testBadNonMap) @@ -103,6 +109,13 @@ func TestInvalidEnvironments(t *testing.T) { t.Errorf("wrong error when passed a non-map environment: %s", err) } + _, err = ExtractEnv(testBadInvalidKey) + if nil == err { + t.Errorf("no error when passed an environment with invalid key") + } else if `invalid identifier as key in environment: "invalid key"` != err.Error() { + t.Errorf("wrong error when passed an environment with invalid key: %s", err) + } + _, err = ExtractEnv(testNoEnv) if nil == err { t.Errorf("no error when passed a non-existent environment") @@ -133,3 +146,45 @@ func TestEscaping(t *testing.T) { } } + +func TestIdentifierPattern(t *testing.T) { + key := "ALL_CAPS123" + if !validIdentifierPattern.MatchString(key) { + t.Errorf("key should match pattern %q: %q", validIdentifierPattern, key) + } + + key = "lowercase" + if !validIdentifierPattern.MatchString(key) { + t.Errorf("key should match pattern %q: %q", validIdentifierPattern, key) + } + + key = "a" + if !validIdentifierPattern.MatchString(key) { + t.Errorf("key should match pattern %q: %q", validIdentifierPattern, key) + } + + key = "_leading_underscore" + if !validIdentifierPattern.MatchString(key) { + t.Errorf("key should match pattern %q: %q", validIdentifierPattern, key) + } + + key = "1_leading_digit" + if validIdentifierPattern.MatchString(key) { + t.Errorf("key should not match pattern %q: %q", validIdentifierPattern, key) + } + + key = "contains whitespace" + if validIdentifierPattern.MatchString(key) { + t.Errorf("key should not match pattern %q: %q", validIdentifierPattern, key) + } + + key = "contains-dash" + if validIdentifierPattern.MatchString(key) { + t.Errorf("key should not match pattern %q: %q", validIdentifierPattern, key) + } + + key = "contains_special_character;" + if validIdentifierPattern.MatchString(key) { + t.Errorf("key should not match pattern %q: %q", validIdentifierPattern, key) + } +}