From f29d432c6983683366dc7d9739bd89c1b4292c02 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Fri, 5 Sep 2025 11:21:58 +1000 Subject: [PATCH] powershell - remove quoting logic (#85781) Attempts to simplify the PowerShell code and special edge cases that removes quotes from a value like a src or destination path on Windows hosts. This should not be needed as paths should not be quoted when it comes to this section of the code. ci_complete --- changelogs/fragments/powershell-quoting.yml | 4 +++ lib/ansible/plugins/action/__init__.py | 8 ------ lib/ansible/plugins/action/fetch.py | 1 - lib/ansible/plugins/connection/psrp.py | 2 -- lib/ansible/plugins/connection/winrm.py | 2 -- lib/ansible/plugins/shell/powershell.py | 31 +++++---------------- 6 files changed, 11 insertions(+), 37 deletions(-) create mode 100644 changelogs/fragments/powershell-quoting.yml diff --git a/changelogs/fragments/powershell-quoting.yml b/changelogs/fragments/powershell-quoting.yml new file mode 100644 index 00000000000..b539f8ab7e2 --- /dev/null +++ b/changelogs/fragments/powershell-quoting.yml @@ -0,0 +1,4 @@ +breaking_changes: + - >- + powershell - Removed code that tried to remote quotes from paths when performing Windows operations like copying + and fetching file. This should not affect normal playbooks unless a value is quoted too many times. diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 0383b8c1dfb..dd1981f3843 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -287,14 +287,6 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): elif leaf_module_name == 'async_status' and collection_name in rewrite_collection_names: module_name = '%s.%s' % (win_collection, leaf_module_name) - # TODO: move this tweak down to the modules, not extensible here - # Remove extra quotes surrounding path parameters before sending to module. - if leaf_module_name in ['win_stat', 'win_file', 'win_copy', 'slurp'] and module_args and \ - hasattr(self._connection._shell, '_unquote'): - for key in ('src', 'dest', 'path'): - if key in module_args: - module_args[key] = self._connection._shell._unquote(module_args[key]) - result = self._shared_loader_obj.module_loader.find_plugin_with_context(module_name, mod_type, collection_list=self._task.collections) if not result.resolved: diff --git a/lib/ansible/plugins/action/fetch.py b/lib/ansible/plugins/action/fetch.py index b9f3bb8661c..4978f029994 100644 --- a/lib/ansible/plugins/action/fetch.py +++ b/lib/ansible/plugins/action/fetch.py @@ -130,7 +130,6 @@ class ActionModule(ActionBase): # calculate the destination name if os.path.sep not in self._connection._shell.join_path('a', ''): - source = self._connection._shell._unquote(source) source_local = source.replace('\\', '/') else: source_local = source diff --git a/lib/ansible/plugins/connection/psrp.py b/lib/ansible/plugins/connection/psrp.py index cef9b4346d7..7a7ddbbaa2f 100644 --- a/lib/ansible/plugins/connection/psrp.py +++ b/lib/ansible/plugins/connection/psrp.py @@ -489,7 +489,6 @@ class Connection(ConnectionBase): def put_file(self, in_path: str, out_path: str) -> None: super(Connection, self).put_file(in_path, out_path) - out_path = self._shell._unquote(out_path) display.vvv("PUT %s TO %s" % (in_path, out_path), host=self._psrp_host) script, in_data = _bootstrap_powershell_script('psrp_put_file.ps1', { @@ -549,7 +548,6 @@ class Connection(ConnectionBase): display.vvv("FETCH %s TO %s" % (in_path, out_path), host=self._psrp_host) - in_path = self._shell._unquote(in_path) out_path = out_path.replace('\\', '/') b_out_path = to_bytes(out_path, errors='surrogate_or_strict') diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index 179d848fe51..571bfece8d1 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -768,7 +768,6 @@ class Connection(ConnectionBase): def put_file(self, in_path: str, out_path: str) -> None: super(Connection, self).put_file(in_path, out_path) - out_path = self._shell._unquote(out_path) display.vvv('PUT "%s" TO "%s"' % (in_path, out_path), host=self._winrm_host) if not os.path.exists(to_bytes(in_path, errors='surrogate_or_strict')): raise AnsibleFileNotFound('file or module does not exist: "%s"' % to_native(in_path)) @@ -806,7 +805,6 @@ class Connection(ConnectionBase): def fetch_file(self, in_path: str, out_path: str) -> None: super(Connection, self).fetch_file(in_path, out_path) - in_path = self._shell._unquote(in_path) out_path = out_path.replace('\\', '/') # consistent with other connection plugins, we assume the caller has created the target dir display.vvv('FETCH "%s" TO "%s"' % (in_path, out_path), host=self._winrm_host) diff --git a/lib/ansible/plugins/shell/powershell.py b/lib/ansible/plugins/shell/powershell.py index 48d6a8d77cd..3f1936c62d2 100644 --- a/lib/ansible/plugins/shell/powershell.py +++ b/lib/ansible/plugins/shell/powershell.py @@ -192,7 +192,7 @@ class ShellModule(ShellBase): def join_path(self, *args): # use normpath() to remove doubled slashed and convert forward to backslashes - parts = [ntpath.normpath(self._unquote(arg)) for arg in args] + parts = [ntpath.normpath(arg) for arg in args] # Because ntpath.join treats any component that begins with a backslash as an absolute path, # we have to strip slashes from at least the beginning, otherwise join will ignore all previous @@ -210,7 +210,6 @@ class ShellModule(ShellBase): def path_has_trailing_slash(self, path): # Allow Windows paths to be specified using either slash. - path = self._unquote(path) return path.endswith('/') or path.endswith('\\') def chmod(self, paths, mode): @@ -223,11 +222,11 @@ class ShellModule(ShellBase): raise NotImplementedError('set_user_facl is not implemented for Powershell') def remove(self, path, recurse=False): - path = self._escape(self._unquote(path)) + quoted_path = self._escape(path) if recurse: - return self._encode_script("""Remove-Item '%s' -Force -Recurse;""" % path) + return self._encode_script("""Remove-Item '%s' -Force -Recurse;""" % quoted_path) else: - return self._encode_script("""Remove-Item '%s' -Force;""" % path) + return self._encode_script("""Remove-Item '%s' -Force;""" % quoted_path) def mkdtemp( self, @@ -240,7 +239,6 @@ class ShellModule(ShellBase): # compatibility in case other action plugins outside Ansible calls this. if not basefile: basefile = self.__class__._generate_temp_dir_name() - basefile = self._escape(self._unquote(basefile)) basetmpdir = self._escape(tmpdir if tmpdir else self.get_option('remote_tmp')) script = f""" @@ -263,7 +261,6 @@ class ShellModule(ShellBase): if not basefile: basefile = self.__class__._generate_temp_dir_name() - basefile = self._unquote(basefile) basetmpdir = tmpdir if tmpdir else self.get_option('remote_tmp') script, stdin = _bootstrap_powershell_script("powershell_mkdtemp.ps1", { @@ -283,7 +280,6 @@ class ShellModule(ShellBase): ) -> str: # This is not called in Ansible anymore but it is kept for backwards # compatibility in case other actions plugins outside Ansible called this. - user_home_path = self._unquote(user_home_path) if user_home_path == '~': script = 'Write-Output (Get-Location).Path' elif user_home_path.startswith('~\\'): @@ -297,7 +293,6 @@ class ShellModule(ShellBase): user_home_path: str, username: str = '', ) -> _ShellCommand: - user_home_path = self._unquote(user_home_path) script, stdin = _bootstrap_powershell_script("powershell_expand_user.ps1", { 'Path': user_home_path, }) @@ -308,7 +303,7 @@ class ShellModule(ShellBase): ) def exists(self, path): - path = self._escape(self._unquote(path)) + path = self._escape(path) script = """ If (Test-Path '%s') { @@ -329,7 +324,7 @@ class ShellModule(ShellBase): version="2.23", help_text="Use `ActionBase._execute_remote_stat()` instead.", ) - path = self._escape(self._unquote(path)) + path = self._escape(path) script = """ If (Test-Path -PathType Leaf '%(path)s') { @@ -364,7 +359,7 @@ class ShellModule(ShellBase): if arg_path: # Running a module without the exec_wrapper and with an argument # file. - script_path = self._unquote(cmd_parts[0]) + script_path = cmd_parts[0] if not script_path.lower().endswith('.ps1'): script_path += '.ps1' @@ -387,7 +382,6 @@ class ShellModule(ShellBase): cmd_parts.insert(0, shebang[2:]) elif not shebang: # The module is assumed to be a binary - cmd_parts[0] = self._unquote(cmd_parts[0]) cmd_parts.append(arg_path) script = """ Try @@ -431,17 +425,6 @@ class ShellModule(ShellBase): super().wrap_for_exec(cmd) return '& %s; exit $LASTEXITCODE' % cmd - def _unquote(self, value): - """Remove any matching quotes that wrap the given value.""" - value = to_text(value or '') - m = re.match(r'^\s*?\'(.*?)\'\s*?$', value) - if m: - return m.group(1) - m = re.match(r'^\s*?"(.*?)"\s*?$', value) - if m: - return m.group(1) - return value - def _escape(self, value): """Return value escaped for use in PowerShell single quotes.""" # There are 5 chars that need to be escaped in a single quote.