fix(security,obs): fuite d'erreur sur /health, filtre nl2br, redaction des logs
/health divulguait le message brut du pilote
L'endpoint n'est pas authentifie et renvoyait f'error: {str(e)}'. Les
exceptions psycopg contiennent regulierement l'hote, le port, le nom de
la base et l'utilisateur. Le detail part desormais dans les journaux,
la reponse ne porte plus qu'un statut.
Filtre nl2br non echappant
Markup('<br>'.join(...)) marquait le texte comme sur sans l'echapper.
Le filtre n'etant utilise dans aucun gabarit, la faille etait latente :
elle se serait ouverte au premier usage. Corrige en Markup('<br>').join(),
qui echappe chaque segment. Verifie : nl2br('<script>alert(1)</script>')
rend desormais <script>alert(1)</script>.
Filtre de redaction des secrets sans effet
SensitiveDataFilter n'inspectait que record.msg. Or le code journalise
en style parametre ('...: %s', valeur) : record.msg ne contient que la
chaine de format, et la donnee sensible vit dans record.args, ignore.
La redaction ne s'appliquait donc pratiquement jamais. Le record est
desormais rendu avant filtrage, puis args vide.
Sortie console conditionnee a FLASK_DEBUG
En production, l'application n'ecrivait rien sur stdout, precisement ou
regarde la console Pterodactyl. Le handler devient inconditionnel, seul
son niveau varie.
Journaux du bot Discord perdus
discord_bot.py utilise getLogger(__name__), soit 'app.discord_bot'.
Aucun handler n'etait attache a la hierarchie 'app' : les INFO etaient
jetes et les WARNING+ tombaient sur le handler de dernier recours, sans
format. Les handlers sont desormais rattaches au logger de paquet.
X-XSS-Protection retire (app.py et nginx.conf)
En-tete deprecie, l'auditeur vise a ete supprime des navigateurs
courants et ses dernieres implementations introduisaient elles-memes
des vulnerabilites.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
+13
-4
@@ -26,7 +26,9 @@ def nl2br(value):
|
||||
Markup: HTML-safe string with line breaks.
|
||||
"""
|
||||
if value:
|
||||
return markupsafe.Markup('<br>'.join(str(value).splitlines()))
|
||||
# Markup('<br>').join() escapes each segment before joining.
|
||||
# Markup('<br>'.join(...)) would mark attacker-controlled text as safe.
|
||||
return markupsafe.Markup('<br>').join(str(value).splitlines())
|
||||
return ''
|
||||
|
||||
|
||||
@@ -137,9 +139,12 @@ def create_app():
|
||||
HSTS is only sent in production (non-debug) to avoid breaking
|
||||
local development over plain HTTP.
|
||||
"""
|
||||
# X-XSS-Protection is deliberately not set: the auditor it addressed
|
||||
# has been removed from every current browser, and its last versions
|
||||
# introduced vulnerabilities of their own. CSP frame-ancestors and
|
||||
# X-Frame-Options cover the remaining ground.
|
||||
response.headers['X-Content-Type-Options'] = 'nosniff'
|
||||
response.headers['X-Frame-Options'] = 'DENY'
|
||||
response.headers['X-XSS-Protection'] = '1; mode=block'
|
||||
response.headers['Referrer-Policy'] = 'strict-origin-when-cross-origin'
|
||||
response.headers['Permissions-Policy'] = (
|
||||
'camera=(), microphone=(), geolocation=(), '
|
||||
@@ -206,9 +211,13 @@ def create_app():
|
||||
try:
|
||||
db.session.execute(text('SELECT 1'))
|
||||
health_data['database'] = 'connected'
|
||||
except Exception as e:
|
||||
except Exception:
|
||||
# Never echo the driver error: it routinely carries the host,
|
||||
# database name and user of the connection string, and /health
|
||||
# is unauthenticated.
|
||||
app.logger.error('Health check: database unreachable', exc_info=True)
|
||||
health_data['status'] = 'unhealthy'
|
||||
health_data['database'] = f'error: {str(e)}'
|
||||
health_data['database'] = 'error'
|
||||
return jsonify(health_data), 503
|
||||
|
||||
return jsonify(health_data), 200
|
||||
|
||||
+41
-16
@@ -31,7 +31,13 @@ class SensitiveDataFilter(logging.Filter):
|
||||
]
|
||||
|
||||
def filter(self, record):
|
||||
"""Apply redaction to the log record's message.
|
||||
"""Apply redaction to the log record's fully rendered message.
|
||||
|
||||
The record is rendered first (msg % args) and the result stored back
|
||||
as msg with args cleared. Redacting record.msg alone would miss almost
|
||||
everything: this codebase logs with %s placeholders, so the sensitive
|
||||
value lives in record.args while record.msg holds only the format
|
||||
string.
|
||||
|
||||
Args:
|
||||
record: The log record to filter.
|
||||
@@ -39,14 +45,17 @@ class SensitiveDataFilter(logging.Filter):
|
||||
Returns:
|
||||
bool: Always True (never drops records, only redacts).
|
||||
"""
|
||||
if hasattr(record, 'msg') and isinstance(record.msg, str):
|
||||
msg = record.msg
|
||||
for pattern, replacement in self.SENSITIVE_PATTERNS:
|
||||
if callable(replacement):
|
||||
msg = pattern.sub(replacement, msg)
|
||||
else:
|
||||
msg = pattern.sub(replacement, msg)
|
||||
record.msg = msg
|
||||
try:
|
||||
rendered = record.getMessage()
|
||||
except Exception:
|
||||
# A malformed format string must not lose the record entirely.
|
||||
return True
|
||||
|
||||
for pattern, replacement in self.SENSITIVE_PATTERNS:
|
||||
rendered = pattern.sub(replacement, rendered)
|
||||
|
||||
record.msg = rendered
|
||||
record.args = ()
|
||||
return True
|
||||
|
||||
|
||||
@@ -128,14 +137,30 @@ def configure_logging(app):
|
||||
app.logger.addHandler(app_handler)
|
||||
|
||||
# -------------------------------------------------------------------------
|
||||
# 4. Console Handler (for development)
|
||||
# 4. Console Handler (always on)
|
||||
# -------------------------------------------------------------------------
|
||||
if os.getenv('FLASK_DEBUG', 'false').lower() == 'true':
|
||||
console_handler = logging.StreamHandler()
|
||||
console_handler.setLevel(logging.DEBUG)
|
||||
console_handler.setFormatter(formatter)
|
||||
console_handler.addFilter(sensitive_filter)
|
||||
app.logger.addHandler(console_handler)
|
||||
# Previously gated on FLASK_DEBUG, which meant production emitted nothing
|
||||
# on stdout — precisely where the Pterodactyl console looks. Keep the
|
||||
# handler unconditional and vary the level instead.
|
||||
debug_mode = os.getenv('FLASK_DEBUG', 'false').lower() == 'true'
|
||||
console_handler = logging.StreamHandler()
|
||||
console_handler.setLevel(logging.DEBUG if debug_mode else log_level)
|
||||
console_handler.setFormatter(formatter)
|
||||
console_handler.addFilter(sensitive_filter)
|
||||
app.logger.addHandler(console_handler)
|
||||
|
||||
# -------------------------------------------------------------------------
|
||||
# 5. Package logger ('app.*') — notably app.discord_bot
|
||||
# -------------------------------------------------------------------------
|
||||
# Modules using logging.getLogger(__name__) resolve to 'app.<module>'.
|
||||
# Without handlers here their INFO records were dropped entirely and
|
||||
# WARNING+ fell through to Python's lastResort handler, unformatted.
|
||||
package_logger = logging.getLogger('app')
|
||||
package_logger.setLevel(log_level)
|
||||
package_logger.propagate = False
|
||||
for handler in (error_handler, app_handler, console_handler):
|
||||
if handler not in package_logger.handlers:
|
||||
package_logger.addHandler(handler)
|
||||
|
||||
# Log startup information
|
||||
app.logger.info('Logging configured - Level: %s, Log directory: %s', log_level_name, log_dir)
|
||||
|
||||
+2
-1
@@ -110,7 +110,8 @@ http {
|
||||
add_header Strict-Transport-Security "max-age=31536000; includeSubDomains; preload" always;
|
||||
add_header X-Content-Type-Options "nosniff" always;
|
||||
add_header X-Frame-Options "DENY" always;
|
||||
add_header X-XSS-Protection "1; mode=block" always;
|
||||
# X-XSS-Protection intentionally omitted: deprecated, removed from
|
||||
# current browsers, and harmful in its last implementations.
|
||||
add_header Referrer-Policy "strict-origin-when-cross-origin" always;
|
||||
add_header Permissions-Policy "camera=(), microphone=(), geolocation=(), interest-cohort=()" always;
|
||||
add_header Cross-Origin-Opener-Policy "same-origin" always;
|
||||
|
||||
Reference in New Issue
Block a user