Skip to content

fix(config): validate 're:' match patterns and report exact paths (closes #118) - #159

Merged
falbany merged 1 commit into
falbany:mainfrom
txltedxgod:fix/issue-118-validate-re-match-patterns
Nov 9, 2026
Merged

falbany merged 1 commit into
falbany:mainfrom
txltedxgod:fix/issue-118-validate-re-match-patterns

Conversation

@txltedxgod

Copy link
Copy Markdown
Contributor

Closes #118.

Problem

Previously, specifying an invalid regex pattern (e.g. re:([unclosed) under match.hostname, match.ip, or match.path was not validated during configuration parsing or schema validation. When ProFiles attempted to match criteria, re.search() raised an unhandled re.error causing a crash at startup or during configuration inspection. Furthermore, diagnostics/validation in collect_issues() did not validate re: syntax to surface inline editor problems.

Solution

  1. Schema Validation (MatchCriteriaSchema): Added @field_validator(hostname, ip, path, mode=after) to ensure any re:-prefixed pattern compiles with re.compile(pattern[3:]) or raises a validation error.
  2. Diagnostics & Semantic Issues (collect_issues): Validates regex patterns in collect_issues() and reports exact issue paths (e.g. configs.<name>.match.<field>[<idx>] or configs.<name>.match.<field>) with severity error so the web editor problems list highlights the malformed regex line.
  3. Runtime Fallback (matcher.py): Added except re.error: return False to match_pattern() as defense-in-depth against unexpected regex compilation errors at runtime.
  4. Test Suite:
    • Added unit tests in ests/core/config/test_schema.py for valid/invalid regex patterns in MatchCriteriaSchema.
    • Added tests in ests/core/config/test_validator.py verifying collect_issues and �alidate_config report exact issue paths.
    • Added tests in ests/core/config/test_matcher.py ensuring invalid regex gracefully returns False.
    • Added test in ests/core/config/test_diagnostics.py verifying config issue reporting.

@falbany

falbany commented Nov 9, 2026

Copy link
Copy Markdown
Owner

Thanks @txltedxgod — this fixes #118 cleanly at all three layers (schema, diagnostics, matcher).

Verified locally at c56c5bd:

  • Full suite: 1518 passed, 1 skipped, coverage 88.96% (Python 3.13)
  • ruff format --check / ruff check clean, pylint 9.88, pyright 0 errors
  • End-to-end reproduction of the config: invalid 're:' match pattern crashes startup and GET /api/config instead of failing validation #118 crash path: a config with hostname: ["re:([unclosed"] now fails at load with CONFIG_INVALID configs.prod.match.hostname[0]: invalid regex pattern 're:([unclosed': unterminated character set and falls back to defaults — instead of loading cleanly and raising re.error at match time, which previously took down startup and GET /api/config.
  • The restructured extends check in collect_issues is behavior-preserving (the old continue would have skipped the new match checks, so the flattening was required) ✓

Non-blocking notes:

  1. collect_issues repeats the same regex-check in the scalar and list branches — a small loop over (pattern, path_suffix) pairs would DRY it up. Fine as-is.
  2. _validate_patterns compiles without re.IGNORECASE while match_pattern searches with it. A pattern valid under re.compile() but invalid under IGNORECASE is vanishingly rare, and the new matcher try/except covers that gap anyway.
  3. validate()'s docstring ("Raises ConfigError: On unknown top-level keys, unknown extends references, or inheritance cycles") could now mention invalid regex patterns — trivial.

Nice touch adding the matcher defense-in-depth: a pattern that somehow slips through now degrades to "never matches" instead of crashing, and that behavior is pinned by the tests.

Verdict: no blocking issues — ready to merge from my side.


Automated PR evaluation — verified locally in a throwaway worktree (pytest, ruff, pylint, pyright); comment-only review.

@falbany
falbany merged commit 15957a2 into falbany:main Nov 9, 2026
1 check passed
@falbany

falbany commented Nov 9, 2026

Copy link
Copy Markdown
Owner

Merged — thank you @txltedxgod! 🎉

Clean, well-scoped fix for #118: validating at the schema layer, surfacing exact editor paths (configs.<name>.match.<field>[<idx>]) via collect_issues, and the defense-in-depth fallback in the matcher. The test coverage across all four files made verification straightforward, and everything passed on the first run locally (full suite 1518 green, pylint 9.88, pyright 0).

Welcome to the project as a contributor, and glad to have you back — if you spot another issue you'd like to pick up, the good first issue and code-quality labels are good places to look, or just open an issue if you find something broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

config: invalid 're:' match pattern crashes startup and GET /api/config instead of failing validation

2 participants