refactor gomod vetting of local file dependencies

Since we no longer care if file deps are allowed, condense the path
checking into a single function that verifies that paths are not either
absolute or lead outside the repository

Signed-off-by: Taylor Madore <tmadore@redhat.com>

Author: taylormadore

cachito/workers/pkg_managers/gomod.py

@@ -459,10 +459,10 @@ def go_list_deps(pattern: Literal["./...", "all"]) -> Iterator[GoPackage]:
             }
             main_packages.append({"pkg": main_pkg, "pkg_deps": pkg_deps})
 
-        _vet_local_deps(main_module_deps, main_module_name, app_source_path, git_dir_path)
+        _vet_local_file_dep_paths(main_module_deps, app_source_path, git_dir_path)
         for package in main_packages:
             # Local dependencies are always relative to the main module, even for subpackages
-            _vet_local_deps(package["pkg_deps"], main_module_name, app_source_path, git_dir_path)
+            _vet_local_file_dep_paths(package["pkg_deps"], app_source_path, git_dir_path)
             _set_full_local_dep_relpaths(package["pkg_deps"], main_module_deps)
 
         return {
@@ -641,53 +641,31 @@ def _vendor_changed(git_dir: Path, app_dir: str) -> bool:
     return False
 
 
-def _vet_local_deps(
+def _vet_local_file_dep_paths(
     dependencies: List[dict],
-    module_name: str,
     app_source_path: Path,
     git_dir_path: Path,
 ) -> None:
-    """Fail if any local file dependency path is either absolute or outside the repository."""
+    """Fail if any local file dependency path is either outside the repository or absolute."""
     for dep in dependencies:
-        name = dep["name"]
         version = dep["version"]
 
-        if not version:
-            continue  # go stdlib
+        if not version:  # go stdlib
+            continue
 
         if version.startswith("."):
-            log.debug(
-                "Module %s wants to replace %s with a local dependency: %s",
-                module_name,
-                name,
-                version,
-            )
-            _validate_local_dependency_path(app_source_path, git_dir_path, version)
+            resolved_dep_path = Path(app_source_path, version).resolve()
+            if not resolved_dep_path.is_relative_to(git_dir_path.resolve()):
+                raise ValidationError(
+                    f"The local file dependency path {version} is outside the repository"
+                )
         elif version.startswith("/") or PureWindowsPath(version).root:
             # This will disallow paths starting with '/', '\' or '<drive letter>:\'
             raise UnsupportedFeature(
                 f"Absolute paths to gomod dependencies are not supported: {version}"
             )
 
 
-def _validate_local_dependency_path(
-    app_source_path: Path, git_dir_path: Path, dep_path: str
-) -> None:
-    """
-    Validate that the local dependency path is not outside the repository.
-
-    :param Path app_source_path: the full path to the application source code
-    :param Path git_dir_path: the full path to the git repository
-    :param str dep_path: the relative path for local replacements (the dep version)
-    :raise ValidationError: if the local dependency path is invalid
-    """
-    try:
-        resolved_dep_path = Path(app_source_path, dep_path).resolve()
-        resolved_dep_path.relative_to(git_dir_path.resolve())
-    except ValueError:
-        raise ValidationError(f"The local dependency path {dep_path} is outside the repository")
-
-
 def _set_full_local_dep_relpaths(pkg_deps: List[dict], main_module_deps: List[dict]):
     """
     Set full relative paths for all local go-package dependencies.

tests/test_workers/test_pkg_managers/test_gomod.py

@@ -73,13 +73,13 @@ def go_mod_file(tmp_path: Path, request: pytest.FixtureRequest) -> Path:
 @mock.patch("cachito.workers.pkg_managers.gomod.get_golang_version")
 @mock.patch("cachito.workers.pkg_managers.gomod.GoCacheTemporaryDirectory")
 @mock.patch("cachito.workers.pkg_managers.gomod._merge_bundle_dirs")
-@mock.patch("cachito.workers.pkg_managers.gomod._vet_local_deps")
+@mock.patch("cachito.workers.pkg_managers.gomod._vet_local_file_dep_paths")
 @mock.patch("cachito.workers.pkg_managers.gomod._set_full_local_dep_relpaths")
 @mock.patch("subprocess.run")
 def test_resolve_gomod(
     mock_run: mock.Mock,
     mock_set_full_relpaths: mock.Mock,
-    mock_vet_local_deps: mock.Mock,
+    mock_vet_local_file_dep_paths: mock.Mock,
     mock_merge_tree: mock.Mock,
     mock_temp_dir: mock.Mock,
     mock_golang_version: mock.Mock,
@@ -182,16 +182,14 @@ def test_resolve_gomod(
         str(RequestBundleDir(request["id"]).gomod_download_dir),
     )
 
-    expect_module_name = expect_gomod["module"]["name"]
     expect_module_deps = expect_gomod["module_deps"]
     expect_pkg_deps = expect_gomod["packages"][0]["pkg_deps"]
 
-    mock_vet_local_deps.assert_has_calls(
+    mock_vet_local_file_dep_paths.assert_has_calls(
         [
-            mock.call(expect_module_deps, expect_module_name, module_dir, module_dir),
+            mock.call(expect_module_deps, module_dir, module_dir),
             mock.call(
                 expect_pkg_deps,
-                expect_module_name,
                 module_dir,
                 module_dir,
             ),
@@ -687,26 +685,17 @@ def test_merge_files(file_1_content, file_2_content, result_file_content):
         assert_directories_equal(dir_2, dir_3)
 
 
-@mock.patch("cachito.workers.pkg_managers.gomod._validate_local_dependency_path")
-def test_vet_local_deps(mock_validate_dep_path):
+def test_vet_local_file_dep_paths():
     dependencies = [
-        {"name": "foo", "version": "./local/foo"},
-        {"name": "bar", "version": "v1.0.0"},
-        {"name": "baz", "version": "./local/baz"},
+        {"name": "stdlib-dep", "version": None},
+        {"name": "versioned-dep", "version": "v1.0.0"},
+        {"name": "local-file-dep", "version": "./local/foo"},
+        {"name": "local-file-dep-parent-dir", "version": "../local/bar"},
     ]
-    module_name = "some-module"
     app_dir = Path("/repo/some-module")
     git_dir = Path("/repo")
-    mock_validate_dep_path.return_value = None
 
-    gomod._vet_local_deps(dependencies, module_name, app_dir, git_dir)
-
-    mock_validate_dep_path.assert_has_calls(
-        [
-            mock.call(app_dir, git_dir, "./local/foo"),
-            mock.call(app_dir, git_dir, "./local/baz"),
-        ],
-    )
+    gomod._vet_local_file_dep_paths(dependencies, app_dir, git_dir)
 
 
 @pytest.mark.parametrize(
@@ -717,37 +706,29 @@ def test_vet_local_deps(mock_validate_dep_path):
         "C:\\Users\\user\\go\\src\\k8s.io\\kubectl",
     ],
 )
-def test_vet_local_deps_abspath(platform_specific_path):
+def test_vet_local_file_dep_paths_abspath(platform_specific_path):
     dependencies = [{"name": "foo", "version": platform_specific_path}]
     app_dir = Path("/some/path")
 
     expect_error = re.escape(
         f"Absolute paths to gomod dependencies are not supported: {platform_specific_path}"
     )
     with pytest.raises(UnsupportedFeature, match=expect_error):
-        gomod._vet_local_deps(dependencies, "some-module", app_dir, app_dir)
+        gomod._vet_local_file_dep_paths(dependencies, app_dir, app_dir)
 
 
-@pytest.mark.parametrize(
-    "app_dir, git_dir, dep_path, expect_error",
-    [
-        ("foo", "foo", "./..", True),
-        ("foo/bar", "foo", "./..", False),
-        ("foo/bar", "foo", "./../..", True),
-    ],
-)
-def test_validate_local_dependency_path(
-    tmp_path: Path, app_dir: str, git_dir: str, dep_path: str, expect_error: bool
-):
-    tmp_git_dir = tmp_path / git_dir
-    tmp_app_dir = tmp_path / app_dir
-    tmp_app_dir.mkdir(parents=True, exist_ok=True)
+def test_vet_local_file_dep_paths_outside_repo():
+    dependencies = [
+        {"name": "local-file-dep", "version": "../../local/foo"},
+    ]
+    app_dir = Path("/repo/some-module")
+    git_dir = Path("/repo")
 
-    if expect_error:
-        with pytest.raises(ValidationError):
-            gomod._validate_local_dependency_path(tmp_app_dir, tmp_git_dir, dep_path)
-    else:
-        gomod._validate_local_dependency_path(tmp_app_dir, tmp_git_dir, dep_path)
+    expect_error = re.escape(
+        f"The local file dependency path {dependencies[0]['version']} is outside the repository"
+    )
+    with pytest.raises(ValidationError, match=expect_error):
+        gomod._vet_local_file_dep_paths(dependencies, app_dir, git_dir)
 
 
 @pytest.mark.parametrize(