Skip to content

Commit

Permalink
cachi2: allow only relative paths
Browse files Browse the repository at this point in the history
For security reasons, only relative paths within cloned remote source
can be specified by users

Don't allow to point to symlink pointing out of cloned remote source

Signed-off-by: Martin Basti <[email protected]>
  • Loading branch information
MartinBasti committed Dec 6, 2024
1 parent d91453c commit 9bd3a64
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 1 deletion.
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

0 comments on commit 9bd3a64

Please sign in to comment.