Enhance sanitization logic w/ base common cases (#18639)

## Description

This change intends to prevent secrets getting leaked into the logs.
Modifications were made to sanitization formatter, so that known cases
were sensitive data is leaked gets formatted straight away, and that
sanitization logic is performed and consolidated inside the telemetry
library. This common cases are also performed before searching for
unknown cases when the sanitization is enabled.

In the future, the plan is to monitor for new detected cases, and add
them to the common cases when detected (which are always enabled and
performed in an efficient way).

Author: jfmendez11

server/routerlicious/packages/services-telemetry/src/index.ts

@@ -40,4 +40,7 @@ export {
 	getGlobalTelemetryContext,
 	setGlobalTelemetryContext,
 } from "./telemetryContext";
-export { SanitizationLumberFormatter } from "./sanitizationLumberFormatter";
+export {
+	SanitizationLumberFormatter,
+	BaseSanitizationLumberFormatter,
+} from "./sanitizationLumberFormatter";

server/routerlicious/packages/services-telemetry/src/lumberjack.ts

@@ -14,7 +14,10 @@ import {
 	ILumberFormatter,
 } from "./resources";
 import { getGlobal, getGlobalTelemetryContext } from "./telemetryContext";
-import { SanitizationLumberFormatter } from "./sanitizationLumberFormatter";
+import {
+	BaseSanitizationLumberFormatter,
+	SanitizationLumberFormatter,
+} from "./sanitizationLumberFormatter";
 
 /**
  * @internal
@@ -170,6 +173,8 @@ export class Lumberjack {
 		const lumberFormatters: ILumberFormatter[] = [];
 		if (this._options.enableSanitization) {
 			lumberFormatters.push(new SanitizationLumberFormatter());
+		} else {
+			lumberFormatters.push(new BaseSanitizationLumberFormatter());
 		}
 		this._formatters = lumberFormatters;
 
@@ -187,6 +192,7 @@ export class Lumberjack {
 			this._engineList,
 			this._schemaValidators,
 			properties,
+			this._formatters,
 		);
 	}
 

server/routerlicious/packages/services-telemetry/src/lumberjackCommonTestUtils.ts

@@ -57,15 +57,25 @@ export class TestFormatter implements ILumberFormatter {
 	public transform(lumber: Lumber) {}
 }
 
+/**
+ * @internal
+ */
 export class TestSensitiveException extends Error {
 	public password = "password";
 	public apiKey = "apikey";
 	public sessionId = "sessionid";
 	public cookie = "cookie";
 	public token = "token";
 	public secret = "secret";
+	public authorization = "authorization";
+	public someProperty = {
+		pass: "pass",
+	};
 }
 
+/**
+ * @internal
+ */
 export class TestRegularException extends Error {
 	public field1 = "field1";
 	public field2 = "field2";
@@ -74,3 +84,38 @@ export class TestRegularException extends Error {
 	public field5 = "field5";
 	public field6 = "field6";
 }
+
+/**
+ * @internal
+ */
+export class TestBaseSensitiveException extends Error {
+	public command: any;
+	public request: any;
+	public response: any;
+	public config: any;
+
+	public constructor(message?: string) {
+		super(message);
+
+		this.command = {
+			args: ["arg1", "arg2"],
+		};
+
+		const authorization = "Bearer aFakeToken1234";
+		const Authorization = "Bearer anotherFakeToken1234";
+
+		this.request = {
+			headers: { authorization },
+		};
+
+		this.response = {
+			request: {
+				headers: { Authorization },
+			},
+		};
+
+		this.config = {
+			headers: { Authorization },
+		};
+	}
+}

server/routerlicious/packages/services-telemetry/src/sanitizationLumberFormatter.ts

@@ -9,7 +9,48 @@ import { Lumber } from "./lumber";
 /**
  * @internal
  */
-export class SanitizationLumberFormatter implements ILumberFormatter {
+export class BaseSanitizationLumberFormatter implements ILumberFormatter {
+	readonly redactedStr = "[LUMBER_REDACTED]";
+
+	public transform(lumber: Lumber<string>): void {
+		if (lumber.exception) {
+			this.sanitizeCommonCases(lumber.exception);
+		}
+	}
+
+	private sanitizeCommonCases(error: any): void {
+		if (error?.command?.args) {
+			error.command.args = [this.redactedStr];
+		}
+		this.handleHeadersSanitization(error);
+	}
+
+	private handleHeadersSanitization(error: any): void {
+		if (error?.request?.headers?.authorization || error?.request?.headers?.Authorization) {
+			delete error.request.headers.authorization;
+			delete error.request.headers.Authorization;
+			error.request.headers.authorization = this.redactedStr;
+		}
+		if (
+			error?.response?.request?.headers?.authorization ||
+			error?.response?.request?.headers?.Authorization
+		) {
+			delete error.response.request.headers.authorization;
+			delete error.response.request.headers.Authorization;
+			error.response.request.headers.authorization = this.redactedStr;
+		}
+		if (error?.config?.headers?.authorization || error?.config?.headers?.Authorization) {
+			delete error.config.headers.authorization;
+			delete error.config.headers.Authorization;
+			error.config.headers.authorization = this.redactedStr;
+		}
+	}
+}
+
+/**
+ * @internal
+ */
+export class SanitizationLumberFormatter extends BaseSanitizationLumberFormatter {
 	private readonly sensitiveKeys = [
 		/cookie/i,
 		/passw(or)?d/i,
@@ -19,29 +60,29 @@ export class SanitizationLumberFormatter implements ILumberFormatter {
 		/token/i,
 		/api[._-]?key/i,
 		/session[._-]?id/i,
+		/^auth/i,
 	];
 
-	private readonly redactedStr = "[LUMBER_REDACTED]";
-
 	public transform(lumber: Lumber<string>): void {
+		super.transform(lumber);
+
 		if (lumber.exception) {
 			const sensitiveKeys = new Set<string>();
-
-			this.redactException(lumber.exception, sensitiveKeys);
+			this.redactException(lumber.exception, sensitiveKeys, "");
 
 			if (sensitiveKeys.size > 0) {
 				lumber.setProperty("detectedSensitiveKeys", sensitiveKeys);
 			}
 		}
 	}
 
-	private redactException(exception: Error, sensitiveKeys: Set<string>): void {
-		Object.keys(exception).forEach((keyStr, value) => {
-			if (typeof value === "object" && value !== null) {
-				this.redactException(value, sensitiveKeys);
+	private redactException(exception: Error, sensitiveKeys: Set<string>, keyPath: string): void {
+		Object.keys(exception).forEach((keyStr) => {
+			if (typeof exception[keyStr] === "object" && exception[keyStr] !== null) {
+				this.redactException(exception[keyStr], sensitiveKeys, `${keyPath}.${keyStr}`);
 			} else if (this.sensitiveKeys.some((regex) => regex.test(keyStr))) {
 				exception[keyStr] = this.redactedStr;
-				sensitiveKeys.add(keyStr);
+				sensitiveKeys.add(`${keyPath}.${keyStr}`);
 			}
 		});
 	}

server/routerlicious/packages/services-telemetry/src/test/baseSanitizationLumberFormatter.spec.ts

@@ -0,0 +1,53 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import assert from "assert";
+import { Lumber } from "../lumber";
+import { LumberEventName } from "../lumberEventNames";
+import * as resources from "../resources";
+import {
+	TestEngine1,
+	TestRegularException,
+	TestBaseSensitiveException,
+} from "../lumberjackCommonTestUtils";
+import { BaseSanitizationLumberFormatter } from "../sanitizationLumberFormatter";
+
+describe("BaseSanitizationLumberFormatter", () => {
+	it("Formatter goes through an exception without sensitive data and doesn't modify it.", () => {
+		const errorMessage = "ErrorMessage";
+		const engine = new TestEngine1();
+		const regularException = new TestRegularException();
+		const formatter = new BaseSanitizationLumberFormatter();
+		const lumber = new Lumber(LumberEventName.UnitTestEvent, resources.LumberType.Metric, [
+			engine,
+		]);
+
+		lumber.error(errorMessage, regularException);
+		formatter.transform(lumber);
+
+		assert.strictEqual(lumber.exception, regularException);
+		assert.strictEqual(lumber.properties.size, 0);
+	});
+
+	it("Formatter goes through an exception with sensitive data and modifies it.", () => {
+		const redactedStr = "[LUMBER_REDACTED]";
+		const errorMessage = "ErrorMessage";
+		const engine = new TestEngine1();
+		const sensitiveException = new TestBaseSensitiveException();
+		const formatter = new BaseSanitizationLumberFormatter();
+		const lumber = new Lumber(LumberEventName.UnitTestEvent, resources.LumberType.Metric, [
+			engine,
+		]);
+
+		lumber.error(errorMessage, sensitiveException);
+		formatter.transform(lumber);
+
+		const baseLumberException = lumber.exception as TestBaseSensitiveException;
+
+		assert.strictEqual(baseLumberException.command.args[0], redactedStr);
+		assert.strictEqual(baseLumberException.request.headers.authorization, redactedStr);
+		assert.strictEqual(baseLumberException.response.request.headers.authorization, redactedStr);
+	});
+});

server/routerlicious/packages/services-telemetry/src/test/sanitizationLumberFormatter.spec.ts

@@ -53,13 +53,17 @@ describe("SanitizationLumberFormatter", () => {
 		assert.strictEqual(lumberException.cookie, redactedStr);
 		assert.strictEqual(lumberException.token, redactedStr);
 		assert.strictEqual(lumberException.secret, redactedStr);
+		assert.strictEqual(lumberException.authorization, redactedStr);
+		assert.strictEqual(lumberException.someProperty.pass, redactedStr);
 
-		assert.strictEqual(sensitiveKeys.size, 6);
-		assert.strictEqual(sensitiveKeys.has("password"), true);
-		assert.strictEqual(sensitiveKeys.has("apiKey"), true);
-		assert.strictEqual(sensitiveKeys.has("sessionId"), true);
-		assert.strictEqual(sensitiveKeys.has("cookie"), true);
-		assert.strictEqual(sensitiveKeys.has("token"), true);
-		assert.strictEqual(sensitiveKeys.has("secret"), true);
+		assert.strictEqual(sensitiveKeys.size, 8);
+		assert.strictEqual(sensitiveKeys.has(".password"), true);
+		assert.strictEqual(sensitiveKeys.has(".apiKey"), true);
+		assert.strictEqual(sensitiveKeys.has(".sessionId"), true);
+		assert.strictEqual(sensitiveKeys.has(".cookie"), true);
+		assert.strictEqual(sensitiveKeys.has(".token"), true);
+		assert.strictEqual(sensitiveKeys.has(".secret"), true);
+		assert.strictEqual(sensitiveKeys.has(".authorization"), true);
+		assert.strictEqual(sensitiveKeys.has(".someProperty.pass"), true);
 	});
 });

server/routerlicious/packages/services-utils/src/auth.ts

@@ -165,15 +165,6 @@ function getTokenFromRequest(request: Request): string {
 
 const defaultMaxTokenLifetimeSec = 60 * 60; // 1 hour
 
-// Used to sanitize Redis error object and remove sensitive information
-function sanitizeError(error: any) {
-	if (error?.command?.args) {
-		error.command.args = ["REDACTED"];
-	}
-	// eslint-disable-next-line @typescript-eslint/no-unsafe-return
-	return error;
-}
-
 /**
  * @internal
  */
@@ -219,11 +210,7 @@ export async function verifyToken(
 		// Check token cache first
 		if ((options.enableTokenCache || options.ensureSingleUseToken) && options.tokenCache) {
 			const cachedToken = await options.tokenCache.get(token).catch((error) => {
-				Lumberjack.error(
-					"Unable to retrieve cached JWT",
-					logProperties,
-					sanitizeError(error),
-				);
+				Lumberjack.error("Unable to retrieve cached JWT", logProperties, error);
 				return false;
 			});
 
@@ -249,7 +236,7 @@ export async function verifyToken(
 					tokenLifetimeMs !== undefined ? Math.floor(tokenLifetimeMs / 1000) : undefined,
 				)
 				.catch((error) => {
-					Lumberjack.error("Unable to cache JWT", logProperties, sanitizeError(error));
+					Lumberjack.error("Unable to cache JWT", logProperties, error);
 				});
 		}
 	} catch (error) {