Skip to content

Commit

Permalink
feat: RD-13971-Masking-does-not-work-for-outbound-JSON-requests (#523)
Browse files Browse the repository at this point in the history
* fix: logging improvments,

* fix: scrub should work for JSON payloads without content-type

---------

Co-authored-by: Eugene Orlovsky <[email protected]>
Co-authored-by: Harel Moshe <[email protected]>
  • Loading branch information
3 people authored Nov 6, 2024
1 parent ae2b58d commit 112600d
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 4 deletions.
86 changes: 86 additions & 0 deletions src/reporter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1291,6 +1291,92 @@ describe('reporter', () => {
shouldScrubDomainMock.mockReset();
});

test('scrubSpans should scrub without content-type', () => {
const spanWithoutSecrets = {
id: '1',
info: { httpInfo: { request: {}, response: { body: 'body' } } },
};
const beforeScrub = [
spanWithoutSecrets,
{
id: '2',
info: {
httpInfo: {
request: {
headers: {},
body: JSON.stringify({ secret: '1234' }),
},
response: {},
},
},
},
{
id: '3',
info: {
httpInfo: {
request: {},
response: {
headers: { 'content-type': 'json' },
body: JSON.stringify({ secret: '1234' }),
},
},
},
},
{
id: '4',
info: {
httpInfo: {
request: {
headers: {},
body: '{"secret":""...}',
},
response: {},
},
},
},
];
const scrubbed = [
spanWithoutSecrets,
{
id: '2',
info: {
httpInfo: {
request: {
headers: '{}',
body: '{"secret":"****"}',
},
response: {},
},
},
},
{
id: '3',
info: {
httpInfo: {
request: {},
response: {
headers: '{"content-type":"json"}',
body: '{"secret":"****"}',
},
},
},
},
{
id: '4',
info: {
httpInfo: {
request: {
headers: '{}',
body: '"{\\"secret\\":\\"\\"...}"',
},
response: {},
},
},
},
];
expect(scrubSpans(beforeScrub)).toEqual(scrubbed);
});

test(`sendSpans -> handle errors in forgeAndScrubRequestBody`, async () => {
setDebug();
// @ts-ignore
Expand Down
22 changes: 21 additions & 1 deletion src/reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,27 @@ export const sendSpans = async (spans: any[], addEnrichmentSpan: boolean = true)
};

const isJsonContent = (payload: any, headers: Object) => {
return isString(payload) && headers['content-type'] && headers['content-type'].includes('json');
const isJsonString = (str: any) => {
try {
JSON.parse(str);
return true;
} catch (e) {
return false;
}
};
const isJsonObjectOrArray = (str: string) => {
const trimmed = str.trim();
return (
(trimmed.startsWith('{') && trimmed.endsWith('}')) ||
(trimmed.startsWith('[') && trimmed.endsWith(']'))
);
};

return (
isString(payload) &&
(headers['content-type']?.toLowerCase().includes('json') ||
(isJsonObjectOrArray(payload) && isJsonString(payload)))
);
};

function scrub(payload: any, headers: any, sizeLimit: number, truncated = false): string {
Expand Down
4 changes: 1 addition & 3 deletions src/utils/payloadStringify.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,10 @@ const invalidMaskingRegexWarning = runOneTimeWrapper((e) => {
});

const shallowMaskByRegex = (payload, regexes) => {
regexes = regexes || keyToOmitRegexes();
logSecretMaskingDebug(logger, 'Shallow masking payload by regexes', {
payloadKeys: Object.keys(payload),
regexes,
});
regexes = regexes || keyToOmitRegexes();
if (isString(payload)) {
return payload;
}
Expand All @@ -302,7 +301,6 @@ const shallowMaskByRegex = (payload, regexes) => {
export const shallowMask = (context, payload) => {
logSecretMaskingDebug(logger, 'Shallow masking payload', {
context,
payloadKeys: Object.keys(payload),
});
let givenSecretRegexes = null;
if (context === 'environment') {
Expand Down

0 comments on commit 112600d

Please sign in to comment.