Disallow invalid identifiers as environment keys
#53
Conversation
| // 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 | ||
| } |
There was a problem hiding this comment.
According to the comment below, we seem to ignore when the values don't convert to strings properly, rather than return an error. IMO we should be loud about these failures (I think non-strings should also be errors, or at least print something to stderr), however if we really think it's better to silently continue, then we could change this to "only export keys that are valid identifiers" (and simply ignore invalid ones).
Separately, I may open a PR to add said stderr message (since making it an error would be a breaking change that could break production deployments).
| ) | ||
|
|
||
| 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`) |
There was a problem hiding this comment.
This would also work, and would be shorter
| var validIdentifierPattern = regexp.MustCompile(`\A[a-zA-Z_][a-zA-Z0-9_]*\z`) | |
| var validIdentifierPattern = regexp.MustCompile(`\A[a-zA-Z_]\w*\z`) |
but after thinking about it, I think the long form is clearer, as it explicitly shows how the first character can't be a digit, but that otherwise it's all the same.
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'
sambostock
force-pushed
the
validate-env-var-names
branch
from
3c6e08d
to
8de5507
Compare
Since the keys under
environmentwill end up as the identifiers inexportlines, we should ensure they are valid identifiers.