Metadata: Tighten to prefer stat taken from the no-follow descriptor that was inspected, to avoid tiny TOCTOU. Ensure schema forbids .. in src_rel (even though caught in validate anyway - defense in depth)
All checks were successful
CI / test (push) Successful in 50s
CI / test (almalinux, docker.io/library/almalinux:9, python3.11) (push) Successful in 11m11s
CI / test (debian, docker.io/library/debian:13, python3) (push) Successful in 17m36s
Lint / test (push) Successful in 51s

This commit is contained in:
Miguel Jacq 2026-07-01 15:52:04 +10:00
parent c6e171e0f0
commit 47c2c8accc
Signed by: mig5
GPG key ID: 03906B4110AAD3B8
3 changed files with 91 additions and 4 deletions

View file

@ -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:

View file

@ -411,7 +411,7 @@
},
"src_rel": {
"minLength": 1,
"pattern": "^[^/].*",
"pattern": "^(?!\\.\\.?(?:/|$))[^/\\u0000-\\u001f\\\\]+(?:/(?!\\.\\.?(?:/|$))[^/\\u0000-\\u001f\\\\]+)*$",
"type": "string"
}
},

View file

@ -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")