fix(cli): quote nginx_config.envs entries so values with spaces work

apisix/cli/ngx_tpl.lua

@@ -57,7 +57,7 @@ env GCP_SERVICE_ACCOUNT;
 
 {% if envs then %}
 {% for _, name in ipairs(envs) do %}
-env {*name*};
+env "{*name*}";
 {% end %}
 {% end %}
 

apisix/cli/ops.lua

@@ -911,6 +911,23 @@ Please modify "admin_key" in conf/config.yaml .
         sys_conf["discovery_shared_dicts"]["consul"] = consul_conf.shared_size or "64m"
     end
 
+    -- env entries are rendered as double-quoted nginx directive parameters
+    -- (`env "{*name*}";` in ngx_tpl.lua), so embedded `\` and `"` must be
+    -- escaped; nginx unescapes them when reading the quoted token
+    -- entries synthesized after schema validation (kubernetes discovery copies
+    -- whatever sits between `${` and `}`) never went through the schema
+    -- pattern, so re-check them here rather than emitting a conf that nginx
+    -- rejects with an error pointing at a line the user never wrote
+    if sys_conf["envs"] then
+        for i, cfg_env in ipairs(sys_conf["envs"]) do
+            if cfg_env:find("%c") then
+                util.die("invalid environment variable entry: ",
+                         "control characters are not allowed\n")
+            end
+            sys_conf["envs"][i] = cfg_env:gsub('[\\"]', '\\%0')
+        end
+    end
+
     -- fix up lua path
     sys_conf["extra_lua_path"] = get_lua_path(yaml_conf.apisix.extra_lua_path)
     sys_conf["extra_lua_cpath"] = get_lua_path(yaml_conf.apisix.extra_lua_cpath)

apisix/cli/schema.lua

@@ -291,6 +291,10 @@ local config_schema = {
                     minItems = 1,
                     items = {
                         type = "string",
+                        -- NUL can never be carried through the C environ and
+                        -- other control chars (newline etc.) have no sane
+                        -- config source; reject early with a clear error
+                        pattern = [[\A[^\x00-\x1f]*\z]],
                     }
                 }
             },

t/cli/test_kubernetes.sh

@@ -30,17 +30,17 @@ discovery:
 
 make init
 
-if ! grep "env HOST_ENV" conf/nginx.conf; then
+if ! grep 'env "HOST_ENV"' conf/nginx.conf; then
   echo "kubernetes discovery env inject failed"
   exit 1
 fi
 
-if ! grep "env KUBERNETES_SERVICE_PORT" conf/nginx.conf; then
+if ! grep 'env "KUBERNETES_SERVICE_PORT"' conf/nginx.conf; then
   echo "kubernetes discovery env inject failed"
   exit 1
 fi
 
-if ! grep "env TOKEN_ENV" conf/nginx.conf; then
+if ! grep 'env "TOKEN_ENV"' conf/nginx.conf; then
   echo "kubernetes discovery env inject failed"
   exit 1
 fi
@@ -70,32 +70,32 @@ discovery:
 
 make init
 
-if ! grep "env DEV_HOST" conf/nginx.conf; then
+if ! grep 'env "DEV_HOST"' conf/nginx.conf; then
   echo "kubernetes discovery env inject failed"
   exit 1
 fi
 
-if ! grep "env DEV_PORT" conf/nginx.conf; then
+if ! grep 'env "DEV_PORT"' conf/nginx.conf; then
   echo "kubernetes discovery env inject failed"
   exit 1
 fi
 
-if ! grep "env DEV_TOKEN" conf/nginx.conf; then
+if ! grep 'env "DEV_TOKEN"' conf/nginx.conf; then
   echo "kubernetes discovery env inject failed"
   exit 1
 fi
 
-if ! grep "env PRO_HOST" conf/nginx.conf; then
+if ! grep 'env "PRO_HOST"' conf/nginx.conf; then
   echo "kubernetes discovery env inject failed"
   exit 1
 fi
 
-if ! grep "env PRO_PORT" conf/nginx.conf; then
+if ! grep 'env "PRO_PORT"' conf/nginx.conf; then
   echo "kubernetes discovery env inject failed"
   exit 1
 fi
 
-if ! grep "env PRO_TOKEN" conf/nginx.conf; then
+if ! grep 'env "PRO_TOKEN"' conf/nginx.conf; then
   echo "kubernetes discovery env inject failed"
   exit 1
 fi

t/cli/test_main.sh

@@ -226,14 +226,83 @@ nginx_config:
 
 make init
 
-grep "env TEST;" conf/nginx.conf > /dev/null
+grep 'env "TEST";' conf/nginx.conf > /dev/null
 if [ ! $? -eq 0 ]; then
     echo "failed: failed to update env"
     exit 1
 fi
 
 echo "passed: change default env"
 
