Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ DOP-4240 implements filtering sensitive values method to prevent sensitive values being logged #1012

Merged
merged 3 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 5 additions & 89 deletions cdk-infra/utils/ssm.ts
Original file line number Diff line number Diff line change
@@ -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>();
Expand All @@ -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>();
Expand Down
11 changes: 11 additions & 0 deletions src/enhanced/utils/filter-sensitive-values.ts
Original file line number Diff line number Diff line change
@@ -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;
};
101 changes: 101 additions & 0 deletions src/enhanced/utils/get-sensitive-values.ts
Original file line number Diff line number Diff line change
@@ -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;
}
11 changes: 8 additions & 3 deletions src/services/logger.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)}`);
}
}

Expand All @@ -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]);
Expand Down
11 changes: 11 additions & 0 deletions tests/unit/services/hybridJobLogger.test.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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);
});
});
});
Loading