fix Windows env handling (#22927)

* fixes #22441
* fixes #22655
* moves all env handling into the exec wrapper; this should work for everything but raw, which is consistent with non-Windows.

8 files changed, 52 insertions, 16 deletions

diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py
index e7384b92af..4f5effe462 100644
--- a/lib/ansible/executor/module_common.py
+++ b/lib/ansible/executor/module_common.py
@@ -831,12 +831,13 @@ def modify_module(module_name, module_path, module_args, task_vars=dict(), modul
 
     return (b_module_data, module_style, to_text(shebang, nonstring='passthru'))
 
-def build_windows_module_payload(module_name, module_path, b_module_data, module_args, task_vars, task, play_context):
+def build_windows_module_payload(module_name, module_path, b_module_data, module_args, task_vars, task, play_context, environment):
     exec_manifest = dict(
         module_entry=base64.b64encode(b_module_data),
         powershell_modules=dict(),
         module_args=module_args,
-        actions=['exec']
+        actions=['exec'],
+        environment=environment
     )
 
     exec_manifest['exec'] = base64.b64encode(to_bytes(leaf_exec))
diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py
index a4372bdaa4..65fae3d17f 100644
--- a/lib/ansible/plugins/action/__init__.py
+++ b/lib/ansible/plugins/action/__init__.py
@@ -162,14 +162,16 @@ class ActionBase(with_metaclass(ABCMeta, object)):
         # FUTURE: we'll have to get fancier about this to support powershell over SSH on Windows...
         if self._connection.transport == "winrm":
             # WinRM always pipelines, so we need to build up a fancier module payload...
+            final_environment = dict()
+            self._compute_environment_string(final_environment)
             module_data = build_windows_module_payload(module_name=module_name, module_path=module_path,
                                                    b_module_data=module_data, module_args=module_args,
                                                    task_vars=task_vars, task=self._task,
-                                                   play_context=self._play_context)
+                                                   play_context=self._play_context, environment=final_environment)
 
         return (module_style, module_shebang, module_data, module_path)
 
-    def _compute_environment_string(self):
+    def _compute_environment_string(self, raw_environment_out=dict()):
         '''
         Builds the environment string to be used when executing the remote task.
         '''
@@ -195,6 +197,11 @@ class ActionBase(with_metaclass(ABCMeta, object)):
                 final_environment.update(temp_environment)
 
         final_environment = self._templar.template(final_environment)
+
+        if isinstance(raw_environment_out, dict):
+            raw_environment_out.clear()
+            raw_environment_out.update(final_environment)
+
         return self._connection._shell.env_prefix(**final_environment)
 
     def _early_needs_tmp_path(self):
diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py
index da4b97d390..32960d4ca8 100644
--- a/lib/ansible/plugins/action/script.py
+++ b/lib/ansible/plugins/action/script.py
@@ -81,11 +81,17 @@ class ActionModule(ActionBase):
         self._fixup_perms2((tmp, tmp_src), execute=True)
 
         # add preparation steps to one ssh roundtrip executing the script
-        env_string = self._compute_environment_string()
+        env_dict = dict()
+        env_string = self._compute_environment_string(env_dict)
         script_cmd = ' '.join([env_string, tmp_src, args])
         script_cmd = self._connection._shell.wrap_for_exec(script_cmd)
 
-        result.update(self._low_level_execute_command(cmd=script_cmd, sudoable=True))
+        exec_data = None
+        # HACK: come up with a sane way to pass around env outside the command
+        if self._connection.transport == "winrm":
+            exec_data = self._connection._create_raw_wrapper_payload(script_cmd, env_dict)
+
+        result.update(self._low_level_execute_command(cmd=script_cmd, in_data=exec_data, sudoable=True))
 
         # clean up after
         self._remove_tmp_path(tmp)
diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py
index aca000d640..7767544949 100644
--- a/lib/ansible/plugins/connection/winrm.py
+++ b/lib/ansible/plugins/connection/winrm.py
@@ -305,12 +305,13 @@ class Connection(ConnectionBase):
         self.shell_id = None
         self._connect()
 
-    def _create_raw_wrapper_payload(self, cmd):
+    def _create_raw_wrapper_payload(self, cmd, environment=dict()):
         payload = {
             'module_entry': base64.b64encode(to_bytes(cmd)),
             'powershell_modules': {},
             'actions': ['exec'],
-            'exec': base64.b64encode(to_bytes(leaf_exec))
+            'exec': base64.b64encode(to_bytes(leaf_exec)),
+            'environment': environment
         }
 
         return json.dumps(payload)
diff --git a/lib/ansible/plugins/shell/powershell.py b/lib/ansible/plugins/shell/powershell.py
index 35ba05e78e..fa735ac654 100644
--- a/lib/ansible/plugins/shell/powershell.py
+++ b/lib/ansible/plugins/shell/powershell.py
@@ -111,6 +111,11 @@ Function Run($payload) {
     # redefine Write-Host to dump to output instead of failing- lots of scripts use it
     $ps.AddStatement().AddScript("Function Write-Host(`$msg){ Write-Output `$msg }") | Out-Null
 
+    ForEach ($env_kv in $payload.environment.GetEnumerator()) {
+        $escaped_env_set = "`$env:{0} = '{1}'" -f $env_kv.Key,$env_kv.Value.Replace("'","''")
+        $ps.AddStatement().AddScript($escaped_env_set) | Out-Null
+    }
+
     # dynamically create/load modules
     ForEach ($mod in $payload.powershell_modules.GetEnumerator()) {
         $decoded_module = [System.Text.Encoding]::UTF8.GetString([System.Convert]::FromBase64String($mod.Value))
@@ -132,6 +137,7 @@ Function Run($payload) {
         If(-not $exit_code) {
             $exit_code = 1
         }
+        # need to use this instead of Exit keyword to prevent runspace from crashing with dynamic modules
         $host.SetShouldExit($exit_code)
     }
 }
@@ -967,9 +973,8 @@ class ShellModule(object):
         return to_text(value, errors='surrogate_or_strict')
 
     def env_prefix(self, **kwargs):
-        env = self.env.copy()
-        env.update(kwargs)
-        return ';'.join(["$env:%s='%s'" % (self.assert_safe_env_key(k), self.safe_env_value(k,v)) for k,v in env.items()])
+        # powershell/winrm env handling is handled in the exec wrapper
+        return ""
 
     def join_path(self, *args):
         parts = []
diff --git a/test/integration/targets/win_script/files/test_script_with_env.ps1 b/test/integration/targets/win_script/files/test_script_with_env.ps1
new file mode 100644
index 0000000000..b54fd9280d
--- /dev/null
+++ b/test/integration/targets/win_script/files/test_script_with_env.ps1
@@ -0,0 +1 @@
+$env:taskenv
\ No newline at end of file
diff --git a/test/integration/targets/win_script/tasks/main.yml b/test/integration/targets/win_script/tasks/main.yml
index e1f08a3dec..b4c586034f 100644
--- a/test/integration/targets/win_script/tasks/main.yml
+++ b/test/integration/targets/win_script/tasks/main.yml
@@ -192,3 +192,16 @@
     that:
       - "test_script_bool_result.stdout_lines[0] == 'System.Boolean'"
       - "test_script_bool_result.stdout_lines[1] == 'True'"
+
+- name: run test script that uses envvars
+  script: test_script_with_env.ps1
+  environment:
+    taskenv: task
+  register: test_script_env_result
+
+- name: ensure that script ran and that environment var was passed
+  assert:
+    that:
+    - test_script_env_result | succeeded
+    - test_script_env_result.stdout_lines[0] == 'task'
+    
\ No newline at end of file
diff --git a/test/integration/targets/win_shell/tasks/main.yml b/test/integration/targets/win_shell/tasks/main.yml
index 5e50b72eda..16759bb283 100644
--- a/test/integration/targets/win_shell/tasks/main.yml
+++ b/test/integration/targets/win_shell/tasks/main.yml
@@ -17,8 +17,10 @@
     - shellout.stdout == "hello from Ansible\r\n"
     - shellout.stdout_lines == ["hello from Ansible"]
 
-- name: execute a powershell cmdlet with multi-line output
-  win_shell: Write-Output "hello from Ansible"; Write-Output "another line"; Write-Output "yet another line"
+- name: execute a powershell cmdlet with multi-line output that uses environment with embedded quotes
+  win_shell: Write-Output "hello from Ansible"; Write-Output "another line"; Write-Output "yet another line"; Write-Output "envvar was $env:taskvar"
+  environment:
+    taskvar: "o'doyle rules"
   register: shellout
 
 - name: validate result
@@ -26,15 +28,15 @@
     that:
     - shellout|success
     - shellout|changed
-    - shellout.cmd == 'Write-Output "hello from Ansible"; Write-Output "another line"; Write-Output "yet another line"'
+    - shellout.cmd == 'Write-Output "hello from Ansible"; Write-Output "another line"; Write-Output "yet another line"; Write-Output "envvar was $env:taskvar"'
     - shellout.delta is match('^\d:(\d){2}:(\d){2}.(\d){6}$')
     - shellout.end is match('^(\d){4}\-(\d){2}\-(\d){2} (\d){2}:(\d){2}:(\d){2}.(\d){6}$')
     - shellout.rc == 0
     - shellout.start is match('^(\d){4}\-(\d){2}\-(\d){2} (\d){2}:(\d){2}:(\d){2}.(\d){6}$')
     # assertion disabled since it does not pass on Windows Server 2016
     # - shellout.stderr == ""
-    - shellout.stdout == "hello from Ansible\r\nanother line\r\nyet another line\r\n"
-    - shellout.stdout_lines == ["hello from Ansible","another line", "yet another line"]
+    - shellout.stdout == "hello from Ansible\r\nanother line\r\nyet another line\r\nenvvar was o'doyle rules\r\n"
+    - shellout.stdout_lines == ["hello from Ansible","another line", "yet another line", "envvar was o'doyle rules"]
 
 - name: execute something nonexistent
   win_shell: bogus_command1234