diff --git a/CHANGELOG.md b/CHANGELOG.md index 67c86a8..d6213bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,6 @@ * BREAKING CHANGE: Group all package and systemd-unit roles into Debian Section/RPM Group roles by default, including managed config files and unit state. This mode is not used if `--fqdn` or `--no-common-roles` is set, in which case, the traditional behaviour of preserving one role per package/unit is used instead. * BREAKING CHANGE: Only capture user-specific .bashrc style files when using `--dangerous` mode, in case they contain sensitive env vars. - * BREAKING CHANGE: Don't allow reading `.enroll.ini` in the CWD. Use only the ENROLL_CONFIG env var, an explicit `--config` path or else the XDG default location (or `~/.config/enroll/enroll.ini` if `XDG_CONFIG_HOME` is not set). * Detect active sysctl parameters and write them to a `/etc/sysctl.d/99-enroll.conf` file * Use `no_log` on systemd unit interrogations to suppress potential sensitive output when applying Ansible * Support manifesting Puppet code, as well as Ansible! @@ -11,8 +10,6 @@ * A lot of under-the-bonnet refactoring to make it easier to extend to cover other config managers (that don't suck) in future. * Support for detecting Docker and Podman images and enforcing their presence (by SHA256 hash). * Add support for detecting Flatpaks and Snaps. - * Stricter validation of harvests to ensure that they meet the schema and don't contain unsafe artifacts (e.g symlinks pointing outside the artifact tree) - * Perform harvest validation before trying to manifest from it. # 0.6.0 diff --git a/README.md b/README.md index 722762a..d2a8735 100644 --- a/README.md +++ b/README.md @@ -656,8 +656,8 @@ Enroll supports reading an ini-style file of all the arguments for each subcomma ### Location of the config file The path the config file can be specified with `-c` or `--config` on the command-line. Otherwise, -Enroll will look for the `ENROLL_CONFIG` environment variable, `$XDG_CONFIG_HOME/enroll/enroll.ini`, -or `~/.config/enroll/enroll.ini`. +Enroll will look for `./enroll.ini`, `./.enroll.ini` (in the current working directory), +`~/.config/enroll/enroll.ini` (or `$XDG_CONFIG_HOME/enroll/enroll.ini`). You may also pass `--no-config` if you deliberately want to ignore the config file even if it existed. diff --git a/enroll/ansible.py b/enroll/ansible.py index e0fcd0c..9794de3 100644 --- a/enroll/ansible.py +++ b/enroll/ansible.py @@ -2,6 +2,7 @@ from __future__ import annotations import os import re +import shutil import stat import tempfile from dataclasses import dataclass @@ -13,11 +14,6 @@ from .jinjaturtle import ( jinjify_managed_files as _jinjify_managed_files, resolve_jinjaturtle_mode, ) -from .manifest_safety import ( - copy_safe_artifact_file, - iter_safe_artifact_files, - prepare_manifest_output_dir, -) from .role_names import avoid_reserved_role_name from .state import inventory_packages_from_state, roles_from_state from .yamlutil import yaml_dump_mapping, yaml_load_mapping @@ -412,8 +408,7 @@ def _prepare_ansible_context( site_mode = fqdn is not None and fqdn != "" jt_exe, jt_enabled = resolve_jinjaturtle_mode(jinjaturtle) - out = prepare_manifest_output_dir(out_dir, allow_existing=site_mode) - out_dir = str(out) + os.makedirs(out_dir, exist_ok=True) roles_root = os.path.join(out_dir, "roles") os.makedirs(roles_root, exist_ok=True) @@ -450,7 +445,7 @@ def _copy2_replace(src: str, dst: str) -> None: fd, tmp = tempfile.mkstemp(prefix=".enroll-tmp-", dir=dst_dir) os.close(fd) try: - copy_safe_artifact_file(src, tmp) + shutil.copy2(src, tmp) # Ensure the working tree stays mergeable: make the file user-writable. st = os.stat(tmp, follow_symlinks=False) @@ -480,23 +475,29 @@ def _copy_artifacts( In --fqdn site mode, this is usually: inventory/host_vars///.files """ - for src, rel in iter_safe_artifact_files(bundle_dir, role): - dst = os.path.join(dst_files_dir, rel) + artifacts_dir = os.path.join(bundle_dir, "artifacts", role) + if not os.path.isdir(artifacts_dir): + return + for root, _, files in os.walk(artifacts_dir): + for fn in files: + src = os.path.join(root, fn) + rel = os.path.relpath(src, artifacts_dir) + dst = os.path.join(dst_files_dir, rel) - # If a file was successfully templatised by JinjaTurtle, do NOT - # also materialise the raw copy in the destination files dir. - if exclude_rels and rel in exclude_rels: - try: - if os.path.isfile(dst): - os.remove(dst) - except Exception: - pass # nosec - continue + # If a file was successfully templatised by JinjaTurtle, do NOT + # also materialise the raw copy in the destination files dir. + if exclude_rels and rel in exclude_rels: + try: + if os.path.isfile(dst): + os.remove(dst) + except Exception: + pass # nosec + continue - if preserve_existing and os.path.exists(dst): - continue - os.makedirs(os.path.dirname(dst), exist_ok=True) - _copy2_replace(str(src), dst) + if preserve_existing and os.path.exists(dst): + continue + os.makedirs(os.path.dirname(dst), exist_ok=True) + _copy2_replace(src, dst) def _write_role_scaffold(role_dir: str) -> None: diff --git a/enroll/cli.py b/enroll/cli.py index 1368ffb..e3816f0 100644 --- a/enroll/cli.py +++ b/enroll/cli.py @@ -39,10 +39,8 @@ def _discover_config_path(argv: list[str]) -> Optional[Path]: 1) --no-config disables loading. 2) --config PATH (or -c PATH) 3) $ENROLL_CONFIG - 4) $XDG_CONFIG_HOME/enroll/enroll.ini (or ~/.config/enroll/enroll.ini) - - Current-directory config files are deliberately not auto-loaded; use - --config ./enroll.ini if that behaviour is desired. + 4) ./enroll.ini, ./.enroll.ini + 5) $XDG_CONFIG_HOME/enroll/enroll.ini (or ~/.config/enroll/enroll.ini) The config file is optional; if no file is found, returns None. """ @@ -68,6 +66,12 @@ def _discover_config_path(argv: list[str]) -> Optional[Path]: if envp: return Path(envp).expanduser() + cwd = Path.cwd() + for name in ("enroll.ini", ".enroll.ini"): + cp = cwd / name + if cp.exists() and cp.is_file(): + return cp + xdg = os.environ.get("XDG_CONFIG_HOME") if xdg: base = Path(xdg).expanduser() diff --git a/enroll/diff.py b/enroll/diff.py index 365d107..9a9bee4 100644 --- a/enroll/diff.py +++ b/enroll/diff.py @@ -923,17 +923,14 @@ def enforce_old_harvest( except OSError: pass - # 1) Generate a manifest in a temp directory. The renderer now - # refuses to write into an existing destination, so use a fresh - # child path under the secure temporary directory. - manifest_dir = td_path / "manifest" - manifest(str(old_b.dir), str(manifest_dir), target=target) + # 1) Generate a manifest in a temp directory. + manifest(str(old_b.dir), str(td_path), target=target) # 2) Apply it locally. cmd, env = _enforcement_command( target, tool_exe, - manifest_dir, + td_path, tags=tags, ) @@ -1457,14 +1454,8 @@ def send_email( try: s.starttls() s.ehlo() - except Exception as e: - if smtp_user or smtp_password: - raise RuntimeError( - "email: SMTP STARTTLS failed; refusing to send credentials " - "without TLS" - ) from e - # Without credentials, keep STARTTLS opportunistic so localhost or - # unauthenticated relay setups continue to work. + except Exception: + # STARTTLS is optional; ignore if unsupported. pass # nosec if smtp_user: s.login(smtp_user, smtp_password or "") diff --git a/enroll/manifest.py b/enroll/manifest.py index bb88d21..c9fca19 100644 --- a/enroll/manifest.py +++ b/enroll/manifest.py @@ -16,7 +16,6 @@ from .sopsutil import ( encrypt_file_binary, require_sops_cmd, ) -from .validate import validate_harvest def _prepare_bundle_dir( @@ -204,14 +203,6 @@ def manifest( td_out: Optional[tempfile.TemporaryDirectory] = None try: - validation = validate_harvest(resolved_bundle_dir) - if not validation.ok: - raise RuntimeError( - "harvest state does not match this Enroll version's schema; " - "please re-harvest the host with this version of Enroll.\n" - + validation.to_text().strip() - ) - if not sops_mode: if target == "puppet": manifest_puppet_from_bundle_dir( @@ -244,6 +235,11 @@ def manifest( td_out = tempfile.TemporaryDirectory(prefix="enroll-manifest-") tmp_out = Path(td_out.name) / "out" + tmp_out.mkdir(parents=True, exist_ok=True) + try: + os.chmod(tmp_out, 0o700) + except OSError: + pass if target == "puppet": manifest_puppet_from_bundle_dir( diff --git a/enroll/manifest_safety.py b/enroll/manifest_safety.py deleted file mode 100644 index 77baafc..0000000 --- a/enroll/manifest_safety.py +++ /dev/null @@ -1,170 +0,0 @@ -from __future__ import annotations - -import os -import shutil -import stat -from pathlib import Path -from typing import Iterator, Tuple - - -class ArtifactSafetyError(RuntimeError): - """Raised when a harvest artifact path is unsafe to consume.""" - - -class ManifestOutputError(RuntimeError): - """Raised when a manifest output path is unsafe to use.""" - - -def _safe_relative_path(value: str, *, field: str) -> Path: - text = str(value or "").strip() - if not text: - raise ArtifactSafetyError(f"empty {field}") - if "\x00" in text: - raise ArtifactSafetyError(f"{field} contains NUL byte: {text!r}") - p = Path(text) - if p.is_absolute(): - raise ArtifactSafetyError(f"{field} must be relative: {text!r}") - if any(part in {"", ".", ".."} for part in p.parts): - raise ArtifactSafetyError(f"{field} contains unsafe path component: {text!r}") - return p - - -def prepare_manifest_output_dir( - out_dir: str | Path, *, allow_existing: bool = False -) -> Path: - """Create a manifest output directory, refusing to overwrite anything. - - Rendering a manifest may be run by root and may target configuration- - management trees. Refuse an existing path rather than deleting or merging - with it by default; callers that intentionally support accumulation, such - as --fqdn site mode, may allow an existing directory but never a symlink or - non-directory path. - """ - - out = Path(out_dir).expanduser() - if os.path.lexists(out): - if not allow_existing: - raise ManifestOutputError( - "manifest output path already exists; refusing to overwrite: " f"{out}" - ) - st = out.lstat() - if stat.S_ISLNK(st.st_mode): - raise ManifestOutputError( - f"manifest output path is a symlink; refusing to use: {out}" - ) - if not out.is_dir(): - raise ManifestOutputError( - f"manifest output path exists but is not a directory: {out}" - ) - return out - out.mkdir(parents=True, exist_ok=False) - return out - - -def _assert_no_symlink_components(path: Path, *, root: Path) -> None: - """Reject symlinks in any existing path component between root and path.""" - - try: - rel = path.relative_to(root) - except ValueError as e: - raise ArtifactSafetyError(f"artifact path escapes artifact root: {path}") from e - - cur = root - for part in rel.parts: - cur = cur / part - try: - st = cur.lstat() - except FileNotFoundError: - # Missing components are handled by the final caller where relevant. - return - if stat.S_ISLNK(st.st_mode): - raise ArtifactSafetyError(f"artifact path contains symlink: {cur}") - - -def safe_artifact_file(bundle_dir: str | Path, role: str, src_rel: str) -> Path: - """Return a harvested artifact file path only if it is safe to copy. - - The path must remain under artifacts/, contain no absolute or '..' - components, contain no symlinks in any path component, and refer to a - regular, non-hardlinked file. This deliberately mirrors the tar extraction - hardening used for remote/SOPS/plain tarball bundles, but applies it to - directory bundles too. - """ - - role_path = _safe_relative_path(role, field="artifact role") - src_path = _safe_relative_path(src_rel, field="artifact src_rel") - - artifacts_root = Path(bundle_dir).expanduser() / "artifacts" - root = artifacts_root / role_path - candidate = root / src_path - - if artifacts_root.exists(): - st = artifacts_root.lstat() - if stat.S_ISLNK(st.st_mode): - raise ArtifactSafetyError( - f"artifacts directory is a symlink: {artifacts_root}" - ) - - if root.exists(): - _assert_no_symlink_components(root, root=artifacts_root) - - _assert_no_symlink_components(candidate, root=artifacts_root) - - try: - st = candidate.lstat() - except FileNotFoundError: - raise - - if stat.S_ISLNK(st.st_mode): - raise ArtifactSafetyError(f"artifact is a symlink: {candidate}") - if not stat.S_ISREG(st.st_mode): - raise ArtifactSafetyError(f"artifact is not a regular file: {candidate}") - if st.st_nlink > 1: - raise ArtifactSafetyError(f"artifact is hardlinked: {candidate}") - - resolved_root = artifacts_root.resolve(strict=True) - resolved_candidate = candidate.resolve(strict=True) - try: - resolved_candidate.relative_to(resolved_root) - except ValueError as e: - raise ArtifactSafetyError( - f"artifact path escapes artifact root: {candidate}" - ) from e - - return candidate - - -def iter_safe_artifact_files( - bundle_dir: str | Path, role: str -) -> Iterator[Tuple[Path, str]]: - """Yield safe artifact files for a role as (path, src_rel).""" - - role_path = _safe_relative_path(role, field="artifact role") - artifacts_dir = Path(bundle_dir).expanduser() / "artifacts" / role_path - if not artifacts_dir.exists(): - return - if not artifacts_dir.is_dir(): - raise ArtifactSafetyError( - f"artifact role path is not a directory: {artifacts_dir}" - ) - - for root, dirs, files in os.walk(artifacts_dir, followlinks=False): - root_p = Path(root) - for dirname in list(dirs): - p = root_p / dirname - try: - st = p.lstat() - except FileNotFoundError: - continue - if stat.S_ISLNK(st.st_mode): - raise ArtifactSafetyError(f"artifact directory is a symlink: {p}") - for filename in files: - p = root_p / filename - rel = p.relative_to(artifacts_dir).as_posix() - yield safe_artifact_file(bundle_dir, role, rel), rel - - -def copy_safe_artifact_file(src: str | Path, dst: str | Path) -> None: - """Copy an already validated artifact file without following symlinks.""" - - shutil.copy2(src, dst, follow_symlinks=False) diff --git a/enroll/puppet.py b/enroll/puppet.py index 1b78789..021ae5f 100644 --- a/enroll/puppet.py +++ b/enroll/puppet.py @@ -4,6 +4,7 @@ import hashlib import json import re import shlex +import shutil from pathlib import Path from typing import Any, Dict, Iterable, List, Optional, Set, Tuple @@ -15,11 +16,6 @@ from .cm import ( role_order_key, markdown_list, ) -from .manifest_safety import ( - copy_safe_artifact_file, - prepare_manifest_output_dir, - safe_artifact_file, -) from .state import inventory_packages_from_state, roles_from_state from .jinjaturtle import ( can_jinjify_path, @@ -632,14 +628,13 @@ def _copy_artifact( ) -> Optional[str]: if not role or not src_rel: return None - try: - src = safe_artifact_file(bundle_dir, role, src_rel) - except FileNotFoundError: + src = Path(bundle_dir) / "artifacts" / role / src_rel + if not src.is_file(): return None module_rel = Path(dst_prefix or "") / src_rel dst = dst_files_dir / module_rel dst.parent.mkdir(parents=True, exist_ok=True) - copy_safe_artifact_file(src, dst) + shutil.copy2(src, dst) return module_rel.as_posix() @@ -1717,8 +1712,10 @@ class PuppetManifestRenderer: no_common_roles = self.no_common_roles state = PuppetRole.load_state(bundle_dir) + out = Path(out_dir) hiera_mode = bool(fqdn) - out = prepare_manifest_output_dir(out_dir, allow_existing=hiera_mode) + if out.exists() and not hiera_mode: + shutil.rmtree(out) manifests_dir = out / "manifests" modules_dir = out / "modules" manifests_dir.mkdir(parents=True, exist_ok=True) diff --git a/enroll/salt.py b/enroll/salt.py index 27ec915..1b0e043 100644 --- a/enroll/salt.py +++ b/enroll/salt.py @@ -17,11 +17,6 @@ from .cm import ( markdown_list, ) from .jinjaturtle import jinjify_artifact, resolve_jinjaturtle_mode -from .manifest_safety import ( - copy_safe_artifact_file, - prepare_manifest_output_dir, - safe_artifact_file, -) from .state import inventory_packages_from_state, roles_from_state from .yamlutil import yaml_dump_mapping, yaml_load_mapping_file @@ -615,14 +610,13 @@ def _copy_artifact( ) -> Optional[str]: if not role or not src_rel: return None - try: - src = safe_artifact_file(bundle_dir, role, src_rel) - except FileNotFoundError: + src = Path(bundle_dir) / "artifacts" / role / src_rel + if not src.is_file(): return None role_rel = Path(dst_prefix or "") / src_rel dst = dst_files_dir / role_rel dst.parent.mkdir(parents=True, exist_ok=True) - copy_safe_artifact_file(src, dst) + shutil.copy2(src, dst) return role_rel.as_posix() @@ -1693,11 +1687,13 @@ class SaltManifestRenderer: def render(self) -> None: state = SaltRole.load_state(self.bundle_dir) - fqdn_mode = bool(self.fqdn) - out = prepare_manifest_output_dir(self.out_dir, allow_existing=fqdn_mode) + out = Path(self.out_dir) states_dir = out / "states" pillar_dir = out / "pillar" + fqdn_mode = bool(self.fqdn) + if out.exists() and not fqdn_mode: + shutil.rmtree(out) states_dir.mkdir(parents=True, exist_ok=True) if fqdn_mode: pillar_dir.mkdir(parents=True, exist_ok=True) diff --git a/enroll/validate.py b/enroll/validate.py index 60d7f7c..0a3e8cb 100644 --- a/enroll/validate.py +++ b/enroll/validate.py @@ -1,8 +1,6 @@ from __future__ import annotations import json -import os -import stat import urllib.request from dataclasses import dataclass from pathlib import Path @@ -11,7 +9,6 @@ from typing import Any, Dict, List, Optional, Set, Tuple import jsonschema from .diff import BundleRef, _bundle_from_input -from .manifest_safety import ArtifactSafetyError, safe_artifact_file from .state import load_state @@ -174,7 +171,7 @@ def validate_harvest( except Exception as e: # noqa: BLE001 errors.append(f"failed to load/validate schema: {e!r}") - # Artifact existence and safety checks. + # Artifact existence checks artifacts_dir = bundle.dir / "artifacts" referenced: Set[Tuple[str, str]] = set() for role_name, mf in _iter_managed_files(state): @@ -191,15 +188,15 @@ def validate_harvest( continue referenced.add((role_name, src_rel)) - try: - safe_artifact_file(bundle.dir, role_name, src_rel) - except FileNotFoundError: + p = artifacts_dir / role_name / src_rel + if not p.exists(): errors.append( f"missing artifact for role {role_name}: artifacts/{role_name}/{src_rel}" ) - except ArtifactSafetyError as e: + continue + if not p.is_file(): errors.append( - f"unsafe artifact for role {role_name}: artifacts/{role_name}/{src_rel}: {e}" + f"artifact is not a file for role {role_name}: artifacts/{role_name}/{src_rel}" ) # Runtime firewall snapshots are generated artifacts rather than managed files. @@ -214,83 +211,43 @@ def validate_harvest( f"firewall_runtime {key} has suspicious src_rel: {src_rel!r}" ) continue - role_name = str(fw.get("role_name") or "firewall_runtime") - referenced.add((role_name, src_rel)) - try: - safe_artifact_file(bundle.dir, role_name, src_rel) - except FileNotFoundError: + referenced.add( + (str(fw.get("role_name") or "firewall_runtime"), src_rel) + ) + p = ( + artifacts_dir + / str(fw.get("role_name") or "firewall_runtime") + / src_rel + ) + if not p.exists(): errors.append( "missing firewall runtime artifact: " - f"artifacts/{role_name}/{src_rel}" + f"artifacts/{fw.get('role_name') or 'firewall_runtime'}/{src_rel}" ) - except ArtifactSafetyError as e: + elif not p.is_file(): errors.append( - "unsafe firewall runtime artifact: " - f"artifacts/{role_name}/{src_rel}: {e}" + "firewall runtime artifact is not a file: " + f"artifacts/{fw.get('role_name') or 'firewall_runtime'}/{src_rel}" ) - # Validate the whole artifact tree too, so unreferenced symlinks, - # hardlinks, special files, and path-shaping tricks do not survive - # validation simply because no managed_file currently references them. + # Warn if there are extra files in artifacts not referenced. if artifacts_dir.exists() and artifacts_dir.is_dir(): - for root, dirs, files in os.walk(artifacts_dir, followlinks=False): - root_p = Path(root) - for name in list(dirs): - fp = root_p / name - try: - st = fp.lstat() - except FileNotFoundError: - continue - if stat.S_ISLNK(st.st_mode): - errors.append(f"artifact directory is a symlink: {fp}") - elif not stat.S_ISDIR(st.st_mode): - errors.append(f"artifact directory is not a directory: {fp}") - - for name in files: - fp = root_p / name - try: - st = fp.lstat() - except FileNotFoundError: - continue - try: - rel = fp.relative_to(artifacts_dir) - except ValueError: - errors.append(f"artifact escapes artifact root: {fp}") - continue - parts = rel.parts - if len(parts) < 2: - errors.append(f"artifact is not under a role directory: {fp}") - continue - role_name = parts[0] - src_rel = "/".join(parts[1:]) - - if stat.S_ISLNK(st.st_mode): - errors.append( - f"artifact is a symlink: artifacts/{role_name}/{src_rel}" - ) - continue - if not stat.S_ISREG(st.st_mode): - errors.append( - f"artifact is not a regular file: artifacts/{role_name}/{src_rel}" - ) - continue - if st.st_nlink > 1: - errors.append( - f"artifact is hardlinked: artifacts/{role_name}/{src_rel}" - ) - continue - try: - safe_artifact_file(bundle.dir, role_name, src_rel) - except (FileNotFoundError, ArtifactSafetyError) as e: - errors.append( - f"unsafe artifact: artifacts/{role_name}/{src_rel}: {e}" - ) - continue - - if (role_name, src_rel) not in referenced: - warnings.append( - f"unreferenced artifact present: artifacts/{role_name}/{src_rel}" - ) + for fp in artifacts_dir.rglob("*"): + if not fp.is_file(): + continue + try: + rel = fp.relative_to(artifacts_dir) + except ValueError: + continue + parts = rel.parts + if len(parts) < 2: + continue + role_name = parts[0] + src_rel = "/".join(parts[1:]) + if (role_name, src_rel) not in referenced: + warnings.append( + f"unreferenced artifact present: artifacts/{role_name}/{src_rel}" + ) return ValidationResult(errors=errors, warnings=warnings) finally: diff --git a/tests/__init__.py b/tests/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/state_helpers.py b/tests/state_helpers.py deleted file mode 100644 index 9cbca20..0000000 --- a/tests/state_helpers.py +++ /dev/null @@ -1,234 +0,0 @@ -from __future__ import annotations - -import copy -import json -from pathlib import Path -from typing import Any - -_VALID_REASON_FALLBACKS = { - "dangerous_user_dotfile": "user_shell_rc", - "possible_secret": "sensitive_content", -} - -_COMMON_ROLES = { - "users", - "apt_config", - "dnf_config", - "etc_custom", - "usr_local_custom", - "extra_paths", -} - - -def _common_role(name: str) -> dict[str, Any]: - out: dict[str, Any] = { - "role_name": name, - "managed_dirs": [], - "managed_files": [], - "excluded": [], - "notes": [], - } - if name == "users": - out["users"] = [] - if name == "extra_paths": - out["include_patterns"] = [] - out["exclude_patterns"] = [] - out["managed_links"] = [] - return out - - -def _normalise_managed_file(mf: dict[str, Any]) -> None: - reason = mf.get("reason") - if reason in _VALID_REASON_FALLBACKS: - mf["reason"] = _VALID_REASON_FALLBACKS[reason] - mf.setdefault("owner", "root") - mf.setdefault("group", "root") - mf.setdefault("mode", "0644") - mf.setdefault("reason", "modified_conffile") - - -def _normalise_managed_dir(md: dict[str, Any]) -> None: - md.setdefault("owner", "root") - md.setdefault("group", "root") - md.setdefault("mode", "0755") - if md.get("reason") in {None, "parent_dir"}: - md["reason"] = "parent_of_managed_file" - - -def _normalise_managed_link(ml: dict[str, Any]) -> None: - ml.setdefault("reason", "enabled_symlink") - - -def _normalise_common_role(role: dict[str, Any], name: str) -> None: - role.setdefault("role_name", name) - role.setdefault("managed_dirs", []) - role.setdefault("managed_files", []) - role.setdefault("excluded", []) - role.setdefault("notes", []) - for mf in role.get("managed_files") or []: - if isinstance(mf, dict): - _normalise_managed_file(mf) - for md in role.get("managed_dirs") or []: - if isinstance(md, dict): - _normalise_managed_dir(md) - for ml in role.get("managed_links") or []: - if isinstance(ml, dict): - _normalise_managed_link(ml) - for ex in role.get("excluded") or []: - if isinstance(ex, dict) and ex.get("reason") in _VALID_REASON_FALLBACKS: - ex["reason"] = _VALID_REASON_FALLBACKS[ex["reason"]] - - -def make_schema_valid_state(state: dict[str, Any]) -> dict[str, Any]: - """Return a current-schema harvest state from a compact renderer fixture. - - Many renderer tests intentionally build only the fields needed by the - renderer under test. Manifest now validates strictly before rendering, so - those fixtures need current-schema boilerplate too. - """ - - st = copy.deepcopy(state) - st.pop("schema_version", None) - - enroll = st.setdefault("enroll", {}) - enroll.setdefault("version", "0.0.test") - enroll.setdefault("harvest_time", 0) - - host = st.setdefault("host", {}) - host.setdefault("hostname", "testhost") - host.setdefault("os", "unknown") - host.setdefault("pkg_backend", "dpkg") - host.setdefault("os_release", {}) - - inv = st.setdefault("inventory", {}) - inv.setdefault("packages", {}) - for pkg in (inv.get("packages") or {}).values(): - if not isinstance(pkg, dict): - continue - pkg.setdefault("version", None) - pkg.setdefault("arches", []) - installations = pkg.setdefault("installations", []) - for inst in installations: - if isinstance(inst, dict): - inst.setdefault("version", str(pkg.get("version") or "1.0")) - inst.setdefault("arch", "amd64") - observed = pkg.setdefault("observed_via", []) - for ov in observed: - if isinstance(ov, dict) and ov.get("kind") not in { - "user_installed", - "systemd_unit", - "package_role", - "firewall_runtime", - }: - ov["kind"] = "package_role" - ov.setdefault("ref", "package") - pkg.setdefault("roles", []) - - roles = st.setdefault("roles", {}) - for name in _COMMON_ROLES: - cur = roles.get(name) - if not isinstance(cur, dict): - roles[name] = _common_role(name) - else: - _normalise_common_role(cur, name) - - roles.setdefault("services", []) - roles.setdefault("packages", []) - - users = roles.get("users") or {} - users.setdefault("users", []) - for user in users.get("users") or []: - if not isinstance(user, dict): - continue - user.setdefault("uid", 0) - user.setdefault("gid", user.get("uid", 0)) - user.setdefault("gecos", "") - user.setdefault("home", f"/home/{user.get('name', 'user')}") - user.setdefault("shell", "/bin/sh") - user.setdefault("primary_group", user.get("name", "users")) - user.setdefault("supplementary_groups", []) - - extra = roles.get("extra_paths") or {} - extra.setdefault("include_patterns", []) - extra.setdefault("exclude_patterns", []) - extra.setdefault("managed_links", []) - - for svc in roles.get("services") or []: - if not isinstance(svc, dict): - continue - _normalise_common_role(svc, str(svc.get("role_name") or "service_role")) - svc.setdefault("unit", "example.service") - svc.setdefault("packages", []) - svc.setdefault("active_state", None) - svc.setdefault("sub_state", None) - svc.setdefault("unit_file_state", None) - svc.setdefault("condition_result", None) - - for pkg in roles.get("packages") or []: - if not isinstance(pkg, dict): - continue - _normalise_common_role( - pkg, str(pkg.get("role_name") or pkg.get("package") or "package_role") - ) - pkg.setdefault("package", str(pkg.get("role_name") or "package")) - - if isinstance(roles.get("sysctl"), dict): - sysctl = roles["sysctl"] - sysctl.setdefault("role_name", "sysctl") - sysctl.setdefault("managed_files", []) - sysctl.setdefault("parameters", {}) - sysctl.setdefault("notes", []) - sysctl.pop("managed_dirs", None) - sysctl.pop("managed_links", None) - for mf in sysctl.get("managed_files") or []: - if isinstance(mf, dict): - _normalise_managed_file(mf) - - if isinstance(roles.get("firewall_runtime"), dict): - fw = roles["firewall_runtime"] - fw.setdefault("role_name", "firewall_runtime") - fw.setdefault("packages", []) - fw.setdefault("ipset_save", None) - fw.setdefault("ipset_sets", []) - fw.setdefault("iptables_v4_save", None) - fw.setdefault("iptables_v6_save", None) - fw.setdefault("notes", []) - - if isinstance(roles.get("flatpak"), dict): - roles["flatpak"].setdefault("role_name", "flatpak") - if isinstance(roles.get("snap"), dict): - roles["snap"].setdefault("role_name", "snap") - if isinstance(roles.get("container_images"), dict): - ci = roles["container_images"] - ci.setdefault("role_name", "container_images") - ci.setdefault("images", []) - ci.setdefault("notes", []) - for img in ci.get("images") or []: - if not isinstance(img, dict): - continue - img.setdefault("engine", "docker") - img.setdefault("scope", "system") - img.setdefault("user", None) - img.setdefault("home", None) - img.setdefault("image_id", None) - img.setdefault("repo_tags", []) - img.setdefault("repo_digests", []) - img.setdefault("pull_ref", None) - img.setdefault("tag_aliases", []) - img.setdefault("os", None) - img.setdefault("architecture", None) - img.setdefault("variant", None) - img.setdefault("platform", None) - img.setdefault("size", None) - img.setdefault("created", None) - img.setdefault("source", "test") - img.setdefault("notes", []) - - return st - - -def write_schema_state(bundle: Path, state: dict[str, Any]) -> None: - bundle.mkdir(parents=True, exist_ok=True) - (bundle / "state.json").write_text( - json.dumps(make_schema_valid_state(state), indent=2), encoding="utf-8" - ) diff --git a/tests/test_cli_config_and_sops.py b/tests/test_cli_config_and_sops.py index 958183d..7e3fe5b 100644 --- a/tests/test_cli_config_and_sops.py +++ b/tests/test_cli_config_and_sops.py @@ -23,10 +23,10 @@ def test_discover_config_path_precedence(monkeypatch, tmp_path: Path): assert _discover_config_path(["harvest"]) == cfg -def test_discover_config_path_ignores_local_and_finds_xdg(monkeypatch, tmp_path: Path): +def test_discover_config_path_finds_local_and_xdg(monkeypatch, tmp_path: Path): from enroll.cli import _discover_config_path - # local files in cwd are deliberately ignored unless passed via --config + # local file in cwd cwd = tmp_path / "cwd" cwd.mkdir() local = cwd / "enroll.ini" @@ -35,8 +35,7 @@ def test_discover_config_path_ignores_local_and_finds_xdg(monkeypatch, tmp_path: monkeypatch.chdir(cwd) monkeypatch.delenv("ENROLL_CONFIG", raising=False) monkeypatch.delenv("XDG_CONFIG_HOME", raising=False) - assert _discover_config_path(["harvest"]) is None - assert _discover_config_path(["--config", str(local), "harvest"]) == local + assert _discover_config_path(["harvest"]) == local # xdg config fallback monkeypatch.chdir(tmp_path) diff --git a/tests/test_cli_helpers.py b/tests/test_cli_helpers.py index dab28b4..264ff85 100644 --- a/tests/test_cli_helpers.py +++ b/tests/test_cli_helpers.py @@ -8,7 +8,7 @@ from pathlib import Path def test_discover_config_path_precedence(tmp_path: Path, monkeypatch): - """_discover_config_path: --config > ENROLL_CONFIG > XDG.""" + """_discover_config_path: --config > ENROLL_CONFIG > ./enroll.ini > XDG.""" from enroll.cli import _discover_config_path cfg1 = tmp_path / "one.ini" @@ -27,14 +27,14 @@ def test_discover_config_path_precedence(tmp_path: Path, monkeypatch): monkeypatch.setenv("ENROLL_CONFIG", str(cfg2)) assert _discover_config_path([]) == cfg2 - # Local ./enroll.ini is ignored unless passed explicitly. + # Local ./enroll.ini fallback. monkeypatch.delenv("ENROLL_CONFIG", raising=False) local = tmp_path / "enroll.ini" local.write_text("[enroll]\n", encoding="utf-8") - assert _discover_config_path([]) is None - assert _discover_config_path(["--config", str(local)]) == local + assert _discover_config_path([]) == local # XDG fallback. + local.unlink() xdg = tmp_path / "xdg" cfg3 = xdg / "enroll" / "enroll.ini" cfg3.parent.mkdir(parents=True) diff --git a/tests/test_diff_ignore_versions_exclude_enforce.py b/tests/test_diff_ignore_versions_exclude_enforce.py index 89e7d7c..d08c60e 100644 --- a/tests/test_diff_ignore_versions_exclude_enforce.py +++ b/tests/test_diff_ignore_versions_exclude_enforce.py @@ -244,7 +244,6 @@ def test_enforce_old_harvest_runs_ansible_with_tags_from_file_drift( # Stub manifest generation to only create playbook.yml (fast, no real roles needed). def fake_manifest(_harvest_dir: str, out_dir: str, **_kwargs): out = Path(out_dir) - out.mkdir(parents=True, exist_ok=False) (out / "playbook.yml").write_text( "---\n- hosts: all\n gather_facts: false\n roles: []\n", encoding="utf-8", @@ -364,10 +363,7 @@ def test_enforce_old_harvest_runs_puppet_target(monkeypatch, tmp_path: Path): argv = calls.get("argv") assert argv and argv[:2] == ["/usr/bin/puppet", "apply"] assert "--modulepath" in argv - assert any( - str(Path(calls["cwd"]) / "manifest" / "manifests" / "site.pp") == str(a) - for a in argv - ) + assert str(Path(calls["cwd"]) / "manifests" / "site.pp") in argv def test_enforce_old_harvest_runs_salt_target(monkeypatch, tmp_path: Path): @@ -422,7 +418,7 @@ def test_enforce_old_harvest_runs_salt_target(monkeypatch, tmp_path: Path): assert "--local" in argv assert "--file-root" in argv assert "state.apply" in argv - assert str(Path(calls["cwd"]) / "manifest" / "states") in argv + assert str(Path(calls["cwd"]) / "states") in argv def test_cli_diff_enforce_forwards_target(monkeypatch): diff --git a/tests/test_diff_notifications.py b/tests/test_diff_notifications.py index 9a433b4..53f6b57 100644 --- a/tests/test_diff_notifications.py +++ b/tests/test_diff_notifications.py @@ -81,42 +81,3 @@ def test_send_email_raises_when_no_delivery_method(monkeypatch): from_addr="a@example.com", to_addrs=["b@example.com"], ) - - -def test_send_email_refuses_smtp_auth_without_starttls(monkeypatch): - from enroll.diff import send_email - - class FakeSMTP: - def __init__(self, *_args, **_kwargs): - pass - - def __enter__(self): - return self - - def __exit__(self, exc_type, exc, tb): - return False - - def ehlo(self): - pass - - def starttls(self): - raise RuntimeError("no starttls") - - def login(self, *_args): - raise AssertionError("login should not be called without TLS") - - def send_message(self, *_args): - raise AssertionError("message should not be sent without TLS") - - monkeypatch.setattr("smtplib.SMTP", FakeSMTP) - - with pytest.raises(RuntimeError, match="STARTTLS failed"): - send_email( - subject="Subj", - body="Body", - from_addr="a@example.com", - to_addrs=["b@example.com"], - smtp="smtp.example.com:587", - smtp_user="user", - smtp_password="secret", - ) diff --git a/tests/test_jinjaturtle.py b/tests/test_jinjaturtle.py index e363193..e4198c7 100644 --- a/tests/test_jinjaturtle.py +++ b/tests/test_jinjaturtle.py @@ -1,7 +1,6 @@ +import json from pathlib import Path -from tests.state_helpers import write_schema_state - import enroll.manifest as manifest_mod import enroll.jinjaturtle as jinjaturtle_mod from enroll.jinjaturtle import JinjifyResult @@ -104,7 +103,7 @@ def test_manifest_uses_jinjaturtle_templates_and_does_not_copy_raw( } bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") # Pretend jinjaturtle exists. monkeypatch.setattr( diff --git a/tests/test_manifest.py b/tests/test_manifest.py index b8898cd..dba3d24 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -1,3 +1,4 @@ +import json from pathlib import Path import os @@ -6,7 +7,6 @@ import tarfile import pytest import enroll.manifest as manifest -from tests.state_helpers import write_schema_state import enroll.jinjaturtle as jinjaturtle_mod from enroll import ansible as ansible_layout from enroll import ansible as ansible_tasks @@ -84,7 +84,8 @@ def _minimal_package_state(packages): def _write_state(bundle: Path, state: dict) -> None: - write_schema_state(bundle, state) + bundle.mkdir(parents=True, exist_ok=True) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") def test_manifest_writes_roles_and_playbook_with_clean_when(tmp_path: Path): @@ -229,7 +230,7 @@ def test_manifest_writes_roles_and_playbook_with_clean_when(tmp_path: Path): } bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") # Create artifact for etc_custom file so copy works (bundle / "artifacts" / "etc_custom" / "etc" / "default").mkdir( @@ -935,7 +936,7 @@ def test_manifest_site_mode_creates_host_inventory_and_raw_files(tmp_path: Path) } bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") # Artifacts for usr_local_custom file so copy works. (bundle / "artifacts" / "usr_local_custom" / "usr" / "local" / "etc").mkdir( @@ -1086,7 +1087,7 @@ def test_manifest_includes_dnf_config_role_when_present(tmp_path: Path): } bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") manifest.manifest(str(bundle), str(out)) @@ -1223,7 +1224,7 @@ def test_manifest_orders_cron_and_logrotate_at_playbook_tail(tmp_path: Path): ) bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") manifest.manifest(str(bundle), str(out)) @@ -1362,7 +1363,9 @@ def test_manifest_applies_jinjaturtle_to_jinjifyable_managed_file( }, }, } - write_schema_state(bundle, state) + (bundle / "state.json").write_text( + __import__("json").dumps(state), encoding="utf-8" + ) monkeypatch.setattr(jinjaturtle_mod, "find_jinjaturtle_cmd", lambda: "jinjaturtle") @@ -1460,7 +1463,7 @@ def test_manifest_writes_firewall_runtime_role(tmp_path: Path): }, }, } - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") manifest.manifest(str(bundle), str(out)) @@ -1765,7 +1768,7 @@ def test_manifest_renders_flatpak_and_snap_details(tmp_path: Path): }, } bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") manifest.manifest(str(bundle), str(out)) @@ -1841,7 +1844,7 @@ def test_users_role_without_portable_apps_omits_community_general_tasks(tmp_path }, } bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") manifest.manifest(str(bundle), str(out)) @@ -1919,7 +1922,7 @@ def test_users_role_only_creates_ssh_dir_when_managed_ssh_files_exist(tmp_path): }, } bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") manifest.manifest(str(bundle), str(out)) @@ -1969,7 +1972,7 @@ def test_manifest_emits_flatpak_role_even_when_no_flatpaks(tmp_path): } } bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") manifest.manifest(str(bundle), str(out)) @@ -2036,7 +2039,7 @@ def test_manifest_avoids_package_role_collision_with_flatpak_singleton(tmp_path) } } bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") manifest.manifest(str(bundle), str(out), no_common_roles=True) @@ -2132,7 +2135,7 @@ def test_manifest_writes_sysctl_role(tmp_path: Path): }, }, } - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") manifest.manifest(str(bundle), str(out)) @@ -2234,7 +2237,7 @@ def test_manifest_renders_container_image_role_for_ansible(tmp_path: Path): } } bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") manifest.manifest(str(bundle), str(out)) @@ -2311,7 +2314,7 @@ def test_manifest_writes_container_images_to_hostvars_in_fqdn_mode(tmp_path: Pat } } bundle.mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") manifest.manifest(str(bundle), str(out), fqdn="host.example.test") @@ -2326,14 +2329,3 @@ def test_manifest_writes_container_images_to_hostvars_in_fqdn_mode(tmp_path: Pat assert "container_images: []" in defaults assert digest in hostvars assert "role: container_images" in playbook - - -def test_manifest_non_fqdn_refuses_existing_output(tmp_path: Path): - bundle = tmp_path / "bundle" - out = tmp_path / "ansible" - bundle.mkdir(parents=True) - out.mkdir() - write_schema_state(bundle, _minimal_package_state([])) - - with pytest.raises(RuntimeError, match="already exists"): - manifest.manifest(str(bundle), str(out), no_common_roles=True) diff --git a/tests/test_manifest_puppet.py b/tests/test_manifest_puppet.py index 24f7b5a..74f0dc6 100644 --- a/tests/test_manifest_puppet.py +++ b/tests/test_manifest_puppet.py @@ -5,8 +5,6 @@ from pathlib import Path import yaml -from tests.state_helpers import write_schema_state - from enroll import manifest from enroll.puppet import ( PuppetRole, @@ -17,7 +15,8 @@ from enroll.puppet import ( def _write_state(bundle: Path, state: dict) -> None: - write_schema_state(bundle, state) + bundle.mkdir(parents=True, exist_ok=True) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") def test_manifest_puppet_writes_control_repo_style_output(tmp_path: Path): diff --git a/tests/test_manifest_salt.py b/tests/test_manifest_salt.py index f629b36..9a35c2b 100644 --- a/tests/test_manifest_salt.py +++ b/tests/test_manifest_salt.py @@ -1,12 +1,11 @@ from __future__ import annotations +import json from collections import OrderedDict from pathlib import Path import yaml -from tests.state_helpers import write_schema_state - from enroll import manifest from enroll.salt import ( SaltRole, @@ -19,7 +18,8 @@ from enroll.salt import ( def _write_state(bundle: Path, state: dict) -> None: - write_schema_state(bundle, state) + bundle.mkdir(parents=True, exist_ok=True) + (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8") def _sample_state() -> dict: diff --git a/tests/test_manifest_symlinks.py b/tests/test_manifest_symlinks.py index 051a009..39ef9a0 100644 --- a/tests/test_manifest_symlinks.py +++ b/tests/test_manifest_symlinks.py @@ -1,7 +1,6 @@ +import json from pathlib import Path -from tests.state_helpers import write_schema_state - import enroll.manifest as manifest @@ -93,7 +92,7 @@ def test_manifest_emits_symlink_tasks_and_vars(tmp_path: Path): bundle.mkdir(parents=True, exist_ok=True) (bundle / "artifacts").mkdir(parents=True, exist_ok=True) - write_schema_state(bundle, state) + (bundle / "state.json").write_text(json.dumps(state), encoding="utf-8") manifest.manifest(str(bundle), str(out)) diff --git a/tests/test_validate.py b/tests/test_validate.py index 5ac33c9..05ee88b 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -411,45 +411,3 @@ def test_validate_harvest_no_schema_option(tmp_path: Path): result = validate_harvest(str(bundle_dir), no_schema=True) assert result.ok is False assert any("failed to parse" in e for e in result.errors) - - -def test_validate_harvest_rejects_artifact_symlink(tmp_path: Path): - bundle_dir = tmp_path / "bundle" - artifact = bundle_dir / "artifacts" / "users" / "etc" / "shadow" - artifact.parent.mkdir(parents=True) - artifact.symlink_to("/etc/shadow") - (bundle_dir / "state.json").write_text( - json.dumps( - { - "roles": { - "users": { - "managed_files": [ - {"path": "/etc/shadow", "src_rel": "etc/shadow"} - ] - } - } - } - ), - encoding="utf-8", - ) - - result = validate_harvest(str(bundle_dir), no_schema=True) - - assert result.ok is False - assert any("symlink" in e for e in result.errors) - - -def test_validate_harvest_rejects_unreferenced_artifact_symlink(tmp_path: Path): - bundle_dir = tmp_path / "bundle" - artifact = bundle_dir / "artifacts" / "users" / "etc" / "shadow" - artifact.parent.mkdir(parents=True) - artifact.symlink_to("/etc/shadow") - (bundle_dir / "state.json").write_text( - json.dumps({"roles": {"users": {"managed_files": []}}}), - encoding="utf-8", - ) - - result = validate_harvest(str(bundle_dir), no_schema=True) - - assert result.ok is False - assert any("symlink" in e for e in result.errors)