From c80878e3de3f93a3ed4d72f7569919f01c46038b Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sat, 19 Sep 2026 04:12:04 +0000 Subject: [PATCH 01/28] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDI?= =?UTF-8?q?UM]=20Fix=20Log=20Forging=20vulnerability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replaced f-strings with deferred string interpolation in Python logging statements to align with best practices. - Wrapped untrusted inputs (e.g., file paths, filenames) with `repr()` before passing them to the logger to escape control characters. - These changes mitigate Log Forging/Injection (CWE-117) vulnerabilities where attackers could inject malicious log entries, such as newlines, to forge log records. - Updated `temporal/analyzer.py` and `cli.py` in the analysis engine. - Documented findings in `.jules/sentinel.md`. --- .jules/sentinel.md | 5 +++++ services/analysis-engine/src/bandscope_analysis/cli.py | 6 +++--- .../src/bandscope_analysis/temporal/analyzer.py | 6 +++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 34122c2b4..23ee52c0f 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-08-01 - Prevent Log Forging / Injection +**Vulnerability:** Untrusted user input (e.g., file paths, filenames) was logged directly using f-strings or without proper sanitization, which allows attackers to inject malicious log entries (like newlines) to forge log records. +**Learning:** Python logging should always use deferred string interpolation (e.g., `%s` formatting) rather than f-strings. Additionally, untrusted inputs must be wrapped with `repr()` before being passed to the logger to escape control characters. +**Prevention:** Use parameterized formatting (`logger.info("msg %s", repr(var))`) consistently for all log statements containing external inputs. diff --git a/services/analysis-engine/src/bandscope_analysis/cli.py b/services/analysis-engine/src/bandscope_analysis/cli.py index 6838ee711..e25de131c 100644 --- a/services/analysis-engine/src/bandscope_analysis/cli.py +++ b/services/analysis-engine/src/bandscope_analysis/cli.py @@ -86,15 +86,15 @@ def main() -> int: audio_path = local_source.get("sourcePath") file_name = local_source.get("fileName", "selected audio") if audio_path: - logging.info("Extracting temporal features from %s...", file_name) + logging.info("Extracting temporal features from %s...", repr(file_name)) try: temporal_analyzer = TemporalAnalyzer() features = temporal_analyzer.analyze(audio_path) - logging.info(f"Extracted BPM: {features['bpm']}") + logging.info("Extracted BPM: %s", features['bpm']) except Exception: logging.warning( "Temporal analysis failed for %s; continuing with safe fallback.", - file_name, + repr(file_name), ) requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z") 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 61a24bab40380227f8f141e29f030134056f7542 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sat, 19 Sep 2026 14:05:11 +0000 Subject: [PATCH 02/28] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[MEDI?= =?UTF-8?q?UM]=20Fix=20Log=20Forging=20vulnerability?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replaced f-strings with deferred string interpolation in Python logging statements to align with best practices. - Wrapped untrusted inputs (e.g., file paths, filenames) with `repr()` before passing them to the logger to escape control characters. - These changes mitigate Log Forging/Injection (CWE-117) vulnerabilities where attackers could inject malicious log entries, such as newlines, to forge log records. - Updated `temporal/analyzer.py` and `cli.py` in the analysis engine. - Formatted source files to ensure GitHub CI checks pass. - Documented findings in `.jules/sentinel.md`. --- services/analysis-engine/src/bandscope_analysis/cli.py | 2 +- services/analysis-engine/tests/test_supply_chain_policy.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/cli.py b/services/analysis-engine/src/bandscope_analysis/cli.py index e25de131c..0bbd154cd 100644 --- a/services/analysis-engine/src/bandscope_analysis/cli.py +++ b/services/analysis-engine/src/bandscope_analysis/cli.py @@ -90,7 +90,7 @@ def main() -> int: try: temporal_analyzer = TemporalAnalyzer() features = temporal_analyzer.analyze(audio_path) - logging.info("Extracted BPM: %s", features['bpm']) + logging.info("Extracted BPM: %s", features["bpm"]) except Exception: logging.warning( "Temporal analysis failed for %s; continuing with safe fallback.", diff --git a/services/analysis-engine/tests/test_supply_chain_policy.py b/services/analysis-engine/tests/test_supply_chain_policy.py index 1d8224c5a..6a0853944 100644 --- a/services/analysis-engine/tests/test_supply_chain_policy.py +++ b/services/analysis-engine/tests/test_supply_chain_policy.py @@ -1275,9 +1275,7 @@ def test_workflow_concurrency_cancels_only_superseded_pr_heads() -> None: workflow = (workflows_dir / workflow_name).read_text(encoding="utf-8") assert "concurrency:" in workflow, workflow_name assert "cancel-in-progress: false" in workflow, workflow_name - assert "contents: read" in workflow or "permissions: read-all" in workflow, ( - workflow_name - ) + assert "contents: read" in workflow or "permissions: read-all" in workflow, workflow_name assert "pull_request:" not in (workflows_dir / "release.yml").read_text(encoding="utf-8") From ebded927b20d4238692a7db6f5402c400730d7e4 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 20 Sep 2026 00:27:35 +0000 Subject: [PATCH 03/28] Trigger CI retry From 3437513cdbc1c4f35dfccc1ac9131394d45ad877 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 20 Sep 2026 01:00:01 +0000 Subject: [PATCH 04/28] Trigger CI retry 2 From a9d8eb9905066ba28e37c82702154dbdf8c560d3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 09:00:35 +0900 Subject: [PATCH 05/28] test(security): reproduce newline injection through decoder error logs --- .../tests/test_logging_safety.py | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 services/analysis-engine/tests/test_logging_safety.py diff --git a/services/analysis-engine/tests/test_logging_safety.py b/services/analysis-engine/tests/test_logging_safety.py new file mode 100644 index 000000000..e816b0c66 --- /dev/null +++ b/services/analysis-engine/tests/test_logging_safety.py @@ -0,0 +1,39 @@ +"""Logging safety regressions for untrusted analysis inputs.""" + +from __future__ import annotations + +import logging +from pathlib import Path + +import pytest + +from bandscope_analysis.temporal import TemporalAnalyzer +from bandscope_analysis.temporal import analyzer as analyzer_module + + +def test_temporal_error_log_escapes_exception_control_characters( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """Decoder failures must not inject a second physical log line.""" + audio_path = tmp_path / "buyer-audio.wav" + audio_path.write_bytes(b"not-a-real-wave") + + def fail_decode(*args: object, **kwargs: object) -> object: + raise RuntimeError("decoder failed\nFORGED SECURITY EVENT") + + monkeypatch.setattr(analyzer_module.librosa, "load", fail_decode) + caplog.set_level(logging.ERROR, logger=analyzer_module.__name__) + + with pytest.raises(ValueError, match="Temporal analysis failed"): + TemporalAnalyzer().analyze(audio_path) + + messages = [ + record.getMessage() + for record in caplog.records + if record.name == analyzer_module.__name__ and record.levelno >= logging.ERROR + ] + assert len(messages) == 1 + assert "\n" not in messages[0] + assert "\\n" in messages[0] From 68141a179f08aa6473f5755f138946db17979393 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 09:00:49 +0900 Subject: [PATCH 06/28] fix(security): escape decoder exception text in temporal logs --- .../analysis-engine/src/bandscope_analysis/temporal/analyzer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py index 4a590f57e..7b5b68bd2 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py @@ -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("Failed to analyze audio %s: %s", repr(path_str), repr(e)) raise ValueError(f"Temporal analysis failed: {e}") from e From 6504c56faeadd5594a6264285b97ccabc52d8909 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 09:02:09 +0900 Subject: [PATCH 07/28] test(security): satisfy logging regression lint contract --- services/analysis-engine/tests/test_logging_safety.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/analysis-engine/tests/test_logging_safety.py b/services/analysis-engine/tests/test_logging_safety.py index e816b0c66..07e8e057e 100644 --- a/services/analysis-engine/tests/test_logging_safety.py +++ b/services/analysis-engine/tests/test_logging_safety.py @@ -20,7 +20,8 @@ def test_temporal_error_log_escapes_exception_control_characters( audio_path = tmp_path / "buyer-audio.wav" audio_path.write_bytes(b"not-a-real-wave") - def fail_decode(*args: object, **kwargs: object) -> object: + def fail_decode(*_args: object, **_kwargs: object) -> object: + """Raise a decoder error containing an injected physical newline.""" raise RuntimeError("decoder failed\nFORGED SECURITY EVENT") monkeypatch.setattr(analyzer_module.librosa, "load", fail_decode) From fd358ac9f8d397a5cf3d8c7e649422fdd62901f0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 09:02:35 +0900 Subject: [PATCH 08/28] repair(security): drop foreign supply-chain formatter delta --- services/analysis-engine/tests/test_supply_chain_policy.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/analysis-engine/tests/test_supply_chain_policy.py b/services/analysis-engine/tests/test_supply_chain_policy.py index 6a0853944..1d8224c5a 100644 --- a/services/analysis-engine/tests/test_supply_chain_policy.py +++ b/services/analysis-engine/tests/test_supply_chain_policy.py @@ -1275,7 +1275,9 @@ def test_workflow_concurrency_cancels_only_superseded_pr_heads() -> None: workflow = (workflows_dir / workflow_name).read_text(encoding="utf-8") assert "concurrency:" in workflow, workflow_name assert "cancel-in-progress: false" in workflow, workflow_name - assert "contents: read" in workflow or "permissions: read-all" in workflow, workflow_name + assert "contents: read" in workflow or "permissions: read-all" in workflow, ( + workflow_name + ) assert "pull_request:" not in (workflows_dir / "release.yml").read_text(encoding="utf-8") From d7b170df08a392e2190a59f7b4dfbd7342c22077 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 09:03:24 +0900 Subject: [PATCH 09/28] docs(security): record actual log-forging boundary --- .jules/sentinel.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 23ee52c0f..c36c4a0a6 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -29,7 +29,7 @@ **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-08-01 - Prevent Log Forging / Injection -**Vulnerability:** Untrusted user input (e.g., file paths, filenames) was logged directly using f-strings or without proper sanitization, which allows attackers to inject malicious log entries (like newlines) to forge log records. -**Learning:** Python logging should always use deferred string interpolation (e.g., `%s` formatting) rather than f-strings. Additionally, untrusted inputs must be wrapped with `repr()` before being passed to the logger to escape control characters. -**Prevention:** Use parameterized formatting (`logger.info("msg %s", repr(var))`) consistently for all log statements containing external inputs. +## 2026-09-21 - Prevent log forging / injection +**Vulnerability:** Untrusted file names, paths, or decoder exception text could carry carriage returns or newlines into plain-text logs and create forged physical log records. +**Learning:** Deferred logging interpolation avoids eager string construction but does not itself neutralize control characters. Every untrusted value that enters a line-oriented log record must be rendered into a single-line representation before the logging formatter receives it. +**Prevention:** Keep message templates parameterized and pass untrusted values through a control-character-safe representation such as `repr()`. Regression tests must include newline-bearing path/name or exception text and assert that one logical event produces one physical log line. From da1128deb76890d7e3115e9d00ec48141a7838ab Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 09:03:50 +0900 Subject: [PATCH 10/28] test(security): cover newline-bearing local audio labels --- .../tests/test_logging_safety.py | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/services/analysis-engine/tests/test_logging_safety.py b/services/analysis-engine/tests/test_logging_safety.py index 07e8e057e..08aa60136 100644 --- a/services/analysis-engine/tests/test_logging_safety.py +++ b/services/analysis-engine/tests/test_logging_safety.py @@ -2,11 +2,14 @@ from __future__ import annotations +import io +import json import logging from pathlib import Path import pytest +from bandscope_analysis import cli from bandscope_analysis.temporal import TemporalAnalyzer from bandscope_analysis.temporal import analyzer as analyzer_module @@ -38,3 +41,61 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: assert len(messages) == 1 assert "\n" not in messages[0] assert "\\n" in messages[0] + + +def test_cli_logs_untrusted_filename_as_single_line( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Local-audio labels must stay on one physical log line on fallback.""" + malicious_name = "buyer.wav\nFORGED SECURITY EVENT" + stdin = io.StringIO( + json.dumps( + { + "jobId": "log-safety", + "request": { + "sourceKind": "local_audio", + "projectId": "project-log-safety", + "sourceLabel": "buyer.wav", + "roleFocus": [], + "localSource": { + "sourcePath": "/synthetic/buyer.wav", + "fileName": malicious_name, + "extension": "wav", + "fileSizeBytes": 1, + }, + }, + } + ) + ) + stdout = io.StringIO() + + class FailingAnalyzer: + """Exercise the CLI fallback without touching a real decoder.""" + + def analyze(self, _path: object) -> object: + """Fail after the buyer-controlled file name has been logged.""" + raise RuntimeError("expected test failure") + + monkeypatch.setattr(cli, "TemporalAnalyzer", FailingAnalyzer) + monkeypatch.setattr( + cli, + "run_analysis_job", + lambda _job_id, _request, requested_at: { + "jobId": "log-safety", + "state": "failed", + "requestedAt": requested_at, + "updatedAt": requested_at, + }, + ) + monkeypatch.setattr(cli.sys, "stdin", stdin) + monkeypatch.setattr(cli.sys, "stdout", stdout) + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) + caplog.set_level(logging.INFO) + + assert cli.main() == 0 + + messages = [record.getMessage() for record in caplog.records if "buyer.wav" in record.getMessage()] + assert len(messages) == 2 + assert all("\n" not in message for message in messages) + assert all("\\n" in message for message in messages) From b57cf82f94760fe62dcd248e4ff5ebc9a0a536ae Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 09:07:24 +0900 Subject: [PATCH 11/28] style(test): keep log-safety regression formatter-clean --- services/analysis-engine/tests/test_logging_safety.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/services/analysis-engine/tests/test_logging_safety.py b/services/analysis-engine/tests/test_logging_safety.py index 08aa60136..0caed5b83 100644 --- a/services/analysis-engine/tests/test_logging_safety.py +++ b/services/analysis-engine/tests/test_logging_safety.py @@ -95,7 +95,11 @@ def analyze(self, _path: object) -> object: assert cli.main() == 0 - messages = [record.getMessage() for record in caplog.records if "buyer.wav" in record.getMessage()] + messages = [ + record.getMessage() + for record in caplog.records + if "buyer.wav" in record.getMessage() + ] assert len(messages) == 2 assert all("\n" not in message for message in messages) assert all("\\n" in message for message in messages) From c7953797ad807e351da616a9750a8ed2fc85455e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 09:08:27 +0900 Subject: [PATCH 12/28] test(security): cover CRLF ANSI C0 and Unicode log inputs --- .../tests/test_logging_safety.py | 46 +++++++++++++++---- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/services/analysis-engine/tests/test_logging_safety.py b/services/analysis-engine/tests/test_logging_safety.py index 0caed5b83..ba2121a90 100644 --- a/services/analysis-engine/tests/test_logging_safety.py +++ b/services/analysis-engine/tests/test_logging_safety.py @@ -13,19 +13,46 @@ from bandscope_analysis.temporal import TemporalAnalyzer from bandscope_analysis.temporal import analyzer as analyzer_module - +_LOG_CONTROL_ESCAPES = ( + ("\n", "\\n"), + ("\r", "\\r"), + ("\t", "\\t"), + ("\x1b", "\\x1b"), + ("\x00", "\\x00"), +) +_HOSTILE_LOG_VALUES = ( + "FORGED\nSECURITY EVENT", + "FORGED\r\nSECURITY EVENT", + "FORGED\tSECURITY EVENT", + "FORGED\x1b[31mSECURITY EVENT", + "FORGED\x00SECURITY EVENT", + "정상-유니코드-é", +) + + +def _assert_log_value_is_single_record(rendered: str, untrusted_value: str) -> None: + for control_character, escaped_form in _LOG_CONTROL_ESCAPES: + assert control_character not in rendered + if control_character in untrusted_value: + assert escaped_form in rendered + if untrusted_value == "정상-유니코드-é": + assert untrusted_value in rendered + + +@pytest.mark.parametrize("untrusted_value", _HOSTILE_LOG_VALUES) def test_temporal_error_log_escapes_exception_control_characters( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, caplog: pytest.LogCaptureFixture, + untrusted_value: str, ) -> None: """Decoder failures must not inject a second physical log line.""" audio_path = tmp_path / "buyer-audio.wav" audio_path.write_bytes(b"not-a-real-wave") def fail_decode(*_args: object, **_kwargs: object) -> object: - """Raise a decoder error containing an injected physical newline.""" - raise RuntimeError("decoder failed\nFORGED SECURITY EVENT") + """Raise a decoder error carrying the parameterized untrusted value.""" + raise RuntimeError(untrusted_value) monkeypatch.setattr(analyzer_module.librosa, "load", fail_decode) caplog.set_level(logging.ERROR, logger=analyzer_module.__name__) @@ -39,16 +66,17 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: if record.name == analyzer_module.__name__ and record.levelno >= logging.ERROR ] assert len(messages) == 1 - assert "\n" not in messages[0] - assert "\\n" in messages[0] + _assert_log_value_is_single_record(messages[0], untrusted_value) +@pytest.mark.parametrize("untrusted_value", _HOSTILE_LOG_VALUES) def test_cli_logs_untrusted_filename_as_single_line( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture, + untrusted_value: str, ) -> None: """Local-audio labels must stay on one physical log line on fallback.""" - malicious_name = "buyer.wav\nFORGED SECURITY EVENT" + malicious_name = f"buyer-{untrusted_value}.wav" stdin = io.StringIO( json.dumps( { @@ -98,8 +126,8 @@ def analyze(self, _path: object) -> object: messages = [ record.getMessage() for record in caplog.records - if "buyer.wav" in record.getMessage() + if "buyer-" in record.getMessage() ] assert len(messages) == 2 - assert all("\n" not in message for message in messages) - assert all("\\n" in message for message in messages) + for message in messages: + _assert_log_value_is_single_record(message, untrusted_value) From 62f69b3818e9dbc2f2d8e04b38dd29f71ead4cfa Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 12:02:13 +0900 Subject: [PATCH 13/28] test(security): cover hostile decoder exception repr --- .../tests/test_logging_safety.py | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/services/analysis-engine/tests/test_logging_safety.py b/services/analysis-engine/tests/test_logging_safety.py index ba2121a90..ff03271f9 100644 --- a/services/analysis-engine/tests/test_logging_safety.py +++ b/services/analysis-engine/tests/test_logging_safety.py @@ -69,6 +69,44 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: _assert_log_value_is_single_record(messages[0], untrusted_value) +def test_temporal_error_log_neutralizes_hostile_exception_repr( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """A decoder exception cannot bypass log neutralization via custom repr.""" + audio_path = tmp_path / "buyer-audio.wav" + audio_path.write_bytes(b"not-a-real-wave") + + class HostileDecoderError(RuntimeError): + """Model a dependency exception whose repr emits raw control characters.""" + + def __repr__(self) -> str: + """Return an intentionally unsafe representation for the regression.""" + return "HostileDecoderError('FORGED\nSECURITY EVENT\x1b[31m')" + + def fail_decode(*_args: object, **_kwargs: object) -> object: + """Raise the dependency-shaped exception through the real analyzer path.""" + raise HostileDecoderError("decoder failed") + + monkeypatch.setattr(analyzer_module.librosa, "load", fail_decode) + caplog.set_level(logging.ERROR, logger=analyzer_module.__name__) + + with pytest.raises(ValueError, match="Temporal analysis failed"): + TemporalAnalyzer().analyze(audio_path) + + messages = [ + record.getMessage() + for record in caplog.records + if record.name == analyzer_module.__name__ and record.levelno >= logging.ERROR + ] + assert len(messages) == 1 + assert "\n" not in messages[0] + assert "\x1b" not in messages[0] + assert "\\n" in messages[0] + assert "\\x1b" in messages[0] + + @pytest.mark.parametrize("untrusted_value", _HOSTILE_LOG_VALUES) def test_cli_logs_untrusted_filename_as_single_line( monkeypatch: pytest.MonkeyPatch, From bd6d31e6883c5aa50af2ddba49bd0ba2f92b598e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 12:02:32 +0900 Subject: [PATCH 14/28] fix(security): neutralize controls after exception repr --- .../bandscope_analysis/temporal/analyzer.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py index 7b5b68bd2..5bb83d83a 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py @@ -28,6 +28,17 @@ BEATS_PER_BAR = 4 +def _single_line_log_repr(value: object) -> str: + """Render a diagnostic value without letting its repr create log controls.""" + rendered = repr(value) + return "".join( + character + if character.isprintable() + else character.encode("unicode_escape").decode("ascii") + for character in rendered + ) + + def _estimate_downbeats( onset_env: NDArray[np.floating[Any]], beat_frames: NDArray[np.integer[Any]], @@ -73,7 +84,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("Loading and decoding audio: %s", _single_line_log_repr(path_str)) try: with path.open("rb") as fileobj: @@ -140,5 +151,9 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: } except Exception as e: - logger.error("Failed to analyze audio %s: %s", repr(path_str), repr(e)) + logger.error( + "Failed to analyze audio %s: %s", + _single_line_log_repr(path_str), + _single_line_log_repr(e), + ) raise ValueError(f"Temporal analysis failed: {e}") from e From 7d6e065462f6d9aac93d19391d25b8d89f290224 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 12:02:55 +0900 Subject: [PATCH 15/28] docs(security): record repr trust-boundary limit --- .jules/sentinel.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index c36c4a0a6..906f28148 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -30,6 +30,6 @@ **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. ## 2026-09-21 - Prevent log forging / injection -**Vulnerability:** Untrusted file names, paths, or decoder exception text could carry carriage returns or newlines into plain-text logs and create forged physical log records. -**Learning:** Deferred logging interpolation avoids eager string construction but does not itself neutralize control characters. Every untrusted value that enters a line-oriented log record must be rendered into a single-line representation before the logging formatter receives it. -**Prevention:** Keep message templates parameterized and pass untrusted values through a control-character-safe representation such as `repr()`. Regression tests must include newline-bearing path/name or exception text and assert that one logical event produces one physical log line. +**Vulnerability:** Untrusted file names, paths, or decoder exception text could carry carriage returns, line separators, terminal controls, or other non-printable characters into line-oriented logs and create forged or misleading records. +**Learning:** Deferred logging interpolation avoids eager string construction but does not neutralize controls. `repr()` is also not a complete trust boundary for arbitrary dependency objects because a custom `__repr__` implementation may itself return raw control characters. The final rendered representation must be checked before it reaches the logging formatter. +**Prevention:** Keep message templates parameterized. For plain strings, use a representation that escapes controls while retaining ordinary printable Unicode. For arbitrary dependency objects, render the object and then escape every non-printable code point in the rendered result. Regression tests must cover LF/CRLF, tab, ANSI ESC, NUL/C0, Unicode line separators, and a custom exception `__repr__` that emits raw controls; one logical event must remain one physical log record. From 27db615a272ea86fbca8480ff7a0e5e16602a7fe Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 12:03:19 +0900 Subject: [PATCH 16/28] test(security): cover Unicode log separators --- services/analysis-engine/tests/test_logging_safety.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/services/analysis-engine/tests/test_logging_safety.py b/services/analysis-engine/tests/test_logging_safety.py index ff03271f9..358a68ef3 100644 --- a/services/analysis-engine/tests/test_logging_safety.py +++ b/services/analysis-engine/tests/test_logging_safety.py @@ -19,6 +19,8 @@ ("\t", "\\t"), ("\x1b", "\\x1b"), ("\x00", "\\x00"), + ("\u2028", "\\u2028"), + ("\u2029", "\\u2029"), ) _HOSTILE_LOG_VALUES = ( "FORGED\nSECURITY EVENT", @@ -26,6 +28,8 @@ "FORGED\tSECURITY EVENT", "FORGED\x1b[31mSECURITY EVENT", "FORGED\x00SECURITY EVENT", + "FORGED\u2028SECURITY EVENT", + "FORGED\u2029SECURITY EVENT", "정상-유니코드-é", ) @@ -83,7 +87,7 @@ class HostileDecoderError(RuntimeError): def __repr__(self) -> str: """Return an intentionally unsafe representation for the regression.""" - return "HostileDecoderError('FORGED\nSECURITY EVENT\x1b[31m')" + return "HostileDecoderError('FORGED\nSECURITY EVENT\x1b[31m\u2028NEXT')" def fail_decode(*_args: object, **_kwargs: object) -> object: """Raise the dependency-shaped exception through the real analyzer path.""" @@ -103,8 +107,10 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: assert len(messages) == 1 assert "\n" not in messages[0] assert "\x1b" not in messages[0] + assert "\u2028" not in messages[0] assert "\\n" in messages[0] assert "\\x1b" in messages[0] + assert "\\u2028" in messages[0] @pytest.mark.parametrize("untrusted_value", _HOSTILE_LOG_VALUES) From ced5a69791fe404d96e5befca77bcd9b78537ff1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 12:04:26 +0900 Subject: [PATCH 17/28] test(security): keep logging from masking decoder errors --- .../tests/test_logging_safety.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/services/analysis-engine/tests/test_logging_safety.py b/services/analysis-engine/tests/test_logging_safety.py index 358a68ef3..63c6db4d9 100644 --- a/services/analysis-engine/tests/test_logging_safety.py +++ b/services/analysis-engine/tests/test_logging_safety.py @@ -113,6 +113,42 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: assert "\\u2028" in messages[0] +def test_temporal_error_log_survives_exception_repr_failure( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """Logging must not replace the decoder failure when exception repr itself fails.""" + audio_path = tmp_path / "buyer-audio.wav" + audio_path.write_bytes(b"not-a-real-wave") + + class BrokenReprDecoderError(RuntimeError): + """Model a dependency exception whose repr is itself faulty.""" + + def __repr__(self) -> str: + """Raise to verify diagnostics do not mask the original failure path.""" + raise RuntimeError("repr failed") + + def fail_decode(*_args: object, **_kwargs: object) -> object: + """Raise the dependency-shaped exception through the real analyzer path.""" + raise BrokenReprDecoderError("decoder failed") + + monkeypatch.setattr(analyzer_module.librosa, "load", fail_decode) + caplog.set_level(logging.ERROR, logger=analyzer_module.__name__) + + with pytest.raises(ValueError, match="Temporal analysis failed: decoder failed"): + TemporalAnalyzer().analyze(audio_path) + + messages = [ + record.getMessage() + for record in caplog.records + if record.name == analyzer_module.__name__ and record.levelno >= logging.ERROR + ] + assert len(messages) == 1 + assert "BrokenReprDecoderError" in messages[0] + assert "repr unavailable" in messages[0] + + @pytest.mark.parametrize("untrusted_value", _HOSTILE_LOG_VALUES) def test_cli_logs_untrusted_filename_as_single_line( monkeypatch: pytest.MonkeyPatch, From 8da947b0ec115b2febf9bc49a16b5f95aae69dc8 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 12:04:52 +0900 Subject: [PATCH 18/28] fix(security): keep diagnostics from masking decoder failure --- .../src/bandscope_analysis/temporal/analyzer.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py index 5bb83d83a..e0cedd90f 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py @@ -30,7 +30,10 @@ def _single_line_log_repr(value: object) -> str: """Render a diagnostic value without letting its repr create log controls.""" - rendered = repr(value) + try: + rendered = repr(value) + except Exception: + rendered = f"<{type(value).__name__} repr unavailable>" return "".join( character if character.isprintable() From f551e05fb2ff716b499c3c823aa2223ee67de3e8 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 12:06:08 +0900 Subject: [PATCH 19/28] docs(security): make diagnostic rendering fail safe --- .jules/sentinel.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 906f28148..ac0a767ea 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -31,5 +31,5 @@ ## 2026-09-21 - Prevent log forging / injection **Vulnerability:** Untrusted file names, paths, or decoder exception text could carry carriage returns, line separators, terminal controls, or other non-printable characters into line-oriented logs and create forged or misleading records. -**Learning:** Deferred logging interpolation avoids eager string construction but does not neutralize controls. `repr()` is also not a complete trust boundary for arbitrary dependency objects because a custom `__repr__` implementation may itself return raw control characters. The final rendered representation must be checked before it reaches the logging formatter. -**Prevention:** Keep message templates parameterized. For plain strings, use a representation that escapes controls while retaining ordinary printable Unicode. For arbitrary dependency objects, render the object and then escape every non-printable code point in the rendered result. Regression tests must cover LF/CRLF, tab, ANSI ESC, NUL/C0, Unicode line separators, and a custom exception `__repr__` that emits raw controls; one logical event must remain one physical log record. +**Learning:** Deferred logging interpolation avoids eager string construction but does not neutralize controls. `repr()` is also not a complete trust boundary for arbitrary dependency objects: a custom `__repr__` implementation may emit raw control characters or raise while the program is already handling another exception. The final rendered representation must be checked before it reaches the logging formatter, and diagnostic rendering must fail safely rather than replace the original analysis failure. +**Prevention:** Keep message templates parameterized. For plain strings, use a representation that escapes controls while retaining ordinary printable Unicode. For arbitrary dependency objects, attempt to render the object, fall back to a code-owned type marker if representation fails, and then escape every non-printable code point in the resulting text. Regression tests must cover LF/CRLF, tab, ANSI ESC, NUL/C0, Unicode line separators, a custom exception `__repr__` that emits raw controls, and a custom `__repr__` that raises; one logical event must remain one physical log record and logging must not mask the original decoder failure. From 8072eeadd017f872b9124d2e8c22d7d217047d6e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 13:02:33 +0900 Subject: [PATCH 20/28] test(logging): bound untrusted diagnostic output --- .../tests/test_logging_safety.py | 123 ++++++++++++++++-- 1 file changed, 112 insertions(+), 11 deletions(-) diff --git a/services/analysis-engine/tests/test_logging_safety.py b/services/analysis-engine/tests/test_logging_safety.py index 63c6db4d9..11cc0fcac 100644 --- a/services/analysis-engine/tests/test_logging_safety.py +++ b/services/analysis-engine/tests/test_logging_safety.py @@ -73,20 +73,23 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: _assert_log_value_is_single_record(messages[0], untrusted_value) -def test_temporal_error_log_neutralizes_hostile_exception_repr( +def test_temporal_error_log_does_not_execute_hostile_exception_repr( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, caplog: pytest.LogCaptureFixture, ) -> None: - """A decoder exception cannot bypass log neutralization via custom repr.""" + """Log rendering must not execute dependency-controlled exception repr code.""" audio_path = tmp_path / "buyer-audio.wav" audio_path.write_bytes(b"not-a-real-wave") + repr_calls = 0 class HostileDecoderError(RuntimeError): - """Model a dependency exception whose repr emits raw control characters.""" + """Model a dependency exception whose repr has attacker-controlled behavior.""" def __repr__(self) -> str: - """Return an intentionally unsafe representation for the regression.""" + """Record execution and return an intentionally unsafe representation.""" + nonlocal repr_calls + repr_calls += 1 return "HostileDecoderError('FORGED\nSECURITY EVENT\x1b[31m\u2028NEXT')" def fail_decode(*_args: object, **_kwargs: object) -> object: @@ -96,7 +99,7 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: monkeypatch.setattr(analyzer_module.librosa, "load", fail_decode) caplog.set_level(logging.ERROR, logger=analyzer_module.__name__) - with pytest.raises(ValueError, match="Temporal analysis failed"): + with pytest.raises(ValueError, match="Temporal analysis failed: decoder failed"): TemporalAnalyzer().analyze(audio_path) messages = [ @@ -105,12 +108,13 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: if record.name == analyzer_module.__name__ and record.levelno >= logging.ERROR ] assert len(messages) == 1 + assert repr_calls == 0 + assert "HostileDecoderError" in messages[0] + assert "decoder failed" in messages[0] + assert "FORGED" not in messages[0] assert "\n" not in messages[0] assert "\x1b" not in messages[0] assert "\u2028" not in messages[0] - assert "\\n" in messages[0] - assert "\\x1b" in messages[0] - assert "\\u2028" in messages[0] def test_temporal_error_log_survives_exception_repr_failure( @@ -118,15 +122,18 @@ def test_temporal_error_log_survives_exception_repr_failure( tmp_path: Path, caplog: pytest.LogCaptureFixture, ) -> None: - """Logging must not replace the decoder failure when exception repr itself fails.""" + """Logging must not call a broken repr while handling the decoder failure.""" audio_path = tmp_path / "buyer-audio.wav" audio_path.write_bytes(b"not-a-real-wave") + repr_calls = 0 class BrokenReprDecoderError(RuntimeError): """Model a dependency exception whose repr is itself faulty.""" def __repr__(self) -> str: - """Raise to verify diagnostics do not mask the original failure path.""" + """Raise if logging improperly executes dependency representation code.""" + nonlocal repr_calls + repr_calls += 1 raise RuntimeError("repr failed") def fail_decode(*_args: object, **_kwargs: object) -> object: @@ -145,8 +152,40 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: if record.name == analyzer_module.__name__ and record.levelno >= logging.ERROR ] assert len(messages) == 1 + assert repr_calls == 0 assert "BrokenReprDecoderError" in messages[0] - assert "repr unavailable" in messages[0] + assert "decoder failed" in messages[0] + assert "repr unavailable" not in messages[0] + + +def test_temporal_error_log_bounds_oversized_exception_message( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """One decoder failure cannot emit an unbounded diagnostic log field.""" + audio_path = tmp_path / "buyer-audio.wav" + audio_path.write_bytes(b"not-a-real-wave") + oversized_message = "X" * 5000 + + def fail_decode(*_args: object, **_kwargs: object) -> object: + """Raise a dependency error with a deliberately oversized message.""" + raise RuntimeError(oversized_message) + + monkeypatch.setattr(analyzer_module.librosa, "load", fail_decode) + caplog.set_level(logging.ERROR, logger=analyzer_module.__name__) + + with pytest.raises(ValueError, match="Temporal analysis failed"): + TemporalAnalyzer().analyze(audio_path) + + messages = [ + record.getMessage() + for record in caplog.records + if record.name == analyzer_module.__name__ and record.levelno >= logging.ERROR + ] + assert len(messages) == 1 + assert "" in messages[0] + assert len(messages[0]) <= 1200 @pytest.mark.parametrize("untrusted_value", _HOSTILE_LOG_VALUES) @@ -211,3 +250,65 @@ def analyze(self, _path: object) -> object: assert len(messages) == 2 for message in messages: _assert_log_value_is_single_record(message, untrusted_value) + + +def test_cli_bounds_oversized_filename_logs( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """A buyer-controlled file name cannot create oversized routine log records.""" + oversized_name = f"buyer-{'A' * 5000}.wav" + stdin = io.StringIO( + json.dumps( + { + "jobId": "log-size-safety", + "request": { + "sourceKind": "local_audio", + "projectId": "project-log-size-safety", + "sourceLabel": "buyer.wav", + "roleFocus": [], + "localSource": { + "sourcePath": "/synthetic/buyer.wav", + "fileName": oversized_name, + "extension": "wav", + "fileSizeBytes": 1, + }, + }, + } + ) + ) + + class FailingAnalyzer: + """Exercise both CLI file-name log events without invoking a decoder.""" + + def analyze(self, _path: object) -> object: + """Fail after the first buyer-controlled file-name log event.""" + raise RuntimeError("expected test failure") + + monkeypatch.setattr(cli, "TemporalAnalyzer", FailingAnalyzer) + monkeypatch.setattr( + cli, + "run_analysis_job", + lambda _job_id, _request, requested_at: { + "jobId": "log-size-safety", + "state": "failed", + "requestedAt": requested_at, + "updatedAt": requested_at, + }, + ) + monkeypatch.setattr(cli.sys, "stdin", stdin) + monkeypatch.setattr(cli.sys, "stdout", io.StringIO()) + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) + caplog.set_level(logging.INFO) + + assert cli.main() == 0 + + messages = [ + record.getMessage() + for record in caplog.records + if "buyer-" in record.getMessage() + ] + assert len(messages) == 2 + for message in messages: + assert "" in message + assert len(message) <= 1200 From 5dd871da5f7c8ff38b56cdca2a8d4f76985d1a37 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 13:03:00 +0900 Subject: [PATCH 21/28] fix(logging): bound untrusted diagnostic rendering --- .../src/bandscope_analysis/logging_safety.py | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 services/analysis-engine/src/bandscope_analysis/logging_safety.py diff --git a/services/analysis-engine/src/bandscope_analysis/logging_safety.py b/services/analysis-engine/src/bandscope_analysis/logging_safety.py new file mode 100644 index 000000000..3bc7bfd55 --- /dev/null +++ b/services/analysis-engine/src/bandscope_analysis/logging_safety.py @@ -0,0 +1,77 @@ +"""Bounded rendering helpers for untrusted analysis log fields.""" + +from __future__ import annotations + +MAX_LOG_DIAGNOSTIC_CHARS = 1024 +MAX_LOG_TYPE_CHARS = 128 +_LOG_TRUNCATION_SUFFIX = "..." + + +def _safe_type_name(value: object) -> str: + """Read a bounded runtime type name without invoking instance or metaclass hooks.""" + value_type = object.__getattribute__(value, "__class__") + type_name = type.__getattribute__(value_type, "__name__") + if type(type_name) is not str: + return "unknown" + return single_line_log_text(type_name, max_chars=MAX_LOG_TYPE_CHARS) + + +def single_line_log_text( + value: str, + *, + max_chars: int = MAX_LOG_DIAGNOSTIC_CHARS, +) -> str: + """Escape controls and cap one untrusted textual log field. + + The function stops reading input once the rendered budget is exhausted, so + oversized buyer/dependency text cannot force a full-size escaped copy solely + for routine diagnostics. + """ + if max_chars < len(_LOG_TRUNCATION_SUFFIX): + raise ValueError("max_chars is too small for the truncation marker") + + chunks: list[str] = [] + rendered_chars = 0 + truncated = False + for character in value: + safe_character = ( + character + if character.isprintable() + else character.encode("unicode_escape").decode("ascii") + ) + if rendered_chars + len(safe_character) > max_chars: + truncated = True + break + chunks.append(safe_character) + rendered_chars += len(safe_character) + + if truncated: + while chunks and rendered_chars + len(_LOG_TRUNCATION_SUFFIX) > max_chars: + rendered_chars -= len(chunks.pop()) + chunks.append(_LOG_TRUNCATION_SUFFIX) + + return "".join(chunks) + + +def safe_log_value(value: object) -> str: + """Render an untrusted log value without executing arbitrary representation code.""" + if type(value) is str: + return single_line_log_text(value) + return f"<{_safe_type_name(value)}>" + + +def safe_exception_summary(error: BaseException) -> str: + """Render a bounded exception summary without calling dependency ``str``/``repr``.""" + error_type = _safe_type_name(error) + try: + args = BaseException.args.__get__(error, type(error)) + except Exception: + return error_type + + if not args or type(args[0]) is not str or not args[0]: + return error_type + + message_budget = MAX_LOG_DIAGNOSTIC_CHARS - len(error_type) - 2 + if message_budget < len(_LOG_TRUNCATION_SUFFIX): + return error_type + return f"{error_type}: {single_line_log_text(args[0], max_chars=message_budget)}" From cc0a504fd6854f37ab93ff8742c4d7f53b954930 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 13:03:18 +0900 Subject: [PATCH 22/28] fix(logging): avoid unbounded dependency repr in temporal diagnostics --- .../bandscope_analysis/temporal/analyzer.py | 21 ++++--------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py index e0cedd90f..82f6de5aa 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py @@ -12,6 +12,7 @@ import numpy as np from numpy.typing import NDArray +from ..logging_safety import safe_exception_summary, safe_log_value from .model import TemporalFeatures logger = logging.getLogger(__name__) @@ -28,20 +29,6 @@ BEATS_PER_BAR = 4 -def _single_line_log_repr(value: object) -> str: - """Render a diagnostic value without letting its repr create log controls.""" - try: - rendered = repr(value) - except Exception: - rendered = f"<{type(value).__name__} repr unavailable>" - return "".join( - character - if character.isprintable() - else character.encode("unicode_escape").decode("ascii") - for character in rendered - ) - - def _estimate_downbeats( onset_env: NDArray[np.floating[Any]], beat_frames: NDArray[np.integer[Any]], @@ -87,7 +74,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", _single_line_log_repr(path_str)) + logger.info("Loading and decoding audio: %s", safe_log_value(path_str)) try: with path.open("rb") as fileobj: @@ -156,7 +143,7 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: except Exception as e: logger.error( "Failed to analyze audio %s: %s", - _single_line_log_repr(path_str), - _single_line_log_repr(e), + safe_log_value(path_str), + safe_exception_summary(e), ) raise ValueError(f"Temporal analysis failed: {e}") from e From 18a47c87a494f2f89ddd7acb51395f2d8d312d50 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 13:03:32 +0900 Subject: [PATCH 23/28] fix(logging): cap buyer-controlled CLI log fields --- services/analysis-engine/src/bandscope_analysis/cli.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/cli.py b/services/analysis-engine/src/bandscope_analysis/cli.py index 0bbd154cd..422b6d6f7 100644 --- a/services/analysis-engine/src/bandscope_analysis/cli.py +++ b/services/analysis-engine/src/bandscope_analysis/cli.py @@ -8,6 +8,7 @@ from datetime import UTC, datetime from bandscope_analysis.api import get_analysis_status, run_analysis_job, run_analysis_job_updates +from bandscope_analysis.logging_safety import safe_log_value from bandscope_analysis.temporal import TemporalAnalyzer logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") @@ -86,7 +87,7 @@ def main() -> int: audio_path = local_source.get("sourcePath") file_name = local_source.get("fileName", "selected audio") if audio_path: - logging.info("Extracting temporal features from %s...", repr(file_name)) + logging.info("Extracting temporal features from %s...", safe_log_value(file_name)) try: temporal_analyzer = TemporalAnalyzer() features = temporal_analyzer.analyze(audio_path) @@ -94,7 +95,7 @@ def main() -> int: except Exception: logging.warning( "Temporal analysis failed for %s; continuing with safe fallback.", - repr(file_name), + safe_log_value(file_name), ) requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z") From d74089481f7e5d8489c68136502c9efae7a639c7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 13:03:54 +0900 Subject: [PATCH 24/28] docs(security): record bounded log diagnostic boundary --- .jules/sentinel.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index ac0a767ea..fa2e46723 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -6,7 +6,7 @@ ## 2024-07-07 - Unsanitized Directory Input Paths API Validation **Vulnerability:** The API logic allowed user-controlled local data directory paths (`cacheRoot` and `tempRoot`) to be directly used without mitigating cross-platform path traversal vulnerabilities. **Learning:** Checking for '..' sequences in untrusted paths fails to parse cross-platform separators reliably for untrusted inputs (e.g., Windows backslashes on POSIX). Relying solely on `os.sep` or `os.altsep` is inadequate because absolute paths can bypass restrictions if not resolved correctly, or if `os.altsep` is None. -**Prevention:** Manually replace backslashes with forward slashes and split by forward slash (e.g., `if '..' in path.replace('\\', '/').split('/')`) to enforce path traversal protections explicitly for restricted directory inputs provided via the API. Do not block `~` for user-selected input files. +**Prevention:** Manually replace backslashes with forward slashes and split by forward slash (e.g. `if '..' in path.replace('\\', '/').split('/')`) to enforce path traversal protections explicitly for restricted directory inputs provided via the API. Do not block `~` for user-selected input files. ## 2024-05-20 - Python Path Traversal Mitigation bypass **Vulnerability:** Path traversal detection in Python backend APIs relied solely on checking the input path string or basic parsed parts which might not adequately catch sequences like `..` when intermixed with different path separators. @@ -29,7 +29,7 @@ **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. -## 2026-09-21 - Prevent log forging / injection -**Vulnerability:** Untrusted file names, paths, or decoder exception text could carry carriage returns, line separators, terminal controls, or other non-printable characters into line-oriented logs and create forged or misleading records. -**Learning:** Deferred logging interpolation avoids eager string construction but does not neutralize controls. `repr()` is also not a complete trust boundary for arbitrary dependency objects: a custom `__repr__` implementation may emit raw control characters or raise while the program is already handling another exception. The final rendered representation must be checked before it reaches the logging formatter, and diagnostic rendering must fail safely rather than replace the original analysis failure. -**Prevention:** Keep message templates parameterized. For plain strings, use a representation that escapes controls while retaining ordinary printable Unicode. For arbitrary dependency objects, attempt to render the object, fall back to a code-owned type marker if representation fails, and then escape every non-printable code point in the resulting text. Regression tests must cover LF/CRLF, tab, ANSI ESC, NUL/C0, Unicode line separators, a custom exception `__repr__` that emits raw controls, and a custom `__repr__` that raises; one logical event must remain one physical log record and logging must not mask the original decoder failure. +## 2026-09-21 - Prevent log forging and excessive diagnostic output +**Vulnerability:** Untrusted file names, paths, or decoder exception text could carry carriage returns, line separators, terminal controls, or oversized payloads into line-oriented logs. Invoking an arbitrary dependency exception's `__repr__` while logging also executes dependency-controlled code during an error path and can allocate an unbounded representation before sanitization. +**Learning:** Deferred logging interpolation avoids eager template construction but does not neutralize controls or bound diagnostic size. Sanitizing the result of `repr()` is too late to make arbitrary dependency representation code a trusted boundary. Routine logs should consume bounded plain-text fields and code-owned exception metadata without invoking dependency `str`/`repr`; control escaping and size limits belong at the final log-field boundary. +**Prevention:** Keep message templates parameterized. Route buyer/dependency text through the shared bounded single-line renderer, which stops consuming input when the rendered budget is exhausted and marks truncation. For arbitrary exceptions, read only the runtime type name and an exact-string base exception argument; do not call dependency `__repr__` or `__str__` for the log record. Regression tests cover LF/CRLF, tab, ANSI ESC, NUL/C0, Unicode line separators, printable Unicode, hostile/broken `__repr__`, oversized exception messages, and oversized buyer-controlled file names. One logical event must remain one bounded physical log record, and logging must not mask the original decoder failure. From 196577c8ef757a0b32532b208a8d951d158b5df6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 13:10:36 +0900 Subject: [PATCH 25/28] test(security): reject dependency str execution in temporal failure wrapping --- .../tests/test_logging_safety.py | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/services/analysis-engine/tests/test_logging_safety.py b/services/analysis-engine/tests/test_logging_safety.py index 11cc0fcac..5ba6ad44c 100644 --- a/services/analysis-engine/tests/test_logging_safety.py +++ b/services/analysis-engine/tests/test_logging_safety.py @@ -158,6 +158,36 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: assert "repr unavailable" not in messages[0] +def test_temporal_failure_does_not_execute_dependency_str( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Wrapping a decoder failure must not execute dependency-controlled str code.""" + audio_path = tmp_path / "buyer-audio.wav" + audio_path.write_bytes(b"not-a-real-wave") + str_calls = 0 + + class BrokenStrDecoderError(RuntimeError): + """Model a decoder exception whose string conversion is unsafe.""" + + def __str__(self) -> str: + """Raise if the analyzer executes dependency string conversion.""" + nonlocal str_calls + str_calls += 1 + raise RuntimeError("str failed") + + def fail_decode(*_args: object, **_kwargs: object) -> object: + """Raise the dependency-shaped exception through the real analyzer path.""" + raise BrokenStrDecoderError("decoder failed") + + monkeypatch.setattr(analyzer_module.librosa, "load", fail_decode) + + with pytest.raises(ValueError, match="Temporal analysis failed: decoder failed"): + TemporalAnalyzer().analyze(audio_path) + + assert str_calls == 0 + + def test_temporal_error_log_bounds_oversized_exception_message( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, @@ -175,7 +205,7 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: monkeypatch.setattr(analyzer_module.librosa, "load", fail_decode) caplog.set_level(logging.ERROR, logger=analyzer_module.__name__) - with pytest.raises(ValueError, match="Temporal analysis failed"): + with pytest.raises(ValueError, match="Temporal analysis failed") as raised: TemporalAnalyzer().analyze(audio_path) messages = [ @@ -186,6 +216,8 @@ def fail_decode(*_args: object, **_kwargs: object) -> object: assert len(messages) == 1 assert "" in messages[0] assert len(messages[0]) <= 1200 + assert "" in str(raised.value) + assert len(str(raised.value)) <= 1200 @pytest.mark.parametrize("untrusted_value", _HOSTILE_LOG_VALUES) From b906e3e926c93050d36474cfb42d9c0c280b78a3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 13:10:51 +0900 Subject: [PATCH 26/28] fix(security): bound temporal failure messages without dependency str --- .../src/bandscope_analysis/logging_safety.py | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/logging_safety.py b/services/analysis-engine/src/bandscope_analysis/logging_safety.py index 3bc7bfd55..e3ad51e7d 100644 --- a/services/analysis-engine/src/bandscope_analysis/logging_safety.py +++ b/services/analysis-engine/src/bandscope_analysis/logging_safety.py @@ -1,4 +1,4 @@ -"""Bounded rendering helpers for untrusted analysis log fields.""" +"""Bounded rendering helpers for untrusted analysis diagnostics.""" from __future__ import annotations @@ -21,11 +21,11 @@ def single_line_log_text( *, max_chars: int = MAX_LOG_DIAGNOSTIC_CHARS, ) -> str: - """Escape controls and cap one untrusted textual log field. + """Escape controls and cap one untrusted textual diagnostic field. The function stops reading input once the rendered budget is exhausted, so oversized buyer/dependency text cannot force a full-size escaped copy solely - for routine diagnostics. + for diagnostics. """ if max_chars < len(_LOG_TRUNCATION_SUFFIX): raise ValueError("max_chars is too small for the truncation marker") @@ -60,8 +60,20 @@ def safe_log_value(value: object) -> str: return f"<{_safe_type_name(value)}>" +def safe_exception_message(error: BaseException) -> str: + """Return bounded exception text without calling dependency ``str``/``repr``.""" + try: + args = BaseException.args.__get__(error, type(error)) + except Exception: + return _safe_type_name(error) + + if not args or type(args[0]) is not str or not args[0]: + return _safe_type_name(error) + return single_line_log_text(args[0]) + + def safe_exception_summary(error: BaseException) -> str: - """Render a bounded exception summary without calling dependency ``str``/``repr``.""" + """Render a bounded typed exception summary without dependency ``str``/``repr``.""" error_type = _safe_type_name(error) try: args = BaseException.args.__get__(error, type(error)) From 6d0e761beca3de842f215cdfb5eeeac93f71cd1f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 13:11:08 +0900 Subject: [PATCH 27/28] fix(security): wrap temporal failures without dependency str execution --- .../src/bandscope_analysis/temporal/analyzer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py index 82f6de5aa..7a5064ce2 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py @@ -12,7 +12,7 @@ import numpy as np from numpy.typing import NDArray -from ..logging_safety import safe_exception_summary, safe_log_value +from ..logging_safety import safe_exception_message, safe_exception_summary, safe_log_value from .model import TemporalFeatures logger = logging.getLogger(__name__) @@ -146,4 +146,4 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: safe_log_value(path_str), safe_exception_summary(e), ) - raise ValueError(f"Temporal analysis failed: {e}") from e + raise ValueError(f"Temporal analysis failed: {safe_exception_message(e)}") from e From 4ad88b6abb738a8a450d980bb7ebf003bd643721 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 21 Sep 2026 13:11:26 +0900 Subject: [PATCH 28/28] docs(security): extend safe diagnostics through temporal failure wrapping --- .jules/sentinel.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index fa2e46723..2dc3a51f0 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -30,6 +30,6 @@ **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. ## 2026-09-21 - Prevent log forging and excessive diagnostic output -**Vulnerability:** Untrusted file names, paths, or decoder exception text could carry carriage returns, line separators, terminal controls, or oversized payloads into line-oriented logs. Invoking an arbitrary dependency exception's `__repr__` while logging also executes dependency-controlled code during an error path and can allocate an unbounded representation before sanitization. -**Learning:** Deferred logging interpolation avoids eager template construction but does not neutralize controls or bound diagnostic size. Sanitizing the result of `repr()` is too late to make arbitrary dependency representation code a trusted boundary. Routine logs should consume bounded plain-text fields and code-owned exception metadata without invoking dependency `str`/`repr`; control escaping and size limits belong at the final log-field boundary. -**Prevention:** Keep message templates parameterized. Route buyer/dependency text through the shared bounded single-line renderer, which stops consuming input when the rendered budget is exhausted and marks truncation. For arbitrary exceptions, read only the runtime type name and an exact-string base exception argument; do not call dependency `__repr__` or `__str__` for the log record. Regression tests cover LF/CRLF, tab, ANSI ESC, NUL/C0, Unicode line separators, printable Unicode, hostile/broken `__repr__`, oversized exception messages, and oversized buyer-controlled file names. One logical event must remain one bounded physical log record, and logging must not mask the original decoder failure. +**Vulnerability:** Untrusted file names, paths, or decoder exception text could carry carriage returns, line separators, terminal controls, or oversized payloads into line-oriented logs or the analyzer's wrapped failure text. Invoking an arbitrary dependency exception's `__repr__` or `__str__` while handling a failure also executes dependency-controlled code on an error path and can replace the original decoder failure or allocate an unbounded representation before sanitization. +**Learning:** Deferred logging interpolation avoids eager template construction but does not neutralize controls or bound diagnostic size. Sanitizing the result of `repr()` or `str()` is too late to make arbitrary dependency representation code a trusted boundary. Diagnostics should consume bounded plain-text fields and code-owned exception metadata without invoking dependency `str`/`repr`; control escaping and size limits belong before both log formatting and error-envelope construction. +**Prevention:** Keep log templates parameterized. Route buyer/dependency text through the shared bounded single-line renderer, which stops consuming input when the rendered budget is exhausted and marks truncation. For arbitrary exceptions, read only the runtime type name and an exact-string base exception argument; do not call dependency `__repr__` or `__str__` for the log record or the analyzer's wrapped `ValueError`. Regression tests cover LF/CRLF, tab, ANSI ESC, NUL/C0, Unicode line separators, printable Unicode, hostile/broken `__repr__`, broken `__str__`, oversized exception messages, and oversized buyer-controlled file names. One logical event must remain one bounded physical log record, and diagnostic handling must not mask the original decoder failure.