Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cachi2: allow only relative paths #2137

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion atomic_reactor/plugins/cachi2_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand Down
44 changes: 44 additions & 0 deletions atomic_reactor/utils/cachi2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
28 changes: 28 additions & 0 deletions tests/plugins/test_cachi2_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
95 changes: 95 additions & 0 deletions tests/utils/test_cachi2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
{
Expand Down
Loading