X Tutup
Skip to content

[3.12] gh-124651: Quote template strings in venv activation scripts (GH-124712)#125947

Closed
ajayk wants to merge 1 commit intopython:3.12from
ajayk:backport-d48cc82-3.12
Closed

[3.12] gh-124651: Quote template strings in venv activation scripts (GH-124712)#125947
ajayk wants to merge 1 commit intopython:3.12from
ajayk:backport-d48cc82-3.12

Conversation

@ajayk
Copy link

@ajayk ajayk commented Oct 24, 2024 •
edited by bedevere-app bot
Loading

…4712)

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

(cherry picked from commit d48cc82)

@y5c4l3 @ajayk
pythongh-124651: Quote template strings in venv activation scripts (p…
4072b98 
…ythonGH-124712)

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

(cherry picked from commit d48cc82)
@ajayk ajayk requested a review from vsajip as a code owner October 24, 2024 23:28
@bedevere-app bedevere-app bot mentioned this pull request Oct 24, 2024
Closed
@vsajip
Copy link
Member

vsajip commented Oct 25, 2024

@ajayk please look into the build failures, I'll review once all tests are passing.

@mcepl
Copy link
Contributor

mcepl commented Oct 25, 2024 •
edited
Loading

When applying this patch to 3.12.7, I get this error:

[  774s] 0:02:16 load avg: 2.76 Re-running 1 failed tests in verbose mode in subprocesses
[  774s] 0:02:16 load avg: 2.76 Run 1 test in parallel using 1 worker process (timeout: 1 hour 30 min, worker timeout: 1 hour 35 min)
[  775s] 0:02:16 load avg: 2.76 [1/1/1] test_venv failed (1 error)
[  775s] Re-running test_venv in verbose mode (matching: test_special_chars_bash)
[  775s] test_special_chars_bash (test.test_venv.BasicTest.test_special_chars_bash)
[  775s] Test that the template strings are quoted properly (bash) ... ERROR
[  775s] 
[  775s] ======================================================================
[  775s] ERROR: test_special_chars_bash (test.test_venv.BasicTest.test_special_chars_bash)
[  775s] Test that the template strings are quoted properly (bash)
[  775s] ----------------------------------------------------------------------
[  775s] Traceback (most recent call last):
[  775s]   File "/home/abuild/rpmbuild/BUILD/Python-3.12.7/Lib/test/test_venv.py", line 477, in test_special_chars_bash
[  775s]     self.assertTrue(env_name.encode() in lines[0])
[  775s]                                          ~~~~~^^^
[  775s] IndexError: list index out of range
[  775s] 
[  775s] ----------------------------------------------------------------------
[  775s] Ran 1 test in 0.021s
[  775s] 
[  775s] FAILED (errors=1)

Complete build log with all packages used and steps taken to reproduce.

@vsajip vsajip marked this pull request as draft October 25, 2024 17:19
@vsajip
Copy link
Member

vsajip commented Oct 25, 2024

I've converted to draft for now, can be made ready for review once the errors are resolved.

@ajayk
Copy link
Author

ajayk commented Oct 25, 2024

@ajayk please look into the build failures, I'll review once all tests are passing.

on it

@picnixz
Copy link
Member

picnixz commented Nov 1, 2024

Closing this one since it was superseeded by #126185.

@picnixz picnixz closed this Nov 1, 2024
@picnixz picnixz changed the title gh-124651: Quote template strings in venv activation scripts (GH-12… [3.12] gh-124651: Quote template strings in venv activation scripts (GH-12… Nov 1, 2024
@bedevere-app bedevere-app bot mentioned this pull request Nov 1, 2024
Merged
@picnixz picnixz changed the title [3.12] gh-124651: Quote template strings in venv activation scripts (GH-12… [3.12] gh-124651: Quote template strings in venv activation scripts (GH-124712) Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vsajip vsajip Awaiting requested review from vsajip vsajip is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ajayk @vsajip @mcepl @picnixz @y5c4l3
X Tutup