Skip to content

Commit

Permalink
fix: redact sensitive flag values from analytics
Browse files Browse the repository at this point in the history
Flag values that contain secrets should not be posted to our analytics
endpoint.

Users of this code who introduce new sensitive flags would have to know
it exists and add their flag there, but this is a starting point that
doesn't involve overhauling our flag parsing.
  • Loading branch information
Craig Furman committed Mar 16, 2022
1 parent c7981d8 commit 33c8c49
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 0 deletions.
13 changes: 13 additions & 0 deletions src/lib/analytics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ import { makeRequest } from '../request';
import { config as userConfig } from '../user-config';
import { getStandardData } from './getStandardData';

// Add flags whose values should be redacted in analytics here.
// TODO make this less error-prone by baking the concept of sensitivity into the
// flag-parsing code, but this is a start.
const sensitiveFlags = ['tfc-token'];

const debug = createDebug('snyk');
const metadata = {};
// analytics module is required at the beginning of the CLI run cycle
Expand All @@ -27,6 +32,14 @@ export function addDataAndSend(
if (Array.isArray(data.args)) {
// this is an overhang from the cli/args.js and we don't want it
delete (data.args.slice(-1).pop() || {})._;

data.args.forEach((argObj) => {
Object.keys(argObj).forEach((field) => {
if (sensitiveFlags.includes(field)) {
argObj[field] = 'REDACTED';
}
});
});
}

if (Object.keys(metadata).length) {
Expand Down
19 changes: 19 additions & 0 deletions test/jest/unit/lib/analytics/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,25 @@ describe('analytics module', () => {
);
});

it('removes sensitive flags', async () => {
const requestSpy = jest.spyOn(request, 'makeRequest');
requestSpy.mockResolvedValue();

await analytics.addDataAndSend({
args: argsFrom({
'tfc-endpoint': "I don't care who sees this",
'tfc-token': 'itsasecret',
}),
});

expect(requestSpy.mock.calls[0][0].body.data).toHaveProperty('args', [
{
'tfc-endpoint': "I don't care who sees this",
'tfc-token': 'REDACTED',
},
]);
});

it('ignores analytics request failures', async () => {
const requestSpy = jest.spyOn(request, 'makeRequest');
requestSpy.mockRejectedValue(new Error('this should be ignored'));
Expand Down

0 comments on commit 33c8c49

Please sign in to comment.