fix(notifiers): redact credential headers, harden http error paths

Author: TheophileDiot

.tests/notifier.sh

@@ -82,7 +82,9 @@ check_echo_notifier() {
 	local plugin="$1" path="$2" shape="$3" site found="ko" code r=0
 	site="${plugin}.example.com"
 	echo "ℹ️ [$plugin] provoking deny on http://$site/blocked ..."
-	code="$(curl -s -o /dev/null -w "%{http_code}" -H "Host: $site" http://localhost/blocked)"
+	# Send a credential header: the notifier must redact it (see redact_header in
+	# each plugin's *_helpers.lua) before forwarding the request to the sink.
+	code="$(curl -s -o /dev/null -w "%{http_code}" -H "Host: $site" -H "Cookie: redactme-supersecret" http://localhost/blocked)"
 	if [ "$code" != "403" ] ; then
 		echo "❌ [$plugin] expected 403 on /blocked, got $code"
 		fail=1
@@ -120,7 +122,7 @@ check_echo_notifier matrix "/_matrix/client" '"formatted_body"'
 # DISCORD_RETRY_IF_LIMITED=yes the plugin retries once, so the mock should see
 # two requests to /discord.
 echo "ℹ️ [discord] provoking deny (retry path) ..."
-code="$(curl -s -o /dev/null -w "%{http_code}" -H "Host: discord.example.com" http://localhost/blocked)"
+code="$(curl -s -o /dev/null -w "%{http_code}" -H "Host: discord.example.com" -H "Cookie: redactme-supersecret" http://localhost/blocked)"
 if [ "$code" != "403" ] ; then
 	echo "❌ [discord] expected 403 on /blocked, got $code"
 	fail=1
@@ -149,12 +151,38 @@ else
 	fail=1
 fi
 
+# discord posts to the ratelimit sink (not the echo mock), so assert its
+# header redaction separately: the Cookie sent on the deny must be [REDACTED].
+if docker compose logs ratelimit 2>/dev/null | grep -qF "redactme-supersecret" ; then
+	echo "❌ [discord] sensitive header value leaked to the notifier payload"
+	fail=1
+elif docker compose logs ratelimit 2>/dev/null | grep -qF "[REDACTED]" ; then
+	echo "✔️ [discord] sensitive header redacted in the notifier payload"
+else
+	echo "❌ [discord] expected [REDACTED] marker missing from the notifier payload"
+	fail=1
+fi
+
 # Cross-check the denied payload reached the echo mock (slack/webhook).
 if ! docker compose logs mock 2>/dev/null | grep -qF "Denied request for IP" ; then
 	echo "❌ echo mock never received a denied-request payload"
 	fail=1
 fi
 
+# Sensitive-header redaction: the Cookie value sent on the denied requests must
+# never reach the notification sink — it is replaced by [REDACTED]. This guards
+# the redact_header() logic shared by all notifier plugins (slack/webhook/matrix
+# post their header table to the echo mock).
+if docker compose logs mock 2>/dev/null | grep -qF "redactme-supersecret" ; then
+	echo "❌ sensitive header value leaked to the notifier payload"
+	fail=1
+elif docker compose logs mock 2>/dev/null | grep -qF "[REDACTED]" ; then
+	echo "✔️ sensitive header redacted in the notifier payload"
+else
+	echo "❌ expected [REDACTED] marker missing from the notifier payload"
+	fail=1
+fi
+
 if [ "$fail" -ne 0 ] ; then
 	docker compose logs
 	docker compose down -v

COMPATIBILITY.json

@@ -14,6 +14,17 @@
     "1.6.0-rc2",
     "1.6.0-rc3",
     "1.6.0-rc4",
-    "1.6.0"
+    "1.6.0",
+    "1.6.1",
+    "1.6.2",
+    "1.6.3",
+    "1.6.4",
+    "1.6.5",
+    "1.6.6",
+    "1.6.7",
+    "1.6.8",
+    "1.6.9",
+    "1.6.10",
+    "1.6.11"
   ]
 }

clamav/README.md

@@ -145,12 +145,12 @@ metadata:
 
 # Settings
 
