Skip to content

Commit a2b71f7

Browse files
committed
fix: set PATH on bare-based rocks
If the PATH is empty, Pebble will be default use the standard Ubuntu value of "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" (see [0]). Follow this lead and set this value on the image's PATH. This PATH setting on the image itself has no bearing on most cases, as the PATH that prevails is the one defined by Pebble and its services, but an empty (or unset) PATH is a potential security issue in cases where the pebble entrypoint is bypassed. 0: https://github.com/canonical/pebble/blob/master/internals/overlord/cmdstate/request.go#L91 Fixes #711
1 parent 1d87e33 commit a2b71f7

File tree

5 files changed

+80
-2
lines changed

5 files changed

+80
-2
lines changed

rockcraft/oci.py

+16
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,22 @@ def set_cmd(self, command: str | None = None) -> None:
389389
_config_image(image_path, cmd_params)
390390
emit.progress(f"CMD set to {opt_args}")
391391

392+
def set_path(self, base: str, build_base: str | None) -> None:
393+
"""Set the default PATH on the image (only for bare rocks)."""
394+
if base != "bare":
395+
emit.debug(f"Not setting a PATH on the image as base is {base!r}")
396+
return
397+
398+
# Follow Pebble's lead here: if PATH is empty, use the standard one.
399+
# This means that containers that bypass the pebble entrypoint will
400+
# have the same behavior as PATH-less pebble services.
401+
# (https://github.com/canonical/pebble/blob/master/internals/overlord/cmdstate/request.go#L91)
402+
pebble_path = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
403+
image_path = self.path / self.image_name
404+
405+
emit.debug(f"Setting bare-based rock PATH to {pebble_path!r}")
406+
_config_image(image_path, ["--config.env", f"PATH={pebble_path}"])
407+
392408
def set_pebble_layer(
393409
self,
394410
services: dict[str, Any],

rockcraft/services/package.py

+2
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,8 @@ def _pack(
157157
if project.services and project.entrypoint_service in project.services:
158158
new_image.set_cmd(project.services[project.entrypoint_service].command)
159159

160+
new_image.set_path(project.base, project.build_base)
161+
160162
dumped = project.marshal()
161163
services = cast(dict[str, typing.Any], dumped.get("services", {}))
162164
checks = cast(dict[str, typing.Any], dumped.get("checks", {}))

tests/spread/rockcraft/bare-base/rockcraft.yaml tests/spread/rockcraft/bare-base/rockcraft.orig.yaml

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ version: latest
33
summary: A tiny rock
44
description: Building a tiny rock from a bare base, with just one package
55
license: Apache-2.0
6-
build-base: [email protected]
6+
build-base: placeholder-base
77
base: bare
88
services:
99
hello:
@@ -18,3 +18,4 @@ parts:
1818
plugin: nil
1919
stage-packages:
2020
- hello
21+
- base-files # for the usrmerge symlinks in [email protected] and newer

tests/spread/rockcraft/bare-base/task.yaml

+14-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
summary: bare base build test
2+
environment:
3+
BASE/24_04: "[email protected]"
4+
PEBBLE_DIR/24_04: "/usr/bin"
5+
6+
BASE/22_04: "[email protected]"
7+
PEBBLE_DIR/22_04: "/bin"
28

39
execute: |
10+
sed "s/placeholder-base/$BASE/" rockcraft.orig.yaml > rockcraft.yaml
11+
412
run_rockcraft pack
513
614
test -f bare-base-test_latest_amd64.rock
@@ -15,6 +23,11 @@ execute: |
1523
grep_docker_log "$id" "ship it!"
1624
docker exec "$id" pebble services | grep hello
1725
docker exec "$id" pebble ls /usr/bin/hello
18-
test "$(docker inspect "$id" -f '{{json .Config.Entrypoint}}')" = '["/bin/pebble","enter"]'
26+
test "$(docker inspect "$id" -f '{{json .Config.Entrypoint}}')" = "[\"${PEBBLE_DIR}/pebble\",\"enter\"]"
1927
28+
# Bare-based rocks should have the default Ubuntu path, as an empty PATH is a
29+
# security issue when containers bypass the pebble entrypoint.
30+
EXPECTED_PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
31+
test "$(docker inspect "$id" -f '{{json .Config.Env}}')" = "[\"PATH=${EXPECTED_PATH}\"]"
32+
2033
docker rm -f "$id"

tests/unit/test_oci.py

+46
Original file line numberDiff line numberDiff line change
@@ -951,3 +951,49 @@ def test_stat(self, new_dir, mock_run, mocker):
951951
)
952952
]
953953
assert mock_loads.called
954+
955+
@pytest.mark.parametrize(
956+
"build_base",
957+
[
958+
959+
"ubuntu@devel",
960+
961+
962+
],
963+
)
964+
def test_set_path_bare(self, mock_run, build_base):
965+
image = oci.Image("a:b", Path("/c"))
966+
967+
image.set_path("bare", build_base)
968+
969+
expected = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
970+
assert mock_run.mock_calls == [
971+
call(
972+
[
973+
"umoci",
974+
"config",
975+
"--image",
976+
"/c/a:b",
977+
"--config.env",
978+
f"PATH={expected}",
979+
]
980+
)
981+
]
982+
983+
@pytest.mark.parametrize(
984+
["base", "build_base"],
985+
[
986+
987+
("[email protected]", None),
988+
989+
("[email protected]", None),
990+
991+
("[email protected]", None),
992+
],
993+
)
994+
def test_set_path_non_bare(self, mock_run, base, build_base):
995+
image = oci.Image("a:b", Path("/c"))
996+
997+
image.set_path(base, build_base)
998+
999+
assert not mock_run.called

0 commit comments

Comments
 (0)