Fix command construction to support spaces

sylane · ziopio · commit 9d6d8e6a345a · 2026-02-19T16:07:05.000+01:00
diff --git a/src/grisp_tools_build.erl b/src/grisp_tools_build.erl
@@ -83,13 +83,15 @@ clone(#{repo_state := _State, otp_version := {_,_,_,Ver}} = S0) ->
     Branch = ["OTP-", Ver],
     % See https://github.com/erlang/rebar3/pull/2660
     S2 = mapz:deep_put([shell, env, "GIT_TERMINAL_PROMPT"], "0", S1),
-    {{ok, Output}, S3} = shell(S2,
-        "git clone "
-        "-b " ++ Branch ++ " "
-        "--depth 1 " ++
-        "--single-branch " ++
-        URL ++ " \"" ++ binary_to_list(BuildPath) ++ "\""
-    ),
+    Cmd = [
+        "git clone ",
+        "-b ", Branch, " ",
+        "--depth 1 ",
+        "--single-branch ",
+        URL, " ",
+        grisp_tools_util:shell_quote(BuildPath)
+    ],
+    {{ok, Output}, S3} = shell(S2, Cmd),
     event(mapz:deep_put([build, download], true, S3), [{output, Output}]).
 
 prepare(S0) ->
@@ -208,10 +210,12 @@ install(S0) ->
     grisp_tools_util:ensure_dir(filename:join(InstallPath, ".")),
     S1 = grisp_tools_util:pipe(S0, [
         fun(S) -> shell_ok(S,
-                           ["rm -rf ", filename:join(InstallPath, "*")],
+                           ["rm -rf ",
+                            grisp_tools_util:shell_quote(InstallPath), "/*"],
                            [{cd, InstallPath}]) end,
         fun(S) -> shell_ok(S,
-                           ["make install DESTDIR=", $", InstallPath, $"],
+                           ["make install DESTDIR=",
+                            grisp_tools_util:shell_quote(InstallPath)],
                            [{cd, BuildPath}]) end
     ]),
 
@@ -254,7 +258,8 @@ tar(#{build := #{flags := #{tar := true}}} = S0) ->
     Name = grisp_tools_util:package_name(S0),
     Package = filename:join(PackagePath, Name),
     grisp_tools_util:ensure_dir(Package),
-    shell_ok(S0, ["tar -zcf ", Package, " ."], [{cd, InstallPath}]),
+    shell_ok(S0, ["tar -zcf ", grisp_tools_util:shell_quote(Package), " ."],
+             [{cd, InstallPath}]),
     event(S0, [{file, relative(Package)}]);
 tar(S0) ->
     event(S0, ['_skip']).
@@ -276,30 +281,40 @@ dockerize_command(Cmd, S0) ->
     BuidPath = binary_to_list(mapz:deep_get([paths, build], S0)),
     {docker, Image} = mapz:deep_get([paths, toolchain], S0),
     BuildSubdir = string:prefix(BuidPath, Cwd),
-    ["docker run",
-    " --rm ",
-    [" -e " ++ K ++ "=" ++ io_lib:format("~s",[V])|| {K,V} <- maps:to_list(Env)],
-    " --volume " ++ Cwd ++ ":" ++ Cwd,
-    " " ++ Image ++ " sh -c \"cd " ++ Cwd ++ BuildSubdir,
-    " && " , Cmd, "\""].
+    VolumeArg = grisp_tools_util:shell_quote([Cwd, ":", Cwd]),
+    EnvArgs = [
+        [" -e ", grisp_tools_util:shell_quote([K, "=", V])]
+        || {K, V} <- maps:to_list(Env)
+    ],
+    Script = ["cd ", grisp_tools_util:shell_quote([Cwd, BuildSubdir]), " && ", Cmd],
+    [
+        "docker run --rm",
+        EnvArgs,
+        " --volume ", VolumeArg,
+        " ", grisp_tools_util:shell_quote(Image),
+        " sh -c ", grisp_tools_util:shell_quote(Script)
+    ].
 
 apply_patch({Name, Patch}, State0) ->
     Dir = mapz:deep_get([paths, build], State0),
     Context = mapz:deep_get([build, context], State0),
     grisp_tools_util:copy_file(Dir, Patch, Context),
     State4 = case shell(State0,
-                ["git apply ", Name, " --ignore-whitespace --reverse --check"],
+                ["git apply ", grisp_tools_util:shell_quote(Name),
+                 " --ignore-whitespace --reverse --check"],
                 [{cd, Dir}, return_on_error]) of
         {{ok, _Output}, State1} ->
             event(State1, [{skip, Patch}]);
         {{error, {1, _}}, State1} ->
             State2 = event(State1, [{apply, Patch}]),
             {{ok, _}, State3} = shell(State2,
-                                      "git apply --ignore-whitespace " ++ Name,
+                                      ["git apply --ignore-whitespace ",
+                                       grisp_tools_util:shell_quote(Name)],
                                       [{cd, Dir}]),
             State3
     end,
-    {{ok, _}, State5} = shell(State4, "rm " ++ Name, [{cd, Dir}]),
+    {{ok, _}, State5} = shell(State4, ["rm ", grisp_tools_util:shell_quote(Name)],
+                              [{cd, Dir}]),
     State5.
 
 sort_files(Apps, Files) ->
@@ -337,7 +352,7 @@ run_hooks(#{paths := #{toolchain := {ToolchainType, _}}} = S0, Type, Opts) ->
 
 delete_directory(BuildPath, S0) ->
     case filelib:is_dir(BuildPath) of
-        true -> shell_ok(S0, "rm -rf \"" ++ binary_to_list(BuildPath) ++ "\"", []);
+        true -> shell_ok(S0, ["rm -rf ", grisp_tools_util:shell_quote(BuildPath)], []);
         false -> S0
     end.
 
@@ -362,11 +377,13 @@ check_git_repository(BuildPath, S0) ->
 repo_sanity_check(Repo, S0) ->
     PlumbingTests = [
         {
-            "git --git-dir "++ Repo ++" fetch --dry-run",
+            ["git --git-dir ", grisp_tools_util:shell_quote(Repo),
+             " fetch --dry-run"],
             fun("") -> true; (_) -> false end
         },
         {
-            "git --git-dir "++ Repo ++" describe --tags",
+            ["git --git-dir ", grisp_tools_util:shell_quote(Repo),
+             " describe --tags"],
             fun(Out) ->
                 {_,_,_,V} = maps:get(otp_version, S0),
                 VersionTag = "OTP-"++binary_to_list(V)++"\n",
diff --git a/src/grisp_tools_deploy.erl b/src/grisp_tools_deploy.erl
@@ -311,7 +311,7 @@ validate_manifest(Data, Expected) ->
                 {internal_manifest_error, Reason, Data}
         end
     after
-        os:cmd("rm -f " ++ TempFilePath)
+        _ = file:delete(TempFilePath)
     end.
 
 dist_write_manifest(State0 = #{platform := Platform,
diff --git a/src/grisp_tools_firmware.erl b/src/grisp_tools_firmware.erl
@@ -250,7 +250,7 @@ cleanup_image(State) ->
     State.
 
 cleanup_temp_dir(State = #{temp_dir := TempDir, cleanup_temp_dir := true}) ->
-    {_, State2} = shell(State, ["rm -rf '", TempDir, "'"]),
+    {_, State2} = shell(State, ["rm -rf ", grisp_tools_util:shell_quote(TempDir)]),
     State2#{cleanup_temp_dir => false};
 cleanup_temp_dir(State) ->
     State.
@@ -336,17 +336,24 @@ select_bootloader(State, BaseDir, [Filename | Rest], CheckFun) ->
     end.
 
 docker_check_image(State, ImageName) ->
-    Command = ["docker manifest inspect '", ImageName, "'"],
+    Command = ["docker manifest inspect ", grisp_tools_util:shell_quote(ImageName)],
     case shell(State, Command, [return_on_error]) of
         {{ok, _}, State2} -> {ok, State2};
         {{error, _}, State2} -> {error, State2}
     end.
 
 docker_export(State, ImageName, InPath, OutDir) ->
     ok = grisp_tools_util:ensure_dir(OutDir),
-    Command = ["docker run --rm --volume ", OutDir,  ":", OutDir,
-               " ", ImageName, " sh -c \"cd ", OutDir,
-               " && cp -rf '", InPath, "' .\""],
+    VolumeArg = grisp_tools_util:shell_quote([OutDir, ":", OutDir]),
+    Script = [
+        "cd ", grisp_tools_util:shell_quote(OutDir),
+        " && cp -rf ", grisp_tools_util:shell_quote(InPath), " ."
+    ],
+    Command = [
+        "docker run --rm --volume ", VolumeArg,
+        " ", grisp_tools_util:shell_quote(ImageName),
+        " sh -c ", grisp_tools_util:shell_quote(Script)
+    ],
     case shell(State, Command, [return_on_error]) of
         {{ok, _}, State2} -> {ok, State2};
         {{error, _}, State2} -> {error, State2}
@@ -370,7 +377,10 @@ deploy_bundle(State = #{edifa_pid := Pid, bundle := BundleFile}, PartId) ->
         {error, Reason, State2} ->
             event(State2, [{error, Reason}]);
         {ok, MountPoint, State2} ->
-            ExpandCmd = ["tar -C ", MountPoint, " -xzf ", BundleFile],
+            ExpandCmd = [
+                "tar -C ", grisp_tools_util:shell_quote(MountPoint),
+                " -xzf ", grisp_tools_util:shell_quote(BundleFile)
+            ],
             {{ok, _}, State3} = shell(State2, ExpandCmd),
             Opts2 = edifa_opts(State2),
             case edifa:unmount(Pid, PartId, Opts2) of
diff --git a/src/grisp_tools_report.erl b/src/grisp_tools_report.erl
@@ -30,7 +30,7 @@ run(State) ->
 
 clean(#{flags := #{tar := true}} = S0) -> S0;
 clean(#{report_dir := ReportDir} = S0) ->
-    Cmd = lists:append(["rm -r ", ReportDir]),
+    Cmd = ["rm -r ", grisp_tools_util:shell_quote(ReportDir)],
     {_, S1} = shell(S0, Cmd, [return_on_error]),
     S1.
 
@@ -55,7 +55,11 @@ tar(#{report_dir := ReportDir, flags := #{tar := true}} = S0) ->
             Dir = filename:dirname(ReportDir),
             TarFilename = "grisp-report_" ++ format_datetime() ++ ".tar.gz",
             TarPath = filename:join(Dir, TarFilename),
-            Cmd = lists:append(["tar -czvf ", TarPath, " -C ", ReportDir, " ."]),
+            Cmd = [
+                "tar -czvf ", grisp_tools_util:shell_quote(TarPath),
+                " -C ", grisp_tools_util:shell_quote(ReportDir),
+                " ."
+            ],
             {{ok, _Res}, S1} = shell(S0, Cmd),
             event(S1, [TarPath])
     end.
diff --git a/src/grisp_tools_util.erl b/src/grisp_tools_util.erl
@@ -44,6 +44,7 @@
 -export([maybe_relative/3]).
 -export([filelib_ensure_path/1]).
 -export([format_term/1]).
+-export([shell_quote/1]).
 
 
 %--- Macros --------------------------------------------------------------------
@@ -363,6 +364,21 @@ format_term(Term) ->
     Bin = unicode:characters_to_binary(IoData, utf8),
     <<"%% coding: utf-8\n", Bin/binary>>.
 
+%% @doc Shell-quotes a value so it can be safely used as a single argument in
+%% POSIX shells (sh/bash/zsh). Uses single quotes and escapes embedded single
+%% quotes using the standard `'"'"'` sequence.
+-spec shell_quote(iodata()) -> iolist().
+shell_quote(Value) ->
+    Bin = unicode:characters_to_binary(iolist_to_binary(Value), utf8, utf8),
+    ["'", shell_quote_bin(Bin), "'"].
+
+shell_quote_bin(<<>>) ->
+    [];
+shell_quote_bin(<<$', Rest/binary>>) ->
+    ["'\"'\"'", shell_quote_bin(Rest)];
+shell_quote_bin(<<C/utf8, Rest/binary>>) ->
+    [C | shell_quote_bin(Rest)].
+
 
 %--- Internal ------------------------------------------------------------------
 
