From dbdc523bff2be90e157e5a2d8d203f074afac11a Mon Sep 17 00:00:00 2001 From: timlichtenberg Date: Tue, 15 Sep 2026 21:24:37 +0200 Subject: [PATCH 1/3] Skip the label line for a name that filters to nothing A dataset name made only of invisible characters passed the manifest's non-empty check but still tried to print a label, leaving a bare indented line in `fwl-io list` output. Now the name is filtered and stripped once, and the label only prints when that leaves something. Also broadened the control-character test to cover tab, carriage return, and the Unicode line separator, not just newline, and added a case that distinguishes the strip from plain non-empty checks. --- src/fwl_io/cli.py | 5 +++-- tests/test_cli.py | 48 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/fwl_io/cli.py b/src/fwl_io/cli.py index de143c7..96af6f2 100644 --- a/src/fwl_io/cli.py +++ b/src/fwl_io/cli.py @@ -39,8 +39,9 @@ def _cmd_list(args: argparse.Namespace) -> int: if ds.name != ds.key: # A name is a single-line label; drop control characters so a # manifest cannot inject extra lines into the listing. - label = ''.join(c for c in ds.name if c.isprintable()) - print(f' {label}') + label = ''.join(c for c in ds.name if c.isprintable()).strip() + if label: + print(f' {label}') for provider, message in sorted(errors.items()): print(f'[{provider}] FAILED TO LOAD: {message}', file=sys.stderr) return 1 if errors else 0 diff --git a/tests/test_cli.py b/tests/test_cli.py index 2e9fe49..6b68b22 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -116,12 +116,17 @@ def load(self): @pytest.mark.unit -def test_list_strips_control_characters_from_a_declared_name(tmp_path, capsys, monkeypatch): - """A name carrying a newline cannot inject an extra line into the listing.""" +@pytest.mark.parametrize( + 'escape', + ['\\n', '\\t', '\\r', '\\u2028'], + ids=['newline', 'tab', 'cr', 'line-separator'], +) +def test_list_strips_control_characters_from_a_declared_name(escape, tmp_path, capsys, monkeypatch): + """A name carrying a control character cannot inject an extra line into the listing.""" manifest = tmp_path / 'manifest.toml' manifest.write_text( '[evil]\n' - 'name = "harmless\\n forged.key required_by: victim [NO REGISTRY]"\n' + f'name = "harmless{escape} forged.key required_by: victim [NO REGISTRY]"\n' 'zenodo = "10.5281/zenodo.1"\n' ) @@ -151,6 +156,43 @@ def load(self): assert not any(line.startswith(' forged') for line in lines) +@pytest.mark.unit +@pytest.mark.parametrize( + 'toml_escaped_name', + ['\\u200b\\u200b', '\\u0001 \\u0001'], + ids=['all-non-printable', 'non-printable-padding-around-spaces'], +) +def test_list_omits_the_label_line_for_an_all_non_printable_name( + toml_escaped_name, tmp_path, capsys, monkeypatch +): + """A name that strips to nothing after filtering must not print a bare indented line.""" + manifest = tmp_path / 'manifest.toml' + manifest.write_text( + f'[evil]\nname = "{toml_escaped_name}"\nzenodo = "10.5281/zenodo.1"\n' + ) + + class _EP: + def __init__(self, name, target): + self.name = name + self._target = target + + def load(self): + return self._target + + monkeypatch.setattr( + 'fwl_io.manifest.entry_points', + lambda group: [_EP('demo', lambda: manifest)], + ) + + code = main(['list']) + out = capsys.readouterr().out + lines = out.splitlines() + assert code == 0 + assert len(lines) == 2 + assert lines[0] == '[demo]' + assert lines[1].startswith(' evil') + + @pytest.mark.unit def test_fetch_unknown_module_exits_nonzero(capsys, monkeypatch): """Asking for a model no manifest declares is an error, not an empty success.""" From a15661af243e58288d4e9d3e5165fe630c9bdc40 Mon Sep 17 00:00:00 2001 From: timlichtenberg Date: Tue, 15 Sep 2026 22:10:42 +0200 Subject: [PATCH 2/3] Skip the label line when it collapses to the key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A name that differs from the key only by trailing whitespace or an invisible character, like "evil " or "evil​" against the key "evil", still triggered the label print because the check ran against the raw name. After filtering it prints the same text as the key line above it, so list output showed the key twice. Now I filter and strip the name once and gate on that value against the key, so the check and the printed text always agree. Extended the tests with the trailing-space and trailing-invisible-character cases that reach this path. --- src/fwl_io/cli.py | 12 ++++++------ tests/test_cli.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/fwl_io/cli.py b/src/fwl_io/cli.py index 96af6f2..2ff78b5 100644 --- a/src/fwl_io/cli.py +++ b/src/fwl_io/cli.py @@ -36,12 +36,12 @@ def _cmd_list(args: argparse.Namespace) -> int: '' if ds.registry_path and ds.registry_path.is_file() else ' [NO REGISTRY]' ) print(f' {ds.key:50s} required_by: {consumers}{registry_note}') - if ds.name != ds.key: - # A name is a single-line label; drop control characters so a - # manifest cannot inject extra lines into the listing. - label = ''.join(c for c in ds.name if c.isprintable()).strip() - if label: - print(f' {label}') + # A name is a single-line label; drop control characters so a manifest + # cannot inject extra lines, then skip the line when there is nothing + # left to show or it collapses to the key already printed above. + label = ''.join(c for c in ds.name if c.isprintable()).strip() + if label and label != ds.key: + print(f' {label}') for provider, message in sorted(errors.items()): print(f'[{provider}] FAILED TO LOAD: {message}', file=sys.stderr) return 1 if errors else 0 diff --git a/tests/test_cli.py b/tests/test_cli.py index 6b68b22..f3aedd7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -193,6 +193,43 @@ def load(self): assert lines[1].startswith(' evil') +@pytest.mark.unit +@pytest.mark.parametrize( + 'toml_escaped_name', + ['evil ', 'evil\\u200b'], + ids=['trailing-space', 'trailing-invisible-char'], +) +def test_list_omits_the_label_line_when_it_collapses_to_the_key( + toml_escaped_name, tmp_path, capsys, monkeypatch +): + """A name that differs from the key raw but not after filtering must not repeat it.""" + manifest = tmp_path / 'manifest.toml' + manifest.write_text( + f'[evil]\nname = "{toml_escaped_name}"\nzenodo = "10.5281/zenodo.1"\n' + ) + + class _EP: + def __init__(self, name, target): + self.name = name + self._target = target + + def load(self): + return self._target + + monkeypatch.setattr( + 'fwl_io.manifest.entry_points', + lambda group: [_EP('demo', lambda: manifest)], + ) + + code = main(['list']) + out = capsys.readouterr().out + lines = out.splitlines() + assert code == 0 + assert len(lines) == 2 + assert lines[0] == '[demo]' + assert lines[1].startswith(' evil') + + @pytest.mark.unit def test_fetch_unknown_module_exits_nonzero(capsys, monkeypatch): """Asking for a model no manifest declares is an error, not an empty success.""" From 370c47a53674241f6d084a6b9bba2acb750a61f0 Mon Sep 17 00:00:00 2001 From: timlichtenberg Date: Tue, 15 Sep 2026 22:15:32 +0200 Subject: [PATCH 3/3] Reformat test_cli.py to match ruff's line-length rule Two manifest.write_text calls wrapped across lines that ruff format collapses to a single line once it fits under the limit. Running ruff format brings the file back in line so lint stays green. --- tests/test_cli.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index f3aedd7..4d6c134 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -167,9 +167,7 @@ def test_list_omits_the_label_line_for_an_all_non_printable_name( ): """A name that strips to nothing after filtering must not print a bare indented line.""" manifest = tmp_path / 'manifest.toml' - manifest.write_text( - f'[evil]\nname = "{toml_escaped_name}"\nzenodo = "10.5281/zenodo.1"\n' - ) + manifest.write_text(f'[evil]\nname = "{toml_escaped_name}"\nzenodo = "10.5281/zenodo.1"\n') class _EP: def __init__(self, name, target): @@ -204,9 +202,7 @@ def test_list_omits_the_label_line_when_it_collapses_to_the_key( ): """A name that differs from the key raw but not after filtering must not repeat it.""" manifest = tmp_path / 'manifest.toml' - manifest.write_text( - f'[evil]\nname = "{toml_escaped_name}"\nzenodo = "10.5281/zenodo.1"\n' - ) + manifest.write_text(f'[evil]\nname = "{toml_escaped_name}"\nzenodo = "10.5281/zenodo.1"\n') class _EP: def __init__(self, name, target):