From c8f7a77430148071c56b89bd4a7aaf15b5b95df7 Mon Sep 17 00:00:00 2001 From: Nick Sweeting Date: Fri, 14 Aug 2026 01:53:10 -0700 Subject: [PATCH] Harden root collection ownership handoff --- .github/workflows/pip.yml | 28 +++++++++++++++++++++++++ archivebox/config/django.py | 4 ++-- archivebox/config/permissions.py | 18 +++++++++------- archivebox/tests/test_permissions.py | 31 +++++++++++++++++++++++++--- 4 files changed, 69 insertions(+), 12 deletions(-) diff --git a/.github/workflows/pip.yml b/.github/workflows/pip.yml index ba5b326e..2c05a9fe 100755 --- a/.github/workflows/pip.yml +++ b/.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 diff --git a/archivebox/config/django.py b/archivebox/config/django.py index cee1f576..3b4daec1 100644 --- a/archivebox/config/django.py +++ b/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 diff --git a/archivebox/config/permissions.py b/archivebox/config/permissions.py index 27c3e6e7..7e71a9f8 100644 --- a/archivebox/config/permissions.py +++ b/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) diff --git a/archivebox/tests/test_permissions.py b/archivebox/tests/test_permissions.py index a9d56a5c..52c260d6 100644 --- a/archivebox/tests/test_permissions.py +++ b/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):