stage: merge #5797

api/config.py

@@ -780,7 +780,8 @@ def _save_yaml_config_file(config_path: Path, config_data: dict) -> None:
         raise RuntimeError("PyYAML is required to write Hermes config.yaml") from exc
 
     config_path.parent.mkdir(parents=True, exist_ok=True)
-    config_path.write_text(
+    _paths._atomic_write_text(
+        config_path,
         _yaml.safe_dump(_config_for_yaml_save(config_data), sort_keys=False, allow_unicode=True),
         encoding="utf-8",
     )

api/onboarding.py

@@ -28,6 +28,7 @@ from api.config import (
     save_settings,
     verify_hermes_imports,
 )
+from api.paths import _atomic_write_text
 from api.providers import _write_env_file  # shared impl with _ENV_LOCK (#1164)
 from api.workspace import get_last_workspace, load_workspaces
 
@@ -251,7 +252,8 @@ def _save_yaml_config(config_path: Path, config: dict) -> None:
         raise RuntimeError("PyYAML is required to write Hermes config.yaml") from exc
 
     config_path.parent.mkdir(parents=True, exist_ok=True)
-    config_path.write_text(
+    _atomic_write_text(
+        config_path,
         _yaml.safe_dump(config, sort_keys=False, allow_unicode=True),
         encoding="utf-8",
     )

api/paths.py

@@ -5,12 +5,258 @@ that need the default Hermes home can import them without triggering config's
 larger startup side effects.
 """
 
+import errno
 import os
+import secrets
+import stat
+import tempfile
+import threading
 from pathlib import Path
 
 HOME = Path.home()
 
 
+def _create_atomic_temp_file(write_path: Path, *, existing: bool) -> tuple[int, str]:
+    """Create a same-directory temp file without reading the process umask.
+
+    ``tempfile.mkstemp`` deliberately creates files as ``0600``. For a brand-new
+    config, use ``os.open(..., 0666)`` instead so the kernel applies the active
+    process umask as it would for ``Path.write_text``. Reading umask by calling
+    ``os.umask`` is unsafe because that syscall mutates process-wide state, and
+    imports may happen after the threaded server has started.
+    """
+    if existing:
+        return tempfile.mkstemp(
+            dir=str(write_path.parent), prefix=f".{write_path.name}_", suffix=".tmp"
+        )
+
+    flags = os.O_RDWR | os.O_CREAT | os.O_EXCL
+    for _ in range(100):
+        tmp = write_path.parent / f".{write_path.name}_{secrets.token_hex(16)}.tmp"
+        try:
+            return os.open(tmp, flags, 0o666), str(tmp)
+        except FileExistsError:
+            continue
+    raise FileExistsError("unable to allocate unique config temporary file")
+
+# In-place compatibility writes cannot use rename isolation. Serialize every
+# such write in this process so concurrent WebUI saves cannot interleave their
+# truncate/write/fsync sequences. A single lock avoids a permanent per-path
+# lock cache for arbitrary profile/config locations.
+_IN_PLACE_WRITE_LOCK = threading.RLock()
+
+
+def _fsync_directory(directory: Path) -> None:
+    """Persist a completed rename on filesystems that support directory fsync."""
+    if os.name == "nt":
+        return
+    flags = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0)
+    try:
+        fd = os.open(directory, flags)
+    except PermissionError:
+        return
+    try:
+        try:
+            os.fsync(fd)
+        except OSError as exc:
+            if exc.errno not in {errno.EINVAL, errno.ENOTSUP}:
+                raise
+    finally:
+        os.close(fd)
+
+
+def _has_extended_attributes(path: Path) -> bool:
+    """Return whether *path* has xattrs that an inode replacement would discard."""
+    listxattr = getattr(os, "listxattr", None)
+    if listxattr is None:
+        return False
+    try:
+        return bool(listxattr(path))
+    except OSError as exc:
+        unsupported_errnos = {
+            errno.ENOSYS,
+            errno.ENOTSUP,
+            getattr(errno, "EOPNOTSUPP", errno.ENOTSUP),
+        }
+        if exc.errno in unsupported_errnos:
+            return False
+        raise
+
+
+def _require_writable_target(write_path: Path) -> os.stat_result | None:
+    """Reject writes to a read-only target before any replacement work.
+
+    ``os.replace`` happily swaps a fresh inode over a ``0444`` file when the
+    directory is writable, which would silently defeat a deliberately locked
+    config. The old in-place ``Path.write_text`` raised ``PermissionError``
+    there; preserve that contract with a non-truncating ``O_WRONLY`` probe,
+    letting the failure propagate. Returns the ``fstat`` of the inode the
+    probe actually opened (so the caller's metadata describes the verified
+    file even if a concurrent writer replaced it since its own ``stat``), or
+    ``None`` if the target vanished concurrently.
+    """
+    try:
+        probe_fd = os.open(write_path, os.O_WRONLY)
+    except FileNotFoundError:
+        return None
+    try:
+        return os.fstat(probe_fd)
+    finally:
+        os.close(probe_fd)
+
+
+def _atomic_write_text(path: Path, text: str, *, encoding: str = "utf-8") -> None:
+    """Atomically replace *path* with *text*.
+
+    Writes to a temp file in the same directory, flushes + ``os.fsync``, then
+    ``os.replace``s it into place, so a crash (or exception) mid-write can never
+    truncate an existing file — the old contents stay intact until the rename
+    commits the new ones in one step.  Mirrors the tempfile+fsync+os.replace
+    pattern already used for ``.env`` and cost snapshots in ``api.providers``;
+    extracted here so ``config.yaml`` / profile ``config.yaml`` writers can share
+    it without pulling in env-file-specific logic.
+
+    Permissions and ownership are preserved: ``os.replace`` carries the temp
+    file's metadata onto the target, and ``tempfile.mkstemp`` starts with the
+    writer's owner/group and mode ``0600``. Without correcting those values,
+    every rewrite could silently reset a shared config's group and tighten a
+    group/other-readable ``config.yaml`` (commonly ``0644``/``0664``) down to
+    owner-only. Existing files copy uid/gid and mode onto the temp descriptor;
+    new files use the active process umask, exactly as a normal
+    ``open(..., "w")`` would.
+    (Unlike ``.env``, ``config.yaml`` holds no secrets and is not meant to be
+    forced to ``0600``.)
+
+    Symlinks and hard links keep the same follow-through semantics as
+    ``Path.write_text``: writing ``config.yaml`` through a symlink updates the
+    referent, while writing an inode with multiple hard links updates every
+    alias rather than severing the selected path. A symlink target change
+    observed before commit is rejected rather than updating a stale referent.
+
+    The temp+rename dance needs WRITE PERMISSION ON THE DIRECTORY, which the
+    plain ``Path.write_text`` it replaced did not: hardened deployments
+    (``HERMES_CONFIG_PATH`` pointing into a root-owned, read-only dir with a
+    writable ``config.yaml`` inside) would otherwise lose the ability to save
+    settings at all. When temp creation is denied, or the original uid/gid
+    cannot be transferred to the temp inode, we fall back to a guarded
+    descriptor write only if the target is still the regular-file inode
+    inspected above. Hard-linked files and files with extended attributes use
+    that same path because replacing their directory entry would respectively
+    break ``Path.write_text`` compatibility or discard metadata such as POSIX
+    ACLs. These in-place writes are serialized across process-local WebUI
+    writers. They give up crash-atomicity (and raw concurrent readers may
+    observe the write in progress) only where atomic replace with preserved
+    semantics is impossible. Other ``PermissionError``s still propagate.
+    Successful renames fsync the parent directory where supported.
+
+    The caller is responsible for ensuring ``path.parent`` exists.
+    """
+    path = Path(path)
+    symlink_target = path.resolve(strict=False) if path.is_symlink() else None
+    write_path = symlink_target or path
+    try:
+        existing_stat = os.stat(write_path)
+    except FileNotFoundError:
+        existing_stat = None
+    mode = stat.S_IMODE(existing_stat.st_mode) if existing_stat else None
+
+    def _verify_symlink_target() -> None:
+        if symlink_target is not None and path.resolve(strict=False) != symlink_target:
+            raise RuntimeError("config symlink target changed during atomic write")
+
+    def _write_in_place() -> None:
+        with _IN_PLACE_WRITE_LOCK:
+            if existing_stat is None or not stat.S_ISREG(existing_stat.st_mode):
+                raise PermissionError(f"Cannot replace config path: {write_path}")
+            _verify_symlink_target()
+            fallback_fd = os.open(write_path, os.O_WRONLY)
+            owns_fallback_fd = True
+            try:
+                opened_stat = os.fstat(fallback_fd)
+                if (opened_stat.st_dev, opened_stat.st_ino) != (
+                    existing_stat.st_dev,
+                    existing_stat.st_ino,
+                ):
+                    raise PermissionError("config target changed before fallback write")
+                _verify_symlink_target()
+                os.ftruncate(fallback_fd, 0)
+                fallback_file = os.fdopen(fallback_fd, "w", encoding=encoding)
+                owns_fallback_fd = False
+                with fallback_file:
+                    fallback_file.write(text)
+                    fallback_file.flush()
+                    os.fsync(fallback_file.fileno())
+            finally:
+                if owns_fallback_fd:
+                    os.close(fallback_fd)
+
+    if existing_stat is not None and stat.S_ISREG(existing_stat.st_mode):
+        probed_stat = _require_writable_target(write_path)
+        if probed_stat is not None and stat.S_ISREG(probed_stat.st_mode):
+            existing_stat = probed_stat
+            mode = stat.S_IMODE(existing_stat.st_mode)
+        else:
+            existing_stat = probed_stat
+            mode = None
+        if existing_stat is not None and (
+            existing_stat.st_nlink > 1 or _has_extended_attributes(write_path)
+        ):
+            _write_in_place()
+            return
+
+    try:
+        fd, tmp = _create_atomic_temp_file(write_path, existing=existing_stat is not None)
+    except PermissionError:
+        _write_in_place()
+        return
+    owns_fd = True
+    try:
+        if existing_stat is not None and hasattr(os, "fchown"):
+            try:
+                os.fchown(fd, existing_stat.st_uid, existing_stat.st_gid)
+            except (OSError, NotImplementedError) as exc:
+                unsupported_ownership_errnos = {
+                    errno.EINVAL,
+                    errno.ENOSYS,
+                    errno.ENOTSUP,
+                }
+                if (
+                    isinstance(exc, OSError)
+                    and not isinstance(exc, PermissionError)
+                    and exc.errno not in unsupported_ownership_errnos
+                ):
+                    raise
+                os.close(fd)
+                owns_fd = False
+                os.unlink(tmp)
+                _write_in_place()
+                return
+        if mode is not None and hasattr(os, "fchmod"):
+            os.fchmod(fd, mode)
+        elif mode is not None:
+            os.chmod(tmp, mode)
+        f = os.fdopen(fd, "w", encoding=encoding)
+        owns_fd = False
+        with f:
+            f.write(text)
+            f.flush()
+            os.fsync(f.fileno())
+        _verify_symlink_target()
+        os.replace(tmp, write_path)
+        _fsync_directory(write_path.parent)
+    except BaseException:
+        if owns_fd:
+            try:
+                os.close(fd)
+            except OSError:
+                pass
+        try:
+            os.unlink(tmp)
+        except OSError:
+            pass
+        raise
+
+
 def _hermes_home_has_webui_state(base: Path) -> bool:
     """Return True when *base* holds real WebUI state under its ``webui/`` dir.
 

api/profiles.py

@@ -21,6 +21,7 @@ from typing import Optional
 
 import yaml
 
+from api.paths import _atomic_write_text
 from api.session_events import publish_session_list_changed
 
 logger = logging.getLogger(__name__)
@@ -2374,7 +2375,7 @@ def _write_endpoint_to_config(profile_dir: Path, base_url: str = None, api_key:
     if base_url:
         model_section['base_url'] = base_url
     cfg['model'] = model_section
-    config_path.write_text(_yaml.dump(cfg, default_flow_style=False, allow_unicode=True), encoding='utf-8')
+    _atomic_write_text(config_path, _yaml.dump(cfg, default_flow_style=False, allow_unicode=True), encoding='utf-8')
 
 
 def _clean_profile_config_value(value: Optional[str], field: str) -> Optional[str]:
@@ -2513,7 +2514,7 @@ def _write_model_defaults_to_config(
     if model_provider:
         model_section['provider'] = model_provider
     cfg['model'] = model_section
-    config_path.write_text(_yaml.dump(cfg, default_flow_style=False, allow_unicode=True), encoding='utf-8')
+    _atomic_write_text(config_path, _yaml.dump(cfg, default_flow_style=False, allow_unicode=True), encoding='utf-8')
 
 
 def create_profile_api(name: str, clone_from: str = None,