From 744275d17f6f67829cb2eb18862d8956f7362118 Mon Sep 17 00:00:00 2001 From: Lukas Date: Mon, 4 May 2026 07:20:20 +0200 Subject: [PATCH] backup --- CONTRIBUTING.md | 26 ++++++++++++++++++++++++ Docs/Tasks.md | 24 ---------------------- backend/app/exceptions.py | 18 ++++++++--------- backend/app/utils/display_sanitizer.py | 28 ++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 34 deletions(-) create mode 100644 backend/app/utils/display_sanitizer.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6a77372..12de40d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -112,6 +112,32 @@ The CI pipeline enforces the same 80% minimum coverage threshold. --- +## Security Rules + +### Never echo raw user input in error messages + +User-supplied values (jail names, filter names, action names, IPs, filenames, etc.) +MUST be sanitized before interpolation into any string that may be rendered in an +HTML context (error messages, admin UI, email notifications). + +Use the `sanitize_for_display()` helper from `app.utils.display_sanitizer`: + +```python +from app.utils.display_sanitizer import sanitize_for_display + +# Good: sanitized before display +super().__init__(f"Jail not found: {sanitize_for_display(name)!r}") + +# Bad: raw user input echoed — XSS vector if rendered as HTML +super().__init__(f"Jail not found: {name!r}") +``` + +This rule applies even when the value has been validated: validation checks the +format, not the rendering context. JSON API responses do NOT need sanitization +(JSON is not HTML); apply it only at HTML render boundaries. + +--- + ## Stack | Layer | Stack | diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 00561bd..e69de29 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,24 +0,0 @@ -### Issue #69: LOW - Jail Names Echoed in Error Messages Without Sanitization - -**Where found**: -- `backend/app/exceptions.py:138,351` – jail names interpolated directly into error strings - -**Why this is needed**: -Although Python's `repr()` provides basic escaping, user-supplied jail names are reflected back in error messages. If these messages are ever rendered in an HTML context (e.g., a future admin UI or email notification), they become XSS vectors. They also act as confirmation oracles when combined with timing attacks. - -**Goal**: -Error messages referencing user input are sanitized before inclusion. - -**What to do**: -1. Pass user-supplied values through a dedicated `sanitize_for_display()` helper before interpolation. -2. Ensure the helper strips or escapes HTML special characters. -3. For API responses, always return the original (validated) field name rather than the raw user input. - -**Possible traps and issues**: -- Over-escaping in JSON responses is not needed (JSON is not HTML); apply sanitization only at HTML render boundaries. - -**Docs changes needed**: -- `CONTRIBUTING.md`: document the rule that user input must not be echoed raw in messages. - -**Doc references**: -- `backend/app/exceptions.py` \ No newline at end of file diff --git a/backend/app/exceptions.py b/backend/app/exceptions.py index 4234757..f8a8682 100644 --- a/backend/app/exceptions.py +++ b/backend/app/exceptions.py @@ -41,6 +41,8 @@ from __future__ import annotations from typing import TYPE_CHECKING +from app.utils.display_sanitizer import sanitize_for_display + if TYPE_CHECKING: from app.models.response import ErrorMetadata @@ -137,7 +139,7 @@ class JailNotFoundError(NotFoundError): def __init__(self, name: str) -> None: self.name = name - super().__init__(f"Jail not found: {name!r}") + super().__init__(f"Jail not found: {sanitize_for_display(name)!r}") def get_error_metadata(self) -> ErrorMetadata: return {"jail_name": self.name} @@ -277,8 +279,7 @@ class FilterRegexTooLongError(BadRequestError): self.max_length = max_length self.actual_length = len(pattern) super().__init__( - f"Regex pattern exceeds maximum length of {max_length} characters: " - f"{self.actual_length} provided" + f"Regex pattern exceeds maximum length of {max_length} characters: {self.actual_length} provided" ) def get_error_metadata(self) -> ErrorMetadata: @@ -318,7 +319,7 @@ class JailNotFoundInConfigError(NotFoundError): def __init__(self, name: str) -> None: self.name = name - super().__init__(f"Jail not found in config: {name!r}") + super().__init__(f"Jail not found in config: {sanitize_for_display(name)!r}") def get_error_metadata(self) -> ErrorMetadata: return {"jail_name": self.name} @@ -350,7 +351,7 @@ class JailAlreadyActiveError(ConflictError): def __init__(self, name: str) -> None: self.name = name - super().__init__(f"Jail is already active: {name!r}") + super().__init__(f"Jail is already active: {sanitize_for_display(name)!r}") def get_error_metadata(self) -> ErrorMetadata: return {"jail_name": self.name} @@ -363,7 +364,7 @@ class JailAlreadyInactiveError(ConflictError): def __init__(self, name: str) -> None: self.name = name - super().__init__(f"Jail is already inactive: {name!r}") + super().__init__(f"Jail is already inactive: {sanitize_for_display(name)!r}") def get_error_metadata(self) -> ErrorMetadata: return {"jail_name": self.name} @@ -463,7 +464,6 @@ class ActionReadonlyError(ConflictError): return {"action_name": self.name} - class SetupAlreadyCompleteError(ConflictError): """Raised when attempting to run setup when it has already been completed.""" @@ -473,7 +473,6 @@ class SetupAlreadyCompleteError(ConflictError): super().__init__("Setup has already been completed.") - class BlocklistSourceNotFoundError(NotFoundError): """Raised when a blocklist source is not found.""" @@ -495,8 +494,7 @@ class BlocklistSourceHasLogsError(ConflictError): def __init__(self, source_id: int) -> None: self.source_id = source_id super().__init__( - f"Blocklist source {source_id} cannot be deleted because it has import logs. " - "Delete the import logs first." + f"Blocklist source {source_id} cannot be deleted because it has import logs. Delete the import logs first." ) def get_error_metadata(self) -> ErrorMetadata: diff --git a/backend/app/utils/display_sanitizer.py b/backend/app/utils/display_sanitizer.py new file mode 100644 index 0000000..1c139c3 --- /dev/null +++ b/backend/app/utils/display_sanitizer.py @@ -0,0 +1,28 @@ +"""Display sanitization utilities for HTML render contexts. + +All user-supplied values echoed in error messages or other HTML-rendered +output MUST be sanitized first. This module provides the canonical +sanitize_for_display() function. +""" + +from __future__ import annotations + +import html + + +def sanitize_for_display(value: str) -> str: + """Escape HTML special characters in user-supplied strings. + + Use this before interpolating user input into any string that will be + rendered in an HTML context (e.g. error messages, admin UI, email). + + Does NOT over-escape: JSON responses are not HTML contexts and do not + need this treatment. Apply sanitization only at HTML render boundaries. + + Args: + value: Raw user-supplied string. + + Returns: + The string with HTML special characters escaped. + """ + return html.escape(value, quote=True)