From b6bdf870a6271b4efb2c92269c7533e648b0db44 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Sun, 23 Jun 2019 23:31:02 +0200 Subject: [PATCH 1/2] fix(cli): error log could crash if error details contained circular refs --- garden-service/package-lock.json | 11 +++++++++++ garden-service/package.json | 2 ++ garden-service/src/events.ts | 3 +-- garden-service/src/logger/renderers.ts | 13 +++++-------- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/garden-service/package-lock.json b/garden-service/package-lock.json index 8e40466cc5..decdb4d05b 100644 --- a/garden-service/package-lock.json +++ b/garden-service/package-lock.json @@ -492,6 +492,12 @@ "integrity": "sha512-5R2/MHILQLDCzTuhs1j4Qqq8AaKUf7Ma4KSSkCtc12+fMs47zfa34qhto9goxpyX00tQK1zxB885VCiawZ5Qhg==", "dev": true }, + "@types/circular-json": { + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/@types/circular-json/-/circular-json-0.4.0.tgz", + "integrity": "sha512-7+kYB7x5a7nFWW1YPBh3KxhwKfiaI4PbZ1RvzBU91LZy7lWJO822CI+pqzSre/DZ7KsCuMKdHnLHHFu8AyXbQg==", + "dev": true + }, "@types/commander": { "version": "2.12.2", "resolved": "https://registry.npmjs.org/@types/commander/-/commander-2.12.2.tgz", @@ -2093,6 +2099,11 @@ "resolved": "https://registry.npmjs.org/ci-info/-/ci-info-2.0.0.tgz", "integrity": "sha512-5tK7EtrZ0N+OLFMthtqOj4fI2Jeb88C4CAZPu25LDVUgXJ0A3Js4PMGqrn0JU1W0Mh1/Z8wZzYPxqUrXeBboCQ==" }, + "circular-json": { + "version": "0.5.9", + "resolved": "https://registry.npmjs.org/circular-json/-/circular-json-0.5.9.tgz", + "integrity": "sha512-4ivwqHpIFJZBuhN3g/pEcdbnGUywkBblloGbkglyloVjjR3uT6tieI89MVOfbP2tHX5sgb01FuLgAOzebNlJNQ==" + }, "class-utils": { "version": "0.3.6", "resolved": "https://registry.npmjs.org/class-utils/-/class-utils-0.3.6.tgz", diff --git a/garden-service/package.json b/garden-service/package.json index 96de6e384a..74dfd34289 100644 --- a/garden-service/package.json +++ b/garden-service/package.json @@ -36,6 +36,7 @@ "child-process-promise": "^2.2.1", "chokidar": "^3.0.0", "ci-info": "^2.0.0", + "circular-json": "^0.5.9", "cli-cursor": "^3.0.0", "cli-highlight": "^2.1.1", "cli-table3": "^0.5.1", @@ -112,6 +113,7 @@ "@types/bluebird": "^3.5.26", "@types/chai": "^4.1.7", "@types/ci-info": "^2.0.0", + "@types/circular-json": "^0.4.0", "@types/cross-spawn": "^6.0.0", "@types/dedent": "^0.7.0", "@types/deep-diff": "1.0.0", diff --git a/garden-service/src/events.ts b/garden-service/src/events.ts index 100aca1370..6122515b69 100644 --- a/garden-service/src/events.ts +++ b/garden-service/src/events.ts @@ -10,7 +10,6 @@ import { EventEmitter2 } from "eventemitter2" import { TaskResult } from "./task-graph" import { ModuleVersion } from "./vcs/vcs" import { LogEntry } from "./logger/log-entry" -import { isObject } from "util" /** * This simple class serves as the central event bus for a Garden instance. Its function @@ -28,7 +27,7 @@ export class EventBus extends EventEmitter2 { } emit(name: T, payload: Events[T]) { - this.log.silly(`Emit event '${name}' with payload: ${isObject(payload) ? JSON.stringify(payload) : payload}`) + this.log.silly(`Emit event '${name}'`) return super.emit(name, payload) } diff --git a/garden-service/src/logger/renderers.ts b/garden-service/src/logger/renderers.ts index ca7ebf0d6e..9986f8475b 100644 --- a/garden-service/src/logger/renderers.ts +++ b/garden-service/src/logger/renderers.ts @@ -11,13 +11,12 @@ import * as nodeEmoji from "node-emoji" import * as yaml from "js-yaml" import chalk from "chalk" import stripAnsi from "strip-ansi" +import * as CircularJSON from "circular-json" import { curryRight, flow, isArray, isEmpty, - reduce, - kebabCase, repeat, } from "lodash" import cliTruncate = require("cli-truncate") @@ -90,14 +89,12 @@ export function renderError(entry: LogEntry) { if (error) { const { detail, message, stack } = error let out = stack || message - if (!isEmpty(detail)) { - const kebabCasedDetail = reduce(detail, (acc, val, key) => { - acc[kebabCase(key)] = val - return acc - }, {}) + const sanitized = JSON.parse(CircularJSON.stringify(detail)) + + if (!isEmpty(detail)) { try { - const yamlDetail = yaml.safeDump(kebabCasedDetail, { noRefs: true, skipInvalid: true }) + const yamlDetail = yaml.safeDump(sanitized, { noRefs: true, skipInvalid: true }) out += `\nError Details:\n${yamlDetail}` } catch (err) { out += `\nUnable to render error details:\n${err.message}` From 5e02c5dfb4e82679f5c9ad8a13c2352de1a29aa2 Mon Sep 17 00:00:00 2001 From: Thorarinn Sigurdsson Date: Mon, 24 Jun 2019 18:40:32 +0200 Subject: [PATCH 2/2] fix(cli): don't log internal fields in error detail --- garden-service/src/logger/renderers.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/garden-service/src/logger/renderers.ts b/garden-service/src/logger/renderers.ts index 9986f8475b..caec644e19 100644 --- a/garden-service/src/logger/renderers.ts +++ b/garden-service/src/logger/renderers.ts @@ -25,7 +25,8 @@ import hasAnsi = require("has-ansi") import { LogEntry, EmojiName } from "./log-entry" import { JsonLogEntry } from "./writers/json-terminal-writer" -import { highlightYaml } from "../util/util" +import { highlightYaml, deepFilter } from "../util/util" +import { isNumber } from "util" export type ToRender = string | ((...args: any[]) => string) export type Renderer = [ToRender, any[]] | ToRender[] @@ -90,10 +91,14 @@ export function renderError(entry: LogEntry) { const { detail, message, stack } = error let out = stack || message - const sanitized = JSON.parse(CircularJSON.stringify(detail)) + // We recursively filter out internal fields (i.e. having names starting with _). + const filteredDetail = deepFilter(detail, (_, key: string | number) => { + return isNumber(key) || !key.startsWith("_") + }) - if (!isEmpty(detail)) { + if (!isEmpty(filteredDetail)) { try { + const sanitized = JSON.parse(CircularJSON.stringify(filteredDetail)) const yamlDetail = yaml.safeDump(sanitized, { noRefs: true, skipInvalid: true }) out += `\nError Details:\n${yamlDetail}` } catch (err) {