✨ DOP-4240 implements filtering sensitive values method to prevent se…

…nsitive values being logged (#1012) * ✨ DOP-4240 implements filtering sensitive values method to prevent sensitive values being logged * ✅ DOP-4240 adds test for the redacted logic from the logger * ✏️ DOP-4240 fixed type "having" to "have" --------- Co-authored-by: Caesar Bell <[email protected]>
mongodb · Mar 12, 2024 · 6800a4f · 6800a4f
1 parent 84ea9ca
commit 6800a4f
Show file tree

Hide file tree

Showing 5 changed files with 136 additions and 92 deletions.
diff --git a/cdk-infra/utils/ssm.ts b/cdk-infra/utils/ssm.ts
@@ -1,82 +1,16 @@
-import { GetParameterCommand, SSMClient } from '@aws-sdk/client-ssm';
 import { getEnv } from './env';
+import {
+  workerSecureStrings,
+  webhookSecureStrings,
+  getSecureStrings,
+} from '../../src/enhanced/utils/get-sensitive-values';
 
 export function getSsmPathPrefix(): string {
   const env = getEnv();
 
   return `/env/${env}/docs/worker_pool`;
 }
 
-/**
- * Returns the secure strings from SSM using the SSM client.
- * @param ssmPrefix the path prefix that should contain the environment for the SSM strings (e.g. /env/dotcomstg/worker_pool/)
- * @returns The map of environment variables.
- */
-async function getSecureStrings(
-  ssmPrefix: string,
-  secureStrings: readonly string[],
-  paramToEnvMap: Map<string, string>,
-  resourceName: string
-) {
-  const ssmClient = new SSMClient({ region: process.env.CDK_DEFAULT_REGION });
-
-  const secureStringsMap: Record<string, string> = {};
-
-  await Promise.all(
-    secureStrings.map(async (paramName: string) => {
-      const getParamCommand = new GetParameterCommand({
-        Name: `${ssmPrefix}${paramName}`,
-        WithDecryption: true,
-      });
-
-      const ssmResponse = await ssmClient.send(getParamCommand);
-      const secureString = ssmResponse.Parameter?.Value;
-
-      if (!secureString) {
-        console.error(`ERROR! Could not retrieve string for the following param: ${paramName}`);
-        return;
-      }
-
-      const envName = paramToEnvMap.get(paramName);
-
-      if (!envName) {
-        console.error(
-          `ERROR! The param '${paramName}' does not having a mapping to an environment variable name. Please define this in the ${resourceName} map.`
-        );
-        return;
-      }
-
-      secureStringsMap[envName] = secureString;
-    })
-  );
-
-  return secureStringsMap;
-}
-
-// This array contains the Parameter Store paths of the secure strings
-// we want to add to the worker environment. These are mapped
-// to their environment variable name in the workerParamPathToEnvName map.
-const workerSecureStrings = [
-  '/npm/auth',
-  '/github/webhook/secret',
-  '/github/bot/password',
-  '/atlas/password',
-  '/fastly/docs/dochub/token',
-  '/npm/auth',
-  '/fastly/docs/dochub/service_id',
-  '/fastly/dochub_map',
-  '/fastly/docs/main/token',
-  '/fastly/docs/main/service_id',
-  '/fastly/docs/cloudmanager/token',
-  '/fastly/docs/cloudmanager/service_id',
-  '/fastly/docs/atlas/token',
-  '/fastly/docs/atlas/service_id',
-  '/fastly/docs/opsmanager/token',
-  '/fastly/docs/opsmanager/service_id',
-  '/cdn/client/id',
-  '/cdn/client/secret',
-] as const;
-
 type WorkerSecureString = (typeof workerSecureStrings)[number];
 
 const workerParamPathToEnvName = new Map<WorkerSecureString, string>();
@@ -103,24 +37,6 @@ export async function getWorkerSecureStrings(ssmPrefix: string): Promise<Record<
   return getSecureStrings(ssmPrefix, workerSecureStrings, workerParamPathToEnvName, 'workerParamPathToEnvName');
 }
 
