Use requirements.txt as lockfile #30

wuan · 2023-05-11T10:46:27Z

The basic requirement is contained in requirements.in. requirements.txt can be updated by using pip-compile from the pip-tools package.

meejah · 2023-05-11T16:11:39Z

mailbox/requirements.in

@@ -0,0 +1 @@
+magic-wormhole-mailbox-server -e git+https://github.com/magic-wormhole/magic-wormhole-mailbox-server/tarball/4b358859ba80de37c3dc0a5f67ec36909fd48234#egg=magic-wormhole-mailbox-server


I think these github hashes (of magically-produced tarballs) are not stable...?

That hash is the commit hash from the repo:

magic-wormhole/magic-wormhole-mailbox-server@4b35885

Pretty stable, you can also use a tag (but this is less stable).

Please note that the source URL did not change with this PR, we already used it before.

Maybe it's a different thing, but there was some issue with using GitHub links (e.g. from the "Download" button, approximately). Maybe it was only for ZIPs?

Had a chat with @hacklschorsch yesterday and he mentioned that the hashes from the archive have been differing in some cases. Here we use a Git hash to fix the source and we do not care about different archive hashes as long as they contain exactly the files which are from that Git hash we use.

Right, okay that was it -- that the bits can change (so if you have a workflow that checks those, it'll fail -- but only "somtimes" e.g. when GitHub nukes a cache or something?)

Can't we use the git hash to access the archive (w/o relying on git)?
Are you suggesting there could be some differences between sources cloned using Git CLI and those "mirrored" with plain HTTP(s)?

I was under the impression that tar and zip archives were part of Git itself (e.g.: git archive)!

It still seems to work as expected with https only. At least for our purpose.
Pip freeze gives me the same result in both cases:

diff <(docker run --rm -t --entrypoint pip fab306454663 freeze) <(docker run --rm -t --entrypoint pip 64a7dd3f2103 freeze) 13c13 < magic-wormhole @ https://github.com/magic-wormhole/magic-wormhole/tarball/f66e759b2494d6713238d86eb0e8001f297ef553 --- > magic-wormhole @ git+https://github.com/magic-wormhole/magic-wormhole@f66e759b2494d6713238d86eb0e8001f297ef553

Only the second image is now 83MB bigger (not much, but still 50% increase!) and present a number of extra packages that could introduce more vulnerabilities:

19a20,21 > git-man/now 1:2.30.2-1+deb11u2 all [installed,local] > git/now 1:2.30.2-1+deb11u2 amd64 [installed,local] 24a27 > less/now 551-2 amd64 [installed,local] 30a34,35 > libbrotli1/now 1.0.9-2+b2 amd64 [installed,local] > libbsd0/now 0.11.3-1 amd64 [installed,local] 34a40 > libcbor0/now 0.5.0+dfsg-2 amd64 [installed,local] 36a43 > libcurl3-gnutls/now 7.74.0-1.3+deb11u7 amd64 [installed,local] 38a46,47 > libedit2/now 3.1-20191231-2+b1 amd64 [installed,local] > liberror-perl/now 0.17029-1 all [installed,local] 41a51 > libfido2-1/now 1.6.0-2 amd64 [installed,local] 43a54 > libgdbm-compat4/now 1.19-2 amd64 [installed,local] 54a66,67 > libldap-2.4-2/now 2.4.57+dfsg-3+deb11u1 amd64 [installed,local] > libldap-common/now 2.4.57+dfsg-3+deb11u1 all [installed,local] 56a70 > libmd0/now 1.0.3-3 amd64 [installed,local] 59a74 > libnghttp2-14/now 1.43.0-1 amd64 [installed,local] 67a83,84 > libperl5.32/now 5.32.1-4+deb11u2 amd64 [installed,local] > libpsl5/now 0.21.0-1.2 amd64 [installed,local] 68a86,89 > librtmp1/now 2.4+20151223.gitfa8646d.1-2+b2 amd64 [installed,local] > libsasl2-2/now 2.1.27+dfsg-2.1+deb11u1 amd64 [installed,local] > libsasl2-modules-db/now 2.1.27+dfsg-2.1+deb11u1 amd64 [installed,local] > libsasl2-modules/now 2.1.27+dfsg-2.1+deb11u1 amd64 [installed,local] 76a98 > libssh2-1/now 1.9.0-2 amd64 [installed,local] 86a109,115 > libx11-6/now 2:1.7.2-1 amd64 [installed,local] > libx11-data/now 2:1.7.2-1 all [installed,local] > libxau6/now 1:1.0.9-1 amd64 [installed,local] > libxcb1/now 1.14-3 amd64 [installed,local] > libxdmcp6/now 1:1.1.2-3 amd64 [installed,local] > libxext6/now 2:1.3.3-1.1 amd64 [installed,local] > libxmuu1/now 2:1.1.2-2+b3 amd64 [installed,local] 96a126 > openssh-client/now 1:8.4p1-5+deb11u1 amd64 [installed,local] 98a129 > patch/now 2.7.6-7 amd64 [installed,local] 99a131,133

Is this worth it? (I'm interested to know more about the risk you guys are talking about - also with @hacklschorsch)

I was under the impression that tar and zip archives were part of Git itself (e.g.: git archive)!

I believe that's correct, but the output isn't stable. So if you have a process that checks hashes, it'll fail sometimes. (Certainly there are ways besides "use the github API" to get the sources of course, I was just highlighting that that particular method has a potential problem).

I believe this discusses the problem: https://github.blog/2023-02-21-update-on-the-future-stability-of-source-code-archives-and-hashes/

I don't believe that pip freeze incantation is doing anything to check the contents...? (e.g. like --require-hahes in pip might)

meejah · 2023-05-11T16:13:51Z

It seems to me that overall pinned requirements would best come from upstream, as is common in lots of other Python command-line application (e.g. magic-folder) and ideally come with hashes as well.

wuan · 2023-05-11T19:13:30Z

It seems to me that overall pinned requirements would best come from upstream, as is common in lots of other Python command-line application (e.g. magic-folder) and ideally come with hashes as well.

Projects usually allow ranges of versions in order to work together with other dependencies, for example see magic-wormhole-mailbox-server/setup.py Line 33:

      install_requires=[
          "attrs >= 16.3.0", # 16.3.0 adds __attrs_post_init__
          "twisted[tls] >= 17.5.0",
          "autobahn[twisted] >= 0.14.1",
      ],

That is the reason why we lock the dependency versions here in order to have a better reproducibility.

meejah · 2023-05-11T19:36:40Z

The recommended approach is for libraries to be open-ended, but for programs to be way more specific (ideally, locking to precise versions). The latter is what e.g. magic-folder does: https://github.com/LeastAuthority/magic-folder/blob/main/requirements/base.txt

meejah · 2023-05-11T19:37:24Z

(I suspect the setup.py stuff is basically just "leftover" from before the 3-way split...? Then, it did make sense to be open-ended because part of the repo was 'a library')

meejah · 2023-05-11T19:38:32Z

That is the reason why we lock the dependency versions here in order to have a better reproducibility.

Yeah, what I'm suggesting is that it could be better to put that effort upstream, since it makes sense to lock them there too. (That is, because it's "a program" and not "a library").

wuan · 2023-05-11T20:55:03Z

Yeah, what I'm suggesting is that it could be better to put that effort upstream, since it makes sense to lock them there too. (That is, because it's "a program" and not "a library").

