Skip to content

Port of #3357: Fix/insecure tempfile#3360

Merged
dmitryduev merged 3 commits intomasterfrom
fix/insecure_tempfile
Mar 10, 2022
Merged

Port of #3357: Fix/insecure tempfile#3360
dmitryduev merged 3 commits intomasterfrom
fix/insecure_tempfile

Conversation

@dmitryduev
Copy link
Copy Markdown
Member

Description

I pulled in #3357 and made a modification to the proposed solution that ensures that the created tempfile descriptor is closed before unlinking.

Testing

tests/test_util.py::test_make_tarfile passes.

@kptkin
Copy link
Copy Markdown
Collaborator

kptkin commented Mar 10, 2022

yeah that should also work... the official documentation (usage example):
https://docs.python.org/3/library/tempfile.html#deprecated-functions-and-variables

you could still use the NamedTemporaryFile just return the descriptor and close it obviously ☺️

@kptkin
Copy link
Copy Markdown
Collaborator

kptkin commented Mar 10, 2022

yeah that should also work... the official documentation (usage example): https://docs.python.org/3/library/tempfile.html#deprecated-functions-and-variables

you could still use the NamedTemporaryFile just return the descriptor

but you solution also great so approving

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 10, 2022

Codecov Report

Merging #3360 (9ec201b) into master (59255ba) will increase coverage by 0.10%.
The diff coverage is 92.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3360      +/-   ##
==========================================
+ Coverage   81.33%   81.44%   +0.10%     
==========================================
  Files         232      233       +1     
  Lines       28320    28545     +225     
==========================================
+ Hits        23035    23248     +213     
- Misses       5285     5297      +12     
Flag Coverage Δ
functest 57.62% <36.28%> (-0.14%) ⬇️
unittest 71.85% <92.92%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
wandb/sdk/wandb_artifacts.py 81.57% <ø> (ø)
wandb/sdk/data_types/saved_model.py 92.45% <92.45%> (ø)
wandb/data_types.py 84.07% <100.00%> (+0.01%) ⬆️
wandb/sdk/interface/artifacts.py 78.74% <100.00%> (+0.84%) ⬆️
wandb/util.py 85.94% <100.00%> (+0.01%) ⬆️
wandb/sdk/internal/artifacts.py 83.15% <0.00%> (-3.16%) ⬇️
wandb/sdk/internal/internal_api.py 83.09% <0.00%> (-0.43%) ⬇️
wandb/sdk/lib/git.py 76.35% <0.00%> (ø)
wandb/sdk/wandb_run.py 89.69% <0.00%> (+0.25%) ⬆️
wandb/sdk/launch/agent/agent.py 93.93% <0.00%> (+0.75%) ⬆️
... and 1 more

@dmitryduev dmitryduev merged commit 7c1306c into master Mar 10, 2022
@dmitryduev dmitryduev deleted the fix/insecure_tempfile branch March 10, 2022 18:22
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