From 9563699ecac1c06d9f2a027267c23e1803166f1a Mon Sep 17 00:00:00 2001 From: Alpamys Date: Mon, 18 May 2026 21:25:56 +0500 Subject: [PATCH] =?UTF-8?q?fix(v0.59.0):=20rewrite=20null-byte=20env=20tes?= =?UTF-8?q?t=20=E2=80=94=20OS=20layer=20rejects=20setenv=20on=20every=20pl?= =?UTF-8?q?atform?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous test_default_log_path_rejects_null_byte_env used monkeypatch.setenv to inject a null byte into SOUP_AUDIT_LOG_PATH and expected default_log_path to fall back gracefully. But the OS layer rejects null bytes in env vars on every platform we ship on: - POSIX (Linux/macOS): `ValueError: embedded null byte` - Windows: `ValueError: embedded null character` The setenv call itself raises, never reaching default_log_path. Split into two tests that hit the actual validation surfaces: 1. test_default_log_path_rejects_null_byte_override — calls the private _validate_log_path_override helper directly with a null-byte string and asserts it returns None (so the caller falls back to the safe default). 2. test_default_log_path_handles_env_read_value_error — monkeypatches os.environ.get to raise ValueError, exercising the defence-in-depth try/except around the env read in default_log_path(). Both tests pass on Linux + macOS + Windows. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_v0590.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/tests/test_v0590.py b/tests/test_v0590.py index 308afc6..e6af989 100644 --- a/tests/test_v0590.py +++ b/tests/test_v0590.py @@ -1104,13 +1104,33 @@ class TestReviewFollowups: resolved = default_log_path() assert resolved == str(target) - def test_default_log_path_rejects_null_byte_env(self, monkeypatch, tmp_path): - from soup_cli.utils.audit_log import default_log_path + def test_default_log_path_rejects_null_byte_override(self): + """The OS layer rejects null bytes in env vars on every platform we ship + on (POSIX raises ValueError, Windows raises "embedded null character"), + so we cannot inject one via monkeypatch.setenv. Test the validator + directly instead — it must return None for any null-byte path so the + caller falls back to the safe default.""" + from soup_cli.utils.audit_log import _validate_log_path_override + + assert _validate_log_path_override("/tmp/\x00/audit.jsonl") is None + + def test_default_log_path_handles_env_read_value_error(self, monkeypatch, tmp_path): + """If the env layer somehow raises ValueError on read (defence in depth + for the POSIX `embedded null byte` path), default_log_path must still + return a safe default.""" + from soup_cli.utils import audit_log monkeypatch.chdir(tmp_path) - monkeypatch.setenv("SOUP_AUDIT_LOG_PATH", "/tmp/\x00/audit.jsonl") - resolved = default_log_path() + + def boom(key, default=None): + if key == "SOUP_AUDIT_LOG_PATH": + raise ValueError("embedded null byte") + return os.environ.get(key, default) + + monkeypatch.setattr(audit_log.os.environ, "get", boom) + resolved = audit_log.default_log_path() assert "\x00" not in resolved + assert resolved.endswith("audit.jsonl") # --- Code review #2 / Security L1: artifact size_bytes validation --- def test_bom_artifact_size_bytes_non_int_rejected(self):