diff --git a/pyproject.toml b/pyproject.toml index 259ca903..ddc4942f 100644 --- a/pyproject.toml +++ b/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" } diff --git a/roar/filters/omit.py b/roar/filters/omit.py index 868f49c2..d4eeb7a8 100644 --- a/roar/filters/omit.py +++ b/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"(? 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"]