From aa35aed05b9403eef089621ebda94b39347d1bf7 Mon Sep 17 00:00:00 2001
From: Karim Radhouani <medkarimrdi@gmail.com>
Date: Mon, 24 Jun 2024 08:53:34 -0700
Subject: [PATCH] Do not env expand "password" and "token" fields unless they
 start with '$'

---
 pkg/app/app.go               |  2 +-
 pkg/config/config.go         | 16 +++++++++++++++-
 pkg/config/environment.go    | 15 ++++++++++++++-
 tests/configs/gnmic_env.yaml |  2 ++
 tests/env_vars.sh            | 16 +++++++++++++---
 5 files changed, 45 insertions(+), 6 deletions(-)
 create mode 100644 tests/configs/gnmic_env.yaml

diff --git a/pkg/app/app.go b/pkg/app/app.go
index 068da26a..0b37397e 100644
--- a/pkg/app/app.go
+++ b/pkg/app/app.go
@@ -222,7 +222,7 @@ func (a *App) PreRunE(cmd *cobra.Command, args []string) error {
 	}
 	a.Logger.SetOutput(logOutput)
 	a.Logger.SetFlags(flags)
-	a.Config.Address = config.SanitizeArrayFlagValue(a.Config.Address)
+	a.Config.Address = config.ParseAddressField(a.Config.Address)
 	a.Logger.Printf("version=%s, commit=%s, date=%s, gitURL=%s, docs=https://gnmic.openconfig.net", version, commit, date, gitURL)
 
 	if a.Config.Debug {
diff --git a/pkg/config/config.go b/pkg/config/config.go
index cb6a9406..f2f0d359 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -863,7 +863,21 @@ func SanitizeArrayFlagValue(ls []string) []string {
 		for strings.HasPrefix(ls[i], "[") && strings.HasSuffix(ls[i], "]") {
 			ls[i] = ls[i][1 : len(ls[i])-1]
 		}
-		res = append(res, strings.Split(ls[i], ",")...)
+		res = append(res, ls[i])
+	}
+	return res
+}
+
+func ParseAddressField(addr []string) []string {
+	res := make([]string, 0, len(addr))
+	for i := range addr {
+		if addr[i] == "[]" {
+			continue
+		}
+		for strings.HasPrefix(addr[i], "[") && strings.HasSuffix(addr[i], "]") {
+			addr[i] = addr[i][1 : len(addr[i])-1]
+		}
+		res = append(res, strings.Split(addr[i], ",")...)
 	}
 	return res
 }
diff --git a/pkg/config/environment.go b/pkg/config/environment.go
index a4f5c590..4b95e40c 100644
--- a/pkg/config/environment.go
+++ b/pkg/config/environment.go
@@ -59,8 +59,21 @@ func (c *Config) mergeEnvVars() {
 
 func (c *Config) SetGlobalsFromEnv(cmd *cobra.Command) {
 	cmd.PersistentFlags().VisitAll(func(f *pflag.Flag) {
+		// expand password and token global attr only if they start with '$'
+		if f.Name == "password" || f.Name == "token" {
+			if !f.Changed && c.FileConfig.IsSet(f.Name) {
+				val := c.FileConfig.GetString(f.Name)
+				if strings.HasPrefix(val, "$") {
+					c.setFlagValue(cmd, f.Name, val)
+				}
+			}
+			return
+		}
+		// other global flags
 		if !f.Changed && c.FileConfig.IsSet(f.Name) {
-			c.setFlagValue(cmd, f.Name, os.ExpandEnv(c.FileConfig.GetString(f.Name)))
+			if val := os.ExpandEnv(c.FileConfig.GetString(f.Name)); val != "" {
+				c.setFlagValue(cmd, f.Name, val)
+			}
 		}
 	})
 }
diff --git a/tests/configs/gnmic_env.yaml b/tests/configs/gnmic_env.yaml
new file mode 100644
index 00000000..3f56a8c2
--- /dev/null
+++ b/tests/configs/gnmic_env.yaml
@@ -0,0 +1,2 @@
+address: $CUSTOM_ADDR
+skip-verify: $SKIPVER
\ No newline at end of file
diff --git a/tests/env_vars.sh b/tests/env_vars.sh
index 31af8d6a..ec922635 100755
--- a/tests/env_vars.sh
+++ b/tests/env_vars.sh
@@ -13,13 +13,15 @@ targets=clab-test1-srl1,clab-test1-srl2,clab-test1-srl3
 ./gnmic-rc1 -u admin -p NokiaSrl1! --skip-verify --debug -a $targets -e json_ietf \
         set \
         --update-path /system/configuration/role[name=readonly]/rule[path-reference="/"]/action \
-        --update-value "read"
+        --update-value "read" \
+        --update-path /system/aaa/authorization/role[rolename=readonly] \
+        --update-value '{"services": ["gnmi"]}'
 
 # create a new user
 ./gnmic-rc1 -u admin -p NokiaSrl1! --skip-verify --debug -a $targets -e json_ietf \
         set \
         --update-path /system/aaa/authentication/user[username=user1]/password \
-        --update-value '|Bo|Z%TYe*&$P33~' 
+        --update-value "|Bo|Z%TYe*&\$P33~"
 
 # assign readonly role to the new user
 ./gnmic-rc1 -u admin -p NokiaSrl1! --skip-verify --debug -a $targets -e json_ietf \
@@ -33,7 +35,7 @@ targets=clab-test1-srl1,clab-test1-srl2,clab-test1-srl3
        --path /system/name
 
 # password from ENV
-GNMIC_PASSWORD='|Bo|Z%TYe*&$P33~' ./gnmic-rc1 -u user1 --skip-verify --debug -a $targets -e json_ietf \
+GNMIC_PASSWORD="|Bo|Z%TYe*&\$P33~" ./gnmic-rc1 -u user1 --skip-verify --debug -a $targets -e json_ietf \
        get \
        --path /system/name
 
@@ -56,3 +58,11 @@ GNMIC_USERNAME=user1 GNMIC_PASSWORD='|Bo|Z%TYe*&$P33~' GNMIC_DEBUG=true ./gnmic-
 GNMIC_USERNAME=user1 GNMIC_PASSWORD='|Bo|Z%TYe*&$P33~' GNMIC_DEBUG=true GNMIC_SKIP_VERIFY=true GNMIC_ENCODING=json_ietf GNMIC_ADDRESS=$targets ./gnmic-rc1 \
        get \
        --path /system/name
+
+## config file expansion
+CUSTOM_ADDR=$targets GNMIC_USERNAME=user1 GNMIC_PASSWORD='|Bo|Z%TYe*&$P33~' GNMIC_SKIP_VERIFY=true GNMIC_ENCODING=json_ietf ./gnmic-rc1 --config configs/gnmic_env.yaml --debug \
+       get \
+       --path /system/name
+CUSTOM_ADDR=$targets GNMIC_USERNAME=user1 GNMIC_PASSWORD='|Bo|Z%TYe*&$P33~' GNMIC_SKIP_VERIFY=true SKIPVER=false GNMIC_ENCODING=json_ietf ./gnmic-rc1 --config configs/gnmic_env.yaml --debug \
+       get \
+       --path /system/dns
\ No newline at end of file