-| Setting          | Default  | Context   | Multiple | Description                                             |
-| ---------------- | -------- | --------- | -------- | ------------------------------------------------------- |
-| `USE_CLAMAV`     | `no`     | multisite | no       | Activate automatic scan of uploaded files with ClamAV.  |
-| `CLAMAV_HOST`    | `clamav` | global    | no       | ClamAV hostname or IP address.                          |
-| `CLAMAV_PORT`    | `3310`   | global    | no       | ClamAV port.                                            |
-| `CLAMAV_TIMEOUT` | `1000`   | global    | no       | Network timeout (in ms) when communicating with ClamAV. |
+| Setting          | Default  | Context   | Multiple | Description                                                                            |
+| ---------------- | -------- | --------- | -------- | -------------------------------------------------------------------------------------- |
+| `USE_CLAMAV`     | `no`     | multisite | no       | Activate automatic scan of uploaded files with ClamAV.                                 |
+| `CLAMAV_HOST`    | `clamav` | global    | no       | ClamAV hostname or IP address.                                                         |
+| `CLAMAV_PORT`    | `3310`   | global    | no       | ClamAV port.                                                                           |
+| `CLAMAV_TIMEOUT` | `1000`   | global    | no       | Network timeout in milliseconds when communicating with ClamAV (e.g. 1000 = 1 second). |
 
 # TODO
 

clamav/plugin.json

