security: prevent command injection in encrypt CLI command

.jules/sentinel.md

@@ -0,0 +1,4 @@
+## 2024-06-13 - Command Injection in CLI Utilities
+**Vulnerability:** Command injection in `src/amber/cli/commands/encrypt.cr` via string interpolation in the `system(...)` command (`system("#{options.editor} #{unencrypted_file}")`). If `env` or `editor` contains shell special characters, arbitrary commands could be executed.
+**Learning:** Passing a single string to `system(...)` causes it to be evaluated by a shell, allowing shell meta-characters to alter execution. User-controlled variables interpolated into this string create command injection vulnerabilities.
+**Prevention:** Avoid string interpolation when calling system commands. Use the array form of `system(executable, args_array)` or `Process.run(executable, args_array)` to bypass the shell entirely and pass arguments safely.

spec/amber/cli/commands/encrypt_spec.cr

@@ -26,5 +26,22 @@ module Amber::CLI
       File.read(".encryption_key").size.should eq 44
       cleanup
     end
+
+    it "does not execute shell metacharacters in the editor option" do
+      scaffold_app(TESTING_APP)
+      # Create an encrypted file to trigger the edit step
+      MainCommand.run ["encrypt", "test"]
+
+      marker = "amber_encrypt_editor_pwned"
+      File.delete(marker) if File.exists?(marker)
+
+      expect_raises(Exception) do
+        # Use an editor with command injection
+        MainCommand.run ["encrypt", "test", "-e", "sh -c 'touch #{marker}' #"]
+      end
+
+      File.exists?(marker).should be_false
+      cleanup
+    end
   end
 end

src/amber/cli/commands/encrypt.cr

@@ -26,15 +26,19 @@ module Amber::CLI
 
         if File.exists?(encrypted_file)
           File.write(unencrypted_file, Support::FileEncryptor.read(encrypted_file))
-          system("#{options.editor} #{unencrypted_file}") unless options.noedit?
+          Process.run(options.editor, [unencrypted_file], output: Process::Redirect::Inherit, error: Process::Redirect::Inherit) unless options.noedit?
         end
 
         if File.exists?(unencrypted_file)
           Support::FileEncryptor.write(encrypted_file, File.read(unencrypted_file))
           File.delete(unencrypted_file)
         end
       rescue e : Exception
-        exit! e.message, error: true
+        if ENV["AMBER_ENV"]? == "test" || ENV["CRYSTAL_CLI_ENV"]? == "test"
+          raise e
+        else
+          exit! e.message, error: true
+        end
       end
     end
   end