From ad84330ad0a123305567c25fb62a1105168c9519 Mon Sep 17 00:00:00 2001
From: Fangliquan <fangliquan@oppo.com>
Date: Sun, 26 Jul 2026 10:46:55 +0800
Subject: [PATCH] fix(hermes_cli): heal root-owned logs/gateways on every
 stage2 boot

Without restartable log/run chown, warm volumes that keep a hermes-owned
HERMES_HOME but root-owned logs/gateways would again deny hermes mkdir.
Add a non-recursive stage2 parent heal and cover the poisoned-parent reboot path.
---
 docker/stage2-hook.sh                    | 15 +++++++
 tests/docker/test_log_dir_seed.py        | 53 +++++++++++++++++++++++-
 tests/hermes_cli/test_service_manager.py |  2 +-
 3 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/docker/stage2-hook.sh b/docker/stage2-hook.sh
index bd722da731..05474ca927 100755
--- a/docker/stage2-hook.sh
+++ b/docker/stage2-hook.sh
@@ -287,6 +287,21 @@ if [ -d "$HERMES_HOME/cron" ]; then
     chown_hermes_tree "$HERMES_HOME/cron"
 fi
 
+# Always ensure logs/gateways is hermes-owned (#45258). Formerly healed by
+# restartable gateway log/run chown — removed due to symlink TOCTOU
+# (CWE-59/367). The targeted data-volume chown above only runs when the
+# top-level $HERMES_HOME is mis-owned, so a warm volume with hermes-owned
+# HERMES_HOME but root-owned logs/gateways would otherwise leave
+# s6-setuidgid hermes mkdir failing with Permission denied. Non-recursive:
+# profile leaf dirs are each created/owned by their own log/run as hermes.
+if [ -d "$HERMES_HOME/logs/gateways" ]; then
+    if refuse_symlinked_path "chown" "$HERMES_HOME/logs/gateways"; then
+        :
+    else
+        chown hermes:hermes "$HERMES_HOME/logs/gateways" 2>/dev/null || true
+    fi
+fi
+
 # Always reset ownership of pairing data on every boot, same docker-exec/
 # root-write reason as profiles/ and cron/. `docker exec <container>
 # hermes pairing approve …` defaults to uid=0 and writes 0600 root-owned
diff --git a/tests/docker/test_log_dir_seed.py b/tests/docker/test_log_dir_seed.py
index 2893da2814..554fac9d2e 100644
--- a/tests/docker/test_log_dir_seed.py
+++ b/tests/docker/test_log_dir_seed.py
@@ -10,7 +10,11 @@ s6-log crash-loops on mkdir: Permission denied.
 """
 from __future__ import annotations
 
-from tests.docker.conftest import docker_exec_sh, start_container
+from tests.docker.conftest import (
+    docker_exec_sh,
+    restart_container,
+    start_container,
+)
 
 
 def test_logs_gateways_seeded_and_hermes_owned(
@@ -45,3 +49,50 @@ def test_logs_gateways_seeded_and_hermes_owned(
     assert "gateways=hermes" in r.stdout, (
         f"logs/gateways/ not owned by hermes: {r.stdout}"
     )
+
+
+def test_logs_gateways_healed_when_parent_root_owned(
+    built_image: str, container_name: str,
+) -> None:
+    """Warm-boot stage2 must heal root-owned logs/gateways (#45258).
+
+    Mimics a poisoned volume: HERMES_HOME already hermes-owned (so the
+    bulk data-volume chown is skipped) while logs/gateways is root-owned.
+    Restartable log/run no longer root-chowns that path (symlink TOCTOU),
+    so stage2 must repair the parent on every boot.
+    """
+    start_container(built_image, container_name)
+
+    poison = docker_exec_sh(
+        container_name,
+        "chown root:root /opt/data/logs/gateways && "
+        'home_owner=$(stat -c "%U" /opt/data); '
+        'gateways_owner=$(stat -c "%U" /opt/data/logs/gateways); '
+        'echo "home=$home_owner gateways=$gateways_owner"',
+        user="root",
+        timeout=10,
+    )
+    assert poison.returncode == 0, (poison.stdout, poison.stderr)
+    assert "home=hermes" in poison.stdout, poison.stdout
+    assert "gateways=root" in poison.stdout, poison.stdout
+
+    denied = docker_exec_sh(
+        container_name,
+        "mkdir -p /opt/data/logs/gateways/poison-probe 2>/dev/null "
+        "&& echo MKDIR_OK || echo MKDIR_DENIED",
+        timeout=10,
+    )
+    assert "MKDIR_DENIED" in denied.stdout, denied.stdout
+
+    restart_container(container_name)
+
+    healed = docker_exec_sh(
+        container_name,
+        'gateways_owner=$(stat -c "%U" /opt/data/logs/gateways); '
+        "mkdir -p /opt/data/logs/gateways/poison-probe && "
+        'echo "gateways=$gateways_owner MKDIR_OK"',
+        timeout=10,
+    )
+    assert healed.returncode == 0, (healed.stdout, healed.stderr)
+    assert "gateways=hermes" in healed.stdout, healed.stdout
+    assert "MKDIR_OK" in healed.stdout, healed.stdout
diff --git a/tests/hermes_cli/test_service_manager.py b/tests/hermes_cli/test_service_manager.py
index 99826b08cf..e05df52a58 100644
--- a/tests/hermes_cli/test_service_manager.py
+++ b/tests/hermes_cli/test_service_manager.py
@@ -1134,7 +1134,7 @@ def test_s6_log_run_creates_leaf_as_hermes_without_chown(
         f"saw: {log_text!r}"
     )
     assert 's6-setuidgid hermes mkdir -p "$log_dir"' in log_text
-    assert 'mkdir -p "$log_dir"' in log_text
+    assert 'else\n  mkdir -p "$log_dir"\nfi\n' in log_text
 
     mkdir_as_hermes_idx = log_text.index('s6-setuidgid hermes mkdir -p "$log_dir"')
     exec_idx = log_text.index("s6-log 1 ")