diff --git a/.github/workflows/cowork-auto-pr.yml b/.github/workflows/cowork-auto-pr.yml index c93d03d..d2e177f 100755 --- a/.github/workflows/cowork-auto-pr.yml +++ b/.github/workflows/cowork-auto-pr.yml @@ -16,7 +16,7 @@ jobs: # without this step every run failed with "not a git repository" and no # PR was ever opened (fleet-wide defect: 11/11 seeded copies lacked it). - name: Check out the pushed branch - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: ref: ${{ github.ref_name }} fetch-depth: 0 diff --git a/pyproject.toml b/pyproject.toml index 0743e97..9e69fcd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,6 +27,7 @@ dependencies = [ "rich>=13.0.0", "pyyaml>=6.0", "tomli>=2.0.0; python_version < '3.11'", + "tomli-w>=1.0.0", ] [project.optional-dependencies] diff --git a/src/configdrift/_atomic.py b/src/configdrift/_atomic.py new file mode 100644 index 0000000..bb6c91f --- /dev/null +++ b/src/configdrift/_atomic.py @@ -0,0 +1,97 @@ +"""Atomic file-write helpers. + +Write to a temporary file in the same directory, fsync, then os.replace() +to atomically swap. If the process crashes mid-write the original file +is preserved intact. +""" + +from __future__ import annotations + +import contextlib +import os +import tempfile +from pathlib import Path + + +def atomic_write_text(path: Path, text: str, encoding: str = "utf-8") -> None: + """Atomically write *text* to *path*. + + Creates a temporary file beside *path*, writes + fsyncs, then + ``os.replace()`` for an atomic rename. Preserves the original + file's permissions when it already exists. When *path* is a + symlink, resolves it first so the referent is updated rather than + the link being replaced by a regular file. + """ + # Resolve symlinks so we update the target file, not replace the link. + resolved = path.resolve() if path.is_symlink() else path + parent = resolved.parent + parent.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp(dir=parent, suffix=".tmp") + try: + with os.fdopen(fd, "w", encoding=encoding, newline="") as fh: + fh.write(text) + fh.flush() + os.fsync(fh.fileno()) + # Preserve target permissions during atomic replacement + if resolved.exists(): + try: + st = resolved.stat() + os.chmod(tmp, st.st_mode) + except OSError: + pass + os.replace(tmp, resolved) + except BaseException: + # Clean up temp file on any failure + with contextlib.suppress(OSError): + os.unlink(tmp) + raise + + +def atomic_write_bytes(path: Path, data: bytes) -> None: + """Atomically write *data* to *path*. + + Preserves the original file's permissions when it already exists. + When *path* is a symlink, resolves it first so the referent is + updated rather than the link being replaced by a regular file. + """ + resolved = path.resolve() if path.is_symlink() else path + parent = resolved.parent + parent.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp(dir=parent, suffix=".tmp") + try: + with os.fdopen(fd, "wb") as fh: + fh.write(data) + fh.flush() + os.fsync(fh.fileno()) + # Preserve target permissions during atomic replacement + if resolved.exists(): + try: + st = resolved.stat() + os.chmod(tmp, st.st_mode) + except OSError: + pass + os.replace(tmp, resolved) + except BaseException: + with contextlib.suppress(OSError): + os.unlink(tmp) + raise + + +def atomic_dump_yaml(path: Path, data: object, **dump_kwargs: object) -> None: + """Serialize *data* via ``yaml.dump`` into a temp file, then atomically rename.""" + import io + import yaml + + buf = io.StringIO() + yaml.dump(data, buf, **dump_kwargs) # type: ignore[arg-type] + atomic_write_text(path, buf.getvalue()) + + +def atomic_dump_toml(path: Path, data: object) -> None: + """Serialize *data* via ``tomli_w.dump`` into a temp file, then atomically rename.""" + import io + import tomli_w + + buf = io.BytesIO() + tomli_w.dump(data, buf) # type: ignore[arg-type] + atomic_write_bytes(path, buf.getvalue()) diff --git a/src/configdrift/cli.py b/src/configdrift/cli.py index cb471ff..b07372d 100644 --- a/src/configdrift/cli.py +++ b/src/configdrift/cli.py @@ -15,15 +15,28 @@ except ImportError: import warnings - warnings.warn( - "revenueholdings-license not installed; license checks skipped", stacklevel=2 - ) + warnings.warn("revenueholdings-license not installed; license checks skipped", stacklevel=2) def require_license(product: str) -> None: # type: ignore[misc] pass from configdrift import __version__ +from configdrift._atomic import atomic_dump_toml, atomic_dump_yaml, atomic_write_text + +def _json_null_handler(obj: Any) -> Any: + """JSON serializer for objects not serializable by default json code. + + Handles None values and date/datetime objects that were preserved through + the flatten cycle so they serialize correctly instead of raising TypeError. + """ + if obj is None: + return None + # Handle date/datetime objects from cross-format fixes (YAML/TOML → JSON) + import datetime + if isinstance(obj, (datetime.date, datetime.datetime)): + return obj.isoformat() + raise TypeError(f"Object of type {type(obj)} is not JSON serializable") from configdrift.diff import ( Severity, diff_environments, @@ -68,12 +81,11 @@ def main( ) -> None: """ConfigDrift CLI — detect and fix configuration drift.""" global _require_license_strict - _require_license_strict = require_license_flag or bool( - os.environ.get("REVENUEHOLDINGS_REQUIRE_LICENSE") - ) + _require_license_strict = require_license_flag or bool(os.environ.get("REVENUEHOLDINGS_REQUIRE_LICENSE")) if _require_license_strict: try: from revenueholdings_license import require_license as _rl + _rl("configdrift") except ImportError: console.print( @@ -97,9 +109,7 @@ class OutputFormat(str, Enum): _DEFAULT_TARGET = "target" _DEFAULT_OUTPUT: OutputFormat = OutputFormat.TABLE _DEFAULT_STRICT = False -_FILES_ARG = typer.Argument( - ..., help="Config files to compare (2+ files; first file is baseline)." -) +_FILES_ARG = typer.Argument(..., help="Config files to compare (2+ files; first file is baseline).") _BASELINE_OPT = typer.Option( _DEFAULT_BASELINE, "--baseline", @@ -118,9 +128,7 @@ class OutputFormat(str, Enum): "-o", help="Output format: table, json, or silent (exit code only).", ) -_STRICT_OPT = typer.Option( - _DEFAULT_STRICT, "--strict", help="Exit 1 on ANY drift, not just breaking changes." -) +_STRICT_OPT = typer.Option(_DEFAULT_STRICT, "--strict", help="Exit 1 on ANY drift, not just breaking changes.") @app.command() @@ -137,11 +145,7 @@ def check( raise typer.Exit(code=1) env_configs: dict[str, dict[str, Any]] = {} - env_labels = ( - [baseline, target] - if len(files) == 2 - else [f"file_{i + 1}" for i in range(len(files))] - ) + env_labels = [baseline, target] if len(files) == 2 else [f"file_{i + 1}" for i in range(len(files))] for label, filepath in zip(env_labels, files, strict=False): try: @@ -165,11 +169,7 @@ def check( _output_table(results, baseline_env) # Exit codes for CI gating - has_drift = ( - any(r.count > 0 for r in results.values()) - if strict - else any(r.has_breaking for r in results.values()) - ) + has_drift = any(r.count > 0 for r in results.values()) if strict else any(r.has_breaking for r in results.values()) if has_drift: raise typer.Exit(code=1) @@ -187,9 +187,7 @@ def _output_table(results: dict[str, Any], baseline_env: str) -> None: table.add_column("Severity", style="magenta") for change in diff_result.changes: - symbol = {"added": "+", "removed": "-", "changed": "~"}[ - change.change_type.value - ] + symbol = {"added": "+", "removed": "-", "changed": "~"}[change.change_type.value] old_str = str(change.old_value) if change.old_value is not None else "" new_str = str(change.new_value) if change.new_value is not None else "" sev_style = ( @@ -270,9 +268,7 @@ def scan( env_name = Path(d).name dir_mapping[env_name] = d else: - console.print( - "[red]ERROR: Provide either --config or directories as arguments.[/red]" - ) + console.print("[red]ERROR: Provide either --config or directories as arguments.[/red]") raise typer.Exit(code=1) if baseline not in dir_mapping: @@ -284,9 +280,7 @@ def scan( env_configs[env_name] = {} p = Path(dir_path) if not p.is_dir(): - console.print( - f"[yellow]Warning: '{dir_path}' is not a directory, skipping.[/yellow]" - ) + console.print(f"[yellow]Warning: '{dir_path}' is not a directory, skipping.[/yellow]") continue # Load all supported config files in the directory and merge for ext in ("*.yaml", "*.yml", "*.json", "*.toml", "*.env"): @@ -310,15 +304,195 @@ def scan( else: _output_table(results, baseline) - has_drift = ( - any(r.count > 0 for r in results.values()) - if strict - else any(r.has_breaking for r in results.values()) - ) + has_drift = any(r.count > 0 for r in results.values()) if strict else any(r.has_breaking for r in results.values()) if has_drift: raise typer.Exit(code=1) +@app.command() +def fix( + files: list[str] = _FILES_ARG, + baseline: str = _BASELINE_OPT, + target: str = _TARGET_OPT, + dry_run: bool = typer.Option( # noqa: B008 + False, "--dry-run", "-n", help="Show what would change without modifying files." + ), +) -> None: + """Apply baseline values to target config files (overwrite drifted keys).""" + if len(files) < 2: + console.print("[red]ERROR: Provide at least 2 config files (baseline + target).[/red]") + raise typer.Exit(code=1) + + baseline_path = Path(files[0]) + if not baseline_path.exists(): + console.print(f"[red]ERROR: Baseline file not found: {baseline_path}[/red]") + raise typer.Exit(code=1) + + try: + baseline_data = load_file(str(baseline_path)) + except Exception as e: + console.print(f"[red]Error loading baseline config: {e}[/red]") + raise typer.Exit(code=1) from e + + # Track targets that could not be fixed so the command returns a + # non-zero exit code when any target is missing, fails to load, or + # uses an unsupported format. + failed_targets: list[str] = [] + + # Process every supplied target file (not just files[1]) + for target_file in files[1:]: + target_path = Path(target_file) + if not target_path.exists(): + console.print(f"[red]ERROR: Target file not found: {target_path}[/red]") + failed_targets.append(str(target_path)) + continue + + try: + target_data = load_file(str(target_path)) + except Exception as e: + console.print(f"[red]Error loading target config {target_path}: {e}[/red]") + failed_targets.append(str(target_path)) + continue + + changes = 0 + for key, value in baseline_data.items(): + # Distinguish missing keys from null values: a baseline null + # must restore a missing target key, not be silently skipped. + if key not in target_data: + changes += 1 + if not dry_run: + target_data[key] = value + elif target_data[key] != value: + changes += 1 + if not dry_run: + target_data[key] = value + + # Skip write-back when no changes detected + if changes == 0: + if dry_run: + console.print(f"[yellow]Dry run: no changes needed in {target_path}[/yellow]") + else: + console.print(f"[green]No drift detected in {target_path}[/green]") + continue + + if dry_run: + # Validate that the target format supports write-back even in + # dry-run mode so --dry-run accurately predicts whether the + # real run would succeed. + ext = target_path.suffix.lower() + # .env files need name-based detection: literal .env, or + # environment-suffixed variants like .env.prod, .env.dev + is_dotenv = ( + target_path.name == ".env" + or target_path.name.startswith(".env.") + ) + supported_exts = {".json", ".yaml", ".yml", ".toml"} + if ext == ".toml": + try: + import tomli_w # noqa: F401 + except ImportError: + console.print("[red]Error: tomli-w is required to write TOML files. Install with: pip install tomli-w[/red]") + failed_targets.append(str(target_path)) + continue + elif not is_dotenv and ext not in supported_exts: + console.print(f"[red]Error: unsupported format '{ext}' for write-back of {target_path}.[/red]") + failed_targets.append(str(target_path)) + continue + console.print(f"[yellow]Dry run: {changes} key(s) would be updated in {target_path}[/yellow]") + else: + ext = target_path.suffix.lower() + is_dotenv = ( + target_path.name == ".env" + or target_path.name.startswith(".env.") + ) + if ext == ".json": + import json as _json + + # Preserve nested JSON structure: rebuild from flat keys. + # Literal dotted keys (keys that already contain '.') in the + # source document are kept as single mapping keys rather + # than being re-split into nested levels. + nested_json: dict[str, Any] = {} + for k, v in target_data.items(): + if "." not in k: + nested_json[k] = v + else: + parts = k.split(".") + d = nested_json + for part in parts[:-1]: + if not isinstance(d.get(part), dict): + d[part] = {} + d = d[part] + d[parts[-1]] = v + atomic_write_text(target_path, _json.dumps(nested_json, indent=2, default=_json_null_handler) + "\n") + elif ext in (".yaml", ".yml"): + # Reconstruct nested structure from flat keys for YAML output + nested: dict[str, Any] = {} + for k, v in target_data.items(): + if "." not in k: + nested[k] = v + else: + parts = k.split(".") + d = nested + for part in parts[:-1]: + if not isinstance(d.get(part), dict): + d[part] = {} + d = d[part] + d[parts[-1]] = v + atomic_dump_yaml(target_path, nested, default_flow_style=False, sort_keys=False) + elif ext == ".toml": + try: + import tomli_w # noqa: F401 + + nested_toml: dict[str, Any] = {} + for k, v in target_data.items(): + if "." not in k: + nested_toml[k] = v + else: + parts = k.split(".") + d = nested_toml + for part in parts[:-1]: + if not isinstance(d.get(part), dict): + d[part] = {} + d = d[part] + d[parts[-1]] = v + atomic_dump_toml(target_path, nested_toml) + except ImportError: + console.print("[red]Error: tomli-w is required to write TOML files. Install with: pip install tomli-w[/red]") + failed_targets.append(str(target_path)) + continue + elif is_dotenv: + # Handle .env targets: write flat KEY=VALUE format + lines = [] + for k, v in target_data.items(): + # Quote values containing spaces, comments, or special chars. + # Escape embedded double quotes so the value round-trips + # through any POSIX-compatible shell or dotenv parser. + # Convert Python booleans to lowercase for dotenv compatibility + if isinstance(v, bool): + str_v = "true" if v else "false" + else: + str_v = str(v) if v is not None else "" + if " " in str_v or "#" in str_v or '"' in str_v: + escaped = str_v.replace('"', '\\"') + lines.append(f'{k}="{escaped}"') + else: + lines.append(f"{k}={str_v}") + atomic_write_text(target_path, "\n".join(lines) + "\n") + else: + console.print(f"[red]Error: unsupported format '{ext}' for write-back of {target_path}.[/red]") + failed_targets.append(str(target_path)) + continue + console.print(f"[green]Fixed {changes} key(s) in {target_path}[/green]") + + if failed_targets: + console.print( + f"[red]ERROR: {len(failed_targets)} target(s) could not be fixed: " + f"{', '.join(failed_targets)}[/red]" + ) + raise typer.Exit(code=1) + + @app.command() def init( path: str = typer.Argument(".", help="Directory to create .configdrift.yaml in."), # noqa: B008 diff --git a/src/configdrift/diff.py b/src/configdrift/diff.py index 8b657d8..a6b8cae 100644 --- a/src/configdrift/diff.py +++ b/src/configdrift/diff.py @@ -76,7 +76,7 @@ def _key_contains_critical_term(key: str, critical_terms: tuple[str, ...]) -> bo # Check for contiguous subsequence match (word boundary) for i in range(len(key_words) - term_len + 1): - if key_words[i:i + term_len] == term_words: + if key_words[i : i + term_len] == term_words: return True # Also check concatenated form for MULTI-WORD terms only. @@ -177,9 +177,7 @@ def diff_configs( return result -def diff_environments( - env_configs: dict[str, dict[str, Any]], baseline_env: str = "dev" -) -> dict[str, DiffResult]: +def diff_environments(env_configs: dict[str, dict[str, Any]], baseline_env: str = "dev") -> dict[str, DiffResult]: """Compare multiple environments against a baseline.""" if baseline_env not in env_configs: raise ValueError(f"Baseline environment '{baseline_env}' not found in configs") diff --git a/src/configdrift/loader.py b/src/configdrift/loader.py index 2bae957..c19bb1b 100644 --- a/src/configdrift/loader.py +++ b/src/configdrift/loader.py @@ -6,9 +6,7 @@ from pathlib import Path from typing import Any -_toml = importlib.import_module( - "tomllib" if __import__("sys").version_info >= (3, 11) else "tomli" -) +_toml = importlib.import_module("tomllib" if __import__("sys").version_info >= (3, 11) else "tomli") def load_file(path: str) -> dict[str, Any]: @@ -43,9 +41,7 @@ def _load_yaml(path: Path) -> dict[str, Any]: with open(path, encoding="utf-8") as f: data = yaml.safe_load(f) if not isinstance(data, dict): - raise ValueError( - f"YAML file must contain a mapping (dict), got {type(data).__name__}" - ) + raise ValueError(f"YAML file must contain a mapping (dict), got {type(data).__name__}") return _flatten_nested(data) @@ -53,9 +49,7 @@ def _load_json(path: Path) -> dict[str, Any]: with open(path, encoding="utf-8") as f: data = json.load(f) if not isinstance(data, dict): - raise ValueError( - f"JSON file must contain a mapping (dict), got {type(data).__name__}" - ) + raise ValueError(f"JSON file must contain a mapping (dict), got {type(data).__name__}") return _flatten_nested(data) @@ -102,22 +96,40 @@ def _load_dotenv(path: Path) -> dict[str, Any]: val = _strip_inline_comment(val) # Strip surrounding quotes if len(val) >= 2 and val[0] == val[-1] and val[0] in ('"', "'"): + quote_char = val[0] val = val[1:-1] + # Unescape backslash-escaped quotes for round-trip + # fidelity with the fix writer (KEY="say \"hi\"") + if quote_char == '"': + val = val.replace('\\"', '"') data[key] = val return data def _flatten_nested(d: dict[str, Any], prefix: str = "") -> dict[str, Any]: - """Flatten nested dicts into dot-separated keys.""" - result = {} + """Flatten nested dicts into dot-separated keys. + + Preserves non-dict collection values (lists, tuples) and scalar values + without converting them to strings. Keys that are already dotted in + the source document are kept literal — they are never re-split on + ``.`` during reconstruction. + """ + result: dict[str, Any] = {} for key, value in d.items(): full_key = f"{prefix}.{key}" if prefix else key if isinstance(value, dict): - result.update(_flatten_nested(value, full_key)) + if not value: + # Preserve empty mappings so reconstruction doesn't + # silently drop them when other keys need fixing. + result[full_key] = {} + else: + result.update(_flatten_nested(value, full_key)) elif value is None: - result[full_key] = "" + # Preserve null as None so the fix cycle can distinguish + # "baseline is null" from "baseline is empty string". + # Writers handle None appropriately per format. + result[full_key] = None else: - result[full_key] = ( - str(value) if not isinstance(value, str | int | float | bool) else value - ) + # Preserve lists, tuples, ints, floats, bools, and strings as-is + result[full_key] = value return result diff --git a/tests/test_atomic_write.py b/tests/test_atomic_write.py new file mode 100644 index 0000000..a0fa2d3 --- /dev/null +++ b/tests/test_atomic_write.py @@ -0,0 +1,86 @@ +"""Tests for atomic write helpers — verify original file survives write failure.""" + +import os +import pytest +from configdrift._atomic import atomic_write_bytes, atomic_write_text +from pathlib import Path +from unittest.mock import patch + + +class TestAtomicWriteText: + """Verify atomic_write_text preserves original on failure.""" + + def test_successful_write(self, tmp_path: Path): + target = tmp_path / "config.json" + target.write_text("original") + atomic_write_text(target, '{"key": "value"}\n') + assert target.read_text() == '{"key": "value"}\n' + + def test_creates_file_if_missing(self, tmp_path: Path): + target = tmp_path / "new.json" + atomic_write_text(target, "hello") + assert target.read_text() == "hello" + + def test_original_preserved_on_oserror(self, tmp_path: Path): + """If os.replace fails, the original file must remain intact.""" + target = tmp_path / "config.json" + target.write_text("original-content") + + with ( + patch("os.replace", side_effect=OSError("Simulated disk full")), + pytest.raises(OSError, match="Simulated disk full"), + ): + atomic_write_text(target, "new-content") + + # Original must be untouched + assert target.read_text() == "original-content" + + def test_no_temp_files_left_on_failure(self, tmp_path: Path): + """Temp files must be cleaned up after a failed write.""" + target = tmp_path / "config.json" + target.write_text("original") + + with patch("os.replace", side_effect=OSError("fail")), pytest.raises(OSError): + atomic_write_text(target, "new") + + temps = list(tmp_path.glob("*.tmp")) + assert temps == [], f"Leftover temp files: {temps}" + + def test_truncation_does_not_corrupt_original(self, tmp_path: Path): + """Even if the temp-file write itself fails mid-stream, original is safe.""" + target = tmp_path / "config.json" + original = "important-data-that-must-survive" + target.write_text(original) + + real_fdopen = os.fdopen + + def failing_fdopen(fd, *args, **kwargs): + fh = real_fdopen(fd, *args, **kwargs) # noqa: F841 + # Simulate failure after opening but before writing + raise OSError("Disk full during write") + + with ( + patch("os.fdopen", side_effect=failing_fdopen), + pytest.raises(OSError, match="Disk full during write"), + ): + atomic_write_text(target, "replacement") + + assert target.read_text() == original + + +class TestAtomicWriteBytes: + """Verify atomic_write_bytes preserves original on failure.""" + + def test_successful_binary_write(self, tmp_path: Path): + target = tmp_path / "data.bin" + atomic_write_bytes(target, b"\x00\x01\x02") + assert target.read_bytes() == b"\x00\x01\x02" + + def test_original_preserved_on_oserror(self, tmp_path: Path): + target = tmp_path / "data.bin" + target.write_bytes(b"original-bytes") + + with patch("os.replace", side_effect=OSError("fail")), pytest.raises(OSError): + atomic_write_bytes(target, b"new-bytes") + + assert target.read_bytes() == b"original-bytes" diff --git a/tests/test_ci_workflow.py b/tests/test_ci_workflow.py index 50c88be..ab6466a 100644 --- a/tests/test_ci_workflow.py +++ b/tests/test_ci_workflow.py @@ -5,9 +5,7 @@ def test_ci_test_step_executes_full_suite(): - workflow = yaml.safe_load( - (ROOT / ".github" / "workflows" / "ci.yml").read_text(encoding="utf-8") - ) + workflow = yaml.safe_load((ROOT / ".github" / "workflows" / "ci.yml").read_text(encoding="utf-8")) test_steps = workflow["jobs"]["test"]["steps"] run_tests = next( (step for step in test_steps if step.get("name") == "Run tests"), diff --git a/tests/test_cli.py b/tests/test_cli.py index 4c4667c..d5530a8 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -32,9 +32,7 @@ def test_check_json_output(self): dev.write_text(json.dumps({"host": "localhost"})) prod.write_text(json.dumps({"host": "prod.example.com", "port": 443})) - result = runner.invoke( - app, ["check", str(dev), str(prod), "--output", "json"] - ) + result = runner.invoke(app, ["check", str(dev), str(prod), "--output", "json"]) assert result.exit_code == 0 data = json.loads(result.stdout) assert "target" in data @@ -126,9 +124,7 @@ def test_scan_two_dirs(self): dev_dir.mkdir() prod_dir.mkdir() (dev_dir / "config.yaml").write_text(yaml.dump({"host": "localhost"})) - (prod_dir / "config.yaml").write_text( - yaml.dump({"host": "prod.example.com"}) - ) + (prod_dir / "config.yaml").write_text(yaml.dump({"host": "prod.example.com"})) result = runner.invoke(app, ["scan", str(dev_dir), str(prod_dir)]) assert result.exit_code == 0 @@ -166,9 +162,7 @@ def test_scan_baseline_not_found(self): dev_dir = Path(tmpdir) / "dev" dev_dir.mkdir() (dev_dir / "c.yaml").write_text(yaml.dump({"k": "v"})) - result = runner.invoke( - app, ["scan", str(dev_dir), "--baseline", "nonexistent"] - ) + result = runner.invoke(app, ["scan", str(dev_dir), "--baseline", "nonexistent"]) assert result.exit_code == 1 assert "not found" in result.stdout @@ -182,9 +176,7 @@ def test_scan_json_output(self): (dev_dir / "c.yaml").write_text(yaml.dump({"host": "localhost"})) (prod_dir / "c.yaml").write_text(yaml.dump({"host": "prod.example.com"})) - result = runner.invoke( - app, ["scan", str(dev_dir), str(prod_dir), "--output", "json"] - ) + result = runner.invoke(app, ["scan", str(dev_dir), str(prod_dir), "--output", "json"]) assert result.exit_code == 0 data = json.loads(result.stdout) assert "prod" in data @@ -223,12 +215,8 @@ def test_scan_breaking_drift_exit_code(self): prod_dir = Path(tmpdir) / "prod" dev_dir.mkdir() prod_dir.mkdir() - (dev_dir / "c.yaml").write_text( - yaml.dump({"database_url": "postgres://dev"}) - ) - (prod_dir / "c.yaml").write_text( - yaml.dump({"database_url": "postgres://prod"}) - ) + (dev_dir / "c.yaml").write_text(yaml.dump({"database_url": "postgres://dev"})) + (prod_dir / "c.yaml").write_text(yaml.dump({"database_url": "postgres://prod"})) result = runner.invoke(app, ["scan", str(dev_dir), str(prod_dir)]) assert result.exit_code == 1 @@ -255,9 +243,7 @@ def test_scan_strict_exits_on_any_drift(self): assert result.exit_code == 0 # With --strict, any drift exits 1 - result = runner.invoke( - app, ["scan", str(dev_dir), str(prod_dir), "--strict"] - ) + result = runner.invoke(app, ["scan", str(dev_dir), str(prod_dir), "--strict"]) assert result.exit_code == 1 def test_scan_strict_no_drift_exits_zero(self): @@ -270,9 +256,7 @@ def test_scan_strict_no_drift_exits_zero(self): (dev_dir / "c.yaml").write_text(yaml.dump({"host": "localhost"})) (prod_dir / "c.yaml").write_text(yaml.dump({"host": "localhost"})) - result = runner.invoke( - app, ["scan", str(dev_dir), str(prod_dir), "--strict"] - ) + result = runner.invoke(app, ["scan", str(dev_dir), str(prod_dir), "--strict"]) assert result.exit_code == 0 def test_scan_silent_breaking_drift(self): @@ -282,16 +266,10 @@ def test_scan_silent_breaking_drift(self): prod_dir = Path(tmpdir) / "prod" dev_dir.mkdir() prod_dir.mkdir() - (dev_dir / "c.yaml").write_text( - yaml.dump({"database_url": "postgres://dev"}) - ) - (prod_dir / "c.yaml").write_text( - yaml.dump({"database_url": "postgres://prod"}) - ) + (dev_dir / "c.yaml").write_text(yaml.dump({"database_url": "postgres://dev"})) + (prod_dir / "c.yaml").write_text(yaml.dump({"database_url": "postgres://prod"})) - result = runner.invoke( - app, ["scan", str(dev_dir), str(prod_dir), "--output", "silent"] - ) + result = runner.invoke(app, ["scan", str(dev_dir), str(prod_dir), "--output", "silent"]) assert result.exit_code == 1 @@ -360,9 +338,7 @@ def test_check_strict_silent_exits_on_any_drift(self): a.write_text(yaml.dump({"host": "localhost"})) b.write_text(yaml.dump({"host": "staging.example.com"})) - result = runner.invoke( - app, ["check", str(a), str(b), "--output", "silent", "--strict"] - ) + result = runner.invoke(app, ["check", str(a), str(b), "--output", "silent", "--strict"]) assert result.exit_code == 1 def test_check_strict_no_drift_exits_zero(self): @@ -398,9 +374,7 @@ def test_scan_env_and_toml_dirs(self): (dev_dir / "app.env").write_text("HOST=localhost\n") (prod_dir / "app.env").write_text("HOST=prod.example.com\n") - result = runner.invoke( - app, ["scan", str(dev_dir), str(prod_dir), "--output", "json"] - ) + result = runner.invoke(app, ["scan", str(dev_dir), str(prod_dir), "--output", "json"]) assert result.exit_code == 0 data = json.loads(result.stdout) assert "prod" in data @@ -447,20 +421,12 @@ def test_scan_no_changes_env_skipped_in_table(self): prod_dir = Path(tmpdir) / "prod" for d in [dev_dir, staging_dir, prod_dir]: d.mkdir() - (dev_dir / "c.yaml").write_text( - yaml.dump({"host": "localhost", "port": 8080}) - ) + (dev_dir / "c.yaml").write_text(yaml.dump({"host": "localhost", "port": 8080})) # staging is identical to dev — no changes - (staging_dir / "c.yaml").write_text( - yaml.dump({"host": "localhost", "port": 8080}) - ) - (prod_dir / "c.yaml").write_text( - yaml.dump({"host": "prod.example.com", "port": 8080}) - ) + (staging_dir / "c.yaml").write_text(yaml.dump({"host": "localhost", "port": 8080})) + (prod_dir / "c.yaml").write_text(yaml.dump({"host": "prod.example.com", "port": 8080})) - result = runner.invoke( - app, ["scan", str(dev_dir), str(staging_dir), str(prod_dir)] - ) + result = runner.invoke(app, ["scan", str(dev_dir), str(staging_dir), str(prod_dir)]) assert result.exit_code == 0 # Should show prod drift but skip staging (no changes) assert "prod" in result.stdout diff --git a/tests/test_coverage_gaps.py b/tests/test_coverage_gaps.py index 6c9ae82..6d151c8 100644 --- a/tests/test_coverage_gaps.py +++ b/tests/test_coverage_gaps.py @@ -19,16 +19,12 @@ def test_double_quote_toggle_with_hash(self): # A value with a " inside it, followed by # outside quotes # The " should be detected as the start/end of double-quoting result = _strip_inline_comment('prefix "hello" # comment') - assert result == 'prefix "hello"', ( - f"Expected comment stripped after quoted section, got: {result!r}" - ) + assert result == 'prefix "hello"', f"Expected comment stripped after quoted section, got: {result!r}" def test_single_quote_toggle_with_hash(self): """Line 74: in_single should toggle when encountering ' outside double quotes.""" result = _strip_inline_comment("prefix 'hello' # comment") - assert result == "prefix 'hello'", ( - f"Expected comment stripped after quoted section, got: {result!r}" - ) + assert result == "prefix 'hello'", f"Expected comment stripped after quoted section, got: {result!r}" class TestLoadDotenvQuoteStrip: diff --git a/tests/test_diff.py b/tests/test_diff.py index 9307850..9c404c3 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -9,6 +9,7 @@ _infer_severity_added, _infer_severity_changed, _infer_severity_removed, + _key_contains_critical_term, diff_configs, diff_environments, ) @@ -22,9 +23,7 @@ def test_change_str_added(self): assert "443" in str(c) def test_change_str_removed(self): - c = Change( - key="host", change_type=ChangeType.REMOVED, old_value="localhost", env="dev" - ) + c = Change(key="host", change_type=ChangeType.REMOVED, old_value="localhost", env="dev") assert "[-]" in str(c) assert "host" in str(c) @@ -76,13 +75,9 @@ def test_by_type(self): def test_by_severity(self): r = DiffResult( changes=[ - Change( - key="a", change_type=ChangeType.CHANGED, severity=Severity.BREAKING - ), + Change(key="a", change_type=ChangeType.CHANGED, severity=Severity.BREAKING), Change(key="b", change_type=ChangeType.CHANGED, severity=Severity.INFO), - Change( - key="c", change_type=ChangeType.ADDED, severity=Severity.WARNING - ), + Change(key="c", change_type=ChangeType.ADDED, severity=Severity.WARNING), ] ) breaking = r.by_severity(Severity.BREAKING) @@ -374,3 +369,26 @@ def test_authentication_not_auth(self): def test_authz_not_auth(self): assert _infer_severity_added("authz", "x") == Severity.WARNING + + +class TestKeyContainsCriticalTerm: + """Direct tests for _key_contains_critical_term, covering the empty-term guard.""" + + def test_empty_term_returns_false(self): + """An empty critical term must be skipped gracefully (diff.py:74-75).""" + assert _key_contains_critical_term("any_key", ("",)) is False + + def test_empty_among_valid_terms(self): + """Empty term mixed with valid terms must not break matching.""" + assert _key_contains_critical_term("database_url", ("", "database")) is True + assert _key_contains_critical_term("cache_ttl", ("", "database")) is False + + def test_all_empty_terms(self): + """Tuple of only empty strings must never match.""" + assert _key_contains_critical_term("auth_token", ("", "", "")) is False + + def test_dot_separated_match(self): + assert _key_contains_critical_term("services.database.password", ("database",)) is True + + def test_no_match(self): + assert _key_contains_critical_term("cache_ttl", ("database", "auth")) is False diff --git a/tests/test_fix_command.py b/tests/test_fix_command.py new file mode 100644 index 0000000..705aa07 --- /dev/null +++ b/tests/test_fix_command.py @@ -0,0 +1,116 @@ +"""Tests for the `fix` CLI command — apply baseline values to target config files.""" + +import json +import pytest +from configdrift.cli import app +from pathlib import Path +from typer.testing import CliRunner + +runner = CliRunner() + + +class TestFixCommandBasic: + """Core fix behavior: overwrite target keys with baseline values.""" + + def test_fix_json_overwrites_target_with_baseline(self, tmp_path: Path): + baseline = tmp_path / "dev.json" + target = tmp_path / "prod.json" + baseline.write_text(json.dumps({"host": "localhost", "port": 8080})) + target.write_text(json.dumps({"host": "prod.example.com", "port": 9090})) + + result = runner.invoke(app, ["fix", str(baseline), str(target)]) + assert result.exit_code == 0, result.output + + fixed = json.loads(target.read_text()) + assert fixed["host"] == "localhost" + assert fixed["port"] == 8080 + + def test_fix_yaml_overwrites_target_with_baseline(self, tmp_path: Path): + baseline = tmp_path / "dev.yaml" + target = tmp_path / "prod.yaml" + baseline.write_text("host: localhost\nport: 8080\n") + target.write_text("host: prod.example.com\nport: 9090\n") + + result = runner.invoke(app, ["fix", str(baseline), str(target)]) + assert result.exit_code == 0, result.output + + # Re-parse via load_file to verify round-trip + from configdrift.loader import load_file + + fixed = load_file(str(target)) + assert fixed["host"] == "localhost" + assert fixed["port"] == 8080 + + def test_fix_preserves_keys_not_in_baseline(self, tmp_path: Path): + """Keys only in target (not in baseline) should be preserved.""" + baseline = tmp_path / "dev.json" + target = tmp_path / "prod.json" + baseline.write_text(json.dumps({"host": "localhost"})) + target.write_text(json.dumps({"host": "prod.example.com", "extra_key": "keep_me"})) + + result = runner.invoke(app, ["fix", str(baseline), str(target)]) + assert result.exit_code == 0, result.output + + fixed = json.loads(target.read_text()) + assert fixed["host"] == "localhost" + assert fixed["extra_key"] == "keep_me" + + def test_fix_adds_missing_baseline_keys_to_target(self, tmp_path: Path): + """Keys in baseline but missing from target should be added.""" + baseline = tmp_path / "dev.json" + target = tmp_path / "prod.json" + baseline.write_text(json.dumps({"host": "localhost", "new_key": "new_value"})) + target.write_text(json.dumps({"host": "prod.example.com"})) + + result = runner.invoke(app, ["fix", str(baseline), str(target)]) + assert result.exit_code == 0, result.output + + fixed = json.loads(target.read_text()) + assert fixed["host"] == "localhost" + assert fixed["new_key"] == "new_value" + + +class TestFixCommandEdgeCases: + """Edge cases and error handling for the fix command.""" + + def test_fix_requires_at_least_two_files(self): + result = runner.invoke(app, ["fix", "only_one.json"]) + assert result.exit_code != 0 + + def test_fix_nonexistent_baseline_exits_error(self, tmp_path: Path): + target = tmp_path / "prod.json" + target.write_text(json.dumps({"host": "x"})) + result = runner.invoke(app, ["fix", str(tmp_path / "missing.json"), str(target)]) + assert result.exit_code != 0 + + def test_fix_dry_run_does_not_modify_target(self, tmp_path: Path): + baseline = tmp_path / "dev.json" + target = tmp_path / "prod.json" + baseline.write_text(json.dumps({"host": "localhost"})) + original_content = json.dumps({"host": "prod.example.com"}) + target.write_text(original_content) + + result = runner.invoke(app, ["fix", "--dry-run", str(baseline), str(target)]) + assert result.exit_code == 0, result.output + + # Target must be unchanged + assert target.read_text() == original_content + # Output should mention dry-run or what would change + assert "dry" in result.output.lower() or "would" in result.output.lower() + + def test_fix_toml_round_trip(self, tmp_path: Path): + """Fix should work with TOML files when tomli-w is installed.""" + pytest.importorskip("tomli_w") + baseline = tmp_path / "dev.toml" + target = tmp_path / "prod.toml" + baseline.write_text('[database]\nhost = "localhost"\nport = 5432\n') + target.write_text('[database]\nhost = "prod.db"\nport = 3306\n') + + result = runner.invoke(app, ["fix", str(baseline), str(target)]) + assert result.exit_code == 0, result.output + + from configdrift.loader import load_file + + fixed = load_file(str(target)) + assert fixed["database.host"] == "localhost" + assert fixed["database.port"] == 5432 diff --git a/tests/test_loader.py b/tests/test_loader.py index f5fd1b2..8de958d 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -178,9 +178,7 @@ def test_dotenv_export_prefix(self): """Lines with 'export ' prefix should be parsed correctly.""" with tempfile.TemporaryDirectory() as tmpdir: p = Path(tmpdir) / ".env" - p.write_text( - "export DATABASE_URL=postgres://localhost\nexport API_KEY=secret123\n" - ) + p.write_text("export DATABASE_URL=postgres://localhost\nexport API_KEY=secret123\n") result = load_file(str(p)) assert result["DATABASE_URL"] == "postgres://localhost" assert result["API_KEY"] == "secret123"