Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-57911: Sanitize symlink targets in tarfile on win32#138309
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
wiomoc commented Aug 31, 2025 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
fac592d to 5956b5dCompare5956b5d to 2ccc462Compareencukou commented Sep 1, 2025
Should this be done for hardlinks as well? I'd prefer saying “rewrite” rather than “sanitize”, as this is not fixing unsafe input. We should probably skip this for leading |
wiomoc commented Sep 1, 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.
This problem doesn't seam to occur when creating hardlinks: >>>os.mkdir("tmp") >>>os.mkdir("tmp\\child") >>>open("tmp\\child\\test.txt", "w").write("hello world") 11>>>os.link("tmp/child/test.txt", "tmp/testlink.txt") >>>open("tmp\\testlink.txt").read() 'hello world'
On the one hand, I see the risk of security implications, but I also note that in pathlib, slashes are also replaced in UNC paths. |
encukou left a comment
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.
OK; re-reading the duscussion I see experts suggesting to always replace, so let's go with that.
Uh oh!
There was an error while loading. Please reload this page.
bedevere-bot commented Sep 3, 2025
🤖 New build scheduled with the buildbot fleet by @encukou for commit 942b6e3 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F138309%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
| When extracting tar files on Windows Posix flavoured path separators in symlink | ||
| targets will be replaced by backward-slashes to prevent corrupted links. |
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.
| When extracting tar files on Windows Posix flavoured path separators in symlink | |
| targets will be replaced by backward-slashes to prevent corrupted links. | |
| When extracting tar files on Windows, slashes in symlink | |
| targets will be replaced by backslashes to prevent corrupted links. |
encukou commented Sep 3, 2025
Looks good! Could you also add an entry to |
c1a9c23 into python:mainUh oh!
There was an error while loading. Please reload this page.
encukou commented Sep 5, 2025
Thank you! |
picnixz commented Sep 7, 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.
I think this broke some buildbots: https://buildbot.python.org/#/builders/914. It wasn't detected until #138276 (which was 20h ago, but this change is 2 days ago and the previous buildbot run was 3 days ago) |
AA-Turner commented Sep 7, 2025
@picnixz if you open a revert PR as draft, you could use A |
picnixz commented Sep 7, 2025
I'm not on a dev session but I'll do it tomorrow if no one beats me to it. |
wiomoc commented Sep 7, 2025
The test only fails, when running on a Windows system with disabled symlink support - therefore it didn't occure on GH actions. |
CC @encukou