feat: ACNA-4061 - aio app pack should include a lockfile to ensure reproducibility and security (#885)

* add `aio app install --[no]-allow-scripts` flag

Author: shazron

src/commands/app/install.js

@@ -19,7 +19,7 @@ const execa = require('execa')
 const unzipper = require('unzipper')
 const { validateJsonWithSchema } = require('../../lib/install-helper')
 const jsYaml = require('js-yaml')
-const { USER_CONFIG_FILE, DEPLOY_CONFIG_FILE } = require('../../lib/defaults')
+const { USER_CONFIG_FILE, DEPLOY_CONFIG_FILE, PACKAGE_LOCK_FILE } = require('../../lib/defaults')
 const ora = require('ora')
 
 // eslint-disable-next-line node/no-missing-require
@@ -51,7 +51,14 @@ class InstallCommand extends BaseCommand {
       await this.unzipFile(args.path, outputPath)
       await this.validateAppConfig(outputPath, USER_CONFIG_FILE)
       await this.validateDeployConfig(outputPath, DEPLOY_CONFIG_FILE)
-      await this.npmInstall(flags.verbose)
+
+      const packageLockPath = path.join(outputPath, PACKAGE_LOCK_FILE)
+      if (fs.existsSync(packageLockPath)) {
+        await this.npmCI(flags.verbose, flags['allow-scripts'])
+      } else {
+        this.warn('No lockfile found, running standard npm install. It is recommended that you include a lockfile with your app bundle.')
+        await this.npmInstall(flags.verbose, flags['allow-scripts'])
+      }
       if (flags.tests) {
         await this.runTests()
       }
@@ -131,15 +138,26 @@ class InstallCommand extends BaseCommand {
     }
   }
 
-  async npmInstall (isVerbose) {
+  async npmInstall (isVerbose = false, allowScripts = true) {
+    const ignoreScripts = allowScripts ? undefined : '--ignore-scripts'
     this.spinner.start('Running npm install...')
     const stdio = isVerbose ? 'inherit' : 'ignore'
-    return execa('npm', ['install'], { stdio })
+    return execa('npm', ['install', ignoreScripts], { stdio })
       .then(() => {
         this.spinner.succeed('Ran npm install')
       })
   }
 
+  async npmCI (isVerbose = false, allowScripts = true) {
+    const ignoreScripts = allowScripts ? undefined : '--ignore-scripts'
+    this.spinner.start('Running npm ci...')
+    const stdio = isVerbose ? 'inherit' : 'ignore'
+    return execa('npm', ['ci', ignoreScripts], { stdio })
+      .then(() => {
+        this.spinner.succeed('Ran npm ci')
+      })
+  }
+
   async runTests (isVerbose) {
     this.spinner.start('Running app tests...')
     return this.config.runCommand('app:test').then((result) => {
@@ -157,6 +175,11 @@ InstallCommand.description = `This command will support installing apps packaged
 
 InstallCommand.flags = {
   ...BaseCommand.flags,
+  'allow-scripts': Flags.boolean({
+    description: 'Allow post and preinstall scripts during npm install/npm ci',
+    default: true,
+    allowNo: true
+  }),
   output: Flags.string({
     description: 'The packaged app output folder path',
     char: 'o',

src/commands/app/pack.js

@@ -71,8 +71,13 @@ class Pack extends BaseCommand {
         this.error(hookResults.failures.map(f => `${f.plugin.name} : ${f.error.message}`).join('\nError: '), { exit: 1 })
       }
 
+      const filesToInclude = flags['lock-file'] ? ['package-lock.json'] : []
+
       // 1a. Get file list to pack
-      const fileList = await this.filesToPack({ filesToExclude: [flags.output, DEFAULTS.DIST_FOLDER, ...distLocations] })
+      const fileList = await this.filesToPack({
+        filesToExclude: [flags.output, DEFAULTS.DIST_FOLDER, ...distLocations],
+        filesToInclude
+      })
       this.log('=== Files to pack ===')
       fileList.forEach((file) => {
         this.log(`  ${file}`)
@@ -265,10 +270,11 @@ class Pack extends BaseCommand {
    *
    * @param {object} options the options for the method
    * @param {Array<string>} options.filesToExclude a list of files to exclude
+   * @param {Array<string>} options.filesToInclude a list of files to include (will always be included)
    * @param {string} options.workingDirectory the working directory to run `npm pack` in
    * @returns {Array<string>} a list of files that are to be packed
    */
-  async filesToPack ({ filesToExclude = [], workingDirectory = process.cwd() } = {}) {
+  async filesToPack ({ filesToExclude = [], filesToInclude = [], workingDirectory = process.cwd() } = {}) {
     const { stdout } = await execa('npm', ['pack', '--dry-run', '--json'], { cwd: workingDirectory })
 
     const noJunkFiles = (file) => {
@@ -290,11 +296,14 @@ class Pack extends BaseCommand {
     }
 
     const { files } = JSON.parse(stdout)[0]
-    return files
-      .map(file => file.path)
-      .filter(file => !filesToExclude.includes(file))
-      .filter(noJunkFiles) // no junk files like .DS_Store
-      .filter(noDotFiles) // no files that start with a '.'
+    return [
+      ...files
+        .map(file => file.path)
+        .filter(file => !filesToExclude.includes(file))
+        .filter(noJunkFiles) // no junk files like .DS_Store
+        .filter(noDotFiles), // no files that start with a '.'
+      ...filesToInclude
+    ]
   }
 
   /**
@@ -361,6 +370,11 @@ Pack.description = `This command will support packaging apps for redistribution.
 
 Pack.flags = {
   ...BaseCommand.flags,
+  'lock-file': Flags.boolean({
+    description: 'Include the package-lock.json file in the packaged app',
+    default: true,
+    allowNo: true
+  }),
   output: Flags.string({
     description: 'The packaged app output file path',
     char: 'o',

src/lib/defaults.js

@@ -33,6 +33,7 @@ module.exports = {
   IMPORT_CONFIG_FILE: 'config.json',
   USER_CONFIG_FILE: 'app.config.yaml',
   DEPLOY_CONFIG_FILE: 'deploy.yaml',
+  PACKAGE_LOCK_FILE: 'package-lock.json',
   LEGACY_RUNTIME_MANIFEST: 'manifest.yml',
   INCLUDE_DIRECTIVE: '$include',
   APPLICATION_CONFIG_KEY: 'application',

test/commands/app/install.test.js

@@ -13,6 +13,7 @@ governing permissions and limitations under the License.
 const TheCommand = require('../../../src/commands/app/install.js')
 const BaseCommand = require('../../../src/BaseCommand.js')
 const fs = require('fs-extra')
+const { PACKAGE_LOCK_FILE } = require('../../../src/lib/defaults.js')
 const unzipper = require('unzipper')
 const execa = require('execa')
 const installHelper = require('../../../src/lib/install-helper')
@@ -221,16 +222,15 @@ describe('npmInstall', () => {
     command = new TheCommand()
   })
 
-  test('success', async () => {
+  test('success (defaults)', async () => {
     execa.mockImplementationOnce((cmd, args, options) => {
       expect(cmd).toEqual('npm')
       expect(args).toEqual(['install'])
       expect(options.stdio).toEqual('ignore')
       return Promise.resolve({ stdout: '' })
     })
 
-    const isVerbose = false
-    await expect(command.npmInstall(isVerbose)).resolves.toEqual(undefined)
+    await expect(command.npmInstall()).resolves.toEqual(undefined)
   })
 
   test('success --verbose', async () => {
@@ -245,6 +245,19 @@ describe('npmInstall', () => {
     await expect(command.npmInstall(isVerbose)).resolves.toEqual(undefined)
   })
 
+  test('success --verbose --no-allow-scripts', async () => {
+    execa.mockImplementationOnce((cmd, args, options) => {
+      expect(cmd).toEqual('npm')
+      expect(args).toEqual(['install', '--ignore-scripts'])
+      expect(options.stdio).toEqual('inherit')
+      return Promise.resolve({ stdout: '' })
+    })
+
+    const isVerbose = true
+    const allowScripts = false
+    await expect(command.npmInstall(isVerbose, allowScripts)).resolves.toEqual(undefined)
+  })
+
   test('failure', async () => {
     const errorMessage = 'npm install error'
 
@@ -258,6 +271,63 @@ describe('npmInstall', () => {
   })
 })
 
+describe('npmCI', () => {
+  let command
+
+  beforeEach(() => {
+    execa.mockReset()
+    command = new TheCommand()
+  })
+
+  test('success (defaults)', async () => {
+    execa.mockImplementationOnce((cmd, args, options) => {
+      expect(cmd).toEqual('npm')
+      expect(args).toEqual(['ci'])
+      expect(options.stdio).toEqual('ignore')
+      return Promise.resolve({ stdout: '' })
+    })
+
+    await expect(command.npmCI()).resolves.toEqual(undefined)
+  })
+
+  test('success --verbose', async () => {
+    execa.mockImplementationOnce((cmd, args, options) => {
+      expect(cmd).toEqual('npm')
+      expect(args).toEqual(['ci'])
+      expect(options.stdio).toEqual('inherit')
+      return Promise.resolve({ stdout: '' })
+    })
+
+    const isVerbose = true
+    await expect(command.npmCI(isVerbose)).resolves.toEqual(undefined)
+  })
+
+  test('success --verbose --no-allow-scripts', async () => {
+    execa.mockImplementationOnce((cmd, args, options) => {
+      expect(cmd).toEqual('npm')
+      expect(args).toEqual(['ci', '--ignore-scripts'])
+      expect(options.stdio).toEqual('inherit')
+      return Promise.resolve({ stdout: '' })
+    })
+
+    const isVerbose = true
+    const allowScripts = false
+    await expect(command.npmCI(isVerbose, allowScripts)).resolves.toEqual(undefined)
+  })
+
+  test('failure', async () => {
+    const errorMessage = 'npm ci error'
+
+    execa.mockImplementationOnce((cmd, args) => {
+      expect(cmd).toEqual('npm')
+      expect(args).toEqual(['ci'])
+      throw new Error(errorMessage)
+    })
+
+    await expect(command.npmCI()).rejects.toThrow(errorMessage)
+  })
+})
+
 describe('runTests', () => {
   let command
 
@@ -284,7 +354,11 @@ describe('runTests', () => {
 })
 
 describe('run', () => {
-  test('no flags', async () => {
+  beforeEach(() => {
+    fs.existsSync.mockReset()
+  })
+
+  test('no flags, no lockfile', async () => {
     const command = new TheCommand()
     command.argv = ['my-app.zip']
 
@@ -295,6 +369,7 @@ describe('run', () => {
     command.validateDeployConfig = jest.fn()
     command.runTests = jest.fn()
     command.npmInstall = jest.fn()
+    command.npmCI = jest.fn()
     command.error = jest.fn()
     await command.run()
 
@@ -305,6 +380,36 @@ describe('run', () => {
     expect(command.validateDeployConfig).toHaveBeenCalledTimes(1)
     expect(command.runTests).toHaveBeenCalledTimes(1)
     expect(command.npmInstall).toHaveBeenCalledTimes(1)
+    expect(command.npmCI).toHaveBeenCalledTimes(0)
+    expect(command.error).toHaveBeenCalledTimes(0)
+  })
+
+  test('no flags, has lockfile', async () => {
+    const command = new TheCommand()
+    command.argv = ['my-app.zip']
+
+    // since we already unit test the methods above, we mock it here
+    command.validateZipDirectoryStructure = jest.fn()
+    command.unzipFile = jest.fn()
+    command.addCodeDownloadAnnotation = jest.fn()
+    command.validateDeployConfig = jest.fn()
+    command.runTests = jest.fn()
+    command.npmInstall = jest.fn()
+    command.npmCI = jest.fn()
+    command.error = jest.fn()
+
+    fs.existsSync.mockImplementation((filePath) => filePath === PACKAGE_LOCK_FILE)
+
+    await command.run()
+
+    expect(command.validateZipDirectoryStructure).toHaveBeenCalledTimes(1)
+    expect(command.unzipFile).toHaveBeenCalledTimes(1)
+    expect(libConfig.coalesce).toHaveBeenCalledTimes(1)
+    expect(libConfig.validate).toHaveBeenCalledTimes(1)
+    expect(command.validateDeployConfig).toHaveBeenCalledTimes(1)
+    expect(command.runTests).toHaveBeenCalledTimes(1)
+    expect(command.npmInstall).toHaveBeenCalledTimes(0)
+    expect(command.npmCI).toHaveBeenCalledTimes(1)
     expect(command.error).toHaveBeenCalledTimes(0)
   })
 
@@ -321,6 +426,7 @@ describe('run', () => {
     command.addCodeDownloadAnnotation = jest.fn()
     command.validateDeployConfig = jest.fn()
     command.npmInstall = jest.fn()
+    command.npmCI = jest.fn()
     command.error = jest.fn()
     command.runTests = jest.fn(() => { throw errorObject })
 
@@ -351,6 +457,7 @@ describe('run', () => {
     command.addCodeDownloadAnnotation = jest.fn()
     command.validateDeployConfig = jest.fn()
     command.npmInstall = jest.fn()
+    command.npmCI = jest.fn()
     command.error = jest.fn()
     command.runTests = jest.fn(() => { throw new Error(errorMessage) })
 
@@ -379,6 +486,7 @@ describe('run', () => {
     command.validateDeployConfig = jest.fn()
     command.runTests = jest.fn()
     command.npmInstall = jest.fn()
+    command.npmCI = jest.fn()
     command.error = jest.fn()
 
     await command.run()
@@ -405,6 +513,7 @@ describe('run', () => {
     command.validateDeployConfig = jest.fn()
     command.runTests = jest.fn()
     command.npmInstall = jest.fn()
+    command.npmCI = jest.fn()
     command.error = jest.fn()
 
     const err = new Error('fake validation error')
@@ -452,6 +561,7 @@ describe('run', () => {
     command.validateDeployConfig = jest.fn()
     command.runTests = jest.fn()
     command.npmInstall = jest.fn()
+    command.npmCI = jest.fn()
     command.error = jest.fn()
 
     await command.run()