Then just open an issue upstream, this topic needs to be discussed there and reference this repo.

meejah · 2023-05-11T23:39:29Z

Sure, please open a ticket -- I don't know your requirements nor what discussion you believe needs to happen?

wuan · 2023-05-12T05:39:17Z

Sure, please open a ticket -- I don't know your requirements nor what discussion you believe needs to happen?

You had the idea to do this, there are no requirements outside of what we were talking about. So just go ahead.

btlogy

This seem to improve the status quo: now the dependencies are pinned and can be easily parsed (hopefully by dependabot and/or other scan tools).

Thank you for fixing this.

btlogy · 2023-05-15T15:52:28Z

As for the upstream dependencies not (yet) been pinned, it could be improved (I can open an issue if that would help).
But this change makes our image more reproducible regardless.
If upstream would pin those deps later, we might be able to skip the pip-compile I guess.

meejah · 2023-05-15T15:53:42Z

The requirements.txt could still benefit from containing hashes, allowing one to use --require-hashes upon install.

(Also, as I said above, if this effort was directed upstream in such a way that releases were done like this, more people would benefit and the actual upstream authors would hopefully take a look at the dependencies themselves)

wuan · 2023-05-15T16:00:57Z

Used hashes with the Git repo which led to errors like

ERROR: Can't verify hashes for these requirements because we don't have a way to hash version control repositories:

We can try again, as we are back to the tarball now.

wuan · 2023-05-15T16:06:23Z

Seems to work. Now we might get errors with archives having a different hash in the future, but that should be easy to fix.

btlogy · 2023-05-15T16:11:28Z

Seems to work. Now we might get errors with archives having a different hash in the future, but that should be easy to fix.

Thank you.

At least, this should bring us a step closer to have an accurate view over the known vulnerabilities in our images.
I'll try to see how we can get there in:

Docker images should be regulary scanned for vulnerabilities in Python modules #37.

* avoid git package - unused since #30 * re-order and document steps * use environment variables as parameter * expose ports Signed-off-by: Benoit Donneaux <[email protected]>

