From 115fac3c29c80d8917158df2275d45fee118d61f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 24 Oct 2024 11:36:48 +0200 Subject: [PATCH 1/5] run0: optionally show superhero emoji on each shell prompt This makes use of the infra introduced in 229d4a980607e9478cf1935793652ddd9a14618b to indicate visually on each prompt that we are in superuser mode temporarily. pick ad5de3222f userdbctl: add some basic client-side filtering --- man/run0.xml | 37 +++++++++++++++++++++ src/basic/glyph-util.c | 2 ++ src/basic/glyph-util.h | 1 + src/run/run.c | 64 +++++++++++++++++++++++++++---------- src/test/test-locale-util.c | 3 +- 5 files changed, 89 insertions(+), 18 deletions(-) diff --git a/man/run0.xml b/man/run0.xml index 422ca6bebbf..907222c0495 100644 --- a/man/run0.xml +++ b/man/run0.xml @@ -207,6 +207,20 @@ + + + + Set a shell prompt prefix string. This ultimately controls the + $SHELL_PROMPT_PREFIX environment variable for the invoked program, which is + typically imported into the shell prompt. By default – if emojis are supported – a superhero emoji is + shown (🦸). This default may also be changed (or turned off) by passing the + $SYSTEMD_RUN_SHELL_PROMPT_PREFIX environment variable to run0, + see below. Set to an empty string to disable shell prompt prefixing. + + + + + @@ -271,7 +285,30 @@ + + + $SHELL_PROMPT_PREFIX + By default set to the superhero emoji (if supported), but may be overriden with the + $SYSTEMD_RUN_SHELL_PROMPT_PREFIX environment variable (see below), or the + switch (see above). + + + + + The following variables may be passed to run0: + + + + $SYSTEMD_RUN_SHELL_PROMPT_PREFIX + If set, overrides the default shell prompt prefix that run0 sets + for the invoked shell (the superhero emoji). Set to an empty string to disable shell prompt + prefixing. + + + + + diff --git a/src/basic/glyph-util.c b/src/basic/glyph-util.c index 68fd3971adf..1108afdf03e 100644 --- a/src/basic/glyph-util.c +++ b/src/basic/glyph-util.c @@ -80,6 +80,7 @@ const char* special_glyph_full(SpecialGlyph code, bool force_utf) { [SPECIAL_GLYPH_YELLOW_CIRCLE] = "o", [SPECIAL_GLYPH_BLUE_CIRCLE] = "o", [SPECIAL_GLYPH_GREEN_CIRCLE] = "o", + [SPECIAL_GLYPH_SUPERHERO] = "S", }, /* UTF-8 */ @@ -149,6 +150,7 @@ const char* special_glyph_full(SpecialGlyph code, bool force_utf) { [SPECIAL_GLYPH_YELLOW_CIRCLE] = u8"🟡", [SPECIAL_GLYPH_BLUE_CIRCLE] = u8"🔵", [SPECIAL_GLYPH_GREEN_CIRCLE] = u8"🟢", + [SPECIAL_GLYPH_SUPERHERO] = u8"🦸", }, }; diff --git a/src/basic/glyph-util.h b/src/basic/glyph-util.h index 9b5c3a8d226..c31c3c18bba 100644 --- a/src/basic/glyph-util.h +++ b/src/basic/glyph-util.h @@ -55,6 +55,7 @@ typedef enum SpecialGlyph { SPECIAL_GLYPH_YELLOW_CIRCLE, SPECIAL_GLYPH_BLUE_CIRCLE, SPECIAL_GLYPH_GREEN_CIRCLE, + SPECIAL_GLYPH_SUPERHERO, _SPECIAL_GLYPH_MAX, _SPECIAL_GLYPH_INVALID = -EINVAL, } SpecialGlyph; diff --git a/src/run/run.c b/src/run/run.c index 529e9d1bcce..88fe8a0c8f7 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -85,6 +85,7 @@ static char *arg_exec_path = NULL; static bool arg_ignore_failure = false; static char *arg_background = NULL; static sd_json_format_flags_t arg_json_format_flags = SD_JSON_FORMAT_OFF; +static char *arg_shell_prompt_prefix = NULL; STATIC_DESTRUCTOR_REGISTER(arg_description, freep); STATIC_DESTRUCTOR_REGISTER(arg_environment, strv_freep); @@ -96,6 +97,7 @@ STATIC_DESTRUCTOR_REGISTER(arg_working_directory, freep); STATIC_DESTRUCTOR_REGISTER(arg_cmdline, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_exec_path, freep); STATIC_DESTRUCTOR_REGISTER(arg_background, freep); +STATIC_DESTRUCTOR_REGISTER(arg_shell_prompt_prefix, freep); static int help(void) { _cleanup_free_ char *link = NULL; @@ -194,6 +196,7 @@ static int help_sudo_mode(void) { " --background=COLOR Set ANSI color for background\n" " --pty Request allocation of a pseudo TTY for stdio\n" " --pipe Request direct pipe for stdio\n" + " --shell-prompt-prefix=PREFIX Set $SHELL_PROMPT_PREFIX\n" "\nSee the %s for details.\n", program_invocation_short_name, ansi_highlight(), @@ -778,29 +781,31 @@ static int parse_argv_sudo_mode(int argc, char *argv[]) { ARG_BACKGROUND, ARG_PTY, ARG_PIPE, + ARG_SHELL_PROMPT_PREFIX, }; /* If invoked as "run0" binary, let's expose a more sudo-like interface. We add various extensions * though (but limit the extension to long options). */ static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, 'V' }, - { "no-ask-password", no_argument, NULL, ARG_NO_ASK_PASSWORD }, - { "machine", required_argument, NULL, ARG_MACHINE }, - { "unit", required_argument, NULL, ARG_UNIT }, - { "property", required_argument, NULL, ARG_PROPERTY }, - { "description", required_argument, NULL, ARG_DESCRIPTION }, - { "slice", required_argument, NULL, ARG_SLICE }, - { "slice-inherit", no_argument, NULL, ARG_SLICE_INHERIT }, - { "user", required_argument, NULL, 'u' }, - { "group", required_argument, NULL, 'g' }, - { "nice", required_argument, NULL, ARG_NICE }, - { "chdir", required_argument, NULL, 'D' }, - { "setenv", required_argument, NULL, ARG_SETENV }, - { "background", required_argument, NULL, ARG_BACKGROUND }, - { "pty", no_argument, NULL, ARG_PTY }, - { "pipe", no_argument, NULL, ARG_PIPE }, + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, 'V' }, + { "no-ask-password", no_argument, NULL, ARG_NO_ASK_PASSWORD }, + { "machine", required_argument, NULL, ARG_MACHINE }, + { "unit", required_argument, NULL, ARG_UNIT }, + { "property", required_argument, NULL, ARG_PROPERTY }, + { "description", required_argument, NULL, ARG_DESCRIPTION }, + { "slice", required_argument, NULL, ARG_SLICE }, + { "slice-inherit", no_argument, NULL, ARG_SLICE_INHERIT }, + { "user", required_argument, NULL, 'u' }, + { "group", required_argument, NULL, 'g' }, + { "nice", required_argument, NULL, ARG_NICE }, + { "chdir", required_argument, NULL, 'D' }, + { "setenv", required_argument, NULL, ARG_SETENV }, + { "background", required_argument, NULL, ARG_BACKGROUND }, + { "pty", no_argument, NULL, ARG_PTY }, + { "pipe", no_argument, NULL, ARG_PIPE }, + { "shell-prompt-prefix", required_argument, NULL, ARG_SHELL_PROMPT_PREFIX }, {}, }; @@ -907,6 +912,12 @@ static int parse_argv_sudo_mode(int argc, char *argv[]) { arg_stdio = ARG_STDIO_DIRECT; break; + case ARG_SHELL_PROMPT_PREFIX: + r = free_and_strdup_warn(&arg_shell_prompt_prefix, optarg); + if (r < 0) + return r; + break; + case '?': return -EINVAL; @@ -1019,6 +1030,25 @@ static int parse_argv_sudo_mode(int argc, char *argv[]) { log_debug_errno(r, "Unable to get terminal background color, not tinting background: %m"); } + if (!arg_shell_prompt_prefix) { + const char *e = secure_getenv("SYSTEMD_RUN_SHELL_PROMPT_PREFIX"); + if (e) { + arg_shell_prompt_prefix = strdup(e); + if (!arg_shell_prompt_prefix) + return log_oom(); + } else if (emoji_enabled()) { + arg_shell_prompt_prefix = strjoin(special_glyph(SPECIAL_GLYPH_SUPERHERO), " "); + if (!arg_shell_prompt_prefix) + return log_oom(); + } + } + + if (!isempty(arg_shell_prompt_prefix)) { + r = strv_env_assign(&arg_environment, "SHELL_PROMPT_PREFIX", arg_shell_prompt_prefix); + if (r < 0) + return log_error_errno(r, "Failed to set $SHELL_PROMPT_PREFIX environment variable: %m"); + } + return 1; } diff --git a/src/test/test-locale-util.c b/src/test/test-locale-util.c index ab2d1f5746c..7afa446bfb6 100644 --- a/src/test/test-locale-util.c +++ b/src/test/test-locale-util.c @@ -82,7 +82,7 @@ TEST(keymaps) { #define dump_glyph(x) log_info(STRINGIFY(x) ": %s", special_glyph(x)) TEST(dump_special_glyphs) { - assert_cc(SPECIAL_GLYPH_GREEN_CIRCLE + 1 == _SPECIAL_GLYPH_MAX); + assert_cc(SPECIAL_GLYPH_SUPERHERO + 1 == _SPECIAL_GLYPH_MAX); log_info("is_locale_utf8: %s", yes_no(is_locale_utf8())); @@ -133,6 +133,7 @@ TEST(dump_special_glyphs) { dump_glyph(SPECIAL_GLYPH_YELLOW_CIRCLE); dump_glyph(SPECIAL_GLYPH_BLUE_CIRCLE); dump_glyph(SPECIAL_GLYPH_GREEN_CIRCLE); + dump_glyph(SPECIAL_GLYPH_SUPERHERO); } DEFINE_TEST_MAIN(LOG_INFO); From 0310b2a60b61df94e459b2be6e2ef6bc3996778b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 24 Oct 2024 12:04:43 +0200 Subject: [PATCH 2/5] run: tweak how we name our transient units The current logic is a bit complex how systemd-run units are called. It used to be just the unique ID of the dbus connection. Which was nice, since its system-widely, uniquely assigned to us. But this didn't work out well, due to direct connections to PID 1 and due to soft reboots. We nowadays have a better ID to use though, with nicer properties: the kernel manages a pidfd ID for every process after all, and it's globally unique, for any process, and regardless of soft reboots. Hence use that for naming preferably, and just keep one branch with a randomized name as fallback. --- src/run/run.c | 76 +++++++++++++++------------------------------------ 1 file changed, 22 insertions(+), 54 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 88fe8a0c8f7..42eb756fa9c 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -1401,70 +1401,38 @@ static int transient_timer_set_properties(sd_bus_message *m) { return 0; } -static int make_unit_name(sd_bus *bus, UnitType t, char **ret) { - unsigned soft_reboots_count = 0; - const char *unique, *id; - char *p; +static int make_unit_name(UnitType t, char **ret) { int r; - assert(bus); assert(t >= 0); assert(t < _UNIT_TYPE_MAX); assert(ret); - r = sd_bus_get_unique_name(bus, &unique); + /* Preferably use our PID + pidfd ID as identifier, if available. It's a boot time unique identifier + * managed by the kernel. Unfortunately only new kernels support this, hence we keep some fallback + * logic in place. */ + + _cleanup_(pidref_done) PidRef self = PIDREF_NULL; + r = pidref_set_self(&self); + if (r < 0) + return log_error_errno(r, "Failed to get reference to my own process: %m"); + + r = pidref_acquire_pidfd_id(&self); if (r < 0) { + log_debug_errno(r, "Failed to acquire pidfd ID of myself, defaulting to randomized unit name: %m"); + + /* We couldn't get the pidfd id. In that case, just pick a random uuid as name */ sd_id128_t rnd; - - /* We couldn't get the unique name, which is a pretty - * common case if we are connected to systemd - * directly. In that case, just pick a random uuid as - * name */ - r = sd_id128_randomize(&rnd); if (r < 0) return log_error_errno(r, "Failed to generate random run unit name: %m"); - if (asprintf(ret, "run-r" SD_ID128_FORMAT_STR ".%s", SD_ID128_FORMAT_VAL(rnd), unit_type_to_string(t)) < 0) - return log_oom(); + r = asprintf(ret, "run-r" SD_ID128_FORMAT_STR ".%s", SD_ID128_FORMAT_VAL(rnd), unit_type_to_string(t)); + } else + r = asprintf(ret, "run-p" PID_FMT "-i%" PRIu64 ".%s", self.pid, self.fd_id, unit_type_to_string(t)); + if (r < 0) + return log_oom(); - return 0; - } - - /* We managed to get the unique name, then let's use that to name our transient units. */ - - id = startswith(unique, ":1."); /* let' strip the usual prefix */ - if (!id) - id = startswith(unique, ":"); /* the spec only requires things to start with a colon, hence - * let's add a generic fallback for that. */ - if (!id) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Unique name %s has unexpected format.", - unique); - - /* The unique D-Bus names are actually unique per D-Bus instance, so on soft-reboot they will wrap - * and start over since the D-Bus broker is restarted. If there's a failed unit left behind that - * hasn't been garbage collected, we'll conflict. Append the soft-reboot counter to avoid clashing. */ - if (arg_runtime_scope == RUNTIME_SCOPE_SYSTEM) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - r = bus_get_property_trivial( - bus, bus_systemd_mgr, "SoftRebootsCount", &error, 'u', &soft_reboots_count); - if (r < 0) - log_debug_errno(r, - "Failed to get SoftRebootsCount property, ignoring: %s", - bus_error_message(&error, r)); - } - - if (soft_reboots_count > 0) { - if (asprintf(&p, "run-u%s-s%u.%s", id, soft_reboots_count, unit_type_to_string(t)) < 0) - return log_oom(); - } else { - p = strjoin("run-u", id, ".", unit_type_to_string(t)); - if (!p) - return log_oom(); - } - - *ret = p; return 0; } @@ -1874,7 +1842,7 @@ static int start_transient_service(sd_bus *bus) { if (r < 0) return log_error_errno(r, "Failed to mangle unit name: %m"); } else { - r = make_unit_name(bus, UNIT_SERVICE, &service); + r = make_unit_name(UNIT_SERVICE, &service); if (r < 0) return r; } @@ -2089,7 +2057,7 @@ static int start_transient_scope(sd_bus *bus) { if (r < 0) return log_error_errno(r, "Failed to mangle scope name: %m"); } else { - r = make_unit_name(bus, UNIT_SCOPE, &scope); + r = make_unit_name(UNIT_SCOPE, &scope); if (r < 0) return r; } @@ -2388,7 +2356,7 @@ static int start_transient_trigger(sd_bus *bus, const char *suffix) { break; } } else { - r = make_unit_name(bus, UNIT_SERVICE, &service); + r = make_unit_name(UNIT_SERVICE, &service); if (r < 0) return r; From d9f68f48f75989b0992705d2fac8ff615671e6b9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 24 Oct 2024 12:07:24 +0200 Subject: [PATCH 3/5] run: prefix unit description with our own process name I think we should try to communicate clearly if something is a run0 session, or a systemd-run invocation. Hence, let's initialize the description so that the command is prefixed by program_invocation_short_name. Effectively this means that our run0 sessions now appear as services with a description of "[run0] -/bin/bash" --- src/run/run.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 42eb756fa9c..d4eb148cf64 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -2435,7 +2435,7 @@ static int run(int argc, char* argv[]) { } if (!arg_description) { - char *t; + _cleanup_free_ char *t = NULL; if (strv_isempty(arg_cmdline)) t = strdup(arg_unit); @@ -2444,7 +2444,9 @@ static int run(int argc, char* argv[]) { if (!t) return log_oom(); - free_and_replace(arg_description, t); + arg_description = strjoin("[", program_invocation_short_name, "] ", t); + if (!arg_description) + return log_oom(); } /* For backward compatibility reasons env var expansion is disabled by default for scopes, and From ff4b6a191563e37b9c5f725bc5ce0d7609c0891e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 24 Oct 2024 12:14:01 +0200 Subject: [PATCH 4/5] run: drop "-" prefix from command line when generating unit description Let's not confuse users with the login shell indicator and drop it from the description. This means a run0 session will now usually show up with a description of "[run0] /bin/bash" rather than "[run0] -/bin/bash". --- src/run/run.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/run/run.c b/src/run/run.c index d4eb148cf64..6b9e19e2d1a 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -2439,7 +2439,19 @@ static int run(int argc, char* argv[]) { if (strv_isempty(arg_cmdline)) t = strdup(arg_unit); - else + else if (startswith(arg_cmdline[0], "-")) { + /* Drop the login shell marker from the command line when generating the description, + * in order to minimize user confusion. */ + _cleanup_strv_free_ char **l = strv_copy(arg_cmdline); + if (!l) + return log_oom(); + + r = free_and_strdup_warn(l + 0, l[0] + 1); + if (r < 0) + return r; + + t = quote_command_line(l, SHELL_ESCAPE_EMPTY); + } else t = quote_command_line(arg_cmdline, SHELL_ESCAPE_EMPTY); if (!t) return log_oom(); From d585085f5791c23f7e8a408703cfaa65b9407719 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 25 Oct 2024 10:18:48 +0200 Subject: [PATCH 5/5] update TODO --- TODO | 3 --- 1 file changed, 3 deletions(-) diff --git a/TODO b/TODO index c0052c56727..70e088210a1 100644 --- a/TODO +++ b/TODO @@ -162,9 +162,6 @@ Features: sd_event_add_child(), and then get rid of many more explicit sigprocmask() calls. -* maybe set shell.prompt.prefix credential in run0 to some warning emoji, - i.e. ⚠️ or ☢️ or ⚡ or 👊 or 🧑‍🔧 or so. - * introduce new structure Tpm2CombinedPolicy, that combines the various TPm2 policy bits into one structure, i.e. public key info, pcr masks, pcrlock stuff, pin and so on. Then pass that around in tpm2_seal() and tpm2_unseal().