Fix #2768: Quote template strings in activation scripts #2771
Conversation
This patch adds `quote` method in `ViaTemplateActivator` so that the magic template strings can be quoted correctly when replacing. This mitigates potential command injection (pypa#2768). Signed-off-by: y5c4l3 <y5c4l3@proton.me>
y5c4l3
force-pushed
the
activation-quote
branch
from
8954643 to
63cb3a8
Compare
| base = bin_dir[: -len(__BIN_NAME__) - 1] # strip away the bin part from the __file__, plus the path separator | ||
|
|
||
| # prepend bin to PATH (this file is inside the bin directory) | ||
| os.environ["PATH"] = os.pathsep.join([bin_dir, *os.environ.get("PATH", "").split(os.pathsep)]) | ||
| os.environ["VIRTUAL_ENV"] = base # virtual env is right above bin directory | ||
| os.environ["VIRTUAL_ENV_PROMPT"] = "__VIRTUAL_PROMPT__" or os.path.basename(base) # noqa: SIM222 | ||
| os.environ["VIRTUAL_ENV_PROMPT"] = __VIRTUAL_PROMPT__ or os.path.basename(base) | ||
|
|
||
| # add the virtual environments libraries to the host python import mechanism | ||
| prev_length = len(sys.path) | ||
| for lib in "__LIB_FOLDERS__".split(os.pathsep): | ||
| for lib in __LIB_FOLDERS__.split(os.pathsep): | ||
| path = os.path.realpath(os.path.join(bin_dir, lib)) | ||
| site.addsitedir(path.decode("utf-8") if "__DECODE_PATH__" else path) | ||
| site.addsitedir(path.decode("utf-8") if __DECODE_PATH__ else path) |
There was a problem hiding this comment.
I don't think you meant to remove quotes in .py files, e.g. "__VIRTUAL_PROMPT__" -> __VIRTUAL_PROMPT__. These simply become unassigned Python identifiers, and an error will be thrown whenever src/virtualenv/activation/python/activate_this.py is executed (imported).
There was a problem hiding this comment.
Or more likely I don't understand what and where sets them and why the quotes were present before this change. With quotes, the code does not make sense to me either.
There was a problem hiding this comment.
Those values are replaced with python representation here https://github.com/pypa/virtualenv/blob/main/src/virtualenv/activation/python/__init__.py#L21-L25. This is a template file.
There was a problem hiding this comment.
So in this case, the quotes must be there...
There was a problem hiding this comment.
Nope, because the replacer inserts it via quotes https://github.com/pypa/virtualenv/blob/main/src/virtualenv/activation/via_template.py#L76
There was a problem hiding this comment.
This is a template file.
Oh, I got it now. The file is not meant to be interpreted by Python before pre-processing (or creation of a real .py file based on it). Thanks for clarifying this for me!
P.S. For context, I was interested in this in relation to cython/cython#1961. Cython Debugger wants to run activate_this.py, which is not present in recent venv environments. Taking the template file activate_this.py is not going to work. Luckily, there is a stale PR for that issue, which contains a solution.
This patch adds
quotemethod inViaTemplateActivatorso that the magic template strings can be quoted correctly when replacing. This mitigates potential command injection (#2768).tox -e fix)docs/changelogfolder