Security hardening (maziggy/bambuddy-security #N)

Subprocess output and user-supplied URLs are scrubbed of credentials
    before they reach the application log. Adds a shared redaction helper in
    core/logging_filters and routes the existing support-bundle sanitizer
    through the same pattern.
This commit is contained in:
maziggy
2026-08-02 09:36:38 +02:00
7 changed files with 182 additions and 13 deletions
+1 -1
View File
@@ -29,7 +29,7 @@ All notable changes to Bambuddy will be documented in this file.
### Fixed
- **P1-series archives kept the worse finish photo when the timelapse arrived late (#2704 follow-up)** — When a print records a timelapse, Bambuddy prefers the video's last frame as the finish photo: the firmware stops recording after the toolhead parks but before the end G-code drops the bed, so it frames the finished print properly, where a live camera grab at that moment catches an already-lowered plate. Bambuddy waited 60 seconds for the video and then gave up, because the print-complete notification is waiting on that photo and holding a notification for minutes is worse than sending it with the live grab. On P1-series printers the video usually arrives later than that — they write MJPEG AVI instead of H.264 MP4 and serve it slowly, so across the support bundles their median was 33 seconds but the 90th percentile was 167 and the slowest observed was 546; every other model finished inside 26 seconds. The result was that the printers most in need of the better photo were the ones that never got it. **Fix.** The notification still goes out on the same 60-second bound with the live grab, so nothing gets slower. If the video was still on its way when that bound expired, Bambuddy now keeps waiting in the background and adds the extracted frame to the archive when it lands, at the front of the photo list so opening the gallery shows it first. The live grab is kept rather than replaced — the notification that already went out links to that exact file, and removing it would leave a broken image in Discord or Telegram. Covered by tests for the ordering, the longer budget, idempotency and the cases where the video never arrives.
- **Timelapses that never got attached, and a Scan button that could not find them (#2704, reporter @anthonyma94)** — Timelapse was on for the print, the video never arrived in the archive, and pressing **Scan for Timelapse** afterwards turned up nothing. Measured across 247 support bundles, this was not rare: of 457 automatic scans only 262 ever attached a video. **Root cause, part one.** The scan looked four times, at 5, 10, 20 and 30 seconds, then stopped. The printer writes the video only after the print ends and a long print makes a large file, so it often arrived after the last look — the attempt that found the video was the first one 272 times and then 17 / 13 / 13, a flat tail against the cutoff rather than a decaying one. What ran after those four attempts was a fallback that searched for the print's name inside the video filename; Bambu firmware only ever writes `video_<timestamp>`, so in 247 bundles it fired 159 times and matched exactly zero. **Root cause, part two.** The manual Scan button had no such snapshot to work from and matched by filename timestamp, by FTP modification time, or by there being exactly one video on the printer — all of which read a clock the printer cannot set, because a printer in LAN Only mode never reaches Bambu's time server. The reporter's P1S was six and a half days out, which defeats every one of those. **Fix.** The automatic scan now polls for several minutes instead of giving up after about a minute, and the name-match fallback is gone. The list of videos present when the print started is saved with the archive, so the comparison survives a Bambuddy restart mid-print and the manual Scan button can use it too — same clock-independent comparison, no timestamps anywhere. When a previous print's video lands late and two files look new, the one already attached to another archive is ruled out by name rather than by picking whichever the printer listed first, which could attach the wrong video. **Bambuddy now deletes a timelapse from the printer once it has been archived**, which keeps the printer's folder down to unclaimed videos and stops P1-series cards filling up with AVIs; your copy is in the archive, where you can watch, edit, download or remove it. That delete only happens after the transfer has been checked against the size the printer reported — which also fixes a silent truncation: an FTPS transfer that ended early produced a partial video that was attached as though it were complete. Because the first look happens seconds after the print ends — while the printer may still be writing the video — the file is also re-checked afterwards and only accepted once it has stopped growing, so a partial video is never mistaken for a finished one and the printer's copy is never removed on the strength of one. Wiki updated. Covered by tests for candidate selection, the download check gating the delete, the poll bounds, baseline persistence and the manual scan.
- **Timelapses that never got attached, and a Scan button that could not find them (#2704)** — Timelapse was on for the print, the video never arrived in the archive, and pressing **Scan for Timelapse** afterwards turned up nothing. Measured across 247 support bundles, this was not rare: of 457 automatic scans only 262 ever attached a video. **Root cause, part one.** The scan looked four times, at 5, 10, 20 and 30 seconds, then stopped. The printer writes the video only after the print ends and a long print makes a large file, so it often arrived after the last look — the attempt that found the video was the first one 272 times and then 17 / 13 / 13, a flat tail against the cutoff rather than a decaying one. What ran after those four attempts was a fallback that searched for the print's name inside the video filename; Bambu firmware only ever writes `video_<timestamp>`, so in 247 bundles it fired 159 times and matched exactly zero. **Root cause, part two.** The manual Scan button had no such snapshot to work from and matched by filename timestamp, by FTP modification time, or by there being exactly one video on the printer — all of which read a clock the printer cannot set, because a printer in LAN Only mode never reaches Bambu's time server. The reporter's P1S was six and a half days out, which defeats every one of those. **Fix.** The automatic scan now polls for several minutes instead of giving up after about a minute, and the name-match fallback is gone. The list of videos present when the print started is saved with the archive, so the comparison survives a Bambuddy restart mid-print and the manual Scan button can use it too — same clock-independent comparison, no timestamps anywhere. When a previous print's video lands late and two files look new, the one already attached to another archive is ruled out by name rather than by picking whichever the printer listed first, which could attach the wrong video. **Bambuddy now deletes a timelapse from the printer once it has been archived**, which keeps the printer's folder down to unclaimed videos and stops P1-series cards filling up with AVIs; your copy is in the archive, where you can watch, edit, download or remove it. That delete only happens after the transfer has been checked against the size the printer reported — which also fixes a silent truncation: an FTPS transfer that ended early produced a partial video that was attached as though it were complete. Because the first look happens seconds after the print ends — while the printer may still be writing the video — the file is also re-checked afterwards and only accepted once it has stopped growing, so a partial video is never mistaken for a finished one and the printer's copy is never removed on the strength of one. Wiki updated. Covered by tests for candidate selection, the download check gating the delete, the poll bounds, baseline persistence and the manual scan.
- **A printer that refuses Bambuddy's access code now says so, instead of reconnecting silently forever (#2698, reporter @djepsylon)** — A printer whose access code or serial was wrong produced no explanation anywhere: the connection attempt was refused, and the only trace was a warning every 30 seconds reading `MQTT disconnected: rc=Unspecified error` — the same line you get from a printer that is simply switched off. The failure branch of the MQTT connect callback set "not connected" and discarded the reason code the printer had just sent, so the one piece of evidence that would have named the cause never reached the log, the support bundle, or the UI. In the report behind this fix, one of three printers had been in that loop for the entire capture and nothing said why. **Fix.** A refused connection is now logged with the printer's own reason ("Not authorized", "Bad user name or password") and, for those two, the remedy — the access code is regenerated every time LAN Only or Developer Mode is toggled, so it has to be re-read from the printer's screen. The reason is kept on the connection, so the **Connection Diagnostic**'s *Printer credentials* check now states plainly that the printer refused the credentials when that is what happened, and falls back to hedged wording when all Bambuddy knows is that there is no session — previously it asserted "the access code is most likely wrong" even for a printer that was merely rebooting or already at its connection limit. The same reason is returned by the pre-add connection test. The access code itself is never written to the log. Translated in all locales; wiki updated. Covered by tests for both refusal codes, the clearing of the reason on a successful reconnect, the diagnostic's reason plumbing, and the two UI variants.
- **A2L AMS filament showed as "?" in Bambu Studio through the Virtual Printer, and manual filament picks reverted (#2697, reporter @qoatzelcoat)** — Every slot of the A2L's AMS Lite rendered as an empty question mark in the slicer's Device tab while Bambuddy's own AMS card showed type, colour and spool correctly; setting a filament by hand in Studio held for a second and then snapped back to "?". **Root cause.** The A2L reports its AMS Lite as physical unit id 16, but packs the slots' presence bits at bit base 24 — so Bambuddy normalises the id to 6 at the MQTT ingest boundary and every internal reader gets the right bits. The Virtual Printer's bridge, however, parses the printer's raw payload itself (by design — the slicer-facing cache has to keep the physical ids, since Bambu Studio addresses the Lite as 16) and so still held id 16 when it ran the shared empty-slot cleanup. That cleanup read bits 64-67, where nothing is ever set, concluded all four slots were empty and wiped `tray_type`, `tray_color`, `tray_info_idx` and the RFID fields from the copy sent to the slicer — once per second, which is also why a manual pick could not survive. **Fix.** The presence-bit helper now folds the physical id 16 onto the same bit base as the normalised 6, so it computes bits 24-27 whichever id reaches it; the cached ids the slicer sees are left untouched. Only the A2L was affected — every other AMS type already reached the helper with an id whose bit base was correct, and Bambuddy's own printer card was correct throughout. Confirmed against the reporter's debug log, which shows the cleanup clearing slots at bits 64-67. Covered by tests pinning the bit base for both ids and a bridge-level regression test built from the reporter's capture.
- **Bambu Cloud sign-in with a TOTP (authenticator app) account always failed with "Invalid code" (#2696, reporter @cmerkle)** — Every TOTP verification was rejected regardless of the code. Bambu Lab added double-submit CSRF protection to the `bambulab.com` web origin, which is where — and only where — Bambuddy posts the two-factor code; the endpoint refused the request with `403 CSRF error: missing_cookie` **before evaluating the code at all**, and Bambuddy surfaced that as "Invalid code". Reproduced against the live endpoint with a deliberately invalid key: a bare POST returns `missing_cookie`, `GET /api/csrf` mints a `bbl_csrf_token` cookie, a POST carrying only that cookie returns `missing_header`, and a POST carrying the cookie plus an `x-bbl-csrf-token` header reaches application logic. Bambuddy now performs that handshake before submitting the code. Note that landing on the sign-in page first — the intuitive fix — does **not** work: that page sets only Cloudflare's `__cf_bm`. **Also fixed:** a CSRF refusal no longer masquerades as a wrong code; it now says the code was never checked, so nobody else loses an evening to clock drift and leading-zero theories. Only TOTP sign-ins were affected — every other cloud call, including the email-code two-factor path, goes to `api.bambulab.com`, which is not gated, and existing stored tokens kept working throughout. Covered by tests that pin the exact header name and the origin used per region.
+7
View File
@@ -19,6 +19,7 @@ from backend.app.core.auth import (
create_camera_stream_token,
)
from backend.app.core.database import get_db
from backend.app.core.logging_filters import redact_url_credentials
from backend.app.core.permissions import Permission
from backend.app.models.printer import Printer
from backend.app.models.user import User
@@ -276,9 +277,15 @@ def _summarize_ffmpeg_stderr(text: str | None) -> str:
any actual error message. Logging the full banner on every retry floods
the log (hundreds of lines per failed stream). This filter drops the
banner and caps output at the last 10 meaningful lines.
Credentials are masked here rather than at each ``logger`` call because
this is the one funnel every stderr log in this module passes through.
ffmpeg echoes the RTSP input URL back in its ``Input #0`` line, which
carries the printer access code.
"""
if not text:
return ""
text = redact_url_credentials(text) or ""
banner_prefixes = (
"ffmpeg version ",
" built with ",
+34 -1
View File
@@ -1,4 +1,4 @@
"""Logging filters for the Bambuddy log pipeline.
"""Logging filters and redaction helpers for the Bambuddy log pipeline.
Holds two filters: ``WriteRequestsOnlyFilter`` keeps the file-side
uvicorn access log focused on state-changing HTTP methods, and
@@ -6,12 +6,45 @@ uvicorn access log focused on state-changing HTTP methods, and
caused by Starlette's ``BaseHTTPMiddleware`` cancellation propagation
(see the filter's docstring for details). Both live here so tests can
import them without pulling in ``backend.app.main``'s startup graph.
Also holds :data:`URL_CREDENTIALS_PATTERN` and
:func:`redact_url_credentials`, the single place where the shape of a
credentialed URL is defined for the whole backend.
"""
from __future__ import annotations
import asyncio
import logging
import re
# ``scheme://user:secret@host`` — the only URL shape that carries a secret.
# Both userinfo parts exclude ``/`` so the match can never run past the
# authority into the path, and exclude whitespace so a wrapped log line can't
# glue two URLs together. ``secret`` is otherwise unrestricted and greedy so
# it reaches the *last* ``@`` before the path, which is where RFC 3986 ends
# the userinfo — that keeps an unescaped ``@`` inside a password (legal in an
# external camera URL) from leaving its tail in the log. Named groups let
# callers choose how much to mask: the log pipeline keeps the username, the
# support-bundle sanitizer drops it (see ``log_reader.sanitize_log_content``).
URL_CREDENTIALS_PATTERN = re.compile(r"(?P<scheme>[a-zA-Z][a-zA-Z0-9+.\-]*://)(?P<user>[^/:@\s]+):(?P<secret>[^/\s]+)@")
def redact_url_credentials(text: str | None) -> str | None:
"""Mask the password in every ``scheme://user:secret@host`` URL in *text*.
Subprocesses echo their input URL back at us — ffmpeg prints the RTSP
input in its ``Input #0`` line, so logging its stderr verbatim publishes
the printer access code (or an external camera's password) into
``bambuddy.log``, which users routinely attach to public issues.
The username, host, port and path survive so the line stays useful for
diagnosis; only the secret is replaced. Returns *text* unchanged when
there is nothing to mask, including ``None``/``""``.
"""
if not text or "://" not in text or "@" not in text:
return text
return URL_CREDENTIALS_PATTERN.sub(r"\g<scheme>\g<user>:[REDACTED]@", text)
class WriteRequestsOnlyFilter(logging.Filter):
+4 -1
View File
@@ -16,6 +16,8 @@ import uuid
from datetime import datetime
from pathlib import Path
from backend.app.core.logging_filters import redact_url_credentials
logger = logging.getLogger(__name__)
# JPEG markers
@@ -608,7 +610,8 @@ async def capture_camera_frame_bytes(
logger.info("Successfully captured camera frame bytes: %s bytes", len(stdout))
return stdout
else:
stderr_text = stderr.decode() if stderr else "Unknown error"
# ffmpeg echoes the RTSP input URL, which carries the access code.
stderr_text = redact_url_credentials(stderr.decode()) if stderr else "Unknown error"
logger.error("ffmpeg frame bytes capture failed (code %s): %s", process.returncode, stderr_text[:200])
return None
+18 -8
View File
@@ -17,6 +17,8 @@ from urllib.parse import urlparse
import aiohttp
from backend.app.core.logging_filters import redact_url_credentials
logger = logging.getLogger(__name__)
@@ -195,9 +197,15 @@ async def capture_frame(
JPEG bytes or None on failure
"""
if snapshot_url:
logger.debug("capture_frame using snapshot override url=%s...", snapshot_url[:50])
# Redact before truncating — slicing first can cut the URL short of the
# ``@`` the pattern anchors on and leave the password in the log.
logger.debug("capture_frame using snapshot override url=%s...", redact_url_credentials(snapshot_url)[:50])
return await _capture_snapshot(snapshot_url, timeout)
logger.debug("capture_frame called: type=%s, url=%s...", camera_type, url[:50] if url else "None")
logger.debug(
"capture_frame called: type=%s, url=%s...",
camera_type,
redact_url_credentials(url)[:50] if url else "None",
)
if camera_type == "mjpeg":
return await _capture_mjpeg_frame(url, timeout)
elif camera_type == "rtsp":
@@ -311,7 +319,7 @@ async def _capture_mjpeg_frame(url: str, timeout: int) -> bytes | None:
"""
safe_url = _sanitize_camera_url(url, ("http", "https"))
if not safe_url:
logger.error("Invalid MJPEG URL format: %s...", url[:50])
logger.error("Invalid MJPEG URL format: %s...", redact_url_credentials(url)[:50])
return None
jpeg_start = b"\xff\xd8"
@@ -438,7 +446,8 @@ async def _capture_rtsp_frame(url: str, timeout: int) -> bytes | None:
)
if process.returncode != 0:
logger.error("ffmpeg RTSP capture failed: %s", stderr.decode()[:200])
# ffmpeg echoes the RTSP input URL, which carries the camera password.
logger.error("ffmpeg RTSP capture failed: %s", redact_url_credentials(stderr.decode())[:200])
return None
if not stdout or len(stdout) < 100:
@@ -504,7 +513,7 @@ async def _capture_snapshot(url: str, timeout: int) -> bytes | None:
# Sanitize URL - returns reconstructed URL from validated components
safe_url = _sanitize_camera_url(url, ("http", "https"))
if not safe_url:
logger.error("Invalid snapshot URL format: %s...", url[:50])
logger.error("Invalid snapshot URL format: %s...", redact_url_credentials(url)[:50])
return None
try:
@@ -559,7 +568,7 @@ async def test_connection(url: str, camera_type: str) -> dict:
Returns:
Dict with {success: bool, error?: str, resolution?: str}
"""
logger.info("Testing camera connection: type=%s, url=%s...", camera_type, url[:50])
logger.info("Testing camera connection: type=%s, url=%s...", camera_type, redact_url_credentials(url)[:50])
try:
frame = await capture_frame(url, camera_type, timeout=10)
logger.info("Capture result: %s bytes", len(frame) if frame else 0)
@@ -700,7 +709,7 @@ async def _stream_mjpeg(url: str) -> AsyncGenerator[bytes, None]:
# Sanitize URL - returns reconstructed URL from validated components
safe_url = _sanitize_camera_url(url, ("http", "https"))
if not safe_url:
logger.error("Invalid MJPEG stream URL: %s...", url[:50])
logger.error("Invalid MJPEG stream URL: %s...", redact_url_credentials(url)[:50])
return
try:
@@ -837,7 +846,8 @@ async def _stream_rtsp(
await asyncio.sleep(0.1)
if process.returncode is not None:
stderr = await process.stderr.read()
logger.error("ffmpeg RTSP stream failed immediately: %s", stderr.decode()[:300])
# ffmpeg echoes the RTSP input URL, which carries the camera password.
logger.error("ffmpeg RTSP stream failed immediately: %s", redact_url_credentials(stderr.decode())[:300])
return
buffer = b""
+6 -2
View File
@@ -14,6 +14,7 @@ from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncSession
from backend.app.core.config import settings
from backend.app.core.logging_filters import URL_CREDENTIALS_PATTERN
from backend.app.models.printer import Printer
from backend.app.models.settings import Settings
from backend.app.models.user import User
@@ -168,8 +169,11 @@ def sanitize_log_content(content: str, sensitive_strings: dict[str, str] | None
continue # Skip very short strings to prevent over-redaction
content = re.sub(re.escape(value), label, content)
# Replace credentials in URLs (e.g. http://user:pass@host, rtsps://bblp:code@host)
content = re.sub(r"((?:https?|rtsps?)://)[^/:@\s]+:[^/@\s]+@", r"\1[CREDENTIALS]@", content)
# Replace credentials in URLs (e.g. http://user:pass@host, rtsps://bblp:code@host).
# Shares its pattern with the log-pipeline redaction in ``core.logging_filters`` so
# the two can't drift; the bundle drops the username too, where the live log keeps
# it for diagnosis.
content = URL_CREDENTIALS_PATTERN.sub(r"\g<scheme>[CREDENTIALS]@", content)
# Replace email addresses
content = re.sub(r"\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b", "[EMAIL]", content)
@@ -0,0 +1,112 @@
"""Credentials must never reach bambuddy.log.
Subprocesses echo their input URL back at us: ffmpeg prints the RTSP input in
its ``Input #0`` line, so logging its stderr verbatim published the printer
access code (or an external camera's password) into the log file — which users
routinely attach to public GitHub issues.
These cover the shared helper plus the two funnels that carry subprocess output
into the log.
"""
import asyncio
from backend.app.api.routes.camera import _read_ffmpeg_stderr, _summarize_ffmpeg_stderr
from backend.app.core.logging_filters import redact_url_credentials
from backend.app.services.log_reader import sanitize_log_content
# What ffmpeg actually prints for the camera's local TLS-proxy input. The
# access code sits in the userinfo of the URL it quotes back.
FFMPEG_INPUT_LINE = "Input #0, rtsp, from 'rtsp://bblp:38A4KQ2P@127.0.0.1:48521/streaming/live/1':"
class TestRedactUrlCredentials:
def test_masks_the_printer_access_code(self):
result = redact_url_credentials(FFMPEG_INPUT_LINE)
assert "38A4KQ2P" not in result
assert result == "Input #0, rtsp, from 'rtsp://bblp:[REDACTED]@127.0.0.1:48521/streaming/live/1':"
def test_keeps_everything_that_is_not_the_secret(self):
"""Host, port, path and username stay — the line has to remain diagnosable."""
result = redact_url_credentials("rtsp://admin:hunter2@192.168.1.50:554/stream1")
assert result == "rtsp://admin:[REDACTED]@192.168.1.50:554/stream1"
def test_masks_every_scheme_not_just_the_ones_we_use_today(self):
for url, expected in (
("http://user:pw@cam.local/snapshot", "http://user:[REDACTED]@cam.local/snapshot"),
("https://user:pw@cam.local/snapshot", "https://user:[REDACTED]@cam.local/snapshot"),
("rtsps://bblp:code@printer:322/streaming/live/1", "rtsps://bblp:[REDACTED]@printer:322/streaming/live/1"),
("ftp://bblp:code@printer:990/", "ftp://bblp:[REDACTED]@printer:990/"),
):
assert redact_url_credentials(url) == expected
def test_masks_a_password_containing_an_at_sign(self):
"""The userinfo ends at the LAST @ before the path — no tail may survive."""
result = redact_url_credentials("rtsp://admin:p@ssw0rd@192.168.1.50/stream")
assert result == "rtsp://admin:[REDACTED]@192.168.1.50/stream"
assert "ssw0rd" not in result
def test_masks_several_urls_in_one_blob(self):
text = "first rtsp://bblp:AAAAAAAA@10.0.0.1/live then rtsp://bblp:BBBBBBBB@10.0.0.2/live"
result = redact_url_credentials(text)
assert "AAAAAAAA" not in result
assert "BBBBBBBB" not in result
assert result.count("[REDACTED]") == 2
def test_never_runs_past_the_authority_into_the_path(self):
"""A later @ in the path must not drag the host into the mask."""
result = redact_url_credentials("rtsp://bblp:code@10.0.0.1/live/user@example")
assert result == "rtsp://bblp:[REDACTED]@10.0.0.1/live/user@example"
def test_leaves_credential_free_text_alone(self):
for untouched in (
"Connection refused",
"rtsp://10.0.0.1:554/stream1",
"mailto and user@example.com in prose",
"Starting USB camera stream from /dev/video0 at 10 fps",
):
assert redact_url_credentials(untouched) == untouched
def test_tolerates_empty_and_none(self):
assert redact_url_credentials("") == ""
assert redact_url_credentials(None) is None
class TestFfmpegStderrFunnel:
"""`_summarize_ffmpeg_stderr` is the one funnel every stderr log in the
camera route passes through, so redaction lands there."""
def test_summary_strips_the_access_code(self):
stderr = f"{FFMPEG_INPUT_LINE}\n[rtsp @ 0x5] Could not find codec parameters\n"
result = _summarize_ffmpeg_stderr(stderr)
assert "38A4KQ2P" not in result
assert "[REDACTED]" in result
# The actionable error is untouched.
assert "Could not find codec parameters" in result
def test_incremental_reader_strips_the_access_code(self):
async def run():
reader = asyncio.StreamReader()
reader.feed_data(f"{FFMPEG_INPUT_LINE}\nError opening input: Connection refused\n".encode())
reader.feed_eof()
class _FakeProcess:
stderr = reader
return await _read_ffmpeg_stderr(_FakeProcess())
result = asyncio.run(run())
assert result is not None
assert "38A4KQ2P" not in result
assert "Connection refused" in result
class TestSupportBundleSanitizerUnchanged:
"""The bundle sanitizer shares the pattern but keeps its own, stricter
replacement — it drops the username too. Guard against drift."""
def test_bundle_still_drops_the_whole_userinfo(self):
result = sanitize_log_content("rtsp://bblp:38A4KQ2P@10.0.0.1/live")
assert "38A4KQ2P" not in result
assert "bblp" not in result
assert "[CREDENTIALS]@" in result