From c7a6bfe97960d29d7634a72f24fe81438c32664b Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Mon, 22 Jun 2026 11:06:24 +1000 Subject: [PATCH] Update tests --- tests/conftest.py | 14 ++++++++ tests/test_cli.py | 58 +++++++++++++++++++++++++++++++++ tests/test_cli_helpers.py | 68 +++++++++++++++++++++++++++++++++++++++ tests/test_jinjaturtle.py | 31 ++++++++++++++++++ tests/test_manifest.py | 41 +++++++++++++++++++++++ 5 files changed, 212 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 2b99213..7c060de 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,21 @@ import sys from pathlib import Path +import pytest + # Ensure repository root is on sys.path so `import enroll` resolves to the local package. ROOT = Path(__file__).resolve().parents[1] if str(ROOT) not in sys.path: sys.path.insert(0, str(ROOT)) + + +@pytest.fixture(autouse=True) +def _disable_cli_root_path_prompt_by_default(monkeypatch): + """Keep CLI tests deterministic when the test runner itself runs as root. + + Individual tests that cover the root PATH guard can override this monkeypatch. + """ + + import enroll.cli as cli + + monkeypatch.setattr(cli, "_is_effective_root", lambda: False) diff --git a/tests/test_cli.py b/tests/test_cli.py index ed3ffe6..86524c0 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -73,6 +73,64 @@ def test_cli_manifest_subcommand_calls_manifest(monkeypatch, tmp_path): assert called["target"] == "ansible" +def test_cli_force_unsafe_path_before_subcommand_reaches_guard(monkeypatch, tmp_path): + seen = {} + + def fake_confirm(*, force: bool = False) -> None: + seen["force"] = force + + def fake_manifest(_harvest_dir: str, _out_dir: str, **_kwargs): + return None + + monkeypatch.setattr(cli, "_confirm_root_path_safety", fake_confirm) + monkeypatch.setattr(cli, "manifest", fake_manifest) + monkeypatch.setattr( + sys, + "argv", + [ + "enroll", + "--assume-safe-path", + "manifest", + "--harvest", + str(tmp_path / "bundle"), + "--out", + str(tmp_path / "ansible"), + ], + ) + + cli.main() + assert seen["force"] is True + + +def test_cli_force_unsafe_path_after_subcommand_reaches_guard(monkeypatch, tmp_path): + seen = {} + + def fake_confirm(*, force: bool = False) -> None: + seen["force"] = force + + def fake_manifest(_harvest_dir: str, _out_dir: str, **_kwargs): + return None + + monkeypatch.setattr(cli, "_confirm_root_path_safety", fake_confirm) + monkeypatch.setattr(cli, "manifest", fake_manifest) + monkeypatch.setattr( + sys, + "argv", + [ + "enroll", + "manifest", + "--assume-safe-path", + "--harvest", + str(tmp_path / "bundle"), + "--out", + str(tmp_path / "ansible"), + ], + ) + + cli.main() + assert seen["force"] is True + + def test_cli_manifest_target_puppet_is_forwarded(monkeypatch, tmp_path): called = {} diff --git a/tests/test_cli_helpers.py b/tests/test_cli_helpers.py index dab28b4..7313b33 100644 --- a/tests/test_cli_helpers.py +++ b/tests/test_cli_helpers.py @@ -2,6 +2,7 @@ from __future__ import annotations import argparse import configparser +import os import types import textwrap from pathlib import Path @@ -175,3 +176,70 @@ def test_resolve_sops_out_file(tmp_path: Path, monkeypatch): cli._resolve_sops_out_file(out=None, hint="bundle.tar.gz") == fake_cache.dir / "harvest.tar.gz.sops" ) + + +def test_unsafe_root_path_reasons_flags_current_and_writable_dirs(tmp_path: Path): + from enroll.cli import _unsafe_root_path_reasons + + group_writable = tmp_path / "group-writable" + world_writable = tmp_path / "world-writable" + safe = tmp_path / "safe" + group_writable.mkdir() + world_writable.mkdir() + safe.mkdir() + group_writable.chmod(0o775) + world_writable.chmod(0o777) + safe.chmod(0o755) + + reasons = _unsafe_root_path_reasons( + os.pathsep.join( + [ + "", + ".", + "relative-bin", + str(group_writable), + str(world_writable), + str(safe), + ] + ) + ) + + text = "\n".join(reasons) + assert ": empty PATH entry" in text + assert "'.' resolves" in text + assert "relative-bin: relative PATH entry" in text + assert f"{group_writable}: directory is group-writable" in text + assert f"{world_writable}: directory is world-writable" in text + assert str(safe) not in text + + +def test_confirm_root_path_safety_refuses_noninteractive(monkeypatch): + from enroll import cli + + monkeypatch.setattr(cli, "_is_effective_root", lambda: True) + monkeypatch.setattr( + cli, + "_unsafe_root_path_reasons", + lambda path_value=None: [".: '.' resolves to the current directory"], + ) + monkeypatch.setattr(cli.sys.stdin, "isatty", lambda: False) + + try: + cli._confirm_root_path_safety(force=False) + except SystemExit as e: + assert "--assume-safe-path" in str(e) + else: # pragma: no cover - defensive assertion path + raise AssertionError("expected SystemExit") + + +def test_confirm_root_path_safety_force_skips_prompt(monkeypatch): + from enroll import cli + + monkeypatch.setattr(cli, "_is_effective_root", lambda: True) + monkeypatch.setattr( + cli, + "_unsafe_root_path_reasons", + lambda path_value=None: [".: '.' resolves to the current directory"], + ) + + cli._confirm_root_path_safety(force=True) diff --git a/tests/test_jinjaturtle.py b/tests/test_jinjaturtle.py index 9ab13c8..464f473 100644 --- a/tests/test_jinjaturtle.py +++ b/tests/test_jinjaturtle.py @@ -231,3 +231,34 @@ def test_jinjify_managed_files_rejects_templates_with_missing_defaults( assert templated == set() assert vars_text == "" assert not (template_root / "etc" / "foo" / "pdk.yaml.j2").exists() + + +def test_jinjify_artifact_rejects_unsafe_src_rel(monkeypatch, tmp_path: Path): + from enroll.jinjaturtle import jinjify_artifact + + bundle = tmp_path / "bundle" + template_root = tmp_path / "templates" + outside = tmp_path / "outside.yaml" + outside.write_text("key: value\n", encoding="utf-8") + + called = False + + def fake_run_jinjaturtle(*_args, **_kwargs): + nonlocal called + called = True + return JinjifyResult(template_text="key: {{ key }}\n", vars_text="key: value\n") + + monkeypatch.setattr(jinjaturtle_mod, "run_jinjaturtle", fake_run_jinjaturtle) + + result = jinjify_artifact( + bundle, + "foo", + "../outside.yaml", + "/etc/foo.yaml", + template_root, + jt_exe="jinjaturtle", + jt_enabled=True, + ) + + assert result is None + assert called is False diff --git a/tests/test_manifest.py b/tests/test_manifest.py index d0f83dc..94148ad 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -814,6 +814,47 @@ def test_manifest_fqdn_implies_no_common_roles(tmp_path: Path): assert not (out / "roles" / "net").exists() +def test_manifest_fqdn_rejects_unsafe_path_like_name(tmp_path: Path): + bundle = tmp_path / "bundle" + out = tmp_path / "ansible" + escape = tmp_path / "escape" + state = _minimal_package_state([]) + _write_state(bundle, state) + + with pytest.raises(Exception, match="--fqdn"): + manifest.manifest(str(bundle), str(out), fqdn=str(escape / "node")) + + assert not (escape / "node.yml").exists() + assert not (escape / "node" / "users.yml").exists() + + +def test_manifest_fqdn_rejects_newline_inventory_injection(tmp_path: Path): + bundle = tmp_path / "bundle" + out = tmp_path / "ansible" + state = _minimal_package_state([]) + _write_state(bundle, state) + + with pytest.raises(Exception, match="--fqdn"): + manifest.manifest(str(bundle), str(out), fqdn="host1\nmalicious: true") + + +def test_manifest_fqdn_existing_output_rejects_symlink_component(tmp_path: Path): + bundle = tmp_path / "bundle" + out = tmp_path / "ansible" + escape = tmp_path / "escape" + state = _minimal_package_state([]) + _write_state(bundle, state) + + (out / "inventory").mkdir(parents=True) + escape.mkdir() + (out / "inventory" / "host_vars").symlink_to(escape, target_is_directory=True) + + with pytest.raises(Exception, match="symlink"): + manifest.manifest(str(bundle), str(out), fqdn="host1.example.test") + + assert not (escape / "host1.example.test" / "users.yml").exists() + + def test_manifest_site_mode_creates_host_inventory_and_raw_files(tmp_path: Path): """In --fqdn mode, host-specific state goes into inventory/host_vars."""