Harden root collection ownership handoff

.github/workflows/pip.yml

@@ -189,5 +189,33 @@ jobs:
               echo "Root-owned files remain in $root_tool_dir/data" >&2
               exit 1
             fi
+
+            sudo mkdir "$root_tool_dir/external"
+            sudo ln -s "$root_tool_dir/external" "$root_tool_dir/data/external-link"
+            sudo env PATH="$root_tool_dir/bin:$PATH" \
+              bash -c "cd '$root_tool_dir/data' && archivebox status"
+            sudo stat -c %U "$root_tool_dir/external" | grep -qx root
+
+            sudo mkdir "$root_tool_dir/data-universal-init"
+            sudo env PATH="$root_tool_dir/bin:$PATH" \
+              bash -c "cd '$root_tool_dir/data-universal-init' && archivebox status --init"
+            sudo stat -c %U "$root_tool_dir/data-universal-init" | grep -qx archivebox
+
+            sudo mkdir "$root_tool_dir/data-install"
+            sudo env PATH="$root_tool_dir/bin:$PATH" \
+              bash -c "cd '$root_tool_dir/data-install' && archivebox install --dry-run"
+            sudo stat -c %U "$root_tool_dir/data-install" | grep -qx archivebox
+
+            sudo useradd --create-home archivebox-smoke-user
+            smoke_user_group="$(id -gn archivebox-smoke-user)"
+            sudo mkdir "$root_tool_dir/data-mixed-owner"
+            sudo chown archivebox-smoke-user:root "$root_tool_dir/data-mixed-owner"
+            sudo env PATH="$root_tool_dir/bin:$PATH" \
+              bash -c "cd '$root_tool_dir/data-mixed-owner' && archivebox init"
+            sudo stat -c %U:%G "$root_tool_dir/data-mixed-owner" | grep -qx "archivebox-smoke-user:$smoke_user_group"
+            if sudo find "$root_tool_dir/data-mixed-owner" -xdev -user root -print -quit | grep -q .; then
+              echo "Root-owned files remain in $root_tool_dir/data-mixed-owner" >&2
+              exit 1
+            fi
             sudo rm -rf "$root_tool_dir"
           fi

archivebox/config/django.py

@@ -59,10 +59,10 @@ def setup_django(check_db=False) -> None:
         with SudoPermission(uid=0):
             # running as root is a special case where it's ok to be a bit slower
             # make sure data dir is always owned by the correct user
-            os.chown(CONSTANTS.DATA_DIR, ARCHIVEBOX_USER, ARCHIVEBOX_GROUP)
+            os.chown(CONSTANTS.DATA_DIR, ARCHIVEBOX_USER, ARCHIVEBOX_GROUP, follow_symlinks=False)
             if CONSTANTS.DATA_DIR.exists():
                 for child in CONSTANTS.DATA_DIR.iterdir():
-                    os.chown(child, ARCHIVEBOX_USER, ARCHIVEBOX_GROUP)
+                    os.chown(child, ARCHIVEBOX_USER, ARCHIVEBOX_GROUP, follow_symlinks=False)
 
     # Suppress the "database access during app initialization" warning
     # This warning can be triggered during django.setup() but is safe to ignore

archivebox/config/permissions.py

@@ -32,11 +32,13 @@ def select_archivebox_user(
     data_dir_gid: int,
     account_uid: int | None,
     account_gid: int | None,
+    data_dir_owner_gid: int | None = None,
     data_dir_owner_exists: bool = True,
 ) -> tuple[int, int]:
     if running_uid == 0:
         if data_dir_uid != 0 and data_dir_owner_exists:
-            return data_dir_uid, data_dir_gid
+            owner_gid = data_dir_owner_gid if data_dir_gid == 0 and data_dir_owner_gid is not None else data_dir_gid
+            return data_dir_uid, owner_gid
         if account_uid is not None and account_gid is not None:
             return account_uid, account_gid
 
@@ -57,9 +59,10 @@ except PermissionError:
     DATA_DIR_GID = 0
 
 try:
-    pwd.getpwuid(DATA_DIR_UID)
+    DATA_DIR_OWNER_GID = pwd.getpwuid(DATA_DIR_UID).pw_gid
     DATA_DIR_OWNER_EXISTS = True
 except KeyError:
+    DATA_DIR_OWNER_GID = None
     DATA_DIR_OWNER_EXISTS = False
 
 DEFAULT_UID = 911
