From 6e8e6b693d869ea93ad06df494fbe17395d4e87b Mon Sep 17 00:00:00 2001 From: Heitor Neiva Date: Fri, 30 Jan 2026 12:23:28 -0800 Subject: [PATCH] feat(iscript) Bug 1812480 - Change file ownership before creating pkgs --- iscript/src/iscript/mac.py | 42 ++++++++++++++++++++++++++++---------- iscript/tests/test_mac.py | 11 +++++++++- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/iscript/src/iscript/mac.py b/iscript/src/iscript/mac.py index c6e001707..865317732 100644 --- a/iscript/src/iscript/mac.py +++ b/iscript/src/iscript/mac.py @@ -1030,6 +1030,7 @@ async def create_pkg_files(config, sign_config, all_paths, requirements_plist_pa cmd_opts = [] if sign_config.get("pkg_cert_id"): cmd_opts = ["--keychain", sign_config["signing_keychain"], "--sign", sign_config["pkg_cert_id"]] + # Setup app metadata before we chown + pkgbuild for app in all_paths: # call set_app_path_and_name because we may not have called sign_app() earlier set_app_path_and_name(app) @@ -1037,17 +1038,36 @@ async def create_pkg_files(config, sign_config, all_paths, requirements_plist_pa app.tmp_pkg_path2 = app.app_path.replace(".appex", ".tmp2.pkg").replace(".app", ".tmp2.pkg") app.pkg_path = app.app_path.replace(".appex", ".pkg").replace(".app", ".pkg") app.pkg_name = os.path.basename(app.pkg_path) - cmd = ( - "pkgbuild", - "--install-location", - "/Applications", - *cmd_opts, - "--component", - app.app_path, - app.tmp_pkg_path1, - ) - futures.append(_retry_run_cmd_semaphore(semaphore=semaphore, cmd=cmd, cwd=app.parent_dir)) - await raise_future_exceptions(futures) + try: + for app in all_paths: + # Set app ownership + await run_command( + ["sudo", "chown", "-R", "root:admin", app.app_path], + cwd=app.parent_dir, + exception=IScriptError, + ) + + # Build pkg + cmd = ( + "pkgbuild", + "--install-location", + "/Applications", + *cmd_opts, + "--ownership=preserve", + "--component", + app.app_path, + app.tmp_pkg_path1, + ) + futures.append(_retry_run_cmd_semaphore(semaphore=semaphore, cmd=cmd, cwd=app.parent_dir)) + await raise_future_exceptions(futures) + finally: + # Reset ownership so we can cleanup after + for app in all_paths: + await run_command( + ["sudo", "chown", "-R", f"{os.getuid()}:{os.getgid()}", app.app_path], + cwd=app.parent_dir, + exception=IScriptError, + ) futures = [] for app in all_paths: pb_opts = [] diff --git a/iscript/tests/test_mac.py b/iscript/tests/test_mac.py index 1a52f5fe6..cd00d0b41 100644 --- a/iscript/tests/test_mac.py +++ b/iscript/tests/test_mac.py @@ -10,6 +10,7 @@ import arrow import mock import pexpect +import re import pytest from scriptworker_client.aio import retry_async from scriptworker_client.utils import makedirs @@ -809,12 +810,20 @@ async def fake_retry_async(**kwargs): assert "--product" not in cmd assert requirements_path not in cmd + async def fake_run_command(*args, **kwargs): + cmd = args[0] + assert len(cmd) == 5 + assert cmd[0:3] == ["sudo", "chown", "-R"] + assert re.compile(r"\w+:\w+").match(cmd[3]) + assert cmd[-1].startswith("foo/") and cmd[-1].endswith(".app") + sign_config = {"pkg_cert_id": pkg_cert_id, "signing_keychain": "signing.keychain"} config = {"concurrency_limit": 2} all_paths = [] for i in range(3): - all_paths.append(mac.App(app_path="foo/{}/{}.app".format(i, i), parent_dir="foo/{}".format(i))) + all_paths.append(mac.App(app_path=f"foo/{i}/{i}.app", parent_dir=f"foo/{i}")) mocker.patch.object(mac, "retry_async", new=fake_retry_async) + mocker.patch.object(mac, "run_command", new=fake_run_command) mocker.patch.object(mac, "copy2", new=noop_sync) if should_raise: with pytest.raises(IScriptError):