From 749bbd5d9841eeeb5848765290da60c1c2a809cd Mon Sep 17 00:00:00 2001
From: Tuomo Tanskanen <tuomo.tanskanen@est.tech>
Date: Mon, 14 Oct 2024 16:15:05 +0300
Subject: [PATCH] fix unmarshaling of expanded environment variables with
special characters (#3770)
If we expand environment values directly with os.ExpandEnv() over whole
config, we might end up in a situation where the environment variable
has escape characters that break the resulting JSON, and unmarshalling
fails. Instead of expanding the entire config with single call, we
recurse through the config and expand the values in leaves one by one.
Signed-off-by: Tuomo Tanskanen <tuomo.tanskanen@est.tech>
---
cmd/dex/config.go | 60 +++++++++++++++++++++++++++++++++++++++---
cmd/dex/config_test.go | 9 +++++--
2 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/cmd/dex/config.go b/cmd/dex/config.go
index dd6d2e2a..aa49a181 100644
--- a/cmd/dex/config.go
+++ b/cmd/dex/config.go
@@ -284,6 +284,27 @@ func getORMBasedSQLStorage(normal, entBased StorageConfig) func() StorageConfig
}
}
+// Recursively expand environment variables in the map to avoid
+// issues with JSON special characters and escapes
+func expandEnvInMap(m map[string]interface{}) {
+ for k, v := range m {
+ switch vt := v.(type) {
+ case string:
+ m[k] = os.ExpandEnv(vt)
+ case map[string]interface{}:
+ expandEnvInMap(vt)
+ case []interface{}:
+ for i, item := range vt {
+ if itemMap, ok := item.(map[string]interface{}); ok {
+ expandEnvInMap(itemMap)
+ } else if itemString, ok := item.(string); ok {
+ vt[i] = os.ExpandEnv(itemString)
+ }
+ }
+ }
+ }
+}
+
var storages = map[string]func() StorageConfig{
"etcd": func() StorageConfig { return new(etcd.Etcd) },
"kubernetes": func() StorageConfig { return new(kubernetes.Config) },
@@ -312,9 +333,24 @@ func (s *Storage) UnmarshalJSON(b []byte) error {
if len(store.Config) != 0 {
data := []byte(store.Config)
if featureflags.ExpandEnv.Enabled() {
- // Caution, we're expanding in the raw JSON/YAML source. This may not be what the admin expects.
- data = []byte(os.ExpandEnv(string(store.Config)))
+ var rawMap map[string]interface{}
+ if err := json.Unmarshal(store.Config, &rawMap); err != nil {
+ return fmt.Errorf("unmarshal config for env expansion: %v", err)
+ }
+
+ // Recursively expand environment variables in the map to avoid
+ // issues with JSON special characters and escapes
+ expandEnvInMap(rawMap)
+
+ // Marshal the expanded map back to JSON
+ expandedData, err := json.Marshal(rawMap)
+ if err != nil {
+ return fmt.Errorf("marshal expanded config: %v", err)
+ }
+
+ data = expandedData
}
+
if err := json.Unmarshal(data, storageConfig); err != nil {
return fmt.Errorf("parse storage config: %v", err)
}
@@ -358,13 +394,29 @@ func (c *Connector) UnmarshalJSON(b []byte) error {
if len(conn.Config) != 0 {
data := []byte(conn.Config)
if featureflags.ExpandEnv.Enabled() {
- // Caution, we're expanding in the raw JSON/YAML source. This may not be what the admin expects.
- data = []byte(os.ExpandEnv(string(conn.Config)))
+ var rawMap map[string]interface{}
+ if err := json.Unmarshal(conn.Config, &rawMap); err != nil {
+ return fmt.Errorf("unmarshal config for env expansion: %v", err)
+ }
+
+ // Recursively expand environment variables in the map to avoid
+ // issues with JSON special characters and escapes
+ expandEnvInMap(rawMap)
+
+ // Marshal the expanded map back to JSON
+ expandedData, err := json.Marshal(rawMap)
+ if err != nil {
+ return fmt.Errorf("marshal expanded config: %v", err)
+ }
+
+ data = expandedData
}
+
if err := json.Unmarshal(data, connConfig); err != nil {
return fmt.Errorf("parse connector config: %v", err)
}
}
+
*c = Connector{
Type: conn.Type,
Name: conn.Name,
diff --git a/cmd/dex/config_test.go b/cmd/dex/config_test.go
index c6d37cb0..68abe1f7 100644
--- a/cmd/dex/config_test.go
+++ b/cmd/dex/config_test.go
@@ -273,7 +273,8 @@ func checkUnmarshalConfigWithEnv(t *testing.T, dexExpandEnv string, wantExpandEn
os.Setenv("DEX_FOO_USER_PASSWORD", "$2a$10$33EMT0cVYVlPy6WAMCLsceLYjWhuHpbz5yuZxu/GAFj03J9Lytjuy")
// For os.ExpandEnv ($VAR -> value_of_VAR):
os.Setenv("DEX_FOO_POSTGRES_HOST", "10.0.0.1")
- os.Setenv("DEX_FOO_OIDC_CLIENT_SECRET", "bar")
+ os.Setenv("DEX_FOO_POSTGRES_PASSWORD", `psql"test\pass`)
+ os.Setenv("DEX_FOO_OIDC_CLIENT_SECRET", `abc"def\ghi`)
if dexExpandEnv != "UNSET" {
os.Setenv("DEX_EXPAND_ENV", dexExpandEnv)
} else {
@@ -288,6 +289,7 @@ storage:
# Env variables are expanded in raw YAML source.
# Single quotes work fine, as long as the env variable doesn't contain any.
host: '$DEX_FOO_POSTGRES_HOST'
+ password: '$DEX_FOO_POSTGRES_PASSWORD'
port: 65432
maxOpenConns: 5
maxIdleConns: 3
@@ -350,10 +352,12 @@ logger:
// This is not a valid hostname. It's only used to check whether os.ExpandEnv was applied or not.
wantPostgresHost := "$DEX_FOO_POSTGRES_HOST"
+ wantPostgresPassword := "$DEX_FOO_POSTGRES_PASSWORD"
wantOidcClientSecret := "$DEX_FOO_OIDC_CLIENT_SECRET"
if wantExpandEnv {
wantPostgresHost = "10.0.0.1"
- wantOidcClientSecret = "bar"
+ wantPostgresPassword = `psql"test\pass`
+ wantOidcClientSecret = `abc"def\ghi`
}
want := Config{
@@ -363,6 +367,7 @@ logger:
Config: &sql.Postgres{
NetworkDB: sql.NetworkDB{
Host: wantPostgresHost,
+ Password: wantPostgresPassword,
Port: 65432,
MaxOpenConns: 5,
MaxIdleConns: 3,
--
GitLab