diff --git a/NEWS b/NEWS index d1e4b79e0f..1fa079bfdb 100644 --- a/NEWS +++ b/NEWS @@ -49,6 +49,10 @@ CHANGES WITH 254 in spe: needed in the system service manager. For more details, see: https://lists.freedesktop.org/archives/systemd-devel/2022-December/048682.html + * systemd-run's switch --expand-environment= which currently is disabled + by default when combined with --scope, will be changed in a future + release to be enabled by default. + Security Relevant Changes: * pam_systemd will now by default pass the CAP_WAKE_ALARM ambient @@ -651,7 +655,12 @@ CHANGES WITH 254 in spe: * systemd-run gained a new switch --expand-environment=no to disable server-side environment variable expansion in specified command - lines. + lines. Expansion defaults to enabled for all execution types except + --scope, where it defaults to off (and prints a warning) for backward + compatbility reasons. --scope will be flipped to default enabled too + in a future release, so if you are using --scope and passing a '$' + character in the payload you should start explicitly using + --expand-environment=yes/no according to the use case. * The systemd-system-update-generator has been updated to also look for the special flag file /etc/system-update in addition to the existing diff --git a/man/systemd-run.xml b/man/systemd-run.xml index 73adbfb927..fecedda25c 100644 --- a/man/systemd-run.xml +++ b/man/systemd-run.xml @@ -177,15 +177,19 @@ - Expand environment variables in command arguments. If enabled (the default), - environment variables specified as ${VARIABLE} will be - expanded in the same way as in commands specified via ExecStart= in units. With + Expand environment variables in command arguments. If enabled, environment variables + specified as ${VARIABLE} will be expanded in the same + way as in commands specified via ExecStart= in units. With --scope, this expansion is performed by systemd-run itself, and in other cases by the service manager that spawns the command. Note that this is similar to, but not the same as variable expansion in bash1 and other shells. + The default is to enable this option in all cases, except for --scope where + it is disabled by default, for backward compatibility reasons. Note that this will be changed in a + future release, where it will be switched to enabled by default as well. + See systemd.service5 for a description of variable expansion. Disabling variable expansion is useful if the specified diff --git a/src/run/run.c b/src/run/run.c index 7880a0ecc7..f6a2ea53de 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -45,7 +45,7 @@ static const char *arg_unit = NULL; static const char *arg_description = NULL; static const char *arg_slice = NULL; static bool arg_slice_inherit = false; -static bool arg_expand_environment = true; +static int arg_expand_environment = -1; static bool arg_send_sighup = false; static BusTransport arg_transport = BUS_TRANSPORT_LOCAL; static const char *arg_host = NULL; @@ -291,11 +291,17 @@ static int parse_argv(int argc, char *argv[]) { arg_slice_inherit = true; break; - case ARG_EXPAND_ENVIRONMENT: - r = parse_boolean_argument("--expand-environment=", optarg, &arg_expand_environment); + case ARG_EXPAND_ENVIRONMENT: { + bool b; + + r = parse_boolean_argument("--expand-environment=", optarg, &b); if (r < 0) return r; + + arg_expand_environment = b; + break; + } case ARG_SEND_SIGHUP: arg_send_sighup = true; @@ -732,7 +738,7 @@ static int transient_service_set_properties(sd_bus_message *m, const char *pty_p /* We disable environment expansion on the server side via ExecStartEx=:. * ExecStartEx was added relatively recently (v243), and some bugs were fixed only later. * So use that feature only if required. It will fail with older systemds. */ - bool use_ex_prop = !arg_expand_environment; + bool use_ex_prop = arg_expand_environment == 0; assert(m); @@ -896,7 +902,7 @@ static int transient_service_set_properties(sd_bus_message *m, const char *pty_p if (use_ex_prop) r = sd_bus_message_append_strv( m, - STRV_MAKE(arg_expand_environment ? NULL : "no-env-expand")); + STRV_MAKE(arg_expand_environment > 0 ? NULL : "no-env-expand")); else r = sd_bus_message_append(m, "b", false); if (r < 0) @@ -1197,7 +1203,7 @@ static int bus_call_with_hint( if (r < 0) { log_error_errno(r, "Failed to start transient %s unit: %s", name, bus_error_message(&error, r)); - if (!arg_expand_environment && + if (arg_expand_environment == 0 && sd_bus_error_has_names(&error, SD_BUS_ERROR_UNKNOWN_PROPERTY, SD_BUS_ERROR_PROPERTY_READ_ONLY)) @@ -1614,7 +1620,7 @@ static int start_transient_scope(sd_bus *bus) { if (!arg_quiet) log_info("Running scope as unit: %s", scope); - if (arg_expand_environment) { + if (arg_expand_environment > 0) { _cleanup_strv_free_ char **expanded_cmdline = NULL, **unset_variables = NULL, **bad_variables = NULL; r = replace_env_argv(arg_cmdline, env, &expanded_cmdline, &unset_variables, &bad_variables); @@ -1868,6 +1874,18 @@ static int run(int argc, char* argv[]) { arg_description = description; } + /* For backward compatibility reasons env var expansion is disabled by default for scopes, and + * enabled by default for everything else. Try to detect it and print a warning, so that we can + * change it in the future and harmonize it. */ + if (arg_expand_environment < 0) { + arg_expand_environment = !arg_scope; + + if (!arg_quiet && arg_scope && strchr(arg_description, '$')) + log_warning("Scope command line contains environment variable, which is not expanded" + " by default for now, but will be expanded by default in the future." + " Use --expand-environment=yes/no to explicitly control it as needed."); + } + /* If --wait is used connect via the bus, unconditionally, as ref/unref is not supported via the limited direct * connection */ if (arg_wait || arg_stdio != ARG_STDIO_NONE || (arg_runtime_scope == RUNTIME_SCOPE_USER && arg_transport != BUS_TRANSPORT_LOCAL))