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

gh-124651: Quote template strings in venv activation scripts #124712

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

y5c4l3
Copy link
Contributor

@y5c4l3 y5c4l3 commented Sep 27, 2024

This patch properly quotes template strings in venv activation scripts. This mitigates potential command injection.

@y5c4l3 y5c4l3 requested a review from vsajip as a code owner September 27, 2024 21:15
@vsajip vsajip added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 30, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vsajip for commit 3034419 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 30, 2024
This patch properly quotes template strings in `venv` activation
scripts. This mitigates potential command injection.

Signed-off-by: y5c4l3 <[email protected]>
@y5c4l3
Copy link
Contributor Author

y5c4l3 commented Oct 1, 2024

buildbot/AMD64 FreeBSD failed because bash was not found. I will add a test for tcsh / csh and skip the original test if bash is not present.

======================================================================
ERROR: test_special_chars (test.test_venv.BasicTest.test_special_chars)
Test that the template strings are quoted properly
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/buildbot/buildarea/pull_request.ware-freebsd/build/Lib/test/test_venv.py", line 515, in test_special_chars
    out, err = check_output([bash, test_script])
               ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
  File "/buildbot/buildarea/pull_request.ware-freebsd/build/Lib/test/test_venv.py", line 50, in check_output
    p = subprocess.Popen(cmd,
        stdout=subprocess.PIPE,
        stderr=subprocess.PIPE,
        env={**os.environ, "PYTHONHOME": ""})
  File "/buildbot/buildarea/pull_request.ware-freebsd/build/Lib/subprocess.py", line 1035, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
    ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                        pass_fds, cwd, env,
                        ^^^^^^^^^^^^^^^^^^^
    ...<5 lines>...
                        gid, gids, uid, umask,
                        ^^^^^^^^^^^^^^^^^^^^^^
                        start_new_session, process_group)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/buildbot/buildarea/pull_request.ware-freebsd/build/Lib/subprocess.py", line 1885, in _execute_child
    executable = os.fsencode(executable)
  File "/buildbot/buildarea/pull_request.ware-freebsd/build/Lib/os.py", line 852, in fsencode
    filename = fspath(filename)  # Does type-checking of `filename`.
TypeError: expected str, bytes or os.PathLike object, not NoneType

----------------------------------------------------------------------
Ran 36 tests in 7.578s

@vsajip vsajip added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 9, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vsajip for commit b6a3bbd 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 9, 2024
@y5c4l3
Copy link
Contributor Author

y5c4l3 commented Oct 10, 2024

@vsajip Some tests were still failing but none of them is related to this PR I guess.

@y5c4l3
Copy link
Contributor Author

y5c4l3 commented Oct 21, 2024

@vsajip Summary of the failing tests are posted here. Since they all appeared repeatedly before or after this build, I think this PR is good to go... no?

AMD64 Arch Linux TraceRefs PR/1443

FAIL: test_audit_subinterpreter (test.test_embed.AuditingTests.test_audit_subinterpreter)

iOS ARM64 Simulator PR/136

FAIL: test_alt_digits_nl_langinfo (test.test__locale._LocaleTests.test_alt_digits_nl_langinfo) (locale='ja_JP')

x86 Debian Installed with X PR/27
x86 Debian Non-Debug with X PR/27

make: *** [Makefile:1534: Modules/_hacl/Hacl_Hash_Blake2s_Simd128.o] Error 1
make: *** Waiting for unfinished jobs....

@vsajip vsajip merged commit d48cc82 into python:main Oct 21, 2024
106 of 110 checks passed
@vsajip vsajip added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes labels Oct 21, 2024
@miss-islington-app
Copy link

Thanks @y5c4l3 for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @y5c4l3 for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @y5c4l3 and @vsajip, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker d48cc82ed25e26b02eb97c6263d95dcaa1e9111b 3.12

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 21, 2024
…ythonGH-124712)

This patch properly quotes template strings in `venv` activation
scripts. This mitigates potential command injection.
(cherry picked from commit d48cc82)

Co-authored-by: Y5 <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Oct 21, 2024

GH-125813 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 21, 2024
vsajip pushed a commit that referenced this pull request Oct 22, 2024
ajayk pushed a commit to ajayk/cpython that referenced this pull request Oct 24, 2024
…ythonGH-124712)

This patch properly quotes template strings in `venv` activation
scripts. This mitigates potential command injection.

(cherry picked from commit d48cc82)
vstinner pushed a commit to vstinner/cpython that referenced this pull request Oct 30, 2024
…ythonGH-124712)

This patch properly quotes template strings in `venv` activation
scripts. This mitigates potential command injection.

(cherry picked from commit d48cc82)
@bedevere-app
Copy link

bedevere-app bot commented Oct 30, 2024

GH-126185 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Oct 30, 2024
vsajip pushed a commit that referenced this pull request Oct 31, 2024
@bedevere-app

This comment was marked as off-topic.

1 similar comment
@bedevere-app

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants