script: add argument validation (#81469)

partially fixes: #81349

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

lib/ansible/modules/command.py

@@ -276,7 +276,7 @@ def main():
     strip = module.params['strip_empty_ends']
     expand_argument_vars = module.params['expand_argument_vars']
 
-    # we promissed these in 'always' ( _lines get autoaded on action plugin)
+    # we promised these in 'always' ( _lines get auto-added on action plugin)
     r = {'changed': False, 'stdout': '', 'stderr': '', 'rc': None, 'cmd': None, 'start': None, 'end': None, 'delta': None, 'msg': ''}
 
     if not shell and executable:

lib/ansible/modules/script.py

@@ -12,15 +12,16 @@ version_added: "0.9"
 short_description: Runs a local script on a remote node after transferring it
 description:
   - The M(ansible.builtin.script) module takes the script name followed by a list of space-delimited arguments.
-  - Either a free form command or O(cmd) parameter is required, see the examples.
+  - Either a free-form command or O(cmd) parameter is required, see the examples.
-  - The local script at path will be transferred to the remote node and then executed.
+  - The local script at the path will be transferred to the remote node and then executed.
   - The given script will be processed through the shell environment on the remote node.
-  - This module does not require python on the remote system, much like the M(ansible.builtin.raw) module.
+  - This module does not require Python on the remote system, much like the M(ansible.builtin.raw) module.
   - This module is also supported for Windows targets.
 options:
   free_form:
     description:
       - Path to the local script file followed by optional arguments.
+    type: str
   cmd:
     type: str
     description:
@@ -29,26 +30,30 @@ options:
     description:
       - A filename on the remote node, when it already exists, this step will B(not) be run.
     version_added: "1.5"
+    type: str
   removes:
     description:
       - A filename on the remote node, when it does not exist, this step will B(not) be run.
     version_added: "1.5"
+    type: str
   chdir:
     description:
       - Change into this directory on the remote node before running the script.
     version_added: "2.4"
+    type: str
   executable:
     description:
-      - Name or path of a executable to invoke the script with.
+      - Name or path of an executable to invoke the script with.
     version_added: "2.6"
+    type: str
 notes:
   - It is usually preferable to write Ansible modules rather than pushing scripts. Convert your script to an Ansible module for bonus points!
   - The P(ansible.builtin.ssh#connection) connection plugin will force pseudo-tty allocation via C(-tt) when scripts are executed.
     Pseudo-ttys do not have a stderr channel and all stderr is sent to stdout. If you depend on separated stdout and stderr result keys,
-    please switch to a copy+command set of tasks instead of using script.
+    please switch to a set of tasks that comprises M(ansible.builtin.copy) with M(ansible.builtin.command) instead of using M(ansible.builtin.script).
   - If the path to the local script contains spaces, it needs to be quoted.
   - This module is also supported for Windows targets.
-  - If the script returns non UTF-8 data, it must be encoded to avoid issues. One option is to pipe
+  - If the script returns non-UTF-8 data, it must be encoded to avoid issues. One option is to pipe
     the output through C(base64).
 seealso:
   - module: ansible.builtin.shell
@@ -106,6 +111,6 @@ EXAMPLES = r'''
   args:
     executable: python3
 
-- name: Run a Powershell script on a windows host
+- name: Run a Powershell script on a Windows host
   script: subdirectories/under/path/with/your/playbook/script.ps1
 '''

lib/ansible/plugins/action/script.py

@@ -40,11 +40,25 @@ class ActionModule(ActionBase):
         if task_vars is None:
             task_vars = dict()
 
+        validation_result, new_module_args = self.validate_argument_spec(
+            argument_spec={
+                '_raw_params': {},
+                'cmd': {'type': 'str'},
+                'creates': {'type': 'str'},
+                'removes': {'type': 'str'},
+                'chdir': {'type': 'str'},
+                'executable': {'type': 'str'},
+            },
+            required_one_of=[
+                ['_raw_params', 'cmd']
+            ]
+        )
+
         result = super(ActionModule, self).run(tmp, task_vars)
         del tmp  # tmp no longer has any effect
 
         try:
-            creates = self._task.args.get('creates')
+            creates = new_module_args['creates']
             if creates:
                 # do not run the command if the line contains creates=filename
                 # and the filename already exists. This allows idempotence
@@ -52,7 +66,7 @@ class ActionModule(ActionBase):
                 if self._remote_file_exists(creates):
                     raise AnsibleActionSkip("%s exists, matching creates option" % creates)
 
-            removes = self._task.args.get('removes')
+            removes = new_module_args['removes']
             if removes:
                 # do not run the command if the line contains removes=filename
                 # and the filename does not exist. This allows idempotence
@@ -62,7 +76,7 @@ class ActionModule(ActionBase):
 
             # The chdir must be absolute, because a relative path would rely on
             # remote node behaviour & user config.
-            chdir = self._task.args.get('chdir')
+            chdir = new_module_args['chdir']
             if chdir:
                 # Powershell is the only Windows-path aware shell
                 if getattr(self._connection._shell, "_IS_WINDOWS", False) and \
@@ -75,13 +89,14 @@ class ActionModule(ActionBase):
             # Split out the script as the first item in raw_params using
             # shlex.split() in order to support paths and files with spaces in the name.
             # Any arguments passed to the script will be added back later.
-            raw_params = to_native(self._task.args.get('_raw_params', ''), errors='surrogate_or_strict')
+            raw_params = to_native(new_module_args.get('_raw_params', ''), errors='surrogate_or_strict')
             parts = [to_text(s, errors='surrogate_or_strict') for s in shlex.split(raw_params.strip())]
             source = parts[0]
 
             # Support executable paths and files with spaces in the name.
-            executable = to_native(self._task.args.get('executable', ''), errors='surrogate_or_strict')
+            executable = new_module_args['executable']
-
+            if executable:
+                executable = to_native(new_module_args['executable'], errors='surrogate_or_strict')
             try:
                 source = self._loader.get_real_file(self._find_needle('files', source), decrypt=self._task.args.get('decrypt', True))
             except AnsibleError as e:
@@ -90,7 +105,7 @@ class ActionModule(ActionBase):
             if self._task.check_mode:
                 # check mode is supported if 'creates' or 'removes' are provided
                 # the task has already been skipped if a change would not occur
-                if self._task.args.get('creates') or self._task.args.get('removes'):
+                if new_module_args['creates'] or new_module_args['removes']:
                     result['changed'] = True
                     raise _AnsibleActionDone(result=result)
                 # If the script doesn't return changed in the result, it defaults to True,

test/integration/targets/script/tasks/main.yml

@@ -37,6 +37,17 @@
 ## script
 ##
 
+- name: Required one of free-form and cmd
+  script:
+  ignore_errors: yes
+  register: script_required
+
+- name: assert that the script fails if neither free-form nor cmd is given
+  assert:
+    that:
+      - script_required.failed
+      - "'one of the following' in script_required.msg"
+
 - name: execute the test.sh script via command
   script: test.sh
   register: script_result0