Sanity check on FQDN name to avoid accidental path traversal and similar woes

This commit is contained in:
Miguel Jacq 2026-06-22 10:59:17 +10:00
parent 3e8ad600e2
commit 205c419a7a
Signed by: mig5
GPG key ID: 03906B4110AAD3B8
2 changed files with 76 additions and 0 deletions

View file

@ -10,6 +10,7 @@ from typing import List, Optional
from .ansible import manifest_from_bundle_dir as manifest_ansible_from_bundle_dir
from .puppet import manifest_from_bundle_dir as manifest_puppet_from_bundle_dir
from .salt import manifest_from_bundle_dir as manifest_salt_from_bundle_dir
from .manifest_safety import validate_site_fqdn
from .remote import _safe_extract_tar
from .sopsutil import (
decrypt_file_binary_to,
@ -194,6 +195,7 @@ def manifest(
target = (target or "ansible").strip().lower()
if target not in {"ansible", "puppet", "salt"}:
raise ValueError(f"unsupported manifest target: {target!r}")
fqdn = validate_site_fqdn(fqdn)
sops_mode = bool(sops_fingerprints)

View file

@ -1,6 +1,7 @@
from __future__ import annotations
import os
import re
import shutil
import stat
from pathlib import Path
@ -15,6 +16,78 @@ class ManifestOutputError(RuntimeError):
"""Raised when a manifest output path is unsafe to use."""
_SITE_FQDN_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]{0,252}$")
def validate_site_fqdn(value: str | None) -> str | None:
"""Validate the optional site-mode host name/FQDN.
Renderers use this value in inventory data and, for Ansible, in output
paths. Keep it deliberately conservative so it cannot become a path
separator, absolute path, YAML/INI newline injection, or shell-ish text in
generated documentation/commands.
"""
if value is None:
return None
text = str(value).strip()
if not text:
return None
if any(ch in text for ch in ("/", "\\", "\x00", "\n", "\r")):
raise ManifestOutputError(
"--fqdn contains unsafe path or newline characters; use a simple "
"host/inventory name"
)
if text in {".", ".."} or not _SITE_FQDN_RE.fullmatch(text):
raise ManifestOutputError(
"--fqdn must start with a letter or digit and contain only "
"letters, digits, dot, underscore, or hyphen"
)
return text
def _assert_no_output_symlinks(root: Path) -> None:
"""Reject pre-existing symlinks in an output tree we are about to merge into.
Non-site mode refuses existing output directories entirely. Site/FQDN modes
intentionally accumulate multiple nodes into one tree, so reject symlinks in
the tree before merging to avoid writes being redirected outside *root*.
Version-control metadata can contain implementation-specific entries and is
not part of Enroll's generated layout, so it is pruned from this check.
"""
skip_dirs = {".git", ".hg", ".svn"}
for dirpath, dirnames, filenames in os.walk(root, followlinks=False):
dirpath_p = Path(dirpath)
for dirname in list(dirnames):
if dirname in skip_dirs:
dirnames.remove(dirname)
continue
p = dirpath_p / dirname
try:
st = p.lstat()
except FileNotFoundError:
continue
if stat.S_ISLNK(st.st_mode):
raise ManifestOutputError(
f"manifest output tree contains a symlink; refusing to merge: {p}"
)
for filename in filenames:
if filename in skip_dirs:
continue
p = dirpath_p / filename
try:
st = p.lstat()
except FileNotFoundError:
continue
if stat.S_ISLNK(st.st_mode):
raise ManifestOutputError(
f"manifest output tree contains a symlink; refusing to merge: {p}"
)
def _safe_relative_path(value: str, *, field: str) -> Path:
text = str(value or "").strip()
if not text:
@ -56,6 +129,7 @@ def prepare_manifest_output_dir(
raise ManifestOutputError(
f"manifest output path exists but is not a directory: {out}"
)
_assert_no_output_symlinks(out)
return out
out.mkdir(parents=True, exist_ok=False)
return out