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

Enable hash checking again (currently hashes from lock file are not validated at all) #113

Closed

Conversation

FlorianLudwig
Copy link

@FlorianLudwig FlorianLudwig commented Nov 26, 2020

Resolves: python-poetry/poetry#2422
Replaces: python-poetry/poetry#2611
Blocks: [TBD]

  • Added tests for changed code.
  • Updated documentation for changed code. - Not needed as this is a bug fix

Questions

  1. The MR template asks me to base this on develop but no branch with this name exists, so I am sticking to master, am I missing something?
  2. I had some trouble setting up my local dev environment for poetry, so no unit tests yet. How do I get proper tracebacks out of poetry? I only get the last line not a full traceback or no line at all and just the exception name.

@FlorianLudwig FlorianLudwig changed the title Cleanup hash checking Emable hash checking again Feb 1, 2021
@FlorianLudwig FlorianLudwig changed the title Emable hash checking again Enable hash checking again (currently hashes from lock file are not validated at all) Feb 1, 2021
Copy link
Contributor

@0xDEC0DE 0xDEC0DE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've adapted the Dockerfile at https://gist.github.com/slai/9d0d442fe7e4f4ea04e8f658b675192a to use this change, and it does indeed fail:

[+] Building 8.8s (23/23) FINISHED
 => [internal] load build definition from Dockerfile                                                                                                                                              0.0s
 => => transferring dockerfile: 37B                                                                                                                                                               0.0s
 => [internal] load .dockerignore                                                                                                                                                                 0.0s
 => => transferring context: 2B                                                                                                                                                                   0.0s
 => [internal] load metadata for docker.io/library/ubuntu:focal                                                                                                                                   0.0s
 => [base 1/9] FROM docker.io/library/ubuntu:focal                                                                                                                                                0.0s
 => [internal] load build context                                                                                                                                                                 0.0s
 => => transferring context: 1.17kB                                                                                                                                                               0.0s
 => CACHED [base 2/9] RUN apt-get update && apt-get install -y python3 python3-distutils python3-venv curl patch                                                                                  0.0s
 => CACHED [base 3/9] RUN curl https://bootstrap.pypa.io/get-pip.py | python3 -                                                                                                                   0.0s
 => CACHED [base 4/9] RUN ln -s /usr/bin/python3 /usr/bin/python                                                                                                                                  0.0s
 => CACHED [base 5/9] RUN python3 -V                                                                                                                                                              0.0s
 => CACHED [base 6/9] RUN curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/master/get-poetry.py | python3                                                                         0.0s
 => [base 7/9] COPY pr.patch /root/                                                                                                                                                               0.0s
 => [base 8/9] RUN cd /root/.poetry/lib/poetry/_vendor/py3.8 && patch --verbose --binary -p1 < /root/pr.patch                                                                                     0.3s
 => [base 9/9] WORKDIR /code                                                                                                                                                                      0.0s
 => [step1 1/6] RUN mkdir app && touch app/__init__.py                                                                                                                                            0.3s
 => [step1 2/6] RUN echo '[tool.poetry]\n'          'name = "test_package"\n'          'version = "1.0.0"\n'          'description = ""\n'          'authors = ["bob"]\n'          'packages = [  0.3s
 => [step1 3/6] RUN poetry lock                                                                                                                                                                   2.9s
 => [step1 4/6] RUN cat poetry.lock                                                                                                                                                               0.4s
 => [step1 5/6] RUN sed -i -E 's/sha256:[0-9a-f]{64}/sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef/g' poetry.lock                                                       0.4s
 => [step1 6/6] RUN cat poetry.lock                                                                                                                                                               0.3s
 => [step2 1/4] COPY --from=step1 /code ./                                                                                                                                                        0.1s
 => [step2 2/4] RUN find .                                                                                                                                                                        0.3s
 => [step2 3/4] RUN cat poetry.lock                                                                                                                                                               0.3s
 => ERROR [step2 4/4] RUN poetry install                                                                                                                                                          2.5s
------
 > [step2 4/4] RUN poetry install:
#23 1.439 Creating virtualenv test-package-MATOk_fk-py3.8 in /root/.cache/pypoetry/virtualenvs
#23 1.982 Installing dependencies from lock file
#23 2.025
#23 2.025 Package operations: 1 install, 0 updates, 0 removals
#23 2.025
#23 2.026   • Installing chardet (3.0.4)
#23 2.236
#23 2.236   RuntimeError
#23 2.236
#23 2.236   Unable to find installation candidates for chardet (3.0.4)
#23 2.236
#23 2.236   at ~/.poetry/lib/poetry/installation/chooser.py:72 in choose_for
#23 2.250        68│
#23 2.250        69│             links.append(link)
#23 2.250        70│
#23 2.250        71│         if not links:
#23 2.251     →  72│             raise RuntimeError(
#23 2.251        73│                 "Unable to find installation candidates for {}".format(package)
#23 2.251        74│             )
#23 2.252        75│
#23 2.252        76│         # Get the best link
#23 2.252
------
executor failed running [/bin/sh -c poetry install]: exit code: 1

...however, as you can see, it does not display anything actionable in the resulting error message. So some additional code would be desirable here.

@finswimmer finswimmer requested a review from a team April 6, 2021 04:13
Comment on lines -66 to +67
def hash(self):
h = hashlib.sha256()
def hash(self, name="sha256"):
h = hashlib.new(name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the docs say the named constructors are faster and should be preferred, and this doesn't do anything to address the issue. It can be safely dropped from the PR.

@@ -406,6 +406,7 @@ def clone(self): # type: () -> "Package"
clone.extras = self.extras
clone.root_dir = self.root_dir
clone.develop = self.develop
clone.files = self.files
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be the "secret sauce" to get signature verification working.

@0xDEC0DE
Copy link
Contributor

0xDEC0DE commented Apr 7, 2021

I have an alternate approach to this, see #159

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.

poetry.lock hashes not checked when running poetry install
2 participants