Fix security issues and potential crashes

This new implementation fixes the following issues that we had in the old implementation:

 - Issues and security risk with paths containing spaces or special parameters
    - This would not only have made the command fail
    - But it also created a security issue for parameter injection (e.g. `apk_output_file_path: 'foo --inject this && echo print the rest'`)
 - Risk of deleting any arbitrary folder on disk (due to use of unchecked `rm -rf`)
    - Calling the action with `apk_output_file_path: '.'` for example (or a path to any directory) would have the side effect of… deleting the whole content of the current directory before failing
    - This was even more problematic that this could be an easy genuine accidental error to make—especially if the caller misread the documentation and thought that the command accepted a folder for the output path instead of the path to the apk file.

In addition, this commit adds the following features to the action's behavior:

 - The action will detect if the parent directory of the `apk_output_file_path` does not exist, and exit early with a nice error if so
 - We now allow `apk_output_file_path` to be a directory, and if so the output file will be put in that directory and use the same basename as the input aab_file_path (but with `.apk` extension instead of `.aab`)
 - The action now returns the path to the generated file (which is especially useful if you didn't provide an explicit value for `apk_output_file_path` and let the action infer it, or if you provided a directory for this output path)

Author: AliSoftware

lib/fastlane/plugin/wpmreleasetoolkit/actions/android/android_generate_apk_from_aab.rb

@@ -1,32 +1,47 @@
 module Fastlane
   module Actions
     class AndroidGenerateApkFromAabAction < Action
-      def self.generate_command(aab_file_path, apk_output_file_path, keystore_path, keystore_password, keystore_key_alias, signing_key_password)
-        command = "bundletool build-apks --mode universal --bundle #{aab_file_path} --output-format DIRECTORY --output #{apk_output_file_path} "
-        code_sign_arguments = "--ks #{keystore_path} --ks-pass #{keystore_password} --ks-key-alias #{keystore_key_alias} --key-pass #{signing_key_password} "
-        move_and_cleanup_command = "&& mv #{apk_output_file_path}/universal.apk #{apk_output_file_path}_tmp && rm -rf #{apk_output_file_path} && mv #{apk_output_file_path}_tmp #{apk_output_file_path}"
+      def self.run(params)
+        begin
+          sh('command', '-v', 'bundletool', print_command: false, print_command_output: false)
+        rescue StandardError
+          UI.user_error!(MISSING_BUNDLETOOL_ERROR_MESSAGE)
+        end
 
-        # Attempt to code sign the APK if a keystore_path parameter is specified
-        command += code_sign_arguments unless keystore_path.nil?
+        # Parse input parameters
+        aab_file_path = parse_aab_param(params)
+        apk_output_file_path = params[:apk_output_file_path] || Pathname(aab_file_path).sub_ext('.apk').to_s
+        code_sign_arguments = {
+          '--ks': params[:keystore_path],
+          '--ks-pass': params[:keystore_password],
+          '--ks-key-alias': params[:keystore_key_alias],
+          '--key-pass': params[:signing_key_password]
+        }.compact.flatten.map(&:to_s)
 
-        # Move and rename the universal.apk file to the specified output path and cleanup the directory created by bundletool
-        command += move_and_cleanup_command
+        if File.directory?(apk_output_file_path)
+          apk_output_file_path = File.join(apk_output_file_path, "#{File.basename(aab_file_path, '.aab')}.apk")
+        end
 
-        return command
+        Dir.mktmpdir('a8c-release-toolkit-bundletool-') do |tmpdir|
+          sh(
+            'bundletool', 'build-apks',
+            '--mode', 'universal',
+            '--bundle', aab_file_path,
+            '--output-format', 'DIRECTORY',
+            '--output', tmpdir,
+            *code_sign_arguments
+          )
+          FileUtils.mkdir_p(File.dirname(apk_output_file_path)) # Create destination directory if it doesn't exist yet
+          FileUtils.mv(File.join(tmpdir, 'universal.apk'), apk_output_file_path)
+        end
+
+        apk_output_file_path
       end
 
-      def self.run(params)
-        begin
-          sh('command -v bundletool > /dev/null')
-        rescue StandardError
-          UI.user_error!('bundletool is not installed. Please install it using the instructions at https://developer.android.com/studio/command-line/bundletool.')
-          raise
-        end
+      #####################################################
 
+      def self.parse_aab_param(params)
         # If no AAB param was provided, attempt to get it from the lane context
-        # First GRADLE_ALL_AAB_OUTPUT_PATHS if only one
-        # Second GRADLE_AAB_OUTPUT_PATH if it is set
-        # Else use the specified parameter value
         if params[:aab_file_path].nil?
           all_aab_paths = Actions.lane_context[SharedValues::GRADLE_ALL_AAB_OUTPUT_PATHS] || []
           aab_file_path = if all_aab_paths.count == 1
@@ -40,19 +55,17 @@ def self.run(params)
 
         # If no AAB file path was found, raise an error
         if aab_file_path.nil?
-          UI.user_error!('No AAB file path was specified and none was found in the lane context. Please specify the `aab_file_path` parameter or ensure that the relevant build action has been run prior to this action.')
-          raise
+          UI.user_error!(NO_AAB_ERROR_MESSAGE)
+        elsif !File.file?(aab_file_path)
+          UI.user_error!("The file `#{aab_file_path}` was not found. Please provide a path to an existing file.")
         end
 
-        apk_output_file_path = params[:apk_output_file_path]
-        keystore_path = params[:keystore_path]
-        keystore_password = params[:keystore_password]
-        keystore_key_alias = params[:keystore_key_alias]
-        signing_key_password = params[:signing_key_password]
-
-        sh(generate_command(aab_file_path, apk_output_file_path, keystore_path, keystore_password, keystore_key_alias, signing_key_password))
+        aab_file_path
       end
 
+      MISSING_BUNDLETOOL_ERROR_MESSAGE = 'bundletool is not installed. Please install it using the instructions at https://developer.android.com/studio/command-line/bundletool.'.freeze
+      NO_AAB_ERROR_MESSAGE = 'No AAB file path was specified and none was found in the lane context. Please specify the `aab_file_path` parameter or ensure that the `gradle` action has been run prior to this action.'.freeze
+
       #####################################################
       # @!group Documentation
       #####################################################
@@ -62,70 +75,77 @@ def self.description
       end
 
       def self.details
-        'Generates an APK from the specified AAB'
+        'Generates an APK file from the specified AAB file using `bundletool`'
       end
 
       def self.available_options
         [
           FastlaneCore::ConfigItem.new(
             key: :aab_file_path,
             env_name: 'ANDROID_AAB_FILE_PATH',
-            description: 'The path to the AAB file. If not speicified, the action will attempt to read from the lane context using the `SharedValues::GRADLE_ALL_AAB_OUTPUT_PATHS` and `SharedValues::GRADLE_AAB_OUTPUT_PATH` keys',
+            description: 'The path to the AAB file. If not specified, the action will attempt to read from the lane context using the `SharedValues::GRADLE_ALL_AAB_OUTPUT_PATHS` and `SharedValues::GRADLE_AAB_OUTPUT_PATH` keys',
             type: String,
             optional: true,
-            default_value: nil,
-            verify_block: proc { |p| UI.user_error!("AAB path `#{p}` is not a valid file path.") unless File.file?(p) }
+            default_value: nil
           ),
           FastlaneCore::ConfigItem.new(
             key: :apk_output_file_path,
             env_name: 'ANDROID_APK_OUTPUT_PATH',
-            description: 'The output path where the APK file will be generated. The directory will be created if it does not yet exist',
+            description: 'The path of the output APK file to generate. If not specified, will use the same path and basename as the `aab_file_path` but with an `.apk` file extension',
             type: String,
-            optional: false,
+            optional: true,
             default_value: nil
           ),
           FastlaneCore::ConfigItem.new(
             key: :keystore_path,
             env_name: 'ANDROID_KEYSTORE_PATH',
-            description: 'The path to the keystore file',
+            description: 'The path to the keystore file (if you want to codesign the APK)',
             type: String,
             optional: true,
             default_value: nil,
-            verify_block: proc { |p| UI.user_error!("Keystore file path `#{p}` is not a valid file path.") unless File.file?(p) || p.nil }
+            verify_block: proc { |p| UI.user_error!("Keystore file path `#{p}` is not a valid file path.") unless p.nil? || File.file?(p) }
           ),
           FastlaneCore::ConfigItem.new(
             key: :keystore_password,
             env_name: 'ANDROID_KEYSTORE_PASSWORD',
-            description: 'The password for the keystore',
+            description: 'The password for the keystore (if you want to codesign the APK)',
             type: String,
             optional: true,
             default_value: nil
           ),
           FastlaneCore::ConfigItem.new(
             key: :keystore_key_alias,
             env_name: 'ANDROID_KEYSTORE_KEY_ALIAS',
-            description: 'The alias of the key in the keystore',
+            description: 'The alias of the key in the keystore (if you want to codesign the APK)',
             type: String,
             optional: true,
             default_value: nil
           ),
           FastlaneCore::ConfigItem.new(
             key: :signing_key_password,
             env_name: 'ANDROID_SIGNING_KEY_PASSWORD',
-            description: 'The password for the signing key',
+            description: 'The password for the signing key (if you want to codesign the APK)',
             type: String,
             optional: true,
             default_value: nil
           ),
         ]
       end
 
+      def self.return_type
+        :string
+      end
+
+      def self.return_value
+        'The path to the APK that has been generated'
+      end
+
       def self.authors
         ['Automattic']
       end
 
       def self.is_supported?(platform)
-        true
+        platform == 'android'
       end
     end
   end