DEVELOPMENT.md

@@ -376,23 +376,14 @@ Puppet and Salt also run `cm.resolve_catalog_conflicts()` after renderer role co
 capture_file(abs_path, role_name, reason, policy, path_filter, ...)
   -> skip if already seen globally or in this role
   -> skip if --exclude-path matches
-  -> ask IgnorePolicy.inspect_file(abs_path)
-     -> open the source through fsutil.open_no_follow_path()
-     -> reject symlinks in any path component, not only the leaf
-     -> fstat() the opened descriptor
-     -> reject non-regular or over-size files
-     -> read the exact bytes from that descriptor
-     -> scan those bytes for binary/secret content unless dangerous
-  -> use the inspected fstat() for owner/group/mode metadata
-  -> write the inspected bytes to artifacts/<role_name>/<abs_path without leading slash>
-     with no-follow destination creation
+  -> ask IgnorePolicy.deny_reason(abs_path)
+  -> stat owner/group/mode with fsutil.stat_triplet()
+  -> copy to artifacts/<role_name>/<abs_path without leading slash>
   -> append ManagedFile
   -> mark seen in role/global
 ```
 
-This ordering is intentional. Enroll should not scan one file and later copy a different file after a race. When `IgnorePolicy.inspect_file()` succeeds, `capture_file()` writes the exact bytes that were inspected and uses the same descriptor's stat metadata.
-
-`fsutil.stat_triplet()` and `stat_triplet_from_stat()` return owner, group, and a zero-padded octal mode string. They fall back to numeric uid/gid strings if user/group names cannot be resolved.
+`fsutil.stat_triplet()` returns owner, group, and a zero-padded octal mode string. It falls back to numeric uid/gid strings if user/group names cannot be resolved.
 
 ### 7.2 `capture_link()`
 
@@ -459,20 +450,6 @@ regex:^/path/...$  regex
 
 `expand_includes()` is conservative: it ignores symlinks, respects excludes, caps file counts, and returns notes for unmatched patterns or caps.
 
-### 7.6 Output, artifact, and cache safety helpers
-
-Several safety helpers protect privileged runs from following attacker-controlled paths:
-
-- `fsutil.open_no_follow_path()` opens source and artifact paths component-by-component, rejecting symlinked parent directories as well as symlinked leaf files.
-- `harvest_safety.prepare_new_private_dir()` is used for user-facing plaintext output directories such as `harvest --out` and default manifest output; it refuses existing final paths and creates `0700` directories.
-- `harvest_safety.ensure_safe_output_parent()` is used when writing output files such as reports or encrypted SOPS bundles. It validates parents before staging a temporary file and atomically replacing the final path.
-- `harvest_safety.ensure_private_dir()` is used for persistent internal directories such as Enroll's cache root. Existing directories are allowed, but symlink components and unsafe root-run parents are refused.
-- `cache.new_harvest_cache_dir()` creates unpredictable per-harvest cache directories beneath the hardened cache root with `mkdtemp()` and private permissions.
-- `manifest_safety.safe_artifact_file()` validates referenced harvested artifacts before renderers copy them. It rejects absolute or `..` paths, symlinks, non-regular files, hardlinks, and paths that resolve outside the artifact root.
-- `manifest_safety.prepare_manifest_output_dir()` refuses unsafe manifest output paths. In `--fqdn` site mode, where an existing tree is intentionally reused, it walks the existing output tree and refuses symlinks before merging generated files.
-
-When adding a new code path that writes plaintext host state, prefer these helpers over raw `mkdir(parents=True)`, `open()`, `shutil.copy*()`, or `tar.extract*()`.
-
 ---
 
 ## 8. Platform and package backends
@@ -811,7 +788,8 @@ SOPS mode:
 
 The renderers do not know about SOPS.
 
-Before dispatching to a renderer, `manifest.manifest()` calls `validate.validate_harvest()` with normal schema validation enabled. That means generated configuration-management code is only rendered after Enroll has checked the bundle schema, referenced artifact existence, and artifact safety. If validation fails, manifest generation stops rather than trying to produce best-effort output from a malformed or tampered bundle.
+Note: Manifest deliberately hooks into validate() to make sure the harvest meets the schema and
+doesn't contain dangerous tamperings before turning it into config management code.
 
 ---
 
@@ -1402,24 +1380,17 @@ This is intended to answer “what did Enroll collect and why?”
 1. `state.json` exists,
 2. it parses as JSON,
 3. it validates against the vendored schema unless `--no-schema` is set,
-4. every `managed_file.src_rel` is relative and points to a safe artifact file,
-5. firewall runtime generated artifacts exist and are safe,
-6. the top-level `artifacts/` path is a real directory rather than a symlink or file,
-7. the whole artifact tree contains no symlink directories, symlink files, hardlinks, special files, or paths that escape the artifact root,
-8. unreferenced artifact files are reported as warnings.
-
-`validate_harvest()` is used in three important contexts:
-
-- `enroll validate` exposes the checks directly to users.
-- `manifest.manifest()` validates before rendering Ansible/Puppet/Salt output.
-- `diff.compare_harvests()` validates both input bundles before comparing them, using `no_schema=True` so older harvests can still be inspected while artifact safety checks remain active.
-
-`diff --enforce` renders the old harvest through `manifest.manifest()`, so enforcement also passes through manifest-time validation before a local apply tool is invoked.
+4. every `managed_file.src_rel` points to an artifact file,
+5. firewall runtime generated artifacts exist,
+6. there are no unreferenced artifact files, reported as warnings.
+7. there are no malicious or unsafe bits such as symlinks/hardlinks etc traversing out of the artifact tree
 
 It returns a `ValidationResult` with `errors`, `warnings`, `ok()`, `to_dict()`, and `to_text()`.
 
 The CLI supports local schema override with `--schema`, warning failure with `--fail-on-warnings`, JSON/text output, and `--out`.
 
+Note that manifest() hooks into validate() to make sure the harvest is safe before rendering it into config management code.
+
 ---
 
 ## 19. Remote harvesting
@@ -1486,7 +1457,7 @@ Unknown host keys are rejected by default through Paramiko's reject policy. User
 - device nodes,
 - anything resolving outside the destination.
 
-This helper is reused by remote harvest, manifest SOPS extraction, validate/diff bundle resolution, and any code path that needs to unpack a harvest tarball. Do not use raw `tar.extractall()` for user- or remote-provided bundles.
+This helper is reused by remote harvest, manifest SOPS extraction, and diff bundle resolution.
 
 ---
 

enroll/cache.py

@@ -8,8 +8,6 @@ from datetime import datetime
 from pathlib import Path
 from typing import Optional
 
-from .harvest_safety import OutputSafetyError, ensure_private_dir
-
 
 def _safe_component(s: str) -> str:
     s = s.strip()
@@ -46,17 +44,16 @@ class HarvestCache:
 
 
 def _ensure_dir_secure(path: Path) -> None:
-    """Create a private cache directory with output-path safety checks.
-
-    Cache roots are persistent, so existing directories are allowed, but they
-    still need the same symlink-component and root-parent trust checks as
-    plaintext harvest/manifest output paths.
-    """
-
+    """Create a directory with restrictive permissions; refuse symlinks."""
+    # Refuse a symlink at the leaf.
+    if path.exists() and path.is_symlink():
+        raise RuntimeError(f"Refusing to use symlink path: {path}")
+    path.mkdir(parents=True, exist_ok=True, mode=0o700)
     try:
-        ensure_private_dir(path, label="cache directory")
-    except OutputSafetyError as e:
-        raise RuntimeError(str(e)) from e
+        os.chmod(path, 0o700)
+    except OSError:
+        # Best-effort; on some FS types chmod may fail.
+        pass
 
 
 def new_harvest_cache_dir(*, hint: Optional[str] = None) -> HarvestCache:

enroll/harvest_safety.py

@@ -69,8 +69,6 @@ def _assert_existing_output_dir_component(path: Path, *, label: str) -> None:
         raise OutputSafetyError(
             f"{label} parent path contains a symlink; refusing: {path}"
         )
-    if not stat.S_ISDIR(st.st_mode):
-        raise OutputSafetyError(f"{label} parent is not a directory: {path}")
     _assert_trusted_root_parent(path, st, label=label)
 
 
@@ -216,24 +214,6 @@ def write_text_output_file(
     return out
 
 
-def ensure_private_dir(path: str | Path, *, label: str = "output") -> Path:
-    """Create or validate a private directory without requiring it to be empty.
-
-    This is for persistent internal directories such as Enroll's cache root,
-    where existing contents are expected across runs. It uses the same
-    component-by-component symlink and root-parent trust checks as user-facing
-    plaintext output directories, but permits an existing final directory.
-    """
-
-    out = Path(path).expanduser()
-    sentinel = out / ".enroll-private-dir-check"
-    _assert_no_existing_symlink_components(sentinel, label=label)
-    out = _mkdir_private_dir_tree(out, label=label, final_must_be_new=False)
-    _assert_no_existing_symlink_components(sentinel, label=label)
-    _chmod_private(out)
-    return out
-
-
 def prepare_new_private_dir(path: str | Path, *, label: str = "output") -> Path:
     """Create a brand-new private output directory.
 

enroll/validate.py

@@ -232,17 +232,7 @@ def validate_harvest(
         # 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.
-        if artifacts_dir.exists():
-            try:
-                artifacts_st = artifacts_dir.lstat()
-            except OSError as e:
-                errors.append(f"unable to inspect artifacts directory: {e}")
-            else:
-                if stat.S_ISLNK(artifacts_st.st_mode):
-                    errors.append(f"artifacts directory is a symlink: {artifacts_dir}")
-                elif not stat.S_ISDIR(artifacts_st.st_mode):
-                    errors.append(f"artifacts path is not a directory: {artifacts_dir}")
-                else:
+        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):
@@ -254,9 +244,7 @@ def validate_harvest(
                     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}"
-                                )
+                        errors.append(f"artifact directory is not a directory: {fp}")
 
                 for name in files:
                     fp = root_p / name
@@ -271,9 +259,7 @@ def validate_harvest(
                         continue
                     parts = rel.parts
                     if len(parts) < 2:
-                                errors.append(
-                                    f"artifact is not under a role directory: {fp}"
-                                )
+                        errors.append(f"artifact is not under a role directory: {fp}")
                         continue
                     role_name = parts[0]
                     src_rel = "/".join(parts[1:])

tests/test_cache_security.py

@@ -95,44 +95,3 @@ def test_enroll_cache_dir_uses_default_when_xdg_not_set(monkeypatch):
     monkeypatch.delenv("XDG_CACHE_HOME", raising=False)
     result = enroll_cache_dir()
     assert str(result).endswith("/.local/cache/enroll")
-
-
-def test_ensure_dir_secure_refuses_symlink_parent(tmp_path: Path):
-    from enroll.cache import _ensure_dir_secure
-
-    target = tmp_path / "target"
-    target.mkdir()
-    link = tmp_path / "link"
-    link.symlink_to(target, target_is_directory=True)
-
-    with pytest.raises(RuntimeError, match="symlink"):
-        _ensure_dir_secure(link / "enroll" / "harvest")
-
-    assert not (target / "enroll" / "harvest").exists()
-
-
-def test_ensure_dir_secure_rejects_unsafe_root_parent(tmp_path: Path, monkeypatch):
-    from enroll.cache import _ensure_dir_secure
-    import enroll.harvest_safety as hs
-
-    untrusted = tmp_path / "untrusted"
-    untrusted.mkdir()
-    untrusted.chmod(0o777)
-
-    monkeypatch.setattr(hs, "_effective_uid", lambda: 0)
-    with pytest.raises(RuntimeError, match="not owned by root|writable by group/other"):
-        _ensure_dir_secure(untrusted / "cache")
-
-
-def test_ensure_dir_secure_rejects_existing_file_when_not_root(
-    tmp_path: Path, monkeypatch
-):
-    from enroll.cache import _ensure_dir_secure
-    import enroll.harvest_safety as hs
-
-    path = tmp_path / "cache"
-    path.write_text("not a dir", encoding="utf-8")
-
-    monkeypatch.setattr(hs, "_effective_uid", lambda: 1000)
-    with pytest.raises(RuntimeError, match="not a directory"):
-        _ensure_dir_secure(path)

tests/test_validate.py

@@ -453,35 +453,3 @@ def test_validate_harvest_rejects_unreferenced_artifact_symlink(tmp_path: Path):
 
     assert result.ok is False
     assert any("symlink" in e for e in result.errors)
-
-
-def test_validate_harvest_rejects_top_level_artifacts_symlink(tmp_path: Path):
-    bundle_dir = tmp_path / "bundle"
-    bundle_dir.mkdir()
-    target = tmp_path / "artifact-target"
-    target.mkdir()
-    (bundle_dir / "artifacts").symlink_to(target, target_is_directory=True)
-    (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("artifacts directory is a symlink" in e for e in result.errors)
-
-
-def test_validate_harvest_rejects_top_level_artifacts_file(tmp_path: Path):
-    bundle_dir = tmp_path / "bundle"
-    bundle_dir.mkdir()
-    (bundle_dir / "artifacts").write_text("not a directory", encoding="utf-8")
-    (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("artifacts path is not a directory" in e for e in result.errors)