Parcourir la source

fix(security): stop leaking webhook tokens via httpx debug logging

  Settings → Support → Debug Logging elevated httpx/httpcore to DEBUG,
  which makes httpx log every outbound request URL. For Discord and
  generic webhook notifications the bearer token is embedded in the URL
  path, so users who turned on debug logging to capture a support bundle
  were writing their webhook tokens straight into bambuddy.log.

  Pin httpx/httpcore to WARNING regardless of the debug toggle. paho.mqtt
  still honours debug. Users who enabled debug logging while notifications
  were sending must rotate any exposed Discord/webhook URLs — the token
  is the path, so the whole URL has to be regenerated in the provider UI.
maziggy il y a 1 mois
Parent
commit
b71b721658
3 fichiers modifiés avec 14 ajouts et 15 suppressions
  1. 1 0
      CHANGELOG.md
  2. 8 12
      backend/app/api/routes/support.py
  3. 5 3
      backend/tests/unit/test_support_helpers.py

+ 1 - 0
CHANGELOG.md

@@ -19,6 +19,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **Plate-Clear Confirmation Disabled by Default** — New installs ship with Settings → Workflow → "Require Plate-Clear Confirmation" off. Multiple new users reported queued prints appearing to not start because the prompt was waiting for acknowledgement; opt in from Workflow if you want the confirmation gate.
 
 ### Fixed
