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

Upgrade Dependencies to fix vulnerabilities #36

Open
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

GenieTim
Copy link

Dear maintainer

Thank you for this repository.
In this pull request, I propose upgrading Babel to version 7 and gulp to version 4. The required changes should all be present.
Reasons for the upgrades include

  • being current to have all new features of the libraries
  • as well as preventing 3 vulnerabilities which were reported by npm audit.

Best regards

@piotrkochan
Copy link

My dist/ zip contains only styles/popup.css file on this code.

@GenieTim
Copy link
Author

True, insofar as that it is not necessarily a vulnerability for the users of the extensions as the dependencies are rather dev-use only. I have to admit that I did not take a closer look at the vulnerabilities reported – I could imagine e.g. a vulnerability in babel could lead to security issues in the compiled JavaScript, therefore affecting the end user. But yeah, just speculating here

@piotrkochan
Copy link

This is what I finally created based on Your PR, unfortunately I have a problem with zip, zip package does not contains scritps/ folder and I don't know why. If I remove clean task from the build then all files are present.

import browserify from 'browserify';
import merge from 'merge-stream';
import fs from 'fs';
import gulp from 'gulp';
import gulpif from 'gulp-if';
import preprocessify from 'preprocessify';
import buffer from 'vinyl-buffer';
import source from 'vinyl-source-stream';

const $ = require('gulp-load-plugins')();

const production = process.env.NODE_ENV === 'production';
const target = process.env.TARGET || 'chrome';
const environment = process.env.NODE_ENV || 'development';

var generic = JSON.parse(fs.readFileSync(`./config/${environment}.json`));
var specific = JSON.parse(fs.readFileSync(`./config/${target}.json`));
var context = Object.assign({}, generic, specific);

var defaultManifest = {
  dev: {
    'background':
      {
        'scripts': [
          'scripts/livereload.js',
          'scripts/icon-changer.js'
        ]
      }
  },
  firefox: { 'applications': { 'gecko': { 'id': '[email protected]' } } }
};

function styles() {
  return gulp.src('src/styles/**/*.scss')
    .pipe($.plumber())
    .pipe(
      $.sass
        .sync({ outputStyle: 'expanded', precision: 10, includePaths: ['.'] })
        .on('error', $.sass.logError)
    )
    .pipe(gulp.dest(`./build/${target}/styles/`));
}

function manifest() {
  return gulp.src('./manifest.json')
    .pipe(gulpif(!production, $.mergeJson({
      fileName: 'manifest.json',
      jsonSpace: ' '.repeat(4),
      endObj: defaultManifest.dev
    })))
    .pipe(gulpif(target === 'firefox', $.mergeJson({
      fileName: 'manifest.json',
      jsonSpace: ' '.repeat(4),
      endObj: defaultManifest.firefox
    })))

    .pipe(gulp.dest(`./build/${target}`))
}

exports.manifest = manifest;

function pipe(src, ...transforms) {
  return transforms.reduce((stream, transform) => {
    const isDest = typeof transform === 'string';
    return stream.pipe(isDest ? gulp.dest(transform) : transform)
  }, gulp.src(src, { allowEmpty: true }))
}

function zip() {
  return gulp.src(`build/${target}/**/*`)
    .pipe($.zip(`${target}.zip`))
    .pipe(gulp.dest('./dist'))
}


function buildJS(target) {
  let files = [
    'contentscript.js', 'icon-changer.js', 'popup.js',
  ];

  if (!production) {
    files.push('livereload.js');
  }

  let tasks = files.map(file => {
    return browserify({ entries: 'src/scripts/' + file, debug: true })
      .transform('babelify', { presets: ['@babel/preset-env'] })
      .transform(preprocessify, { includeExtensions: ['.js'], context: context })
      .bundle()
      .pipe(source(file))
      .pipe(buffer())
      .pipe(gulpif(!production, $.sourcemaps.init({ loadMaps: true })))
      .pipe(gulpif(!production, $.sourcemaps.write('./')))
      .pipe(gulpif(production, $.uglify({ 'mangle': false, 'output': { 'ascii_only': true } })))
      .pipe(gulp.dest(`./build/${target}/scripts`));
  });

  return merge(tasks)
}

function js(done) {
  buildJS(target);
  done()
}

function clean() {
  return pipe(`./build/${target}`, $.clean())
}

function watch() {
  $.livereload.listen();
  gulp.watch('./src/**/*', gulp.series('build', function (done) {
    $.livereload.reload();
    done();
  }))

  // gulp.watch('./src/**/*', gulp.series('build'))
}

function files() {
  return merge(
    pipe('./src/icons/**/*', `./build/${target}/icons`),
    pipe(['./src/_locales/**/*'], `./build/${target}/_locales`),
    pipe([`./src/images/${target}/**/*`], `./build/${target}/images`),
    pipe(['./src/images/shared/**/*'], `./build/${target}/images`),
    pipe(['./src/**/*.html'], `./build/${target}`))
}

exports.clean = clean;
exports.assets = gulp.series(styles, js);
exports.build = gulp.series(clean, exports.assets, manifest, files);
exports.watch = gulp.series(exports.build, watch);
exports.dist = gulp.series(exports.build, zip);

exports.default = exports.build;

@GenieTim
Copy link
Author

GenieTim commented Jul 18, 2019

I am sorry, @piotrkochan, I may not understand your inquiry. The code you posted is not mine, nor the one in this PR (see gulpfile.babel.js). The error you mention looks like you have a typo somewhere: scritps instead of scripts, but I do not understand when and where this error is thrown. Would you please mind open an issue if you have problems with the code in this repo, instead of commenting here, especially if unrelated to this PR.

@hhaoao hhaoao mentioned this pull request Jan 25, 2020
CedricL46
CedricL46 approved these changes Mar 12, 2020
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.

3 participants