Remove dependency on Gulp

[felixfbecker]

There is no reason why this depends on Gulp. Gulp brings in a lot of dependencies like vinyl-fs and in turn graceful-fs, which we dont need at all. All Gulp is used for is task registration, which orchestrator (undertaker for gulp 4) are used for.

Using yargs would do the job much better.

[davidgoli]

👍

[sdepold]

I agree. Back then I thought it would be a funny an easy going idea but it turned out to be a nightmare instead. I'm all for it!

[davidgoli]

At the very least a short-term fix would involve upgrading gulp to silence the graceful-fs deprecation messages.

[tdreyno]

Not to mention this! https://snyk.io/test/npm/sequelize-cli

[felixfbecker]

@tdreyno This is a CLI tool, not something you expose via HTTP or something, and sequelize-cli doesn't use the minimatch globbing at all. There is no vulnerability here.

[tdreyno]

Fair. I'm using automated security checking tools for node and it's complaining because of that, regardless of if the vector is real or not.

[andrew-cunliffe]

This is a pain, we have been asked to run nsp check as part of our CI build, but this dependency is the only one flagging a problem and its a false positive on the check :(

👍 to remove gulp as a required dependency

nsp check
(+) 2 vulnerabilities found
┌───────────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│               │ Regular Expression Denial of Service                                                                                                                           │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Name          │ minimatch                                                                                                                                                      │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Installed     │ 2.0.10                                                                                                                                                         │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Vulnerable    │ <=3.0.1                                                                                                                                                        │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Patched       │ >=3.0.2                                                                                                                                                        │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Path          │ ........ > sequelize-cli@2.4.0 > gulp@3.9.1 > vinyl-fs@0.3.14 > glob-stream@3.1.18 > minimatch@2.0.10                                          │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ More Info     │ https://nodesecurity.io/advisories/118                                                                                                                         │
└───────────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│               │ Regular Expression Denial of Service                                                                                                                           │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Name          │ minimatch                                                                                                                                                      │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Installed     │ 0.2.14                                                                                                                                                         │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Vulnerable    │ <=3.0.1                                                                                                                                                        │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Patched       │ >=3.0.2                                                                                                                                                        │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Path          │ ........ > sequelize-cli@2.4.0 > gulp@3.9.1 > vinyl-fs@0.3.14 > glob-watcher@0.0.6 > gaze@0.5.2 > globule@0.1.0 > minimatch@0.2.14             │
├───────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ More Info     │ https://nodesecurity.io/advisories/118                                                                                                                         │
└───────────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

[felixfbecker]

nsp allows to exclude certain packages iirc. Especially since sequelize-cli is a devdep...

[eddiejaoude]

sequelize-cli is a Dev Dep? Doesn't state that in the docs. Otherwise would have --save-dev there.

[andrew-cunliffe]

So, I just tried our code without this dependency and migrations don't work now.... last I looked AWS Elastic Beanstalk will only install non dev dependencies, but it has been a while since I tried to check this....

Running "shell:migrateUp" (shell) task
Migrating blog service up in test mode
{ [String: '']
  stdout: '',
  stderr: '/bin/sh: ./node_modules/.bin/sequelize: No such file or directory\n',
  code: 127,
  cat: [Function: bound ],
  head: [Function: bound ],
  tail: [Function: bound ],
  to: [Function: bound ],
  toEnd: [Function: bound ],
  sed: [Function: bound ],
  sort: [Function: bound ],
  uniq: [Function: bound ],
  grep: [Function: bound ],
  exec: [Function: bound ] }
Warning: Done, with errors: command "./scripts/migrations.js migrate" (target "migrateUp") exited with code 1. Use --force to continue.

What is the recommended way to run migrations on deployment if this is a dev dependency?

While I understand it might not be part of the running production code we need to execute the dependency on the production servers to run the migrations, this means we need to be happy it has no vulnerabilities....?

Open to your thoughts / ideas on this

Some insights to the script if it helps

program
    .command('migrate [services...]')
    .description('run migrations for a service')
    .option('-d, --direction [direction]', 'Which direction to go up / down', 'up')
    .action((services, options) => {
        
        // removed snippet

        services.forEach(item => {
            let path = `scripts/migrations.tmp/${item}`;
            console.log(`Migrating ${item} service ${options.direction} in ${program.env} mode`);
            shell.mkdir('-p', path);

            // removed snippet

            var shellExec = shell.exec(`./node_modules/.bin/sequelize ${migrateCmd} \
                --env=${program.env} \
                --config=src/common/db.config.js \
                --migrations-path=${path}`,
                { silent: true }
            );

            // removed snippet

        });
    });

[felixfbecker]

My understanding that devDependencies are everything needed to build and test your code. For example also Babel or TypeScript. In your production env you of course also need to install those to build your project, so I would never use --production flag.

[sonicoder86]

Created pull request that fixes it #430

[sonicoder86]

@felixfbecker Isn't it used only for testing? I don't see any sign of it in the bin file.

[idris]

Would an upgrade to Gulp 4 fix this issue? Especially in the 2.x.x version of sequelize-cli

/cc @sushantdhiman

[webmobiles]

why use one thing old like Gulp, is not better webpack ? why sequelize tool don't generate ES6 js ? i am newbie with sequelize, but i don't understand why to write code like 5 years ago

[sushantdhiman]

Because this project is around 7 years old, be patient we are working on fixing things. If you want things to accelerate send me a PR :)

[webmobiles]

oh my God,; 7 years old :-) , must too look another ES6/ES7 Orm

[sushantdhiman]

@webmobiles I suggest https://github.com/Vincit/objection.js/ , you are welcome to try Sequelize any time :). Its open source after all

[webmobiles]

thanks susan , it was a joke, i am a dinosaur that comes from old php 4, in the new node world, i feel like i'm not working with modern tools so that gets me crazy, thanks , hope to learn to contribute to modernize all these old packages

[sushantdhiman]

480f3fc