From 3e61a4da9e6849773319f4acf8e745c7eb3cd0dc Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Mon, 13 Sep 2021 12:21:29 +0200 Subject: [PATCH 01/15] backend: add some todos for redacting secrets from logs Co-authored-by: Harry Hogg Signed-off-by: Himanshu Mishra --- packages/backend-common/src/config.ts | 3 ++- .../backend-common/src/logging/rootLogger.ts | 26 +++++++++++++++++++ packages/backend/src/index.ts | 2 ++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/backend-common/src/config.ts b/packages/backend-common/src/config.ts index fa49d5f015..f4cbd9038f 100644 --- a/packages/backend-common/src/config.ts +++ b/packages/backend-common/src/config.ts @@ -21,6 +21,7 @@ import { findPaths } from '@backstage/cli-common'; import { Config, ConfigReader } from '@backstage/config'; import { JsonValue } from '@backstage/types'; import { loadConfig } from '@backstage/config-loader'; +import { setRootLoggerFilteredKeys } from './logging'; export class ObservableConfigProxy implements Config { private config: Config = new ConfigReader({}); @@ -186,6 +187,6 @@ export async function loadBackendConfig(options: { ); config.setConfig(ConfigReader.fromConfigs(configs)); - + setRootLoggerFilteredKeys({ 'secret-1': 'GOATS', 'secret-2': 'SHARKS' }); return config; } diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index d06037863c..ee750da1f6 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -15,11 +15,22 @@ */ import { merge } from 'lodash'; +import { Config } from '@backstage/config'; import * as winston from 'winston'; import { LoggerOptions } from 'winston'; import { coloredFormat } from './formats'; +type FilteredKeys = Record; + let rootLogger: winston.Logger; +let filteredKeys: FilteredKeys; + +/** + { + secret-1: 'integrations.github[0].token', + secrets-2: 'something' + } + */ /** @public */ export function getRootLogger(): winston.Logger { @@ -31,16 +42,31 @@ export function setRootLogger(newLogger: winston.Logger) { rootLogger = newLogger; } +export function setRootLoggerFilteredKeys(_filteredKeys: FilteredKeys) { + filteredKeys = _filteredKeys; +} + /** @public */ export function createRootLogger( options: winston.LoggerOptions = {}, env = process.env, ): winston.Logger { + // TODO(Harry/Himanshu): Get the config schema, filter all the configs with @visibility secret, and pass that to winston so that it can mask it in the logs. https://github.com/winstonjs/winston/issues/1079#issuecomment-382861053 + const logger = winston.createLogger( merge( { level: env.LOG_LEVEL || 'info', format: winston.format.combine( + winston.format(info => { + // TODO(Harry/Himanshu): Iterate over all secrets, and substitute info.message string. Or dynamically create regex from all the secrets and do a one time substitution. + // example: info.message = info.message.replace(new RegExp('abc123', 'g'), "**[Redacted: Config integration.github.token]**"); + // Make sure do it in a case-insensitive way + Object.entries(filteredKeys || {}).forEach(([key, value]) => { + info.message = info.message.replace(new RegExp(key, 'g'), value); + }); + return info; + })(), env.NODE_ENV === 'production' ? winston.format.json() : coloredFormat, ), defaultMeta: { diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index b6c148dfba..b32aa5901e 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -88,6 +88,8 @@ async function main() { }); const createEnv = makeCreateEnv(config); + logger.info('hiiiii secret-1'); + const healthcheckEnv = useHotMemoize(module, () => createEnv('healthcheck')); const catalogEnv = useHotMemoize(module, () => createEnv('catalog')); const codeCoverageEnv = useHotMemoize(module, () => From ee801b5c2b1e207be84baf5d538c88ca957e2168 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Wed, 29 Sep 2021 11:51:21 +0100 Subject: [PATCH 02/15] feat(config): Added a getMap method to return a flattened map of the loaded config Signed-off-by: Harry Hogg --- packages/config/src/reader.ts | 23 +++++++++++++++++++++++ packages/config/src/types.ts | 6 ++++++ 2 files changed, 29 insertions(+) diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index bc8a860c85..0947f8c8ab 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -17,6 +17,7 @@ import { JsonValue, JsonObject } from '@backstage/types'; import { AppConfig, Config } from './types'; import cloneDeep from 'lodash/cloneDeep'; +import merge from 'lodash/merge'; import mergeWith from 'lodash/mergeWith'; // Update the same pattern in config-loader package if this is changed @@ -118,6 +119,28 @@ export class ConfigReader implements Config { return value as T; } + getMap() { + const map: Record = {}; + + const flatten = (data: JsonValue, path = '') => { + if (isObject(data)) { + Object.entries(data).forEach(([key, value]: [string, any]) => + flatten(value, `${path}${path ? '.' : ''}${key}`), + ); + } else if (Array.isArray(data)) { + data.forEach((value, key) => { + flatten(value, `${path}[${key}]`); + }); + } else { + map[path] = data; + } + }; + + flatten(merge({}, this.fallback?.data, this.data)); + + return map; + } + getOptional(key?: string): T | undefined { const value = this.readValue(key); const fallbackValue = this.fallback?.getOptional(key); diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts index a543233277..628178379e 100644 --- a/packages/config/src/types.ts +++ b/packages/config/src/types.ts @@ -65,6 +65,12 @@ export type Config = { */ keys(): string[]; + /** + * Returns a flattened map of the config with the full path to the keys and + * the config value as the value. + */ + getMap(): Record; + /** * Same as `getOptional`, but will throw an error if there's no value for the given key. */ From a2b66f2f4d12fde3e8d2d89fcb1d1f4ead1b6c65 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Wed, 29 Sep 2021 11:52:34 +0100 Subject: [PATCH 03/15] feat(logger): Added redaction filter to the rootLogger Signed-off-by: Harry Hogg --- .../backend-common/src/logging/rootLogger.ts | 47 +++++++++---------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index ee750da1f6..edeb235d4d 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -13,24 +13,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - import { merge } from 'lodash'; -import { Config } from '@backstage/config'; import * as winston from 'winston'; import { LoggerOptions } from 'winston'; import { coloredFormat } from './formats'; -type FilteredKeys = Record; +type RedactionMap = Record; let rootLogger: winston.Logger; -let filteredKeys: FilteredKeys; - -/** - { - secret-1: 'integrations.github[0].token', - secrets-2: 'something' - } - */ +let redactionMap: RedactionMap; /** @public */ export function getRootLogger(): winston.Logger { @@ -42,8 +33,26 @@ export function setRootLogger(newLogger: winston.Logger) { rootLogger = newLogger; } -export function setRootLoggerFilteredKeys(_filteredKeys: FilteredKeys) { - filteredKeys = _filteredKeys; +/** @public */ +export function setRedactionMap(newRedactionMap: RedactionMap) { + redactionMap = newRedactionMap; +} + +/** + * A winston formatting function that finds occurrences of filteredKeys + * and replaces them with the corresponding identifier. + */ +export function redactLogLine(info: winston.Logform.TransformableInfo) { + // TODO(hhogg): The logger is created before the config is loaded, + // because the logger is needed in the config loader. There is a risk of + // a secret being logged out during the config loading stage 🤷‍♂️ + if (redactionMap) { + Object.entries(redactionMap || {}).forEach(([key, value]) => { + info.message = info.message.replace(new RegExp(key, 'g'), `{{${value}}}`); + }); + } + + return info; } /** @public */ @@ -51,22 +60,12 @@ export function createRootLogger( options: winston.LoggerOptions = {}, env = process.env, ): winston.Logger { - // TODO(Harry/Himanshu): Get the config schema, filter all the configs with @visibility secret, and pass that to winston so that it can mask it in the logs. https://github.com/winstonjs/winston/issues/1079#issuecomment-382861053 - const logger = winston.createLogger( merge( { level: env.LOG_LEVEL || 'info', format: winston.format.combine( - winston.format(info => { - // TODO(Harry/Himanshu): Iterate over all secrets, and substitute info.message string. Or dynamically create regex from all the secrets and do a one time substitution. - // example: info.message = info.message.replace(new RegExp('abc123', 'g'), "**[Redacted: Config integration.github.token]**"); - // Make sure do it in a case-insensitive way - Object.entries(filteredKeys || {}).forEach(([key, value]) => { - info.message = info.message.replace(new RegExp(key, 'g'), value); - }); - return info; - })(), + winston.format(redactLogLine)(), env.NODE_ENV === 'production' ? winston.format.json() : coloredFormat, ), defaultMeta: { From 94929707e93f5a7be1f05b1138704deda96be2cf Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Wed, 29 Sep 2021 12:00:25 +0100 Subject: [PATCH 04/15] feat(backend-common): Pass the redaction map from the backend config-loader to the logger Signed-off-by: Harry Hogg --- packages/backend-common/package.json | 1 + packages/backend-common/src/config.ts | 47 +++++++++++++++++++++++---- packages/backend/src/index.ts | 3 +- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 60824f0c40..c0705ae32d 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -36,6 +36,7 @@ "@backstage/integration": "^0.6.7", "@backstage/types": "^0.1.1", "@google-cloud/storage": "^5.8.0", + "@lerna/project": "^4.0.0", "@octokit/rest": "^18.5.3", "@types/cors": "^2.8.6", "@types/dockerode": "^3.2.1", diff --git a/packages/backend-common/src/config.ts b/packages/backend-common/src/config.ts index f4cbd9038f..ca41d8c1bf 100644 --- a/packages/backend-common/src/config.ts +++ b/packages/backend-common/src/config.ts @@ -18,10 +18,40 @@ import { resolve as resolvePath } from 'path'; import parseArgs from 'minimist'; import { Logger } from 'winston'; import { findPaths } from '@backstage/cli-common'; -import { Config, ConfigReader } from '@backstage/config'; -import { JsonValue } from '@backstage/types'; -import { loadConfig } from '@backstage/config-loader'; -import { setRootLoggerFilteredKeys } from './logging'; +import { loadConfigSchema, loadConfig } from '@backstage/config-loader'; +import { AppConfig, Config, ConfigReader, JsonValue } from '@backstage/config'; + +import { setRedactionMap } from './logging'; + +// Fetch the schema and get all the secrets to pass to the rootLogger for redaction +const updateRedactionMap = async (configs: AppConfig[], logger: Logger) => { + // Consider all packages in the monorepo when loading in config + const { Project } = require('@lerna/project'); + const project = new Project(); + const packages = await project.getPackages(); + const localPackageNames = packages.map((p: any) => p.name); + + const schema = await loadConfigSchema({ dependencies: localPackageNames }); + const secretAppConfigs = schema.process(configs, { visibility: ['secret'] }); + const secretConfig = ConfigReader.fromConfigs(secretAppConfigs); + const configMap = secretConfig.getMap(); + + logger.info( + `${ + Object.keys(configMap).length + } secrets found in the config which will be redacted`, + ); + + setRedactionMap( + Object.entries(configMap).reduce>( + (map, [key, value]) => { + map[value] = key; + return map; + }, + {}, + ), + ); +}; export class ObservableConfigProxy implements Config { private config: Config = new ConfigReader({}); @@ -90,6 +120,9 @@ export class ObservableConfigProxy implements Config { get(key?: string): T { return this.select(true).get(key); } + getMap() { + return this.config.getMap(); + } getOptional(key?: string): T | undefined { return this.select(false)?.getOptional(key); } @@ -161,12 +194,13 @@ export async function loadBackendConfig(options: { configRoot: paths.targetRoot, configPaths: configPaths.map(opt => resolvePath(opt)), watch: { - onChange(newConfigs) { + async onChange(newConfigs) { options.logger.info( `Reloaded config from ${newConfigs.map(c => c.context).join(', ')}`, ); config.setConfig(ConfigReader.fromConfigs(newConfigs)); + await updateRedactionMap(configs, options.logger); }, stopSignal: new Promise(resolve => { if (currentCancelFunc) { @@ -187,6 +221,7 @@ export async function loadBackendConfig(options: { ); config.setConfig(ConfigReader.fromConfigs(configs)); - setRootLoggerFilteredKeys({ 'secret-1': 'GOATS', 'secret-2': 'SHARKS' }); + await updateRedactionMap(configs, options.logger); + return config; } diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index b32aa5901e..cda618f4a8 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -86,9 +86,8 @@ async function main() { argv: process.argv, logger, }); - const createEnv = makeCreateEnv(config); - logger.info('hiiiii secret-1'); + const createEnv = makeCreateEnv(config); const healthcheckEnv = useHotMemoize(module, () => createEnv('healthcheck')); const catalogEnv = useHotMemoize(module, () => createEnv('catalog')); From 1be8d2abdbfb0c12ef1dcb199cc2e410b8d9ac5a Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Wed, 29 Sep 2021 12:04:09 +0100 Subject: [PATCH 05/15] docs(changeset): Addded changeset for the sercret redaction Signed-off-by: Harry Hogg --- .changeset/slimy-frogs-allow.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/slimy-frogs-allow.md diff --git a/.changeset/slimy-frogs-allow.md b/.changeset/slimy-frogs-allow.md new file mode 100644 index 0000000000..41cc0119ef --- /dev/null +++ b/.changeset/slimy-frogs-allow.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Any set configurations which have been tagged with a visibility 'secret', are now redacted from log lines. From 54decb6266b644b932fe6e91bca7ef6f7b703d6c Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Wed, 29 Sep 2021 13:47:13 +0100 Subject: [PATCH 06/15] docs(api-report): Updated API documentation Signed-off-by: Harry Hogg --- packages/backend-common/api-report.md | 6 ++++++ packages/backend-common/src/logging/rootLogger.ts | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 2336a57ec3..d38d9a72be 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -472,6 +472,9 @@ export type ReadUrlResponse = { etag?: string; }; +// @public (undocumented) +export type RedactionMap = Record; + // @public export function requestLoggingHandler(logger?: Logger_2): RequestHandler; @@ -539,6 +542,9 @@ export type ServiceBuilder = { start(): Promise; }; +// @public (undocumented) +export function setRedactionMap(newRedactionMap: RedactionMap): void; + // @public (undocumented) export function setRootLogger(newLogger: winston.Logger): void; diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index edeb235d4d..aaf91ff232 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -18,7 +18,8 @@ import * as winston from 'winston'; import { LoggerOptions } from 'winston'; import { coloredFormat } from './formats'; -type RedactionMap = Record; +/** @public */ +export type RedactionMap = Record; let rootLogger: winston.Logger; let redactionMap: RedactionMap; @@ -42,7 +43,7 @@ export function setRedactionMap(newRedactionMap: RedactionMap) { * A winston formatting function that finds occurrences of filteredKeys * and replaces them with the corresponding identifier. */ -export function redactLogLine(info: winston.Logform.TransformableInfo) { +function redactLogLine(info: winston.Logform.TransformableInfo) { // TODO(hhogg): The logger is created before the config is loaded, // because the logger is needed in the config loader. There is a risk of // a secret being logged out during the config loading stage 🤷‍♂️ From 12428bf338deeaa1e32e9bb68092e2f8cf762645 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Thu, 30 Sep 2021 13:31:07 +0100 Subject: [PATCH 07/15] fix(config): Use config get() and find secrets using one big RegExp instead Signed-off-by: Harry Hogg --- packages/backend-common/api-report.md | 5 +- packages/backend-common/src/config.ts | 49 +++++++++---------- .../backend-common/src/logging/rootLogger.ts | 16 +++--- packages/config/src/reader.ts | 23 --------- packages/config/src/types.ts | 6 --- 5 files changed, 30 insertions(+), 69 deletions(-) diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index d38d9a72be..92271f8521 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -472,9 +472,6 @@ export type ReadUrlResponse = { etag?: string; }; -// @public (undocumented) -export type RedactionMap = Record; - // @public export function requestLoggingHandler(logger?: Logger_2): RequestHandler; @@ -543,7 +540,7 @@ export type ServiceBuilder = { }; // @public (undocumented) -export function setRedactionMap(newRedactionMap: RedactionMap): void; +export function setRedactionList(redactionList: string[]): void; // @public (undocumented) export function setRootLogger(newLogger: winston.Logger): void; diff --git a/packages/backend-common/src/config.ts b/packages/backend-common/src/config.ts index ca41d8c1bf..39d0ef1096 100644 --- a/packages/backend-common/src/config.ts +++ b/packages/backend-common/src/config.ts @@ -18,39 +18,36 @@ import { resolve as resolvePath } from 'path'; import parseArgs from 'minimist'; import { Logger } from 'winston'; import { findPaths } from '@backstage/cli-common'; -import { loadConfigSchema, loadConfig } from '@backstage/config-loader'; +import { + loadConfigSchema, + loadConfig, + ConfigSchema, +} from '@backstage/config-loader'; import { AppConfig, Config, ConfigReader, JsonValue } from '@backstage/config'; -import { setRedactionMap } from './logging'; +import { setRedactionList } from './logging'; // Fetch the schema and get all the secrets to pass to the rootLogger for redaction -const updateRedactionMap = async (configs: AppConfig[], logger: Logger) => { - // Consider all packages in the monorepo when loading in config - const { Project } = require('@lerna/project'); - const project = new Project(); - const packages = await project.getPackages(); - const localPackageNames = packages.map((p: any) => p.name); - - const schema = await loadConfigSchema({ dependencies: localPackageNames }); +const updateRedactionMap = ( + schema: ConfigSchema, + configs: AppConfig[], + logger: Logger, +) => { const secretAppConfigs = schema.process(configs, { visibility: ['secret'] }); const secretConfig = ConfigReader.fromConfigs(secretAppConfigs); - const configMap = secretConfig.getMap(); + const values = new Set(); + const data = secretConfig.get(); + + JSON.parse( + JSON.stringify(data), + (_, v) => typeof v === 'string' && values.add(v), + ); logger.info( - `${ - Object.keys(configMap).length - } secrets found in the config which will be redacted`, + `${values.size} secrets found in the config which will be redacted`, ); - setRedactionMap( - Object.entries(configMap).reduce>( - (map, [key, value]) => { - map[value] = key; - return map; - }, - {}, - ), - ); + setRootLoggerRedactionList(Array.from(values)); }; export class ObservableConfigProxy implements Config { @@ -120,9 +117,6 @@ export class ObservableConfigProxy implements Config { get(key?: string): T { return this.select(true).get(key); } - getMap() { - return this.config.getMap(); - } getOptional(key?: string): T | undefined { return this.select(false)?.getOptional(key); } @@ -185,6 +179,9 @@ export async function loadBackendConfig(options: { const args = parseArgs(options.argv); const configPaths: string[] = [args.config ?? []].flat(); + const schema = await loadConfigSchema({ + dependencies: ['@backstage/backend-common'], + }); const config = new ObservableConfigProxy(options.logger); /* eslint-disable-next-line no-restricted-syntax */ diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index aaf91ff232..3691f5cd91 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -19,10 +19,8 @@ import { LoggerOptions } from 'winston'; import { coloredFormat } from './formats'; /** @public */ -export type RedactionMap = Record; - let rootLogger: winston.Logger; -let redactionMap: RedactionMap; +let redactionRegExp: RegExp; /** @public */ export function getRootLogger(): winston.Logger { @@ -35,8 +33,8 @@ export function setRootLogger(newLogger: winston.Logger) { } /** @public */ -export function setRedactionMap(newRedactionMap: RedactionMap) { - redactionMap = newRedactionMap; +export function setRedactionList(redactionList: string[]) { + redactionRegExp = new RegExp(`(${redactionList.join('|')})`, 'g'); } /** @@ -46,11 +44,9 @@ export function setRedactionMap(newRedactionMap: RedactionMap) { function redactLogLine(info: winston.Logform.TransformableInfo) { // TODO(hhogg): The logger is created before the config is loaded, // because the logger is needed in the config loader. There is a risk of - // a secret being logged out during the config loading stage 🤷‍♂️ - if (redactionMap) { - Object.entries(redactionMap || {}).forEach(([key, value]) => { - info.message = info.message.replace(new RegExp(key, 'g'), `{{${value}}}`); - }); + // a secret being logged out during the config loading stage. + if (redactionRegExp) { + info.message = info.message.replace(redactionRegExp, '[REDACTED]'); } return info; diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 0947f8c8ab..bc8a860c85 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -17,7 +17,6 @@ import { JsonValue, JsonObject } from '@backstage/types'; import { AppConfig, Config } from './types'; import cloneDeep from 'lodash/cloneDeep'; -import merge from 'lodash/merge'; import mergeWith from 'lodash/mergeWith'; // Update the same pattern in config-loader package if this is changed @@ -119,28 +118,6 @@ export class ConfigReader implements Config { return value as T; } - getMap() { - const map: Record = {}; - - const flatten = (data: JsonValue, path = '') => { - if (isObject(data)) { - Object.entries(data).forEach(([key, value]: [string, any]) => - flatten(value, `${path}${path ? '.' : ''}${key}`), - ); - } else if (Array.isArray(data)) { - data.forEach((value, key) => { - flatten(value, `${path}[${key}]`); - }); - } else { - map[path] = data; - } - }; - - flatten(merge({}, this.fallback?.data, this.data)); - - return map; - } - getOptional(key?: string): T | undefined { const value = this.readValue(key); const fallbackValue = this.fallback?.getOptional(key); diff --git a/packages/config/src/types.ts b/packages/config/src/types.ts index 628178379e..a543233277 100644 --- a/packages/config/src/types.ts +++ b/packages/config/src/types.ts @@ -65,12 +65,6 @@ export type Config = { */ keys(): string[]; - /** - * Returns a flattened map of the config with the full path to the keys and - * the config value as the value. - */ - getMap(): Record; - /** * Same as `getOptional`, but will throw an error if there's no value for the given key. */ From 18d14d655eba82c981698d0f4f94b96afd739312 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Thu, 30 Sep 2021 13:32:03 +0100 Subject: [PATCH 08/15] fix(config): Subscribe to config changes instead of piggy backing off the loader watch. Signed-off-by: Harry Hogg --- packages/backend-common/src/config.ts | 8 +++++--- packages/backend-common/src/logging/rootLogger.ts | 1 + packages/backend/src/index.ts | 1 - 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/backend-common/src/config.ts b/packages/backend-common/src/config.ts index 39d0ef1096..0e6f993d8f 100644 --- a/packages/backend-common/src/config.ts +++ b/packages/backend-common/src/config.ts @@ -191,13 +191,12 @@ export async function loadBackendConfig(options: { configRoot: paths.targetRoot, configPaths: configPaths.map(opt => resolvePath(opt)), watch: { - async onChange(newConfigs) { + onChange(newConfigs) { options.logger.info( `Reloaded config from ${newConfigs.map(c => c.context).join(', ')}`, ); config.setConfig(ConfigReader.fromConfigs(newConfigs)); - await updateRedactionMap(configs, options.logger); }, stopSignal: new Promise(resolve => { if (currentCancelFunc) { @@ -218,7 +217,10 @@ export async function loadBackendConfig(options: { ); config.setConfig(ConfigReader.fromConfigs(configs)); - await updateRedactionMap(configs, options.logger); + + // Subscribe to config changes and update the redaction list for logging + updateRedactionMap(schema, configs, options.logger); + config.subscribe(() => updateRedactionMap(schema, configs, options.logger)); return config; } diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index 3691f5cd91..114f4b4f48 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { merge } from 'lodash'; import * as winston from 'winston'; import { LoggerOptions } from 'winston'; diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index cda618f4a8..b6c148dfba 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -86,7 +86,6 @@ async function main() { argv: process.argv, logger, }); - const createEnv = makeCreateEnv(config); const healthcheckEnv = useHotMemoize(module, () => createEnv('healthcheck')); From f8a7f5723a849195ad5946dfabe18dac7fcc33d0 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Tue, 5 Oct 2021 12:34:27 +0100 Subject: [PATCH 09/15] Removed added dependency Signed-off-by: Harry Hogg --- packages/backend-common/package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index c0705ae32d..60824f0c40 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -36,7 +36,6 @@ "@backstage/integration": "^0.6.7", "@backstage/types": "^0.1.1", "@google-cloud/storage": "^5.8.0", - "@lerna/project": "^4.0.0", "@octokit/rest": "^18.5.3", "@types/cors": "^2.8.6", "@types/dockerode": "^3.2.1", From 3c10980ec5a4052ad15b771395fafb4ef1e7253d Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Tue, 5 Oct 2021 15:17:32 +0100 Subject: [PATCH 10/15] test(rootLogger): Redaction formatting Signed-off-by: Harry Hogg --- .../src/logging/rootLogger.test.ts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/backend-common/src/logging/rootLogger.test.ts b/packages/backend-common/src/logging/rootLogger.test.ts index 50cf867c85..f24a757155 100644 --- a/packages/backend-common/src/logging/rootLogger.test.ts +++ b/packages/backend-common/src/logging/rootLogger.test.ts @@ -15,7 +15,12 @@ */ import * as winston from 'winston'; -import { createRootLogger, getRootLogger, setRootLogger } from './rootLogger'; +import { + createRootLogger, + getRootLogger, + setRootLogger, + setRedactionList, +} from './rootLogger'; describe('rootLogger', () => { it('can replace the default logger', () => { @@ -30,6 +35,19 @@ describe('rootLogger', () => { ); }); + it('redacts given secrets', () => { + const logger = createRootLogger(); + jest.spyOn(logger, 'write'); + setRedactionList(['SECRET_1', 'SECRET_2']); + logger.info('Logging SECRET_1 and SECRET_2 but not SECRET_3'); + + expect(logger.write).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Logging [REDACTED] and [REDACTED] but not SECRET_3', + }), + ); + }); + describe('createRootLoger', () => { it('creates a new logger', () => { const oldLogger = getRootLogger(); From a9025f70ba765fc85a4af0c4d50b925d594ef898 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Thu, 14 Oct 2021 13:30:43 +0100 Subject: [PATCH 11/15] fix(rootLogger): Added util for escaping a regular expression Signed-off-by: Harry Hogg --- .../src/util/escapeRegExp.test.ts | 80 +++++++++++++++++++ .../backend-common/src/util/escapeRegExp.ts | 24 ++++++ 2 files changed, 104 insertions(+) create mode 100644 packages/backend-common/src/util/escapeRegExp.test.ts create mode 100644 packages/backend-common/src/util/escapeRegExp.ts diff --git a/packages/backend-common/src/util/escapeRegExp.test.ts b/packages/backend-common/src/util/escapeRegExp.test.ts new file mode 100644 index 0000000000..60b12739b3 --- /dev/null +++ b/packages/backend-common/src/util/escapeRegExp.test.ts @@ -0,0 +1,80 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { escapeRegExp } from './escapeRegExp'; + +describe('escapeRegExp', () => { + test('all the characters', () => { + expect(escapeRegExp('^$\\.*+?()[]{}|')).toBe( + '\\^\\$\\\\\\.\\*\\+\\?\\(\\)\\[\\]\\{\\}\\|', + ); + }); + + test('character: ^', () => { + expect(escapeRegExp('^')).toBe('\\^'); + }); + + test('character: $', () => { + expect(escapeRegExp('$')).toBe('\\$'); + }); + + test('character: \\', () => { + expect(escapeRegExp('\\')).toBe('\\\\'); + }); + + test('character: .', () => { + expect(escapeRegExp('.')).toBe('\\.'); + }); + + test('character: *', () => { + expect(escapeRegExp('*')).toBe('\\*'); + }); + + test('character: +', () => { + expect(escapeRegExp('+')).toBe('\\+'); + }); + + test('character: ?', () => { + expect(escapeRegExp('?')).toBe('\\?'); + }); + + test('character: (', () => { + expect(escapeRegExp('(')).toBe('\\('); + }); + + test('character: )', () => { + expect(escapeRegExp(')')).toBe('\\)'); + }); + + test('character: [', () => { + expect(escapeRegExp('[')).toBe('\\['); + }); + + test('character: ]', () => { + expect(escapeRegExp(']')).toBe('\\]'); + }); + + test('character: {', () => { + expect(escapeRegExp('{')).toBe('\\{'); + }); + + test('character: }', () => { + expect(escapeRegExp('}')).toBe('\\}'); + }); + + test('character: |', () => { + expect(escapeRegExp('|')).toBe('\\|'); + }); +}); diff --git a/packages/backend-common/src/util/escapeRegExp.ts b/packages/backend-common/src/util/escapeRegExp.ts new file mode 100644 index 0000000000..58a8d2da15 --- /dev/null +++ b/packages/backend-common/src/util/escapeRegExp.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Escapes a given string to be used inside a RegExp. + * + * Taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions + */ +export const escapeRegExp = (text: string) => { + return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +}; From 488e6145cbe04ac0a7ec8708b26615a0fe9a6ff0 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Thu, 14 Oct 2021 13:33:46 +0100 Subject: [PATCH 12/15] fix(rootLogger): Renamed to setRootLoggerRedactionList and do not expose publically Signed-off-by: Harry Hogg --- packages/backend-common/api-report.md | 3 --- packages/backend-common/src/config.ts | 11 ++++++----- packages/backend-common/src/logging/index.ts | 2 +- .../backend-common/src/logging/rootLogger.test.ts | 10 +++++----- packages/backend-common/src/logging/rootLogger.ts | 10 ++++++---- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 92271f8521..2336a57ec3 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -539,9 +539,6 @@ export type ServiceBuilder = { start(): Promise; }; -// @public (undocumented) -export function setRedactionList(redactionList: string[]): void; - // @public (undocumented) export function setRootLogger(newLogger: winston.Logger): void; diff --git a/packages/backend-common/src/config.ts b/packages/backend-common/src/config.ts index 0e6f993d8f..7a79d1df38 100644 --- a/packages/backend-common/src/config.ts +++ b/packages/backend-common/src/config.ts @@ -23,12 +23,13 @@ import { loadConfig, ConfigSchema, } from '@backstage/config-loader'; -import { AppConfig, Config, ConfigReader, JsonValue } from '@backstage/config'; +import { AppConfig, Config, ConfigReader } from '@backstage/config'; +import { JsonValue } from '@backstage/types'; -import { setRedactionList } from './logging'; +import { setRootLoggerRedactionList } from './logging/rootLogger'; // Fetch the schema and get all the secrets to pass to the rootLogger for redaction -const updateRedactionMap = ( +const updateRedactionList = ( schema: ConfigSchema, configs: AppConfig[], logger: Logger, @@ -219,8 +220,8 @@ export async function loadBackendConfig(options: { config.setConfig(ConfigReader.fromConfigs(configs)); // Subscribe to config changes and update the redaction list for logging - updateRedactionMap(schema, configs, options.logger); - config.subscribe(() => updateRedactionMap(schema, configs, options.logger)); + updateRedactionList(schema, configs, options.logger); + config.subscribe(() => updateRedactionList(schema, configs, options.logger)); return config; } diff --git a/packages/backend-common/src/logging/index.ts b/packages/backend-common/src/logging/index.ts index 71e9618f0c..f50114a9ae 100644 --- a/packages/backend-common/src/logging/index.ts +++ b/packages/backend-common/src/logging/index.ts @@ -15,5 +15,5 @@ */ export * from './formats'; -export * from './rootLogger'; +export { createRootLogger, getRootLogger, setRootLogger } from './rootLogger'; export * from './voidLogger'; diff --git a/packages/backend-common/src/logging/rootLogger.test.ts b/packages/backend-common/src/logging/rootLogger.test.ts index f24a757155..192decb653 100644 --- a/packages/backend-common/src/logging/rootLogger.test.ts +++ b/packages/backend-common/src/logging/rootLogger.test.ts @@ -19,7 +19,7 @@ import { createRootLogger, getRootLogger, setRootLogger, - setRedactionList, + setRootLoggerRedactionList, } from './rootLogger'; describe('rootLogger', () => { @@ -38,17 +38,17 @@ describe('rootLogger', () => { it('redacts given secrets', () => { const logger = createRootLogger(); jest.spyOn(logger, 'write'); - setRedactionList(['SECRET_1', 'SECRET_2']); - logger.info('Logging SECRET_1 and SECRET_2 but not SECRET_3'); + setRootLoggerRedactionList(['SECRET-1', 'SECRET_2', 'SECRET.3']); + logger.info('Logging SECRET-1 and SECRET_2 and SECRET.3'); expect(logger.write).toHaveBeenCalledWith( expect.objectContaining({ - message: 'Logging [REDACTED] and [REDACTED] but not SECRET_3', + message: 'Logging [REDACTED] and [REDACTED] and [REDACTED]', }), ); }); - describe('createRootLoger', () => { + describe('createRootLogger', () => { it('creates a new logger', () => { const oldLogger = getRootLogger(); const newLogger = createRootLogger(); diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index 114f4b4f48..766746fd3c 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -18,8 +18,8 @@ import { merge } from 'lodash'; import * as winston from 'winston'; import { LoggerOptions } from 'winston'; import { coloredFormat } from './formats'; +import { escapeRegExp } from '../util/escapeRegExp'; -/** @public */ let rootLogger: winston.Logger; let redactionRegExp: RegExp; @@ -33,9 +33,11 @@ export function setRootLogger(newLogger: winston.Logger) { rootLogger = newLogger; } -/** @public */ -export function setRedactionList(redactionList: string[]) { - redactionRegExp = new RegExp(`(${redactionList.join('|')})`, 'g'); +export function setRootLoggerRedactionList(redactionList: string[]) { + redactionRegExp = new RegExp( + `(${redactionList.map(escapeRegExp).join('|')})`, + 'g', + ); } /** From deaec1ee3fe5c55b8fdc0b4a7a3c248416d700ac Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Mon, 18 Oct 2021 10:59:21 +0100 Subject: [PATCH 13/15] fix(rootLogger): Only set a regex if there are redactions given, otherwise everything gets redacted Signed-off-by: Harry Hogg --- packages/backend-common/src/logging/rootLogger.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index 766746fd3c..b05763a42c 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -34,10 +34,12 @@ export function setRootLogger(newLogger: winston.Logger) { } export function setRootLoggerRedactionList(redactionList: string[]) { - redactionRegExp = new RegExp( - `(${redactionList.map(escapeRegExp).join('|')})`, - 'g', - ); + if (redactionList.length) { + redactionRegExp = new RegExp( + `(${redactionList.map(escapeRegExp).join('|')})`, + 'g', + ); + } } /** From 4b92638259ff7a4a0a39f3e51a81254f151609b1 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Mon, 18 Oct 2021 11:00:01 +0100 Subject: [PATCH 14/15] test(escapeRegExp): Just an extra test to make sure non-regexy strings don't get escaped Signed-off-by: Harry Hogg --- packages/backend-common/src/util/escapeRegExp.test.ts | 4 ++++ packages/backend-common/src/util/escapeRegExp.ts | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/backend-common/src/util/escapeRegExp.test.ts b/packages/backend-common/src/util/escapeRegExp.test.ts index 60b12739b3..13c6ae4a5a 100644 --- a/packages/backend-common/src/util/escapeRegExp.test.ts +++ b/packages/backend-common/src/util/escapeRegExp.test.ts @@ -16,6 +16,10 @@ import { escapeRegExp } from './escapeRegExp'; describe('escapeRegExp', () => { + test('does not escape non-regex characters', () => { + expect(escapeRegExp('Backstage Backstage')).toBe('Backstage Backstage'); + }); + test('all the characters', () => { expect(escapeRegExp('^$\\.*+?()[]{}|')).toBe( '\\^\\$\\\\\\.\\*\\+\\?\\(\\)\\[\\]\\{\\}\\|', diff --git a/packages/backend-common/src/util/escapeRegExp.ts b/packages/backend-common/src/util/escapeRegExp.ts index 58a8d2da15..bc78967ebe 100644 --- a/packages/backend-common/src/util/escapeRegExp.ts +++ b/packages/backend-common/src/util/escapeRegExp.ts @@ -20,5 +20,5 @@ * Taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions */ export const escapeRegExp = (text: string) => { - return text.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return text.replace(/[.*+?^${}(\)|[\]\\]/g, '\\$&'); }; From f879e8b2b8f5736dc10e42d4aa36bcad95094a7c Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Mon, 18 Oct 2021 11:43:22 +0100 Subject: [PATCH 15/15] fix(backend): Load all schemas from monorepo packages Signed-off-by: Harry Hogg --- packages/backend-common/package.json | 1 + packages/backend-common/src/config.ts | 16 +++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 60824f0c40..c0705ae32d 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -36,6 +36,7 @@ "@backstage/integration": "^0.6.7", "@backstage/types": "^0.1.1", "@google-cloud/storage": "^5.8.0", + "@lerna/project": "^4.0.0", "@octokit/rest": "^18.5.3", "@types/cors": "^2.8.6", "@types/dockerode": "^3.2.1", diff --git a/packages/backend-common/src/config.ts b/packages/backend-common/src/config.ts index 7a79d1df38..cfb655cbeb 100644 --- a/packages/backend-common/src/config.ts +++ b/packages/backend-common/src/config.ts @@ -180,14 +180,20 @@ export async function loadBackendConfig(options: { const args = parseArgs(options.argv); const configPaths: string[] = [args.config ?? []].flat(); - const schema = await loadConfigSchema({ - dependencies: ['@backstage/backend-common'], - }); - const config = new ObservableConfigProxy(options.logger); - /* eslint-disable-next-line no-restricted-syntax */ const paths = findPaths(__dirname); + // TODO(hhogg): This is fetching _all_ of the packages of the monorepo + // in order to find the secrets for redactions, however we only care about + // the backend ones, we need to find a way to exclude the frontend packages. + const { Project } = require('@lerna/project'); + const project = new Project(paths.targetDir); + const packages = await project.getPackages(); + const schema = await loadConfigSchema({ + dependencies: packages.map((p: any) => p.name), + }); + + const config = new ObservableConfigProxy(options.logger); const configs = await loadConfig({ configRoot: paths.targetRoot, configPaths: configPaths.map(opt => resolvePath(opt)),