hardlinked source files are refused, remote schema URLs require an explicit flag, generated harvest artifacts use the hardened bundle writer, and task/handler/playbook YAML writes go through one safety gate.

This commit is contained in:
Miguel Jacq 2026-06-28 18:17:45 +10:00
parent f56f076765
commit 44d1a22db4
Signed by: mig5
GPG key ID: 03906B4110AAD3B8
11 changed files with 202 additions and 37 deletions

View file

@ -186,7 +186,11 @@ def _read_first_existing_text(
try: try:
fd = open_no_follow_path(path) fd = open_no_follow_path(path)
st = os.fstat(fd) st = os.fstat(fd)
if not stat.S_ISREG(st.st_mode) or st.st_size > max_bytes: if (
not stat.S_ISREG(st.st_mode)
or st.st_nlink > 1
or st.st_size > max_bytes
):
continue continue
data = os.read(fd, max_bytes + 1) data = os.read(fd, max_bytes + 1)
if len(data) > max_bytes: if len(data) > max_bytes:

View file

@ -526,6 +526,16 @@ def _role_tag(role: str) -> str:
return f"role_{safe}" return f"role_{safe}"
def _write_generated_task_yaml(path: str, text: str, *, label: str) -> None:
"""Write generated task/handler/playbook YAML through one safety gate."""
body = text.rstrip() + "\n"
assert_generated_yaml_safe(body, label=label)
os.makedirs(os.path.dirname(path) or ".", exist_ok=True)
with open(path, "w", encoding="utf-8") as f:
f.write(body)
def _write_playbook_all(path: str, roles: List[str]) -> None: def _write_playbook_all(path: str, roles: List[str]) -> None:
pb_lines = [ pb_lines = [
"---", "---",
@ -540,9 +550,7 @@ def _write_playbook_all(path: str, roles: List[str]) -> None:
pb_lines.append(f" - role: {safe}") pb_lines.append(f" - role: {safe}")
pb_lines.append(f" tags: [{_role_tag(safe)}]") pb_lines.append(f" tags: [{_role_tag(safe)}]")
text = "\n".join(pb_lines) + "\n" text = "\n".join(pb_lines) + "\n"
assert_generated_yaml_safe(text, label="playbook.yml") _write_generated_task_yaml(path, text, label="playbook.yml")
with open(path, "w", encoding="utf-8") as f:
f.write(text)
def _write_playbook_host(path: str, fqdn: str, roles: List[str]) -> None: def _write_playbook_host(path: str, fqdn: str, roles: List[str]) -> None:
@ -560,9 +568,7 @@ def _write_playbook_host(path: str, fqdn: str, roles: List[str]) -> None:
pb_lines.append(f" - role: {safe}") pb_lines.append(f" - role: {safe}")
pb_lines.append(f" tags: [{_role_tag(safe)}]") pb_lines.append(f" tags: [{_role_tag(safe)}]")
text = "\n".join(pb_lines) + "\n" text = "\n".join(pb_lines) + "\n"
assert_generated_yaml_safe(text, label="host playbook") _write_generated_task_yaml(path, text, label="host playbook")
with open(path, "w", encoding="utf-8") as f:
f.write(text)
def _ensure_ansible_cfg(cfg_path: str) -> None: def _ensure_ansible_cfg(cfg_path: str) -> None:
@ -772,16 +778,16 @@ def _write_ansible_role(
# and keeps all harvested data in variable files; if any harvested value ever # and keeps all harvested data in variable files; if any harvested value ever
# leaked into this text and changed its shape, fail closed here rather than # leaked into this text and changed its shape, fail closed here rather than
# emit a poisoned playbook. # emit a poisoned playbook.
assert_generated_yaml_safe(tasks, label=f"role '{role}' tasks/main.yml") _write_generated_task_yaml(
assert_generated_yaml_safe(handlers, label=f"role '{role}' handlers/main.yml") os.path.join(role_dir, "tasks", "main.yml"),
tasks,
with open(os.path.join(role_dir, "tasks", "main.yml"), "w", encoding="utf-8") as f: label=f"role '{role}' tasks/main.yml",
f.write(tasks.rstrip() + "\n") )
_write_generated_task_yaml(
with open( os.path.join(role_dir, "handlers", "main.yml"),
os.path.join(role_dir, "handlers", "main.yml"), "w", encoding="utf-8" handlers,
) as f: label=f"role '{role}' handlers/main.yml",
f.write(handlers.rstrip() + "\n") )
_write_role_meta(role_dir, collections) _write_role_meta(role_dir, collections)
@ -1798,13 +1804,16 @@ def _write_managed_files_role(
_write_role_defaults(role_dir, vars_map) _write_role_defaults(role_dir, vars_map)
tasks = _render_role_tasks(AnsibleRole(role), managed_content=True) tasks = _render_role_tasks(AnsibleRole(role), managed_content=True)
with open(os.path.join(role_dir, "tasks", "main.yml"), "w", encoding="utf-8") as f: _write_generated_task_yaml(
f.write(tasks.rstrip() + "\n") os.path.join(role_dir, "tasks", "main.yml"),
tasks,
with open( label=f"role '{role}' tasks/main.yml",
os.path.join(role_dir, "handlers", "main.yml"), "w", encoding="utf-8" )
) as f: _write_generated_task_yaml(
f.write(handlers.rstrip() + "\n") os.path.join(role_dir, "handlers", "main.yml"),
handlers,
label=f"role '{role}' handlers/main.yml",
)
with open(os.path.join(role_dir, "meta", "main.yml"), "w", encoding="utf-8") as f: with open(os.path.join(role_dir, "meta", "main.yml"), "w", encoding="utf-8") as f:
f.write("---\ndependencies: []\n") f.write("---\ndependencies: []\n")

View file

@ -100,6 +100,8 @@ def copy_into_bundle(
st = os.fstat(fd) st = os.fstat(fd)
if not stat.S_ISREG(st.st_mode): if not stat.S_ISREG(st.st_mode):
raise OSError("refusing to copy non-regular source") raise OSError("refusing to copy non-regular source")
if st.st_nlink > 1:
raise OSError("refusing to copy hardlinked source")
chunks: list[bytes] = [] chunks: list[bytes] = []
while True: while True:
chunk = os.read(fd, 1024 * 1024) chunk = os.read(fd, 1024 * 1024)

View file

@ -895,8 +895,17 @@ def main() -> None:
v.add_argument( v.add_argument(
"--schema", "--schema",
help=( help=(
"Optional JSON schema source (file path or https:// URL). " "Optional JSON schema source. File paths are loaded by default; "
"If omitted, uses the schema vendored in the enroll codebase." "http(s) URLs require --allow-remote-schema. If omitted, uses "
"the schema vendored in the enroll codebase."
),
)
v.add_argument(
"--allow-remote-schema",
action="store_true",
help=(
"Allow --schema to fetch an http(s) URL. Disabled by default so "
"validation never makes network requests unless explicitly requested."
), ),
) )
v.add_argument( v.add_argument(
@ -1066,6 +1075,7 @@ def main() -> None:
sops_mode=bool(getattr(args, "sops", False)), sops_mode=bool(getattr(args, "sops", False)),
schema=getattr(args, "schema", None), schema=getattr(args, "schema", None),
no_schema=bool(getattr(args, "no_schema", False)), no_schema=bool(getattr(args, "no_schema", False)),
allow_remote_schema=bool(getattr(args, "allow_remote_schema", False)),
) )
fmt = str(getattr(args, "format", "text")) fmt = str(getattr(args, "format", "text"))

View file

@ -31,7 +31,7 @@ from .harvest_types import (
SysctlSnapshot, SysctlSnapshot,
) )
from .capture import capture_file from .capture import capture_file, write_bytes_into_bundle
from . import system_paths from . import system_paths
from .package_hints import package_section_from_installations, safe_name from .package_hints import package_section_from_installations, safe_name
@ -187,10 +187,12 @@ def _write_generated_artifact(
bundle_dir: str, role_name: str, src_rel: str, content: str bundle_dir: str, role_name: str, src_rel: str, content: str
) -> None: ) -> None:
"""Write a generated harvest artifact that did not exist as a file on disk.""" """Write a generated harvest artifact that did not exist as a file on disk."""
dst = os.path.join(bundle_dir, "artifacts", role_name, src_rel)
os.makedirs(os.path.dirname(dst), exist_ok=True) # Keep generated artifacts on the same no-follow/exclusive-create path as
with open(dst, "w", encoding="utf-8") as f: # copied source artifacts. This preserves the invariant that nothing under
f.write(content) # artifacts/ is written via a plain open() that could follow a symlink if a
# directory were unexpectedly reused or raced.
write_bytes_into_bundle(bundle_dir, role_name, src_rel, content.encode("utf-8"))
_SYSCTL_KEY_RE = re.compile(r"^[A-Za-z0-9_.-]+$") _SYSCTL_KEY_RE = re.compile(r"^[A-Za-z0-9_.-]+$")

View file

@ -355,6 +355,12 @@ class IgnorePolicy:
if not stat.S_ISREG(st.st_mode): if not stat.S_ISREG(st.st_mode):
return "not_regular_file", None return "not_regular_file", None
if st.st_nlink > 1:
# A hardlink gives a file another safe-looking pathname. When
# Enroll is run as root, path-based deny rules such as
# /etc/shadow must not be bypassed by copying the same inode via
# an attacker-controlled alias under an otherwise allowed tree.
return "hardlink_source", None
if st.st_size > self.max_file_bytes: if st.st_size > self.max_file_bytes:
return "too_large", None return "too_large", None

View file

@ -58,12 +58,15 @@ def _default_schema_path() -> Path:
return Path(__file__).resolve().parent / "schema" / "state.schema.json" return Path(__file__).resolve().parent / "schema" / "state.schema.json"
def _load_schema(schema: Optional[str]) -> Dict[str, Any]: def _load_schema(
schema: Optional[str], *, allow_remote_schema: bool = False
) -> Dict[str, Any]:
"""Load a JSON schema. """Load a JSON schema.
If schema is None, load the vendored schema. If schema is None, load the vendored schema.
If schema begins with http(s)://, fetch it. If schema begins with http(s)://, fetch it only when remote schema
Otherwise, treat it as a local file path. loading was explicitly enabled by the caller. Otherwise, treat it as a
local file path.
""" """
if not schema: if not schema:
@ -72,6 +75,11 @@ def _load_schema(schema: Optional[str]) -> Dict[str, Any]:
return json.load(f) return json.load(f)
if schema.startswith("http://") or schema.startswith("https://"): if schema.startswith("http://") or schema.startswith("https://"):
if not allow_remote_schema:
raise ValueError(
"remote schema URLs are disabled by default; use "
"--allow-remote-schema only when the schema source is trusted"
)
with urllib.request.urlopen(schema, timeout=10) as resp: # nosec with urllib.request.urlopen(schema, timeout=10) as resp: # nosec
data = resp.read() data = resp.read()
return json.loads(data.decode("utf-8")) return json.loads(data.decode("utf-8"))
@ -136,6 +144,7 @@ def validate_harvest(
sops_mode: bool = False, sops_mode: bool = False,
schema: Optional[str] = None, schema: Optional[str] = None,
no_schema: bool = False, no_schema: bool = False,
allow_remote_schema: bool = False,
) -> ValidationResult: ) -> ValidationResult:
"""Validate an enroll harvest bundle. """Validate an enroll harvest bundle.
@ -165,7 +174,7 @@ def validate_harvest(
if not no_schema: if not no_schema:
try: try:
sch = _load_schema(schema) sch = _load_schema(schema, allow_remote_schema=allow_remote_schema)
validator = jsonschema.Draft202012Validator(sch) validator = jsonschema.Draft202012Validator(sch)
for err in sorted(validator.iter_errors(state), key=str): for err in sorted(validator.iter_errors(state), key=str):
ptr = _json_pointer(err) ptr = _json_pointer(err)

View file

@ -320,3 +320,49 @@ def test_ensure_private_empty_dir_creates_private_dir(tmp_path: Path):
out = hs.ensure_private_empty_dir(tmp_path / "new-cache", label="cache") out = hs.ensure_private_empty_dir(tmp_path / "new-cache", label="cache")
assert out.is_dir() assert out.is_dir()
assert (out.stat().st_mode & 0o777) == 0o700 assert (out.stat().st_mode & 0o777) == 0o700
def test_capture_file_excludes_hardlinked_source(tmp_path: Path):
source = tmp_path / "source.conf"
source.write_text("safe=true\n", encoding="utf-8")
alias = tmp_path / "alias.conf"
os.link(source, alias)
bundle = tmp_path / "bundle"
bundle.mkdir()
managed: list[ManagedFile] = []
excluded: list[ExcludedFile] = []
ok = capture_file(
bundle_dir=str(bundle),
role_name="role",
abs_path=str(alias),
reason="test",
policy=IgnorePolicy(),
path_filter=PathFilter(),
managed_out=managed,
excluded_out=excluded,
)
assert ok is False
assert managed == []
assert excluded == [ExcludedFile(path=str(alias), reason="hardlink_source")]
assert not (bundle / "artifacts" / "role" / str(alias).lstrip("/")).exists()
def test_generated_artifact_writer_refuses_existing_symlink(tmp_path: Path):
from enroll.harvest import _write_generated_artifact
bundle = tmp_path / "bundle"
dst_dir = bundle / "artifacts" / "firewall_runtime" / "runtime"
dst_dir.mkdir(parents=True)
target = tmp_path / "target"
target.write_text("old\n", encoding="utf-8")
link = dst_dir / "iptables-v4.save"
os.symlink(target, link)
with pytest.raises(OSError):
_write_generated_artifact(
str(bundle), "firewall_runtime", "runtime/iptables-v4.save", "new\n"
)
assert target.read_text(encoding="utf-8") == "old\n"

View file

@ -416,3 +416,29 @@ def test_inspect_file_still_captures_normal_nested_file(tmp_path: Path):
assert reason is None assert reason is None
assert inspection is not None assert inspection is not None
assert inspection.data == b"workers=4\n" assert inspection.data == b"workers=4\n"
def test_inspect_file_refuses_hardlinked_source(tmp_path: Path):
pol = IgnorePolicy()
original = tmp_path / "original.conf"
original.write_text("safe=true\n", encoding="utf-8")
alias = tmp_path / "alias.conf"
os.link(original, alias)
reason, inspection = pol.inspect_file(str(alias))
assert reason == "hardlink_source"
assert inspection is None
def test_inspect_file_refuses_hardlinked_source_even_in_dangerous_mode(tmp_path: Path):
pol = IgnorePolicy(dangerous=True)
original = tmp_path / "original.conf"
original.write_text("safe=true\n", encoding="utf-8")
alias = tmp_path / "alias.conf"
os.link(original, alias)
reason, inspection = pol.inspect_file(str(alias))
assert reason == "hardlink_source"
assert inspection is None

View file

@ -106,3 +106,16 @@ def test_ansible_unsafe_data_still_tags_jinja_values():
assert not isinstance(out["safe"], AnsibleUnsafeText) assert not isinstance(out["safe"], AnsibleUnsafeText)
assert is_ansible_template_like("{{ x }}") assert is_ansible_template_like("{{ x }}")
assert not is_ansible_template_like("plain") assert not is_ansible_template_like("plain")
def test_write_generated_task_yaml_uses_safety_gate(tmp_path):
from enroll.ansible import _write_generated_task_yaml
path = tmp_path / "tasks.yml"
with pytest.raises(RenderSafetyError):
_write_generated_task_yaml(
str(path), "---\nkey: value\n", label="test tasks/main.yml"
)
assert not path.exists()

View file

@ -305,16 +305,54 @@ def test_validate_harvest_invalid_json(tmp_path: Path):
assert any("failed to parse" in e for e in result.errors) assert any("failed to parse" in e for e in result.errors)
def test_validate_harvest_schema_error(tmp_path: Path): def test_validate_harvest_rejects_remote_schema_by_default(tmp_path: Path):
bundle_dir = tmp_path / "bundle" bundle_dir = tmp_path / "bundle"
bundle_dir.mkdir() bundle_dir.mkdir()
state_file = bundle_dir / "state.json" state_file = bundle_dir / "state.json"
state_file.write_text("{}", encoding="utf-8") state_file.write_text("{}", encoding="utf-8")
result = validate_harvest( result = validate_harvest(
str(bundle_dir), schema="https://invalid.invalid/schema.json" str(bundle_dir), schema="https://invalid.invalid/schema.json"
) )
assert result.ok is False assert result.ok is False
assert any("failed to load/validate schema" in e for e in result.errors) assert any("remote schema URLs are disabled" in e for e in result.errors)
def test_validate_harvest_allows_remote_schema_when_explicit(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
):
bundle_dir = tmp_path / "bundle"
bundle_dir.mkdir()
state_file = bundle_dir / "state.json"
state_file.write_text("{}", encoding="utf-8")
class _Response:
def __enter__(self):
return self
def __exit__(self, exc_type, exc, tb):
return False
def read(self):
return b'{"type":"object"}'
called = []
def _fake_urlopen(url: str, timeout: int):
called.append((url, timeout))
return _Response()
monkeypatch.setattr("enroll.validate.urllib.request.urlopen", _fake_urlopen)
result = validate_harvest(
str(bundle_dir),
schema="https://example.invalid/schema.json",
allow_remote_schema=True,
)
assert called == [("https://example.invalid/schema.json", 10)]
assert not any("remote schema URLs are disabled" in e for e in result.errors)
def test_validate_harvest_missing_artifact(tmp_path: Path): def test_validate_harvest_missing_artifact(tmp_path: Path):