This repository was archived by the owner on Mar 6, 2026. It is now read-only.
Commit cf6fc3c
feat: Add shlex to correctly parse executable commands with spaces (#1855)
The `subprocess.run` command was using `.split()` which does not handle
quoted paths with spaces correctly. This would cause a
`FileNotFoundError` when the path to the executable contained spaces.
This change replaces `.split()` with `shlex.split()` to correctly parse
the command string.
A test case has been added to verify the fix and prevent regressions.
This was reported in b/237606033
Co-authored-by: Daniel Sanche <d.sanche14@gmail.com>1 parent b55aa11 commit cf6fc3c
2 files changed
Lines changed: 33 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -37,6 +37,7 @@ | |||
| 37 | 37 | from collections import Mapping # type: ignore | |
| 38 | 38 | import json | |
| 39 | 39 | import os | |
| 40 | + import shlex | ||
| 40 | 41 | import subprocess | |
| 41 | 42 | import sys | |
| 42 | 43 | import time | |
@@ -220,7 +221,7 @@ def retrieve_subject_token(self, request): | |||
| 220 | 221 | exe_stderr = sys.stdout if self.interactive else subprocess.STDOUT | |
| 221 | 222 | ||
| 222 | 223 | result = subprocess.run( | |
| 223 | - self._credential_source_executable_command.split(), | ||
| 224 | + shlex.split(self._credential_source_executable_command), | ||
| 224 | 225 | timeout=exe_timeout, | |
| 225 | 226 | stdin=exe_stdin, | |
| 226 | 227 | stdout=exe_stdout, | |
@@ -273,7 +274,7 @@ def revoke(self, request): | |||
| 273 | 274 | ||
| 274 | 275 | # Run executable | |
| 275 | 276 | result = subprocess.run( | |
| 276 | - self._credential_source_executable_command.split(), | ||
| 277 | + shlex.split(self._credential_source_executable_command), | ||
| 277 | 278 | timeout=self._credential_source_executable_interactive_timeout_millis | |
| 278 | 279 | / 1000, | |
| 279 | 280 | stdout=subprocess.PIPE, | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1239,6 +1239,36 @@ def test_retrieve_subject_token_python_2(self): | |||
| 1239 | 1239 | ||
| 1240 | 1240 | assert excinfo.match(r"Pluggable auth is only supported for python 3.7+") | |
| 1241 | 1241 | ||
| 1242 | + @mock.patch.dict(os.environ, {"GOOGLE_EXTERNAL_ACCOUNT_ALLOW_EXECUTABLES": "1"}) | ||
| 1243 | + def test_retrieve_subject_token_with_quoted_command(self): | ||
| 1244 | + command_with_spaces = '"/path/with spaces/to/executable" "arg with spaces"' | ||
| 1245 | + credential_source = { | ||
| 1246 | + "executable": {"command": command_with_spaces, "timeout_millis": 30000} | ||
| 1247 | + } | ||
| 1248 | + | ||
| 1249 | + with mock.patch( | ||
| 1250 | + "subprocess.run", | ||
| 1251 | + return_value=subprocess.CompletedProcess( | ||
| 1252 | + args=[], | ||
| 1253 | + stdout=json.dumps( | ||
| 1254 | + self.EXECUTABLE_SUCCESSFUL_OIDC_RESPONSE_ID_TOKEN | ||
| 1255 | + ).encode("UTF-8"), | ||
| 1256 | + returncode=0, | ||
| 1257 | + ), | ||
| 1258 | + ) as mock_run: | ||
| 1259 | + credentials = self.make_pluggable(credential_source=credential_source) | ||
| 1260 | + subject_token = credentials.retrieve_subject_token(None) | ||
| 1261 | + | ||
| 1262 | + assert subject_token == self.EXECUTABLE_OIDC_TOKEN | ||
| 1263 | + mock_run.assert_called_once_with( | ||
| 1264 | + ["/path/with spaces/to/executable", "arg with spaces"], | ||
| 1265 | + timeout=30.0, | ||
| 1266 | + stdin=None, | ||
| 1267 | + stdout=subprocess.PIPE, | ||
| 1268 | + stderr=subprocess.STDOUT, | ||
| 1269 | + env=mock.ANY, | ||
| 1270 | + ) | ||
| 1271 | + | ||
| 1242 | 1272 | @mock.patch.dict(os.environ, {"GOOGLE_EXTERNAL_ACCOUNT_ALLOW_EXECUTABLES": "1"}) | |
| 1243 | 1273 | def test_revoke_subject_token_python_2(self): | |
| 1244 | 1274 | with mock.patch("sys.version_info", (2, 7)): | |
0 commit comments