Fix log loading issue (#121)

* Don't try to load log messages (causes occasional crash, add path to fdesetup

* don't crash when kmd command exits with non-zero status

* version bump, moved minimum mac/windows os versions

* Removed log reading code, fixed all warnings, added colorized console output to make messages easier to see. Fixed error messaging in app to show actual error rather than "Timeout". commented server code.

* Added missing cross-env setting in build:electron script

.npmrc

@@ -1 +1,2 @@
 registry=https://registry.npmjs.org/
+scripts-prepend-node-path=true

package.json

@@ -1,6 +1,6 @@
 {
   "name": "Stethoscope",
-  "version": "3.0.3",
+  "version": "3.0.4",
   "private": true,
   "homepage": "./",
   "author": "Netflix",
@@ -99,7 +99,7 @@
     ]
   },
   "scripts": {
-    "start": "nf start -p 12000",
+    "start": "cross-env FORCE_COLOR=true nf start -p 12000",
     "test": "react-scripts test --env=jsdom",
     "electron": "cross-env STETHOSCOPE_ENV=development electron .",
     "electron:start": "node src/start-react",
@@ -109,7 +109,7 @@
     "build:react": "react-scripts build && node update-download-page.js",
     "build:mac": "rm -r dist/; npm run build:react && npm run build:electron -m && npm run test:spectron",
     "build:windows": " npm run build:react && npm run build:electron -w && npm run test:spectron",
-    "build:electron": "ELECTRON_BUILDER_COMPRESSION_LEVEL=9 electron-builder",
+    "build:electron": "cross-env ELECTRON_BUILDER_COMPRESSION_LEVEL=9 electron-builder",
     "test:spectron": "node src/__tests__/test-build.js",
     "build:linux": "rm -r dist/ ; react-scripts build && electron-builder -l",
     "lint": "standard --fix src/*.js src/**/*.js resolvers/*.js sources/*.js server.js"
@@ -118,6 +118,7 @@
     "applescript": "^1.0.0",
     "auto-launch": "^5.0.5",
     "body-parser": "^1.18.2",
+    "chalk": "^2.4.2",
     "classnames": "^2.2.5",
     "cors": "^2.8.4",
     "cross-env": "^5.2.0",

practices/policy.yaml

@@ -1,14 +1,14 @@
 osVersion:
   darwin:
     # High Sierra
-    ok: ">=10.13.6"
+    ok: ">=10.14.3"
     # Sierra
     nudge: ">=10.12.6"
   win32:
-    # Version 1803 - April 2018 Update
-    ok: ">=10.0.17134"
-    # Version 1803 - Redstone 3 Fall Creators Update
-    nudge: ">=10.0.16299"
+    # Version 1809
+    ok: ">=10.0.17763"
+    # Version 1803
+    nudge: ">=10.0.17134"
   awsWorkspace:
     ok: ">=10.0.14393"
     nudge: ">=10.0.10240"

server.js

@@ -109,8 +109,11 @@ module.exports = async function startServer (env, log, language = 'en-US', appAc
   }
 
   app.use(['/scan', '/graphql'], cors(corsOptions), async (req, res) => {
-    req.setTimeout(60000)
+    // set upper boundary on scan time
+    const MAX_SCAN_SECONDS = 45
+    req.setTimeout(MAX_SCAN_SECONDS * 1000)
 
+    // allow GET/POST requests and determine what property to use
     const key = req.method === 'POST' ? 'body' : 'query'
     const origin = req.get('origin')
     const remote = origin !== 'stethoscope://main'
@@ -133,37 +136,48 @@ module.exports = async function startServer (env, log, language = 'en-US', appAc
     // are throttled by the users's session id
     let showNotification = sessionId && !alertCache.has(sessionId)
     const start = performance.now()
-    // TODO each of these checks should probably be individually executed
+    // TODO each of these checks should be individually executed
     // by relecvant resolvers. Since it is currently super fast, there is no
     // real performance penalty for running all checks on each request
+    // this would require loading the script files differently so the resolvers
+    // could execute the appropriate pre-compiled scripts
     const checkData = await Promise.all(checks.map(async script => {
-      const response = await run(script)
-      return response
+      try { return await run(script) }
+      catch (e) { return '' }
     }))
+    // perf data
     const total = performance.now() - start
 
     context.kmdResponse = extend(true, {}, ...checkData)
-
-    policy = policy || {}
-
+    // throttle native push notifications to user by session id
     if (sessionId && !alertCache.has(sessionId)) {
       alertCache.set(sessionId, true)
     }
 
+    // policy needs to be an object, regardless of whether or not one was
+    // supplied in the request, parse if String was supplied
     if (typeof policy === 'string') {
       policy = JSON.parse(policy)
+    } else {
+      policy = Object.assign({}, policy)
     }
 
-    // tell the app if a policy was passed to display scanning status
+    // if a policy was passed, tell the app display scanning status
     if (Object.keys(policy).length) {
       // show the scan is happening in the UI
       io.sockets.emit('scan:init', { remote, remoteLabel })
     }
 
-    graphql(schema, query, null, context, policy).then((result) => {
-      const { data = {} } = result
+    graphql(schema, query, null, context, policy).then(result => {
+      const { data = {}, errors } = result
       let scanResult = { noResults: true }
 
+      if (errors && !remote) {
+        const errMessage = errors.reduce((p, c) => p + c + '\n', '')
+        io.sockets.emit('scan:error', { error: errMessage })
+        throw new Error(errMessage)
+      }
+
       // update the tray icon if a policy result is in the response
       if (data.policy && data.policy.validate) {
         appActions.setScanStatus(data.policy.validate.status)
@@ -177,7 +191,7 @@ module.exports = async function startServer (env, log, language = 'en-US', appAc
       res.json(result)
     }).catch(err => {
       log.error(err.message)
-      io.sockets.emit('scan:error')
+      io.sockets.emit('scan:error', { error: err.message })
       res.status(500).json({ error: err.message })
     })
   })

sources/darwin/file-vault.sh

@@ -1,3 +1,3 @@
 #!/usr/bin/env kmd
-exec fdesetup isactive
+exec /usr/bin/fdesetup isactive
 save disks.fileVaultEnabled

src/Action.js

@@ -2,9 +2,9 @@ import React, { Component } from 'react'
 import ReactDOMServer from 'react-dom/server'
 import Accessible from './Accessible'
 import ActionIcon from './ActionIcon'
-import Handlebars from 'handlebars'
 import semver from 'semver'
 import showdown from 'showdown'
+import Handlebars from 'handlebars/dist/handlebars.min.js';
 
 const converter = new showdown.Converter()
 
@@ -186,7 +186,11 @@ class Action extends Component {
     }
 
     return (
-      <li className={type} key={action.title} ref={el => { this.el = el }}>
+      <li
+        className={type}
+        key={String(action.title).replace(/[^a-zA-Z]+/g, '')}
+        ref={el => { this.el = el }}
+      >
         <span className='title' onClick={this.toggleDescription}>
           <ActionIcon
             className='action-icon'

src/App.js

@@ -18,16 +18,13 @@ import './App.css'
 const socket = openSocket(HOST)
 
 let platform = MAC
-let shell; let ipcRenderer; let readLastLines; let log; let remote; let settings; let app; let logPath = ''
+let shell; let ipcRenderer; let log; let remote; let settings;
 // CRA doesn't like importing native node modules, have to use window.require AFAICT
 try {
   const os = window.require('os')
   shell = window.require('electron').shell
   remote = window.require('electron').remote
-  readLastLines = window.require('read-last-lines')
   settings = window.require('electron-settings')
-  app = remote.getGlobal('app')
-  logPath = app.getPath('userData')
   log = remote.getGlobal('log')
   platform = os.platform()
   ipcRenderer = window.require('electron').ipcRenderer
@@ -47,7 +44,6 @@ class App extends Component {
     scanIsRunning: false,
     loading: false,
     lastScanDuration: 0,
-    recentLogs: '',
     // determines loading screen language
     remoteScan: false,
     // surface which app performed the most recent scan
@@ -71,8 +67,6 @@ class App extends Component {
 
     this.setState({ recentHang: settings.get('recentHang', 0) > 1 })
 
-    this.getRecentLogs()
-
     ipcRenderer.send('scan:init')
     // perform the initial policy load & scan
     await this.loadPractices()
@@ -87,7 +81,6 @@ class App extends Component {
     // trigger scan from main process
     ipcRenderer.on('autoscan:start', ({ notificationOnViolation = false }) => {
       if (!this.state.scanIsRunning) {
-        console.log('autoscan')
         ipcRenderer.send('scan:init')
         if (Object.keys(this.state.policy).length) {
           this.scan()
@@ -100,6 +93,8 @@ class App extends Component {
     socket.on('scan:init', this.onScanInit)
     // setup a socket io listener to refresh the app when a scan is performed
     socket.on('scan:complete', this.onScanComplete)
+    // TODO handle errors that happen on local scans
+    // socket.on('scan:error', this.onScanError)
     // the focus/blur handlers are used to update the last scanned time
     window.addEventListener('focus', () => this.setState({ focused: true }))
     window.addEventListener('blur', () => this.setState({ focused: false }))
@@ -134,6 +129,11 @@ class App extends Component {
     })
   }
 
+  onScanError = ({ error }) => {
+    this.errorThrown = true
+    throw new Error(error)
+  }
+
   onScanInit = ({ remote, remoteLabel }) => {
     ipcRenderer.send('scan:init')
     this.setState({
@@ -275,21 +275,13 @@ class App extends Component {
       }).catch(err => {
         console.log(err)
         log.error(JSON.stringify(err))
-        let message = new Error('Request timeout')
+        let message = new Error(err.message)
         if (err.errors) message = new Error(JSON.stringify(err.errors))
         this.handleErrorGraphQL({ message })
       })
     })
   }
 
-  getRecentLogs = () => {
-    const today = moment().format('YYYY-MM-DD')
-    const path = `${logPath}/dev-application-${today}.log`
-    readLastLines.read(path, 10).then(recentLogs =>
-      this.setState({ recentLogs })
-    ).catch(err => console.error(err))
-  }
-
   highlightRescanButton = event => this.setState({ highlightRescan: true })
 
   render () {

src/Device.js

@@ -71,16 +71,17 @@ class Device extends Component {
 
       if (hasResults) {
         return (
-          <Action {...actionProps}>
-            <ul className='result-list'>
+          <Action {...actionProps} key={`action-container-${action.name}`}>
+            <ul className='result-list' key={`action-ul-${action.name}`}>
               {results.map(({ name, url, status, description }) => {
                 const iconProps = status === 'PASS'
                   ? { name: 'checkmark', color: '#bbd8ca' }
                   : { name: 'blocked', color: '#a94442' }
+
                 return (
                   <li
                     className='result-list-item'
-                    key={name}
+                    key={`action-li-${action.name}`}
                   >
                     <div className='result-heading'>
                       <strong>
@@ -106,7 +107,7 @@ class Device extends Component {
         )
       } else {
         return (
-          <Action {...actionProps} />
+          <Action {...actionProps} key={`action-container-${action.name}`} />
         )
       }
     })
@@ -178,7 +179,7 @@ class Device extends Component {
           <h4>{org} {this.props.strings.policyDescription}</h4>
 
           <div className='action-list'>
-            <ul>
+            <ul key="action-list-main-ul">
               { this.actions(device.critical, 'critical', device) }
               { this.actions(device.suggested, 'suggested', device) }
               { this.actions(device.done, 'done', device) }

src/ErrorBoundary.js

@@ -18,13 +18,20 @@ export default class ErrorBoundary extends React.Component {
   }
 
   componentDidCatch (error, info) {
-    this.setState({ hasError: true })
+    this.setState({ hasError: true, error: serializeError(error) })
     log.error(JSON.stringify(serializeError(error)))
   }
 
   render () {
     if (this.state.hasError) {
-      return <h1>Something went wrong.</h1>
+      return (
+        <React.Fragment>
+          <h1>Something went wrong.</h1>
+          <pre>
+            {JSON.stringify(this.state.error, null, 3)}
+          </pre>
+        </React.Fragment>
+      )
     }
     return this.props.children
   }

src/lib/logger.js

@@ -10,6 +10,7 @@
 const { app } = require('electron')
 const winston = require('winston')
 const path = require('path')
+const chalk = require('chalk')
 require('winston-daily-rotate-file')
 const IS_DEV = process.env.STETHOSCOPE_ENV === 'development'
 
@@ -24,6 +25,7 @@ try {
 let envPrefix = IS_DEV ? 'dev-' : ''
 let maxFiles = IS_DEV ? '1d' : '3d'
 const logLevels = ['error', 'warn', 'info', 'verbose', 'debug', 'silly']
+const logColors = ['red', 'yellow', 'cyan', 'magenta']
 // logs that will continue to output in prod
 // change if you want more than 'error' and 'warn'
 const productionLogs = logLevels.slice(0, 2)
@@ -46,17 +48,27 @@ if (!global.log) {
   })
 
   // support multiple arguments to winston logger
-  const wrapper = ( original ) => {
+  const wrapper = ( original, level ) => {
     return (...args) => original(args.map(o => {
-      if (typeof o === 'string') return o
-      return JSON.stringify(o, null, 2)
+      let color = false
+      let transform = s => s
+
+      if (IS_DEV) {
+        let index = logLevels.indexOf(level)
+        if (index > -1) {
+          color = logColors[index]
+          transform = s => chalk[color](s)
+        }
+      }
+      if (typeof o === 'string') return transform(o)
+      return transform(JSON.stringify(o, null, 2))
     }).join(' '))
   }
 
   // log all levels in DEV, to default file logger AND console
   if (IS_DEV) {
     logLevels.forEach(level =>
-      log[level] = wrapper(log[level])
+      log[level] = wrapper(log[level], level)
     )
     log.add(new winston.transports.Console({
       format: winston.format.simple()

src/start.js

@@ -141,8 +141,6 @@ async function createWindow () {
     isLaunching = false
   }
 
-  log.info('isFirstLaunch', isFirstLaunch)
-
   if (isFirstLaunch) {
     dialog.showMessageBox({
       type: 'info',