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

Verify the js-yaml package integrity #700

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

LaurentGoderre
Copy link
Member

No description provided.

Copy link

@whalelines whalelines left a comment

Choose a reason for hiding this comment

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

A couple suggestions

Dockerfile-linux.template Outdated Show resolved Hide resolved
Dockerfile-linux.template Outdated Show resolved Hide resolved
Copy link

@whalelines whalelines left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

I don't love the added /dist/ directory, but as noted in #700 (comment), the alternatives are probably very slightly worse (IMO) 👍

There is a minor nit with the checksum though that I think we should move up next to the JSYAML_VERSION variable since when one changes they both have to. If that checksum is published on a webpage somewhere, it'd also be helpful to make sure we're linking to that in a comment next to it (so that it's easier to figure out where it comes from if/when we update it).

Dockerfile-linux.template Outdated Show resolved Hide resolved
Dockerfile-linux.template Outdated Show resolved Hide resolved
@LaurentGoderre
Copy link
Member Author

LaurentGoderre commented Jul 6, 2024

I don't love the added dist either but couldn't get rid of it with the extract command alone. If I use sttip-components 2, the package.json doesn't get extracted

@LaurentGoderre
Copy link
Member Author

@tianon ok to merge?

Dockerfile-linux.template Outdated Show resolved Hide resolved
Copy link
Member

@yosifkit yosifkit left a comment

Choose a reason for hiding this comment

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

Just the leftover/outdated comment, but otherwise it looks good to me.

@yosifkit yosifkit requested a review from tianon October 3, 2024 19:11
@yosifkit
Copy link
Member

yosifkit commented Oct 3, 2024

On a side note, https://registry.npmjs.org/js-yaml has some embedded signatures that we may want to verify in the future:

            "dist": {
                "integrity": "sha512-YfbcO7jXDdyj0DGxYVSlSeQNHbD7XPWvrVWeVUujrQEoZzWJIRrCPoyk6kL6IAjAG2IolMK4T0hNUe0HOUs5Jw==",
                "shasum": "aff151b30bfdfa8e49e05da22e7415e9dfa37847",
                "tarball": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.13.1.tgz",
                "fileCount": 37,
                "unpackedSize": 283005,
                "npm-signature": "-----BEGIN PGP SIGNATURE-----\r\nVersion: OpenPGP.js v3.0.4\r\nComment: https://openpgpjs.org\r\n\r\nwsFcBAEBCAAQBQJcp38bCRA9TVsSAnZWagAAhE0P/0hyn9bXVGMTE7DJSmeU\nxZMBBwOP6Ofyr6597JzDE/mpqUDnqRTbArd4Pf1FoM38YDIdoFcvSayCtyaA\nqTodzwt6UIw0ljoI7VnpiSq4S8Gbo6vKeEpbv4Gh6Jc9YHmNIJLfR/iNJ/Ti\np4oAfBogNdWhP6Oqb6Sp3OyF2IOvr7+q90IlkvfDk+pm3ZVF744CjZ0/6rZO\nX5UOWWQsF7bpZDsBV9MAQ+9ifHmHTSbDKsBFxOhnxSvZMSWgJnY8JHHbusk0\nWxCE+DJ9C9C7w+Li22inlNd7y+VNRzK2K3H7pJ0A+fSZp0sWePUuj+ObUarT\nJJU7HbkQT/VyyZdlaprl1GTSWEQdyxvT82fXgqXD3H6X1v5o9kSuKyJaNo8k\nDOFAYQhHBVfY2zMaT+S34uf1hKC9iQJq7kvQ65h/9DYlpcLkSCYhlaANx3V+\nuXdQHLlSRnZHgOaS8HvwVWsA5ZeFjS8xA58d671Li3LsXStZV0x0fgxDJYR0\nUU32uvth2/nCwLaxzPWDFA98FEpOU8/C5ywRk4JIJ1nGbP1qz7XtBwFnL6h4\nmewcAO1JgavPjkMe6qPmiw5jlZd0ebGCRdiaKGoPq1GXRnl+ctOeRSvyfUPf\nx2UNiVHRjzDb7PFpkW7KogNOkmN2TEIrJa3GKAT+GlLn8W0+lkUk/qCUeR5N\nIDd6\r\n=IAWU\r\n-----END PGP SIGNATURE-----\r\n",
                "signatures": [
                    {
                        "keyid": "SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA",
                        "sig": "MEQCICcyMFOpLvWhCJ6rfrqOhJurrDbHHY44FcmJBW04GRpUAiBVXUy35hdps/0FXT9SX3X6rsqOGLU6EbAm2HmGr4PRsw=="
                    }
                ]
            },

@LaurentGoderre LaurentGoderre merged commit 6b5b166 into docker-library:master Oct 3, 2024
17 checks passed
@LaurentGoderre LaurentGoderre deleted the check-js-yaml branch October 3, 2024 20:33
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Oct 3, 2024
Changes:

- docker-library/mongo@6b5b166: Merge pull request docker-library/mongo#700 from LaurentGoderre/check-js-yaml
- docker-library/mongo@1e10aea: Verify the js-yaml package integrity
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Oct 9, 2024
Changes:

- docker-library/mongo@625c85c: Update 7.0-rc to 7.0.15-rc1
- docker-library/mongo@6b5b166: Merge pull request docker-library/mongo#700 from LaurentGoderre/check-js-yaml
- docker-library/mongo@1e10aea: Verify the js-yaml package integrity
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Oct 9, 2024
Changes:

- docker-library/mongo@a47dd91: Update 8.0-rc
- docker-library/mongo@c7174de: Update 8.0 to 8.0.1
- docker-library/mongo@625c85c: Update 7.0-rc to 7.0.15-rc1
- docker-library/mongo@6b5b166: Merge pull request docker-library/mongo#700 from LaurentGoderre/check-js-yaml
- docker-library/mongo@1e10aea: Verify the js-yaml package integrity
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.

4 participants