mirror of
https://github.com/rommapp/romm.git
synced 2026-10-12 02:21:58 +00:00
fix(config): reject exclude lists that are not lists
The exclude lists were spread into a set before validation ran, so the isinstance checks could never fail: 'exclude.platforms: ps2' excluded the folders 'p', 's' and '2', and a number crashed startup with a TypeError. Validate the raw YAML values instead, and read only list values. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01XZW9ChvmiTKXYYWY6JvTKU
This commit is contained in:
@@ -303,6 +303,14 @@ PLATFORM_MEDIA_DIRS: Final = {
|
||||
DEFAULT_EXCLUDED_MULTI_FILE_DIRS: Final = sorted(
|
||||
{*DEFAULT_EXCLUDED_PLATFORM_DIRS, *PLATFORM_MEDIA_DIRS.values()}
|
||||
)
|
||||
EXCLUDE_LIST_PATHS: Final = (
|
||||
"exclude.platforms",
|
||||
"exclude.roms.single_file.extensions",
|
||||
"exclude.roms.single_file.names",
|
||||
"exclude.roms.multi_file.names",
|
||||
"exclude.roms.multi_file.parts.extensions",
|
||||
"exclude.roms.multi_file.parts.names",
|
||||
)
|
||||
|
||||
|
||||
class ExclusionType(enum.StrEnum):
|
||||
@@ -679,6 +687,11 @@ class ConfigManager:
|
||||
query=query,
|
||||
)
|
||||
|
||||
def _raw_exclude_list(self, path: str) -> list[str]:
|
||||
"""Read a user exclude list; non-lists are rejected by `_validate_config`."""
|
||||
value = pydash.get(self._raw_config, path)
|
||||
return value if isinstance(value, list) else []
|
||||
|
||||
def _parse_config(self) -> None:
|
||||
"""Parses each entry in the config.yml"""
|
||||
|
||||
@@ -689,7 +702,7 @@ class ConfigManager:
|
||||
EXCLUDED_PLATFORMS=sorted(
|
||||
{
|
||||
*DEFAULT_EXCLUDED_PLATFORM_DIRS,
|
||||
*pydash.get(self._raw_config, "exclude.platforms", []),
|
||||
*self._raw_exclude_list("exclude.platforms"),
|
||||
}
|
||||
),
|
||||
EXCLUDED_SINGLE_EXT=sorted(
|
||||
@@ -697,10 +710,8 @@ class ConfigManager:
|
||||
*(e.lower() for e in DEFAULT_EXCLUDED_EXTENSIONS),
|
||||
*(
|
||||
e.lower()
|
||||
for e in pydash.get(
|
||||
self._raw_config,
|
||||
"exclude.roms.single_file.extensions",
|
||||
[],
|
||||
for e in self._raw_exclude_list(
|
||||
"exclude.roms.single_file.extensions"
|
||||
)
|
||||
),
|
||||
}
|
||||
@@ -708,21 +719,13 @@ class ConfigManager:
|
||||
EXCLUDED_SINGLE_FILES=sorted(
|
||||
{
|
||||
*DEFAULT_EXCLUDED_FILES,
|
||||
*pydash.get(
|
||||
self._raw_config,
|
||||
"exclude.roms.single_file.names",
|
||||
[],
|
||||
),
|
||||
*self._raw_exclude_list("exclude.roms.single_file.names"),
|
||||
}
|
||||
),
|
||||
EXCLUDED_MULTI_FILES=sorted(
|
||||
{
|
||||
*DEFAULT_EXCLUDED_MULTI_FILE_DIRS,
|
||||
*pydash.get(
|
||||
self._raw_config,
|
||||
"exclude.roms.multi_file.names",
|
||||
[],
|
||||
),
|
||||
*self._raw_exclude_list("exclude.roms.multi_file.names"),
|
||||
}
|
||||
),
|
||||
EXCLUDED_MULTI_PARTS_EXT=sorted(
|
||||
@@ -730,10 +733,8 @@ class ConfigManager:
|
||||
*(e.lower() for e in DEFAULT_EXCLUDED_EXTENSIONS),
|
||||
*(
|
||||
e.lower()
|
||||
for e in pydash.get(
|
||||
self._raw_config,
|
||||
"exclude.roms.multi_file.parts.extensions",
|
||||
[],
|
||||
for e in self._raw_exclude_list(
|
||||
"exclude.roms.multi_file.parts.extensions"
|
||||
)
|
||||
),
|
||||
}
|
||||
@@ -741,11 +742,7 @@ class ConfigManager:
|
||||
EXCLUDED_MULTI_PARTS_FILES=sorted(
|
||||
{
|
||||
*DEFAULT_EXCLUDED_FILES,
|
||||
*pydash.get(
|
||||
self._raw_config,
|
||||
"exclude.roms.multi_file.parts.names",
|
||||
[],
|
||||
),
|
||||
*self._raw_exclude_list("exclude.roms.multi_file.parts.names"),
|
||||
}
|
||||
),
|
||||
PLATFORMS_BINDING=pydash.get(self._raw_config, "system.platforms", {})
|
||||
@@ -1004,39 +1001,11 @@ class ConfigManager:
|
||||
"""Validates the config.yml file"""
|
||||
self._check_retired_filesystem_keys()
|
||||
|
||||
if not isinstance(self.config.EXCLUDED_PLATFORMS, list):
|
||||
log.critical("Invalid config.yml: exclude.platforms must be a list")
|
||||
sys.exit(3)
|
||||
|
||||
if not isinstance(self.config.EXCLUDED_SINGLE_EXT, list):
|
||||
log.critical(
|
||||
"Invalid config.yml: exclude.roms.single_file.extensions must be a list"
|
||||
)
|
||||
sys.exit(3)
|
||||
|
||||
if not isinstance(self.config.EXCLUDED_SINGLE_FILES, list):
|
||||
log.critical(
|
||||
"Invalid config.yml: exclude.roms.single_file.names must be a list"
|
||||
)
|
||||
sys.exit(3)
|
||||
|
||||
if not isinstance(self.config.EXCLUDED_MULTI_FILES, list):
|
||||
log.critical(
|
||||
"Invalid config.yml: exclude.roms.multi_file.names must be a list"
|
||||
)
|
||||
sys.exit(3)
|
||||
|
||||
if not isinstance(self.config.EXCLUDED_MULTI_PARTS_EXT, list):
|
||||
log.critical(
|
||||
"Invalid config.yml: exclude.roms.multi_file.parts.extensions must be a list"
|
||||
)
|
||||
sys.exit(3)
|
||||
|
||||
if not isinstance(self.config.EXCLUDED_MULTI_PARTS_FILES, list):
|
||||
log.critical(
|
||||
"Invalid config.yml: exclude.roms.multi_file.parts.names must be a list"
|
||||
)
|
||||
sys.exit(3)
|
||||
for path in EXCLUDE_LIST_PATHS:
|
||||
value = pydash.get(self._raw_config, path)
|
||||
if value is not None and not isinstance(value, list):
|
||||
log.critical(f"Invalid config.yml: {path} must be a list")
|
||||
sys.exit(3)
|
||||
|
||||
if not isinstance(self.config.GAMELIST_AUTO_EXPORT_ON_SCAN, bool):
|
||||
log.critical("Invalid config.yml: scan.gamelist.export must be a boolean")
|
||||
|
||||
@@ -10,6 +10,7 @@ from config.config_manager import (
|
||||
DEFAULT_EXCLUDED_FILES,
|
||||
DEFAULT_EXCLUDED_MULTI_FILE_DIRS,
|
||||
DEFAULT_EXCLUDED_PLATFORM_DIRS,
|
||||
EXCLUDE_LIST_PATHS,
|
||||
ConfigManager,
|
||||
parse_firmware_template,
|
||||
parse_platform_templates,
|
||||
@@ -704,6 +705,32 @@ def critical(mocker):
|
||||
return lambda: str(spy.call_args[0][0])
|
||||
|
||||
|
||||
def _write_exclude_config(tmp_path: Path, path: str, value: str) -> ConfigManager:
|
||||
"""Write `value` at the dotted `path`, nesting one YAML level per segment."""
|
||||
lines = [f"{' ' * depth}{key}:" for depth, key in enumerate(path.split("."))]
|
||||
lines[-1] += f" {value}"
|
||||
config_file = tmp_path / "config.yml"
|
||||
config_file.write_text("\n".join(lines) + "\n")
|
||||
return ConfigManager(str(config_file))
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", EXCLUDE_LIST_PATHS)
|
||||
@pytest.mark.parametrize("value", ["ps2", "5", "{ps2: true}"])
|
||||
def test_exclude_list_rejects_a_non_list(tmp_path, critical, path, value):
|
||||
"""A bare string would otherwise be split into one exclusion per character."""
|
||||
with pytest.raises(SystemExit) as excinfo:
|
||||
_write_exclude_config(tmp_path, path, value)
|
||||
|
||||
assert excinfo.value.code == 3
|
||||
assert critical() == f"Invalid config.yml: {path} must be a list"
|
||||
|
||||
|
||||
def test_empty_exclude_list_keeps_the_defaults(tmp_path):
|
||||
loader = _write_exclude_config(tmp_path, "exclude.platforms", "")
|
||||
|
||||
assert loader.config.EXCLUDED_PLATFORMS == sorted(DEFAULT_EXCLUDED_PLATFORM_DIRS)
|
||||
|
||||
|
||||
def _write_filesystem_config(tmp_path: Path, block: str) -> ConfigManager:
|
||||
config_file = tmp_path / "config.yml"
|
||||
config_file.write_text(f"filesystem:\n{block}")
|
||||
|
||||
Reference in New Issue
Block a user