@@ -138,6 +141,7 @@ ARCHIVEBOX_USER, ARCHIVEBOX_GROUP = select_archivebox_user(
     sudo_gid=SUDO_GID,
     data_dir_uid=DATA_DIR_UID,
     data_dir_gid=DATA_DIR_GID,
+    data_dir_owner_gid=DATA_DIR_OWNER_GID,
     account_uid=ARCHIVEBOX_ACCOUNT.pw_uid if ARCHIVEBOX_ACCOUNT is not None else None,
     account_gid=ARCHIVEBOX_ACCOUNT.pw_gid if ARCHIVEBOX_ACCOUNT is not None else None,
     data_dir_owner_exists=DATA_DIR_OWNER_EXISTS,
@@ -205,7 +209,7 @@ def root_data_dir_handoff_paths(data_dir: Path, argv: list[str]) -> tuple[Path,
     except (FileNotFoundError, PermissionError):
         return ()
 
-    is_init = "init" in argv[1:]
+    is_init = any(arg in ("init", "install", "--init", "--quick-init") for arg in argv[1:])
     collection_exists = any((data_dir / marker).exists() for marker in (".archivebox_id", "ArchiveBox.conf", "index.sqlite3"))
     if not collection_exists and not (is_init and not children):
         return ()
@@ -238,9 +242,9 @@ def root_parent_can_grant_group_traversal(*, parent_uid: int, parent_gid: int, p
 
 
 def grant_archivebox_group_traversal(path: Path) -> None:
-    """Let the archivebox account traverse private root-owned parents."""
+    """Let the selected runtime user traverse private root-owned parents."""
 
-    if not IS_ROOT or ARCHIVEBOX_ACCOUNT is None:
+    if not IS_ROOT or not ARCHIVEBOX_USER_EXISTS:
         return
 
     for parent in path.resolve().parents:
@@ -251,9 +255,9 @@ def grant_archivebox_group_traversal(path: Path) -> None:
             parent_uid=parent_stat.st_uid,
             parent_gid=parent_stat.st_gid,
             parent_mode=parent_stat.st_mode,
-            account_gid=ARCHIVEBOX_ACCOUNT.pw_gid,
+            account_gid=ARCHIVEBOX_GROUP,
         ):
-            os.chown(parent, -1, ARCHIVEBOX_ACCOUNT.pw_gid, follow_symlinks=False)
+            os.chown(parent, -1, ARCHIVEBOX_GROUP, follow_symlinks=False)
             os.chmod(parent, stat.S_IMODE(parent_stat.st_mode) | stat.S_IXGRP, follow_symlinks=False)
 
 

archivebox/tests/test_permissions.py

@@ -45,6 +45,22 @@ def test_root_preserves_existing_non_root_data_dir_owner():
     ) == (1001, 1002)
 
 
+def test_root_uses_owner_primary_group_for_mixed_user_root_data_dir():
+    assert select_archivebox_user(
+        running_uid=0,
+        running_gid=0,
+        effective_uid=0,
+        effective_gid=0,
+        sudo_uid=1001,
+        sudo_gid=1002,
+        data_dir_uid=1001,
+        data_dir_gid=0,
+        data_dir_owner_gid=1002,
+        account_uid=911,
+        account_gid=911,
+    ) == (1001, 1002)
+
+
 def test_root_uses_archivebox_account_for_unknown_data_dir_owner():
     assert select_archivebox_user(
         running_uid=0,
@@ -105,14 +121,23 @@ def test_root_hands_off_root_or_archivebox_owned_collection_boundaries():
     )
 
 
-def test_root_init_hands_off_only_an_empty_data_dir(tmp_path):
+def test_root_setup_commands_hand_off_only_an_empty_data_dir(tmp_path):
     from archivebox.config.permissions import root_data_dir_handoff_paths
 
-    assert root_data_dir_handoff_paths(tmp_path, ["archivebox", "init"]) == (tmp_path,)
+    setup_commands = (
+        ["archivebox", "init"],
+        ["archivebox", "install"],
+        ["archivebox", "server", "--init"],
+        ["archivebox", "server", "--quick-init"],
+        ["archivebox", "add", "--init", "https://example.com"],
+    )
+    for argv in setup_commands:
+        assert root_data_dir_handoff_paths(tmp_path, argv) == (tmp_path,)
 
     unrelated = tmp_path / "unrelated.txt"
     unrelated.write_text("keep root ownership")
-    assert root_data_dir_handoff_paths(tmp_path, ["archivebox", "init"]) == ()
+    for argv in setup_commands:
+        assert root_data_dir_handoff_paths(tmp_path, argv) == ()
 
 
 def test_existing_collection_handoff_is_bounded_to_known_top_level_paths(tmp_path):