google-deepmind / google-deepmind/xmanager
`ARG_ESCAPER` shell-quotes arguments for direct `argv` backends
- Dominant language
- Python
- Stars
- 921
- Forks
- 69
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 1
Description
Hello!
I expected a job argument value to reach a job unchanged across executors, but observed that values requiring posix shell quoting arrive with literal `'` characters on `LOCAL_DOCKER`, `Vertex`, and `Kubernetes` executor. I see this causing, for example, a json serialized arguments failing.
I believe the problem is that `xm.utils.ARG_ESCAPER` applies `shlex.quote` while `SequentialArgs.to_list` constructs the argument list. This works for the local-binary path, where `asyncio.create_subprocess_shell` removes the quoting. However, the container backends pass each resulting token directly as `argv`, so nothing removes the quoting.
The `tmux` path has a different but related issue: it applies `subprocess.list2cmdline` to the shell-quoted tokens and then passes the result to a POSIX shell. `subprocess.list2cmdline` implements Windows quoting rules, so the two quoting schemes do not work together.
The exec-form Docker `ENTRYPOINT` path also constructs its json array by wrapping each argument in `"`. An argument containing `"` can therefore produce an invalid `Dockerfile`.
Here's a minimal reproduction:
Reproduction
```python
import subprocess
import sys
from xmanager import xm
from xmanager.xm import utils
value = '{"d": 4}'
arguments = xm.merge_args(
xm.SequentialArgs(),
{'config': value},
).to_list(utils.ARG_ESCAPER)
print(arguments)
subprocess.run(
[
sys.executable,
'-c',
'import sys; print(repr(sys.argv[1]))',
*arguments,
],
check=True,
)
```
```console
$ python arg_escaping_repro.py
['--config=\'{"d": 4}\'']
'--config=\'{"d": 4}\''
```
The job receives `'{"d": 4}'` rather than `{"d": 4}`.
I can work around this by using `ShellSafeArg` only for container backends. The annoying thing is that `ShellSafeArg` suppresses quoting for the shell path as well, so there is no single argument representation that works across executors.
I think the serialization needs to distinguish direct `argv` consumers from commands interpreted by a shell. I have a patch that I can open a PR with in a bit.
Contributor guide
Research direction
Start by tracing xm.utils.ARG_ESCAPER through SequentialArgs.to_list, then inspect the local-binary, container, tmux, and Docker ENTRYPOINT paths described in the issue. Verify the argument representation used by each executor with the supplied reproduction. Done means raw values reach direct argv consumers, shell consumers remain correctly quoted, and arguments containing double quotes produce valid Docker JSON.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100