Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap unsafe script shebangs in /bin/sh #2097

Merged
merged 1 commit into from
Feb 29, 2024
Merged

Wrap unsafe script shebangs in /bin/sh #2097

merged 1 commit into from
Feb 29, 2024

Conversation

charliermarsh
Copy link
Member

Summary

This is based on Pradyun's installer branch (https://github.com/pradyunsg/installer/blob/d01624e5f20963f046e67d58f5319e21a07aa03e/src/installer/scripts.py#L54), which is itself based on pip (https://github.com/pypa/pip/blob/0ad4c94be74cc24874c6feb5bb3c2152c398a18e/src/pip/_vendor/distlib/scripts.py#L136).

The gist of it is: on Posix platforms, if a path contains a space (or is too long), we wrap the shebang in a /bin/sh invocation.

Closes #2076.

Test Plan

❯ cargo run venv "foo"
    Finished dev [unoptimized + debuginfo] target(s) in 0.14s
     Running `target/debug/uv venv foo`
Using Python 3.12.0 interpreter at: /Users/crmarsh/.local/share/rtx/installs/python/3.12.0/bin/python3
Creating virtualenv at: foo
Activate with: source foo/bin/activate

❯ source "foo bar/bin/activate"

❯ which black
black not found

❯ cargo run pip install black
Resolved 6 packages in 177ms
Installed 6 packages in 17ms
 + black==24.2.0
 + click==8.1.7
 + mypy-extensions==1.0.0
 + packaging==23.2
 + pathspec==0.12.1
 + platformdirs==4.2.0

❯ which black
/Users/crmarsh/workspace/uv/foo bar/bin/black

❯ black
Usage: black [OPTIONS] SRC ...

One of 'SRC' or 'code' is required.

❯ cat "foo bar/bin/black"
#!/bin/sh
'''exec' '/Users/crmarsh/workspace/uv/foo bar/bin/python' "$0" "$@"
' '''
# -*- coding: utf-8 -*-
import re
import sys
from black import patched_main
if __name__ == "__main__":
    sys.argv[0] = re.sub(r"(-script\.pyw|\.exe)?$", "", sys.argv[0])
    sys.exit(patched_main())

@charliermarsh charliermarsh added the bug Something isn't working label Feb 29, 2024
@charliermarsh charliermarsh changed the title Wrap unsafe script shebangs in /bin/sh Wrap unsafe script shebangs in /bin/sh Feb 29, 2024
@charliermarsh charliermarsh marked this pull request as ready for review February 29, 2024 21:49
512
} else {
127
};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't really care, we could remove the platform and OS casing. (I assume this is safe to do on Windows, just not required?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would gently nudge toward using just one value at first and see if that causes any issues. I assume shebangs longer than 127 are pretty rare?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@charliermarsh charliermarsh force-pushed the charlie/shebang branch 5 times, most recently from b754f7c to 6e8b7bf Compare February 29, 2024 22:23
@charliermarsh charliermarsh enabled auto-merge (squash) February 29, 2024 23:15
@charliermarsh charliermarsh merged commit e811070 into main Feb 29, 2024
7 checks passed
@charliermarsh charliermarsh deleted the charlie/shebang branch February 29, 2024 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support paths with spaces in launchers
2 participants