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

feat: Pass down strict out of sync flag to the lockfile parser #369

Merged
merged 1 commit into from
Feb 18, 2019

Conversation

lili2311
Copy link
Contributor

@lili2311 lili2311 commented Feb 14, 2019

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

Allow users to disable strict out of sync by passing it as a flag, the default lockfile based project testing behaviour is strict mode of CLI. It is problematic for repos that use yarn workspaces or link to deps in the same repo.

Where should the reviewer start?

Tests

How should this be manually tested?

snyk test --strictOutOfSync=false on a project that is out of sync (make it out of sync by manually adding a dep to package.json and not re-locking the deps)

@lili2311 lili2311 self-assigned this Feb 14, 2019
@lili2311 lili2311 force-pushed the feat/out-of-sync-node-flag branch 6 times, most recently from 568322f to c664888 Compare February 15, 2019 18:43
@yuliabaron
Copy link
Contributor

LGTM

@lili2311 lili2311 merged commit 80e1a0c into master Feb 18, 2019
@lili2311 lili2311 deleted the feat/out-of-sync-node-flag branch February 18, 2019 12:52
@snyksec
Copy link

snyksec commented Feb 18, 2019

🎉 This PR is included in version 1.134.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@rylek90
Copy link

rylek90 commented Mar 7, 2019

Hmm, it doesn't seem to work for me 🤔

It happens mainly for our yarn workspace local dependencies but I was able to reproduce it on the simplest project possible, too.

Let's say we have package.json defined as following:

{
  "name": "snyk-sync",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "dependencies": {    
    "snyk": "1.134.0"
  }
}

Then, let's run yarn install && yarn snyk test.

Add any dependency (let's say react) to and re-run yarn snyk test --strictOutOfSync=false.

Dependency react was not found in yarn.lock. Your package.json and yarn.lock are probably out of sync. Please run "yarn install" and try again.
    at /Users/maciekrylko/GIT/snyk-sync/node_modules/snyk/src/cli/commands/test.ts:141:19
    at step (/Users/maciekrylko/GIT/snyk-sync/node_modules/snyk/dist/cli/commands/test.js:32:23)
    at Object.throw (/Users/maciekrylko/GIT/snyk-sync/node_modules/snyk/dist/cli/commands/test.js:13:53)
    at rejected (/Users/maciekrylko/GIT/snyk-sync/node_modules/snyk/dist/cli/commands/test.js:5:65)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Am I doing something wrong?

@lili2311
Copy link
Contributor Author

lili2311 commented Mar 7, 2019

👋 Could you please share your package.json and yarn.lock and send it to support@snyk.io for us to take a look? Also you mentioned it is a yarn workspace, what is the example structure of that as the example above implies you have the lockfile and package.json in the same folder but in a workspace that is normally not the case

@lili2311
Copy link
Contributor Author

lili2311 commented Mar 7, 2019

I followed the steps you provided:

lili@ ~/www/yarn-out-of-sync () $ open package.json
lili@ ~/www/yarn-out-of-sync () $ yarn install
yarn install v1.12.3
info No lockfile found.
[1/4] 🔍  Resolving packages...
warning snyk > proxy-agent > socks-proxy-agent > socks@1.1.10: If using 2.x branch, please upgrade to at least 2.1.6 to avoid a serious bug with socket data flow and an import issue introduced in 2.1.0
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...

success Saved lockfile.
warning Your current version of Yarn is out of date. The latest version is "1.13.0", while you're on "1.12.3".
info To upgrade, run the following command:
$ curl --compressed -o- -L https://yarnpkg.com/install.sh | bash
✨  Done in 13.14s.
lili@ ~/www/yarn-out-of-sync () $ ls
total 152
drwxr-xr-x   60 lili  staff   1920  7 Mar 10:57 ..
-rw-r--r--@   1 lili  staff    137  7 Mar 10:57 package.json
drwxr-xr-x  247 lili  staff   7904  7 Mar 10:58 node_modules
drwxr-xr-x    5 lili  staff    160  7 Mar 10:58 .
-rw-r--r--    1 lili  staff  72339  7 Mar 10:58 yarn.lock
lili@ ~/www/yarn-out-of-sync () $ snyk test

Testing /Users/lili/www/yarn-out-of-sync...

Organisation:      lili2311
Package manager:   yarn
Target file:       yarn.lock
Open source:       no
Project path:      /Users/lili/www/yarn-out-of-sync
Licenses:          enabled

✓ Tested 264 dependencies for known issues , no vulnerable paths found.

Next steps:
- Run `snyk monitor` to be notified about new related vulnerabilities.
- Run `snyk test` as part of your CI/test.

When running yarn install the project will no longer be out of sync, out of sync implies a dependency was added to the package.json but the lockfile was not updated to contain an entry for it. I think we can best help with your full set of files via support@snyk.io and take it from there.

@rylek90
Copy link

rylek90 commented Mar 7, 2019

Thanks for quick response @lili2311 🙌
I will ensure if we can share those data with you, if so I'll send an email.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants