Repository: canonical/charmcraft

Issue:
The `pack` command does not validate the entrypoint unless it's specified
If `pack` is run without using the `--entrypoint` option, even if it defaults to `src/charm.py`, it's not running any validation on it (e.g. that is executable).

Discussion:

Patch:
diff --git a/charmcraft/commands/build.py b/charmcraft/commands/build.py
index e00ea4a9..e75c9bba 100644
--- a/charmcraft/commands/build.py
+++ b/charmcraft/commands/build.py
@@ -240,13 +240,28 @@ class Builder:
                     "--entrypoint not supported when charm-entrypoint "
                     "specified in charmcraft.yaml"
                 )
+            entrypoint = self._special_charm_part.get("charm-entrypoint")
+        elif self.entrypoint:
+            entrypoint = self.entrypoint
+            self.entrypoint = None
         else:
-            if self.entrypoint:
-                rel_entrypoint = self.entrypoint.relative_to(self.charmdir)
-                self._special_charm_part["charm-entrypoint"] = str(rel_entrypoint)
-                self.entrypoint = None
-            else:
-                self._special_charm_part["charm-entrypoint"] = "src/charm.py"
+            entrypoint = "src/charm.py"
+
+        # store the entrypoint always relative to the project's path (no matter if the origin
+        # was relative or absolute)
+        rel_entrypoint = (self.charmdir / entrypoint).relative_to(self.charmdir)
+        self._special_charm_part["charm-entrypoint"] = rel_entrypoint.as_posix()
+
+        # validate the entrypoint (no matter if it came from the config or it's the default)
+        filepath = (self.charmdir / self._special_charm_part["charm-entrypoint"]).resolve()
+        if not filepath.exists():
+            raise CraftError("Charm entry point was not found: {!r}".format(str(filepath)))
+        if self.charmdir not in filepath.parents:
+            raise CraftError(
+                "Charm entry point must be inside the project: {!r}".format(str(filepath))
+            )
+        if not os.access(filepath, os.X_OK):
+            raise CraftError("Charm entry point must be executable: {!r}".format(str(filepath)))
 
     def _set_prime_filter(self):
         """Add mandatory and optional charm files to the prime filter.
@@ -571,10 +586,7 @@ class Validator:
         return filepath
 
     def validate_requirement(self, filepaths):
-        """Validate that the given requirement(s) (if any) exist.
-
-        If not specified, default to requirements.txt if there.
-        """
+        """Validate that the given requirement(s) (if any) exist."""
         if filepaths is None:
             return []
 


Test patch:
diff --git a/tests/commands/test_build.py b/tests/commands/test_build.py
index 9354b0a0..29982d1a 100644
--- a/tests/commands/test_build.py
+++ b/tests/commands/test_build.py
@@ -81,7 +81,7 @@ def get_builder(
 
 
 @pytest.fixture
-def basic_project(tmp_path):
+def basic_project(tmp_path, monkeypatch):
     """Create a basic Charmcraft project."""
     build_dir = tmp_path / BUILD_DIRNAME
     build_dir.mkdir()
@@ -109,6 +109,7 @@ def basic_project(tmp_path):
     src_dir.mkdir()
     charm_script = src_dir / "charm.py"
     charm_script.write_bytes(b"all the magic")
+    charm_script.chmod(0o755)
 
     # the license file
     license = tmp_path / "LICENSE"
@@ -122,6 +123,9 @@ def basic_project(tmp_path):
     readme = tmp_path / "README.md"
     readme.write_text("README content")
 
+    # paths are relative, make all tests to run in the project's directory
+    monkeypatch.chdir(tmp_path)
+
     yield tmp_path
 
 
@@ -497,7 +501,6 @@ def test_build_basic_complete_structure(basic_project, caplog, monkeypatch, conf
     caplog.set_level(logging.WARNING, logger="charmcraft")
     host_base = get_host_as_base()
     host_arch = host_base.architectures[0]
-    monkeypatch.chdir(basic_project)  # so the zip file is left in the temp dir
     builder = get_builder(config)
 
     # save original metadata and verify later
@@ -534,13 +537,12 @@ def test_build_basic_complete_structure(basic_project, caplog, monkeypatch, conf
     assert caplog.records == []
 
 
-def test_build_error_without_metadata_yaml(basic_project, monkeypatch):
+def test_build_error_without_metadata_yaml(basic_project):
     """Validate error if trying to build project without metadata.yaml."""
     metadata = basic_project / CHARM_METADATA
     metadata.unlink()
 
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
 
     msg = re.escape(
         "Cannot read the metadata.yaml file: FileNotFoundError(2, 'No such file or directory')"
@@ -775,7 +777,6 @@ def test_build_project_is_cwd(
     mock_capture_logs_from_instance,
     mock_instance,
     mock_provider,
-    monkeypatch,
 ):
     """Test cases for base-index parameter."""
     emit.set_mode(EmitterMode.NORMAL)
@@ -796,7 +797,6 @@ def test_build_project_is_cwd(
     config = load(basic_project)
     builder = get_builder(config)
 
-    monkeypatch.chdir(basic_project)
     zipnames = builder.run([0])
 
     assert zipnames == [
@@ -829,6 +829,7 @@ def test_build_project_is_not_cwd(
     mock_capture_logs_from_instance,
     mock_instance,
     mock_provider,
+    monkeypatch,
 ):
     """Test cases for base-index parameter."""
     emit.set_mode(EmitterMode.NORMAL)
@@ -849,6 +850,7 @@ def test_build_project_is_not_cwd(
     config = load(basic_project)
     builder = get_builder(config)
 
+    monkeypatch.chdir("/")  # make the working directory NOT the project's one
     zipnames = builder.run([0])
 
     assert zipnames == [
@@ -898,7 +900,6 @@ def test_build_bases_index_scenarios_provider(
     mock_capture_logs_from_instance,
     mock_instance,
     mock_provider,
-    monkeypatch,
 ):
     """Test cases for base-index parameter."""
     emit.set_mode(mode)
@@ -923,7 +924,6 @@ def test_build_bases_index_scenarios_provider(
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config)
 
     zipnames = builder.run([0])
@@ -1102,7 +1102,6 @@ def test_build_bases_index_scenarios_managed_mode(basic_project, monkeypatch, tm
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config)
 
     monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
@@ -1152,7 +1151,6 @@ def test_build_error_no_match_with_charmcraft_yaml(
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config)
 
     # Managed bases build.
@@ -1201,7 +1199,7 @@ def test_build_error_no_match_with_charmcraft_yaml(
 
 
 @pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported")
-def test_build_package_tree_structure(tmp_path, monkeypatch, config):
+def test_build_package_tree_structure(tmp_path, config):
     """The zip file is properly built internally."""
     # the metadata
     metadata_data = {"name": "name-from-metadata"}
@@ -1253,7 +1251,6 @@ def test_build_package_tree_structure(tmp_path, monkeypatch, config):
             "run-on": [Base(name="xname", channel="xchannel", architectures=["xarch1"])],
         }
     )
-    monkeypatch.chdir(tmp_path)  # so the zip file is left in the temp dir
     builder = get_builder(config, entrypoint="whatever")
     zipname = builder.handle_package(to_be_zipped_dir, bases_config)
 
@@ -1269,7 +1266,7 @@ def test_build_package_tree_structure(tmp_path, monkeypatch, config):
     assert zf.read("linkeddir/file_ext") == b"external file"  # from file in the outside linked dir
 
 
-def test_build_package_name(tmp_path, monkeypatch, config):
+def test_build_package_name(tmp_path, config):
     """The zip file name comes from the metadata."""
     to_be_zipped_dir = tmp_path / BUILD_DIRNAME
     to_be_zipped_dir.mkdir()
@@ -1287,7 +1284,6 @@ def test_build_package_name(tmp_path, monkeypatch, config):
             "run-on": [Base(name="xname", channel="xchannel", architectures=["xarch1"])],
         }
     )
-    monkeypatch.chdir(tmp_path)  # so the zip file is left in the temp dir
     builder = get_builder(config, entrypoint="whatever")
     zipname = builder.handle_package(to_be_zipped_dir, bases_config)
 
@@ -1307,10 +1303,15 @@ def test_build_with_entrypoint_argument_issues_dn04(basic_project, emitter, monk
     )
 
 
-def test_build_entrypoint_from_parts(basic_project, monkeypatch):
+@pytest.mark.parametrize("absolute_entrypoint", [True, False])
+def test_build_entrypoint_from_parts_ok(basic_project, monkeypatch, absolute_entrypoint):
     """Test cases for base-index parameter."""
     host_base = get_host_as_base()
     charmcraft_file = basic_project / "charmcraft.yaml"
+    if absolute_entrypoint:
+        entrypoint_path = basic_project / "my_entrypoint.py"
+    else:
+        entrypoint_path = "my_entrypoint.py"
     charmcraft_file.write_text(
         dedent(
             f"""\
