Skip to content

Commit

Permalink
fix(node): Remove Cookie header from requestdata.headers if cookies s…
Browse files Browse the repository at this point in the history
…hould not be sent to Sentry (#5898)

When using the `Sentry.requestMiddleware` in Node (Express), users have the option to define which properties of the incoming requests should be added to the event that is sent to Sentry. Among other options, they can set

* `headers` to add the request's http headers to `event.request.headers`
* `cookies` to add the request's cookies to `event.request.cookies`

However, there is a problem if `cookies` is not specified but `headers` is: The `Cookie` http header of the incoming request is still added to `event.request.headers` as it is treated like a normal http header. Therefore, cookie data (potentially containing sensitive data) is added to the event although users expect it to not be added.

This patch this behaviour by removing the `Cookie` http header from the collected headers, if `cookies` is not specified in the requestHandler options but `headers` is. Additionally, it adds two tests to check this new behaviour.

fixes #5458
  • Loading branch information
Lms24 committed Oct 6, 2022
1 parent a1d5398 commit 458374a
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 0 deletions.
6 changes: 6 additions & 0 deletions packages/node/src/requestdata.ts
Expand Up @@ -174,6 +174,12 @@ export function extractRequestData(
switch (key) {
case 'headers': {
requestData.headers = headers;

// Remove the Cookie header in case cookie data should not be included in the event
if (!include.includes('cookies')) {
delete (requestData.headers as { cookie?: string }).cookie;
}

break;
}
case 'method': {
Expand Down
31 changes: 31 additions & 0 deletions packages/node/test/requestdata.test.ts
Expand Up @@ -298,6 +298,37 @@ describe.each([oldExtractRequestData, newExtractRequestData])(
});
});

describe('headers', () => {
it('removes the `Cookie` header from requestdata.headers, if `cookies` is not set in the options', () => {
const mockReq = {
cookies: { foo: 'bar' },
headers: { cookie: 'foo=bar', otherHeader: 'hello' },
};
const optionsWithCookies = ['headers'];

const [req, options] = formatArgs(fn, mockReq, optionsWithCookies);

expect(fn(req, options as any)).toStrictEqual({
headers: { otherHeader: 'hello' },
});
});

it('includes the `Cookie` header in requestdata.headers, if `cookies` is not set in the options', () => {
const mockReq = {
cookies: { foo: 'bar' },
headers: { cookie: 'foo=bar', otherHeader: 'hello' },
};
const optionsWithCookies = ['headers', 'cookies'];

const [req, options] = formatArgs(fn, mockReq, optionsWithCookies);

expect(fn(req, options as any)).toStrictEqual({
headers: { otherHeader: 'hello', cookie: 'foo=bar' },
cookies: { foo: 'bar' },
});
});
});

describe('cookies', () => {
it('uses `req.cookies` if available', () => {
const mockReq = {
Expand Down

0 comments on commit 458374a

Please sign in to comment.