Skip to content

Commit

Permalink
feat(APIM-344): handle sensitive details in log messages (#324)
Browse files Browse the repository at this point in the history
### Introduction
Add functionality to remove sensitive data before writing it to the
logs.

### Resolution
- Redact errors in PinoLogger logs by unsetting some attributes in
error.
- Functionality copied from TFS-API. PinoLogger config gets new
attribute `redact`.
- Bootstrap might have errors with DB domain. Bootstrap errors are
solved by using custom logger in src/main.ts
    - Functionality is done using custom logger ConsoleLoggerWithRedact
  - Redact errors in TypeORM error message strings.
- Error strings are modified using PinoLogger hook logMethod. Hook is
set in main.module.ts.
    - This is done using helper function redactStringsInLogArgs.

### Miscellaneous 
- Changed how Logger is loaded in the services. Now they use dependency
injection.
- New environment variable `REDACT_LOGS`

### Todo
- Try catch could be removed from services, but default global exception
handler logs smaller error dump.
  • Loading branch information
avaitonis committed Jul 26, 2023
2 parents b1511d8 + 3bf0ec4 commit 2e5a948
Show file tree
Hide file tree
Showing 36 changed files with 759 additions and 51 deletions.
1 change: 1 addition & 0 deletions .env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
PORT=

LOG_LEVEL=debug
REDACT_LOGS=false

# Swagger
SWAGGER_USER=
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/deployment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ jobs:
"PORT=${{ secrets.PORT }}" \
"NODE_ENV=${{ secrets.NODE_ENV }}" \
"LOG_LEVEL=${{ secrets.LOG_LEVEL }}" \
"REDACT_LOGS=${{ secrets.REDACT_LOGS }}" \
"TZ=${{ secrets.TZ }}" \
"SWAGGER_USER=${{ secrets.SWAGGER_USER }}" \
"SWAGGER_PASSWORD=${{ secrets.SWAGGER_PASSWORD }}" \
Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ services:
PORT:
NODE_ENV:
LOG_LEVEL:
REDACT_LOGS:
TZ:
SWAGGER_USER:
SWAGGER_PASSWORD:
Expand Down
44 changes: 26 additions & 18 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"date-fns": "^2.30.0",
"dotenv": "^16.3.1",
"express-basic-auth": "^1.2.1",
"lodash": "^4.17.21",
"mssql": "^9.1.1",
"nestjs-pino": "^3.3.0",
"passport": "^0.6.0",
Expand All @@ -61,6 +62,7 @@
"@types/compression": "^1.7.2",
"@types/express": "^4.17.17",
"@types/jest": "^29.5.3",
"@types/lodash": "^4.14.195",
"@types/node": "^20.4.4",
"@types/supertest": "^2.0.12",
"@typescript-eslint/eslint-plugin": "^6.2.0",
Expand Down
135 changes: 135 additions & 0 deletions src/config/app.config.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import { RandomValueGenerator } from '@ukef-test/support/generator/random-value-generator';

import appConfig from './app.config';
import { InvalidConfigException } from './invalid-config.exception';

describe('appConfig', () => {
const valueGenerator = new RandomValueGenerator();

let originalProcessEnv: NodeJS.ProcessEnv;

beforeEach(() => {
originalProcessEnv = process.env;
});

afterEach(() => {
process.env = originalProcessEnv;
});

describe('parsing LOG_LEVEL', () => {
it('throws an InvalidConfigException if LOG_LEVEL is specified but is not a valid log level', () => {
replaceEnvironmentVariables({
LOG_LEVEL: 'not-a-real-log-level',
});

const gettingTheAppConfig = () => appConfig();

expect(gettingTheAppConfig).toThrow(InvalidConfigException);
expect(gettingTheAppConfig).toThrow(`LOG_LEVEL must be one of fatal,error,warn,info,debug,trace,silent or not specified.`);
});

it('uses info as the logLevel if LOG_LEVEL is not specified', () => {
replaceEnvironmentVariables({});

const config = appConfig();

expect(config.logLevel).toBe('info');
});

it('uses info as the logLevel if LOG_LEVEL is empty', () => {
replaceEnvironmentVariables({
LOG_LEVEL: '',
});

const config = appConfig();

expect(config.logLevel).toBe('info');
});

it.each([
{
LOG_LEVEL: 'fatal',
},
{
LOG_LEVEL: 'error',
},
{
LOG_LEVEL: 'warn',
},
{
LOG_LEVEL: 'info',
},
{
LOG_LEVEL: 'debug',
},
{
LOG_LEVEL: 'trace',
},
{
LOG_LEVEL: 'silent',
},
])('uses LOG_LEVEL as the logLevel if LOG_LEVEL is valid ($LOG_LEVEL)', ({ LOG_LEVEL }) => {
replaceEnvironmentVariables({
LOG_LEVEL,
});

const config = appConfig();

expect(config.logLevel).toBe(LOG_LEVEL);
});
});

describe('parsing REDACT_LOGS', () => {
it('sets redactLogs to true if REDACT_LOGS is true', () => {
replaceEnvironmentVariables({
REDACT_LOGS: 'true',
});

const config = appConfig();

expect(config.redactLogs).toBe(true);
});

it('sets redactLogs to false if REDACT_LOGS is false', () => {
replaceEnvironmentVariables({
REDACT_LOGS: 'false',
});

const config = appConfig();

expect(config.redactLogs).toBe(false);
});

it('sets redactLogs to true if REDACT_LOGS is not specified', () => {
replaceEnvironmentVariables({});

const config = appConfig();

expect(config.redactLogs).toBe(true);
});

it('sets redactLogs to true if REDACT_LOGS is the empty string', () => {
replaceEnvironmentVariables({
REDACT_LOGS: '',
});

const config = appConfig();

expect(config.redactLogs).toBe(true);
});

it('sets redactLogs to true if REDACT_LOGS is any string other than true or false', () => {
replaceEnvironmentVariables({
REDACT_LOGS: valueGenerator.string(),
});

const config = appConfig();

expect(config.redactLogs).toBe(true);
});
});

const replaceEnvironmentVariables = (newEnvVariables: Record<string, string>): void => {
process.env = newEnvVariables;
};
});
3 changes: 2 additions & 1 deletion src/config/app.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export default registerAs('app', (): Record<string, any> => {
env: process.env.APP_ENV || 'development',

versioning: {
enable: process.env.HTTP_VERSIONING_ENABLE === 'true' || false,
enable: process.env.HTTP_VERSIONING_ENABLE === 'true',
prefix: 'v',
version: process.env.HTTP_VERSION || '1',
},
Expand All @@ -23,5 +23,6 @@ export default registerAs('app', (): Record<string, any> => {
port: process.env.HTTP_PORT ? Number.parseInt(process.env.HTTP_PORT, 10) : 3003,
apiKey: process.env.API_KEY,
logLevel: process.env.LOG_LEVEL || 'info',
redactLogs: process.env.REDACT_LOGS !== 'false',
};
});
4 changes: 4 additions & 0 deletions src/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* 3. Auth strategy
* 4. Products
* 5. Customers
* 6. Strings to redact
* 7. Strings locations to redact
*/

export * from './auth.constant';
Expand All @@ -15,4 +17,6 @@ export * from './database-name.constant';
export * from './date.constant';
export * from './enums';
export * from './products.constant';
export * from './redact-strings.constant';
export * from './redact-strings-paths.constant';
export * from './ukef-id.constant';
Loading

0 comments on commit 2e5a948

Please sign in to comment.