@@ -1324,13 +1325,12 @@ def test_build_entrypoint_from_parts(basic_project, monkeypatch):
                         channel: {host_base.channel!r}
                 parts:
                   charm:
-                    charm-entrypoint: "my_entrypoint.py"
+                    charm-entrypoint: {entrypoint_path}
                     charm-requirements: ["reqs.txt"]
                 """
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None)
 
     entrypoint = basic_project / "my_entrypoint.py"
@@ -1395,7 +1395,6 @@ def test_build_entrypoint_from_commandline(basic_project, monkeypatch):
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     entrypoint = basic_project / "my_entrypoint.py"
     builder = get_builder(config, entrypoint=entrypoint)
 
@@ -1438,7 +1437,7 @@ def test_build_entrypoint_from_commandline(basic_project, monkeypatch):
     )
 
 
-def test_build_entrypoint_default(basic_project, monkeypatch):
+def test_build_entrypoint_default_ok(basic_project, monkeypatch):
     """Test cases for base-index parameter."""
     host_base = get_host_as_base()
     charmcraft_file = basic_project / "charmcraft.yaml"
@@ -1460,7 +1459,6 @@ def test_build_entrypoint_default(basic_project, monkeypatch):
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None, force=True)
 
     monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
@@ -1523,7 +1521,6 @@ def test_build_entrypoint_from_both(basic_project, monkeypatch):
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     entrypoint = basic_project / "my_entrypoint.py"
 
     builder = get_builder(config, entrypoint=entrypoint, force=True)
@@ -1539,6 +1536,157 @@ def test_build_entrypoint_from_both(basic_project, monkeypatch):
     )
 
 
+def test_build_entrypoint_default_missing(basic_project, monkeypatch):
+    """The entrypoint is not specified and there is no file for its default."""
+    host_base = get_host_as_base()
+    charmcraft_yaml_content = f"""\
+       type: charm
+       bases:
+         - build-on:
+             - name: {host_base.name!r}
+               channel: {host_base.channel!r}
+           run-on:
+             - name: {host_base.name!r}
+               channel: {host_base.channel!r}
+       parts:
+         charm:
+            charm-requirements: ["reqs.txt"]
+       """
+    (basic_project / "charmcraft.yaml").write_text(dedent(charmcraft_yaml_content))
+    config = load(basic_project)
+    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
+
+    # remove the default entrypoint to make it fail
+    entrypoint = basic_project / "src" / "charm.py"
+    entrypoint.unlink()
+
+    builder = get_builder(config, entrypoint=None, force=True)
+    with pytest.raises(CraftError) as cm:
+        builder.run([0])
+    assert str(cm.value) == f"Charm entry point was not found: {str(entrypoint)!r}"
+
+
+@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported")
+def test_build_entrypoint_default_nonexec(basic_project, monkeypatch):
+    """The entrypoint is not specified and the file for its default is not executable."""
+    host_base = get_host_as_base()
+    charmcraft_yaml_content = f"""\
+       type: charm
+       bases:
+         - build-on:
+             - name: {host_base.name!r}
+               channel: {host_base.channel!r}
+           run-on:
+             - name: {host_base.name!r}
+               channel: {host_base.channel!r}
+       parts:
+         charm:
+            charm-requirements: ["reqs.txt"]
+       """
+    (basic_project / "charmcraft.yaml").write_text(dedent(charmcraft_yaml_content))
+    config = load(basic_project)
+    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
+
+    # remove exec properties from the file
+    entrypoint = basic_project / "src" / "charm.py"
+    entrypoint.chmod(0o666)
+
+    builder = get_builder(config, entrypoint=None, force=True)
+    with pytest.raises(CraftError) as cm:
+        builder.run([0])
+    assert str(cm.value) == f"Charm entry point must be executable: {str(entrypoint)!r}"
+
+
+def test_build_entrypoint_from_parts_missing(basic_project, monkeypatch):
+    """The specified entrypoint does not point to an existing file."""
+    host_base = get_host_as_base()
+    charmcraft_yaml_content = f"""\
+        type: charm
+        bases:
+          - build-on:
+              - name: {host_base.name!r}
+                channel: {host_base.channel!r}
+            run-on:
+              - name: {host_base.name!r}
+                channel: {host_base.channel!r}
+        parts:
+          charm:
+            charm-entrypoint: my_entrypoint.py
+        """
+    (basic_project / "charmcraft.yaml").write_text(dedent(charmcraft_yaml_content))
+    config = load(basic_project)
+    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
+
+    builder = get_builder(config, entrypoint=None)
+    entrypoint = basic_project / "my_entrypoint.py"
+    with pytest.raises(CraftError) as cm:
+        builder.run([0])
+    assert str(cm.value) == f"Charm entry point was not found: {str(entrypoint)!r}"
+
+
+def test_build_entrypoint_from_parts_outside(basic_project, monkeypatch):
+    """The specified entrypoint not points to a file outside the project."""
+    host_base = get_host_as_base()
+    outside = basic_project.parent
+    charmcraft_yaml_content = f"""\
+        type: charm
+        bases:
+          - build-on:
+              - name: {host_base.name!r}
+                channel: {host_base.channel!r}
+            run-on:
+              - name: {host_base.name!r}
+                channel: {host_base.channel!r}
+        parts:
+          charm:
+            charm-entrypoint: ../my_entrypoint.py
+        """
+    (basic_project / "charmcraft.yaml").write_text(dedent(charmcraft_yaml_content))
+    config = load(basic_project)
+    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
+
+    # file exists and it's fine
+    entrypoint = outside / "my_entrypoint.py"
+    entrypoint.touch(mode=0o755)
+
+    builder = get_builder(config, entrypoint=None)
+    with pytest.raises(CraftError) as cm:
+        builder.run([0])
+    assert str(cm.value) == (
+        f"Charm entry point must be inside the project: {str(entrypoint.resolve())!r}"
+    )
+
+
+@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported")
+def test_build_entrypoint_from_parts_nonexec(basic_project, monkeypatch):
+    """The specified entrypoint not points to a non-executable file."""
+    host_base = get_host_as_base()
+    charmcraft_yaml_content = f"""\
+        type: charm
+        bases:
+          - build-on:
+              - name: {host_base.name!r}
+                channel: {host_base.channel!r}
+            run-on:
+              - name: {host_base.name!r}
+                channel: {host_base.channel!r}
+        parts:
+          charm:
+            charm-entrypoint: entrypoint.py
+        """
+    (basic_project / "charmcraft.yaml").write_text(dedent(charmcraft_yaml_content))
+    config = load(basic_project)
+    monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
+
+    entrypoint = basic_project / "entrypoint.py"
+    entrypoint.touch(mode=0o666)
+
+    builder = get_builder(config, entrypoint=None)
+    with pytest.raises(CraftError) as cm:
+        builder.run([0])
+    assert str(cm.value) == f"Charm entry point must be executable: {str(entrypoint)!r}"
+
+
 def test_build_with_requirement_argment_issues_dn05(basic_project, emitter, monkeypatch):
     """Test cases for base-index parameter."""
     config = load(basic_project)
@@ -1576,7 +1724,6 @@ def test_build_requirements_from_parts(basic_project, monkeypatch):
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None, force=True)
 
     reqs = basic_project / "reqs.txt"
@@ -1641,7 +1788,6 @@ def test_build_requirements_from_commandline(basic_project, monkeypatch, emitter
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None, force=True, requirement=["reqs.txt"])
 
     reqs = basic_project / "reqs.txt"
@@ -1706,7 +1852,6 @@ def test_build_requirements_default(basic_project, monkeypatch, emitter):
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None, force=True)
 
     # create a requirements.txt file
@@ -1771,7 +1916,6 @@ def test_build_requirements_no_requirements_txt(basic_project, monkeypatch, emit
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None, force=True)
 
     monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
@@ -1832,7 +1976,6 @@ def test_build_requirements_from_both(basic_project, monkeypatch, emitter):
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None, force=True, requirement=["reqs.txt"])
 
     reqs = basic_project / "reqs.txt"
@@ -1870,7 +2013,6 @@ def test_build_python_packages_from_parts(basic_project, monkeypatch):
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None, force=True)
 
     monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
@@ -1934,7 +2076,6 @@ def test_build_binary_python_packages_from_parts(basic_project, monkeypatch):
         )
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None, force=True)
 
     monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
@@ -2169,7 +2310,6 @@ def test_parts_not_defined(basic_project, charmcraft_yaml, monkeypatch):
     charmcraft_yaml(basic_project, "")
 
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None)
 
     # create a requirements.txt file
@@ -2232,7 +2372,6 @@ def test_parts_with_charm_part(basic_project, charmcraft_yaml, monkeypatch):
     )
 
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None)
 
     # create a requirements.txt file
@@ -2295,7 +2434,6 @@ def test_parts_without_charm_part(basic_project, charmcraft_yaml, monkeypatch):
         ),
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None)
 
     monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")
@@ -2339,7 +2477,6 @@ def test_parts_with_charm_part_with_plugin(basic_project, charmcraft_yaml, monke
         ),
     )
     config = load(basic_project)
-    monkeypatch.chdir(basic_project)
     builder = get_builder(config, entrypoint=None)
 
     monkeypatch.setenv("CHARMCRAFT_MANAGED_MODE", "1")