From aa5f54815a817f422944f11d4ab056c94349afd0 Mon Sep 17 00:00:00 2001 From: Erik Skultety Date: Tue, 31 Oct 2023 18:59:45 +0100 Subject: [PATCH] package_managers: yarn: Refuse projects using zero-install workflow The concept of zero installs, i.e. no install needed (git clone is sufficient), is inherently flawed for a number of reasons: - taking over maintenance (by the means of manual updates) of a project's dependencies by baking their sources in to the given project's repository - creating unnecessary bloat (often in form of binary formats) in the repository - moving the trust in package contents from the official packaging tooling and official public registries to a given project which doesn't really solve the biggest security problem of many public packaging repositories - unvetted contents just to mention a few. In context of Yarn what the above would mean is checking in dependencies' ZIP files into the repository. While that may sound like an acceptable use case since Yarn can verify integrity of the ZIP archives, some dependencies (due to e.g. post-install scripts) may end up being unpacked into a .yarn/unplugged directory, effectively creating an exploded node_modules/ dependency tree hierarchy inside the repository which would be needed for the zero install use case to work. However, we would have to employ a complex methodology (still preventing arbitrary code execution) of reliably verifying such dependencies in order to produce an accurate SBOM. Since we already reject projects containing 'node_modules' directory inside the repository for NPM, we can use it as a precedent here. The whole situation would be different if Yarn provided a mechanism to verify integrity of 'unplugged' contents the same way it does it for ZIP files, but unfortunately it doesn't [1]. As a result of this patch some test variants involving the zero-install use case which no longer applies have been adjusted accordingly and dedicated test cases dealing with zero installs were added. [1] Even if one sets the 'immutablePatterns' [2] YarnRc configuration option to something like '**/.yarn/unplugged/**' Yarn doesn't seem to care about the glob pattern unless the whole .yarn/unplugged//node_modules/ subdirectory of a given unplugged package tree would end up being removed in which case Yarn finally notices and throws a immutable cache error: The checksum for **/.yarn/unplugged/**/* has been modified by this install, which is explicitly forbidden [2] https://v3.yarnpkg.com/configuration/yarnrc#immutablePatterns Signed-off-by: Erik Skultety --- cachi2/core/package_managers/yarn/main.py | 29 ++++----- tests/unit/package_managers/yarn/test_main.py | 37 +++++++----- .../package_managers/yarn/test_project.py | 59 +++++++++++-------- 3 files changed, 69 insertions(+), 56 deletions(-) diff --git a/cachi2/core/package_managers/yarn/main.py b/cachi2/core/package_managers/yarn/main.py index 41d8fda96..4b2190703 100644 --- a/cachi2/core/package_managers/yarn/main.py +++ b/cachi2/core/package_managers/yarn/main.py @@ -40,14 +40,20 @@ def _resolve_yarn_project(project: Project, output_dir: RootedPath) -> list[Comp _configure_yarn_version(project) + if project.is_zero_installs: + raise PackageRejected( + ("Yarn zero install detected, PnP zero installs are unsupported by cachi2"), + solution=( + "Please convert your project to a regular install-based one.\n" + "Depending on whether you use Yarn's PnP or a different node linker Yarn setting " + "make sure to remove '.yarn/cache' or 'node_modules' directories respectively." + ), + ) + try: _set_yarnrc_configuration(project, output_dir) packages = resolve_packages(project.source_dir) - - if project.is_zero_installs: - _check_yarn_cache(project.source_dir) - else: - _fetch_dependencies(project.source_dir, output_dir) + _fetch_dependencies(project.source_dir, output_dir) finally: _undo_changes(project) @@ -97,8 +103,7 @@ def _configure_yarn_version(project: Project) -> None: def _set_yarnrc_configuration(project: Project, output_dir: RootedPath) -> None: """Set all the necessary configuration in yarnrc for the project processing. - :param project: the configuration changes dependending on if the project uses the zero-installs - or the regular workflow. + :param project: a Project instance :param output_dir: in case the dependencies need to be fetched, this is where they will be downloaded to. """ @@ -112,13 +117,9 @@ def _set_yarnrc_configuration(project: Project, output_dir: RootedPath) -> None: yarn_rc.enable_telemetry = False yarn_rc.ignore_path = True yarn_rc.unsafe_http_whitelist = [] - - if project.is_zero_installs: - yarn_rc.enable_immutable_cache = True - else: - yarn_rc.enable_mirror = True - yarn_rc.enable_scripts = False - yarn_rc.global_folder = str(output_dir) + yarn_rc.enable_mirror = True + yarn_rc.enable_scripts = False + yarn_rc.global_folder = str(output_dir) yarn_rc.write() diff --git a/tests/unit/package_managers/yarn/test_main.py b/tests/unit/package_managers/yarn/test_main.py index f5387d599..9b827c29b 100644 --- a/tests/unit/package_managers/yarn/test_main.py +++ b/tests/unit/package_managers/yarn/test_main.py @@ -9,6 +9,7 @@ from cachi2.core.package_managers.yarn.main import ( _configure_yarn_version, _fetch_dependencies, + _resolve_yarn_project, _set_yarnrc_configuration, ) from cachi2.core.package_managers.yarn.project import YarnRc @@ -122,19 +123,27 @@ def test_fetch_dependencies(mock_yarn_cmd: mock.Mock, rooted_tmp_path: RootedPat assert str(exc_info.value) == "berryscary" -@pytest.mark.parametrize( - "is_zero_installs", - ( - pytest.param(True, id="zero-installs-project"), - pytest.param(False, id="regular-workflow-project"), - ), -) +@mock.patch("cachi2.core.package_managers.yarn.main._configure_yarn_version") +def test_resolve_zero_installs_fail( + mock_configure_yarn_version: mock.Mock, rooted_tmp_path: RootedPath +) -> None: + mock_configure_yarn_version.return_value = None + project = mock.Mock() + project.is_zero_installs = True + output_dir = rooted_tmp_path.join_within_root("cachi2-output") + + with pytest.raises( + PackageRejected, + match=("Yarn zero install detected, PnP zero installs are unsupported " "by cachi2"), + ): + _resolve_yarn_project(project, output_dir) + + @mock.patch("cachi2.core.package_managers.yarn.project.YarnRc.write") -def test_set_yarnrc_configuration(mock_write: mock.Mock, is_zero_installs: bool) -> None: +def test_set_yarnrc_configuration(mock_write: mock.Mock) -> None: yarn_rc = YarnRc(RootedPath("/tmp/.yarnrc.yml"), {}) project = mock.Mock() - project.is_zero_installs = is_zero_installs project.yarn_rc = yarn_rc output_dir = RootedPath("/tmp/output") @@ -144,20 +153,16 @@ def test_set_yarnrc_configuration(mock_write: mock.Mock, is_zero_installs: bool) expected_data = { "checksumBehavior": "throw", "enableImmutableInstalls": True, + "enableMirror": True, + "enableScripts": False, "enableStrictSsl": True, "enableTelemetry": False, + "globalFolder": "/tmp/output", "ignorePath": True, "unsafeHttpWhitelist": [], "pnpMode": "strict", "plugins": [], } - if project.is_zero_installs: - expected_data["enableImmutableCache"] = True - else: - expected_data["enableMirror"] = True - expected_data["enableScripts"] = False - expected_data["globalFolder"] = "/tmp/output" - assert yarn_rc._data == expected_data assert mock_write.called_once() diff --git a/tests/unit/package_managers/yarn/test_project.py b/tests/unit/package_managers/yarn/test_project.py index 8f95c05f6..a6d03dbe4 100644 --- a/tests/unit/package_managers/yarn/test_project.py +++ b/tests/unit/package_managers/yarn/test_project.py @@ -190,25 +190,21 @@ def _add_mock_yarn_cache_file(cache_path: RootedPath) -> None: file.path.touch() -@pytest.mark.parametrize( - "is_zero_installs", - ( - pytest.param(True, id="zero-installs-project"), - pytest.param(False, id="regular-workflow-project"), - ), -) -def test_parse_project_folder(rooted_tmp_path: RootedPath, is_zero_installs: bool) -> None: +def _setup_zero_installs(nodeLinker: str, rooted_tmp_path: RootedPath) -> None: + if nodeLinker == "pnp" or nodeLinker == "": + _add_mock_yarn_cache_file(rooted_tmp_path.join_within_root("./.custom/cache")) + else: + rooted_tmp_path.join_within_root("node_modules").path.mkdir() + + +def test_parse_project_folder(rooted_tmp_path: RootedPath) -> None: _prepare_package_json_file(rooted_tmp_path, VALID_PACKAGE_JSON_FILE) _prepare_yarnrc_file(rooted_tmp_path, VALID_YARNRC_FILE) cache_path = "./.custom/cache" - if is_zero_installs: - _add_mock_yarn_cache_file(rooted_tmp_path.join_within_root(cache_path)) - project = Project.from_source_dir(rooted_tmp_path) - assert project.is_zero_installs == is_zero_installs assert project.yarn_cache == rooted_tmp_path.join_within_root(cache_path) assert project.yarn_rc is not None @@ -216,24 +212,11 @@ def test_parse_project_folder(rooted_tmp_path: RootedPath, is_zero_installs: boo assert project.package_json._path == rooted_tmp_path.join_within_root("package.json") -@pytest.mark.parametrize( - "is_zero_installs", - ( - pytest.param(True, id="zero-installs-project"), - pytest.param(False, id="regular-workflow-project"), - ), -) -def test_parse_project_folder_without_yarnrc( - rooted_tmp_path: RootedPath, is_zero_installs: bool -) -> None: +def test_parse_project_folder_without_yarnrc(rooted_tmp_path: RootedPath) -> None: _prepare_package_json_file(rooted_tmp_path, VALID_PACKAGE_JSON_FILE) - if is_zero_installs: - _add_mock_yarn_cache_file(rooted_tmp_path.join_within_root("./.yarn/cache")) - project = Project.from_source_dir(rooted_tmp_path) - assert project.is_zero_installs == is_zero_installs assert project.yarn_cache == rooted_tmp_path.join_within_root("./.yarn/cache") assert project.yarn_rc._data == {} @@ -374,3 +357,27 @@ def test_get_semver_from_package_manager( def test_get_semver_from_package_manager_fail(package_manager: str, expected_error: str) -> None: with pytest.raises(UnexpectedFormat, match=re.escape(expected_error)): get_semver_from_package_manager(package_manager) + + +@pytest.mark.parametrize( + "is_zero_installs, nodeLinker", + [ + pytest.param(True, "pnp", id="nodeLinker-pnp"), + pytest.param(True, "pnpm", id="nodeLinker-pnpm"), + pytest.param(True, "node-modules", id="nodeLinker-node-modules"), + pytest.param(True, "", id="nodeLinker-empty-use-default"), + pytest.param(False, "", id="regular-workflow"), + ], +) +def test_zero_installs_detection( + rooted_tmp_path: RootedPath, is_zero_installs: bool, nodeLinker: str +) -> None: + yarn_rc = VALID_YARNRC_FILE.replace("nodeLinker: pnp", f"nodeLinker: {nodeLinker}") + + _prepare_package_json_file(rooted_tmp_path, VALID_PACKAGE_JSON_FILE) + _prepare_yarnrc_file(rooted_tmp_path, yarn_rc) + project = Project.from_source_dir(rooted_tmp_path) + + if is_zero_installs: + _setup_zero_installs(nodeLinker, rooted_tmp_path) + assert project.is_zero_installs is is_zero_installs