From 12e6d04ca0f1fa057589ed529869fd44281e91b8 Mon Sep 17 00:00:00 2001 From: Chris Geyer Date: Thu, 10 Sep 2026 09:23:24 -0400 Subject: [PATCH 1/2] A package name is not a credential name, in any serialization (#299) * fix(filters): a package name is not a credential name, in any serialization 0.4.6 fixed "tiktoken==0.11.0" and shipped. It did not fix {"tiktoken": "0.11.0"}, and roar records packages as dict[str, str] keyed by package name, so the published freeze went on carrying "tiktoken": "[REDACTED]" Reproduced on a real job: the on-host checks all passed -- roar 0.4.6 verified by wheel hash, requirements pinned 0.11.0, the installed distribution was 0.11.0 -- and the published record was still wrong, because only the serialized form is filtered through json_named_secret. Row 024 spent a second 3.5-hour run discovering that. Both rules now share one pattern for a name that DENOTES a credential rather than merely containing a keyword. Three shapes qualify, and no package name matches any: UPPERCASE HF_TOKEN, API_KEY, MYTOKEN env vars, POSIX convention delimited api_key, access-token, token the keyword is its own word camelCase apiKey, accessToken the capital is the delimiter tiktoken fails all three: lowercase, "token" undelimited within it, no capital. So do authlib, keyring, tokenizers and secretstorage, all of which 0.4.6 still redacted in the JSON form. This is also strictly better than 0.4.6 in the other direction. That fix dropped IGNORECASE wholesale and stopped matching lowercase names entirely, so api_key=... and {"api_key": ...} went unredacted. The delimiter test restores them. Tests cover both directions in both serializations, including the full record shape as it is actually written. Negative control: restoring the 0.4.6 json rule fails the serialized-map case while the string-form cases still pass -- the gap that let this ship. Full unit suite green, 1234 tests. Version bumped to 0.4.7 here so the rc branch is release-ready; 0.4.6's release build failed because the tag was cut ahead of the bump. Co-Authored-By: Claude Opus 5 (1M context) * test: the true positive and the false positive in one record A filter that simply stopped redacting would pass every package-survives case and be catastrophically wrong. This asserts both halves of the same artifact: dependency versions come through intact, and credentials sitting beside them in the captured environment do not -- by name shape (uppercase, delimited, camelCase) and by value. Also pins the innocent neighbours: PATH survives, and --depth=14 survives in a command whose HF_TOKEN= is redacted. Negative-controlled in both directions. Neutering the credential-name pattern fails all 22; restoring the 0.4.6 json rule fails only the serialized-map case. * style: f-strings for the JSON secret fixtures (ruff UP031) --------- Co-authored-by: Chris Geyer Co-authored-by: Claude Opus 5 (1M context) --- pyproject.toml | 2 +- roar/filters/omit.py | 33 ++++-- tests/unit/test_omit_filter_version_pins.py | 113 +++++++++++++++++++- 3 files changed, 138 insertions(+), 10 deletions(-) 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..a35ae59f 100644 --- a/roar/filters/omit.py +++ b/roar/filters/omit.py @@ -39,6 +39,31 @@ def was_modified(self) -> bool: # Built-in patterns for common secret formats +# A name that DENOTES a credential, as opposed to a name 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, which +# made the recorded environment uninstallable -- and cost a completed 3.5-hour +# training run its reproducibility gate twice, because the first fix covered only the +# "name==version" string form and not the {"name": "version"} form the record +# actually serializes. +# +# Three shapes denote a credential, and none of them matches a package name: +# +# UPPERCASE HF_TOKEN, API_KEY, MYTOKEN -- env vars, POSIX convention +# delimited api_key, access-token, token -- the keyword is its own word +# camelCase apiKey, accessToken -- the capital is the delimiter +# +# "tiktoken" fails all three: it is lowercase, "token" is not delimited within it, +# and there is no capital. Same for authlib, keyring, tokenizers, secretstorage. +_SECRET_NAME = ( + r"ROAR_SESSION_ID" + r"|[A-Z0-9_]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|PWD|CREDENTIAL|AUTH)[A-Z0-9_]*" + r"|(?:[a-z0-9]+[_-])*(?:key|token|secret|password|passwd|pwd|credential|auth)" + r"(?:[_-][a-z0-9]+)*" + r"|[a-z0-9]+(?:Key|Token|Secret|Password|Passwd|Pwd|Credential|Auth)[A-Za-z0-9]*" +) + # Each pattern is a tuple of (id, compiled_regex, replacement) BUILTIN_PATTERNS: list[tuple[str, re.Pattern, str]] = [ # AWS credentials @@ -186,10 +211,7 @@ def was_modified(self) -> bool: # -- 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_]*)" - r"(? bool: ( "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, + r"((?:\\?[\"'])(?:" + _SECRET_NAME + r")(?:\\?[\"'])\s*:\s*(?:\\?[\"']))(.*?)(\\?[\"'])" ), r"\1[REDACTED]\3", ), diff --git a/tests/unit/test_omit_filter_version_pins.py b/tests/unit/test_omit_filter_version_pins.py index 838aeb64..c16e4ff9 100644 --- a/tests/unit/test_omit_filter_version_pins.py +++ b/tests/unit/test_omit_filter_version_pins.py @@ -1,4 +1,4 @@ -"""A version pin is not a credential. +"""A version pin is not a credential, in any serialization. The env-var rule matched any name *containing* key/token/secret/..., case insensitively, followed by "=". A pip requirement satisfies that: ``tiktoken==0.12.0`` @@ -7,8 +7,15 @@ recorded environment could not be rebuilt, which is the one thing a freeze exists to make possible. -These pin both directions against the real patterns: package pins survive, and actual -environment assignments are still redacted. +0.4.6 fixed the ``name==version`` string form and shipped. It did not fix +``{"name": "version"}`` -- and ``roar`` records packages as ``dict[str, str]`` keyed by +package name, so the published freeze went on carrying ``"tiktoken": "[REDACTED]"``. +The on-host checks all passed, because the installed distribution and the string form +were both genuinely fine; only the serialized record was wrong. A second 3.5-hour run +was spent discovering that. + +So these pin both directions in BOTH serializations: the string form, the JSON form, +and the full record shape as it is actually written. """ from __future__ import annotations @@ -98,3 +105,103 @@ def test_provider_token_is_caught_by_value_even_when_the_name_is_lowercase( result = omit_filter.filter_string(f"hf_token={FAKE_HF_TOKEN}", field="command") assert FAKE_HF_TOKEN not in result.filtered + + +# The shape roar actually serializes: dict[str, str] keyed by package name. +# See roar/core/models/provenance.py -- used_packages, installed_packages, packages. +def test_serialized_package_map_survives(omit_filter: OmitFilter) -> None: + import json + + packages = { + "requests": "2.34.2", + "tiktoken": "0.11.0", + "authlib": "1.3.2", + "keyring": "25.4.1", + "tokenizers": "0.20.3", + "secretstorage": "3.3.3", + "torch": "2.9.1", + } + blob = json.dumps({"pip": packages, "used_packages": packages}) + + result = omit_filter.filter_string(blob, field="freeze") + + assert "[REDACTED]" not in result.filtered + assert json.loads(result.filtered)["pip"] == packages + + +JSON_SECRETS = [ + pytest.param(f'{{"HF_TOKEN": "{FAKE_HF_TOKEN}"}}', id="uppercase-env"), + pytest.param(f'{{"api_key": "{FAKE_OPENAI_KEY}"}}', id="delimited-lowercase"), + pytest.param(f'{{"accessToken": "{FAKE_HF_TOKEN}"}}', id="camelcase"), + pytest.param('{"MYTOKEN": "abc123def456ghi"}', id="unprefixed-uppercase"), + pytest.param('{"password": "hunter2hunter2"}', id="bare-keyword"), +] + + +@pytest.mark.parametrize("blob", JSON_SECRETS) +def test_json_named_secret_is_still_redacted(omit_filter: OmitFilter, blob: str) -> None: + # Narrowing the name pattern must not cost the case the rule exists for. A + # credential in a serialized environment is the thing being protected. + result = omit_filter.filter_string(blob, field="runtime") + + assert "[REDACTED]" in result.filtered + assert blob.rsplit('": "', 1)[1].rstrip('"}') not in result.filtered + + +def test_lowercase_delimited_assignment_is_redacted(omit_filter: OmitFilter) -> None: + # Strictly better than 0.4.6, which dropped IGNORECASE wholesale and so stopped + # matching lowercase names entirely. A delimiter distinguishes a credential name + # from a package name without giving up on lowercase. + result = omit_filter.filter_string(f"api_key={FAKE_OPENAI_KEY}", field="command") + + assert result.filtered == "api_key=[REDACTED]" + + +def test_a_real_record_keeps_its_versions_and_loses_its_secrets(omit_filter: OmitFilter) -> None: + """The true positive and the false positive in one artifact. + + A filter that stopped redacting would pass every "package survives" case above + and be catastrophically wrong. This asserts both halves of the same record: the + dependency versions come through intact, and a credential sitting beside them in + the captured environment does not. + """ + import json + + record = { + "pip": { + "requests": "2.34.2", + "tiktoken": "0.11.0", + "authlib": "1.3.2", + "keyring": "25.4.1", + "tokenizers": "0.20.3", + "secretstorage": "3.3.3", + "torch": "2.9.1", + }, + "runtime": { + "env_vars": { + "HF_TOKEN": FAKE_HF_TOKEN, + "OPENAI_API_KEY": FAKE_OPENAI_KEY, + "api_key": "sk-" + "lowercaseDelimited123", + "accessToken": "camel" + "CaseSecret456", + "PATH": "/usr/local/bin:/usr/bin", + }, + "command": f"env HF_TOKEN={FAKE_HF_TOKEN} python -m scripts.train --depth=14", + }, + } + + result = omit_filter.filter_string(json.dumps(record), field="freeze") + out = json.loads(result.filtered) + + # Every version intact -- the freeze must remain installable. + assert out["pip"] == record["pip"] + + # Every credential gone, by name shape and by value. + for name in ("HF_TOKEN", "OPENAI_API_KEY", "api_key", "accessToken"): + assert out["runtime"]["env_vars"][name] == "[REDACTED]", name + for secret in (FAKE_HF_TOKEN, FAKE_OPENAI_KEY): + assert secret not in result.filtered + + # Innocent environment survives, including an unrelated "=" in the command. + assert out["runtime"]["env_vars"]["PATH"] == "/usr/local/bin:/usr/bin" + assert "--depth=14" in out["runtime"]["command"] + assert "HF_TOKEN=[REDACTED]" in out["runtime"]["command"] From 5e2e17a901d8ddcfaee6f8b052b09b2671d5cf45 Mon Sep 17 00:00:00 2001 From: Chris Geyer Date: Thu, 10 Sep 2026 11:34:25 -0400 Subject: [PATCH 2/2] Tell a version pin from a credential by value and word boundary, not case (#301) * Revert the two attempts to fix this by narrowing the credential-name pattern Both fixes treated "a package version was redacted" as a problem with which NAMES look like credentials, and both narrowed the pattern to exclude package names. Both broke redaction doing it, measured over a 325-case corpus: 0.4.6 dropped IGNORECASE wholesale, so api_key=, hf_token= and authtoken= stopped being redacted at all -- a leak 0.4.5 caught. It also fixed only the "name==version" string form, leaving {"name": "version"} broken, which is the form the record actually serializes. 0.4.7 replaced that with three casing branches (UPPERCASE / delimited-lowercase / camelCase). 42 credential names that 0.4.5 redacted stopped being redacted: Hf_Token, MyToken, Api_Key, APIKey, Github_Token, TOKEN_my, secretValue and the bare Token / Key / Secret / Password / Credential forms. Adding "-" to the delimiter set to catch api-key simultaneously pulled in the *-auth package family, so google-auth, google-auth-oauthlib, dj-rest-auth and social-auth-core had their versions redacted in the serialized form -- the original bug again, in the same serialization, introduced by its own fix. Casing was never the discriminator. This restores the 0.4.5 pattern so the real one can be applied on top of a clean base rather than layered over two abandoned designs. The tests both commits added are kept: they assert the right behavior, they were just enforcing it through the wrong mechanism. * Tell a version pin from a credential by the value and the word boundary, not the case Two guards replace the casing branches the previous attempts relied on: word boundary the keyword must be its own word within the name -- bounded by a delimiter, a case transition, or an edge. "api_key" and "MyToken" qualify; "tiktoken" and "tokenizer" are single words that merely contain one. This is what keeps --tokenizer=gpt2 executable in a recorded command. version guard a PEP 440 version is never a credential. This is what spares google-auth, dj-rest-auth and social-auth-core, whose names DO carry a delimited keyword and which no name-only pattern can get right. Because both guards work on structure rather than case, the pattern no longer excludes any casing, so every credential name 0.4.5 redacted is redacted again -- the 42 that 0.4.7 dropped (Hf_Token, MyToken, Api_Key, APIKey, TOKEN_my, secretValue, bare Token / Key / Secret / Password / Credential) and the lowercase ones 0.4.6 dropped (api_key=, hf_token=, authtoken=). Measured over a 325-case corpus, against both prior baselines: vs 0.4.5 vs 0.4.7 regressions 0 0 new false positives 0 0 false positives fixed 25 3 credentials newly caught 3 43 Tests consolidated from 208 lines / 22 cases to 133 / 76, now asserting every case in all four serializations the record is written in. They fail against each prior version: 0.4.5 (44), 0.4.6 (46), 0.4.7 (37). Unchanged and tracked separately: URL credential rules still enumerate schemes, so postgresql://, mongodb+srv:// and rediss:// leak; bare-userinfo form leaks for every scheme but https/ssh/git; provider shapes cover only hf_/ghp_/sk-/AKIA; and redacted URIs are not parseable because the marker's brackets read as an IPv6 literal. * style: ruff format and fromkeys fixup --------- Co-authored-by: Chris Geyer --- roar/filters/omit.py | 96 ++++--- tests/unit/test_omit_filter_version_pins.py | 292 +++++++++----------- 2 files changed, 176 insertions(+), 212 deletions(-) diff --git a/roar/filters/omit.py b/roar/filters/omit.py index a35ae59f..d4eeb7a8 100644 --- a/roar/filters/omit.py +++ b/roar/filters/omit.py @@ -38,32 +38,53 @@ def was_modified(self) -> bool: return len(self.detections) > 0 -# Built-in patterns for common secret formats -# A name that DENOTES a credential, as opposed to a name 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, which -# made the recorded environment uninstallable -- and cost a completed 3.5-hour -# training run its reproducibility gate twice, because the first fix covered only the -# "name==version" string form and not the {"name": "version"} form the record -# actually serializes. +# 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. # -# Three shapes denote a credential, and none of them matches a package name: +# 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: # -# UPPERCASE HF_TOKEN, API_KEY, MYTOKEN -- env vars, POSIX convention -# delimited api_key, access-token, token -- the keyword is its own word -# camelCase apiKey, accessToken -- the capital is the delimiter +# 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. # -# "tiktoken" fails all three: it is lowercase, "token" is not delimited within it, -# and there is no capital. Same for authlib, keyring, tokenizers, secretstorage. +# 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" - r"|[A-Z0-9_]*(?:KEY|TOKEN|SECRET|PASSWORD|PASSWD|PWD|CREDENTIAL|AUTH)[A-Z0-9_]*" - r"|(?:[a-z0-9]+[_-])*(?:key|token|secret|password|passwd|pwd|credential|auth)" - r"(?:[_-][a-z0-9]+)*" - r"|[a-z0-9]+(?:Key|Token|Secret|Password|Passwd|Pwd|Credential|Auth)[A-Za-z0-9]*" + # 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]] = [ # AWS credentials @@ -186,41 +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(rf"({_SECRET_NAME})" 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 +@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 -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" +@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 - 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_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_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 +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 -# The shape roar actually serializes: dict[str, str] keyed by package name. -# See roar/core/models/provenance.py -- used_packages, installed_packages, packages. -def test_serialized_package_map_survives(omit_filter: OmitFilter) -> None: - import json - - packages = { - "requests": "2.34.2", - "tiktoken": "0.11.0", - "authlib": "1.3.2", - "keyring": "25.4.1", - "tokenizers": "0.20.3", - "secretstorage": "3.3.3", - "torch": "2.9.1", - } - blob = json.dumps({"pip": packages, "used_packages": packages}) - - result = omit_filter.filter_string(blob, field="freeze") +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. - assert "[REDACTED]" not in result.filtered - assert json.loads(result.filtered)["pip"] == packages - - -JSON_SECRETS = [ - pytest.param(f'{{"HF_TOKEN": "{FAKE_HF_TOKEN}"}}', id="uppercase-env"), - pytest.param(f'{{"api_key": "{FAKE_OPENAI_KEY}"}}', id="delimited-lowercase"), - pytest.param(f'{{"accessToken": "{FAKE_HF_TOKEN}"}}', id="camelcase"), - pytest.param('{"MYTOKEN": "abc123def456ghi"}', id="unprefixed-uppercase"), - pytest.param('{"password": "hunter2hunter2"}', id="bare-keyword"), -] - - -@pytest.mark.parametrize("blob", JSON_SECRETS) -def test_json_named_secret_is_still_redacted(omit_filter: OmitFilter, blob: str) -> None: - # Narrowing the name pattern must not cost the case the rule exists for. A - # credential in a serialized environment is the thing being protected. - result = omit_filter.filter_string(blob, field="runtime") - - assert "[REDACTED]" in result.filtered - assert blob.rsplit('": "', 1)[1].rstrip('"}') not in result.filtered - - -def test_lowercase_delimited_assignment_is_redacted(omit_filter: OmitFilter) -> None: - # Strictly better than 0.4.6, which dropped IGNORECASE wholesale and so stopped - # matching lowercase names entirely. A delimiter distinguishes a credential name - # from a package name without giving up on lowercase. - result = omit_filter.filter_string(f"api_key={FAKE_OPENAI_KEY}", field="command") - - assert result.filtered == "api_key=[REDACTED]" - - -def test_a_real_record_keeps_its_versions_and_loses_its_secrets(omit_filter: OmitFilter) -> None: - """The true positive and the false positive in one artifact. - - A filter that stopped redacting would pass every "package survives" case above - and be catastrophically wrong. This asserts both halves of the same record: the - dependency versions come through intact, and a credential sitting beside them in - the captured environment does not. + 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. """ - import json - + packages = dict.fromkeys(PACKAGES, "1.2.3") record = { - "pip": { - "requests": "2.34.2", - "tiktoken": "0.11.0", - "authlib": "1.3.2", - "keyring": "25.4.1", - "tokenizers": "0.20.3", - "secretstorage": "3.3.3", - "torch": "2.9.1", - }, + "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": FAKE_HF_TOKEN, - "OPENAI_API_KEY": FAKE_OPENAI_KEY, - "api_key": "sk-" + "lowercaseDelimited123", - "accessToken": "camel" + "CaseSecret456", - "PATH": "/usr/local/bin:/usr/bin", - }, - "command": f"env HF_TOKEN={FAKE_HF_TOKEN} python -m scripts.train --depth=14", + "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"}, } - result = omit_filter.filter_string(json.dumps(record), field="freeze") - out = json.loads(result.filtered) + out, _ = f.filter_metadata(json.loads(json.dumps(record))) + blob = json.dumps(out) - # Every version intact -- the freeze must remain installable. - assert out["pip"] == record["pip"] + # 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 shape and by value. - for name in ("HF_TOKEN", "OPENAI_API_KEY", "api_key", "accessToken"): - assert out["runtime"]["env_vars"][name] == "[REDACTED]", name - for secret in (FAKE_HF_TOKEN, FAKE_OPENAI_KEY): - assert secret not in result.filtered + # 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 an unrelated "=" in the command. - assert out["runtime"]["env_vars"]["PATH"] == "/usr/local/bin:/usr/bin" + # 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 "HF_TOKEN=[REDACTED]" in out["runtime"]["command"] + assert "--tokenizer=gpt2" in out["runtime"]["command"]