fix(sys-svc-webserver-core): keep URL tokens out of the openresty access log

The front proxy logged the full request line: production had no
access_log, so nginx fell back to `combined` ($request, Referer,
$remote_user), and the debug format logged $request and the Referer.
Query-string tokens (?access_token=, ?key=, ?token=) and token routes
such as /password/reset/{token} and /i/confirm-email/{user}/{random}
ended up in plain text in the container log.

nginx.conf.j2 now declares a map from $request_uri to
$infinito_access_log_path:

- the path is written without the query string;
- the segments after a known token route (password/reset,
  password-reset, reset-password, confirm-email,
  lostpassword/reset/form, email-login, activate-account; matched
  case-insensitively at any depth) are written as [redacted], so the
  request, its status and timings stay visible without the token.

Both modes write an explicit access_log to /dev/stdout:

- production uses the new `redacted` format, laid out like `combined`
  (remote address, time, "method path protocol", status, bytes,
  user agent) with Referer and basic-auth user written as "-", so
  combined-style parsers keep working;
- debug keeps its fields (host, timings, upstream time, lengths,
  X-Forwarded-For, scheme, server name) but logs
  "method path protocol" instead of $request and drops the Referer.

The path comes from $request_uri instead of $uri so internal rewrites
(error_page, try_files) do not replace the path the client requested.
Nothing in the repository parses the proxy access log (Wazuh, the
health checks and the Prometheus log_by_lua metrics do not read it),
so no consumer needs to change. The error log still carries
`request:` and `referrer:` on error-level entries; that is outside
this change.

A unit test renders nginx.conf.j2 in both modes and asserts that every
access_log uses a format declared there, that no format references
$request, $request_uri, $args, $query_string, $is_args, $arg_*,
$http_referer or $remote_user, and, by resolving the map as nginx does,
that query strings are dropped, token routes are redacted and
lookalike paths stay unchanged.

Verified live with web-svc-html in production mode (deploy failed=0, so
openresty accepted the config). A probe request to
/password/reset/<probe>?token=<probe> was logged as
"GET /password/reset/[redacted] HTTP/1.1" 404 62 "-" "curl/8.14.1",
and /plain/page as "GET /plain/page HTTP/1.1". Negative control: the
same request against the previous config logged the probe path token
and the query token in plain text. The debug format with the fix is
covered by the unit test only.

Closes #1192

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
kevinveenbirkenbachandClaude Opus 5.5 committed 2026-10-31 10:40:40 +01:00
1 parent ba4c06b943
commit 10d2b11cc9
5 files changed
+210 -2

No files matched your search

