From acd7716eff55fa72feb7748c5afe1ff56fb40382 Mon Sep 17 00:00:00 2001 From: Chris Geyer Date: Thu, 10 Sep 2026 12:57:54 +0000 Subject: [PATCH 1/3] 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) --- pyproject.toml | 2 +- roar/filters/omit.py | 33 +++++++++-- tests/unit/test_omit_filter_version_pins.py | 63 ++++++++++++++++++++- 3 files changed, 88 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..b9ccff73 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,53 @@ 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('{"HF_TOKEN": "%s"}' % FAKE_HF_TOKEN, id="uppercase-env"), + pytest.param('{"api_key": "%s"}' % FAKE_OPENAI_KEY, id="delimited-lowercase"), + pytest.param('{"accessToken": "%s"}' % 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]" From a3b124f821ef136b93534efd37dc5e197a06520f Mon Sep 17 00:00:00 2001 From: Chris Geyer Date: Thu, 10 Sep 2026 12:59:41 +0000 Subject: [PATCH 2/3] 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. --- tests/unit/test_omit_filter_version_pins.py | 50 +++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/unit/test_omit_filter_version_pins.py b/tests/unit/test_omit_filter_version_pins.py index b9ccff73..cd1654fe 100644 --- a/tests/unit/test_omit_filter_version_pins.py +++ b/tests/unit/test_omit_filter_version_pins.py @@ -155,3 +155,53 @@ def test_lowercase_delimited_assignment_is_redacted(omit_filter: OmitFilter) -> 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 238667a119b6658b03ba3bf4fd3bdae8337dc695 Mon Sep 17 00:00:00 2001 From: Chris Geyer Date: Thu, 10 Sep 2026 13:01:42 +0000 Subject: [PATCH 3/3] style: f-strings for the JSON secret fixtures (ruff UP031) --- tests/unit/test_omit_filter_version_pins.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_omit_filter_version_pins.py b/tests/unit/test_omit_filter_version_pins.py index cd1654fe..c16e4ff9 100644 --- a/tests/unit/test_omit_filter_version_pins.py +++ b/tests/unit/test_omit_filter_version_pins.py @@ -130,9 +130,9 @@ def test_serialized_package_map_survives(omit_filter: OmitFilter) -> None: JSON_SECRETS = [ - pytest.param('{"HF_TOKEN": "%s"}' % FAKE_HF_TOKEN, id="uppercase-env"), - pytest.param('{"api_key": "%s"}' % FAKE_OPENAI_KEY, id="delimited-lowercase"), - pytest.param('{"accessToken": "%s"}' % FAKE_HF_TOKEN, id="camelcase"), + 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"), ]