@@ -35,7 +35,7 @@
     "CLAMAV_TIMEOUT": {
       "context": "global",
       "default": "1000",
-      "help": "Network timeout (in ms) when communicating with ClamAV.",
+      "help": "Network timeout in milliseconds when communicating with ClamAV (e.g. 1000 = 1 second).",
       "id": "clamav-timeout",
       "label": "Network timeout",
       "regex": "^.*$",

clamav/ui/actions.py

@@ -18,10 +18,7 @@ def pre_render(**kwargs):
     except BaseException as e:
         logger.debug(format_exc())
         logger.error(f"Failed to get clamav ping: {e}")
-        ret["error"] = str(e)
-
-    if "error" in ret:
-        return ret
+        ret["error"] = "Could not retrieve the plugin status"
 
     return ret
 

coraza/ui/actions.py

@@ -18,10 +18,7 @@ def pre_render(**kwargs):
     except BaseException as e:
         logger.debug(format_exc())
         logger.error(f"Failed to get coraza ping: {e}")
-        ret["error"] = str(e)
-
-    if "error" in ret:
-        return ret
+        ret["error"] = "Could not retrieve the plugin status"
 
     return ret
 

discord/discord.lua

@@ -25,6 +25,7 @@ local format = string.format
 local encode = cjson.encode
 local floor = math.floor
 local date = os.date
+local redact_header = discord_helpers.redact_header
 
 function discord:initialize(ctx)
 	-- Call parent initialize
@@ -98,14 +99,18 @@ function discord:log(bypass_use_discord)
 		if count > 23 then
 			data.embeds[1].description = "Headers :\n```"
 			for header, value in pairs(headers) do
-				data.embeds[1].description = data.embeds[1].description .. header .. ": " .. value .. "\n"
+				data.embeds[1].description = data.embeds[1].description
+					.. header
+					.. ": "
+					.. redact_header(header, value)
+					.. "\n"
 			end
 			data.embeds[1].description = data.embeds[1].description .. "```"
 		else
 			for header, value in pairs(headers) do
 				table.insert(data.embeds[1].fields, {
 					name = header,
-					value = formatField(value),
+					value = formatField(redact_header(header, value)),
 					inline = true,
 				})
 			end
@@ -125,6 +130,7 @@ function discord.send(premature, self, data)
 	local httpc, err = http_new()
 	if not httpc then
 		self.logger:log(ERR, "can't instantiate http object : " .. err)
+		return
 	end
 	local res, err_http = httpc:request_uri(self.variables["DISCORD_WEBHOOK_URL"], {
 		method = "POST",
@@ -136,6 +142,7 @@ function discord.send(premature, self, data)
 	httpc:close()
 	if not res then
 		self.logger:log(ERR, "error while sending request : " .. err_http)
+		return
 	end
 	if self.variables["DISCORD_RETRY_IF_LIMITED"] == "yes" and res.status == 429 and res.headers["Retry-After"] then
 		self.logger:log(WARN, "Discord API is rate-limiting us, retrying in " .. res.headers["Retry-After"] .. "s")
@@ -211,6 +218,7 @@ function discord:api()
 		httpc, err = http_new()
 		if not httpc then
 			self.logger:log(ERR, "can't instantiate http object : " .. err)
+			return self:ret(true, "can't instantiate http object", HTTP_INTERNAL_SERVER_ERROR)
 		end
 		local res, err_http = httpc:request_uri(self.variables["DISCORD_WEBHOOK_URL"], {
 			method = "POST",

discord/discord_helpers.lua

@@ -3,9 +3,30 @@
 -- spec/discord_helpers_spec.lua.
 local len = string.len
 local sub = string.sub
+local lower = string.lower
+local concat = table.concat
+local tostring = tostring
+local type = type
 
 local _M = {}
 
+-- Request headers that carry credentials/secrets. Their values are never
+-- forwarded to the third-party notification service. Keys are lowercase so the
+-- lookup is case-insensitive (HTTP header names are case-insensitive).
+local SENSITIVE_HEADERS = {
+	["authorization"] = true,
+	["proxy-authorization"] = true,
+	["cookie"] = true,
+	["set-cookie"] = true,
+	["x-api-key"] = true,
+	["x-csrf-token"] = true,
+	["x-xsrf-token"] = true,
+	["x-auth-token"] = true,
+	["x-access-token"] = true,
+	["x-session-token"] = true,
+	["x-amz-security-token"] = true,
+}
+
 -- Discord embed field values are capped at 1024 characters. Truncate to 1021 and
 -- append "..." so the value always fits, leaving shorter strings untouched.
 function _M.format_field(input_string)
@@ -15,4 +36,22 @@ function _M.format_field(input_string)
 	return sub(input_string, 1, 1021) .. "..."
 end
 
+-- Repeated headers are returned by ngx.req.get_headers() as an array table.
+-- Flatten to a single string so downstream concatenation never fails on a table.
+function _M.flatten_header_value(value)
+	if type(value) == "table" then
+		return concat(value, ", ")
+	end
+	return tostring(value)
+end
+
+-- Return a notification-safe value for a header: "[REDACTED]" for sensitive
+-- headers, otherwise the flattened value.
+function _M.redact_header(name, value)
+	if SENSITIVE_HEADERS[lower(name)] then
+		return "[REDACTED]"
+	end
+	return _M.flatten_header_value(value)
+end
+
 return _M

discord/ui/actions.py

@@ -18,10 +18,7 @@ def pre_render(**kwargs):
     except BaseException as e:
         logger.debug(format_exc())
         logger.error(f"Failed to get discord ping: {e}")
-        ret["error"] = str(e)
-
-    if "error" in ret:
-        return ret
+        ret["error"] = "Could not retrieve the plugin status"
 
     return ret
 

matrix/matrix.lua

@@ -31,6 +31,7 @@ local escape_uri = ngx.escape_uri
 local html_escape = matrix_helpers.html_escape
 local escape_pattern = matrix_helpers.escape_pattern
 local anonymize_ip = matrix_helpers.anonymize_ip
+local redact_header = matrix_helpers.redact_header
 
 -- Per-worker, monotonically increasing counter to guarantee transaction-ID uniqueness.
 -- ngx.now() is cached per event-loop cycle, so time + pid alone can still collide.
@@ -96,7 +97,7 @@ function matrix:log(bypass_use_matrix)
 	data["formatted_body"] = "<p>Denied "
 		.. html_escape(request_method)
 		.. " from <b>"
-		.. remote_addr
+		.. html_escape(remote_addr)
 		.. "</b> ("
 		.. country
 		.. ' • "<i>'
@@ -139,8 +140,9 @@ function matrix:log(bypass_use_matrix)
 			data["formatted_body"] = data["formatted_body"] .. "<table><tr><th>Header</th><th>Value</th></tr>"
 			data["body"] = data["body"] .. "\n\n"
 			for header, value in pairs(headers) do
-				-- Repeated headers are returned as a table by ngx.req.get_headers()
-				local header_value = type(value) == "table" and table.concat(value, ", ") or value
+				-- Redact credential headers and flatten repeated headers (which arrive
+				-- as a table from ngx.req.get_headers()) before leaving the instance.
+				local header_value = redact_header(header, value)
 				data["formatted_body"] = data["formatted_body"]
 					.. "<tr><td>"
 					.. html_escape(header)