Make encryption work with zip files #196

twiggler · 2024-10-16T10:27:26Z

Fix encryption in combination with zip files:

Fixed encryption stream being passed as invalid argument to ZipFile.
Let ZipFile detect there is no seek support on the encryption stream by raising the proper exception.
Add unit test

Bonus:

Fix bug in decrypter where on succes an error was thrown (invoking all() on a boolean)
Add typings to decrypter
Test encryption with tar output.

Closes #195

This type inherits from OSError so ZipFile can still detect that the stream lacks seek support

codecov · 2024-10-16T10:29:02Z

Codecov Report

Attention: Patch coverage is 92.85714% with 2 lines in your changes missing coverage. Please review.

Project coverage is 47.07%. Comparing base (3d77696) to head (ec8e239).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
acquire/tools/decrypter.py	92.30%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #196      +/-   ##
==========================================
+ Coverage   44.66%   47.07%   +2.41%     
==========================================
  Files          26       26              
  Lines        3401     3405       +4     
==========================================
+ Hits         1519     1603      +84     
+ Misses       1882     1802      -80

Flag	Coverage Δ
unittests	`47.07% <92.85%> (+2.41%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

twiggler · 2024-10-16T11:16:55Z

Windows pypy is failing:

Collecting requests_toolbelt

  Downloading requests_toolbelt-1.0.0-py2.py3-none-any.whl.metadata (14 kB)
  error: subprocess-exited-with-error

  

  × Preparing metadata (pyproject.toml) did not run successfully.

  │ exit code: 1

  ╰─> [44 lines of output]

      Traceback (most recent call last):

        File "C:\hostedtoolcache\windows\PyPy\3.10.14\x86\Lib\cffi\_shimmed_dist_utils.py", line 33, in <module>

          from distutils.msvc9compiler import MSVCCompiler

      ModuleNotFoundError: No module named 'distutils.msvc9compiler'

      

      The above exception was the direct cause of the following exception:

      

      Traceback (most recent call last):

        File "D:\a\acquire\acquire\.tox\pypy3.10\lib\site-packages\pip\_vendor\pyproject_hooks\_in_process\_in_process.py", line 353, in <module>

          main()

        File "D:\a\acquire\acquire\.tox\pypy3.10\lib\site-packages\pip\_vendor\pyproject_hooks\_in_process\_in_process.py", line 335, in main

          json_out['return_val'] = hook(**hook_input['kwargs'])

                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

        File "D:\a\acquire\acquire\.tox\pypy3.10\lib\site-packages\pip\_vendor\pyproject_hooks\_in_process\_in_process.py", line 149, in prepare_metadata_for_build_wheel

          return hook(metadata_directory, config_settings)

                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\build_meta.py", line 373, in prepare_metadata_for_build_wheel

          self.run_setup()

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\build_meta.py", line 318, in run_setup

          exec(code, locals())

        File "<string>", line 86, in <module>

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\__init__.py", line 117, in setup

          return distutils.core.setup(**attrs)

                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\_distutils\core.py", line 145, in setup

          _setup_distribution = dist = klass(attrs)

                                       ^^^^^^^^^^^^

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\dist.py", line 322, in __init__

          _Distribution.__init__(self, dist_attrs)

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\_distutils\dist.py", line 279, in __init__

          self.finalize_options()

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\dist.py", line 674, in finalize_options

          ep(self)

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\dist.py", line 694, in _finalize_setup_keywords

          ep.load()(self, ep.name, value)

        File "C:\hostedtoolcache\windows\PyPy\3.10.14\x86\Lib\cffi\setuptools_ext.py", line 216, in cffi_modules

          add_cffi_module(dist, cffi_module)

        File "C:\hostedtoolcache\windows\PyPy\3.10.14\x86\Lib\cffi\setuptools_ext.py", line 71, in add_cffi_module

          _add_c_module(dist, ffi, module_name, source, source_extension, kwds)

        File "C:\hostedtoolcache\windows\PyPy\3.10.14\x86\Lib\cffi\setuptools_ext.py", line 109, in _add_c_module

          from cffi._shimmed_dist_utils import Extension, log, mkpath

        File "C:\hostedtoolcache\windows\PyPy\3.10.14\x86\Lib\cffi\_shimmed_dist_utils.py", line 39, in <module>

          raise Exception("This CFFI feature requires distutils. Please install the distutils or setuptools package.") from ex

      Exception: This CFFI feature requires distutils. Please install the distutils or setuptools package.

      [end of output]

  

  note: This error originates from a subprocess, and is likely not a problem with pip.

error: metadata-generation-failed



× Encountered error while generating package metadata.

╰─> See above for output.



note: This is an issue with the package mentioned above, not pip.

hint: See above for details.

Seems unrelated to this PR.
Will take a look.

Schamper · 2024-10-16T12:38:22Z

acquire/tools/decrypter.py

+    out_path: Path,
+    key_file: Path | None = None,
+    key_server: str | None = None,
+    clobber=False,


Suggested change

clobber=False,

clobber: bool = False,

Miauwkeru · 2024-10-16T14:23:32Z

Windows pypy is failing:

Collecting requests_toolbelt

  Downloading requests_toolbelt-1.0.0-py2.py3-none-any.whl.metadata (14 kB)
  error: subprocess-exited-with-error

  

  × Preparing metadata (pyproject.toml) did not run successfully.

  │ exit code: 1

  ╰─> [44 lines of output]

      Traceback (most recent call last):

        File "C:\hostedtoolcache\windows\PyPy\3.10.14\x86\Lib\cffi\_shimmed_dist_utils.py", line 33, in <module>

          from distutils.msvc9compiler import MSVCCompiler

      ModuleNotFoundError: No module named 'distutils.msvc9compiler'

      

      The above exception was the direct cause of the following exception:

      

      Traceback (most recent call last):

        File "D:\a\acquire\acquire\.tox\pypy3.10\lib\site-packages\pip\_vendor\pyproject_hooks\_in_process\_in_process.py", line 353, in <module>

          main()

        File "D:\a\acquire\acquire\.tox\pypy3.10\lib\site-packages\pip\_vendor\pyproject_hooks\_in_process\_in_process.py", line 335, in main

          json_out['return_val'] = hook(**hook_input['kwargs'])

                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

        File "D:\a\acquire\acquire\.tox\pypy3.10\lib\site-packages\pip\_vendor\pyproject_hooks\_in_process\_in_process.py", line 149, in prepare_metadata_for_build_wheel

          return hook(metadata_directory, config_settings)

                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\build_meta.py", line 373, in prepare_metadata_for_build_wheel

          self.run_setup()

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\build_meta.py", line 318, in run_setup

          exec(code, locals())

        File "<string>", line 86, in <module>

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\__init__.py", line 117, in setup

          return distutils.core.setup(**attrs)

                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\_distutils\core.py", line 145, in setup

          _setup_distribution = dist = klass(attrs)

                                       ^^^^^^^^^^^^

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\dist.py", line 322, in __init__

          _Distribution.__init__(self, dist_attrs)

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\_distutils\dist.py", line 279, in __init__

          self.finalize_options()

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\dist.py", line 674, in finalize_options

          ep(self)

        File "C:\Users\runneradmin\AppData\Local\Temp\pip-build-env-6f0_va15\overlay\Lib\site-packages\setuptools\dist.py", line 694, in _finalize_setup_keywords

          ep.load()(self, ep.name, value)

        File "C:\hostedtoolcache\windows\PyPy\3.10.14\x86\Lib\cffi\setuptools_ext.py", line 216, in cffi_modules

          add_cffi_module(dist, cffi_module)

        File "C:\hostedtoolcache\windows\PyPy\3.10.14\x86\Lib\cffi\setuptools_ext.py", line 71, in add_cffi_module

          _add_c_module(dist, ffi, module_name, source, source_extension, kwds)

        File "C:\hostedtoolcache\windows\PyPy\3.10.14\x86\Lib\cffi\setuptools_ext.py", line 109, in _add_c_module

          from cffi._shimmed_dist_utils import Extension, log, mkpath

        File "C:\hostedtoolcache\windows\PyPy\3.10.14\x86\Lib\cffi\_shimmed_dist_utils.py", line 39, in <module>

          raise Exception("This CFFI feature requires distutils. Please install the distutils or setuptools package.") from ex

      Exception: This CFFI feature requires distutils. Please install the distutils or setuptools package.

      [end of output]

  

  note: This error originates from a subprocess, and is likely not a problem with pip.

error: metadata-generation-failed



× Encountered error while generating package metadata.

╰─> See above for output.



note: This is an issue with the package mentioned above, not pip.

hint: See above for details.

Seems unrelated to this PR. Will take a look.

this is related to an issue in pypy, it ships a specific cffi version with it that wants to use a deprecated function inside distutils. It should be fixed when a new pypy3.10 gets released

Miauwkeru · 2024-10-16T15:10:25Z

acquire/tools/decrypter.py

@@ -480,7 +494,7 @@ def main():
            elif not all(successes):
                exit_code = 2
            # Else, if all were successful but there were still tasks to handle, return 2
-            elif all(success) and tasks:
+            elif success and tasks:


this is not specifically a comment on your change, but i don't understand why success is here to begin with

You are absolutely right.

I think the original author meant all(successes).
This is superfluous, because we know at this point that not all(successes) == False,
which implies that all(successes) == True.

I also changed the exit_code to 3 in this case.
Not sure what the author intended, but otherwise he would probably have written not all(sucesses) or tasks in the second condition.
(Arbitrary in any case).

Indeed, all(successes) is not needed there, I think the all(success) is a residu of changing code around. At the end, you just need to check if there are still tasks to be handled, while the handled tasks are all successful.

I also agree with changing the exit code to 3.

twiggler added 6 commits October 15, 2024 15:08

Fix encryption for zip files

7822951

Fix error when decryption is success

9bcb7f2

Add typing to decrypter

b822b20

Test encryption in combination with zip

45a6371

Fix lint and add missing cstruct annotations

3cfe1e0

Use semantically clearer exception type

608fa64

This type inherits from OSError so ZipFile can still detect that the stream lacks seek support

Add unit test for tar encryption

9079bdb

twiggler force-pushed the fix-zip-encrypts branch from 7eeabec to 9079bdb Compare October 16, 2024 10:29

twiggler requested a review from Miauwkeru October 16, 2024 10:30

twiggler marked this pull request as draft October 16, 2024 10:43

twiggler force-pushed the fix-zip-encrypts branch 2 times, most recently from 43950be to 47ce747 Compare October 16, 2024 11:02

Fix typing

fb3037a

twiggler force-pushed the fix-zip-encrypts branch from 47ce747 to fb3037a Compare October 16, 2024 11:04

twiggler marked this pull request as ready for review October 16, 2024 11:12

Schamper requested changes Oct 16, 2024

View reviewed changes

Miauwkeru reviewed Oct 16, 2024

View reviewed changes

twiggler added 2 commits October 17, 2024 16:47

Make boolean typing explicit

4aff150

Remove superfluous check on success and make exit code unique

ec8e239

twiggler requested review from Schamper and Miauwkeru October 21, 2024 06:39

Schamper approved these changes Oct 21, 2024

View reviewed changes

Miauwkeru approved these changes Oct 24, 2024

View reviewed changes

twiggler merged commit 7177169 into main Oct 24, 2024
16 of 20 checks passed

twiggler deleted the fix-zip-encrypts branch October 24, 2024 08:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make encryption work with zip files #196

Make encryption work with zip files #196

twiggler commented Oct 16, 2024 •

edited

Loading

codecov bot commented Oct 16, 2024 •

edited

Loading

twiggler commented Oct 16, 2024

Schamper Oct 16, 2024

Miauwkeru commented Oct 16, 2024

Miauwkeru Oct 16, 2024

twiggler Oct 17, 2024

Poeloe Oct 24, 2024 •

edited

Loading

Make encryption work with zip files #196

Make encryption work with zip files #196

Conversation

twiggler commented Oct 16, 2024 • edited Loading

codecov bot commented Oct 16, 2024 • edited Loading

Codecov Report

twiggler commented Oct 16, 2024

Schamper Oct 16, 2024

Choose a reason for hiding this comment

Miauwkeru commented Oct 16, 2024

Miauwkeru Oct 16, 2024

Choose a reason for hiding this comment

twiggler Oct 17, 2024

Choose a reason for hiding this comment

Poeloe Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

twiggler commented Oct 16, 2024 •

edited

Loading

codecov bot commented Oct 16, 2024 •

edited

Loading

Poeloe Oct 24, 2024 •

edited

Loading