nixos: add functions and documentation for escaping systemd Exec* directives

it's really easy to accidentally write the wrong systemd Exec* directive, ones
that works most of the time but fails when users include systemd metacharacters
in arguments that are interpolated into an Exec* directive. add a few functions
analogous to escapeShellArg{,s} and some documentation on how and when to use them.

nixos/doc/manual/development/writing-modules.chapter.md

@@ -90,6 +90,17 @@ modules: `systemd.services` (the set of all systemd services) and
 `systemd.timers` (the list of commands to be executed periodically by
 `systemd`).
 
+Care must be taken when writing systemd services using `Exec*` directives. By
+default systemd performs substitution on `%<char>` specifiers in these
+directives, expands environment variables from `$FOO` and `${FOO}`, splits
+arguments on whitespace, and splits commands on `;`. All of these must be escaped
+to avoid unexpected substitution or splitting when interpolating into an `Exec*`
+directive, e.g. when using an `extraArgs` option to pass additional arguments to
+the service. The functions `utils.escapeSystemdExecArg` and
+`utils.escapeSystemdExecArgs` are provided for this, see [Example: Escaping in
+Exec directives](#exec-escaping-example) for an example. When using these
+functions system environment substitution should *not* be disabled explicitly.
+
 ::: {#locate-example .example}
 ::: {.title}
 **Example: NixOS Module for the "locate" Service**
@@ -153,6 +164,37 @@ in {
 ```
 :::
 
+::: {#exec-escaping-example .example}
+::: {.title}
+**Example: Escaping in Exec directives**
+:::
+```nix
+{ config, lib, pkgs, utils, ... }:
+
+with lib;
+
+let
+  cfg = config.services.echo;
+  echoAll = pkgs.writeScript "echo-all" ''
+    #! ${pkgs.runtimeShell}
+    for s in "$@"; do
+      printf '%s\n' "$s"
+    done
+  '';
+  args = [ "a%Nything" "lang=\${LANG}" ";" "/bin/sh -c date" ];
+in {
+  systemd.services.echo =
+    { description = "Echo to the journal";
+      wantedBy = [ "multi-user.target" ];
+      serviceConfig.Type = "oneshot";
+      serviceConfig.ExecStart = ''
+        ${echoAll} ${utils.escapeSystemdExecArgs args}
+      '';
+    };
+}
+```
+:::
+
 ```{=docbook}
 <xi:include href="option-declarations.section.xml" />
 <xi:include href="option-types.section.xml" />

nixos/doc/manual/from_md/development/writing-modules.chapter.xml

@@ -122,6 +122,25 @@
     services) and <literal>systemd.timers</literal> (the list of
     commands to be executed periodically by <literal>systemd</literal>).
   </para>
+  <para>
+    Care must be taken when writing systemd services using
+    <literal>Exec*</literal> directives. By default systemd performs
+    substitution on <literal>%&lt;char&gt;</literal> specifiers in these
+    directives, expands environment variables from
+    <literal>$FOO</literal> and <literal>${FOO}</literal>, splits
+    arguments on whitespace, and splits commands on
+    <literal>;</literal>. All of these must be escaped to avoid
+    unexpected substitution or splitting when interpolating into an
+    <literal>Exec*</literal> directive, e.g. when using an
+    <literal>extraArgs</literal> option to pass additional arguments to
+    the service. The functions
+    <literal>utils.escapeSystemdExecArg</literal> and
+    <literal>utils.escapeSystemdExecArgs</literal> are provided for
+    this, see <link linkend="exec-escaping-example">Example: Escaping in
+    Exec directives</link> for an example. When using these functions
+    system environment substitution should <emphasis>not</emphasis> be
+    disabled explicitly.
+  </para>
   <anchor xml:id="locate-example" />
   <para>
     <emphasis role="strong">Example: NixOS Module for the
@@ -183,6 +202,36 @@ in {
       };
   };
 }
+</programlisting>
+  <anchor xml:id="exec-escaping-example" />
+  <para>
+    <emphasis role="strong">Example: Escaping in Exec
+    directives</emphasis>
+  </para>
+  <programlisting language="bash">
+{ config, lib, pkgs, utils, ... }:
+
+with lib;
+
+let
+  cfg = config.services.echo;
+  echoAll = pkgs.writeScript &quot;echo-all&quot; ''
+    #! ${pkgs.runtimeShell}
+    for s in &quot;$@&quot;; do
+      printf '%s\n' &quot;$s&quot;
+    done
+  '';
+  args = [ &quot;a%Nything&quot; &quot;lang=\${LANG}&quot; &quot;;&quot; &quot;/bin/sh -c date&quot; ];
+in {
+  systemd.services.echo =
+    { description = &quot;Echo to the journal&quot;;
+      wantedBy = [ &quot;multi-user.target&quot; ];
+      serviceConfig.Type = &quot;oneshot&quot;;
+      serviceConfig.ExecStart = ''
+        ${echoAll} ${utils.escapeSystemdExecArgs args}
+      '';
+    };
+}
 </programlisting>
   <xi:include href="option-declarations.section.xml" />
   <xi:include href="option-types.section.xml" />

