Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.1k
Fix #2768: Quote template strings in activation scripts#2771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This patch adds `quote` method in `ViaTemplateActivator` so that themagic template strings can be quoted correctly when replacing. Thismitigates potential command injection (pypa#2768).Signed-off-by: y5c4l3 <y5c4l3@proton.me>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
86ddded intopypa:mainUh oh!
There was an error while loading.Please reload this page.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 wheneversrc/virtualenv/activation/python/activate_this.py is executed (imported).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Or more likely I don't understand what and where sets them andwhy the quotes were present before this change. With quotes, the code does not make sense to me either.
gaborbernatFeb 4, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Those values are replaced with python representation herehttps://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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
So in this case, the quotes must be there...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nope, because the replacer inserts it via quoteshttps://github.com/pypa/virtualenv/blob/main/src/virtualenv/activation/via_template.py#L76
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is a template file.
Oh, I got it now. Thefile 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 tocython/cython#1961. Cython Debugger wants to runactivate_this.py, which is not present in recentvenv environments. Taking the template fileactivate_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