fix(conf_loader) escape '#' characters in 'KONG_*' environment variables

Automatically escape any unescaped `#` characters in parsed `KONG_*`
environment variables. The configuration parser normally strips `#` and
all text after to remove comments from `kong.conf` specified values.

However, environment variables should never include comments, so this
stripping is unnecessary for them, and mangles intended configuration
values (e.g. an issue with passwords specified via env variables).

Add test helper functions to set and unset environment variables.

Fix Kong#4525
From Kong#5062

Signed-off-by: Thibault Charbonnier <[email protected]>

kong/conf_loader.lua

@@ -563,6 +563,7 @@ local function overrides(k, default_v, opts, file_conf, arg_conf)
   opts = opts or {}
 
   local value -- definitive value for this property
+  local escape -- whether to escape a value's octothorpes
 
   -- default values have lowest priority
 
@@ -590,12 +591,24 @@ local function overrides(k, default_v, opts, file_conf, arg_conf)
     end
 
     log.debug('%s ENV found with "%s"', env_name, to_print)
+
     value = env
+    escape = true
   end
 
   -- arg_conf have highest priority
   if arg_conf and arg_conf[k] ~= nil then
     value = arg_conf[k]
+    escape = true
+  end
+
+  if escape and type(value) == "string" then
+    -- Escape "#" in env vars or overrides to avoid them being mangled by
+    -- comments stripping logic.
+    repeat
+      local s, n = string.gsub(value, [[([^\])#]], [[%1\#]])
+      value = s
+    until n == 0
   end
 
   return value, k

spec/01-unit/03-conf_loader_spec.lua

@@ -229,6 +229,61 @@ describe("Configuration loader", function()
     local conf = assert(conf_loader("spec/fixtures/to-strip.conf"))
     assert.equal("test#123", conf.pg_password)
   end)
+  it("escapes unescaped octothorpes in environment variables", function()
+    finally(function()
+      helpers.unsetenv("KONG_PG_PASSWORD")
+    end)
+    helpers.setenv("KONG_PG_PASSWORD", "test#123")
+    local conf = assert(conf_loader())
+    assert.equal("test#123", conf.pg_password)
+
+    helpers.setenv("KONG_PG_PASSWORD", "test#12#3")
+    local conf = assert(conf_loader())
+    assert.equal("test#12#3", conf.pg_password)
+
+    helpers.setenv("KONG_PG_PASSWORD", "test##12##3#")
+    local conf = assert(conf_loader())
+    assert.equal("test##12##3#", conf.pg_password)
+  end)
+  it("escapes unescaped octothorpes in custom_conf overrides", function()
+    local conf = assert(conf_loader(nil, {
+      pg_password = "test#123",
+    }))
+    assert.equal("test#123", conf.pg_password)
+
+    local conf = assert(conf_loader(nil, {
+      pg_password = "test#12#3",
+    }))
+    assert.equal("test#12#3", conf.pg_password)
+
+    local conf = assert(conf_loader(nil, {
+      pg_password = "test##12##3#",
+    }))
+    assert.equal("test##12##3#", conf.pg_password)
+  end)
+  it("does not modify existing escaped octothorpes in environment variables", function()
+    finally(function()
+      helpers.unsetenv("KONG_PG_PASSWORD")
+    end)
+    helpers.setenv("KONG_PG_PASSWORD", [[test\#123]])
+    local conf = assert(conf_loader())
+    assert.equal("test#123", conf.pg_password)
+
+    helpers.setenv("KONG_PG_PASSWORD", [[test\#\#12\#\#3\#]])
+    local conf = assert(conf_loader())
+    assert.equal("test##12##3#", conf.pg_password)
+  end)
+  it("does not modify existing escaped octothorpes in custom_conf overrides", function()
+    local conf = assert(conf_loader(nil, {
+      pg_password = [[test\#123]],
+    }))
+    assert.equal("test#123", conf.pg_password)
+
+    local conf = assert(conf_loader(nil, {
+      pg_password = [[test\#\#12\#\#3\#]],
+    }))
+    assert.equal("test##12##3#", conf.pg_password)
+  end)
 
   describe("dynamic directives", function()
     it("loads flexible prefix based configs from a file", function()

spec/helpers.lua

@@ -46,6 +46,13 @@ local http = require "resty.http"
 local nginx_signals = require "kong.cmd.utils.nginx_signals"
 local log = require "kong.cmd.utils.log"
 local DB = require "kong.db"
+local ffi = require "ffi"
+
+
+ffi.cdef [[
+  int setenv(const char *name, const char *value, int overwrite);
+  int unsetenv(const char *name);
+]]
 
 
 log.set_lvl(log.levels.quiet) -- disable stdout logs in tests
@@ -2005,6 +2012,12 @@ return {
       wait_pid(pid_path, timeout)
     end
   end,
+  setenv = function(env, value)
+    return ffi.C.setenv(env, value, 1) == 0
+  end,
+  unsetenv = function(env)
+    return ffi.C.unsetenv(env) == 0
+  end,
 
   make_yaml_file = make_yaml_file,
 }