nixos/lib/utils.nix

@@ -45,6 +45,26 @@ rec {
    replaceChars ["/" "-" " "] ["-" "\\x2d" "\\x20"]
    (removePrefix "/" s);
 
+  # Quotes an argument for use in Exec* service lines.
+  # systemd accepts "-quoted strings with escape sequences, toJSON produces
+  # a subset of these.
+  # Additionally we escape % to disallow expansion of % specifiers. Any lone ;
+  # in the input will be turned it ";" and thus lose its special meaning.
+  # Every $ is escaped to $$, this makes it unnecessary to disable environment
+  # substitution for the directive.
+  escapeSystemdExecArg = arg:
+    let
+      s = if builtins.isPath arg then "${arg}"
+        else if builtins.isString arg then arg
+        else if builtins.isInt arg || builtins.isFloat arg then toString arg
+        else throw "escapeSystemdExecArg only allows strings, paths and numbers";
+    in
+      replaceChars [ "%" "$" ] [ "%%" "$$" ] (builtins.toJSON s);
+
+  # Quotes a list of arguments into a single string for use in a Exec*
+  # line.
+  escapeSystemdExecArgs = concatMapStringsSep " " escapeSystemdExecArg;
+
   # Returns a system path for a given shell package
   toShellPath = shell:
     if types.shellPackage.check shell then

nixos/tests/all-tests.nix

@@ -459,6 +459,7 @@ in
   systemd-boot = handleTest ./systemd-boot.nix {};
   systemd-confinement = handleTest ./systemd-confinement.nix {};
   systemd-cryptenroll = handleTest ./systemd-cryptenroll.nix {};
+  systemd-escaping = handleTest ./systemd-escaping.nix {};
   systemd-journal = handleTest ./systemd-journal.nix {};
   systemd-networkd = handleTest ./systemd-networkd.nix {};
   systemd-networkd-dhcpserver = handleTest ./systemd-networkd-dhcpserver.nix {};

nixos/tests/systemd-escaping.nix

@@ -0,0 +1,45 @@
+import ./make-test-python.nix ({ pkgs, ... }:
+
+let
+  echoAll = pkgs.writeScript "echo-all" ''
+    #! ${pkgs.runtimeShell}
+    for s in "$@"; do
+      printf '%s\n' "$s"
+    done
+  '';
+  # deliberately using a local empty file instead of pkgs.emptyFile to have
+  # a non-store path in the test
+  args = [ "a%Nything" "lang=\${LANG}" ";" "/bin/sh -c date" ./empty-file 4.2 23 ];
+in
+{
+  name = "systemd-escaping";
+
+  machine = { pkgs, lib, utils, ... }: {
+    systemd.services.echo =
+      assert !(builtins.tryEval (utils.escapeSystemdExecArgs [ [] ])).success;
+      assert !(builtins.tryEval (utils.escapeSystemdExecArgs [ {} ])).success;
+      assert !(builtins.tryEval (utils.escapeSystemdExecArgs [ null ])).success;
+      assert !(builtins.tryEval (utils.escapeSystemdExecArgs [ false ])).success;
+      assert !(builtins.tryEval (utils.escapeSystemdExecArgs [ (_:_) ])).success;
+      { description = "Echo to the journal";
+        serviceConfig.Type = "oneshot";
+        serviceConfig.ExecStart = ''
+          ${echoAll} ${utils.escapeSystemdExecArgs args}
+        '';
+      };
+  };
+
+  testScript = ''
+    machine.wait_for_unit("multi-user.target")
+    machine.succeed("systemctl start echo.service")
+    # skip the first 'Starting <service> ...' line
+    logs = machine.succeed("journalctl -u echo.service -o cat").splitlines()[1:]
+    assert "a%Nything" == logs[0]
+    assert "lang=''${LANG}" == logs[1]
+    assert ";" == logs[2]
+    assert "/bin/sh -c date" == logs[3]
+    assert "/nix/store/ij3gw72f4n5z4dz6nnzl1731p9kmjbwr-empty-file" == logs[4]
+    assert "4.2" in logs[5] # toString produces extra fractional digits!
+    assert "23" == logs[6]
+  '';
+})