Skip to content

Commit b70264a

Browse files
Sanitize peer-controlled addresses in cluster debug log lines
The find_leader and _query_leader debug logs interpolated ``node.address`` (and the equivalent ``address`` parameter) directly via ``%s``. The sibling ``errors.append`` / ``ClusterError`` paths route the same value through ``_sanitize_display_text`` to strip CRLF / control chars / U+2028 / U+2029 / bidi formatting characters. The asymmetry meant a hostile peer could inject log lines into operator log streams via the DEBUG channel even though the same string was sanitised at every other display surface. The inline comment at the existing aggregate-error sanitise sites already commits the module to "sanitise at every user-facing boundary" — debug logs are an operator- facing boundary too. Sanitize: - ``find_leader`` no-leader / timed-out / transport-error debug lines. - ``_query_leader`` malformed-redirect debug lines. - ``_check_redirect`` policy-rejected debug line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c0cc5ad commit b70264a

1 file changed

Lines changed: 21 additions & 16 deletions

File tree

src/dqliteclient/cluster.py

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,9 @@ def _check_redirect(self, address: str) -> None:
206206
# at DEBUG so SSRF-style attempts or policy
207207
# misconfigurations are traceable from logs alone,
208208
# not only through an exception stack.
209-
logger.debug("cluster: redirect rejected by policy to=%s", address)
209+
logger.debug(
210+
"cluster: redirect rejected by policy to=%s", _sanitize_display_text(address)
211+
)
210212
raise ClusterPolicyError(f"Leader redirect to {address!r} rejected by redirect_policy")
211213

212214
async def find_leader(self, *, trust_server_heartbeat: bool = False) -> str:
@@ -356,41 +358,44 @@ async def _find_leader_impl(self, *, trust_server_heartbeat: bool) -> str:
356358
# ``"Could not find leader. Errors: "`` message — the
357359
# operator cannot tell "all nodes returned no-leader"
358360
# from "all nodes failed unreachable".
361+
# Sanitise the address before interpolating into both
362+
# the aggregate error AND the debug log line so a
363+
# hostile leader cannot inject CRLF / control-chars /
364+
# line separators into either user-facing surface.
365+
# ``node.address`` flows through
366+
# ``LeaderResponse.address`` which is deliberately NOT
367+
# sanitised at wire decode (allowlist semantics) —
368+
# sanitisation must happen at every display surface,
369+
# debug logs included.
370+
_safe_addr = _sanitize_display_text(node.address)
359371
logger.debug(
360372
"find_leader: %s reports no leader known (%d/%d)",
361-
node.address,
373+
_safe_addr,
362374
idx + 1,
363375
total_nodes,
364376
)
365-
# Sanitise the address before interpolating into the
366-
# aggregate error so a hostile leader cannot inject
367-
# CRLF / control-chars / line separators into log
368-
# output. ``node.address`` flows through
369-
# ``LeaderResponse.address`` which is deliberately NOT
370-
# sanitised at wire decode (allowlist semantics) —
371-
# sanitisation must happen at the user-facing error
372-
# boundary.
373-
_safe_addr = _sanitize_display_text(node.address)
374377
errors.append(f"{_safe_addr}: no leader known")
375378
continue
376379
except TimeoutError as e:
380+
_safe_addr = _sanitize_display_text(node.address)
377381
logger.debug(
378382
"find_leader: %s timed out after %.3fs (%d/%d)",
379-
node.address,
383+
_safe_addr,
380384
self._timeout,
381385
idx + 1,
382386
total_nodes,
383387
)
384-
errors.append(f"{_sanitize_display_text(node.address)}: timed out")
388+
errors.append(f"{_safe_addr}: timed out")
385389
last_exc = e
386390
continue
387391
except (DqliteConnectionError, ProtocolError, OperationalError, OSError) as e:
388392
# Narrow the catch so programming bugs (TypeError, KeyError,
389393
# etc.) propagate directly instead of being stringified into
390394
# a retryable ClusterError.
395+
_safe_addr = _sanitize_display_text(node.address)
391396
logger.debug(
392397
"find_leader: %s failed with %s: %s (%d/%d)",
393-
node.address,
398+
_safe_addr,
394399
type(e).__name__,
395400
_truncate_error(str(e)),
396401
idx + 1,
@@ -452,7 +457,7 @@ async def _query_leader(
452457
# per-node probes.
453458
logger.debug(
454459
"query_leader: %s returned malformed redirect (node_id=%s, address=%r)",
455-
address,
460+
_sanitize_display_text(address),
456461
node_id,
457462
_sanitize_display_text(leader_addr),
458463
)
@@ -469,7 +474,7 @@ async def _query_leader(
469474
# is not trusted without a matching id.
470475
logger.debug(
471476
"query_leader: %s returned malformed redirect (node_id=0, address=%r)",
472-
address,
477+
_sanitize_display_text(address),
473478
_sanitize_display_text(leader_addr),
474479
)
475480
raise ProtocolError(

0 commit comments

Comments
 (0)