This thread contains a patchset. You're looking at the original emails,
but you may wish to use the patch review UI.
Review patch
[PATCH pmbootstrap v4] Don't use 'sudo' when running as root
This cancels the need to install and configure `sudo` or `doas` on
single-user installations (e.g.: a VM dedicated to running pmbootstrap).
Fixes: https://gitlab.com/postmarketOS/pmbootstrap/-/issues/2224
---
Appologies for the noise with this patch.
This fourth version addresses all previous comments and passes all CI checks.
pmb/chroot/root.py | 6 ++++ --
pmb/config/__init__.py | 11 ++++++++++ -
pmb/config/sudo.py | 11 +++++++++ --
pmb/helpers/run.py | 2 + -
test/test_run_core.py | 10 ++++++ ----
5 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/pmb/chroot/root.py b/pmb/chroot/root.py
index a14177ad..4e5fc586 100644
--- a/pmb/chroot/root.py
+++ b/pmb/chroot/root.py
@@ -78,8 +78,10 @@ def root(args, cmd, suffix="native", working_dir="/", output="log",
executables = executables_absolute_path()
cmd_chroot = [executables["chroot"], chroot, "/bin/sh", "-c",
pmb.helpers.run.flat_cmd(cmd, working_dir)]
- cmd_sudo = [pmb.config.sudo, "env", "-i", executables["sh"], "-c",- pmb.helpers.run.flat_cmd(cmd_chroot, env=env_all)]+ cmd_sudo = pmb.config.sudo([+ "env", "-i", executables["sh"], "-c",+ pmb.helpers.run.flat_cmd(cmd_chroot, env=env_all)]+ ) return pmb.helpers.run_core.core(args, msg, cmd_sudo, None, output,
output_return, check, True,
disable_timeout)
diff --git a/pmb/config/__init__.py b/pmb/config/__init__.py
index fae75b17..59af6f76 100644
--- a/pmb/config/__init__.py
+++ b/pmb/config/__init__.py
@@ -58,7 +58,16 @@ required_programs = [
"ps",
"tar",
]
- sudo = which_sudo()+ + + def sudo(cmd: list[str]) -> list[str]:+ """Adapt a command to run as root."""+ sudo = which_sudo()+ if sudo:+ return [sudo, *cmd]+ else:+ return cmd+
# Keys saved in the config file (mostly what we ask in 'pmbootstrap init')
config_keys = [
diff --git a/pmb/config/sudo.py b/pmb/config/sudo.py
index fe876987..1e227534 100644
--- a/pmb/config/sudo.py
+++ b/pmb/config/sudo.py
@@ -2,13 +2,20 @@
# SPDX-License-Identifier: GPL-3.0-or-later
import os
import shutil
+ from functools import lru_cache
- def which_sudo():- """+ @lru_cache()+ def which_sudo() -> str | None:+ """Returns a command required to run commands as root, if any.+ Find whether sudo or doas is installed for commands that require root.
Allows user to override preferred sudo with PMB_SUDO env variable.
"""
+ + if os.getuid() == 0:+ return None+ supported_sudos = ['doas', 'sudo']
user_set_sudo = os.getenv("PMB_SUDO")
diff --git a/pmb/helpers/run.py b/pmb/helpers/run.py
index b172b356..9835434a 100644
--- a/pmb/helpers/run.py
+++ b/pmb/helpers/run.py
@@ -72,7 +72,7 @@ def root(args, cmd, working_dir=None, output="log", output_return=False,
"""
if env:
cmd = ["sh", "-c", flat_cmd(cmd, env=env)]
- cmd = [pmb.config.sudo] + cmd+ cmd = pmb.config.sudo(cmd)
return user(args, cmd, working_dir, output, output_return, check, env,
True)
diff --git a/test/test_run_core.py b/test/test_run_core.py
index a4600fb8..aa2aaf65 100644
--- a/test/test_run_core.py
+++ b/test/test_run_core.py
@@ -83,7 +83,7 @@ def test_foreground_pipe(args):
assert ret == (-9, "first\n")
# Kill with output timeout as root
- cmd = [pmb.config.sudo, "sh", "-c", "printf first; sleep 2; printf second"]+ cmd = pmb.config.sudo(["sh", "-c", "printf first; sleep 2; printf second"]) args.timeout = 0.3
ret = func(args, cmd, output_return=True, output_timeout=True,
sudo=True)
@@ -99,9 +99,11 @@ def test_foreground_pipe(args):
# Check if all child processes are killed after timeout.
# The first command uses ps to get its process group id (pgid) and echo it
# to stdout. All of the test commands will be running under that pgid.
- cmd = [pmb.config.sudo, "sh", "-c",- "pgid=$(ps -o pgid= | grep ^${1:-$$});echo $pgid | tr -d '\n';" +- "sleep 10 | sleep 20 | sleep 30"]+ cmd = pmb.config.sudo([+ "sh", "-c",+ "pgid=$(ps -o pgid= | grep ^${1:-$$});echo $pgid | tr -d '\n';"+ "sleep 10 | sleep 20 | sleep 30"+ ]) args.timeout = 0.3
ret = func(args, cmd, output_return=True, output_timeout=True,
sudo=True)
--
2.40.1
[pmbootstrap/patches/.build.yml] build success
On Mon May 29, 2023 at 10:39 PM CEST, Hugo Osvaldo Barrera wrote:
> This cancels the need to install and configure `sudo` or `doas` on
> single-user installations (e.g.: a VM dedicated to running pmbootstrap).
>
> Fixes: https://gitlab.com/postmarketOS/pmbootstrap/-/issues/2224
Looks good now, thanks a lot!
Reviewed-by: Oliver Smith <ollieparanoid@postmarketos.org >
> ---
> Appologies for the noise with this patch.
>
> This fourth version addresses all previous comments and passes all CI checks.
> pmb/chroot/root.py | 6 ++++--
> pmb/config/__init__.py | 11 ++++++++++-
> pmb/config/sudo.py | 11 +++++++++--
> pmb/helpers/run.py | 2 +-
> test/test_run_core.py | 10 ++++++----
> 5 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/pmb/chroot/root.py b/pmb/chroot/root.py
> index a14177ad..4e5fc586 100644
> --- a/pmb/chroot/root.py
> +++ b/pmb/chroot/root.py
> @@ -78,8 +78,10 @@ def root(args, cmd, suffix="native", working_dir="/", output="log",
> executables = executables_absolute_path()
> cmd_chroot = [executables["chroot"], chroot, "/bin/sh", "-c",
> pmb.helpers.run.flat_cmd(cmd, working_dir)]
> - cmd_sudo = [pmb.config.sudo, "env", "-i", executables["sh"], "-c",
> - pmb.helpers.run.flat_cmd(cmd_chroot, env=env_all)]
> + cmd_sudo = pmb.config.sudo([
> + "env", "-i", executables["sh"], "-c",
> + pmb.helpers.run.flat_cmd(cmd_chroot, env=env_all)]
> + )
> return pmb.helpers.run_core.core(args, msg, cmd_sudo, None, output,
> output_return, check, True,
> disable_timeout)
> diff --git a/pmb/config/__init__.py b/pmb/config/__init__.py
> index fae75b17..59af6f76 100644
> --- a/pmb/config/__init__.py
> +++ b/pmb/config/__init__.py
> @@ -58,7 +58,16 @@ required_programs = [
> "ps",
> "tar",
> ]
> -sudo = which_sudo()
> +
> +
> +def sudo(cmd: list[str]) -> list[str]:
> + """Adapt a command to run as root."""
> + sudo = which_sudo()
> + if sudo:
> + return [sudo, *cmd]
> + else:
> + return cmd
> +
>
> # Keys saved in the config file (mostly what we ask in 'pmbootstrap init')
> config_keys = [
> diff --git a/pmb/config/sudo.py b/pmb/config/sudo.py
> index fe876987..1e227534 100644
> --- a/pmb/config/sudo.py
> +++ b/pmb/config/sudo.py
> @@ -2,13 +2,20 @@
> # SPDX-License-Identifier: GPL-3.0-or-later
> import os
> import shutil
> +from functools import lru_cache
>
>
> -def which_sudo():
> - """
> +@lru_cache()
> +def which_sudo() -> str | None:
> + """Returns a command required to run commands as root, if any.
> +
> Find whether sudo or doas is installed for commands that require root.
> Allows user to override preferred sudo with PMB_SUDO env variable.
> """
> +
> + if os.getuid() == 0:
> + return None
> +
> supported_sudos = ['doas', 'sudo']
>
> user_set_sudo = os.getenv("PMB_SUDO")
> diff --git a/pmb/helpers/run.py b/pmb/helpers/run.py
> index b172b356..9835434a 100644
> --- a/pmb/helpers/run.py
> +++ b/pmb/helpers/run.py
> @@ -72,7 +72,7 @@ def root(args, cmd, working_dir=None, output="log", output_return=False,
> """
> if env:
> cmd = ["sh", "-c", flat_cmd(cmd, env=env)]
> - cmd = [pmb.config.sudo] + cmd
> + cmd = pmb.config.sudo(cmd)
>
> return user(args, cmd, working_dir, output, output_return, check, env,
> True)
> diff --git a/test/test_run_core.py b/test/test_run_core.py
> index a4600fb8..aa2aaf65 100644
> --- a/test/test_run_core.py
> +++ b/test/test_run_core.py
> @@ -83,7 +83,7 @@ def test_foreground_pipe(args):
> assert ret == (-9, "first\n")
>
> # Kill with output timeout as root
> - cmd = [pmb.config.sudo, "sh", "-c", "printf first; sleep 2; printf second"]
> + cmd = pmb.config.sudo(["sh", "-c", "printf first; sleep 2; printf second"])
> args.timeout = 0.3
> ret = func(args, cmd, output_return=True, output_timeout=True,
> sudo=True)
> @@ -99,9 +99,11 @@ def test_foreground_pipe(args):
> # Check if all child processes are killed after timeout.
> # The first command uses ps to get its process group id (pgid) and echo it
> # to stdout. All of the test commands will be running under that pgid.
> - cmd = [pmb.config.sudo, "sh", "-c",
> - "pgid=$(ps -o pgid= | grep ^${1:-$$});echo $pgid | tr -d '\n';" +
> - "sleep 10 | sleep 20 | sleep 30"]
> + cmd = pmb.config.sudo([
> + "sh", "-c",
> + "pgid=$(ps -o pgid= | grep ^${1:-$$});echo $pgid | tr -d '\n';"
> + "sleep 10 | sleep 20 | sleep 30"
> + ])
> args.timeout = 0.3
> ret = func(args, cmd, output_return=True, output_timeout=True,
> sudo=True)
> --
> 2.40.1
On Mon, 29 May 2023 22:39:23 +0200, Hugo Osvaldo Barrera wrote:
> This cancels the need to install and configure `sudo` or `doas` on
> single-user installations (e.g.: a VM dedicated to running pmbootstrap).
>
>
Applied, thanks!
[1/1] Don't use 'sudo' when running as root
commit: d31313f7dc8018aeb92eeb8caf2dccf71a443a43
Best regards,
--
Oliver Smith <ollieparanoid@postmarketos.org >