diff --git a/apisix/cli/ngx_tpl.lua b/apisix/cli/ngx_tpl.lua index ce4cf8020e24..be78bfd70025 100644 --- a/apisix/cli/ngx_tpl.lua +++ b/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 %} diff --git a/apisix/cli/ops.lua b/apisix/cli/ops.lua index d514de4a8c45..0f6a4fae6a00 100644 --- a/apisix/cli/ops.lua +++ b/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) diff --git a/apisix/cli/schema.lua b/apisix/cli/schema.lua index b57b30fed3ce..3e3e855106c8 100644 --- a/apisix/cli/schema.lua +++ b/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]], } } }, diff --git a/t/cli/test_kubernetes.sh b/t/cli/test_kubernetes.sh index c74718409a9a..5734bcc47e2b 100755 --- a/t/cli/test_kubernetes.sh +++ b/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 diff --git a/t/cli/test_main.sh b/t/cli/test_main.sh index 4dbd96ae5cfc..c0e7d90c094c 100755 --- a/t/cli/test_main.sh +++ b/t/cli/test_main.sh @@ -226,7 +226,7 @@ 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 @@ -234,6 +234,75 @@ 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 diff --git a/t/cli/test_standalone.sh b/t/cli/test_standalone.sh index c1e728d7ace9..e366182b1fc8 100755 --- a/t/cli/test_standalone.sh +++ b/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