+# env value with spaces must be rendered as a quoted directive (#11467)
+echo '
+nginx_config:
+    envs:
+        - TEST=a b
+' > conf/config.yaml
+
+make init
+
+if ! grep 'env "TEST=a b";' conf/nginx.conf > /dev/null; then
+    echo "failed: env value with spaces should be quoted"
+    exit 1
+fi
+
+mkdir -p logs
+if ! openresty -p "$PWD" -c "$PWD/conf/nginx.conf" -t; then
+    echo "failed: nginx rejects generated conf with quoted env"
+    exit 1
+fi
+
+# embedded quote / backslash are escaped
+cat > conf/config.yaml <<'EOF'
+nginx_config:
+    envs:
+        - "TEST=a\"b\\c"
+EOF
+
+make init
+
+if ! grep -F 'env "TEST=a\"b\\c";' conf/nginx.conf > /dev/null; then
+    echo "failed: quote/backslash in env value should be escaped"
+    exit 1
+fi
+
+if ! openresty -p "$PWD" -c "$PWD/conf/nginx.conf" -t; then
+    echo "failed: nginx rejects generated conf with escaped env"
+    exit 1
+fi
+
+# control characters are rejected at schema level
+cat > conf/config.yaml <<'EOF'
+nginx_config:
+    envs:
+        - "TEST=a\nb"
+EOF
+
+out=$(make init 2>&1 || true)
+if ! echo "$out" | grep "failed to validate config"; then
+    echo "failed: env value with control chars should be rejected"
+    exit 1
+fi
+
+# entries synthesized by kubernetes discovery bypass the config schema, so the
+# control-character check must also run on the final list
+cat > conf/config.yaml <<'EOF'
+discovery:
+    kubernetes:
+        client:
+            token_file: "${A\nB}"
+EOF
+
+out=$(make init 2>&1 || true)
+if ! echo "$out" | grep "control characters are not allowed"; then
+    echo "failed: synthesized env entry with control chars should be rejected"
+    exit 1
+fi
+
+echo "passed: env value quoting (#11467)"
+
 # support environment variables
 echo '
 nginx_config:
@@ -243,7 +312,7 @@ nginx_config:
 
 var_test=TEST FOO=bar make init
 
-if ! grep "env TEST_bar;" conf/nginx.conf > /dev/null; then
+if ! grep 'env "TEST_bar";' conf/nginx.conf > /dev/null; then
     echo "failed: failed to resolve variables"
     exit 1
 fi
@@ -328,7 +397,7 @@ nginx_config:
 
 var_test=TEST FOO=bar make init
 
-if ! grep "env TEST_bar;" conf/nginx.conf > /dev/null; then
+if ! grep 'env "TEST_bar";' conf/nginx.conf > /dev/null; then
     echo "failed: failed to resolve variables wrapped with whitespace"
     exit 1
 fi
@@ -348,7 +417,7 @@ deployment:
 
 ETCD_HOST=127.0.0.1 ETCD_PORT=2379 make init
 
-if ! grep "env ETCD_HOST;" conf/nginx.conf > /dev/null; then
+if ! grep 'env "ETCD_HOST";' conf/nginx.conf > /dev/null; then
     echo "failed: support environment variables in local_conf"
     exit 1
 fi
@@ -369,12 +438,12 @@ nginx_config:
 
 ETCD_HOST=127.0.0.1 ETCD_PORT=2379 make init
 
-if grep "env ETCD_HOST=.*;" conf/nginx.conf > /dev/null; then
+if grep 'env "ETCD_HOST=.*";' conf/nginx.conf > /dev/null; then
     echo "failed: support environment variables in local_conf"
     exit 1
 fi
 
-if ! grep "env ETCD_HOST;" conf/nginx.conf > /dev/null; then
+if ! grep 'env "ETCD_HOST";' conf/nginx.conf > /dev/null; then
     echo "failed: support environment variables in local_conf"
     exit 1
 fi
@@ -394,12 +463,12 @@ nginx_config:
 
 ETCD_HOST=127.0.0.1 ETCD_PORT=2379 make init
 
-if grep "env ETCD_HOST;" conf/nginx.conf > /dev/null; then
+if grep 'env "ETCD_HOST";' conf/nginx.conf > /dev/null; then
     echo "failed: support environment variables in local_conf"
     exit 1
 fi
 
-if ! grep "env ETCD_HOST=1.1.1.1;" conf/nginx.conf > /dev/null; then
+if ! grep 'env "ETCD_HOST=1.1.1.1";' conf/nginx.conf > /dev/null; then
     echo "failed: support environment variables in local_conf"
     exit 1
 fi
@@ -414,7 +483,7 @@ tests:
 
 make init
 
-if ! grep "env TEST_ENV;" conf/nginx.conf > /dev/null; then
+if ! grep 'env "TEST_ENV";' conf/nginx.conf > /dev/null; then
     echo "failed: should use default value when environment not set"
     exit 1
 fi
@@ -426,7 +495,7 @@ tests:
 
 make init
 
-if ! grep "env TEST_ENV;" conf/nginx.conf > /dev/null; then
+if ! grep 'env "TEST_ENV";' conf/nginx.conf > /dev/null; then
     echo "failed: should use default value when environment not set"
     exit 1
 fi
@@ -438,7 +507,7 @@ tests:
 
 TEST_ENV=127.0.0.1 make init
 
-if ! grep "env TEST_ENV;" conf/nginx.conf > /dev/null; then
+if ! grep 'env "TEST_ENV";' conf/nginx.conf > /dev/null; then
     echo "failed: should use environment variable when environment is set"
     exit 1
 fi

t/cli/test_standalone.sh

@@ -54,7 +54,7 @@ routes:
 # check for resolve variables
 var_test_path=/test make init
 
-if ! grep "env var_test_path;" conf/nginx.conf > /dev/null; then
+if ! grep 'env "var_test_path";' conf/nginx.conf > /dev/null; then
     echo "failed: failed to resolve variables"
     exit 1
 fi