diff --git a/CHANGELOG.md b/CHANGELOG.md index e87feeb03..1db2cbbd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to Bambuddy will be documented in this file. ## [1.2.6b1] - Unreleased ### Fixed +- **LDAP Distinguished Names weren't redacted from the support bundle / bug report (#2681, reporter @MaxBareiss)** — With LDAP auth in use, the debug log carried lines like `LDAP authentication successful for user: … (DN: CN=Joe Schmoe,CN=Users,DC=ad,DC=example,DC=com, …)`. A DN's leaf `CN` is the user's real name — PII on par with the email address Bambuddy already redacts — and it passed straight through into an uploaded support bundle. **Fix.** The log sanitizer (used by both the support bundle and the in-app bug report) now redacts LDAP DNs to `[DN]` wherever they appear — the auth line, ldap3 exception strings, and group DNs alike — matching a run of `attr=value` RDN components (`CN/OU/DC/UID/…`) so ordinary `key=value` log text isn't affected. As primary hygiene the LDAP service also no longer logs the raw DN on successful auth (the username plus group count is enough). Covered by tests, including the exact reported line and non-DN `key=value` lines that must be left intact. Redaction list on the Bug Report wiki page updated. - **An external USB camera could stay locked (LED stuck on) after closing the live view, blocking reopen (#2675, reporter @bitbarista)** — Closing an external USB (V4L2) camera's live view abruptly — tab/popup closed, or a dropped connection — could leave the backend's `ffmpeg` process running and holding `/dev/videoN` open. The camera LED stayed lit and the next attempt to open the view (or click Test) failed or took 10-30+ seconds while the new `ffmpeg` fought for exclusive device access. **Root cause.** This is the same class of leak as #776 (fixed for the built-in RTSP path), but the external/USB path was never wired into that fix. #776 added the `_active_streams` / `_disconnect_events` / spawned-PID registries so both the `/camera/stop` endpoint and the periodic orphan janitor could find and kill leaked ffmpeg — but external streams registered into none of them, so for USB cameras both were structurally blind: `/camera/stop` returned `{"stopped": 0}` even while a stream was genuinely running, and the janitor's `/proc` net matched only `rtsp(s)://bblp:` cmdlines, never a USB `ffmpeg`. Cleanup ran only via the stream generator's own `finally`, which an abrupt disconnect can skip. **Fix.** External USB (and external-RTSP) streams now register their `ffmpeg` process into the same registries the built-in path uses, so `/camera/stop` terminates them promptly (now `{"stopped": 1}`) and the janitor reaps any that leak within its cleanup interval. The `/proc` safety-net scan also now recognises USB (`-f v4l2`) `ffmpeg`, so orphans surviving an app restart are caught too; a leaked process that hangs on a still-locked device (rather than exiting) is registered before the startup probe so it can still be killed. Covered by tests: the stream hands its process to the registry, the stop endpoint and janitor both reap a registered external stream, and the `/proc` scan matches `v4l2` while ignoring unrelated `ffmpeg`. Thanks to @bitbarista for the precise diagnosis. (Reported alongside a working fix; implemented here.) - **A broken slicer sidecar silently produced tiny corrupt files that were queued and printed anyway, and a reverse-proxy 413 wasn't self-explanatory (#2671, reporter @Austinzveare)** — With the slicer-API sidecar behind a reverse proxy, slicing produced ~28-byte files that "did nothing" (and could still be sent to the printer), while a separate proxy attempt failed with a bare **413 Request Entity Too Large** that the recommended nginx fix didn't seem to resolve. **Root cause.** Bambuddy's slice client only validated the sidecar's HTTP *status*, not its body. When the sidecar — or a proxy in front of it — returned `200 OK` with a body that wasn't a real 3MF (a stock/misconfigured sidecar, a proxy error page, a truncated response, or an OrcaSlicer/Bambu Studio CLI crash that emitted no output), Bambuddy wrote that tiny blob straight to a `.gcode.3mf`, stored it as a valid sliced file (the 3MF-parse failure was swallowed as merely "no thumbnail"), and let it be queued and FTP'd to the printer. Separately, a genuine 413 comes from the reverse proxy in front of the sidecar rejecting the multi-MB upload (model + profiles), not from the slicer — so raising the body limit on the wrong proxy layer had no effect. **Fix.** The slice client now validates the sidecar's output: when a 3MF export was requested, the response body must be a real ZIP (3MF container) or the job fails loudly with an actionable message ("…the body is not a valid 3MF (N bytes) — check the sidecar URL and any proxy in front of it") instead of persisting a corrupt file. A 413 now yields a targeted message naming the fix — raise `client_max_body_size` (or equivalent) on the proxy directly in front of the sidecar. Covered by tests: a 200 with a non-3MF body raises a server error (both the profile and embedded-settings paths), a 413 surfaces the reverse-proxy guidance, a valid 3MF still slices, and raw-gcode preview output is not zip-validated. Wiki troubleshooting updated with both scenarios. - **File Manager "sort by recent activity" didn't match `ls -t`, and there was no way to see a file's modified date (#2680 / #1770 follow-up, reporter @Kingbuzz0)** — For external (mapped/NAS) folders the folder tree's activity sort and the file pane's date sort put things in a seemingly random order — some entries roughly right, most not — instead of the real newest-first order shown by `ls -t` or Windows Explorer. **Root cause.** Nothing captured the files' actual on-disk modification time. The sort keyed off Bambuddy's own database `updated_at`/`created_at` timestamps, which for a bulk external scan are all the same instant (the scan time), so a whole block of files tied and sorted arbitrarily; only the few rows Bambuddy had later touched individually looked "partially correct." The folder tree also only bubbled up *immediate* child-file activity, so a file added deep in a subtree never lifted its parent folders. **Fix.** External scans now record each file's and each directory's real filesystem mtime (`os.stat().st_mtime`), refreshing it on every re-scan so a file edited over the mount re-sorts correctly. The folder tree's "recent activity" is now a **recursive** newest-descendant roll-up — a freshly-added file anywhere inside a folder lifts every ancestor — and both the tree sort and the file pane's date sort use the real mtime (falling back to `created_at` for managed uploads that have none). A new toolbar toggle shows/hides each item's **last-modified date** in the right-hand pane (grid and list views). Existing external folders backfill their mtimes on the next scan. Covered by tests: scan captures real file/folder mtimes, a re-scan refreshes a changed file, and a deep file bubbles its subtree's root ahead of a sibling with only a middle-aged file. diff --git a/backend/app/services/ldap_service.py b/backend/app/services/ldap_service.py index 091dfb8eb..8f45fef84 100644 --- a/backend/app/services/ldap_service.py +++ b/backend/app/services/ldap_service.py @@ -256,10 +256,14 @@ def authenticate_ldap_user(config: LDAPConfig, username: str, password: str) -> return None info = _extract_user_info(service_conn, config, user_entry, username) + # Don't log the raw DN — its leaf CN is the user's real name (PII, #2681). + # The username + group count is enough to confirm a successful auth; the + # support-bundle sanitizer also redacts any DN that slips through (e.g. an + # ldap3 exception string), but keeping it out of the log at the source is + # the primary hygiene per the "no private data in logs" rule. logger.info( - "LDAP authentication successful for user: %s (DN: %s, groups: %d)", + "LDAP authentication successful for user: %s (groups: %d)", info.username, - user_dn, len(info.groups), ) return info diff --git a/backend/app/services/log_reader.py b/backend/app/services/log_reader.py index 0a8e357a6..9088b802d 100644 --- a/backend/app/services/log_reader.py +++ b/backend/app/services/log_reader.py @@ -25,6 +25,21 @@ logger = logging.getLogger(__name__) # parse it out; the log-health scanner does not. LOG_LINE_PATTERN = re.compile(r"^(\d{4}-\d{2}-\d{2}\s+\d{2}:\d{2}:\d{2},\d{3})\s+(\w+)\s+\[([^\]]+)\]\s+(.*)$") +# LDAP Distinguished Names carry PII — the leaf ``CN=`` is the user's real name +# (#2681). Match a run of at least two ``attr=value`` RDN components joined by +# commas, where ``attr`` is a known LDAP attribute type. Requiring two components +# keeps this from clobbering an incidental ``key=value`` in an unrelated log line, +# while still catching DNs wherever they surface — the deliberate "auth successful" +# line, ldap3 exception strings, and group DNs alike. Bias is intentionally toward +# redaction: over-redacting a rare debug line to ``[DN]`` is a safe failure; leaking +# a name is not. +# The value char class excludes `<>;+` — RFC 4514 requires those escaped inside a +# DN value, so an unescaped one marks the end of the DN, not part of it. That stops +# the final (comma-unbounded) component from greedily swallowing trailing log text +# such as ``… -> GroupName``. +_LDAP_RDN = r"(?:CN|OU|DC|UID|O|L|ST|C|SN|GN|DN|E|MAIL|STREET|GIVENNAME|SURNAME)=[^,\n<>;+]+" +_LDAP_DN_PATTERN = re.compile(rf"(?i)\b{_LDAP_RDN}(?:\s*,\s*{_LDAP_RDN})+") + class LogEntry(BaseModel): """A single parsed log entry.""" @@ -159,6 +174,9 @@ def sanitize_log_content(content: str, sensitive_strings: dict[str, str] | None # Replace email addresses content = re.sub(r"\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b", "[EMAIL]", content) + # Replace LDAP Distinguished Names (#2681) — PII on par with email. + content = _LDAP_DN_PATTERN.sub("[DN]", content) + # Replace Bambu Lab printer serial numbers (format: 00M/01D/01S/01P/03W + alphanumeric, 12-16 chars total) content = re.sub(r"\b0[0-3][A-Z0-9][A-Z0-9]{9,13}\b", "[SERIAL]", content, flags=re.IGNORECASE) diff --git a/backend/tests/unit/test_support_helpers.py b/backend/tests/unit/test_support_helpers.py index d717d457c..243e9fbfa 100644 --- a/backend/tests/unit/test_support_helpers.py +++ b/backend/tests/unit/test_support_helpers.py @@ -330,6 +330,55 @@ class TestSanitizeLogContent: assert "/home/[user]/" in result assert "[IP]" in result + def test_ldap_dn_redacted_reporter_line(self): + """#2681: the exact reporter line — the CN (real name) must not survive.""" + from backend.app.services.log_reader import sanitize_log_content as _sanitize_log_content + + content = ( + "LDAP authentication successful for user: jschmoe " + "(DN: CN=Joe Schmoe,CN=Users,DC=ad,DC=example,DC=com, groups: 4)" + ) + result = _sanitize_log_content(content) + assert "Joe Schmoe" not in result + assert "DC=example" not in result + assert result == "LDAP authentication successful for user: jschmoe (DN: [DN], groups: 4)" + + def test_ldap_dn_redacted_in_exception_string(self): + """DNs that leak indirectly via ldap3 exception text are caught too.""" + from backend.app.services.log_reader import sanitize_log_content as _sanitize_log_content + + content = "LDAP bind failed for user jschmoe: invalidCredentials at uid=jschmoe,ou=people,dc=example,dc=org" + result = _sanitize_log_content(content) + assert "uid=jschmoe" not in result + assert "[DN]" in result + + def test_ldap_group_dn_redacted(self): + """Group DNs (from group-mapping logs) are PII-bearing and redacted.""" + from backend.app.services.log_reader import sanitize_log_content as _sanitize_log_content + + content = "Mapped CN=Admins,OU=Groups,DC=corp,DC=local -> Administrators" + result = _sanitize_log_content(content) + assert "CN=Admins" not in result + assert "DC=corp" not in result + assert "[DN]" in result + assert "Administrators" in result # the non-PII target group name survives + + def test_non_dn_key_value_line_not_clobbered(self): + """An ordinary key=value log line must not be mistaken for a DN.""" + from backend.app.services.log_reader import sanitize_log_content as _sanitize_log_content + + content = "Dispatch decision: mode=queue, state=FINISH, printer=1" + result = _sanitize_log_content(content) + assert result == content + + def test_single_rdn_not_redacted(self): + """A lone attr=value (not a multi-component DN) is left alone.""" + from backend.app.services.log_reader import sanitize_log_content as _sanitize_log_content + + content = "Country C=US selected" + result = _sanitize_log_content(content) + assert result == content + class TestCollectSupportInfo: """Tests for _collect_support_info() new diagnostic sections."""