Release 0.4.7 — tell a version pin from a credential by value, not case

pyproject.toml

@@ -5,7 +5,7 @@ build-backend = "maturin"
 
 [project]
 name = "roar-cli"
-version = "0.4.6"
+version = "0.4.7"
 description = "Reproducibility and provenance tracker for ML training pipelines"
 authors = [
     { name="TReqs Team", email="info@treqs.ai" }

roar/filters/omit.py

@@ -38,6 +38,52 @@ def was_modified(self) -> bool:
         return len(self.detections) > 0
 
 
+# A name that DENOTES a credential, as opposed to one that merely contains a keyword.
+# The distinction is the whole problem: "tiktoken" ends in "token" and is a tokeniser,
+# "authlib" begins with "auth" and is a library, "keyring" is a keyring. Matching those
+# redacted their VERSIONS out of published dependency freezes, so the recorded
+# environment could not be reinstalled -- the one thing a freeze exists to make
+# possible. It cost a completed 3.5-hour training run its reproducibility gate twice.
+#
+# Casing is NOT the discriminator. Two earlier fixes assumed it was -- one dropped
+# case-insensitivity, the other enumerated three casing shapes -- and both stopped
+# redacting real credentials (Hf_Token, MyToken, api_key) while still breaking package
+# names in one serialization or another. What actually separates the two:
+#
+#   1. a WORD BOUNDARY in the name. "api_key" and "MyToken" carry the keyword as its
+#      own word, bounded by a delimiter, a case transition, or an edge. "tiktoken" and
+#      "tokenizer" are single words that happen to contain one.
+#   2. the VALUE. A version pin is never a credential -- see _VERSION below.
+#
+# With both guards in place the pattern does not need to exclude any casing, which is
+# why every form 0.4.5 caught is still caught here.
+_KW = r"(?:key|token|secret|password|passwd|pwd|credential|auth)"
+_KW_CAP = r"(?:Key|Token|Secret|Password|Passwd|Pwd|Credential|Auth)"
+_KW_UPPER = r"(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|PWD|CREDENTIAL|AUTH)"
+
+_SECRET_NAME = (
+    r"ROAR_SESSION_ID"
+    # delimiter-bounded, any casing: api_key, Api_Key, HF_TOKEN, my-token, X-Api-Key,
+    # TOKEN_my, google-auth, or the bare word itself (Token, secret, PASSWORD).
+    rf"|(?:[A-Za-z0-9]+[_-])*(?i:{_KW})(?:[_-][A-Za-z0-9]+)*"
+    # camel/Pascal boundary: apiKey, myToken, xApiKey, MyToken, ApiKey, APIKey, HfToken
+    rf"|(?:[A-Za-z][A-Za-z0-9]*)?{_KW_CAP}[A-Za-z0-9]*"
+    # lowercase keyword immediately followed by a capital: secretValue, authToken
+    rf"|{_KW}(?=[A-Z])[A-Za-z0-9]*"
+    # all-caps run around an all-caps keyword: MYTOKEN, AWS_SECRET
+    rf"|[A-Z0-9_]*{_KW_UPPER}[A-Z0-9_]*"
+)
+
+# A version pin is never a credential, in any casing or serialization. This guard --
+# not the name pattern -- is what spares tiktoken==0.11.0, {"tiktoken": "0.11.0"} and
+# the google-auth / dj-rest-auth family alike, including package names that genuinely
+# do carry a delimited keyword. Covers PEP 440: 1.2.3, 2.0.0rc1, 1.0.dev4, 2.9.1+cu121.
+_VERSION = (
+    r"\d+(?:\.\d+)*"
+    r"(?:[._-]?(?:a|b|c|rc|alpha|beta|dev|post|final)\d*)*"
+    r"(?:\+[A-Za-z0-9._-]+)?"
+)
+
 # Built-in patterns for common secret formats
 # Each pattern is a tuple of (id, compiled_regex, replacement)
 BUILTIN_PATTERNS: list[tuple[str, re.Pattern, str]] = [
@@ -161,45 +207,28 @@ def was_modified(self) -> bool:
     ),
     # 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.
+    # A single "=" only, and never when the value is a version pin. Both restrictions
+    # exist so that "tiktoken==0.11.0" in a recorded pip command survives intact.
     (
         "env_var_assignment",
         re.compile(
-            r"([A-Z_]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|PWD|CREDENTIAL|AUTH)[A-Z_]*)"
-            r"(?<!=)=(?!=)([^\s]+)"
+            rf"({_SECRET_NAME})" r"(?<!=)=(?!=)" rf"((?!{_VERSION}(?:\s|$))[^\s]+)",
         ),
         r"\1=[REDACTED]",
     ),
     # Sensitive environment values embedded in JSON, including Ray's
     # --runtime-env-json command argument. Optional backslashes cover JSON
     # nested inside a serialized command string.
+    #
+    # This is the form roar actually publishes package maps in, so the version guard
+    # matters more here than in the assignment rule: {"tiktoken": "0.11.0"} reached a
+    # published freeze as {"tiktoken": "[REDACTED]"} when only the "==" form was fixed.
     (
         "json_named_secret",
         re.compile(
-            r"((?:\\?[\"'])(?:ROAR_SESSION_ID|[A-Z_]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|PWD|CREDENTIAL|AUTH)[A-Z_]*)(?:\\?[\"'])\s*:\s*(?:\\?[\"']))(.*?)(\\?[\"'])",
-            re.IGNORECASE,
+            rf"((?:\\?[\"'])(?:{_SECRET_NAME})(?:\\?[\"'])\s*:\s*(?:\\?[\"']))"
+            rf"((?!{_VERSION}\\?[\"'])"
+            r".*?)(\\?[\"'])",
         ),
         r"\1[REDACTED]\3",
     ),

tests/unit/test_omit_filter_version_pins.py

@@ -1,100 +1,163 @@
-"""A version pin is not a credential.
+"""A package version is not a credential; a credential is not a package version.
 
-The env-var 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. A published
-freeze then carried ``'tiktoken==[REDACTED]'``, which no installer can execute — the
-recorded environment could not be rebuilt, which is the one thing a freeze exists to
-make possible.
+Both halves have been broken by a fix for the other. Redacting a version makes the
+published freeze uninstallable, which cost a 3.5-hour training run its reproducibility
+gate twice. Narrowing the name pattern to stop that stopped real credentials from being
+redacted -- first every lowercase name, then 42 mixed-case ones, then package names
+containing a delimited keyword (google-auth) in the serialized form.
 
-These pin both directions against the real patterns: package pins survive, and actual
-environment assignments are still redacted.
+So each case below is asserted in every serialization roar actually writes: the
+``name==version`` string, the ``{"name": "version"}`` JSON the record serializes, and
+the escaped JSON nested inside a serialized command.
 """
 
 from __future__ import annotations
 
+import json
+
 import pytest
 
 from roar.filters.omit import OmitFilter
 
-# Assembled at runtime so secret scanners do not flag the fixtures as real credentials.
-FAKE_HF_TOKEN = "hf_" + "AbCdEfGhIjKlMnOpQrStUvWxYz012345"
-FAKE_OPENAI_KEY = "sk-" + "AbCdEfGhIjKlMnOpQrStUv"
+# Assembled at runtime so secret scanners do not flag the fixtures.
+HF = "hf_" + "AbCdEfGhIjKlMnOpQrStUvWxYz012345"
+SK = "sk-" + "AbCdEfGhIjKlMnOpQrStUv"
+OPAQUE = "hunter2hunter2"  # a real secret with no recognisable shape
 
 
 @pytest.fixture()
-def omit_filter() -> OmitFilter:
+def f() -> 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"),
+def forms(name: str, value: str) -> list[str]:
+    """The name/value pair in every serialization the record is written in."""
+    return [
+        f"{name}=={value}",
+        f"{name}={value}",
+        json.dumps({name: value}),
+        json.dumps(json.dumps({name: value})),  # escaped, nested in a command string
+    ]
+
+
+# Real dependencies whose names carry a keyword. None is a credential.
+# google-auth and friends carry a *delimited* keyword, so only the version guard
+# spares them -- they are the case a name-only pattern cannot get right.
+PACKAGES = [
+    "tiktoken",
+    "authlib",
+    "keyring",
+    "tokenizers",
+    "secretstorage",
+    "python-jose",
+    "google-auth",
+    "google-auth-oauthlib",
+    "dj-rest-auth",
+    "social-auth-core",
+    "oauthlib",
+    "azure-keyvault",
 ]
 
-
-@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"),
+# Names that denote a credential, across every casing convention in use. Each was
+# redacted by 0.4.5 and silently stopped being redacted by one of its successors.
+SECRET_NAMES = [
+    "HF_TOKEN",
+    "hf_token",
+    "Hf_Token",
+    "HfToken",
+    "MYTOKEN",
+    "MyToken",
+    "myToken",
+    "API_KEY",
+    "api_key",
+    "api-key",
+    "Api_Key",
+    "apiKey",
+    "ApiKey",
+    "APIKey",
+    "secretValue",
+    "SecretValue",
+    "TOKEN_my",
+    "Github_Token",
+    "X-Api-Key",
+    "Token",
+    "Key",
+    "Secret",
+    "Password",
+    "Credential",
+    "DB_PASSWORD",
 ]
 
 
-@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
+@pytest.mark.parametrize("package", PACKAGES)
+@pytest.mark.parametrize("version", ["0.11.0", "2.0.0rc1", "1.0.dev4", "2.9.1+cu121"])
+def test_a_version_pin_survives_every_serialization(f: OmitFilter, package, version):
+    for text in forms(package, version):
+        assert f.filter_string(text, field="packages").filtered == text, text
+
+
+@pytest.mark.parametrize("name", SECRET_NAMES)
+def test_a_credential_is_redacted_in_every_serialization(f: OmitFilter, name):
+    # Narrowing the name pattern must never cost this. Every casing here is a name a
+    # real project uses for a real secret.
+    for text in forms(name, OPAQUE):
+        if text.startswith(f"{name}=="):
+            continue  # "==" is the version-pin form, deliberately exempt
+        assert OPAQUE not in f.filter_string(text, field="runtime").filtered, text
+
+
+def test_a_keyword_inside_a_word_is_not_a_credential_name(f: OmitFilter):
+    # "tokenizer" contains "token" but is one word; "api_key" carries it as its own.
+    # This boundary is what keeps a recorded command executable.
+    command = "python -m train --tokenizer=gpt2 --depth=14 --lr=3e-4"
+    assert f.filter_string(command, field="command").filtered == command
+
+    install = "pip install torch==2.9.1 tiktoken==0.11.0 google-auth==2.35.0"
+    assert f.filter_string(install, field="command").filtered == install
+
+
+def test_a_secret_is_caught_by_value_even_when_the_name_says_nothing(f: OmitFilter):
+    # The backstop for names no pattern anticipates.
+    assert HF not in f.filter_string(f"whatever={HF}", field="command").filtered
+    assert SK not in f.filter_string(f'{{"opts": "{SK}"}}', field="runtime").filtered
+
+
+def test_a_real_record_keeps_its_versions_and_loses_its_secrets(f: OmitFilter):
+    """Both halves of one record, through the entry point that publishes it.
+
+    A filter that stopped redacting would pass every "version survives" case above and
+    be catastrophically wrong, so the two are asserted together. Driven through
+    filter_metadata rather than filter_string because a fix verified only on the string
+    form is exactly how the broken version shipped.
+    """
+    packages = dict.fromkeys(PACKAGES, "1.2.3")
+    record = {
+        "packages": {"pip": packages},
+        # the same map as a serialized blob -- the form that reached a published freeze
+        # as {"tiktoken": "[REDACTED]"} when only the string form had been fixed
+        "python_capture": json.dumps({"pip": packages}),
+        "runtime": {
+            "env_vars": {"HF_TOKEN": HF, "api_key": SK, "PATH": "/usr/local/bin"},
+            "command": f"env HF_TOKEN={HF} python -m train --tokenizer=gpt2 --depth=14",
+        },
+        "git": {"remote_url": f"https://x-access-token:{HF}@github.com/org/repo.git"},
+    }
+
+    out, _ = f.filter_metadata(json.loads(json.dumps(record)))
+    blob = json.dumps(out)
+
+    # Every version intact, in both serializations -- the freeze must stay installable.
+    assert out["packages"]["pip"] == packages
+    assert json.loads(out["python_capture"])["pip"] == packages
+
+    # Every credential gone, by name and by value.
+    for secret in (HF, SK):
+        assert secret not in blob
+    for name in ("HF_TOKEN", "api_key"):
+        # marker varies: a value-shaped rule may claim it first ([HF_TOKEN_REDACTED])
+        assert "REDACTED" in out["runtime"]["env_vars"][name], name
+
+    # Innocent environment survives, including unrelated "=" in the command.
+    assert out["runtime"]["env_vars"]["PATH"] == "/usr/local/bin"
+    assert "--depth=14" in out["runtime"]["command"]
+    assert "--tokenizer=gpt2" in out["runtime"]["command"]