diff --git a/README.md b/README.md index 6c1610e..8736644 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,7 @@ Required fields for both formats on every row or record: Input and output validation: +- run configs reject unknown top-level, time, feature, and rule fields to catch typos - config paths, event inputs, and plot CSV inputs must point to files - required event fields must be present and non-empty - custom timestamp columns cannot reuse required event field names diff --git a/src/telemetry_window_demo/cli.py b/src/telemetry_window_demo/cli.py index 8c73a70..26807ec 100644 --- a/src/telemetry_window_demo/cli.py +++ b/src/telemetry_window_demo/cli.py @@ -41,6 +41,13 @@ "source_spread_spike": frozenset(("absolute_threshold", "multiplier", "severity")), "rare_event_repeat": frozenset(("threshold", "event_types", "severity")), } +RUN_CONFIG_FIELDS = frozenset(("input_path", "output_dir", "time", "features", "rules")) +RUN_TIME_CONFIG_FIELDS = frozenset( + ("timestamp_col", "window_size_seconds", "step_size_seconds") +) +RUN_FEATURE_CONFIG_FIELDS = frozenset( + ("count_event_types", "error_statuses", "severity_levels") +) def main(argv: Sequence[str] | None = None) -> None: @@ -280,8 +287,18 @@ def _demo_root_path(value: str | None, default_root: Path) -> Path: def _validate_run_config(config: Mapping[str, Any]) -> dict[str, Any]: + _reject_unknown_config_fields(config, RUN_CONFIG_FIELDS) + time_config = _optional_mapping(config.get("time", {}), "time") + _reject_unknown_config_fields(time_config, RUN_TIME_CONFIG_FIELDS, parent="time") + feature_config = _optional_mapping(config.get("features", {}), "features") + _reject_unknown_config_fields( + feature_config, + RUN_FEATURE_CONFIG_FIELDS, + parent="features", + ) + rules_config = _validate_rules_config(config.get("rules")) return { @@ -331,13 +348,7 @@ def _validate_rules_config(raw_rules_config: Any) -> dict[str, Any]: else dict(_optional_mapping(raw_rules_config, "rules")) ) allowed_rule_keys = {"cooldown_seconds", *RUN_RULE_SECTION_NAMES} - unknown_rule_keys = sorted( - str(key) for key in rules_config if key not in allowed_rule_keys - ) - if unknown_rule_keys: - raise ValueError( - "Unknown config field(s) under 'rules': " + ", ".join(unknown_rule_keys) - ) + _reject_unknown_config_fields(rules_config, allowed_rule_keys, parent="rules") rules_config["cooldown_seconds"] = _int_config_value( rules_config.get("cooldown_seconds", 0), @@ -366,14 +377,11 @@ def _validate_rule_section_config( rule_config: dict[str, Any], ) -> dict[str, Any]: allowed_fields = RUN_RULE_CONFIG_FIELDS[rule_name] - unknown_fields = sorted( - str(key) for key in rule_config if key not in allowed_fields + _reject_unknown_config_fields( + rule_config, + allowed_fields, + parent=f"rules.{rule_name}", ) - if unknown_fields: - raise ValueError( - f"Unknown config field(s) under 'rules.{rule_name}': " - + ", ".join(unknown_fields) - ) if "severity" in rule_config: rule_config["severity"] = _string_config_value( @@ -450,6 +458,22 @@ def _optional_mapping(value: Any, field_name: str) -> Mapping[str, Any]: return value +def _reject_unknown_config_fields( + config: Mapping[str, Any], + allowed_fields: set[str] | frozenset[str], + *, + parent: str | None = None, +) -> None: + unknown_fields = sorted(str(key) for key in config if key not in allowed_fields) + if not unknown_fields: + return + + location = f" under '{parent}'" if parent else "" + raise ValueError( + f"Unknown config field(s){location}: " + ", ".join(unknown_fields) + ) + + def _path_config_value(value: Any, field_name: str) -> str: if not isinstance(value, str) or not value.strip(): raise ValueError(f"Config field '{field_name}' must be a non-empty path string.") diff --git a/tests/test_cli_errors.py b/tests/test_cli_errors.py index 18b3a3e..7295cf4 100644 --- a/tests/test_cli_errors.py +++ b/tests/test_cli_errors.py @@ -36,6 +36,32 @@ def test_main_reports_missing_config_without_traceback(tmp_path, capsys) -> None assert "Traceback" not in stderr +def test_main_reports_unknown_run_config_field_without_traceback( + tmp_path, + capsys, +) -> None: + config_path = tmp_path / "typo.yaml" + config_path.write_text( + yaml.safe_dump( + { + "input": "events.csv", + "output_dir": str(tmp_path / "processed"), + } + ), + encoding="utf-8", + ) + + with pytest.raises(SystemExit) as excinfo: + main(["run", "--config", str(config_path)]) + + assert excinfo.value.code == 1 + stderr = capsys.readouterr().err + assert stderr.startswith("error: ") + assert "Unknown config field" in stderr + assert "input" in stderr + assert "Traceback" not in stderr + + def test_main_reports_invalid_yaml_config_without_traceback(tmp_path, capsys) -> None: config_path = tmp_path / "broken.yaml" config_path.write_text("input_path: [\n", encoding="utf-8") diff --git a/tests/test_run_config_validation.py b/tests/test_run_config_validation.py index 6f82a8b..279a5b3 100644 --- a/tests/test_run_config_validation.py +++ b/tests/test_run_config_validation.py @@ -37,6 +37,33 @@ def test_run_config_requires_input_path(tmp_path) -> None: run_command(Namespace(config=str(config_path))) +def test_run_config_rejects_unknown_top_level_field(tmp_path) -> None: + config = _base_config(tmp_path) + config["input"] = config["input_path"] + config_path = _write_config(tmp_path, config) + + with pytest.raises(ValueError, match="Unknown config field.*input"): + run_command(Namespace(config=str(config_path))) + + +def test_run_config_rejects_unknown_time_field(tmp_path) -> None: + config = _base_config(tmp_path) + config["time"]["window_seconds"] = 60 + config_path = _write_config(tmp_path, config) + + with pytest.raises(ValueError, match="time.*window_seconds"): + run_command(Namespace(config=str(config_path))) + + +def test_run_config_rejects_unknown_feature_field(tmp_path) -> None: + config = _base_config(tmp_path) + config["features"]["event_counts"] = ["login_fail"] + config_path = _write_config(tmp_path, config) + + with pytest.raises(ValueError, match="features.*event_counts"): + run_command(Namespace(config=str(config_path))) + + def test_run_config_rejects_boolean_window_size(tmp_path) -> None: config = _base_config(tmp_path) config["time"]["window_size_seconds"] = True