diff --git a/roar/filters/omit.py b/roar/filters/omit.py index ebd85356..868f49c2 100644 --- a/roar/filters/omit.py +++ b/roar/filters/omit.py @@ -159,12 +159,36 @@ def was_modified(self) -> bool: re.compile(r"(hooks\.slack\.com/services/)([A-Z0-9/]+)", re.IGNORECASE), r"\1[REDACTED]", ), - # Environment variable assignments in commands + # Environment variable assignments in commands. + # + # Case-sensitive, and a single "=" only. Both restrictions are load-bearing. + # + # This rule matched any name CONTAINING key/token/secret/..., case-insensitively, + # followed by "=". A pip requirement satisfies that: "tiktoken==0.12.0" is a name + # ending in "token" followed by "=", so the VERSION was redacted as if it were a + # credential. A published freeze then carried + # + # 'tiktoken==[REDACTED]' + # + # which no installer can execute, so the recorded environment could not be rebuilt + # -- the one thing the freeze exists to make possible. It cost a 3.5-hour training + # run its reproducibility gate, and it is not specific to tiktoken: authlib, + # keyring, tokenizers and python-jose all contain a keyword. + # + # Environment variables are uppercase by convention, and POSIX reserves that space + # for them, so dropping IGNORECASE keeps HF_TOKEN=, API_KEY= and MYTOKEN= while + # sparing every lowercase package name. Requiring a single "=" spares version pins + # regardless of case. + # + # The gap this leaves is a lowercase-named variable holding a secret with no + # recognisable prefix (hf_token=..., where the value is not hf_...). That is + # unconventional, and the value-shaped rules above -- hf_, sk-, ghp_, glpat-, AKIA + # -- catch the real providers by their token format rather than by variable name. ( "env_var_assignment", re.compile( - r"([A-Z_]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|PWD|CREDENTIAL|AUTH)[A-Z_]*)=([^\s]+)", - re.IGNORECASE, + r"([A-Z_]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|PWD|CREDENTIAL|AUTH)[A-Z_]*)" + r"(? OmitFilter: + return OmitFilter({}) + + +# Every one of these contains a keyword the rule looks for, and every one is a +# dependency people really install. +PACKAGE_PINS = [ + pytest.param("tiktoken==0.12.0", id="tiktoken"), + pytest.param("authlib==1.3.2", id="authlib"), + pytest.param("keyring==25.4.1", id="keyring"), + pytest.param("tokenizers==0.20.3", id="tokenizers"), + pytest.param("python-jose[cryptography]==3.3.0", id="python-jose"), + pytest.param("secretstorage==3.3.3", id="secretstorage"), +] + + +@pytest.mark.parametrize("requirement", PACKAGE_PINS) +def test_package_pin_survives_filtering(omit_filter: OmitFilter, requirement: str) -> None: + result = omit_filter.filter_string(requirement, field="packages") + + assert result.filtered == requirement + assert result.detections == [] + + +def test_full_install_command_survives(omit_filter: OmitFilter) -> None: + # The shape that actually broke: a generated install line from a freeze. If any + # version is replaced the command cannot be executed literally, which is exactly + # the failure the reproducibility gate catches -- after the compute is spent. + command = "pip install torch==2.9.1 tiktoken==0.12.0 regex==2025.9.1 authlib==1.3.2" + + result = omit_filter.filter_string(command, field="command") + + assert result.filtered == command + assert "[REDACTED]" not in result.filtered + + +ENV_ASSIGNMENTS = [ + pytest.param(f"HF_TOKEN={FAKE_HF_TOKEN}", "HF_TOKEN", id="hf-token"), + pytest.param(f"OPENAI_API_KEY={FAKE_OPENAI_KEY}", "OPENAI_API_KEY", id="openai-key"), + pytest.param("MYTOKEN=abc123def456", "MYTOKEN", id="unprefixed-uppercase"), + pytest.param("DB_PASSWORD=hunter2hunter2", "DB_PASSWORD", id="password"), + pytest.param("AWS_SECRET=abcdefghijklmnop", "AWS_SECRET", id="secret"), +] + + +@pytest.mark.parametrize("assignment,name", ENV_ASSIGNMENTS) +def test_environment_assignment_is_still_redacted( + omit_filter: OmitFilter, assignment: str, name: str +) -> None: + # The reason the rule exists. Narrowing it must not cost this. + result = omit_filter.filter_string(assignment, field="command") + + assert result.filtered == f"{name}=[REDACTED]" + assert assignment.split("=", 1)[1] not in result.filtered + + +def test_assignment_inside_a_command_is_still_redacted(omit_filter: OmitFilter) -> None: + command = f"env HF_TOKEN={FAKE_HF_TOKEN} python -m scripts.base_train --depth=14" + + result = omit_filter.filter_string(command, field="command") + + assert FAKE_HF_TOKEN not in result.filtered + assert "HF_TOKEN=[REDACTED]" in result.filtered + # The unrelated argument must survive intact. + assert "--depth=14" in result.filtered + + +def test_provider_token_is_caught_by_value_even_when_the_name_is_lowercase( + omit_filter: OmitFilter, +) -> None: + # The gap the narrowing leaves is a lowercase variable name. Real providers are + # still caught by the shape of the value, which is why that gap is acceptable. + result = omit_filter.filter_string(f"hf_token={FAKE_HF_TOKEN}", field="command") + + assert FAKE_HF_TOKEN not in result.filtered