From c56c5bdb4716f6e1c6d7d239ce4a711ab61672ea Mon Sep 17 00:00:00 2001 From: txltedXgod Date: Mon, 9 Nov 2026 07:15:16 +0100 Subject: [PATCH] fix(config): validate 're:' match patterns and report exact paths (closes #118) --- src/profiles/core/config/matcher.py | 5 ++- src/profiles/core/config/schema.py | 12 +++++++ src/profiles/core/config/validator.py | 34 +++++++++++++++++-- tests/core/config/test_diagnostics.py | 13 ++++++++ tests/core/config/test_matcher.py | 1 + tests/core/config/test_schema.py | 27 +++++++++++++++ tests/core/config/test_validator.py | 47 +++++++++++++++++++++++++++ 7 files changed, 135 insertions(+), 4 deletions(-) diff --git a/src/profiles/core/config/matcher.py b/src/profiles/core/config/matcher.py index ad095b1..a8450b2 100644 --- a/src/profiles/core/config/matcher.py +++ b/src/profiles/core/config/matcher.py @@ -31,7 +31,10 @@ def match_pattern(pattern: str, value: str, is_path: bool = False) -> bool: if pattern.startswith("re:"): regex = pattern[3:] target = _normalize_path(value) if is_path else value - return bool(re.search(regex, target, re.IGNORECASE)) + try: + return bool(re.search(regex, target, re.IGNORECASE)) + except re.error: + return False if is_path: norm_pattern = _normalize_path(pattern) diff --git a/src/profiles/core/config/schema.py b/src/profiles/core/config/schema.py index 1c330cf..40b87d4 100644 --- a/src/profiles/core/config/schema.py +++ b/src/profiles/core/config/schema.py @@ -174,6 +174,18 @@ def _coerce_list(cls, value: object) -> list[str]: return [str(item) for item in value] return [] + @field_validator("hostname", "ip", "path") + @classmethod + def _validate_patterns(cls, patterns: list[str]) -> list[str]: + for pattern in patterns: + if pattern.startswith("re:"): + regex = pattern[3:] + try: + re.compile(regex) + except re.error as exc: + raise ValueError(f"invalid regex pattern '{pattern}': {exc}") from exc + return patterns + class MachineConfig(BaseModel): """A named configuration block in YAML configs dict. diff --git a/src/profiles/core/config/validator.py b/src/profiles/core/config/validator.py index bda5cb3..09ed776 100644 --- a/src/profiles/core/config/validator.py +++ b/src/profiles/core/config/validator.py @@ -8,6 +8,8 @@ from __future__ import annotations +import re + from profiles.core.config.schema import ConfigError _KNOWN_TOP_LEVEL = {"version", "defaults", "columns", "hooks", "configs"} @@ -47,9 +49,7 @@ def collect_issues(raw: dict) -> list[ConfigError]: if not isinstance(cfg, dict): continue extends = cfg.get("extends") - if extends is None: - continue - if not isinstance(extends, str) or extends not in configs: + if extends is not None and (not isinstance(extends, str) or extends not in configs): errors.append( ConfigError( f"configs.{name}.extends", @@ -57,6 +57,34 @@ def collect_issues(raw: dict) -> list[ConfigError]: ) ) + match = cfg.get("match") + if isinstance(match, dict): + for field in ("hostname", "ip", "path"): + raw_val = match.get(field) + if isinstance(raw_val, str): + if raw_val.startswith("re:"): + try: + re.compile(raw_val[3:]) + except re.error as exc: + errors.append( + ConfigError( + f"configs.{name}.match.{field}", + f"invalid regex pattern '{raw_val}': {exc}", + ) + ) + elif isinstance(raw_val, list): + for idx, item in enumerate(raw_val): + if isinstance(item, str) and item.startswith("re:"): + try: + re.compile(item[3:]) + except re.error as exc: + errors.append( + ConfigError( + f"configs.{name}.match.{field}[{idx}]", + f"invalid regex pattern '{item}': {exc}", + ) + ) + # Cycle detection via DFS; each cycle met along the way is reported. visiting: set[str] = set() visited: set[str] = set() diff --git a/tests/core/config/test_diagnostics.py b/tests/core/config/test_diagnostics.py index 630fd8c..10c96f3 100644 --- a/tests/core/config/test_diagnostics.py +++ b/tests/core/config/test_diagnostics.py @@ -82,3 +82,16 @@ def test_non_mapping_root_reports_error() -> None: def test_non_hashable_extends_reports_error_without_crash() -> None: issues = collect_config_issues("configs: {a: {extends: [b]}}\n") assert any("configs.a.extends" in i["message"] for i in issues) + + +def test_invalid_regex_pattern_reports_issue() -> None: + yaml_text = """ +version: 1 +configs: + prod: + match: + hostname: ["re:([unclosed"] +""" + issues = collect_config_issues(yaml_text) + assert any("configs.prod.match.hostname" in i["message"] for i in issues) + assert any("invalid regex pattern" in i["message"] for i in issues) diff --git a/tests/core/config/test_matcher.py b/tests/core/config/test_matcher.py index ca67525..beb3760 100644 --- a/tests/core/config/test_matcher.py +++ b/tests/core/config/test_matcher.py @@ -10,6 +10,7 @@ def test_match_pattern_glob_and_regex(): assert match_pattern("WORKSTATION-*", "workstation-1") assert match_pattern("re:^192\\.168\\.\\d+\\.\\d+$", "192.168.1.50") assert not match_pattern("WORKSTATION-*", "SERVER-1") + assert not match_pattern("re:([unclosed", "workstation-1") def test_matches_machine_config_or_logic(): diff --git a/tests/core/config/test_schema.py b/tests/core/config/test_schema.py index 1e3c9b6..6ddfdda 100644 --- a/tests/core/config/test_schema.py +++ b/tests/core/config/test_schema.py @@ -72,3 +72,30 @@ def test_config_name_is_dict_key(self) -> None: ) assert "prod" in cfg.configs assert cfg.configs["prod"].match.hostname == ["PC1"] + + +class TestMatchCriteriaSchema: + def test_valid_glob_and_regex(self) -> None: + schema = MatchCriteriaSchema( + hostname=["WORKSTATION-*", "re:^station-\\d+$"], + ip=["192.168.1.*"], + path=["re:^/data/.*"], + ) + assert len(schema.hostname) == 2 + assert schema.ip == ["192.168.1.*"] + assert schema.path == ["re:^/data/.*"] + + def test_invalid_regex_raises_validation_error(self) -> None: + with pytest.raises(ValidationError) as exc: + MatchCriteriaSchema(hostname=["re:([unclosed"]) + assert "invalid regex pattern" in str(exc.value) + assert "re:([unclosed" in str(exc.value) + + def test_invalid_regex_in_ip_and_path_raises(self) -> None: + with pytest.raises(ValidationError) as exc_ip: + MatchCriteriaSchema(ip=["re:*bad"]) + assert "invalid regex pattern" in str(exc_ip.value) + + with pytest.raises(ValidationError) as exc_path: + MatchCriteriaSchema(path=["re:[unclosed"]) + assert "invalid regex pattern" in str(exc_path.value) diff --git a/tests/core/config/test_validator.py b/tests/core/config/test_validator.py index 98d4dd4..87c6649 100644 --- a/tests/core/config/test_validator.py +++ b/tests/core/config/test_validator.py @@ -94,3 +94,50 @@ def test_non_string_extends(self) -> None: raw = {"configs": {"a": {"extends": ["b"]}}} errors = collect_issues(raw) assert any(e.path == "configs.a.extends" for e in errors) + + def test_invalid_regex_match_pattern_collected(self) -> None: + raw = { + "configs": { + "prod": { + "match": { + "hostname": ["re:([unclosed"], + "ip": ["192.168.1.1"], + } + } + } + } + errors = collect_issues(raw) + assert len(errors) == 1 + assert errors[0].path == "configs.prod.match.hostname[0]" + assert "invalid regex pattern" in errors[0].message + assert "re:([unclosed" in errors[0].message + + def test_invalid_regex_match_pattern_scalar(self) -> None: + raw = { + "configs": { + "prod": { + "match": { + "path": "re:[unclosed", + } + } + } + } + errors = collect_issues(raw) + assert len(errors) == 1 + assert errors[0].path == "configs.prod.match.path" + assert "invalid regex pattern" in errors[0].message + + def test_validate_invalid_regex_raises(self) -> None: + raw = { + "configs": { + "prod": { + "match": { + "hostname": ["re:([unclosed"], + } + } + } + } + with pytest.raises(ConfigError) as exc: + validate(raw) + assert exc.value.path == "configs.prod.match.hostname[0]" + assert "invalid regex pattern" in exc.value.message