Refactor writers

Author: felixhammerl

bin/hawkeye-scan

@@ -36,28 +36,9 @@ if (!rc.target) {
   rc.withTarget(process.env.PWD)
 }
 
-scan(rc).then(results => {
-  let total = 0
-  let exitCode = 0
-  results.forEach(moduleResult => {
-    Object.keys(moduleResult.results).forEach(key => {
-      const levelResults = moduleResult.results[key].length
-      const threshold = { low: 1, medium: 2, high: 4, critical: 8 }
-      if (levelResults > 0 && threshold[key] >= threshold[rc.failOn]) { exitCode = 1 }
-      total = total + levelResults
-    })
+scan(rc)
+  .then(code => { process.exit(code) })
+  .catch(e => {
+    logger.error('Unexpected error occurred!', e.message)
+    process.exit(42)
   })
-  logger.log('scan complete, ' + total + ' issues found')
-  rc.writers.forEach(writer => {
-    logger.log('Doing writer:', writer.key)
-    const state = exitCode === 1 ? 'fail' : 'pass'
-    writer.write(results, { state }, err => err && logger.error(err.message))
-  })
-  logger.log('Scan complete')
-  if (total > 0) {
-    process.exit(exitCode)
-  }
-}).catch(e => {
-  console.error(e)
-  process.exit(1)
-})

lib/__tests__/rc-unit.js

@@ -1,4 +1,7 @@
 'use strict'
+
+/* eslint-disable no-unused-expressions */
+
 const Rc = require('../rc')
 const path = require('path')
 
@@ -35,38 +38,47 @@ describe('RC', () => {
   })
 
   describe('withSumo', () => {
-    it('should let me add a sumo writer ', () => {
-      rc.withSumo('http://url.com')
-      expect(rc.sumo).to.equal('http://url.com')
+    it('should configure writer from hawkeyerc', () => {
+      expect(rc.writers.filter(w => w.key === 'writer-sumo').length === 1).to.be.true
     })
-    it('sumo writer should not allow invalid urls', () => {
-      expect(() => {
-        rc.withSumo('bad-url')
-      }).to.throw()
+    it('should add writer ', () => {
+      noRc.withSumo('http://url.com')
+      const [writer] = rc.writers.filter(w => w.key === 'writer-sumo')
+      expect(writer.key).to.equal('writer-sumo')
+      expect(writer.opts).to.deep.equal({ url: 'http://url.com' })
+    })
+    it('should not allow invalid urls', () => {
+      expect(() => { noRc.withSumo('bad-url') }).to.throw()
     })
   })
 
   describe('withJson', () => {
-    it('should let me add a json writer ', () => {
-      rc.withJson('path')
-      expect(rc.json).to.equal('path')
+    it('should configure from hawkeyerc', () => {
+      expect(rc.writers.filter(w => w.key === 'writer-json').length === 1).to.be.true
+    })
+    it('should add writer ', () => {
+      noRc.withJson('path')
+      const [writer] = rc.writers.filter(w => w.key === 'writer-json')
+      expect(writer.key).to.equal('writer-json')
+      expect(writer.opts).to.deep.equal({ file: 'path' })
     })
     it('should reject bad paths', () => {
-      expect(() => {
-        rc.withJson('*!&@*$^path')
-      }).to.throw()
+      expect(() => { rc.withJson('*!&@*$^path') }).to.throw()
     })
   })
 
   describe('withHttp', () => {
-    it('should let me add a http writer ', () => {
-      rc.withHttp('http://url.com')
-      expect(rc.http).to.equal('http://url.com')
+    it('should configure writer from hawkeyerc', () => {
+      expect(rc.writers.filter(w => w.key === 'writer-http').length === 1).to.be.true
     })
-    it('http writer should not allow invalid urls', () => {
-      expect(() => {
-        rc.withHttp('bad-url')
-      }).to.throw()
+    it('should add writer ', () => {
+      noRc.withHttp('http://url.com')
+      const [writer] = rc.writers.filter(w => w.key === 'writer-http')
+      expect(writer.key).to.equal('writer-http')
+      expect(writer.opts).to.deep.equal({ url: 'http://url.com' })
+    })
+    it('should not allow invalid urls', () => {
+      expect(() => { noRc.withHttp('bad-url') }).to.throw()
     })
   })
 

lib/rc.js

@@ -3,10 +3,10 @@ const fs = require('fs')
 const path = require('path')
 const util = require('./util')
 const isValidPath = require('is-valid-path')
-const ConsoleWriter = require('./writers/console')
-const JsonWriter = require('./writers/json')
-const SumoWriter = require('./writers/sumologic')
-const HttpWriter = require('./writers/http')
+const consoleWriter = require('./writers/console')
+const jsonWriter = require('./writers/json')
+const sumoWriter = require('./writers/sumo')
+const httpWriter = require('./writers/http')
 const logger = require('./logger')
 
 module.exports = class RC {
@@ -16,7 +16,7 @@ module.exports = class RC {
     this.modules = ['all']
     this.all = false
     this.staged = false
-    this.writers = [new ConsoleWriter()]
+    this.writers = [consoleWriter]
   }
 
   withStaged () {
@@ -122,33 +122,27 @@ module.exports = class RC {
     return this
   }
 
-  withJson (path) {
-    if (!isValidPath(path)) {
-      throw new Error(path + ' is not a valid path')
+  withJson (file) {
+    if (!isValidPath(file)) {
+      throw new Error(file + ' is not a valid path')
     }
-    this.json = path
-    this.writers.push(new JsonWriter({ path: this.json }))
+    this.writers.push({ ...jsonWriter, opts: { file } })
     return this
   }
 
   withSumo (url) {
-    isValidUrl(url)
-    this.sumo = url
-    this.writers.push(new SumoWriter({ url }))
+    if (!/^(https?):\/\/.*$/.test(url)) {
+      throw new Error('Invalid URL: ' + url)
+    }
+    this.writers.push({ ...sumoWriter, opts: { url } })
     return this
   }
 
   withHttp (url) {
-    isValidUrl(url)
-    this.http = url
-    this.writers.push(new HttpWriter({ url: this.http }))
+    if (!/^(https?):\/\/.*$/.test(url)) {
+      throw new Error('Invalid URL: ' + url)
+    }
+    this.writers.push({ ...httpWriter, opts: { url } })
     return this
   }
 }
-
-const isValidUrl = (userInput) => {
-  var urlregex = /^(https?):\/\/.*$/
-  if (!urlregex.test(userInput)) {
-    throw new Error('Invalid URL: ' + userInput)
-  }
-}

lib/scan.js

@@ -37,38 +37,38 @@ module.exports = async (rc = {}) => {
     throw new Error('We found no modules that would run on the target folder')
   }
 
-  const results = []
-  for (const module of activeModules) {
-    logger.log('Running module'.bold, module.key)
-    try {
-      const result = await module.run(fm)
-      results.push(result)
-    } catch (e) {
-      logger.error(module.key, 'returned an error!', e.message)
-    }
+  let results = await activeModules
+    .reduce((prom, { key, run }) => prom.then(async allRes => {
+      logger.log('Running module'.bold, key)
+      try {
+        const res = await run(fm)
+        return allRes.concat(res)
+      } catch (e) {
+        logger.error(key, 'returned an error!', e.message)
+        return allRes
+      }
+    }), Promise.resolve([]))
+
+  const threshold = { low: 1, medium: 2, high: 4, critical: 8 }
+
+  results = results
+    .map(({ key, data }) => ({
+      critical: data.critical.map(res => Object.assign({ module: key, level: 'critical' }, res, {})),
+      high: data.high.map(res => Object.assign({ module: key, level: 'high' }, res, {})),
+      medium: data.medium.map(res => Object.assign({ module: key, level: 'medium' }, res, {})),
+      low: data.low.map(res => Object.assign({ module: key, level: 'low' }, res, {}))
+    }))
+    .map(module => Object.keys(module).reduce((acc, lvl) => acc.concat(module[lvl]), []))
+    .reduce((flatmap, results) => flatmap.concat(results), [])
+    .map(res => _.omit(res, ['code']))
+    .filter(res => threshold[res.level] >= threshold[rc.failOn])
+
+  logger.log(`Scan complete, ${results.length} issues found`)
+
+  for (const { key, write, opts } of rc.writers) {
+    logger.log(`Writing to: ${key}`)
+    await write(results, opts)
   }
 
-  return results.map(result => {
-    let tmp = result.results
-    switch (rc.threshold) {
-      case 'critical':
-        delete tmp.high
-        delete tmp.medium
-        delete tmp.low
-        break
-      case 'high':
-        delete tmp.medium
-        delete tmp.low
-        break
-      case 'medium':
-        delete tmp.low
-        break
-      default:
-        break
-    }
-    return {
-      module: result.key,
-      results: tmp
-    }
-  })
+  return results.length ? 1 : 0
 }

lib/writers/__tests__/console-unit.js

@@ -0,0 +1,38 @@
+'use strict'
+
+/* eslint-disable no-unused-expressions */
+
+const { write } = require('../console')
+
+describe('Writer', () => {
+  it('should write to console', async () => {
+    sinon.stub(console, 'table')
+    const payload = [{
+      module: 'files-ccnumber',
+      level: 'critical',
+      code: 'files-secrets-47',
+      offender: 'testfile1.yml',
+      description: 'Contains word: password',
+      mitigation: 'Check contents of the file'
+    }, {
+      module: 'files-ccnumber',
+      level: 'critical',
+      code: 'files-secrets-47',
+      offender: 'testfile2.yml',
+      description: 'Contains word: password',
+      mitigation: 'Check contents of the file'
+    }, {
+      module: 'files-contents',
+      level: 'critical',
+      code: 'files-contents-2',
+      offender: 'testfile3.yml',
+      description: 'Private key in file',
+      mitigation: 'Check line number: 3'
+    }]
+
+    await write(payload)
+
+    expect(console.table).to.have.been.calledOnce
+    expect(console.table).to.have.been.calledWith(payload)
+  })
+})

lib/writers/__tests__/http-unit.js

@@ -0,0 +1,47 @@
+'use strict'
+
+const nock = require('nock')
+const { write } = require('../http')
+
+const host = 'http://host.foobar'
+const path = '/collector'
+const opts = { url: host + path }
+
+describe('Writer', () => {
+  it('should send to collector', async () => {
+    const payload1 = {
+      module: 'files-ccnumber',
+      level: 'critical',
+      code: 'files-secrets-47',
+      offender: 'testfile1.yml',
+      description: 'Contains word: password',
+      mitigation: 'Check contents of the file'
+    }
+    const payload2 = {
+      module: 'files-ccnumber',
+      level: 'critical',
+      code: 'files-secrets-47',
+      offender: 'testfile2.yml',
+      description: 'Contains word: password',
+      mitigation: 'Check contents of the file'
+    }
+    const payload3 = {
+      module: 'files-contents',
+      level: 'critical',
+      code: 'files-contents-2',
+      offender: 'testfile3.yml',
+      description: 'Private key in file',
+      mitigation: 'Check line number: 3'
+    }
+
+    nock(host, { reqheaders: { 'User-Agent': 'hawkeye' } })
+      .post(path, payload1)
+      .reply(200)
+      .post(path, payload2)
+      .reply(200)
+      .post(path, payload3)
+      .reply(200)
+
+    await write([payload1, payload2, payload3], opts)
+  })
+})

lib/writers/__tests__/json-unit.js

@@ -0,0 +1,21 @@
+'use strict'
+
+const { write } = require('../json')
+const path = require('path')
+const { readFileSync, unlinkSync } = require('fs')
+
+const metadata = {
+  file: path.join(__dirname, 'testfile.json')
+}
+
+describe('JSON Writer', () => {
+  it('should write JSON to a file', async () => {
+    const payload = { 'key': 'value' }
+    const expected = JSON.stringify(payload)
+
+    await write(payload, metadata)
+
+    expect(readFileSync(metadata.file).toString()).to.equal(expected)
+    unlinkSync(metadata.file)
+  })
+})