From ce72766c9615a2b22920ac4e6de6d6ec0e539657 Mon Sep 17 00:00:00 2001 From: Rob McVey Date: Mon, 4 Mar 2019 16:06:19 -0800 Subject: [PATCH] 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 + package.json | 7 ++++--- practices/policy.yaml | 10 +++++----- server.js | 36 +++++++++++++++++++++++++----------- sources/darwin/file-vault.sh | 2 +- src/Action.js | 8 ++++++-- src/App.js | 26 +++++++++----------------- src/Device.js | 11 ++++++----- src/ErrorBoundary.js | 11 +++++++++-- src/lib/logger.js | 20 ++++++++++++++++---- src/start.js | 2 -- 11 files changed, 82 insertions(+), 52 deletions(-) diff --git a/.npmrc b/.npmrc index 214c29d1..43396fe6 100644 --- a/.npmrc +++ b/.npmrc @@ -1 +1,2 @@ registry=https://registry.npmjs.org/ +scripts-prepend-node-path=true diff --git a/package.json b/package.json index 1c7d7358..2c8a2455 100644 --- a/package.json +++ b/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", diff --git a/practices/policy.yaml b/practices/policy.yaml index aebcdb35..e4f1ca27 100644 --- a/practices/policy.yaml +++ b/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" diff --git a/server.js b/server.js index ba5a6223..0b038780 100644 --- a/server.js +++ b/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 }) }) }) diff --git a/sources/darwin/file-vault.sh b/sources/darwin/file-vault.sh index c9c59055..81c7c98b 100644 --- a/sources/darwin/file-vault.sh +++ b/sources/darwin/file-vault.sh @@ -1,3 +1,3 @@ #!/usr/bin/env kmd -exec fdesetup isactive +exec /usr/bin/fdesetup isactive save disks.fileVaultEnabled diff --git a/src/Action.js b/src/Action.js index e8b6f377..d686d822 100644 --- a/src/Action.js +++ b/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 ( -
  • { this.el = el }}> +
  • { this.el = el }} + > 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 () { diff --git a/src/Device.js b/src/Device.js index fe3abb3c..3fabea89 100644 --- a/src/Device.js +++ b/src/Device.js @@ -71,16 +71,17 @@ class Device extends Component { if (hasResults) { return ( - -
      + +
        {results.map(({ name, url, status, description }) => { const iconProps = status === 'PASS' ? { name: 'checkmark', color: '#bbd8ca' } : { name: 'blocked', color: '#a94442' } + return (
      • @@ -106,7 +107,7 @@ class Device extends Component { ) } else { return ( - + ) } }) @@ -178,7 +179,7 @@ class Device extends Component {

        {org} {this.props.strings.policyDescription}

        -
          +
            { this.actions(device.critical, 'critical', device) } { this.actions(device.suggested, 'suggested', device) } { this.actions(device.done, 'done', device) } diff --git a/src/ErrorBoundary.js b/src/ErrorBoundary.js index 16678af8..94cc4eec 100644 --- a/src/ErrorBoundary.js +++ b/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

            Something went wrong.

            + return ( + +

            Something went wrong.

            +
            +            {JSON.stringify(this.state.error, null, 3)}
            +          
            +
            + ) } return this.props.children } diff --git a/src/lib/logger.js b/src/lib/logger.js index 5e52c256..9dd43d1a 100644 --- a/src/lib/logger.js +++ b/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() diff --git a/src/start.js b/src/start.js index c120906a..dae3e5c0 100644 --- a/src/start.js +++ b/src/start.js @@ -141,8 +141,6 @@ async function createWindow () { isLaunching = false } - log.info('isFirstLaunch', isFirstLaunch) - if (isFirstLaunch) { dialog.showMessageBox({ type: 'info',