diff --git a/enroll/capture.py b/enroll/capture.py index 78ecc47..dd95a3d 100644 --- a/enroll/capture.py +++ b/enroll/capture.py @@ -170,10 +170,22 @@ def capture_file( return False try: - if metadata is not None: - owner, group, mode = metadata - elif inspection is not None: + if inspection is not None: + # Prefer the stat taken from the no-follow descriptor that was + # actually inspected and whose bytes are about to be written. A + # caller-supplied ``metadata`` triplet (see below) may have been + # derived from a separate, symlink-following stat with its own + # time-of-check/time-of-use window, so it must not override the + # authoritative descriptor stat when we have one. owner, group, mode = stat_triplet_from_stat(inspection.stat_result) + elif metadata is not None: + # Fallback for callers that pre-computed metadata and are not using a + # real IgnorePolicy that returns a FileInspection (e.g. tests, or the + # /usr/local scanner when inspection is unavailable). This value is a + # convenience/perf optimisation only; it never widens what gets + # captured, since inspection (secret scan, no-follow, size/hardlink + # checks) has already run above. + owner, group, mode = metadata else: owner, group, mode = stat_triplet(abs_path) except OSError: diff --git a/enroll/schema/state.schema.json b/enroll/schema/state.schema.json index 8806b2e..c1412fb 100644 --- a/enroll/schema/state.schema.json +++ b/enroll/schema/state.schema.json @@ -411,7 +411,7 @@ }, "src_rel": { "minLength": 1, - "pattern": "^[^/].*", + "pattern": "^(?!\\.\\.?(?:/|$))[^/\\u0000-\\u001f\\\\]+(?:/(?!\\.\\.?(?:/|$))[^/\\u0000-\\u001f\\\\]+)*$", "type": "string" } }, diff --git a/tests/test_harvest_helpers.py b/tests/test_harvest_helpers.py index 07ae690..77aa71c 100644 --- a/tests/test_harvest_helpers.py +++ b/tests/test_harvest_helpers.py @@ -396,3 +396,78 @@ def test_collect_sysctl_snapshot_writes_generated_artifact(monkeypatch, tmp_path text = conf.read_text(encoding="utf-8") assert "net.ipv4.ip_forward = 1" in text assert "vm.swappiness = 10" in text + + +def test_capture_file_prefers_inspected_stat_over_metadata(tmp_path): + """When a real IgnorePolicy inspection is available, capture_file must record + owner/group/mode from the no-follow descriptor it actually inspected, not + from a caller-supplied ``metadata`` triplet (which may come from a separate, + symlink-following stat with its own TOCTOU window).""" + + import os + + from enroll.capture import capture_file + from enroll.ignore import IgnorePolicy + from enroll.pathfilter import PathFilter + from enroll.harvest_types import ExcludedFile, ManagedFile + + src = tmp_path / "conf" + src.write_text("key = value\n", encoding="utf-8") + os.chmod(src, 0o640) + bundle = tmp_path / "bundle" + bundle.mkdir() + + managed: list[ManagedFile] = [] + excluded: list[ExcludedFile] = [] + ok = capture_file( + bundle_dir=str(bundle), + role_name="r", + abs_path=str(src), + reason="custom_unowned", + policy=IgnorePolicy(), + path_filter=PathFilter([], []), + managed_out=managed, + excluded_out=excluded, + metadata=("BOGUSOWNER", "BOGUSGROUP", "7777"), + ) + assert ok + mf = managed[0] + # The inspected descriptor's real mode wins over the bogus supplied metadata. + assert mf.mode == "0640" + assert mf.owner != "BOGUSOWNER" + assert mf.group != "BOGUSGROUP" + + +def test_capture_file_metadata_used_when_no_inspection(tmp_path): + """A caller-supplied metadata triplet is still honoured as a fallback for + non-real policies that expose only deny_reason() (tests/legacy callers).""" + + from enroll.capture import capture_file + from enroll.pathfilter import PathFilter + from enroll.harvest_types import ExcludedFile, ManagedFile + + class _DenyReasonOnlyPolicy: + def deny_reason(self, _path): + return None + + src = tmp_path / "conf" + src.write_text("x\n", encoding="utf-8") + bundle = tmp_path / "bundle" + bundle.mkdir() + + managed: list[ManagedFile] = [] + excluded: list[ExcludedFile] = [] + ok = capture_file( + bundle_dir=str(bundle), + role_name="r", + abs_path=str(src), + reason="custom_unowned", + policy=_DenyReasonOnlyPolicy(), + path_filter=PathFilter([], []), + managed_out=managed, + excluded_out=excluded, + metadata=("myowner", "mygroup", "0600"), + ) + assert ok + mf = managed[0] + assert (mf.owner, mf.group, mf.mode) == ("myowner", "mygroup", "0600")