+- **Webhook Tokens Leaked into Logs When Debug Logging Enabled (Security)** — Turning on Settings → Support → Debug Logging elevated the `httpx` and `httpcore` loggers to DEBUG, which caused httpx to log the full URL of every outbound HTTP request. For Discord notifications and generic webhook notifications, the URL *is* the secret — the bearer token is embedded in the path — so any user who enabled debug logging (typically to capture logs for a bug report) was writing their Discord webhook token to `bambuddy.log` and then pasting it into GitHub issues or support bundles. `httpx`/`httpcore` are now pinned to `WARNING` regardless of the debug toggle; `paho.mqtt` still honours debug. If you enabled debug logging while notifications were sending, rotate any exposed Discord/webhook URLs — the token is in the path, so the whole URL must be regenerated in the provider's UI.
 - **Queue Item Stuck in "Printing" When Start Command is Dropped** ([#967](https://github.com/maziggy/bambuddy/issues/967)) — If the physical printer dropped or ignored the MQTT `project_file` start command (same half-broken-session shape as #887/#936), the queue item was permanently orphaned in the `printing` status at 100% because the scheduler optimistically flipped the DB row to `printing` right after the publish succeeded locally and had no watchdog to revert it. Recovery required manually editing the SQLite `print_queue` table. A new watchdog now captures the printer's pre-dispatch state and polls for up to 45 s after `start_print()` returns; if the printer never transitions, the item is reverted to `pending` so the scheduler picks it up again, and the MQTT session is force-reconnected so the retry lands without a printer reboot. Thanks to @stringham for reporting.
 - **Queued Prints Require Printer Reboot to Start** ([#936](https://github.com/maziggy/bambuddy/issues/936)) — On some printers, a queued print would be uploaded via FTP and the `project_file` MQTT command would be sent, but the printer never transitioned out of `FINISH`/`IDLE` and required a power cycle to unstick — after which it often started a previously cancelled print rather than the intended one. Root cause is a half-broken MQTT session (same shape as #887): the printer keeps publishing telemetry so Bambuddy reports it as connected, but our publishes on the command topic never reach the firmware. Existing recovery only triggered via the developer-mode probe path, which skips printers that already have a known `developer_mode` value. The print-dispatch verifier now treats an unacknowledged `project_file` (state unchanged after 15 s) as the same "commands not reaching printer" signal and forces a fresh MQTT session so the next dispatch can land without a printer reboot. The existing dev-mode probe path is refactored to share the same helper.
 - **Clear Plate Confirmation Bypassed on Power Cycle** ([#961](https://github.com/maziggy/bambuddy/issues/961)) — With Auto Off enabled and another job queued, the smart plug would cut power when a print finished and immediately re-power when the scheduler saw the queue, at which point the printer booted fresh into `IDLE` and the next job auto-dispatched without the "Clear Plate & Start Next" confirmation. Root cause: the plate-cleared gate lived only in the in-memory `PrinterManager._plate_cleared` set, and the scheduler's idle check treated `IDLE` as always-idle regardless of whether a previous finish had been acknowledged — so the gate was lost across both Bambuddy restarts and the IDLE-on-boot state transition. The gate is now an `awaiting_plate_clear` column on the `printers` table, set by `on_print_complete` when a print finishes or fails, cleared by the `/printers/{id}/clear-plate` endpoint and by the scheduler when it dispatches the next job, and rehydrated from the DB into `PrinterManager` on startup. `_is_printer_idle` now short-circuits to not-idle whenever `require_plate_clear` is on and the printer is awaiting ack, regardless of the currently reported state — so the prompt survives Auto Off cycles, Bambuddy restarts, and the printer booting back into `IDLE`. The clear-plate endpoint no longer requires the printer to currently report `FINISH`/`FAILED` (it accepts the ack whenever the awaiting flag is set), and the Printers page widget prompts based on the flag rather than the reported state. Thanks to @miaopas for reporting.

+ 8 - 12
backend/app/api/routes/support.py

@@ -103,18 +103,14 @@ def _apply_log_level(debug: bool):
     for handler in root_logger.handlers:
         handler.setLevel(new_level)
 
-    # Also adjust third-party loggers
-    if debug:
-        logging.getLogger("sqlalchemy.engine").setLevel(logging.WARNING)
-        logging.getLogger("aiosqlite").setLevel(logging.WARNING)
-        logging.getLogger("httpcore").setLevel(logging.DEBUG)
-        logging.getLogger("httpx").setLevel(logging.DEBUG)
-        logging.getLogger("paho.mqtt").setLevel(logging.DEBUG)
-    else:
-        logging.getLogger("sqlalchemy.engine").setLevel(logging.WARNING)
-        logging.getLogger("httpcore").setLevel(logging.WARNING)
-        logging.getLogger("httpx").setLevel(logging.WARNING)
-        logging.getLogger("paho.mqtt").setLevel(logging.WARNING)
+    # Also adjust third-party loggers. httpx/httpcore stay pinned to WARNING
+    # even in debug mode — at INFO/DEBUG they log full request URLs, which
+    # leaks secrets embedded in webhook URLs (Discord, generic webhooks, etc.).
+    logging.getLogger("sqlalchemy.engine").setLevel(logging.WARNING)
+    logging.getLogger("aiosqlite").setLevel(logging.WARNING)
+    logging.getLogger("httpcore").setLevel(logging.WARNING)
+    logging.getLogger("httpx").setLevel(logging.WARNING)
+    logging.getLogger("paho.mqtt").setLevel(logging.DEBUG if debug else logging.WARNING)
 
     logger.info("Log level changed to %s", "DEBUG" if debug else "INFO")
 

+ 5 - 3
backend/tests/unit/test_support_helpers.py

@@ -35,15 +35,17 @@ class TestApplyLogLevel:
 
         assert logging.getLogger("aiosqlite").level == logging.WARNING
 
-    def test_debug_mode_enables_httpcore_debug(self):
-        """Verify httpcore stays at DEBUG in debug mode."""
+    def test_debug_mode_keeps_httpx_pinned_to_warning(self):
+        """httpx/httpcore must stay at WARNING even in debug mode — at INFO/DEBUG
+        they log full request URLs, leaking webhook tokens (Discord etc.)."""
         import logging
 
         from backend.app.api.routes.support import _apply_log_level
 
         _apply_log_level(True)
 
-        assert logging.getLogger("httpcore").level == logging.DEBUG
+        assert logging.getLogger("httpcore").level == logging.WARNING
+        assert logging.getLogger("httpx").level == logging.WARNING
 
     def test_non_debug_mode_suppresses_all_noisy_loggers(self):
         """Verify all noisy loggers are set to WARNING in non-debug mode."""