Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mkosi/sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ def acquire_privileges(
have_effective_cap(CAP_SYS_ADMIN)
and identity
and (not foreign or have_effective_cap(CAP_CHOWN))
and not delegate
and (not delegate or (os.getuid() == 0 and os.getgid() == 0))
and (not become_root or (os.getuid() == 0 and os.getgid() == 0))
):
return False
Expand Down
22 changes: 22 additions & 0 deletions tests/test_sandbox.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regression test is a nice thought, but I don't see it surviving the next refactor. The internal API of sandbox.py changes as requirements for it change. The relevant thing is the behaviour as seen from the outside.

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# SPDX-License-Identifier: LGPL-2.1-or-later

from unittest import mock

from mkosi.sandbox import acquire_privileges


def test_acquire_privileges_root_with_delegate_skips_userns() -> None:
"""Running as root with delegate>0 should not enter a user namespace.

Regression test for https://github.com/systemd/mkosi/issues/4233:
when mkosi runs as root, acquire_privileges(foreign=True, delegate=3)
must return False (skip namespace entry) so that subsequent calls to
ensure_directories_exist() retain host-root filesystem access.
"""
with (
mock.patch("mkosi.sandbox.have_effective_cap", return_value=True),
mock.patch("os.getuid", return_value=0),
mock.patch("os.getgid", return_value=0),
):
result = acquire_privileges(foreign=True, delegate=3)
assert result is False
Comment on lines +21 to +22

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
result = acquire_privileges(foreign=True, delegate=3)
assert result is False
assert not acquire_privileges(foreign=True, delegate=3)

Loading