oci: set default PATH when missing or empty

rockcraft/oci.py

@@ -405,23 +405,26 @@ def set_cmd(self, command: list[str] | None = None) -> None:
         _config_image(image_path, cmd_params, comment="Set default commands")
         emit.progress(f"CMD set to {command}")
 
-    def set_default_path(self, base: str) -> None:
-        """Set the default PATH on the image (only for bare rocks)."""
-        if base != "bare":
-            emit.debug(f"Not setting a PATH on the image as base is {base!r}")
+    def set_default_path(self) -> None:
+        """Ensure the OCI image has a sane PATH when PATH is missing or empty."""
+        image_path = self.path / self.image_name
+
+        env = _read_image_env_from_skopeo(image_path)
+        path_value = _get_env_value(env, "PATH")
+
+        if path_value:
+            emit.debug("PATH already set on the image; not overriding.")
             return
 
         # Follow Pebble's lead here: if PATH is empty, use the standard one.
-        # This means that containers that bypass the pebble entrypoint will
-        # have the same behavior as PATH-less pebble services.
+        # This means that containers that bypass the Pebble entrypoint will
+        # have the same behavior as PATH-less Pebble services.
         pebble_path = Pebble.DEFAULT_ENV_PATH
-        image_path = self.path / self.image_name
-
-        emit.debug(f"Setting bare-based rock PATH to {pebble_path!r}")
+        emit.debug(f"Setting default PATH on the image to {pebble_path!r}")
         _config_image(
             image_path,
             ["--config.env", f"PATH={pebble_path}"],
-            comment="Set default PATH for bare-based rock",
+            comment="Set default PATH",
         )
 
     def set_pebble_layer(
@@ -578,6 +581,28 @@ def _copy_image(
     )
 
 
+def _read_image_env_from_skopeo(image_path: Path) -> list[str]:
+    """Read the environment from `skopeo inspect` output."""
+    output = _process_run(["skopeo", "inspect", f"oci:{image_path}"]).stdout
+    data: object = json.loads(output)
+
+    if not isinstance(data, dict):
+        return []
+
+    env = data.get("Env")
+    if not isinstance(env, list):
+        return []
+
+    return [entry for entry in env if isinstance(entry, str)]
+
+
+def _get_env_value(env: list[str], key: str) -> str | None:
+    """Return the value for KEY from env entries like ['K=V', ...]. Last wins."""
+    prefix = f"{key}="
+    values = [e.split("=", 1)[1] for e in env if e.startswith(prefix)]
+    return values[-1] if values else None
+
+
 def _config_image(
     image_path: Path, params: list[str], comment: str | None = None
 ) -> None:

rockcraft/services/package.py

@@ -152,7 +152,6 @@ def _pack(
 
     new_image.set_entrypoint(entrypoint)
     new_image.set_cmd(cmd)
-    new_image.set_default_path(project.base)
 
     dumped = project.marshal()
     services = cast(dict[str, typing.Any], dumped.get("services", {}))
@@ -172,6 +171,9 @@ def _pack(
     if project.environment:
         new_image.set_environment(project.environment)
 
+    # Check the final environment after applying project-defined variables.
+    new_image.set_default_path()
+
     # Set annotations and metadata, both dynamic and the ones based on user-provided properties
     # Also include the "created" timestamp, just before packing the image
     emit.progress("Adding metadata")

tests/unit/services/test_package.py

@@ -15,6 +15,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 from pathlib import Path
 from typing import cast
+from unittest.mock import call
 
 import pytest
 from craft_application import ServiceFactory
@@ -69,11 +70,11 @@ def test_pack(fake_services: ServiceFactory, default_image_info, mocker):
 @pytest.mark.parametrize(
     ("project_keys", "expected_entrypoint", "expected_cmd"),
     [
-        # Most common scenario
+        # Project environment explicitly clears PATH.
         (
             {
                 "run_user": "_daemon_",
-                "environment": {"test": "foo"},
+                "environment": {"PATH": ""},
                 "services": {"test": {"override": "replace", "command": "echo foo"}},
             },
             ["/usr/bin/pebble", "enter"],
@@ -208,7 +209,7 @@ def test_inner_pack(
     image.set_default_user.assert_called_once_with(584792, project.run_user)
     image.set_entrypoint.assert_called_once_with(expected_entrypoint)
     image.set_cmd.assert_called_once_with(expected_cmd)
-    image.set_default_path.assert_called_once_with(project.base)
+    image.set_default_path.assert_called_once_with()
     image.set_pebble_layer.assert_called_once_with(
         services=project.marshal().get("services", {}),
         checks=project.marshal().get("checks", {}),
@@ -220,6 +221,12 @@ def test_inner_pack(
     )
     image.set_environment.assert_called_once_with(project.environment)
     image.set_annotations.assert_called_once_with(annotations)
+
+    environment_call = call.set_environment(project.environment)
+    default_path_call = call.set_default_path()
+    assert image.method_calls.index(environment_call) < image.method_calls.index(
+        default_path_call
+    )
     image.set_control_data.assert_called_once_with(metadata)
     image.set_media_type.assert_called_once_with(arch="amd64")
     image.to_oci_archive.assert_called_once_with(

tests/unit/test_oci.py

@@ -1012,40 +1012,80 @@ def test_get_manifest(self, new_dir, mock_inject_oci_fields, mock_run, mocker):
         ]
         assert mock_loads.called
 
-    def test_set_path_bare(self, mock_run):
-        image = oci.Image("a:b", Path("/c"))
+    def _mock_skopeo_inspect_env(
+        self,
+        mock_run,
+        mocker,
+        *,
+        env: list[str],
+    ) -> None:
+        def _run_side_effect(cmd, **kwargs):
+            result = mocker.MagicMock()
 
-        image.set_default_path("bare")
+            if cmd[:2] == ["skopeo", "inspect"]:
+                result.stdout = json.dumps({"Env": env})
+                return result
 
-        expected_path = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
-        expected_cmd = [
-            "umoci",
-            "config",
-            "--image",
-            "/c/a:b",
-            "--config.env",
-            f"PATH={expected_path}",
-        ]
+            if cmd[:2] == ["umoci", "config"] and "--image" in cmd:
+                return result
 
-        assert mock_run.mock_calls == [
-            call(
-                [
-                    *expected_cmd,
-                    "--history.created_by",
-                    " ".join(expected_cmd),
-                    "--history.comment",
-                    "Set default PATH for bare-based rock",
-                ]
-            )
-        ]
+            raise AssertionError(f"Unexpected command: {cmd!r}")
+
+        mock_run.side_effect = _run_side_effect
 
     @pytest.mark.parametrize(
-        "base",
-        ["ubuntu@24.04", "ubuntu@22.04", "ubuntu@20.04"],
+        ("env", "should_set_default"),
+        [
+            pytest.param(["PATH=/usr/bin"], False, id="path-present"),
+            pytest.param(["FOO=bar"], True, id="path-missing"),
+            pytest.param(["PATH="], True, id="path-empty"),
+            pytest.param(
+                ["PATH=/usr/bin", "PATH="],
+                True,
+                id="last-path-empty",
+            ),
+            pytest.param(
+                ["PATH=", "PATH=/custom/bin"],
+                False,
+                id="last-path-present",
+            ),
+        ],
     )
-    def test_set_path_non_bare(self, mock_run, base):
+    def test_set_default_path(
+        self,
+        mock_run,
+        mocker,
+        env,
+        should_set_default,
+    ):
         image = oci.Image("a:b", Path("/c"))
+        self._mock_skopeo_inspect_env(mock_run, mocker, env=env)
+
+        image.set_default_path()
 
-        image.set_default_path(base)
+        expected_calls = [
+            call(["skopeo", "inspect", "oci:/c/a:b"]),
+        ]
+
+        if should_set_default:
+            config_cmd = [
+                "umoci",
+                "config",
+                "--image",
+                "/c/a:b",
+                "--config.env",
+                f"PATH={Pebble.DEFAULT_ENV_PATH}",
+            ]
+            expected_calls.append(
+                call(
+                    [
+                        *config_cmd,
+                        "--history.created_by",
+                        " ".join(config_cmd),
+                        "--history.comment",
+                        "Set default PATH",
+                    ]
+                )
+            )
 
-        assert not mock_run.called
+        assert mock_run.mock_calls == expected_calls