* Improve Dockerfiles * avoid git package - unused since #30 * re-order and document steps * use environment variables as parameter * expose ports * Remove useless apt step --------- Signed-off-by: Benoit Donneaux <[email protected]>

wuan added 2 commits May 11, 2023 12:43

use requirements.txt as lockfile

be7eb61

add files

afde197

meejah reviewed May 11, 2023

View reviewed changes

wuan added 8 commits May 12, 2023 07:54

transform the rest

95ae3e9

copy requirements.txt

3ba4529

add hashes

925b5f9

fix referencing git hashes:

00ef945

update readme

da67504

skip hash for repo source

199c1cf

do not use hashes

6025d0e

install git

e53886a

wuan force-pushed the pin_requirements branch from 75e9d42 to e53886a Compare May 15, 2023 07:56

wuan marked this pull request as ready for review May 15, 2023 09:53

btlogy mentioned this pull request May 15, 2023

Docker images should be regulary scanned for vulnerabilities in Python modules #37

Open

use tarball instead of git

dc2bfb6

btlogy approved these changes May 15, 2023

View reviewed changes

wuan added 2 commits May 15, 2023 18:01

reintroduce hashes

7649ab7

remove comment regarding hashes

ebb430b

wuan merged commit 64a8739 into main May 15, 2023

wuan deleted the pin_requirements branch May 15, 2023 16:07

This was referenced May 16, 2023

Update magic-wormwhole version #25

Closed

Update magic-wormhole image with latest upstream code #23

Closed

meejah mentioned this pull request May 20, 2023

pin dependencies? magic-wormhole/magic-wormhole-mailbox-server#39

Open

btlogy mentioned this pull request May 22, 2023

Unexpected non-reproducibility between images build using the same commit #38

Closed

btlogy added a commit that referenced this pull request May 26, 2023

Improve Dockerfiles

7201808

* avoid git package - unused since #30 * re-order and document steps * use environment variables as parameter * expose ports Signed-off-by: Benoit Donneaux <[email protected]>

btlogy mentioned this pull request May 26, 2023

Improve Dockerfiles #40

Merged

btlogy added a commit that referenced this pull request May 26, 2023

Improve Dockerfiles

c34f07e

* avoid git package - unused since #30 * re-order and document steps * use environment variables as parameter * expose ports Signed-off-by: Benoit Donneaux <[email protected]>

btlogy added a commit that referenced this pull request May 26, 2023

Improve Dockerfiles

d0a29ec

* avoid git package - unused since #30 * re-order and document steps * use environment variables as parameter * expose ports Signed-off-by: Benoit Donneaux <[email protected]>

btlogy mentioned this pull request May 30, 2023

New git tag could push docker tags rather than entirely new images #44

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use requirements.txt as lockfile #30

Use requirements.txt as lockfile #30

wuan commented May 11, 2023 •

edited

Loading

meejah May 11, 2023

wuan May 11, 2023

meejah May 11, 2023

wuan May 12, 2023 •

edited

Loading

meejah May 12, 2023

btlogy May 15, 2023

btlogy May 15, 2023

meejah May 15, 2023

meejah May 15, 2023

meejah commented May 11, 2023

wuan commented May 11, 2023

meejah commented May 11, 2023

meejah commented May 11, 2023

meejah commented May 11, 2023

wuan commented May 11, 2023 •

edited

Loading

meejah commented May 11, 2023

wuan commented May 12, 2023

btlogy left a comment

btlogy commented May 15, 2023

meejah commented May 15, 2023

wuan commented May 15, 2023

wuan commented May 15, 2023

btlogy commented May 15, 2023

		@@ -0,0 +1 @@
		magic-wormhole-mailbox-server -e git+https://github.com/magic-wormhole/magic-wormhole-mailbox-server/tarball/4b358859ba80de37c3dc0a5f67ec36909fd48234#egg=magic-wormhole-mailbox-server

Use requirements.txt as lockfile #30

Use requirements.txt as lockfile #30

Conversation

wuan commented May 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wuan May 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meejah commented May 11, 2023

wuan commented May 11, 2023

meejah commented May 11, 2023

meejah commented May 11, 2023

meejah commented May 11, 2023

wuan commented May 11, 2023 • edited Loading

meejah commented May 11, 2023

wuan commented May 12, 2023

btlogy left a comment

Choose a reason for hiding this comment

btlogy commented May 15, 2023

meejah commented May 15, 2023

wuan commented May 15, 2023

wuan commented May 15, 2023

btlogy commented May 15, 2023

wuan commented May 11, 2023 •

edited

Loading

wuan May 12, 2023 •

edited

Loading

wuan commented May 11, 2023 •

edited

Loading