diff --git a/atomic_reactor/plugins/cachi2_init.py b/atomic_reactor/plugins/cachi2_init.py index 485c8c23b..ad09145ba 100644 --- a/atomic_reactor/plugins/cachi2_init.py +++ b/atomic_reactor/plugins/cachi2_init.py @@ -25,7 +25,7 @@ ) from atomic_reactor.plugin import Plugin from atomic_reactor.util import map_to_user_params -from atomic_reactor.utils.cachi2 import remote_source_to_cachi2, clone_only +from atomic_reactor.utils.cachi2 import remote_source_to_cachi2, clone_only, validate_paths class Cachi2InitPlugin(Plugin): @@ -129,6 +129,8 @@ def process_remote_sources(self) -> List[Dict[str, Any]]: remote_source_data["ref"] ) + validate_paths(source_path_app, remote_source_data.get("packages", {})) + if clone_only(remote_source_data): # OSBS is doing all work here self.write_metadata(source_path) diff --git a/atomic_reactor/utils/cachi2.py b/atomic_reactor/utils/cachi2.py index c2c7d943b..fe11a791c 100644 --- a/atomic_reactor/utils/cachi2.py +++ b/atomic_reactor/utils/cachi2.py @@ -9,10 +9,54 @@ """ from typing import Any, Callable, Dict, Optional, Tuple, List +from pathlib import Path +import os.path from packageurl import PackageURL +def validate_paths(repo_path: Path, remote_sources_packages: dict) -> None: + """Paths must be relative and within cloned repo""" + def is_path_ok(path_string): + path = Path(path_string) + if path.is_absolute(): + return False + + # using real repo to properly resolve and block bad symlinks + full_path = (repo_path/path).resolve() + + # using commonpath to be compatible with py3.8 + if os.path.commonpath((full_path, repo_path)) != str(repo_path): + return False + + return True + + for pkg_mgr, options in remote_sources_packages.items(): + if not options: + continue + + for option in options: + for key, val in option.items(): + if key == "path": + if not is_path_ok(val): + raise ValueError( + f"{pkg_mgr}:{key}: path '{val}' must be relative " + "within remote source repository" + ) + elif ( + pkg_mgr == "pip" and + key in ("requirements_files", "requirements_build_files") + ): + for v in val: + if not is_path_ok(v): + raise ValueError( + f"{pkg_mgr}:{key}: path '{v}' must be relative " + "within remote source repository" + ) + else: + raise ValueError(f"unexpected key '{key}' in '{pkg_mgr}' config") + + def remote_source_to_cachi2(remote_source: Dict[str, Any]) -> Dict[str, Any]: """Converts remote source into cachi2 expected params. diff --git a/tests/plugins/test_cachi2_init.py b/tests/plugins/test_cachi2_init.py index 4cfb192fa..e2dcbba4b 100644 --- a/tests/plugins/test_cachi2_init.py +++ b/tests/plugins/test_cachi2_init.py @@ -335,6 +335,34 @@ def test_multiple_remote_sources_non_unique_names(workflow): assert result is None +def test_path_out_of_repo(workflow, mocked_cachi2_init): + """Should fail when path is outside of repository""" + container_yaml_config = dedent("""\ + remote_sources: + - name: bit-different + remote_source: + repo: https://git.example.com/team/repo.git + ref: a55c00f45ec3dfee0c766cea3d395d6e21cc2e5a + packages: + gomod: + - path: "/out/of/repo" + """) + mock_repo_config(workflow, data=container_yaml_config) + + reactor_config = dedent("""\ + allow_multiple_remote_sources: True + """) + mock_reactor_config(workflow, reactor_config) + + err_msg = ( + "gomod:path: path '/out/of/repo' must be relative within remote source repository" + ) + with pytest.raises(ValueError) as exc_info: + mocked_cachi2_init(workflow).run() + + assert err_msg in str(exc_info) + + def test_dependency_replacements(workflow): run_plugin_with_args(workflow, dependency_replacements={"dep": "something"}, expect_error="Dependency replacements are not supported by Cachi2") diff --git a/tests/utils/test_cachi2.py b/tests/utils/test_cachi2.py index f8d59ca2a..67b4531b1 100644 --- a/tests/utils/test_cachi2.py +++ b/tests/utils/test_cachi2.py @@ -6,12 +6,15 @@ of the BSD license. See the LICENSE file for details. """ +from pathlib import Path + from atomic_reactor.utils.cachi2 import ( convert_SBOM_to_ICM, remote_source_to_cachi2, gen_dependency_from_sbom_component, generate_request_json, clone_only, + validate_paths, ) import pytest @@ -106,6 +109,98 @@ def test_remote_source_to_cachi2_conversion(input_remote_source, expected_cachi2 assert remote_source_to_cachi2(input_remote_source) == expected_cachi2 +@pytest.mark.parametrize("remote_source_packages,expected_err", [ + pytest.param( + {"gomod": [{"path": "/path/to/secret"}]}, + "gomod:path: path '/path/to/secret' must be relative within remote source repository", + id="absolute_path" + ), + pytest.param( + {"gomod": [{"unknown": "/path/to/secret"}]}, + "unexpected key 'unknown' in 'gomod' config", + id="unknown_option" + ), + pytest.param( + {"gomod": [{"path": "path/../../../to/secret"}]}, + ( + "gomod:path: path 'path/../../../to/secret' must be relative " + "within remote source repository" + ), + id="path_traversal" + ), + pytest.param( + { + "pip": [{ + "path": "/src/web", + }] + }, + "pip:path: path '/src/web' must be relative within remote source repository", + id="pip_absolute_path" + ), + pytest.param( + { + "pip": [{ + "requirements_files": ["requirements.txt", "/requirements-extras.txt"], + }] + }, + ( + "pip:requirements_files: path '/requirements-extras.txt' " + "must be relative within remote source repository" + ), + id="pip_requirements_files_absolute_path" + ), + pytest.param( + { + "pip": [{ + "requirements_build_files": [ + "requirements-build.txt", "/requirements-build-extras.txt" + ] + }] + }, + ( + "pip:requirements_build_files: path '/requirements-build-extras.txt' " + "must be relative within remote source repository" + ), + id="pip_requirements_build_files_absolute_path" + ) +]) +def test_remote_source_invalid_paths(tmpdir, remote_source_packages, expected_err): + """Test conversion of remote_source (cachito) configuration from container yaml + into cachi2 params""" + with pytest.raises(ValueError) as exc_info: + validate_paths(Path(tmpdir), remote_source_packages) + + assert str(exc_info.value) == expected_err + + +def test_remote_source_path_to_symlink_out_of_repo(tmpdir): + """If cloned repo contains symlink that points out fo repo, + path in packages shouldn't be allowed""" + tmpdir_path = Path(tmpdir) + + symlink_target = tmpdir_path/"dir_outside" + symlink_target.mkdir() + + cloned = tmpdir_path/'app' + cloned.mkdir() + + symlink = cloned/"symlink" + symlink.symlink_to(symlink_target, target_is_directory=True) + + remote_source_packages = { + "pip": [{ + "path": "symlink", + }] + } + + expected_err = "pip:path: path 'symlink' must be relative within remote source repository" + + with pytest.raises(ValueError) as exc_info: + validate_paths(cloned, remote_source_packages) + + assert str(exc_info.value) == expected_err + + @pytest.mark.parametrize(('sbom', 'expected_icm'), [ pytest.param( {