Skip to content

Commit

Permalink
Disallow invalid identifiers as environment keys
Browse files Browse the repository at this point in the history
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'
  • Loading branch information
sambostock committed Jul 10, 2023
1 parent 04e699c commit 8de5507
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 0 deletions.
9 changes: 9 additions & 0 deletions env.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
55 changes: 55 additions & 0 deletions env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
Expand Down Expand Up @@ -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)
}
}

0 comments on commit 8de5507

Please sign in to comment.