Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-8vg2-wf3q-mwv7
* fix(api): redact header cookie

This is a quick PoC for a fix. I am not sure if it's the best answer and have not added tests yet. If we feel good about it, I can add tests and open a PR. Please let me know how you'd like to proceed!

* cleaner

* rework to handle multiple inputs and add unit tests

* Added same redacting logic for teh response set-cookie

---------

Co-authored-by: Brainslug <tim@brainslug.nl>
Co-authored-by: Brainslug <br41nslug@users.noreply.github.com>
  • Loading branch information
3 people committed Mar 23, 2023
1 parent 59f965c commit 3495363
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 2 deletions.
20 changes: 18 additions & 2 deletions api/src/logger.ts
@@ -1,11 +1,12 @@
import { toArray } from '@directus/shared/utils';
import type { Request, RequestHandler } from 'express';
import { merge } from 'lodash';
import pino, { LoggerOptions } from 'pino';
import pino, { LoggerOptions, SerializedResponse } from 'pino';
import type { Request, RequestHandler } from 'express';
import pinoHTTP, { stdSerializers } from 'pino-http';
import { URL } from 'url';
import env from './env';
import { getConfigFromEnv } from './utils/get-config-from-env';
import { redactHeaderCookie } from './utils/redact-header-cookies';

const pinoOptions: LoggerOptions = {
level: env.LOG_LEVEL || 'info',
Expand Down Expand Up @@ -86,8 +87,23 @@ export const expressLogger = pinoHTTP({
req(request: Request) {
const output = stdSerializers.req(request);
output.url = redactQuery(output.url);
if (output.headers?.cookie) {
output.headers.cookie = redactHeaderCookie(output.headers.cookie, [
'access_token',
`${env.REFRESH_TOKEN_COOKIE_NAME}`,
]);
}
return output;
},
res(response: SerializedResponse) {
if (response.headers?.['set-cookie']) {
response.headers['set-cookie'] = redactHeaderCookie(response.headers['set-cookie'], [
'access_token',
`${env.REFRESH_TOKEN_COOKIE_NAME}`,
]);
}
return response;
},
},
}) as RequestHandler;

Expand Down
42 changes: 42 additions & 0 deletions api/src/utils/redact-header-cookies.test.ts
@@ -0,0 +1,42 @@
import { describe, expect, test } from 'vitest';
import env from '../env';
import { redactHeaderCookie } from './redact-header-cookies';

describe('redactHeaderCookie', () => {
describe('Given auth cookies', () => {
test('When it finds a refresh_token, it should redact the value', () => {
const tokenKey = env.REFRESH_TOKEN_COOKIE_NAME;
const cookieHeader = `${tokenKey}=shh;`;
const cookieNames = [`${tokenKey}`];

const redactedCookie = redactHeaderCookie(cookieHeader, cookieNames);
expect(redactedCookie).toBe(`${tokenKey}=--redacted--;`);
});
test('When it finds an access_token, it should redact the value', () => {
const tokenKey = 'access_token';
const cookieHeader = `${tokenKey}=secret;`;
const cookieNames = [`${tokenKey}`];

const redactedCookie = redactHeaderCookie(cookieHeader, cookieNames);
expect(redactedCookie).toBe(`${tokenKey}=--redacted--;`);
});
test('When it finds both an access_token and refresh_token, it should redact both values', () => {
const cookieHeader = `access_token=secret; ${env.REFRESH_TOKEN_COOKIE_NAME}=shhhhhhh; randomCookie=Erdtree;`;
const cookieNames = ['access_token', `${env.REFRESH_TOKEN_COOKIE_NAME}`];

const redactedCookie = redactHeaderCookie(cookieHeader, cookieNames);
expect(redactedCookie).toBe(
`access_token=--redacted--; ${env.REFRESH_TOKEN_COOKIE_NAME}=--redacted--; randomCookie=Erdtree;`
);
});
});
describe('Given negligible cookies', () => {
test('It should return the orignal value', () => {
const originalCookie = `Crown=Swords; Hail=Sithis;`;
const cookieNames = [env.REFRESH_TOKEN_COOKIE_NAME, 'access_token'];

const redactedCookie = redactHeaderCookie(originalCookie, cookieNames);
expect(redactedCookie).toBe(originalCookie);
});
});
});
7 changes: 7 additions & 0 deletions api/src/utils/redact-header-cookies.ts
@@ -0,0 +1,7 @@
export function redactHeaderCookie(cookieHeader: string, cookieNames: string[]) {
for (const cookieName of cookieNames) {
const re = new RegExp(`(${cookieName}=)([^;]+)`);
cookieHeader = cookieHeader.replace(re, `$1--redacted--`);
}
return cookieHeader;
}

0 comments on commit 3495363

Please sign in to comment.