feat: enhance filename sanitization to replace non-identifier characters

Signed-off-by: d9pouces <github@19pouces.net>
This commit is contained in:
d9pouces
2026-10-24 00:02:40 +02:00
parent 653579b59e
commit 3b6eb7290f
5 changed files with 33 additions and 19 deletions
+12 -15
View File
@@ -37,22 +37,23 @@ def all_modules() -> list[type[Module]]:
return list(_REGISTRY) return list(_REGISTRY)
_IDENTIFIER_FORBIDDEN_CHARS = re.compile(r"[^A-Za-z0-9_-]")
def sanitize_filename(name: str) -> str: def sanitize_filename(name: str) -> str:
"""Make a user-supplied name safe to use as a single filesystem path """Make a user-supplied name safe to use as a single filesystem path
component. component.
Modules that derive a file's name from configuration data (a Modules that derive a file's name from configuration data (a
certificate's name, a repository's name, ...) must run it through this certificate's name, a repository's name, ...) must run it through this
first: without it, a name containing `/` (or `\\`, harmless on the first. Every character that isn't a letter, a digit, `_` or `-` is
target systems this tool writes to but stripped for good measure) replaced with `_`: this rules out `/` and `\\` (which would silently
would silently turn into extra, unintended path segments - creating turn into extra, unintended path segments), `.` (so the result can
nested directories a careless or malicious name shouldn't be able to never be `.` or `..`), as well as spaces, control characters and any
create - instead of the single file the caller asked for. 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("\\", "_") return _IDENTIFIER_FORBIDDEN_CHARS.sub("_", name) or "_"
_REPOSITORY_NAME_FORBIDDEN_CHARS = re.compile(r"[^A-Za-z0-9_-]")
def sanitize_repository_name(name: str) -> str: 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 header, a TOML table key, ... - where an unrestricted name could break
out of that syntax (e.g. `]`, a newline, or - for TOML - a literal out of that syntax (e.g. `]`, a newline, or - for TOML - a literal
`.`, which would silently open extra, nested tables instead of naming `.`, which would silently open extra, nested tables instead of naming
the one table it's meant to). Excluding `.` from the allowed the one table it's meant to). Spaces and every other non-identifier
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
character are replaced the same way, for consistency across callers. character are replaced the same way, for consistency across callers.
""" """
return _REPOSITORY_NAME_FORBIDDEN_CHARS.sub("_", name) return _IDENTIFIER_FORBIDDEN_CHARS.sub("_", name)
+1 -1
View File
@@ -361,6 +361,6 @@ def test_apply_sanitizes_source_name_with_slash(tmp_path) -> None:
AptModule().apply(context) 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 source_file.exists()
assert not (dst / "etc/apt/sources.list.d/evil.sources").exists() assert not (dst / "etc/apt/sources.list.d/evil.sources").exists()
+1 -1
View File
@@ -67,7 +67,7 @@ def test_apply_sanitizes_certificate_name_with_slash(tmp_path) -> None:
CertificatesModule().apply(context) 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" assert local_cert.read_bytes() == b"fake-cert-bytes"
# The "/" must never be interpreted as a path separator: without # The "/" must never be interpreted as a path separator: without
# sanitizing, "../../evil" would have reshuffled the write up to # sanitizing, "../../evil" would have reshuffled the write up to
+18 -1
View File
@@ -29,8 +29,25 @@ def test_sanitize_filename_leaves_plain_names_untouched() -> None:
def test_sanitize_filename_replaces_forward_slash() -> 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: def test_sanitize_filename_replaces_backslash() -> None:
assert sanitize_filename(r"a\b") == "a_b" 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("") == "_"
+1 -1
View File
@@ -327,7 +327,7 @@ def test_apply_sanitizes_key_name_with_slash(tmp_path, monkeypatch) -> None:
YumModule().apply(context) 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" assert key_content == b"fake-rpm-gpg-key-bytes"
mock_run.assert_called_once() mock_run.assert_called_once()