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

[BUG] package-lock.json integrity value for git dependencies depends on the architecture (Apple Silicon M1 differs) #2846

Closed
feross opened this issue Mar 10, 2021 · 26 comments · Fixed by npm/pacote#71
Assignees
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release

Comments

@feross
Copy link

feross commented Mar 10, 2021

Current Behavior:

The package-lock.json integrity value seems to depend on the OS/architecture. Take the following git dependency which specifies a commit hash:

npm pack "git+ssh://git@github.com/jhiesey/idb-kv-store.git#109ccad165fd6470e12fd66025da9e4743a46043"

The integrity value produced is different on these OSes/architectures:

  • Ubuntu 20.04, and macOS 11.2.3 (Intel): sha512-DnBTbDDxd9/9mwPehyraeuRTbNEqbWLcAdE3GC1trdBWWwKnkWsaU/X6mVLIKKB/IYWmG+cnL3ihg/Ql/rW5kg==
  • macOS 11.2.3 (Apple Silicon): sha512-T3ZWOM1TT+Ch/splApkEe1HwktWs+n/iHvDvtIGEI+4xuMGHite6mMujuNd8sen49ofLP/PxzprQMSPJK8APww==

Expected Behavior:

The integrity value should not be different on Apple Silicon (M1 chip) machines.

Steps To Reproduce:

Run npm pack "git+ssh://git@github.com/jhiesey/idb-kv-store.git#109ccad165fd6470e12fd66025da9e4743a46043" and inspect the integrity value from an M1 Mac. Node.js was installed from Homebrew using brew install node and the amd64 version was installed.

Also... @jhiesey and I dug into this a bit and found that the tarballs fetched from the GitHub CDN are exactly the same on M1 and other architectures, byte-for-byte. Same for the ungzipped tarballs – they are the same byte-for-byte. What differs, though, is the gzipped tarballs (.tar.gz) files. Those appear to have substantial differences when viewed in a hex editor.

Environment:

  • OS: Various, see above
  • Node: v15.11.0
  • npm: 7.6.2
@feross feross added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Mar 10, 2021
@feross
Copy link
Author

feross commented Mar 10, 2021

I've included both the Intel and M1 gzipped tarballs here, if that's helpful: Archive.zip

@reggi
Copy link
Contributor

reggi commented Mar 10, 2021

I have both M1 and Intel mac, let me know if I can be of help.

Here's the intel response:

Thomass-MacBook-Pro-2:~ thomas$ npm pack "git+ssh://git@github.com/jhiesey/idb-kv-store.git#109ccad165fd6470e12fd66025da9e4743a46043"
npm notice 
npm notice 📦  idb-kv-store@4.5.0
npm notice === Tarball Contents === 
npm notice 1.1kB  LICENSE          
npm notice 16.5kB idbkvstore.min.js
npm notice 14.5kB index.js         
npm notice 2.5kB  karma.conf.js    
npm notice 17.7kB test.js          
npm notice 1.2kB  package.json     
npm notice 10.4kB README.md        
npm notice 1.5kB  .travis.yml      
npm notice === Tarball Details === 
npm notice name:          idb-kv-store                            
npm notice version:       4.5.0                                   
npm notice filename:      idb-kv-store-4.5.0.tgz                  
npm notice package size:  16.5 kB                                 
npm notice unpacked size: 65.4 kB                                 
npm notice shasum:        f760c3e1f1ca0959863df9e56a0609cebdc941a3
npm notice integrity:     sha512-xAVBTqp3mAtgt[...]B/PjnXc8eSTBg==
npm notice total files:   8                                       
npm notice 

Here's the M1

thomasreggi@Thomass-MacBook-Air ~ % npm pack "git+ssh://git@github.com/jhiesey/idb-kv-store.git#109ccad165fd6470e12fd66025da9e4743a46043"
npm notice 
npm notice 📦  idb-kv-store@4.5.0
npm notice === Tarball Contents === 
npm notice 1.1kB  LICENSE          
npm notice 10.4kB README.md        
npm notice 1.5kB  .travis.yml      
npm notice 16.5kB idbkvstore.min.js
npm notice 14.5kB index.js         
npm notice 2.5kB  karma.conf.js    
npm notice 1.2kB  package.json     
npm notice 17.7kB test.js          
npm notice === Tarball Details === 
npm notice name:          idb-kv-store                            
npm notice version:       4.5.0                                   
npm notice filename:      idb-kv-store-4.5.0.tgz                  
npm notice package size:  16.5 kB                                 
npm notice unpacked size: 65.4 kB                                 
npm notice shasum:        bfbf05231b88deb3ec0ca8e9bf6ec02bbb96ac7d
npm notice integrity:     sha512-T3ZWOM1TT+Ch/[...]prQMSPJK8APww==
npm notice total files:   8                                       
npm notice 