+1
View File
@@ -40,6 +40,7 @@ Solid `1:1` edges are fixed relationships; dashed `0..1` edges are conditional (
- **General logs**: `journalctl -f -u NGINX`
- **Filter by host**: `journalctl -u NGINX -f | grep "{{ inventory_hostname }}"`
- **Enable detailed format**: set `MODE_DEBUG: true` and reload NGINX.
- **Redacted request path**: both access log formats write the path without query string, Referer or basic-auth user. Path segments after a password-reset, e-mail confirmation, e-mail login or account activation route (e.g. `/password/reset/`, `/confirm-email/`) are written as `[redacted]`; extend the `$infinito_access_log_path` map in [nginx.conf.j2](./templates/nginx.conf.j2) for further token routes.
## Credits
@@ -26,10 +26,13 @@ http
server_names_hash_bucket_size 128; # size of each bucket for server_name lookups (in bytes)
server_names_hash_max_size 512; # maximum total buckets for the server_name hash table
{% include 'roles/sys-svc-proxy/templates/headers/buffers.conf.j2' %}
map $request_uri $infinito_access_log_path {
"~*^(?P<infinito_access_log_token_route>[^?]*?/(?:password/reset|password-reset|reset-password|confirm-email|lostpassword/reset/form|email-login|activate-account)/)" "${infinito_access_log_token_route}[redacted]";
"~^(?P<infinito_access_log_plain_path>[^?]*)" "${infinito_access_log_plain_path}";
}
{% if MODE_DEBUG | bool %}
log_format debug '$host - $remote_addr [$time_local] '
'"$request" $status $body_bytes_sent '
'"Referer: $http_referer" '
'"$request_method $infinito_access_log_path $server_protocol" $status $body_bytes_sent '
'"User-Agent: $http_user_agent" '
'"ReqTime: $request_time" "UpstreamTime: $upstream_response_time" '
'"ReqLength: $request_length" "BytesSent: $bytes_sent" '
@@ -37,6 +40,11 @@ http
'"X-Forwarded-For: $http_x_forwarded_for" '
'"Scheme: $scheme" "Protocol: $server_protocol" "ServerName: $server_name"';
access_log /dev/stdout debug;
{% else %}
log_format redacted '$remote_addr - - [$time_local] '
'"$request_method $infinito_access_log_path $server_protocol" $status $body_bytes_sent '
'"-" "$http_user_agent"';
access_log /dev/stdout redacted;
{% endif %}
error_log /dev/stderr info;
sendfile on;
Whitespace-only changes.
@@ -0,0 +1,5 @@
from pathlib import Path
PROJECT_ROOT: Path = Path(__file__).resolve().parents[6]
__all__ = ["PROJECT_ROOT"]
@@ -0,0 +1,194 @@
"""The front proxy's access log keeps no tokens from URLs."""
from __future__ import annotations
import re
import unittest
from jinja2 import Environment, FileSystemLoader, StrictUndefined, select_autoescape
from . import PROJECT_ROOT
TEMPLATE = "roles/sys-svc-webserver-core/templates/nginx.conf.j2"
LEAKING_VARIABLE = re.compile(
r"\$\{?(request|request_uri|args|query_string|is_args|arg_\w+|http_referer|remote_user)\b"
)
REDACTED_PATH = "$infinito_access_log_path"
MODES = {"debug": True, "production": False}
def render(debug: bool) -> str:
"""Return the rendered nginx.conf.
Args:
debug: the value of ``MODE_DEBUG``.
"""
env = Environment(
loader=FileSystemLoader(str(PROJECT_ROOT)),
undefined=StrictUndefined,
autoescape=select_autoescape(),
)
env.filters["bool"] = bool
return env.get_template(TEMPLATE).render(
MODE_DEBUG=debug,
DEPLOYMENT_MODE="compose",
WEBSERVER_PROXY_WORKER_PROCESSES=1,
WEBSERVER_PROXY_WORKER_CONNECTIONS=256,
WEBSERVER_PROXY_WORKER_RLIMIT_NOFILE=512,
lookup=lambda _kind, key: (
["/etc/nginx/conf.d/http/"]
if key.endswith("http_includes")
else f"/var/lib/nginx/{key}"
),
)
def directives(rendered: str, name: str) -> list[str]:
"""Return the arguments of every ``name`` directive, joined across lines.
Args:
rendered: the rendered nginx.conf.
name: the directive, e.g. ``log_format``.
"""
return re.findall(rf"^\s*{name}\s+([^;]*);", rendered, re.MULTILINE)
def log_formats(rendered: str) -> dict[str, str]:
"""Return every declared log format body by its name.
Args:
rendered: the rendered nginx.conf.
"""
return {
body.split(maxsplit=1)[0]: body.split(maxsplit=1)[1]
for body in directives(rendered, "log_format")
}
def logged_path(rendered: str, request_uri: str) -> str:
"""Return the path the access log writes, resolved as nginx's map does.
Args:
rendered: the rendered nginx.conf.
request_uri: the request URI as the client sent it.
"""
block = re.search(
r"map \$request_uri \$infinito_access_log_path \{(.*?)\n\s*\}",
rendered,
re.DOTALL,
)
if block is None:
raise AssertionError("nginx.conf declares no map for $infinito_access_log_path")
for source, value in re.findall(
r'^\s*"([^"]+)"\s+"([^"]*)";', block.group(1), re.MULTILINE
):
caseless = source.startswith("~*")
match = re.search(
source[2:] if caseless else source[1:],
request_uri,
re.IGNORECASE if caseless else 0,
)
if match:
return re.sub(
r"\$\{(\w+)\}|\$(\w+)",
lambda var, m=match: m.group(var.group(1) or var.group(2)),
value,
)
return ""
class TestAccessLogFormat(unittest.TestCase):
def test_every_mode_writes_its_access_log_in_a_format_declared_here(self):
for mode, debug in MODES.items():
with self.subTest(mode=mode):
rendered = render(debug)
logs = [args.split() for args in directives(rendered, "access_log")]
self.assertTrue(logs)
for log in logs:
self.assertEqual(len(log), 2, log)
self.assertIn(log[1], log_formats(rendered))
def test_no_format_logs_the_query_string_the_referer_or_the_raw_request(self):
for mode, debug in MODES.items():
for name, body in log_formats(render(debug)).items():
with self.subTest(mode=mode, format=name):
self.assertIsNone(LEAKING_VARIABLE.search(body), body)
def test_every_format_logs_method_redacted_path_and_status(self):
for mode, debug in MODES.items():
for name, body in log_formats(render(debug)).items():
with self.subTest(mode=mode, format=name):
self.assertIn(
f'"$request_method {REDACTED_PATH} $server_protocol"', body
)
self.assertIn("$status", body)
def test_the_debug_format_keeps_host_timings_and_upstream(self):
body = log_formats(render(True))["debug"]
for variable in ("$host", "$request_time", "$upstream_response_time"):
with self.subTest(variable=variable):
self.assertIn(variable, body)
class TestLoggedPath(unittest.TestCase):
rendered: str
@classmethod
def setUpClass(cls) -> None:
cls.rendered = render(False)
def test_the_query_string_is_dropped(self):
for request_uri, expected in (
("/", "/"),
("/index.php?access_token=s3cr3t", "/index.php"),
("/api/v1/items?page=2&token=s3cr3t", "/api/v1/items"),
(
"/realms/x/login-actions/action-token?key=s3cr3t",
"/realms/x/login-actions/action-token",
),
):
with self.subTest(request_uri=request_uri):
self.assertEqual(logged_path(self.rendered, request_uri), expected)
def test_token_routes_are_redacted(self):
for request_uri, expected in (
("/password/reset/s3cr3t?email=a%40b.c", "/password/reset/[redacted]"),
("/i/confirm-email/s3cr3t/s3cr3t2", "/i/confirm-email/[redacted]"),
("/accounts/confirm-email/s3cr3t/", "/accounts/confirm-email/[redacted]"),
(
"/index.php/lostpassword/reset/form/s3cr3t/alice",
"/index.php/lostpassword/reset/form/[redacted]",
),
("/u/password-reset/s3cr3t", "/u/password-reset/[redacted]"),
("/reset-password/s3cr3t", "/reset-password/[redacted]"),
("/session/email-login/s3cr3t", "/session/email-login/[redacted]"),
("/u/activate-account/s3cr3t", "/u/activate-account/[redacted]"),
("/Password/Reset/s3cr3t", "/Password/Reset/[redacted]"),
):
with self.subTest(request_uri=request_uri):
logged = logged_path(self.rendered, request_uri)
self.assertEqual(logged, expected)
self.assertNotIn("s3cr3t", logged)
def test_lookalike_paths_are_logged_unchanged(self):
for request_uri in (
"/password/reset",
"/xpassword/reset/abc",
"/settings/password",
"/confirm-email",
):
with self.subTest(request_uri=request_uri):
self.assertEqual(logged_path(self.rendered, request_uri), request_uri)
def test_both_modes_share_the_same_map(self):
self.assertEqual(
logged_path(render(True), "/password/reset/s3cr3t?x=1"),
logged_path(self.rendered, "/password/reset/s3cr3t?x=1"),
)
if __name__ == "__main__":
unittest.main()