-// This array contains the Parameter Store paths of the secure strings
-// we want to add to the webhooks environment. These are mapped
-// to their environment variable name in the webhookParamPathToEnvName map.
-const webhookSecureStrings = [
-  '/github/bot/password',
-  '/github/webhook/secret',
-  '/github/webhook/deletionSecret',
-  '/atlas/password',
-  '/fastly/docs/dochub/token',
-  '/fastly/docs/dochub/service_id',
-  '/fastly/dochub_map',
-  '/cdn/client/id',
-  '/cdn/client/secret',
-  '/slack/webhook/secret',
-  '/slack/auth/token',
-  '/snooty/webhook/secret',
-] as const;
-
 type WebhookSecureString = (typeof webhookSecureStrings)[number];
 
 const webhookParamPathToEnvName = new Map<WebhookSecureString, string>();

diff --git a/src/enhanced/utils/filter-sensitive-values.ts b/src/enhanced/utils/filter-sensitive-values.ts
@@ -0,0 +1,11 @@
+import { sensitiveKeys } from './get-sensitive-values';
+
+export const filterSensitiveValues = (message: string) => {
+  for (const key of sensitiveKeys) {
+    if (message.includes(key)) {
+      message = message.replace(key, '*******');
+    }
+  }
+
+  return message;
+};
diff --git a/src/enhanced/utils/get-sensitive-values.ts b/src/enhanced/utils/get-sensitive-values.ts
@@ -0,0 +1,101 @@
+import { GetParameterCommand, SSMClient } from '@aws-sdk/client-ssm';
+
+// This array contains the Parameter Store paths of the secure strings
+// we want to add to the worker environment. These are mapped
+// to their environment variable name in the workerParamPathToEnvName map.
+export const workerSecureStrings = [
+  '/npm/auth',
+  '/github/webhook/secret',
+  '/github/bot/password',
+  '/atlas/password',
+  '/fastly/docs/dochub/token',
+  '/npm/auth',
+  '/fastly/docs/dochub/service_id',
+  '/fastly/dochub_map',
+  '/fastly/docs/main/token',
+  '/fastly/docs/main/service_id',
+  '/fastly/docs/cloudmanager/token',
+  '/fastly/docs/cloudmanager/service_id',
+  '/fastly/docs/atlas/token',
+  '/fastly/docs/atlas/service_id',
+  '/fastly/docs/opsmanager/token',
+  '/fastly/docs/opsmanager/service_id',
+  '/cdn/client/id',
+  '/cdn/client/secret',
+] as const;
+
+// This array contains the Parameter Store paths of the secure strings
+// we want to add to the webhooks environment. These are mapped
+// to their environment variable name in the webhookParamPathToEnvName map.
+export const webhookSecureStrings = [
+  '/github/bot/password',
+  '/github/webhook/secret',
+  '/github/webhook/deletionSecret',
+  '/atlas/password',
+  '/fastly/docs/dochub/token',
+  '/fastly/docs/dochub/service_id',
+  '/fastly/dochub_map',
+  '/cdn/client/id',
+  '/cdn/client/secret',
+  '/slack/webhook/secret',
+  '/slack/auth/token',
+  '/snooty/webhook/secret',
+] as const;
+
+/**
+ * Used to capture the sensitive values and store them
+ * as sensitive keys, to later be filtered in the Logger
+ */
+export const sensitiveKeys: string[] = [];
+
+/**
+ * Returns the secure strings from SSM using the SSM client.
+ * @param ssmPrefix the path prefix that should contain the environment for the SSM strings (e.g. /env/dotcomstg/worker_pool/)
+ * @returns The map of environment variables.
+ */
+export async function getSecureStrings(
+  ssmPrefix: string,
+  secureStrings: readonly string[],
+  paramToEnvMap: Map<string, string>,
+  resourceName: string
+) {
+  const ssmClient = new SSMClient({ region: process.env.CDK_DEFAULT_REGION });
+
+  const secureStringsMap: Record<string, string> = {};
+
+  await Promise.all(
+    secureStrings.map(async (paramName: string) => {
+      const getParamCommand = new GetParameterCommand({
+        Name: `${ssmPrefix}${paramName}`,
+        WithDecryption: true,
+      });
+
+      const ssmResponse = await ssmClient.send(getParamCommand);
+      const secureString = ssmResponse.Parameter?.Value;
+
+      if (!secureString) {
+        console.error(`ERROR! Could not retrieve string for the following param: ${paramName}`);
+        return;
+      }
+
+      /**
+       * Hijacks the getSecureString to populate the
+       * sensitive key array, used in the filterSensitiveValue
+       */
+      sensitiveKeys.push(secureString);
+
+      const envName = paramToEnvMap.get(paramName);
+
+      if (!envName) {
+        console.error(
+          `ERROR! The param '${paramName}' does not have a mapping to an environment variable name. Please define this in the ${resourceName} map.`
+        );
+        return;
+      }
+
+      secureStringsMap[envName] = secureString;
+    })
+  );
+
+  return secureStringsMap;
+}
diff --git a/src/services/logger.ts b/src/services/logger.ts
@@ -1,4 +1,5 @@
 import { JobRepository } from '../repositories/jobRepository';