@Myriachan
Copy link

A friend linked me this, and I just wanted to point out that relying on compressed data to match on all machines given the same uncompressed input is rife with problems. Implementations of Zlib exist that use hardware acceleration instructions, and they do not take care to ensure that each accelerated version will come up with the same compression as every other implementation.

As an example, there is a Zlib implementation that uses x86 SSE4.2 instructions to accelerate pattern matching, and ARM doesn't have anything like that, so this is an x86-specific code path. It does not find the same exact dictionary matches, so won't come up with the same results as the pure C implementation.

@feross
Copy link
Author

feross commented Mar 10, 2021

A friend linked me this, and I just wanted to point out that relying on compressed data to match on all machines given the same uncompressed input is rife with problems

I also felt like this design decision might have this issue, even with the portable: true option that is passed to minizlib. I wonder why they're not instead using the .tar file for integrity purposes?

@darcyclarke darcyclarke added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels Mar 12, 2021
@darcyclarke darcyclarke added this to the OSS - Sprint 26 milestone Mar 12, 2021
@darcyclarke
Copy link
Contributor

@feross thanks for filing this - we'll take a look right away (apologized it wasn't triaged quicker)

@jaydenseric
Copy link

This bug had absolutely stumped me since december 2020, regarding this dependency tree:

npm ls bls12377js
[redacted]@ [redacted]
└─┬ bitgo@11.11.0
  └─┬ @bitgo/account-lib@2.8.0
    └─┬ @celo/contractkit@0.4.11
      └─┬ @celo/utils@0.1.17
        └── bls12377js@0.1.0 (git+ssh://git@github.com/celo-org/bls12377js.git#cb38a4cfb643c778619d79b20ca3e5283a2122a6)

I created the package-lock.json file locally, and when attempting to push it to Heroku it would result in the following deployment build error (log excerpt):

-----> Installing dependencies
       Installing node modules
       npm ERR! code EINTEGRITY
       npm ERR! sha512-7EgC8H/EFwDko7LjlkOQslnckOdsknH8zVPQ2s/4Gya3jXGvxFLqNhTFOiHkaz1mSxbC7dhEK4LbHJEjjBsfIA== integrity checksum failed when using sha512: wanted sha512-7EgC8H/EFwDko7LjlkOQslnckOdsknH8zVPQ2s/4Gya3jXGvxFLqNhTFOiHkaz1mSxbC7dhEK4LbHJEjjBsfIA== but got sha512-oUocNv3wT2CYJys8ZjPK5TfynQ81T7IfAmDn4ZmZIA7Hv/RdiD9HeDvVgTUUfUx8udJmYYh57A78zqdhKe6x7Q==. (47748 bytes)

Heroku support and I gave up trying to figure out the problem and for that GraphQL API I ended up downgrading to npm v6.

Running with Node.js v15.12.0 and npm v7.6.3:

npm pack "https://github.com/celo-org/bls12377js#cb38a4cfb643c778619d79b20ca3e5283a2122a6"

In macOS v11.2.1 (Intel chip):

npm notice === Tarball Details === 
npm notice name:          bls12377js                              
npm notice version:       0.1.0                                   
npm notice filename:      bls12377js-0.1.0.tgz                    
npm notice package size:  47.7 kB                                 
npm notice unpacked size: 144.3 kB                                
npm notice shasum:        13e4f2f388d4f8f28e0fa45a8ceaef4ca7514789
npm notice integrity:     sha512-7EgC8H/EFwDko[...]4LbHJEjjBsfIA==
npm notice total files:   33 

In Ubuntu 20.04.2 LTS (Heroku, not sure what chip):

npm notice === Tarball Details === 
npm notice name:          bls12377js                              
npm notice version:       0.1.0                                   
npm notice filename:      bls12377js-0.1.0.tgz                    
npm notice package size:  47.7 kB                                 
npm notice unpacked size: 144.3 kB                                
npm notice shasum:        616f7b4f14365eff44c82539994634ba94d33911
npm notice integrity:     sha512-oUocNv3wT2CYJ[...]A78zqdhKe6x7Q==
npm notice total files:   33

At first I noticed the file permissions were created differently for the dependency installed locally (drwxr-xr-x/755) vs installed in Heroku (drwx------/700), but it turns out that doesn't have an affect on npm pack shasum. I verified the contents of literally every file and they are identical locally and in Heroku. The npm shasums are literally platform dependant / incorrect, as per this issue raised 😕

This is a critical, show-stopping bug; how long until it's fixed in a stable npm release? Is there a workaround in the meantime, that does not involve downgrading npm v7 to npm v6? One gross possibility is not committing a lockfile and let Heroku generate one each deployment; the checksums can be whatever that way, since they're not compared against anything. Obviously that would have undesirable stability and security implications.

@nlf
Copy link
Contributor

nlf commented Mar 22, 2021

i absolutely agree the root issue here is generating and comparing integrity hashes against a gzipped result.

this patch to pacote forces the compression level to its highest setting, which on the OS, platform, and node versions i tested against (listed in the linked PR) did give me consistent results. it's definitely not "this is fixed forever" quality, but i'm hopeful it's enough to unblock folks while we determine a better approach.

@jaydenseric
Copy link

@nlf or @isaacs please reopen this issue; it has not been fixed:

-----> Prebuild
       Running heroku-prebuild
       
       > heroku-prebuild
       > node -v && npm -v
       
       v15.12.0
       7.7.5
       
-----> Installing dependencies
       Installing node modules
       npm ERR! code EINTEGRITY
       npm ERR! sha512-genS+AxHQKkrKA2xa1C1HCzSlZM8S/ZQEnjqET9u78/fpbHrNbXWvFYypYA3h7ukTD+WpF0hh+S9yxH6mhx4iA== integrity checksum failed when using sha512: wanted sha512-genS+AxHQKkrKA2xa1C1HCzSlZM8S/ZQEnjqET9u78/fpbHrNbXWvFYypYA3h7ukTD+WpF0hh+S9yxH6mhx4iA== but got sha512-k6ubeZJ0g6yz22IiZiC9lIv++VThPyORMQrsRTQ6ChkJpbdXfeWMJCAUUX+12fbmkKZPL5yVQgWpwnC2T0v6CQ==. (47473 bytes)
       
       npm ERR! A complete log of this run can be found in:
       npm ERR!     /tmp/npmcache.dnSao/_logs/2021-03-27T05_08_41_644Z-debug.log
-----> Build failed

@ghost
Copy link

ghost commented Jun 9, 2021

Had this issue when doing "npm ci" in a github workflow environnement, "npm update" right before "npm ci" did the trick for me

PS: dependabot was creating different a shasum for a npm package (github private repository)

jpolitz added a commit to brownplt/code.pyret.org that referenced this issue Jul 15, 2021
@saghul
Copy link

saghul commented Oct 14, 2021

Can we get this reopened please? It's clearly not fixed: npm/pacote#76

@scottohara
Copy link

Getting this issue with node@16.9.1 (arm64) & npm@7.23.0 when pushing to Heroku.

The problematic entry in package-lock.json is (a git dependency):

  "tether": {
   "version": "git+ssh://git@github.com/atlassian/tether.git#bf85430889b5231fbe5b383416cce6281225bf06",
   "integrity": "sha512-/LwlT+VkWwl2WLu8PcceylDVJ+RlqnTVM6s3vMphdNysTvJOpWgJjA7HKM2ZaXiyWYgJvECUGiMuEhmJlkw8FA==",
   "from": "tether@github:atlassian/tether#amd-with-global"
  },

The package-lock.json file was generated on my M1 Mac. When pushed to Heroku I get this:

remote: Compressing source files... done.
remote: Building source:
remote: 
remote: -----> Building on the Heroku-20 stack
remote: -----> Deleting 9 files matching .slugignore patterns.
remote: -----> Using buildpack: heroku/nodejs
remote: -----> Node.js app detected
remote:        
remote: -----> Creating runtime environment
remote:        
remote:        NPM_CONFIG_LOGLEVEL=error
remote:        NODE_VERBOSE=false
remote:        NODE_ENV=production
remote:        NODE_MODULES_CACHE=true
remote:        
remote: -----> Installing binaries
remote:        engines.node (package.json):  16.9.1
remote:        engines.npm (package.json):   7.23.0
remote:        
remote:        Resolving node version 16.9.1...
remote:        Downloading and installing node 16.9.1...
remote:        Bootstrapping npm 7.23.0 (replacing 7.21.1)...
remote:        npm 7.23.0 installed
remote:        
remote: -----> Restoring cache
remote:        Cached directories were not restored due to a change in version of node, npm, yarn or stack
remote:        Module installation may take longer for this build
remote:        
remote: -----> Installing dependencies
remote:        Installing node modules
remote:        npm ERR! code EINTEGRITY
remote:        npm ERR! sha512-/LwlT+VkWwl2WLu8PcceylDVJ+RlqnTVM6s3vMphdNysTvJOpWgJjA7HKM2ZaXiyWYgJvECUGiMuEhmJlkw8FA== integrity checksum failed when using sha512: wanted sha512-/LwlT+VkWwl2WLu8PcceylDVJ+RlqnTVM6s3vMphdNysTvJOpWgJjA7HKM2ZaXiyWYgJvECUGiMuEhmJlkw8FA== but got sha512-pRnmgBfj2qUKMXOCmjUBmYMNER/uxl9fF9Vg9HCRqYW3b1wvA8/8hhQXevSpTQ0jdNOtfphY3Es9GENIyTAzNw==. (291036 bytes)
remote:        
remote:        npm ERR! A complete log of this run can be found in:
remote:        npm ERR!     /tmp/npmcache.xkZUB/_logs/2021-10-18T01_04_55_513Z-debug.log
remote: 
remote: -----> Build failed

Previously I was running the x86_64 build of node (still on my M1 Mac, but running under Rosetta2); and it did not get this error (though that was approximately 4 weeks ago).

Today is the first time pushing to Heroku since switching to the native arm64 build of node.

@isaacs
Copy link
Contributor

isaacs commented Oct 19, 2021

We've talked about this internally, and I think the consensus is that we really should not be tracking integrity for any dependency that gets built on the client, especially git dependencies that already benefit from git's comprehensive content hashing. (cc: @nlf)

@isaacs isaacs reopened this Oct 19, 2021
@saghul
Copy link

saghul commented Oct 20, 2021

We've talked about this internally, and I think the consensus is that we really should not be tracking integrity for any dependency that gets built on the client, especially git dependencies that already benefit from git's comprehensive content hashing. (cc: @nlf)

Sounds reasonable!

@tuomassalo
Copy link

tuomassalo commented Oct 25, 2021

Before an actual fix is available, here's a hacky oneliner that attempts to remove the checksums for git dependencies from package-lock.json. Yes, it makes some bold assumptions, such as relying on the integrity line to immediately follow a resolved/version line, but it does help me for now.) EDIT: see comment below for possible problems.

perl -pi -e '$_="" if /"integrity":/ and $p=~/"(resolved|version)": "git\+ssh:/; $p=$_' package-lock.json

A brave soul could run a proper version of this as a pre-commit hook.

@nlf
Copy link
Contributor

nlf commented Mar 1, 2022

as of npm@8.5.2 we no longer compare the hash of git dependencies against an integrity string found in a package-lock.json. newly installed/updated git dependencies should not store an integrity field, though existing integrity strings may remain behind. if one of these is found, npm will log a warning telling you that the integrity value for the git repo was ignored. if you like, you can hand edit your package-lock.json and remove the offending line.

@nlf nlf closed this as completed Mar 1, 2022
jbraese added a commit to sitcomlab/simport-learning-app that referenced this issue Mar 16, 2022
jaltekruse referenced this issue in jaltekruse/Free-Math Mar 23, 2022
jaltekruse added a commit to jaltekruse/Free-Math that referenced this issue Mar 23, 2022
It appears that different machines can produce different hashes for
git-based dependencies, so the npm team moved to completely remove
integrity checksums for them. Apparently these checksums were based
on gzipped archives, which are not guaranteed to be binary identical
for the same inputs across different CPU architectures. There is
still some cryptographic integrity defense as the dependency is
pinned to a git commit and that relies on the entire previous history
of the repo, as discussed in the later parts of this issue on npm.

npm/cli#2846
jaltekruse added a commit to jaltekruse/toast-ui.react-image-editor that referenced this issue Apr 14, 2022
It appears that different machines can produce different hashes for
git-based dependencies, so the npm team moved to completely remove
integrity checksums for them. Apparently these checksums were based
on gzipped archives, which are not guaranteed to be binary identical
for the same inputs across different CPU architectures. There is
still some cryptographic integrity defense as the dependency is
pinned to a git commit and that relies on the entire previous history
of the repo, as discussed in the later parts of this issue on npm.

npm/cli#2846
jaltekruse added a commit to jaltekruse/Free-Math that referenced this issue Apr 14, 2022
…tion SHA integrity hashes for the downstream depedency

Toast UI is built without React but we are depending on a wrapper
published by the devlopers of the library. Both the underlying project
and the react wrapper are customized.

Removing the integrity SHAs because of this issue in NPM.

npm/cli#2846
@luxaritas
Copy link

luxaritas commented Jun 13, 2022

newly installed/updated git dependencies should not store an integrity field, though existing integrity strings may remain behind.

@nlf Running npm 8.12.1, running npm install with an updated github: dependency resolved to git+ssh: still adds the integrity field when it is not originally present. Is this a bug, or is there some other reason for this?

@jayaddison
Copy link

@luxaritas I could be mistaken, but I think that although the NPM CLI v8.5.2 no longer verifies the integrity of packages installed from git references, it does still calculate and update a local integrity value (potentially with architecture-specific results, as noted in this issue).

@pbasista
Copy link

pbasista commented Sep 19, 2022

The change of behavior mentioned here states that:

as of npm@8.5.2 we no longer compare the hash of git dependencies against an integrity string

It seems though that npm behaves differently when installing packages locally from a tarball produced by npm pack. From what I have tried, npm@8.19.2 still verifies their integrity and refuses to continue if there is a mismatch:

npm WARN tarball tarball data for package@file:/path/to/local/package/package-0.0.1.tgz (sha512-j6GmhvvhuoKF5XJvyqPSFHE6q/b7wmXt9c+RZnKVq1EcabsREC3DAgm0Gyv55/Jumrxv4pOCI1jZqqmFOCkY4w==) seems to be corrupted. Trying again.
...
npm ERR! code EINTEGRITY
npm ERR! sha512-j6GmhvvhuoKF5XJvyqPSFHE6q/b7wmXt9c+RZnKVq1EcabsREC3DAgm0Gyv55/Jumrxv4pOCI1jZqqmFOCkY4w== integrity checksum failed when using sha512: wanted sha512-j6GmhvvhuoKF5XJvyqPSFHE6q/b7wmXt9c+RZnKVq1EcabsREC3DAgm0Gyv55/Jumrxv4pOCI1jZqqmFOCkY4w== but got sha512-QW67wfGbWXDP5EiKac+sa6/UzAnJ1LS4K0hPdKwjoj85asVtpW+Qk7F5ZSDAw5HcA8/tZAWJSPEvpBpnM6szlA==. (334966 bytes)

The local integrity checksums are made from the compressed tarballs which might be architecture-specific, as was mentioned here.

kainino0x added a commit to kainino0x/cts that referenced this issue Mar 2, 2023
Prior to this version, the @webgpu/types dependency-by-revision won't
work because the integrity hash is system-dependent. Starting with
8.5.2, integrity checks are skipped for git dependencies (though
integrity hashes are still generated, despite the comment in the
thread). See:
npm/cli#2846 (comment)
kainino0x added a commit to gpuweb/cts that referenced this issue Mar 2, 2023
Prior to this version, the @webgpu/types dependency-by-revision won't
work because the integrity hash is system-dependent. Starting with
8.5.2, integrity checks are skipped for git dependencies (though
integrity hashes are still generated, despite the comment in the
thread). See:
npm/cli#2846 (comment)
@viluon
Copy link

viluon commented Apr 30, 2023

This is indeed a deeper problem with npm pack, treating Git dependencies differently is a workaround, not a fix.

ErichDonGubler pushed a commit to mozilla/gpuweb-cts that referenced this issue Aug 16, 2023
Prior to this version, the @webgpu/types dependency-by-revision won't
work because the integrity hash is system-dependent. Starting with
8.5.2, integrity checks are skipped for git dependencies (though
integrity hashes are still generated, despite the comment in the
thread). See:
npm/cli#2846 (comment)
@boutell
Copy link

boutell commented May 7, 2024

As @pbasista and @viluon say, this is not fixed for npm pack tarballs installed via file: dependencies. Is a preinstall hook to strip the integrity values the only workaround?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release
Projects
None yet