diff --git a/id_configurator/modules/base.py b/id_configurator/modules/base.py index ccfb80a..119b1d7 100644 --- a/id_configurator/modules/base.py +++ b/id_configurator/modules/base.py @@ -37,22 +37,23 @@ def all_modules() -> list[type[Module]]: return list(_REGISTRY) +_IDENTIFIER_FORBIDDEN_CHARS = re.compile(r"[^A-Za-z0-9_-]") + + def sanitize_filename(name: str) -> str: """Make a user-supplied name safe to use as a single filesystem path component. Modules that derive a file's name from configuration data (a certificate's name, a repository's name, ...) must run it through this - first: without it, a name containing `/` (or `\\`, harmless on the - target systems this tool writes to but stripped for good measure) - would silently turn into extra, unintended path segments - creating - nested directories a careless or malicious name shouldn't be able to - create - instead of the single file the caller asked for. + first. Every character that isn't a letter, a digit, `_` or `-` is + replaced with `_`: this rules out `/` and `\\` (which would silently + turn into extra, unintended path segments), `.` (so the result can + never be `.` or `..`), as well as spaces, control characters and any + other character that could confuse the tools reading these files. An + empty name yields `_`, so the result is always a non-empty file name. """ - return name.replace("/", "_").replace("\\", "_") - - -_REPOSITORY_NAME_FORBIDDEN_CHARS = re.compile(r"[^A-Za-z0-9_-]") + return _IDENTIFIER_FORBIDDEN_CHARS.sub("_", name) or "_" def sanitize_repository_name(name: str) -> str: @@ -64,11 +65,7 @@ def sanitize_repository_name(name: str) -> str: header, a TOML table key, ... - where an unrestricted name could break out of that syntax (e.g. `]`, a newline, or - for TOML - a literal `.`, which would silently open extra, nested tables instead of naming - the one table it's meant to). Excluding `.` from the allowed - characters also means the result can never come out as exactly `.` or - `..`, which matter if the name is ever reused as a filesystem path - component (`sanitize_filename` alone wouldn't catch those, since it - only strips `/` and `\\`). Spaces and every other non-identifier + the one table it's meant to). Spaces and every other non-identifier character are replaced the same way, for consistency across callers. """ - return _REPOSITORY_NAME_FORBIDDEN_CHARS.sub("_", name) + return _IDENTIFIER_FORBIDDEN_CHARS.sub("_", name) diff --git a/tests/test_apt.py b/tests/test_apt.py index 4c9a348..0d8c54f 100644 --- a/tests/test_apt.py +++ b/tests/test_apt.py @@ -361,6 +361,6 @@ def test_apply_sanitizes_source_name_with_slash(tmp_path) -> None: AptModule().apply(context) - source_file = dst / "etc/apt/sources.list.d/.._.._evil.sources" + source_file = dst / "etc/apt/sources.list.d/______evil.sources" assert source_file.exists() assert not (dst / "etc/apt/sources.list.d/evil.sources").exists() diff --git a/tests/test_certificates.py b/tests/test_certificates.py index 6f5af38..8186a59 100644 --- a/tests/test_certificates.py +++ b/tests/test_certificates.py @@ -67,7 +67,7 @@ def test_apply_sanitizes_certificate_name_with_slash(tmp_path) -> None: CertificatesModule().apply(context) - local_cert = dst / "usr/local/share/ca-certificates/.._.._evil.crt" + local_cert = dst / "usr/local/share/ca-certificates/______evil.crt" assert local_cert.read_bytes() == b"fake-cert-bytes" # The "/" must never be interpreted as a path separator: without # sanitizing, "../../evil" would have reshuffled the write up to diff --git a/tests/test_modules_base.py b/tests/test_modules_base.py index 6eba11d..2f4b484 100644 --- a/tests/test_modules_base.py +++ b/tests/test_modules_base.py @@ -29,8 +29,25 @@ def test_sanitize_filename_leaves_plain_names_untouched() -> None: def test_sanitize_filename_replaces_forward_slash() -> None: - assert sanitize_filename("../../etc/passwd") == ".._.._etc_passwd" + assert sanitize_filename("../../etc/passwd") == "______etc_passwd" def test_sanitize_filename_replaces_backslash() -> None: assert sanitize_filename(r"a\b") == "a_b" + + +def test_sanitize_filename_replaces_non_identifier_characters() -> None: + assert sanitize_filename("My Root CA (v2).pem\n") == "My_Root_CA__v2__pem_" + + +def test_sanitize_filename_keeps_letters_digits_underscore_and_dash() -> None: + assert sanitize_filename("Inter_Diode-2026") == "Inter_Diode-2026" + + +def test_sanitize_filename_never_returns_dot_names() -> None: + assert sanitize_filename(".") == "_" + assert sanitize_filename("..") == "__" + + +def test_sanitize_filename_never_returns_empty_string() -> None: + assert sanitize_filename("") == "_" diff --git a/tests/test_yum.py b/tests/test_yum.py index 329afae..e699ce4 100644 --- a/tests/test_yum.py +++ b/tests/test_yum.py @@ -327,7 +327,7 @@ def test_apply_sanitizes_key_name_with_slash(tmp_path, monkeypatch) -> None: YumModule().apply(context) - key_content = (dst / "etc/pki/rpm-gpg/RPM-GPG-KEY-.._.._evil-key").read_bytes() + key_content = (dst / "etc/pki/rpm-gpg/RPM-GPG-KEY-______evil-key").read_bytes() assert key_content == b"fake-rpm-gpg-key-bytes" mock_run.assert_called_once()