+import { filterSensitiveValues } from '../enhanced/utils/filter-sensitive-values';
 
 export interface ILogger {
   info(contextId: string, message: string): void;
@@ -11,14 +12,17 @@ export interface IJobRepoLogger extends ILogger {
 }
 
 export class ConsoleLogger implements ILogger {
+  private filterMessage(message: string) {
+    return filterSensitiveValues(message);
+  }
   info(contextId: string, message: string): void {
-    console.info(`Context: ${contextId} message: ${message}`);
+    console.info(`Context: ${contextId} message: ${this.filterMessage(message)}`);
   }
   warn(contextId: string, message: string): void {
-    console.warn(`Context: ${contextId} message: ${message}`);
+    console.warn(`Context: ${contextId} message: ${this.filterMessage(message)}`);
   }
   error(contextId: string, message: string): void {
-    console.error(`Context: ${contextId} message: ${message}`);
+    console.error(`Context: ${contextId} message: ${this.filterMessage(message)}`);
   }
 }
 
@@ -29,6 +33,7 @@ export class HybridJobLogger extends ConsoleLogger implements IJobRepoLogger {
     this._jobRepo = jobRepo;
   }
   async save(contextId: string, message: string): Promise<void> {
+    message = filterSensitiveValues(message);
     try {
       this.info(contextId, message);
       await this._jobRepo.insertLogStatement(contextId, [message]);

diff --git a/tests/unit/services/hybridJobLogger.test.ts b/tests/unit/services/hybridJobLogger.test.ts
@@ -1,5 +1,6 @@
 import { HybridJobLogger } from '../../../src/services/logger';
 import { JobRepository } from '../../../src/repositories/jobRepository';
+import { sensitiveKeys } from '../../../src/enhanced/utils/get-sensitive-values';
 import { mockDeep } from 'jest-mock-extended';
 
 describe('HybridJobLogger Tests', () => {
@@ -62,4 +63,14 @@ describe('HybridJobLogger Tests', () => {
       expect(jobRepo.insertLogStatement.mock.calls).toHaveLength(1);
     });
   });
+
+  describe('HybridJobLogger Redact Test', () => {
+    test('HybridJobLogger redacts secrets from the message', async () => {
+      sensitiveKeys.push('my_secret');
+      await hybridJobLogger.save('SensitiveKeyContext', 'Oops I just logged my_secret');
+      expect(console.info).toBeCalledWith('Context: SensitiveKeyContext message: Oops I just logged *******');
+      expect(console.info.mock.calls).toHaveLength(1);
+      expect(jobRepo.insertLogStatement.mock.calls).toHaveLength(1);
+    });
+  });
 });