From 3c9d80347578acaf9cfcc066508d5f05382eebbe Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Wed, 16 Sep 2026 04:42:25 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=EB=B3=B4=EC=95=88:=20=ED=8C=8C=EC=9D=B4?= =?UTF-8?q?=EC=8D=AC=20=EB=A1=9C=EA=B9=85=EC=97=90=EC=84=9C=20Log=20Inject?= =?UTF-8?q?ion=20=EB=B0=A9=EC=A7=80=EB=A5=BC=20=EC=9C=84=ED=95=B4=20f-stri?= =?UTF-8?q?ng=20=EB=8C=80=EC=8B=A0=20%s=20=EC=82=AC=EC=9A=A9=20=EB=B0=8F?= =?UTF-8?q?=20repr()=20=EC=A0=81=EC=9A=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/sentinel.md | 5 +++++ .../src/bandscope_analysis/temporal/analyzer.py | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 34122c2b4..9b8332540 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -28,3 +28,8 @@ **Vulnerability:** The Rust backend (`apps/desktop/src-tauri/src/main.rs`) did not enforce a maximum URL length limit when processing YouTube URLs via `import_youtube_url`. While the frontend enforced `MAX_YOUTUBE_URL_LENGTH = 2000` via the input element, this could be bypassed by an attacker sending requests directly to the Tauri backend API, potentially causing a Denial of Service (DoS) due to unbounded URL parsing and regex matching. **Learning:** Input validation must occur at the entry point of untrusted data on the backend, even if it is also validated on the frontend. Relying solely on frontend validation for constraints like string length can expose the backend to resource exhaustion vulnerabilities. **Prevention:** Always enforce constraints like maximum length, format validation, and sanitization at the earliest possible point on the backend, typically at the API boundary, regardless of frontend safeguards. + +## 2024-05-30 - Log Injection / CWE-117 Prevention in Python Logging +**Vulnerability:** Untrusted user input (like file paths or request parameters) interpolated into log messages using f-strings (e.g., `logger.error(f"Failed to analyze {path_str}")`). +**Learning:** This exposes the application to Log Injection / Forging if the path string contains newline characters (`\n` or `\r`), allowing an attacker to inject fake log entries. Also, standard logging best practices require deferred parameter interpolation (e.g., `logger.info("msg %s", var)`) rather than f-strings. +**Prevention:** Always use deferred string interpolation in Python logging and sanitize untrusted variables with `repr()` (e.g., `logger.error("Failed to analyze audio %s: %s", repr(path_str), e)`). diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py index 7fe5ae6f7..4a590f57e 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py @@ -73,7 +73,7 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: if not path.exists() or not path.is_file(): raise FileNotFoundError(f"Audio file not found: {path_str}") - logger.info(f"Loading and decoding audio: {path_str}") + logger.info("Loading and decoding audio: %s", repr(path_str)) try: with path.open("rb") as fileobj: @@ -128,7 +128,7 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: bpm_val = float(tempo[0]) if isinstance(tempo, np.ndarray) else float(tempo) - logger.info(f"Analysis complete: {bpm_val:.1f} BPM, {len(beat_times)} beats detected.") + logger.info("Analysis complete: %.1f BPM, %d beats detected.", bpm_val, len(beat_times)) return { "bpm": bpm_val, @@ -140,5 +140,5 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: } except Exception as e: - logger.error(f"Failed to analyze audio {path_str}: {e}") + logger.error("Failed to analyze audio %s: %s", repr(path_str), e) raise ValueError(f"Temporal analysis failed: {e}") from e From ebd35156262481325eb9fc4b9dbf10f97b750c3e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 16 Sep 2026 15:04:25 +0900 Subject: [PATCH 2/2] repair(privacy): route duplicate log finding to canonical owner The CR/LF log-forging finding is valid, but this generated implementation remains weaker than the canonical temporal privacy contract: it still emits the selected local-audio path and raw decoder exception text. The same finding is already preserved in #1211 for canonical owner #1055, which requires path-free bounded context plus exception type after the active #866 source lane releases. Restore this duplicate branch to the protected develop tree as an ordinary descendant so it cannot become a second temporal source writer. Preserve the finding through the existing canonical/preservation path rather than merging a weaker repr(path) implementation. No force update, destructive rebase, self-approval, gate weakening, or security-completion claim. --- .jules/sentinel.md | 5 ----- .../src/bandscope_analysis/temporal/analyzer.py | 6 +++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 9b8332540..34122c2b4 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -28,8 +28,3 @@ **Vulnerability:** The Rust backend (`apps/desktop/src-tauri/src/main.rs`) did not enforce a maximum URL length limit when processing YouTube URLs via `import_youtube_url`. While the frontend enforced `MAX_YOUTUBE_URL_LENGTH = 2000` via the input element, this could be bypassed by an attacker sending requests directly to the Tauri backend API, potentially causing a Denial of Service (DoS) due to unbounded URL parsing and regex matching. **Learning:** Input validation must occur at the entry point of untrusted data on the backend, even if it is also validated on the frontend. Relying solely on frontend validation for constraints like string length can expose the backend to resource exhaustion vulnerabilities. **Prevention:** Always enforce constraints like maximum length, format validation, and sanitization at the earliest possible point on the backend, typically at the API boundary, regardless of frontend safeguards. - -## 2024-05-30 - Log Injection / CWE-117 Prevention in Python Logging -**Vulnerability:** Untrusted user input (like file paths or request parameters) interpolated into log messages using f-strings (e.g., `logger.error(f"Failed to analyze {path_str}")`). -**Learning:** This exposes the application to Log Injection / Forging if the path string contains newline characters (`\n` or `\r`), allowing an attacker to inject fake log entries. Also, standard logging best practices require deferred parameter interpolation (e.g., `logger.info("msg %s", var)`) rather than f-strings. -**Prevention:** Always use deferred string interpolation in Python logging and sanitize untrusted variables with `repr()` (e.g., `logger.error("Failed to analyze audio %s: %s", repr(path_str), e)`). diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py index 4a590f57e..7fe5ae6f7 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py @@ -73,7 +73,7 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: if not path.exists() or not path.is_file(): raise FileNotFoundError(f"Audio file not found: {path_str}") - logger.info("Loading and decoding audio: %s", repr(path_str)) + logger.info(f"Loading and decoding audio: {path_str}") try: with path.open("rb") as fileobj: @@ -128,7 +128,7 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: bpm_val = float(tempo[0]) if isinstance(tempo, np.ndarray) else float(tempo) - logger.info("Analysis complete: %.1f BPM, %d beats detected.", bpm_val, len(beat_times)) + logger.info(f"Analysis complete: {bpm_val:.1f} BPM, {len(beat_times)} beats detected.") return { "bpm": bpm_val, @@ -140,5 +140,5 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: } except Exception as e: - logger.error("Failed to analyze audio %s: %s", repr(path_str), e) + logger.error(f"Failed to analyze audio {path_str}: {e}") raise ValueError(f"Temporal analysis failed: {e}") from e